From d9dad2f4a15aa49c7464b9b3eef2588f14437e22 Mon Sep 17 00:00:00 2001 From: ocornut Date: Wed, 12 Mar 2025 14:15:51 +0100 Subject: [PATCH] Scrollbar: stabilize visibility of ScrollbarX when detecting a feedback loop. (#8488, #3285, #4539) --- docs/CHANGELOG.txt | 8 ++++++++ imgui.cpp | 14 ++++++++++++++ imgui_internal.h | 9 ++++++--- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index cc5964f07..bbf9255c3 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -82,6 +82,14 @@ Other changes: visibility of the preview/hint buffer. (#8368) [@m9710797, @ocornut] - Scrollbar: Rework logic that fades-out scrollbar when it becomes too small, which amusingly made it disappear when using very big font/frame size. +- Scrollbar: Automatically stabilize ScrollbarX visibility when detecting a + feedback loop manifesting with ScrollbarX visibility toggling on and off + repeatedly. (#8488, #3285, #4539) + (feedback loops of this sort can manifest in various situations, but combining + horizontal + vertical scrollbar + using a clipper with varying width items is + one frequent cause. The better solution is to, either: (1) enforce visibility + by using ImGuiWindowFlags_AlwaysHorizontalScrollbar or (2) declare stable + contents width with SetNextWindowContentSize(), if you can compute it.) - Tables: fixed calling SetNextWindowScroll() on clipped scrolling table to not leak the value into a subsequent window. (#8196) - Tables: fixed an issue where Columns Visible/Width state wouldn't be correctly diff --git a/imgui.cpp b/imgui.cpp index ae81cc64a..093ba6375 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -7456,9 +7456,23 @@ bool ImGui::Begin(const char* name, bool* p_open, ImGuiWindowFlags flags) ImVec2 needed_size_from_last_frame = window_just_created ? ImVec2(0, 0) : window->ContentSize + window->WindowPadding * 2.0f; float size_x_for_scrollbars = use_current_size_for_scrollbar_x ? avail_size_from_current_frame.x : avail_size_from_last_frame.x; float size_y_for_scrollbars = use_current_size_for_scrollbar_y ? avail_size_from_current_frame.y : avail_size_from_last_frame.y; + bool scrollbar_x_prev = window->ScrollbarX; //bool scrollbar_y_from_last_frame = window->ScrollbarY; // FIXME: May want to use that in the ScrollbarX expression? How many pros vs cons? window->ScrollbarY = (flags & ImGuiWindowFlags_AlwaysVerticalScrollbar) || ((needed_size_from_last_frame.y > size_y_for_scrollbars) && !(flags & ImGuiWindowFlags_NoScrollbar)); window->ScrollbarX = (flags & ImGuiWindowFlags_AlwaysHorizontalScrollbar) || ((needed_size_from_last_frame.x > size_x_for_scrollbars - (window->ScrollbarY ? style.ScrollbarSize : 0.0f)) && !(flags & ImGuiWindowFlags_NoScrollbar) && (flags & ImGuiWindowFlags_HorizontalScrollbar)); + + // Track when ScrollbarX visibility keeps toggling, which is a sign of a feedback loop, and stabilize by enforcing visibility (#3285, #8488) + // (Feedback loops of this sort can manifest in various situations, but combining horizontal + vertical scrollbar + using a clipper with varying width items is one frequent cause. + // The better solution is to, either (1) enforce visibility by using ImGuiWindowFlags_AlwaysHorizontalScrollbar or (2) declare stable contents width with SetNextWindowContentSize(), if you can compute it) + window->ScrollbarXStabilizeToggledHistory <<= 1; + window->ScrollbarXStabilizeToggledHistory |= (scrollbar_x_prev != window->ScrollbarX) ? 0x01 : 0x00; + const bool scrollbar_x_stabilize = (window->ScrollbarXStabilizeToggledHistory != 0) && ImCountSetBits(window->ScrollbarXStabilizeToggledHistory) >= 4; // 4 == half of bits in our U8 history. + if (scrollbar_x_stabilize) + window->ScrollbarX = true; + //if (scrollbar_x_stabilize && !window->ScrollbarXStabilizeEnabled) + // IMGUI_DEBUG_LOG("[scroll] Stabilize ScrollbarX for Window '%s'\n", window->Name); + window->ScrollbarXStabilizeEnabled = scrollbar_x_stabilize; + if (window->ScrollbarX && !window->ScrollbarY) window->ScrollbarY = (needed_size_from_last_frame.y > size_y_for_scrollbars - style.ScrollbarSize) && !(flags & ImGuiWindowFlags_NoScrollbar); window->ScrollbarSizes = ImVec2(window->ScrollbarY ? style.ScrollbarSize : 0.0f, window->ScrollbarX ? style.ScrollbarSize : 0.0f); diff --git a/imgui_internal.h b/imgui_internal.h index aaa2027df..6d9120a9e 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -367,9 +367,10 @@ static inline void ImQsort(void* base, size_t count, size_t size_of_element IMGUI_API ImU32 ImAlphaBlendColors(ImU32 col_a, ImU32 col_b); // Helpers: Bit manipulation -static inline bool ImIsPowerOfTwo(int v) { return v != 0 && (v & (v - 1)) == 0; } -static inline bool ImIsPowerOfTwo(ImU64 v) { return v != 0 && (v & (v - 1)) == 0; } -static inline int ImUpperPowerOfTwo(int v) { v--; v |= v >> 1; v |= v >> 2; v |= v >> 4; v |= v >> 8; v |= v >> 16; v++; return v; } +static inline bool ImIsPowerOfTwo(int v) { return v != 0 && (v & (v - 1)) == 0; } +static inline bool ImIsPowerOfTwo(ImU64 v) { return v != 0 && (v & (v - 1)) == 0; } +static inline int ImUpperPowerOfTwo(int v) { v--; v |= v >> 1; v |= v >> 2; v |= v >> 4; v |= v >> 8; v |= v >> 16; v++; return v; } +static inline unsigned int ImCountSetBits(unsigned int v) { unsigned int count = 0; while (v > 0) { v = v & (v - 1); count++; } return count; } // Helpers: String #define ImStrlen strlen @@ -2514,6 +2515,8 @@ struct IMGUI_API ImGuiWindow ImVec2 ScrollTargetEdgeSnapDist; // 0.0f = no snapping, >0.0f snapping threshold ImVec2 ScrollbarSizes; // Size taken by each scrollbars on their smaller axis. Pay attention! ScrollbarSizes.x == width of the vertical scrollbar, ScrollbarSizes.y = height of the horizontal scrollbar. bool ScrollbarX, ScrollbarY; // Are scrollbars visible? + bool ScrollbarXStabilizeEnabled; // Was ScrollbarX previously auto-stabilized? + ImU8 ScrollbarXStabilizeToggledHistory; // Used to stabilize scrollbar visibility in case of feedback loops bool Active; // Set to true on Begin(), unless Collapsed bool WasActive; bool WriteAccessed; // Set to true when any widget access the current window