Tables: Fix invalid data in TableGetSortSpecs() when SpecsDirty flag …

…is unset. (#4233) Amend 4ce6bd8, but with usage of ImPool<> bug existed even before 4ce6bd8. Would only materialize if user called (ableGetSortSpecs and used data without checking SpecsDirty.
ocornut · Jun 17, 2021 · 7c44d06 · 7c44d06
1 parent d5828cd
commit 7c44d06
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 15 deletions.
diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt
@@ -43,6 +43,7 @@ Other Changes:
 - Tables: Added ImGuiTableColumnFlags_NoHeaderLabel to request TableHeadersRow() to not submit label for a column.
   Convenient for some small columns. Name will still appear in context menu. (#4206).
 - Tables: Fix columns order on TableSetupScrollFreeze() if previous data got frozen columns out of their section.
+- Tables: Fix invalid data in TableGetSortSpecs() when SpecsDirty flag is unset. (#4233)
 - Fixed printf-style format checks on non-MinGW flavors. (#4183, #3592)
 - Demo: Fixed requirement in 1.83 to link with imgui_demo.cpp if IMGUI_DISABLE_METRICS_WINDOW is not set. (#4171)
   Normally the right way to disable compiling the demo is to set IMGUI_DISABLE_DEMO_WINDOWS, but we want to avoid

diff --git a/imgui_internal.h b/imgui_internal.h
@@ -2188,6 +2188,8 @@ struct ImGuiTable
     ImGuiWindow*                InnerWindow;                // Window holding the table data (== OuterWindow or a child window)
     ImGuiTextBuffer             ColumnsNames;               // Contiguous buffer holding columns names
     ImDrawListSplitter*         DrawSplitter;               // Shortcut to TempData->DrawSplitter while in table. Isolate draw commands per columns to avoid switching clip rect constantly
+    ImGuiTableColumnSortSpecs   SortSpecsSingle;
+    ImVector<ImGuiTableColumnSortSpecs> SortSpecsMulti;     // FIXME-OPT: Using a small-vector pattern would be good.
     ImGuiTableSortSpecs         SortSpecs;                  // Public facing sorts specs, this is what we return in TableGetSortSpecs()
     ImGuiTableColumnIdx         SortSpecsCount;
     ImGuiTableColumnIdx         ColumnsEnabledCount;        // Number of enabled columns (<= ColumnsCount)
@@ -2237,16 +2239,13 @@ struct ImGuiTable
 // Transient data that are only needed between BeginTable() and EndTable(), those buffers are shared (1 per level of stacked table).
 // - Accessing those requires chasing an extra pointer so for very frequently used data we leave them in the main table structure.
 // - We also leave out of this structure data that tend to be particularly useful for debugging/metrics.
-// FIXME-TABLE: more transient data could be stored here: DrawSplitter, incoming RowData?
 struct ImGuiTableTempData
 {
     int                         TableIndex;                 // Index in g.Tables.Buf[] pool
     float                       LastTimeActive;             // Last timestamp this structure was used
 
     ImVec2                      UserOuterSize;              // outer_size.x passed to BeginTable()
     ImDrawListSplitter          DrawSplitter;
-    ImGuiTableColumnSortSpecs   SortSpecsSingle;
-    ImVector<ImGuiTableColumnSortSpecs> SortSpecsMulti;     // FIXME-OPT: Using a small-vector pattern would be good.
 
     ImRect                      HostBackupWorkRect;         // Backup of InnerWindow->WorkRect at the end of BeginTable()
     ImRect                      HostBackupParentWorkRect;   // Backup of InnerWindow->ParentWorkRect at the end of BeginTable()

diff --git a/imgui_tables.cpp b/imgui_tables.cpp
@@ -2613,8 +2613,7 @@ ImGuiTableSortSpecs* ImGui::TableGetSortSpecs()
     if (!table->IsLayoutLocked)
         TableUpdateLayout(table);
 
-    if (table->IsSortSpecsDirty)
-        TableSortSpecsBuild(table);
+    TableSortSpecsBuild(table);
 
     return &table->SortSpecs;
 }
@@ -2753,14 +2752,18 @@ void ImGui::TableSortSpecsSanitize(ImGuiTable* table)
 
 void ImGui::TableSortSpecsBuild(ImGuiTable* table)
 {
-    IM_ASSERT(table->IsSortSpecsDirty);
-    TableSortSpecsSanitize(table);
+    bool dirty = table->IsSortSpecsDirty;
+    if (dirty)
+    {
+        TableSortSpecsSanitize(table);
+        table->SortSpecsMulti.resize(table->SortSpecsCount <= 1 ? 0 : table->SortSpecsCount);
+        table->SortSpecs.SpecsDirty = true; // Mark as dirty for user
+        table->IsSortSpecsDirty = false; // Mark as not dirty for us
+    }
 
     // Write output
-    ImGuiTableTempData* temp_data = table->TempData;
-    temp_data->SortSpecsMulti.resize(table->SortSpecsCount <= 1 ? 0 : table->SortSpecsCount);
-    ImGuiTableColumnSortSpecs* sort_specs = (table->SortSpecsCount == 0) ? NULL : (table->SortSpecsCount == 1) ? &temp_data->SortSpecsSingle : temp_data->SortSpecsMulti.Data;
-    if (sort_specs != NULL)
+    ImGuiTableColumnSortSpecs* sort_specs = (table->SortSpecsCount == 0) ? NULL : (table->SortSpecsCount == 1) ? &table->SortSpecsSingle : table->SortSpecsMulti.Data;
+    if (dirty && sort_specs != NULL)
         for (int column_n = 0; column_n < table->ColumnsCount; column_n++)
         {
             ImGuiTableColumn* column = &table->Columns[column_n];
@@ -2773,10 +2776,9 @@ void ImGui::TableSortSpecsBuild(ImGuiTable* table)
             sort_spec->SortOrder = (ImGuiTableColumnIdx)column->SortOrder;
             sort_spec->SortDirection = column->SortDirection;
         }
+
     table->SortSpecs.Specs = sort_specs;
     table->SortSpecs.SpecsCount = table->SortSpecsCount;
-    table->SortSpecs.SpecsDirty = true; // Mark as dirty for user
-    table->IsSortSpecsDirty = false; // Mark as not dirty for us
 }
 
 //-------------------------------------------------------------------------
@@ -3465,7 +3467,8 @@ void ImGui::TableGcCompactTransientBuffers(ImGuiTable* table)
     ImGuiContext& g = *GImGui;
     IM_ASSERT(table->MemoryCompacted == false);
     table->SortSpecs.Specs = NULL;
-    table->IsSortSpecsDirty = true;
+    table->SortSpecsMulti.clear();
+    table->IsSortSpecsDirty = true; // FIXME: shouldn't have to leak into user performing a sort
     table->ColumnsNames.clear();
     table->MemoryCompacted = true;
     for (int n = 0; n < table->ColumnsCount; n++)
@@ -3476,7 +3479,6 @@ void ImGui::TableGcCompactTransientBuffers(ImGuiTable* table)
 void ImGui::TableGcCompactTransientBuffers(ImGuiTableTempData* temp_data)
 {
     temp_data->DrawSplitter.ClearFreeMemory();
-    temp_data->SortSpecsMulti.clear();
     temp_data->LastTimeActive = -1.0f;
 }