Browse Source

Tables: Disable initial output prior to NextRow call to avoid misleading users.

Fixed some inconsistency with BeginTable/EndTable without row.
Move some of the TableBegin() code in TableBeginUpdateColumns().
Allow to submit multiple header lines.
omar 5 years ago
parent
commit
78b12068d9
2 changed files with 56 additions and 34 deletions
  1. 1 1
      imgui_internal.h
  2. 55 33
      imgui_tables.cpp

+ 1 - 1
imgui_internal.h

@@ -2183,7 +2183,7 @@ namespace ImGui
     //IMGUI_API int         GetTableLineNo();
     //IMGUI_API int         GetTableLineNo();
 
 
     IMGUI_API bool          BeginTableEx(const char* name, ImGuiID id, int columns_count, ImGuiTableFlags flags = 0, const ImVec2& outer_size = ImVec2(0, 0), float inner_width = 0.0f);
     IMGUI_API bool          BeginTableEx(const char* name, ImGuiID id, int columns_count, ImGuiTableFlags flags = 0, const ImVec2& outer_size = ImVec2(0, 0), float inner_width = 0.0f);
-    IMGUI_API void          TableBeginInitVisibility(ImGuiTable* table);
+    IMGUI_API void          TableBeginUpdateColumns(ImGuiTable* table);
     IMGUI_API void          TableUpdateDrawChannels(ImGuiTable* table);
     IMGUI_API void          TableUpdateDrawChannels(ImGuiTable* table);
     IMGUI_API void          TableUpdateLayout(ImGuiTable* table);
     IMGUI_API void          TableUpdateLayout(ImGuiTable* table);
     IMGUI_API void          TableUpdateBorders(ImGuiTable* table);
     IMGUI_API void          TableUpdateBorders(ImGuiTable* table);

+ 55 - 33
imgui_tables.cpp

@@ -64,7 +64,7 @@
 // Typical call flow: (root level is public API):
 // Typical call flow: (root level is public API):
 // - BeginTable()                               user begin into a table
 // - BeginTable()                               user begin into a table
 //    - BeginChild()                            - (if ScrollX/ScrollY is set)
 //    - BeginChild()                            - (if ScrollX/ScrollY is set)
-//    - TableBeginInitVisibility()              - lock columns visibility
+//    - TableBeginUpdateColumns()               - apply resize/order requests, lock columns active state, order
 // - TableSetupColumn()                         user submit columns details (optional)
 // - TableSetupColumn()                         user submit columns details (optional)
 // - TableAutoHeaders() or TableHeader()        user submit a headers row (optional)
 // - TableAutoHeaders() or TableHeader()        user submit a headers row (optional)
 //    - TableSortSpecsClickColumn()
 //    - TableSortSpecsClickColumn()
