Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drag/Slider clamping Min and Max value when using CTRL+Click #1829

Closed
luizimloko opened this issue May 21, 2018 · 6 comments
Closed

Drag/Slider clamping Min and Max value when using CTRL+Click #1829

luizimloko opened this issue May 21, 2018 · 6 comments

Comments

@luizimloko
Copy link

Hello,
So..., how i can limits a value of slider when i use keyboard input ?.

Example:

ImGui::SliderFloat( label, *&value, 1, 10 );

min = 1, max = 10

but if i type by keyboard 100, it will change the variable to 100, even the maximum value being 10.

@sonoro1234
Copy link

@ocornut
Copy link
Owner

ocornut commented Jun 2, 2018 via email

@yshklarov
Copy link

yshklarov commented Apr 8, 2020

For now all you can do is test the return value of SliderFloat() and clamp your value again when it returns true, if your underlying code really needs the value to be clamped.

Testing return values is rather finicky, because the old value will remain in the text input and SliderFloat() will return true in the next frame too (indicating that the value has changed, when in fact it has not). So any code that must perform a special action every time the value is changed needs to have an extra variable to keep track of the "true" value. This is incredibly annoying. The natural expected behavior for a clamped slider is to simply never set the data out-of-bounds.

However, imgui.h contains the comment "Manually input values aren't clamped and can go off-bounds." This almost seems like it's advertising a quasi-feature, and I bet some users are relying on the current behavior.

If we do decide to clamp keyboard inputs, it seems the easiest way would be to modify both TempInputScalar and DataTypeApplyOpFromText (in imgui_widgets.cpp) to accept min and max parameters, and to clamp the value inside the "expression evaluator" DataTypeApplyOpFromText. But it's probably not the right time because the expression evaluator wants to be rewritten. Still, I'll be happy to make the change if you don't object!

Edit: I just found Issue 946, which explains that the current behavior is as intended.

@ocornut ocornut changed the title Slider Min and Max value Drag/Slider Min and Max value May 7, 2020
@ocornut ocornut changed the title Drag/Slider Min and Max value Drag/Slider clamping Min and Max value when using CTRL+Click May 7, 2020
@ocornut
Copy link
Owner

ocornut commented May 7, 2020

Resurrecting this old discussion (will copy this comment to #3209 as well)

So any code that must perform a special action every time the value is changed needs to have an extra variable to keep track of the "true" value. This is incredibly annoying.

You can easily wrap that behavior:

    bool SliderFloatClamp(const char* label, float* v, float v_min, float v_max)
    {
        float v_backup = *v;
        if (!SliderFloat(label, v, v_min, v_max))
            return false;
        if (*v < v_min) *v = v_min;
        else if (*v > v_max) *v = v_max;
        return v_backup != *v;
    }

If you display a color square to "verify" that the return value is correct when out of bound (doesn't return true every frame):

bool ret = ImGui::SliderFloatClamp("float", &f, 0.0f, 1.0f);
ImGui::ColorButton("result", ret ? ImVec4(0, 1, 0, 1) : ImVec4(1, 0, 0, 1));

I've also made internal changes to TempInputScalar() to make it honor clamping BUT neither stock version of Slider and Drag are using this feature:

// Our current specs do NOT clamp when using CTRL+Click manual input, but we should eventually add a flag for that..
if (temp_input_is_active || temp_input_start)
     return TempInputScalar(frame_bb, id, label, data_type, p_data, format);// , p_min, p_max);

And TempInputScalar() now has the following comment:

// Note that Drag/Slider functions are currently NOT forwarding the min/max values clamping values!
// This is intended: this way we allow CTRL+Click manual input to set a value out of bounds, for maximum flexibility.
// However this may not be ideal for all uses, as some user code may break on out of bound values.
// In the future we should add flags to Slider/Drag to specify how to enforce min/max values with CTRL+Click.
// See GitHub issues #1829 and #3209
// In the meanwhile, you can easily "wrap" those functions to enforce clamping, using wrapper functions, e.g.
//   bool SliderFloatClamp(const char* label, float* v, float v_min, float v_max)
//   {
//      float v_backup = *v;
//      if (!SliderFloat(label, v, v_min, v_max))
//         return false;
//      *v = ImClamp(*v, v_min, v_max);
//      return v_backup != *v;
//   }

The only thing missing would be to find a nice way to expose new flags to the Slider/Drag api, being considerate of other desirable features that those function wants (#701), so then we can expose clamping behavior in the function signature. I think the power parameter may be getting in the way of some signature and #1316 suggest we could potentially remove it (and remove with flags such as ImGuiSliderFlags_Logarithmic etc.

ocornut added a commit that referenced this issue Aug 17, 2020
…ampOnInput flags to force clamping value when using CTRL+Click to type in a value manually. (#1829, #3209)
@ocornut
Copy link
Owner

ocornut commented Aug 17, 2020

This is now supported with ImGuiDragFlags_ClampOnInput and ImGuiSliderFlags_ClampOnInput flags. See #3361 for details.

@ocornut ocornut closed this as completed Aug 17, 2020
ocornut added a commit that referenced this issue Aug 18, 2020
…, #1823, #1316, #642, #1829, #3209)

Technically API breaking (but ImGuiDragFlags were pushed on master 16 hours ago)
@ocornut
Copy link
Owner

ocornut commented Sep 25, 2020

FYI renamed ImGuiDragFlags_ClampOnInput to ImGuiSliderFlags_AlwaysClamp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants