From a6835d41a2435f208a7a11cb7b11dca634dbf28d Mon Sep 17 00:00:00 2001 From: ocornut Date: Fri, 3 Feb 2023 19:20:18 +0100 Subject: [PATCH] Tables: Solved an ID conflict issue with multiple-instances of a same table. Storing instance id for convenience. (#6140) TableGetColumnResizeID() are still using an incorrect table, but having only one-level left tends to cancel things out. --- docs/CHANGELOG.txt | 2 ++ imgui.h | 2 +- imgui_internal.h | 10 ++++++---- imgui_tables.cpp | 46 +++++++++++++++++++++++++++++++--------------- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index a79fa0ae2f55..87fd3e7b5164 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -59,6 +59,8 @@ All changes: - Tables: Raised max Columns count from 64 to 512. (#6094, #5305, #4876, #3572) The previous limit was due to using 64-bit integers but we moved to bits-array and tweaked the system enough to ensure no performance loss. +- Tables: Solved an ID conflict issue with multiple-instances of a same table, + due to how unique table instance id was generated. (#6140) [@ocornut, @rodrigorc] - Text: Fixed layouting of wrapped-text block skipping successive empty lines, regression from the fix in 1.89.2. (#5720, #5919) - Text: Fixed clipping of single-character "..." ellipsis (U+2026 or U+0085) when font diff --git a/imgui.h b/imgui.h index 39879eea5bc1..3736a46112fc 100644 --- a/imgui.h +++ b/imgui.h @@ -23,7 +23,7 @@ // Library Version // (Integer encoded as XYYZZ for use in #if preprocessor conditionals, e.g. '#if IMGUI_VERSION_NUM > 12345') #define IMGUI_VERSION "1.89.3 WIP" -#define IMGUI_VERSION_NUM 18926 +#define IMGUI_VERSION_NUM 18927 #define IMGUI_HAS_TABLE /* diff --git a/imgui_internal.h b/imgui_internal.h index 571b1598663b..73512921fd5f 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -2509,14 +2509,15 @@ struct ImGuiTableCellData ImGuiTableColumnIdx Column; // Column number }; -// Per-instance data that needs preserving across frames (seemingly most others do not need to be preserved aside from debug needs, does that needs they could be moved to ImGuiTableTempData ?) +// Per-instance data that needs preserving across frames (seemingly most others do not need to be preserved aside from debug needs. Does that means they could be moved to ImGuiTableTempData?) struct ImGuiTableInstanceData { + ImGuiID TableInstanceID; float LastOuterHeight; // Outer height from last frame float LastFirstRowHeight; // Height of first row from last frame (FIXME: this is used as "header height" and may be reworked) float LastFrozenHeight; // Height of frozen section from last frame - ImGuiTableInstanceData() { LastOuterHeight = LastFirstRowHeight = LastFrozenHeight = 0.0f; } + ImGuiTableInstanceData() { TableInstanceID = 0; LastOuterHeight = LastFirstRowHeight = LastFrozenHeight = 0.0f; } }; // FIXME-TABLE: more transient data could be stored in a stacked ImGuiTableTempData: e.g. SortSpecs, incoming RowData @@ -2996,7 +2997,8 @@ namespace ImGui IMGUI_API void TableDrawContextMenu(ImGuiTable* table); IMGUI_API bool TableBeginContextMenuPopup(ImGuiTable* table); IMGUI_API void TableMergeDrawChannels(ImGuiTable* table); - inline ImGuiTableInstanceData* TableGetInstanceData(ImGuiTable* table, int instance_no) { if (instance_no == 0) return &table->InstanceDataFirst; return &table->InstanceDataExtra[instance_no - 1]; } + inline ImGuiTableInstanceData* TableGetInstanceData(ImGuiTable* table, int instance_no) { if (instance_no == 0) return &table->InstanceDataFirst; return &table->InstanceDataExtra[instance_no - 1]; } + inline ImGuiID TableGetInstanceID(ImGuiTable* table, int instance_no) { return TableGetInstanceData(table, instance_no)->TableInstanceID; } IMGUI_API void TableSortSpecsSanitize(ImGuiTable* table); IMGUI_API void TableSortSpecsBuild(ImGuiTable* table); IMGUI_API ImGuiSortDirection TableGetColumnNextSortDirection(ImGuiTableColumn* column); @@ -3008,7 +3010,7 @@ namespace ImGui IMGUI_API void TableEndCell(ImGuiTable* table); IMGUI_API ImRect TableGetCellBgRect(const ImGuiTable* table, int column_n); IMGUI_API const char* TableGetColumnName(const ImGuiTable* table, int column_n); - IMGUI_API ImGuiID TableGetColumnResizeID(const ImGuiTable* table, int column_n, int instance_no = 0); + IMGUI_API ImGuiID TableGetColumnResizeID(ImGuiTable* table, int column_n, int instance_no = 0); IMGUI_API float TableGetMaxColumnWidth(const ImGuiTable* table, int column_n); IMGUI_API void TableSetColumnWidthAutoSingle(ImGuiTable* table, int column_n); IMGUI_API void TableSetColumnWidthAutoAll(ImGuiTable* table); diff --git a/imgui_tables.cpp b/imgui_tables.cpp index 89fe8a732416..8452c06e27c4 100644 --- a/imgui_tables.cpp +++ b/imgui_tables.cpp @@ -332,11 +332,7 @@ bool ImGui::BeginTableEx(const char* name, ImGuiID id, int columns_count, ImG // Acquire storage for the table ImGuiTable* table = g.Tables.GetOrAddByKey(id); - const int instance_no = (table->LastFrameActive != g.FrameCount) ? 0 : table->InstanceCurrent + 1; - const ImGuiID instance_id = id + instance_no; const ImGuiTableFlags table_last_flags = table->Flags; - if (instance_no > 0) - IM_ASSERT(table->ColumnsCount == columns_count && "BeginTable(): Cannot change columns count mid-frame while preserving same ID"); // Acquire temporary buffers const int table_idx = g.Tables.GetIndex(table); @@ -352,17 +348,34 @@ bool ImGui::BeginTableEx(const char* name, ImGuiID id, int columns_count, ImG flags = TableFixFlags(flags, outer_window); // Initialize + const int instance_no = (table->LastFrameActive != g.FrameCount) ? 0 : table->InstanceCurrent + 1; table->ID = id; table->Flags = flags; - table->InstanceCurrent = (ImS16)instance_no; table->LastFrameActive = g.FrameCount; table->OuterWindow = table->InnerWindow = outer_window; table->ColumnsCount = columns_count; table->IsLayoutLocked = false; table->InnerWidth = inner_width; temp_data->UserOuterSize = outer_size; - if (instance_no > 0 && table->InstanceDataExtra.Size < instance_no) - table->InstanceDataExtra.push_back(ImGuiTableInstanceData()); + + // Instance data (for instance 0, TableID == TableInstanceID) + ImGuiID instance_id; + table->InstanceCurrent = (ImS16)instance_no; + if (instance_no > 0) + { + IM_ASSERT(table->ColumnsCount == columns_count && "BeginTable(): Cannot change columns count mid-frame while preserving same ID"); + if (table->InstanceDataExtra.Size < instance_no) + table->InstanceDataExtra.push_back(ImGuiTableInstanceData()); + char instance_desc[12]; + int instance_desc_len = ImFormatString(instance_desc, IM_ARRAYSIZE(instance_desc), "##Instance%d", instance_no); + instance_id = GetIDWithSeed(instance_desc, instance_desc + instance_desc_len, id); + } + else + { + instance_id = id; + } + ImGuiTableInstanceData* table_instance = TableGetInstanceData(table, table->InstanceCurrent); + table_instance->TableInstanceID = instance_id; // When not using a child window, WorkRect.Max will grow as we append contents. if (use_child_window) @@ -412,7 +425,9 @@ bool ImGui::BeginTableEx(const char* name, ImGuiID id, int columns_count, ImG } // Push a standardized ID for both child-using and not-child-using tables - PushOverrideID(instance_id); + PushOverrideID(id); + if (instance_no > 0) + PushOverrideID(instance_id); // FIXME: Somehow this is not resolved by stack-tool, even tho GetIDWithSeed() submitted the symbol. // Backup a copy of host window members we will modify ImGuiWindow* inner_window = table->InnerWindow; @@ -1348,8 +1363,10 @@ void ImGui::EndTable() } // Pop from id stack - IM_ASSERT_USER_ERROR(inner_window->IDStack.back() == table->ID + table->InstanceCurrent, "Mismatching PushID/PopID!"); + IM_ASSERT_USER_ERROR(inner_window->IDStack.back() == table_instance->TableInstanceID, "Mismatching PushID/PopID!"); IM_ASSERT_USER_ERROR(outer_window->DC.ItemWidthStack.Size >= temp_data->HostBackupItemWidthStackSize, "Too many PopItemWidth!"); + if (table->InstanceCurrent > 0) + PopID(); PopID(); // Restore window data that we modified @@ -1619,11 +1636,11 @@ ImRect ImGui::TableGetCellBgRect(const ImGuiTable* table, int column_n) } // Return the resizing ID for the right-side of the given column. -ImGuiID ImGui::TableGetColumnResizeID(const ImGuiTable* table, int column_n, int instance_no) +ImGuiID ImGui::TableGetColumnResizeID(ImGuiTable* table, int column_n, int instance_no) { IM_ASSERT(column_n >= 0 && column_n < table->ColumnsCount); - ImGuiID id = table->ID + 1 + (instance_no * table->ColumnsCount) + column_n; - return id; + ImGuiID instance_id = TableGetInstanceID(table, instance_no); + return instance_id + 1 + column_n; // FIXME: #6140: still not ideal } // Return -1 when table is not hovered. return columns_count if the unused space at the right of visible columns is hovered. @@ -2878,10 +2895,9 @@ void ImGui::TableHeadersRow() continue; // Push an id to allow unnamed labels (generally accidental, but let's behave nicely with them) - // - in your own code you may omit the PushID/PopID all-together, provided you know they won't collide - // - table->InstanceCurrent is only >0 when we use multiple BeginTable/EndTable calls with same identifier. + // In your own code you may omit the PushID/PopID all-together, provided you know they won't collide. const char* name = (TableGetColumnFlags(column_n) & ImGuiTableColumnFlags_NoHeaderLabel) ? "" : TableGetColumnName(column_n); - PushID(table->InstanceCurrent * table->ColumnsCount + column_n); + PushID(column_n); TableHeader(name); PopID(); }