@@ -306,8 +306,26 @@ bool    ImGui::BeginTableEx(const char* name, ImGuiID id, int columns_count, ImG
     if (table->IsFirstFrame || table->IsSettingsRequestLoad)
     if (table->IsFirstFrame || table->IsSettingsRequestLoad)
         TableLoadSettings(table);
         TableLoadSettings(table);
 
 
+    // Grab a copy of window fields we will modify
+    table->BackupSkipItems = inner_window->SkipItems;
+    table->BackupWorkRect = inner_window->WorkRect;
+    table->BackupCursorMaxPos = inner_window->DC.CursorMaxPos;
+
+    // Disable output until user calls TableNextRow() or TableNextCell() leading to the TableUpdateLayout() call..
+    // This is not strictly necessary but will reduce cases were misleading "out of table" output will be confusing to the user.
+    // Because we cannot safely assert in EndTable() when no rows have been created, this seems like our best option.
+    inner_window->SkipItems = true;
+
+    // Update/lock which columns will be Active for the frame
+    TableBeginUpdateColumns(table);
+
+    return true;
+}
+
+void ImGui::TableBeginUpdateColumns(ImGuiTable* table)
+{
     // Handle resizing request
     // Handle resizing request
-    // (We process this at the first beginning of the frame)
+    // (We process this at the first TableBegin of the frame)
     // FIXME-TABLE: Preserve contents width _while resizing down_ until releasing.
     // FIXME-TABLE: Preserve contents width _while resizing down_ until releasing.
     // FIXME-TABLE: Contains columns if our work area doesn't allow for scrolling.
     // FIXME-TABLE: Contains columns if our work area doesn't allow for scrolling.
     if (table->InstanceNo == 0)
     if (table->InstanceNo == 0)
@@ -341,29 +359,12 @@ bool    ImGui::BeginTableEx(const char* name, ImGuiID id, int columns_count, ImG
     // Handle display order reset request
     // Handle display order reset request
     if (table->IsResetDisplayOrderRequest)
     if (table->IsResetDisplayOrderRequest)
     {
     {
-        for (int n = 0; n < columns_count; n++)
+        for (int n = 0; n < table->ColumnsCount; n++)
             table->DisplayOrder[n] = table->Columns[n].IndexDisplayOrder = (ImU8)n;
             table->DisplayOrder[n] = table->Columns[n].IndexDisplayOrder = (ImU8)n;
         table->IsResetDisplayOrderRequest = false;
         table->IsResetDisplayOrderRequest = false;
         table->IsSettingsDirty = true;
         table->IsSettingsDirty = true;
     }
     }
 
 
-    TableBeginInitVisibility(table);
-
-    // Grab a copy of window fields we will modify
-    table->BackupSkipItems = inner_window->SkipItems;
-    table->BackupWorkRect = inner_window->WorkRect;
-    table->BackupCursorMaxPos = inner_window->DC.CursorMaxPos;
-
-    if (flags & ImGuiTableFlags_NoClipX)
-        table->DrawSplitter.SetCurrentChannel(inner_window->DrawList, 1);
-    else
-        inner_window->DrawList->PushClipRect(inner_window->ClipRect.Min, inner_window->ClipRect.Max, false);
-
-    return true;
-}
-
-void ImGui::TableBeginInitVisibility(ImGuiTable* table)
-{
     // Setup and lock Active state and order
     // Setup and lock Active state and order
     table->ColumnsActiveCount = 0;
     table->ColumnsActiveCount = 0;
     table->IsDefaultDisplayOrder = true;
     table->IsDefaultDisplayOrder = true;
@@ -706,7 +707,7 @@ void    ImGui::TableUpdateLayout(ImGuiTable* table)
         column->ClipRect.Max.x = column->MaxX;// -1.0f;
         column->ClipRect.Max.x = column->MaxX;// -1.0f;
         column->ClipRect.Max.y = FLT_MAX;
         column->ClipRect.Max.y = FLT_MAX;
         column->ClipRect.ClipWithFull(host_clip_rect);
         column->ClipRect.ClipWithFull(host_clip_rect);
-        
+
         column->IsClipped = (column->ClipRect.Max.x <= column->ClipRect.Min.x) && (column->AutoFitQueue & 1) == 0 && (column->CannotSkipItemsQueue & 1) == 0;
         column->IsClipped = (column->ClipRect.Max.x <= column->ClipRect.Min.x) && (column->AutoFitQueue & 1) == 0 && (column->CannotSkipItemsQueue & 1) == 0;
         if (column->IsClipped)
         if (column->IsClipped)
         {
         {
@@ -776,6 +777,13 @@ void    ImGui::TableUpdateLayout(ImGuiTable* table)
             table->IsContextPopupOpen = false;
             table->IsContextPopupOpen = false;
         }
         }
     }
     }
+
+    // Initial state
+    ImGuiWindow* inner_window = table->InnerWindow;
+    if (table->Flags & ImGuiTableFlags_NoClipX)
+        table->DrawSplitter.SetCurrentChannel(inner_window->DrawList, 1);
+    else
+        inner_window->DrawList->PushClipRect(inner_window->ClipRect.Min, inner_window->ClipRect.Max, false);
 }
 }
 
 
 // Process interaction on resizing borders. Actual size change will be applied in EndTable()
 // Process interaction on resizing borders. Actual size change will be applied in EndTable()
@@ -847,6 +855,15 @@ void    ImGui::EndTable()
     ImGuiTable* table = g.CurrentTable;
     ImGuiTable* table = g.CurrentTable;
     IM_ASSERT(table != NULL && "Only call EndTable() is BeginTable() returns true!");
     IM_ASSERT(table != NULL && "Only call EndTable() is BeginTable() returns true!");
 
 
+    // This assert would be very useful to catch a common error... unfortunately it would probably trigger in some cases,
+    // and for consistency user may sometimes output empty tables (and still benefit from e.g. outer border)
+    //IM_ASSERT(table->IsLayoutLocked && "Table unused: never called TableNextRow(), is that the intent?");
+
+    // If the user never got to call TableNextRow() or TableNextCell(), we call layout ourselves to ensure all our
+    // code paths are consistent (instead of just hoping that TableBegin/TableEnd will work), get borders drawn, etc.
+    if (!table->IsLayoutLocked)
+        TableUpdateLayout(table);
+
     const ImGuiTableFlags flags = table->Flags;
     const ImGuiTableFlags flags = table->Flags;
     ImGuiWindow* inner_window = table->InnerWindow;
     ImGuiWindow* inner_window = table->InnerWindow;
     ImGuiWindow* outer_window = table->OuterWindow;
     ImGuiWindow* outer_window = table->OuterWindow;
@@ -1664,7 +1681,7 @@ const char* ImGui::TableGetColumnName(ImGuiTable* table, int column_no)
 
 
 void    ImGui::TableSetColumnAutofit(ImGuiTable* table, int column_no)
 void    ImGui::TableSetColumnAutofit(ImGuiTable* table, int column_no)
 {
 {
-    // Disable clipping then auto-fit, will take 2 frames 
+    // Disable clipping then auto-fit, will take 2 frames
     // (we don't take a shortcut for unclipped columns to reduce inconsistencies when e.g. resizing multiple columns)
     // (we don't take a shortcut for unclipped columns to reduce inconsistencies when e.g. resizing multiple columns)
     ImGuiTableColumn* column = &table->Columns[column_no];
     ImGuiTableColumn* column = &table->Columns[column_no];
     column->CannotSkipItemsQueue = (1 << 0);
     column->CannotSkipItemsQueue = (1 << 0);
@@ -1759,23 +1776,23 @@ void    ImGui::TableDrawContextMenu(ImGuiTable* table, int selected_column_n)
 }
 }
 
 
 // This is a helper to output TableHeader() calls based on the column names declared in TableSetupColumn().
 // This is a helper to output TableHeader() calls based on the column names declared in TableSetupColumn().
