SetScrollX/SetScrollY is not symmetrical

[tombuskens]

Version/Branch of Dear ImGui:

Dear ImGui 1.89.7 WIP (18962)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=202002
define: _WIN32
define: _WIN64
define: _MSC_VER=1936
define: _MSVC_LANG=202002
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_dx12
io.ConfigFlags: 0x00000441
 NavEnableKeyboard
 DockingEnable
 ViewportsEnable
io.ConfigViewportsNoDecoration
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00001C0E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 RendererHasVtxOffset
 RendererHasViewports
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1264.00,761.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

My Issue/Question:

Observed behavior
When using either SetScrollX or SetScrollY to scroll small increments, scrolling the same amount in a different direction (e.g.

float x_offset = scrollLeft ? -0.8f : 0.8f; 
ImGui::SetScrollX(ImGui::GetScrollX() + x_offset );

will result in either

• content will scroll in one direction, but not the other - when abs(x_offset) is smaller than 1.0f
• content will scroll noticeably faster in one direction, when abs(x_offset) is small, but larger than 1.0f

Expected behavior
The expected behavior would be to scroll the same amount in both directions.

Potential fix
It looks like this is caused by the IM_FLOOR call in CalcNextScrollFromScrollTargetAndClamp. Replacing IM_FLOOR with IM_ROUND seems to solve the issue.

Standalone, minimal, complete and verifiable example:
Code modified from the Layout/Scrolling/Horizontal (more) demo. Only change is fixing scroll_x_delta to +/-0.8f.

        static int lines = 7;
        ImGui::PushStyleVar(ImGuiStyleVar_FrameRounding, 3.0f);
        ImGui::PushStyleVar(ImGuiStyleVar_FramePadding, ImVec2(2.0f, 1.0f));
        ImVec2 scrolling_child_size = ImVec2(0, ImGui::GetFrameHeightWithSpacing() * 7 + 30);
        ImGui::BeginChild("scrolling", scrolling_child_size, true, ImGuiWindowFlags_HorizontalScrollbar);
        for (int line = 0; line < lines; line++)
        {
            // Display random stuff. For the sake of this trivial demo we are using basic Button() + SameLine()
            // If you want to create your own time line for a real application you may be better off manipulating
            // the cursor position yourself, aka using SetCursorPos/SetCursorScreenPos to position the widgets
            // yourself. You may also want to use the lower-level ImDrawList API.
            int num_buttons = 10 + ((line & 1) ? line * 9 : line * 3);
            for (int n = 0; n < num_buttons; n++)
            {
                if (n > 0) ImGui::SameLine();
                ImGui::PushID(n + line * 1000);
                char num_buf[16];
                sprintf(num_buf, "%d", n);
                const char* label = (!(n % 15)) ? "FizzBuzz" : (!(n % 3)) ? "Fizz" : (!(n % 5)) ? "Buzz" : num_buf;
                float hue = n * 0.05f;
                ImGui::PushStyleColor(ImGuiCol_Button, (ImVec4)ImColor::HSV(hue, 0.6f, 0.6f));
                ImGui::PushStyleColor(ImGuiCol_ButtonHovered, (ImVec4)ImColor::HSV(hue, 0.7f, 0.7f));
                ImGui::PushStyleColor(ImGuiCol_ButtonActive, (ImVec4)ImColor::HSV(hue, 0.8f, 0.8f));
                ImGui::Button(label, ImVec2(40.0f + sinf((float)(line + n)) * 20.0f, 0.0f));
                ImGui::PopStyleColor(3);
                ImGui::PopID();
            }
        }
        float scroll_x = ImGui::GetScrollX();
        float scroll_max_x = ImGui::GetScrollMaxX();
        ImGui::EndChild();
        ImGui::PopStyleVar(2);
        float scroll_x_delta = 0.0f;
        ImGui::SmallButton("<<");
        if (ImGui::IsItemActive())
            scroll_x_delta = -0.8f; // <== CHANGED: use a small fixed small increment
        ImGui::SameLine();
        ImGui::Text("Scroll from code"); ImGui::SameLine();
        ImGui::SmallButton(">>");
        if (ImGui::IsItemActive())
            scroll_x_delta = +0.8f;  // <== CHANGED: use a small fixed small increment
        ImGui::SameLine();
        ImGui::Text("%.0f/%.0f", scroll_x, scroll_max_x);
        if (scroll_x_delta != 0.0f)
        {
            // Demonstrate a trick: you can use Begin to set yourself in the context of another window
            // (here we are already out of your child window)
            ImGui::BeginChild("scrolling");
            ImGui::SetScrollX(ImGui::GetScrollX() + scroll_x_delta);
            ImGui::EndChild();
        }

[ocornut]

The internal scrolling is indeed truncated/rounded to nearest integer.
I don't think there is a good solution for it other than us holding sub-integer value and resurfacing it back, which we may attempt later, potentially at the same time we'll try to remove pixel-rounding in many other places.

Your solution isn't super correct IMHO: it does help with symmetry but doesn't help surfacing the fact that in your situation your attempt to scroll by 0.8 will in fact scroll by 1.0 in both direction.

You may maintain an accumulator yourself to provide a feeling of scrolling slower, but it may feel a bit janky:

scroll float scroll_x_accum = 0.0f;
ImGui::SmallButton("<<");
if (ImGui::IsItemActive())
    scroll_x_accum -= 0.8f;
ImGui::SmallButton(">>");
if (ImGui::IsItemActive())
    scroll_x_accum += 0.8f;
[...]
if (fabsf(scroll_x_accum) >= 1.0f)
{
    ImGui::SetScrollX(ImGui::GetScrollX() + ImFloor(scroll_x_accum));
    scroll_x_accum -= ImFloor(scroll_x_accum);
}

(PS: tangential, note this change 94da584 which happened in 1.90)

[ocornut]

FYI my previous message was ambiguous about it but I have pushed 5366bd0 which does what you suggested, but note my comments above.