From 7c44d067e8d1662549db73f80099e01a9c1a5f46 Mon Sep 17 00:00:00 2001 From: ocornut Date: Thu, 17 Jun 2021 15:18:11 +0200 Subject: [PATCH] Tables: Fix invalid data in TableGetSortSpecs() when SpecsDirty flag is unset. (#4233) Amend 4ce6bd8cf, but with usage of ImPool<> bug existed even before 4ce6bd8c. Would only materialize if user called (ableGetSortSpecs and used data without checking SpecsDirty. --- docs/CHANGELOG.txt | 1 + imgui_internal.h | 5 ++--- imgui_tables.cpp | 26 ++++++++++++++------------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index e03c4a31..6fb4f098 100644 --- 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 index d064b9dd..09dc408c 100644 --- 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 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,7 +2239,6 @@ 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 @@ -2245,8 +2246,6 @@ struct ImGuiTableTempData ImVec2 UserOuterSize; // outer_size.x passed to BeginTable() ImDrawListSplitter DrawSplitter; - ImGuiTableColumnSortSpecs SortSpecsSingle; - ImVector 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 index 71ee70d5..74c7cd12 100644 --- 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; }