Clipper: bunch of rework. (#3841, #1725)

- Focused/NavId now always included in display range. - Any number of steps (while preserving zero-alloc policy). - Non contiguous ranges for nav processing - Moved new fields internally (+ moved StepNo away from sight so it doesn't get missused). - Generally tweaks/refactors.
2025-07-06 04:58:47 +02:00 · 2021-10-11 17:19:20 +02:00
parent cd1b5f7883
commit 93cccd27f6
4 changed files with 197 additions and 150 deletions
--- a/imgui.cpp
+++ b/imgui.cpp
@ -2248,6 +2248,8 @@ static bool GetSkipItemForListClipping()
 // Helper to calculate coarse clipping of large list of evenly sized items.
 // NB: Prefer using the ImGuiListClipper higher-level helper if you can! Read comments and instructions there on how those use this sort of pattern.
 // NB: 'items_count' is only used to clamp the result, if you don't know your count you can use INT_MAX
+// FIXME: This legacy API is not ideal because it assume we will return a single contiguous rectangle.
+// Prefer using ImGuiListClipper which returns disconnected ranges.
 void ImGui::CalcListClipping(int items_count, float items_height, int* out_items_display_start, int* out_items_display_end)
 {
    ImGuiContext& g = *GImGui;
@ -2266,15 +2268,16 @@ void ImGui::CalcListClipping(int items_count, float items_height, int* out_items
    }

    // We create the union of the ClipRect and the scoring rect which at worst should be 1 page away from ClipRect
-    ImRect unclipped_rect = window->ClipRect;
+    // We don't include g.NavId's rectangle in there (unless g.NavJustMovedToId is set) because the rectangle enlargement can get costly.
+    ImRect rect = window->ClipRect;
    if (g.NavMoveScoringItems)
-        unclipped_rect.Add(g.NavScoringRect);
+        rect.Add(g.NavScoringNoClipRect);
    if (g.NavJustMovedToId && window->NavLastIds[0] == g.NavJustMovedToId)
-        unclipped_rect.Add(WindowRectRelToAbs(window, window->NavRectRel[0])); // Could store and use NavJustMovedToRectRel
+        rect.Add(WindowRectRelToAbs(window, window->NavRectRel[0])); // Could store and use NavJustMovedToRectRel

    const ImVec2 pos = window->DC.CursorPos;
-    int start = (int)((unclipped_rect.Min.y - pos.y) / items_height);
-    int end = (int)((unclipped_rect.Max.y - pos.y) / items_height);
+    int start = (int)((rect.Min.y - pos.y) / items_height);
+    int end = (int)((rect.Max.y - pos.y) / items_height);

    // When performing a navigation request, ensure we have one item extra in the direction we are moving to
    if (g.NavMoveScoringItems && g.NavMoveClipDir == ImGuiDir_Up)
@ -2288,47 +2291,35 @@ void ImGui::CalcListClipping(int items_count, float items_height, int* out_items
    *out_items_display_end = end;
 }

-static int SortAndFuseRanges(int* range_start, int* range_end, int range_count)
+static void ImGuiListClipper_SortAndFuseRanges(ImVector<ImGuiListClipperRange>& ranges, int offset = 0)
 {
-    // Helper to order ranges and fuse them together if possible.
-    // First sort both rangeStart and rangeEnd by rangeStart. Since this helper will just sort 2 or 3 entries, a bubble sort will do fine.
-    for (int sort_end = range_count - 1; sort_end > 0; --sort_end)
-    {
-        for (int i = 0; i < sort_end; ++i)
-        {
-            if (range_start[i] > range_start[i + 1])
-            {
-                ImSwap(range_start[i], range_start[i + 1]);
-                ImSwap(range_end[i], range_end[i + 1]);
-            }
-        }
-    }
+    if (ranges.Size - offset <= 1)
+        return;
+
+    // Helper to order ranges and fuse them together if possible (bubble sort is fine as we are only sorting 2-3 entries)
+    for (int sort_end = ranges.Size - offset - 1; sort_end > 0; --sort_end)
+        for (int i = offset; i < sort_end + offset; ++i)
+            if (ranges[i].Min > ranges[i + 1].Min)
+                ImSwap(ranges[i], ranges[i + 1]);

    // Now fuse ranges together as much as possible.
-    for (int i = 1; i < range_count;)
+    for (int i = 1 + offset; i < ranges.Size; i++)
    {
-        if (range_end[i - 1] >= range_start[i])
-        {
-            range_end[i - 1] = ImMax(range_end[i - 1], range_end[i]);
-            range_count--;
-            for (int j = i; j < range_count; ++j)
-            {
-                range_start[j] = range_start[j + 1];
-                range_end[j] = range_end[j + 1];
-            }
-        }
-        else
-            i++;
+        IM_ASSERT(!ranges[i].PosToIndexConvert && !ranges[i - 1].PosToIndexConvert);
+        if (ranges[i - 1].Max < ranges[i].Min)
+            continue;
+        ranges[i - 1].Min = ImMin(ranges[i - 1].Min, ranges[i].Min);
+        ranges[i - 1].Max = ImMax(ranges[i - 1].Max, ranges[i].Max);
+        ranges.erase(ranges.Data + i);
+        i--;
    }
-
-    return range_count;
 }

-static void SetCursorPosYAndSetupForPrevLine(float pos_y, float line_height)
+static void ImGuiListClipper_SeekCursorAndSetupPrevLine(float pos_y, float line_height)
 {
    // Set cursor position and a few other things so that SetScrollHereY() and Columns() can work when seeking cursor.
    // FIXME: It is problematic that we have to do that here, because custom/equivalent end-user code would stumble on the same issue.
-    // The clipper should probably have a 4th step to display the last item in a regular manner.
+    // The clipper should probably have a final step to display the last item in a regular manner, maybe with an opt-out flag for data sets which may have costly seek?
    ImGuiContext& g = *GImGui;
    ImGuiWindow* window = g.CurrentWindow;
    float off_y = pos_y - window->DC.CursorPos.y;
@ -2349,6 +2340,14 @@ static void SetCursorPosYAndSetupForPrevLine(float pos_y, float line_height)
    }
 }

+static void ImGuiListClipper_SeekCursorForItem(ImGuiListClipper* clipper, int item_n)
+{
+    // StartPosY starts from ItemsFrozen hence the subtraction
+    ImGuiListClipperData* data = (ImGuiListClipperData*)clipper->TempData;
+    float pos_y = clipper->StartPosY + (item_n - data->ItemsFrozen) * clipper->ItemsHeight;
+    ImGuiListClipper_SeekCursorAndSetupPrevLine(pos_y, clipper->ItemsHeight);
+}
+
 ImGuiListClipper::ImGuiListClipper()
 {
    memset(this, 0, sizeof(*this));
@ -2375,48 +2374,60 @@ void ImGuiListClipper::Begin(int items_count, float items_height)
    StartPosY = window->DC.CursorPos.y;
    ItemsHeight = items_height;
    ItemsCount = items_count;
-    ItemsFrozen = 0;
-    StepNo = 0;
    DisplayStart = -1;
    DisplayEnd = 0;
+
+    // Acquire temporary buffer
+    if (++g.ClipperTempDataStacked > g.ClipperTempData.Size)
+        g.ClipperTempData.resize(g.ClipperTempDataStacked, ImGuiListClipperData());
+    ImGuiListClipperData* data = &g.ClipperTempData[g.ClipperTempDataStacked - 1];
+    data->Reset(this);
+    TempData = data;
 }

 void ImGuiListClipper::End()
 {
+    ImGuiContext& g = *GImGui;
    if (ItemsCount < 0) // Already ended
        return;

-    // In theory here we should assert that ImGui::GetCursorPosY() == StartPosY + DisplayEnd * ItemsHeight, but it feels saner to just seek at the end and not assert/crash the user.
+    // In theory here we should assert that we are already at the right position, but it seems saner to just seek at the end and not assert/crash the user.
+    ImGuiListClipperData* data = (ImGuiListClipperData*)TempData;
    if (ItemsCount < INT_MAX && DisplayStart >= 0)
-        SetCursorPosYAndSetupForPrevLine(StartPosY + (ItemsCount - ItemsFrozen) * ItemsHeight, ItemsHeight);
+        ImGuiListClipper_SeekCursorForItem(this, ItemsCount);
    ItemsCount = -1;
-    StepNo = RangeCount;
+    data->StepNo = data->Ranges.Size;
+
+    // Restore temporary buffer and fix back pointers which may be invalidated when nesting
+    IM_ASSERT(g.ClipperTempDataStacked > 0);
+    data = (--g.ClipperTempDataStacked > 0) ? &g.ClipperTempData[g.ClipperTempDataStacked - 1] : NULL;
+    if (data)
+        data->ListClipper->TempData = data;
 }

-void ImGuiListClipper::ForceDisplayRange(int item_start, int item_end)
+void ImGuiListClipper::ForceDisplayRangeByIndices(int item_min, int item_max)
 {
-    if (DisplayStart < 0 && RangeCount + YRangeCount < 1)  // Only allowed after Begin() and if there has not been a specified range yet.
-    {
-        RangeStart[RangeCount] = item_start;
-        RangeEnd[RangeCount] = item_end;
-        RangeCount++;
-    }
+    ImGuiListClipperData* data = (ImGuiListClipperData*)TempData;
+    IM_ASSERT(DisplayStart < 0); // Only allowed after Begin() and if there has not been a specified range yet.
+    IM_ASSERT(item_min <= item_max);
+    if (item_min < item_max)
+        data->Ranges.push_back(ImGuiListClipperRange::FromIndices(item_min, item_max));
 }

-void ImGuiListClipper::ForceDisplayYRange(float y_min, float y_max)
+void ImGuiListClipper::ForceDisplayRangeByPositions(float y_min, float y_max)
 {
-    if (DisplayStart < 0 && RangeCount + YRangeCount < 1)  // Only allowed after Begin() and if there has not been a specified range yet.
-    {
-        YRangeMin[YRangeCount] = y_min;
-        YRangeMax[YRangeCount] = y_max;
-        YRangeCount++;
-    }
+    ImGuiListClipperData* data = (ImGuiListClipperData*)TempData;
+    IM_ASSERT(DisplayStart < 0); // Only allowed after Begin() and if there has not been a specified range yet.
+    IM_ASSERT(y_min <= y_max);
+    if (y_min < y_max)
+        data->Ranges.push_back(ImGuiListClipperRange::FromPositions(y_min, y_max, 0, 0));
 }

 bool ImGuiListClipper::Step()
 {
    ImGuiContext& g = *GImGui;
    ImGuiWindow* window = g.CurrentWindow;
+    ImGuiListClipperData* data = (ImGuiListClipperData*)TempData;

    ImGuiTable* table = g.CurrentTable;
    if (table && table->IsInsideRow)
@ -2429,46 +2440,41 @@ bool ImGuiListClipper::Step()
        return false;
    }

-    bool calc_clipping = false;
+    // While we are in frozen row state, keep displaying items one by one, unclipped
+    // FIXME: Could be stored as a table-agnostic state.
+    if (data->StepNo == 0 && table != NULL && !table->IsUnfrozenRows)
+    {
+        DisplayStart = data->ItemsFrozen;
+        DisplayEnd = data->ItemsFrozen + 1;
+        data->ItemsFrozen++;
+        return true;
+    }

    // Step 0: Let you process the first element (regardless of it being visible or not, so we can measure the element height)
-    if (StepNo == 0)
+    bool calc_clipping = false;
+    if (data->StepNo == 0)
    {
-        // While we are in frozen row state, keep displaying items one by one, unclipped
-        // FIXME: Could be stored as a table-agnostic state.
-        if (table != NULL && !table->IsUnfrozenRows)
-        {
-            DisplayStart = ItemsFrozen;
-            DisplayEnd = ItemsFrozen + 1;
-            ItemsFrozen++;
-            return true;
-        }
-
        StartPosY = window->DC.CursorPos.y;
        if (ItemsHeight <= 0.0f)
        {
-            // Submit the first item (or range) so we can measure its height (generally it is 0..1)
-            RangeStart[RangeCount] = ItemsFrozen;
-            RangeEnd[RangeCount] = ItemsFrozen + 1;
-            if (++RangeCount > 1)
-                RangeCount = SortAndFuseRanges(RangeStart, RangeEnd, RangeCount);
-            DisplayStart = ImMax(RangeStart[0], ItemsFrozen);
-            DisplayEnd = ImMin(RangeEnd[0], ItemsCount);
-            StepNo = 1;
+            // Submit the first item (or range) so we can measure its height (generally the first range is 0..1)
+            data->Ranges.push_front(ImGuiListClipperRange::FromIndices(data->ItemsFrozen, data->ItemsFrozen + 1));
+            DisplayStart = ImMax(data->Ranges[0].Min, data->ItemsFrozen);
+            DisplayEnd = ImMin(data->Ranges[0].Max, ItemsCount);
+            data->StepNo = 1;
            return true;
        }
-
        calc_clipping = true;   // If on the first step with known item height, calculate clipping.
    }

    // Step 1: Let the clipper infer height from first range
    if (ItemsHeight <= 0.0f)
    {
-        IM_ASSERT(StepNo == 1);
+        IM_ASSERT(data->StepNo == 1);
        if (table)
        {
-            const float pos_y1 = table->RowPosY1;   // Using this instead of StartPosY to handle clipper straddling the frozen row
-            const float pos_y2 = table->RowPosY2;   // Using this instead of CursorPos.y to take account of tallest cell.
+            const float pos_y1 = table->RowPosY1;   // Using RowPosY1 instead of StartPosY to handle clipper straddling the frozen row
+            const float pos_y2 = table->RowPosY2;   // Using RowPosY2 instead of CursorPos.y to take account of tallest cell.
            ItemsHeight = pos_y2 - pos_y1;
            window->DC.CursorPos.y = pos_y2;
        }
@ -2477,67 +2483,64 @@ bool ImGuiListClipper::Step()
            ItemsHeight = (window->DC.CursorPos.y - StartPosY) / (float)(DisplayEnd - DisplayStart);
        }
        IM_ASSERT(ItemsHeight > 0.0f && "Unable to calculate item height! First item hasn't moved the cursor vertically!");
-
        calc_clipping = true;   // If item height had to be calculated, calculate clipping afterwards.
    }

-    // Step 0 or 1: Calculate the actual range of visible elements.
+    // Step 0 or 1: Calculate the actual ranges of visible elements.
+    const int already_submitted = DisplayEnd;
    if (calc_clipping)
    {
-        IM_ASSERT(ItemsHeight > 0.0f);
-
-        int already_submitted = DisplayEnd;
-        ImGui::CalcListClipping(ItemsCount - already_submitted, ItemsHeight, &RangeStart[RangeCount], &RangeEnd[RangeCount]);
-
-        // Only add another range if it hasn't been handled by the initial range.
-        if (RangeStart[RangeCount] < RangeEnd[RangeCount])
+        if (g.LogEnabled)
        {
-            RangeStart[RangeCount] += already_submitted;
-            RangeEnd[RangeCount] += already_submitted;
-            RangeCount++;
+            // If logging is active, do not perform any clipping
+            data->Ranges.push_back(ImGuiListClipperRange::FromIndices(0, ItemsCount));
+        }
+        else
+        {
+            // Add range selected to be included for navigation
+            if (g.NavMoveScoringItems)
+                data->Ranges.push_back(ImGuiListClipperRange::FromPositions(g.NavScoringNoClipRect.Min.y, g.NavScoringNoClipRect.Max.y, 0, 0));
+
+            // Add focused/active item
+            ImRect nav_rect_abs = ImGui::WindowRectRelToAbs(window, window->NavRectRel[0]);
+            if (g.NavId != 0 && window->NavLastIds[0] == g.NavId)
+                data->Ranges.push_back(ImGuiListClipperRange::FromPositions(nav_rect_abs.Min.y, nav_rect_abs.Max.y, 0, 0));
+
+            // Add visible range
+            const int off_min = (g.NavMoveScoringItems && g.NavMoveClipDir == ImGuiDir_Up) ? -1 : 0;
+            const int off_max = (g.NavMoveScoringItems && g.NavMoveClipDir == ImGuiDir_Down) ? 1 : 0;
+            data->Ranges.push_back(ImGuiListClipperRange::FromPositions(window->ClipRect.Min.y, window->ClipRect.Max.y, off_min, off_max));
        }

-        // Convert specified y ranges to item index ranges.
-        for (int i = 0; i < YRangeCount; ++i)
-        {
-            int start = already_submitted + (int)((YRangeMin[i] - window->DC.CursorPos.y) / ItemsHeight);
-            int end = already_submitted + (int)((YRangeMax[i] - window->DC.CursorPos.y) / ItemsHeight) + 1;
-
-            start = ImMax(start, already_submitted);
-            end = ImMin(end, ItemsCount);
-
-            if (start < end)
+        // Convert position ranges to item index ranges
+        // - Very important: when a starting position is after our maximum item, we set Min to (ItemsCount - 1). This allows us to handle most forms of wrapping.
+        // - Due to how Selectable extra padding they tend to be "unaligned" with exact unit in the item list,
+        //   which with the flooring/ceiling tend to lead to 2 items instead of one being submitted.
+        for (int i = 0; i < data->Ranges.Size; i++)
+            if (data->Ranges[i].PosToIndexConvert)
            {
-                RangeStart[RangeCount] = start;
-                RangeEnd[RangeCount] = end;
-                RangeCount++;
+                data->Ranges[i].Min = ImClamp(already_submitted + (int)ImFloor((data->Ranges[i].Min - window->DC.CursorPos.y) / ItemsHeight) + data->Ranges[i].PosToIndexOffsetMin, already_submitted, ItemsCount - 1);
+                data->Ranges[i].Max = ImClamp(already_submitted + (int)ImCeil((data->Ranges[i].Max - window->DC.CursorPos.y) / ItemsHeight) + 0 + data->Ranges[i].PosToIndexOffsetMax, data->Ranges[i].Min + 1, ItemsCount);
+                data->Ranges[i].PosToIndexConvert = false;
            }
-        }
-
-        // Try to sort and fuse only if there is more than 1 range remaining.
-        if (RangeCount > StepNo + 1)
-            RangeCount = StepNo + SortAndFuseRanges(&RangeStart[StepNo], &RangeEnd[StepNo], RangeCount - StepNo);
+        ImGuiListClipper_SortAndFuseRanges(data->Ranges, data->StepNo);
    }

    // Step 0+ (if item height is given in advance) or 1+: Display the next range in line.
-    if (StepNo < RangeCount)
+    if (data->StepNo < data->Ranges.Size)
    {
-        int already_submitted = DisplayEnd;
-        DisplayStart = ImMax(RangeStart[StepNo], already_submitted);
-        DisplayEnd = ImMin(RangeEnd[StepNo], ItemsCount);
-
-        // Seek cursor
-        if (DisplayStart > already_submitted)
-            SetCursorPosYAndSetupForPrevLine(StartPosY + (DisplayStart - ItemsFrozen) * ItemsHeight, ItemsHeight);
-
-        StepNo++;
+        DisplayStart = ImMax(data->Ranges[data->StepNo].Min, already_submitted);
+        DisplayEnd = ImMin(data->Ranges[data->StepNo].Max, ItemsCount);
+        if (DisplayStart > already_submitted) //-V1051
+            ImGuiListClipper_SeekCursorForItem(this, DisplayStart);
+        data->StepNo++;
        return true;
    }

    // After the last step: Let the clipper validate that we have reached the expected Y position (corresponding to element DisplayEnd),
    // Advance the cursor to the end of the list and then returns 'false' to end the loop.
    if (ItemsCount < INT_MAX)
-        SetCursorPosYAndSetupForPrevLine(StartPosY + (ItemsCount - ItemsFrozen) * ItemsHeight, ItemsHeight); // advance cursor
+        ImGuiListClipper_SeekCursorForItem(this, ItemsCount);
    ItemsCount = -1;

    return false;
@ -4336,6 +4339,8 @@ void ImGui::Shutdown(ImGuiContext* context)
    g.CurrentTabBarStack.clear();
    g.ShrinkWidthBuffer.clear();

+    g.ClipperTempData.clear_destruct();
+
    g.Tables.Clear();
    g.TablesTempData.clear_destruct();
    g.DrawChannelsTempMergeBuffer.clear();
@ -9605,6 +9610,7 @@ void ImGui::NavUpdateCreateMoveRequest()
            if (!IsActiveIdUsingNavDir(ImGuiDir_Down)  && (IsNavInputTest(ImGuiNavInput_DpadDown,  read_mode) || IsNavInputTest(ImGuiNavInput_KeyDown_,  read_mode))) { g.NavMoveDir = ImGuiDir_Down; }
        }
        g.NavMoveClipDir = g.NavMoveDir;
