Skip to content

Commit

Permalink
Internals: made ScrollbarEx() use ImS64 to facilitate use with larger…
Browse files Browse the repository at this point in the history
… ranges (not fully tested) + clipper tweaks (#3609, #3962 + ocornut/imgui_club#20)

This does NOT fix all problems with large ranges and floating point precision, it merely attenuate them.
  • Loading branch information
ocornut committed Dec 6, 2021
1 parent eea8361 commit 6e141a9
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 14 deletions.
1 change: 1 addition & 0 deletions docs/CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Other Changes:
by the clipper to display. (#3841)
- Clipper: fixed content height declaration slightly mismatching the value of when not using a clipper.
(an additional ItemSpacing.y was declared, affecting scrollbar range).
- Clipper: minor and incomplete changes to tame down precision issues on very large ranges (#3609, #3962).
- Drag and Drop: BeginDragDropSource() with ImGuiDragDropFlags_SourceAllowNullID doesn't lose
tooltip when scrolling. (#143)
- Metrics: Added a node showing windows in submission order and showing the Begin() stack.
Expand Down
9 changes: 6 additions & 3 deletions imgui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2348,8 +2348,9 @@ static void ImGuiListClipper_SeekCursorAndSetupPrevLine(float pos_y, float line_
static void ImGuiListClipper_SeekCursorForItem(ImGuiListClipper* clipper, int item_n)
{
// StartPosY starts from ItemsFrozen hence the subtraction
// Perform the add and multiply with double to allow seeking through larger ranges
ImGuiListClipperData* data = (ImGuiListClipperData*)clipper->TempData;
float pos_y = clipper->StartPosY + (item_n - data->ItemsFrozen) * clipper->ItemsHeight;
float pos_y = (float)((double)clipper->StartPosY + (double)(item_n - data->ItemsFrozen) * clipper->ItemsHeight);
ImGuiListClipper_SeekCursorAndSetupPrevLine(pos_y, clipper->ItemsHeight);
}

Expand Down Expand Up @@ -2520,8 +2521,10 @@ bool ImGuiListClipper::Step()
for (int i = 0; i < data->Ranges.Size; i++)
if (data->Ranges[i].PosToIndexConvert)
{
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);
int m1 = (int)(((double)data->Ranges[i].Min - window->DC.CursorPos.y) / ItemsHeight);
int m2 = (int)((((double)data->Ranges[i].Max - window->DC.CursorPos.y) / ItemsHeight) + 0.999999f);
data->Ranges[i].Min = ImClamp(already_submitted + m1 + data->Ranges[i].PosToIndexOffsetMin, already_submitted, ItemsCount - 1);
data->Ranges[i].Max = ImClamp(already_submitted + m2 + data->Ranges[i].PosToIndexOffsetMax, data->Ranges[i].Min + 1, ItemsCount);
data->Ranges[i].PosToIndexConvert = false;
}
ImGuiListClipper_SortAndFuseRanges(data->Ranges, data->StepNo);
Expand Down
2 changes: 1 addition & 1 deletion imgui_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -2707,7 +2707,7 @@ namespace ImGui
IMGUI_API bool CollapseButton(ImGuiID id, const ImVec2& pos);
IMGUI_API bool ArrowButtonEx(const char* str_id, ImGuiDir dir, ImVec2 size_arg, ImGuiButtonFlags flags = 0);
IMGUI_API void Scrollbar(ImGuiAxis axis);
IMGUI_API bool ScrollbarEx(const ImRect& bb, ImGuiID id, ImGuiAxis axis, float* p_scroll_v, float avail_v, float contents_v, ImDrawFlags flags);
IMGUI_API bool ScrollbarEx(const ImRect& bb, ImGuiID id, ImGuiAxis axis, ImS64* p_scroll_v, ImS64 avail_v, ImS64 contents_v, ImDrawFlags flags);
IMGUI_API bool ImageButtonEx(ImGuiID id, ImTextureID texture_id, const ImVec2& size, const ImVec2& uv0, const ImVec2& uv1, const ImVec2& padding, const ImVec4& bg_col, const ImVec4& tint_col);
IMGUI_API ImRect GetWindowScrollbarRect(ImGuiWindow* window, ImGuiAxis axis);
IMGUI_API ImGuiID GetWindowScrollbarID(ImGuiWindow* window, ImGuiAxis axis);
Expand Down
22 changes: 12 additions & 10 deletions imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,9 @@ void ImGui::Scrollbar(ImGuiAxis axis)
}
float size_avail = window->InnerRect.Max[axis] - window->InnerRect.Min[axis];
float size_contents = window->ContentSize[axis] + window->WindowPadding[axis] * 2.0f;
ScrollbarEx(bb, id, axis, &window->Scroll[axis], size_avail, size_contents, rounding_corners);
ImS64 scroll = (ImS64)window->Scroll[axis];
ScrollbarEx(bb, id, axis, &scroll, (ImS64)size_avail, (ImS64)size_contents, rounding_corners);
window->Scroll[axis] = (float)scroll;
}

// Vertical/Horizontal scrollbar
Expand All @@ -905,7 +907,7 @@ void ImGui::Scrollbar(ImGuiAxis axis)
// - We store values as normalized ratio and in a form that allows the window content to change while we are holding on a scrollbar
// - We handle both horizontal and vertical scrollbars, which makes the terminology not ideal.
// Still, the code should probably be made simpler..
bool ImGui::ScrollbarEx(const ImRect& bb_frame, ImGuiID id, ImGuiAxis axis, float* p_scroll_v, float size_avail_v, float size_contents_v, ImDrawFlags flags)
bool ImGui::ScrollbarEx(const ImRect& bb_frame, ImGuiID id, ImGuiAxis axis, ImS64* p_scroll_v, ImS64 size_avail_v, ImS64 size_contents_v, ImDrawFlags flags)
{
ImGuiContext& g = *GImGui;
ImGuiWindow* window = g.CurrentWindow;
Expand Down Expand Up @@ -936,22 +938,22 @@ bool ImGui::ScrollbarEx(const ImRect& bb_frame, ImGuiID id, ImGuiAxis axis, floa
// Calculate the height of our grabbable box. It generally represent the amount visible (vs the total scrollable amount)
// But we maintain a minimum size in pixel to allow for the user to still aim inside.
IM_ASSERT(ImMax(size_contents_v, size_avail_v) > 0.0f); // Adding this assert to check if the ImMax(XXX,1.0f) is still needed. PLEASE CONTACT ME if this triggers.
const float win_size_v = ImMax(ImMax(size_contents_v, size_avail_v), 1.0f);
const float grab_h_pixels = ImClamp(scrollbar_size_v * (size_avail_v / win_size_v), style.GrabMinSize, scrollbar_size_v);
const ImS64 win_size_v = ImMax(ImMax(size_contents_v, size_avail_v), (ImS64)1);
const float grab_h_pixels = ImClamp(scrollbar_size_v * ((float)size_avail_v / (float)win_size_v), style.GrabMinSize, scrollbar_size_v);
const float grab_h_norm = grab_h_pixels / scrollbar_size_v;

// Handle input right away. None of the code of Begin() is relying on scrolling position before calling Scrollbar().
bool held = false;
bool hovered = false;
ButtonBehavior(bb, id, &hovered, &held, ImGuiButtonFlags_NoNavFocus);

float scroll_max = ImMax(1.0f, size_contents_v - size_avail_v);
float scroll_ratio = ImSaturate(*p_scroll_v / scroll_max);
const ImS64 scroll_max = ImMax((ImS64)1, size_contents_v - size_avail_v);
float scroll_ratio = ImSaturate((float)*p_scroll_v / (float)scroll_max);
float grab_v_norm = scroll_ratio * (scrollbar_size_v - grab_h_pixels) / scrollbar_size_v; // Grab position in normalized space
if (held && allow_interaction && grab_h_norm < 1.0f)
{
float scrollbar_pos_v = bb.Min[axis];
float mouse_pos_v = g.IO.MousePos[axis];
const float scrollbar_pos_v = bb.Min[axis];
const float mouse_pos_v = g.IO.MousePos[axis];

// Click position in scrollbar normalized space (0.0f->1.0f)
const float clicked_v_norm = ImSaturate((mouse_pos_v - scrollbar_pos_v) / scrollbar_size_v);
Expand All @@ -971,10 +973,10 @@ bool ImGui::ScrollbarEx(const ImRect& bb_frame, ImGuiID id, ImGuiAxis axis, floa
// Apply scroll (p_scroll_v will generally point on one member of window->Scroll)
// It is ok to modify Scroll here because we are being called in Begin() after the calculation of ContentSize and before setting up our starting position
const float scroll_v_norm = ImSaturate((clicked_v_norm - g.ScrollbarClickDeltaToGrabCenter - grab_h_norm * 0.5f) / (1.0f - grab_h_norm));
*p_scroll_v = IM_ROUND(scroll_v_norm * scroll_max);//(win_size_contents_v - win_size_v));
*p_scroll_v = (ImS64)(scroll_v_norm * scroll_max);

// Update values for rendering
scroll_ratio = ImSaturate(*p_scroll_v / scroll_max);
scroll_ratio = ImSaturate((float)*p_scroll_v / (float)scroll_max);
grab_v_norm = scroll_ratio * (scrollbar_size_v - grab_h_pixels) / scrollbar_size_v;

// Update distance to grab now that we have seeked and saturated
Expand Down

0 comments on commit 6e141a9

Please sign in to comment.