-// The intent is that advanced users would not need to use this helper and may create their own.
+// The intent is that advanced users willing to create customized headers would not need to use this helper and may create their own.
+// However presently this function uses too many internal structures/calls.
 void    ImGui::TableAutoHeaders()
 void    ImGui::TableAutoHeaders()
 {
 {
     ImGuiContext& g = *GImGui;
     ImGuiContext& g = *GImGui;
     ImGuiWindow* window = g.CurrentWindow;
     ImGuiWindow* window = g.CurrentWindow;
-    if (window->SkipItems)
-        return;
 
 
     ImGuiTable* table = g.CurrentTable;
     ImGuiTable* table = g.CurrentTable;
     IM_ASSERT(table != NULL && "Need to call TableAutoHeaders() after BeginTable()!");
     IM_ASSERT(table != NULL && "Need to call TableAutoHeaders() after BeginTable()!");
-    IM_ASSERT(table->CurrentRow == -1);
 
 
-    int open_context_popup = INT_MAX;
+    TableNextRow(ImGuiTableRowFlags_Headers, GetTextLineHeight());
+    if (window->SkipItems)
+        return;
 
 
     // This for loop is constructed to not make use of internal functions,
     // This for loop is constructed to not make use of internal functions,
     // as this is intended to be a base template to copy and build from.
     // as this is intended to be a base template to copy and build from.
-    TableNextRow(ImGuiTableRowFlags_Headers, GetTextLineHeight());
+    int open_context_popup = INT_MAX;
     const int columns_count = table->ColumnsCount;
     const int columns_count = table->ColumnsCount;
     for (int column_n = 0; column_n < columns_count; column_n++)
     for (int column_n = 0; column_n < columns_count; column_n++)
     {
     {
@@ -1788,7 +1805,7 @@ void    ImGui::TableAutoHeaders()
 #if 0
 #if 0
         if (column_n < 2)
         if (column_n < 2)
         {
         {
-            static bool b[10] = {};
+            static bool b[2] = {};
             PushID(column_n);
             PushID(column_n);
             PushStyleVar(ImGuiStyleVar_FramePadding, ImVec2(0, 0));
             PushStyleVar(ImGuiStyleVar_FramePadding, ImVec2(0, 0));
             Checkbox("##", &b[column_n]);
             Checkbox("##", &b[column_n]);
@@ -1801,7 +1818,8 @@ void    ImGui::TableAutoHeaders()
         // [DEBUG]
         // [DEBUG]
         //if (g.IO.KeyCtrl) { static char buf[32]; name = buf; ImGuiTableColumn* c = &table->Columns[column_n]; if (c->Flags & ImGuiTableColumnFlags_WidthStretch) ImFormatString(buf, 32, "%.3f>%.1f", c->ResizeWeight, c->WidthGiven); else ImFormatString(buf, 32, "%.1f", c->WidthGiven); }
         //if (g.IO.KeyCtrl) { static char buf[32]; name = buf; ImGuiTableColumn* c = &table->Columns[column_n]; if (c->Flags & ImGuiTableColumnFlags_WidthStretch) ImFormatString(buf, 32, "%.3f>%.1f", c->ResizeWeight, c->WidthGiven); else ImFormatString(buf, 32, "%.1f", c->WidthGiven); }
 
 
-        PushID(table->InstanceNo * table->ColumnsCount + column_n); // Allow unnamed labels (generally accidental, but let's behave nicely with them)
+        // Push an id to allow unnamed labels (generally accidental, but let's behave nicely with them)
+        PushID(table->InstanceNo * table->ColumnsCount + column_n);
         TableHeader(name);
         TableHeader(name);
         PopID();
         PopID();
 
 
@@ -1811,15 +1829,19 @@ void    ImGui::TableAutoHeaders()
     }
     }
 
 
     // FIXME-TABLE: This is not user-land code any more...
     // FIXME-TABLE: This is not user-land code any more...
+    // FIXME-TABLE: Need to explain why this is here!
     window->SkipItems = table->BackupSkipItems;
     window->SkipItems = table->BackupSkipItems;
 
 
     // Allow opening popup from the right-most section after the last column
     // Allow opening popup from the right-most section after the last column
     // FIXME-TABLE: This is not user-land code any more... perhaps instead we should expose hovered column.
     // FIXME-TABLE: This is not user-land code any more... perhaps instead we should expose hovered column.
     // and allow some sort of row-centric IsItemHovered() for full flexibility?
     // and allow some sort of row-centric IsItemHovered() for full flexibility?
-    const float unused_x1 = (table->RightMostActiveColumn != -1) ? table->Columns[table->RightMostActiveColumn].MaxX : table->WorkRect.Min.x;
+    float unused_x1 = table->WorkRect.Min.x;
+    if (table->RightMostActiveColumn != -1)
+        unused_x1 = ImMax(unused_x1, table->Columns[table->RightMostActiveColumn].MaxX);
     if (unused_x1 < table->WorkRect.Max.x)
     if (unused_x1 < table->WorkRect.Max.x)
     {
     {
-        // FIXME: We inherit ClipRect/SkipItem from last submitted column (active or not), let's override
+        // FIXME: We inherit ClipRect/SkipItem from last submitted column (active or not), let's temporarily override it.
+        // Because we don't perform any rendering here we just overwrite window->ClipRect used by logic.
         window->ClipRect = table->InnerClipRect;
         window->ClipRect = table->InnerClipRect;
 
 
         ImVec2 backup_cursor_max_pos = window->DC.CursorMaxPos;
         ImVec2 backup_cursor_max_pos = window->DC.CursorMaxPos;
@@ -1839,7 +1861,7 @@ void    ImGui::TableAutoHeaders()
         window->ClipRect = window->DrawList->_ClipRectStack.back();
         window->ClipRect = window->DrawList->_ClipRectStack.back();
     }
     }
 
 
-    // Context Menu
+    // Open Context Menu
     if (open_context_popup != INT_MAX)
     if (open_context_popup != INT_MAX)
         if (table->Flags & (ImGuiTableFlags_Resizable | ImGuiTableFlags_Reorderable | ImGuiTableFlags_Hideable))
         if (table->Flags & (ImGuiTableFlags_Resizable | ImGuiTableFlags_Reorderable | ImGuiTableFlags_Hideable))
         {
         {