+        g.NavScoringNoClipRect = ImRect(+FLT_MAX, +FLT_MAX, -FLT_MAX, -FLT_MAX);
    }

    // Update PageUp/PageDown/Home/End scroll
@ -9613,6 +9619,11 @@ void ImGui::NavUpdateCreateMoveRequest()
    float scoring_rect_offset_y = 0.0f;
    if (window && g.NavMoveDir == ImGuiDir_None && nav_keyboard_active)
        scoring_rect_offset_y = NavUpdatePageUpPageDown();
+    if (scoring_rect_offset_y != 0.0f)
+    {
+        g.NavScoringNoClipRect = window->InnerRect;
+        g.NavScoringNoClipRect.TranslateY(scoring_rect_offset_y);
+    }

    // [DEBUG] Always send a request
 #if IMGUI_DEBUG_NAV_SCORING
@ -9666,8 +9677,10 @@ void ImGui::NavUpdateCreateMoveRequest()
        scoring_rect.Max.x = scoring_rect.Min.x;
        IM_ASSERT(!scoring_rect.IsInverted()); // Ensure if we have a finite, non-inverted bounding box here will allows us to remove extraneous ImFabs() calls in NavScoreItem().
        //GetForegroundDrawList()->AddRect(scoring_rect.Min, scoring_rect.Max, IM_COL32(255,200,0,255)); // [DEBUG]
+        //if (!g.NavScoringNoClipRect.IsInverted()) { GetForegroundDrawList()->AddRect(g.NavScoringNoClipRect.Min, g.NavScoringNoClipRect.Max, IM_COL32(255, 200, 0, 255)); } // [DEBUG]
    }
    g.NavScoringRect = scoring_rect;
+    g.NavScoringNoClipRect.Add(scoring_rect);
 }

 // Apply result from previous frame navigation directional move request. Always called from NavUpdate()