Increase maximum table columns from 64 to 320

[rolandhill]

Increase the maximum number of columns in tables from 64 to 320.

This PR is developed from a patch file originally contributed by @Necrolis in issue #3572.

I've been applying these changes to various versions of ImGui on Kubuntu for approximately 2 years and have had no issues with tables larger than 64 columns. It works well combined with the clipper.

[ocornut]

Hello and thanks for the PR. This seems indeed like a tweaking of #3572.

As mentioned in that issue, I'm not sure about the added cost of this, increasing mask size from 32 to 160 bytes spreading those fields over more cache-lines to handle a rare scenario, while keeping an arbitrary compile-time limit. Going to add try adding more formal perf tests in the Test Suite, namely I think we should go for all 4 combinations of table with few/many columns/rows and compare perfs of all fours with/without the change.

I want to see if I can achieve the best of both worlds and fully lift lifting by allocating those via the ImSpanAllocator in TableBeginInitMemory() and massaging the accessors/syntax so it is easy and fast to use. Which would become an issue for TableMergeDrawChannels() but we could store reusable/amortized buffers ImGuiTableTempData. Since a majority of uses are in a same function TableUpdateLayout() it would be acceptable if we need to recompute their address via a helper.

I think I've got a base good enough with all our PR to attempt a rework.

I'm also going to probably remove table->RequestOutputMaskByIndex now it seems like a mistake since it's used from code-paths that are already reading from the large- Column structure.

[ocornut]

For references, I applied this PR (over db55422 which should have benefited it slightly)

Debug builds performances:

Release builds performances

With this code:

// Shared function to test basic table performances.
struct TablePerfFuncVars { int TablesCount = 1; int RowsCount = 50; int ColumnsCount = 3; };
auto TablePerfFunc = [](ImGuiTestContext* ctx)
{
    auto& vars = ctx->GetVars<TablePerfFuncVars>();
    ImGui::SetNextWindowSize(ImVec2(800, 800));
    ImGui::Begin("Test Func", NULL, ImGuiWindowFlags_NoSavedSettings | ImGuiWindowFlags_AlwaysAutoResize);

    const int tables_count = vars.TablesCount;
    const int columns_count = vars.ColumnsCount;
    const int rows_count = vars.RowsCount;
    if (ctx->IsFirstGuiFrame())
        ctx->LogDebug("%d tables x %d columns x %d rows", tables_count, columns_count, rows_count);
    for (int n = 0; n < tables_count; n++)
    {
        ImGui::PushID(n);
        if (ImGui::BeginTable("table1", columns_count, ImGuiTableFlags_BordersV))
        {
            for (int row = 0; row < rows_count; row++)
            {
                for (int col = 0; col < columns_count; col++)
                {
                    ImGui::TableNextColumn();
                    if (col == 0)
                        ImGui::Text("Cell 0,%d", n);
                    else if (col == 1)
                        ImGui::TextUnformatted("Cell 1");
                    else if (col == 2)
                        ImGui::TextUnformatted("Cell 2");
                    // No output on further columns to not interfere with performances more than necessary
                }
            }
            ImGui::EndTable();
        }
        ImGui::Separator();
        ImGui::PopID();
    }
    ImGui::End();
};

// ## Measure the cost of TableNextCell(), TableNextRow(): one table, few columns, many rows
t = IM_REGISTER_TEST(e, "perf", "perf_tables_basic_tables_lo_cols_lo_rows_hi");
t->SetVarsDataType<TablePerfFuncVars>([](ImGuiTestContext* ctx, auto& vars)
    {
        vars.TablesCount = 1;
        vars.ColumnsCount = 3;
        vars.RowsCount = 100 * ctx->PerfStressAmount;
    });
t->GuiFunc = TablePerfFunc;
t->TestFunc = PerfCaptureFunc;

// ## Measure the cost of BeginTable(): many tables, few columns, few rows
t = IM_REGISTER_TEST(e, "perf", "perf_tables_basic_tables_hi_cols_lo_rows_lo");
t->SetVarsDataType<TablePerfFuncVars>([](ImGuiTestContext* ctx, auto& vars)
    {
        vars.TablesCount = 30 * ctx->PerfStressAmount;
        vars.ColumnsCount = 3;
        vars.RowsCount = 3;
    });
t->GuiFunc = TablePerfFunc;
t->TestFunc = PerfCaptureFunc;

// ## Measure the cost of BeginTable(): many tables, many columns, few rows
t = IM_REGISTER_TEST(e, "perf", "perf_tables_basic_tables_hi_cols_hi_rows_lo");
t->SetVarsDataType<TablePerfFuncVars>([](ImGuiTestContext* ctx, auto& vars)
    {
        vars.TablesCount = 30 * ctx->PerfStressAmount;
        vars.ColumnsCount = 32;
        vars.RowsCount = 3;
    });
t->GuiFunc = TablePerfFunc;
t->TestFunc = PerfCaptureFunc;

// ## Measure the cost of BeginTable(): one table, many columns, many rows
t = IM_REGISTER_TEST(e, "perf", "perf_tables_basic_tables_lo_cols_hi_rows_hi");
t->SetVarsDataType<TablePerfFuncVars>([](ImGuiTestContext* ctx, auto& vars)
    {
        vars.TablesCount = 1;
        vars.ColumnsCount = 32;
        vars.RowsCount = 30 * ctx->PerfStressAmount;
    });
t->GuiFunc = TablePerfFunc;
t->TestFunc = PerfCaptureFunc;

[ocornut]

Out of curiosity I also tried with the patch before I removed RequestOutputMaskByIndex, so my intuition there was right that it would help this patch:

I'll dig further to try to lift the limit without increasing CPU cost.

[ocornut]

Pushed my version of this as db55422 + 14908cb + 9166743, closing #6094, #5305, #4876, #3572.
Columns maximum count now lifted to 512. Technically there's no hard limit anymore, 512 is only used for an assert we could decide to remove. But as clipping on horizontal axis works very differently from typical per-row clipping, don't expect a 512-columns tables to be as as performant, as lots of small fixed cost will accumulate. Still decently fast.

TL;DR;

• Removed RequestOutputMaskByIndex.
• Bit-arrays dynamically sizes. Allocating the bit-arrays in the shared per-table arena. There's one extra indirection but the data is compact and contiguous.
• Used macros in key locations. Bit fugly but helps ensuring good code-gen especially on debug builds.
• TableMergeDrawChannels() needs to use a shared temporary buffers as the bit-array are dynamically sized.

The four tests are now as fast as with previous code:

Debug

Release

Original PR (Debug) vs final solution (Debug)

Thanks everyone for your input on that!

[rolandhill]

Fantastic - thank you! I'll try it out today.

Roland