BUG: vertical and horizontal SliderInt broken for inverted min/max ranges. Patch supplied

[nobody-special666]

Version/Branch of Dear ImGui:

Version: docking
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_allegro5.cpp
Compiler: gcc
Operating System: Ubuntu linux 20.04

My Issue/Question:

The SliderInt appears broken both in vertical and horizontal directions when the min/max values are inverted.
The slider can't reach the top/right edge.
Inverted min/max ranges are supported for floating point, so I assume this is a bug.
See the example test below.

Thank you for ImGui.
Cheers.

void draw_test_window()
{
    bool visible = true;

    if (ImGui::Begin("sliders test", &visible))
    {
        // integer sliders

        // horizontal sliders
        {
            static int hval0=0;
            static int hval1=4;
            ImGui::VSliderInt("int_horz_reg", ImVec2(30, 100), &hval0, 0, 4, "%i");
            ImGui::SameLine();
            // BROKEN: can't reach top
            ImGui::VSliderInt("int_horz_inv", ImVec2(30, 100), &hval1, 4, 0, "%i");

            // horizontal sliders
            static int vval0=0;
            static int vval1=4;
            ImGui::SetNextItemWidth(100);
            ImGui::SliderInt("int_vert_reg", &vval0, 0, 4, "%i");

            ImGui::NewLine();

            // BROKEN: can't reach far right
            ImGui::SetNextItemWidth(100);
            ImGui::SliderInt("int_vert_inv", &vval1, 4, 0, "%i");
        }

        ImGui::NewLine();

        // float sliders
        {
            static float hval0=0;
            static float hval1=4;
            ImGui::VSliderFloat("float_horz_reg", ImVec2(30, 100), &hval0, 0, 4, "%.2f");
            ImGui::SameLine();
            ImGui::VSliderFloat("float_horz_inv", ImVec2(30, 100), &hval1, 4, 0, "%.2f");

            // horizontal sliders
            static float vval0=0;
            static float vval1=4;
            ImGui::SetNextItemWidth(100);
            ImGui::SliderFloat("float_vert_reg", &vval0, 0, 4, "%.2f");

            ImGui::NewLine();

            ImGui::SetNextItemWidth(100);
            ImGui::SliderFloat("float_vert_inv", &vval1, 4, 0, "%.2f");
        }

    }
    ImGui::End();

}

[rokups]

Duplicate of #3432.

[nobody-special666]

sorry for the duplicate ticket. I didn't realize this was already reported.
So I'll close this.

[nobody-special666]

The following patch seems to have fixed the problem for me, without breaking anything.
Please review. I hope this helps.
Thanks for ImGui

diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp
index 9d6f336..0e08920 100644
--- a/imgui_widgets.cpp
+++ b/imgui_widgets.cpp
@@ -2521,7 +2521,11 @@ bool ImGui::SliderBehaviorT(const ImRect& bb, ImGuiID id, ImGuiDataType data_typ
                     // This code is carefully tuned to work with large values (e.g. high ranges of U64) while preserving this property..
                     FLOATTYPE v_new_off_f = (v_max - v_min) * clicked_t;
                     TYPE v_new_off_floor = (TYPE)(v_new_off_f);
-                    TYPE v_new_off_round = (TYPE)(v_new_off_f + (FLOATTYPE)0.5);
+                    TYPE v_new_off_round;
+                    if (v_min <= v_max)
+                        v_new_off_round= (TYPE)(v_new_off_f + (FLOATTYPE)0.5);
+                    else
+                        v_new_off_round= (TYPE)(v_new_off_f - (FLOATTYPE)0.5);
                     if (v_new_off_floor < v_new_off_round)
                         v_new = v_min + v_new_off_round;
                     else

[ocornut]

Hello, we saw your message in the other thread so will close this one again.

I am tempted to merge your suggested change as a step forward, but Rokas established that its not enough in particular for large values (that wasn’t published in the thread but we have tests for it), so we’ll need a more complex fix eventually (Rokas made one a while ago pending my review, but its a wider change so i am not super confident about it yet).

[ocornut]

Further examination here and it looks like we came up with a much simpler solution, closer to yours:

            // For integer values we want the clicking position to match the grab box so we round above
            // This code is carefully tuned to work with large values (e.g. high ranges of U64) while preserving this property..
            // Not doing a *1.0 multiply at the end of a range as it tends to be lossy. While e.g. large s64/u64 ranges are
            // likely to be imprecise anyway, with this check we at least make the edge values matches expected limits.
            if (t < 1.0)
            {
                FLOATTYPE v_new_off_f = (v_max - v_min) * t;
                result = v_min + (TYPE)(v_new_off_f + (FLOATTYPE)(v_min > v_max ? -0.5 : 0.5));
            }
            else
            {
                result = v_max;
            }

Should merge soon (waiting for Rokas feedback right now).
Thanks again for the help!

[ocornut]

Fixed by @rokups with b2039aa