imgui: API breaking change in 1.78: Slider/Drag final "float power = 1.0f" arg becomes flags

This is a post to discuss a upcoming/possible API breaking change we devised with @ShironekoBen that may have larger side-effects on codebase than the usual breaking changes.

WHY

Both DragFloatXXX and SliderFloatXXX have a trailing float power = 1.0f parameter designed to make editing non-linear (e.g. have more precision in the small values than the large ones). Various people have rightly commented and discussed on the fact that my maths behind that parameter has always been a little peculiar and suggested using logarithmic widgets instead (#642, #1316, #1823). I expected most people who had an incentive of editing with more precision just used power=2.0f or 3.0f and stuck with “whatever it did” which sort of did the job.

Aside from that, there are many requests which would benefits from exposing flags to the outer-most public layer of the Drag and Slider functions. e.g. Enforcing or disabling clamping when using CTRL+Click (#3209, #1829), Vertical dragging behavior, Request underlying value to only be written at end of edit (#701), Mouse wrap-around (#228), data type wrap-around options, disable text input etc…

The bottleneck for adding those flags in the outer-most layer of the public API has always been the function signature are long and I sort of expected the trailing float power=1.0f parameter should go so I’ve been hesitant with adding yet another optional parameter…

SUGGESTED CHANGE

We would like to obsolete the trailing float power = 1.0f and REPLACE it with new set of flags e.g. ImGuiSliderFlags flags = 0. Specifically, ImGuiSliderFlags_Logarithmic would provide a similar-but-difference experience as toying with the power parameter. Addition of flags means many other requested features can more easily be added.

So essentially this family of functions:

bool DragFloat(
   const char* label, float* v, float v_speed = 1.0f, float v_min = 0.0f, float v_max = 0.0f, 
   const char* format = "%.3f", float power = 1.0f);

bool SliderFloat(
   const char* label, float* v, float v_min, float v_max, 
   const char* format = "%.3f", float power = 1.0f);

Will become:

bool DragFloat(
   const char* label, float* v, float v_speed = 1.0f, float v_min = 0.0f, float v_max = 0.0f, 
   const char* format = "%.3f", ImGuiSliderFlags flags = 0);

bool SliderFloat(
  const char* label, float* v, float v_min, float v_max, 
  const char* format = "%.3f", ImGuiSliderFlags flags = 0); 

It’s a rather simple change but in term of breaking API compatibility is quite unusual.

In addition for the most-common functions we would leave a redirection entry point (unless IMGUI_DISABLE_OBSOLETE_FUNCTIONS is defined at compile-time) with the float power argument but without a default, so omitting the last parameter would call the new functions.

WHAT WOULD IT BREAK AND HOW WOULD WE HANDLE IT?

TL;DR;

  • Any call with power > 1.0f should likely be replaced to use the _Logarithmic flag instead. You may be wondering: a single flag is not going to be as expressive as a float, but the way power was handled seemingly it unpractical for people to actually tweak it wisely.

Specifically:

  • Case 1: A call to DragFloat() and SliderFloat() omitting the last parameter. I expect this to be the most common in C++ codebase. For that case, recompiling would just call the new function with no issue.

  • Case 2: A call is made with power = 1.0f and without IMGUI_DISABLE_OBSOLETE_FUNCTIONS: will call the redirection function. It will check that the value if 1.0f and call the new function.

  • Case 3: A call is made with power = 1.0f and with IMGUI_DISABLE_OBSOLETE_FUNCTIONS or after we decide to remove the redirection functions: the new function will be called with 1.0f cast to an integer ImGuiSliderFlags flags = 1. With some - but not all - compiler settings, the compiler will emit a warning: warning C4244: 'argument' : conversion from 'float' to 'ImGuiSliderFlags', possible loss of data Which is hepful to detect the issue. The new function will safely accept the 1 at runtime and ignore it as it is equivalent to the default flags.

  • Case 4: A call is made with power != 1.0f and without IMGUI_DISABLE_OBSOLETE_FUNCTIONS: will call the redirection function. The redirection function will trigger an assert: IM_ASSERT(power == 1.0f && "Call Slider function with ImGuiSliderFlags_Logarithmic instead of using the old 'float power' function!"); As a weird courtesy we’ll also set the ImGuiSliderFlags_Logarithmic flag in this code path, so code compiled without asserts will somehow behave (tempted to remove this courtesy as it may lead user into a false sense that things are ok)

  • Case 5: A call is made with power != 1.0f and with IMGUI_DISABLE_OBSOLETE_FUNCTIONS or after we decide to remove the redirection functions: the new function will be called with float power casted to the integer flag. So power = (float)1.8f will be cast to (int)1, (float)3.1f will be cast to (int)3 etc. With some - but not all - compiler settings, the compiler will emit a warning: warning C4244: 'argument' : conversion from 'float' to 'ImGuiSliderFlags', possible loss of data Which is hepful to detect the issue. Here’s the twist: the new function will assert if any of the lower 4-bit or upper 4-bit of the integer flags are set. The lower 4-bits check means that any value from 0 to 15 (excluding 1) will assert. So floating point value from 0.0f to 16.0f (exclusive) will assert, and they constitute a large enough reasonable range of values that could have been used correctly as the float power parameter. The upper 4-bits check is a little overkill but means that most float >1.0f value which somehow made its way and got reinterpreted as a integer (unlikely in C++, as function signature change will require a rebuild, but may be more possible in high-level language) will be detected and assert. (e.g. 1.0f is stored as 0x3f800000). https://www.h-schmidt.net/FloatConverter/IEEE754.html

A last thing is that ImGuiSliderFlags_Logarithmic do NOT behave the same as any use of power = 2.0f, but in spirit it provide a similar functionality.

HELP NEEDED

I think I got cases covered but since I’m incredibly paranoid about breaking people code in mysterious ways…

I would like people to double-check my reasoning:

  • Under the lens of maximum empathy for people involved large and shared codebase.
  • Under the lens of maximum empathy for casual programmer confused by C++
  • Under the lens of maximum empathy for people using Dear ImGui in other languages bindings, some auto-generated, some not, some with static typing, some not etc… (cc. @mellinoe @sonoro1234)

And then TRY those changes in your codebase so you can help us validate the reasoning and provide feedback. You will ideally want to evaluate if the compiler/asserts are helping you, and then do additional searches/grep in your code to get a feel if you could have missed something.

Branch over master: https://github.com/ocornut/imgui/tree/features/logarithmic_sliders

Branch over docking: https://github.com/ocornut/imgui/tree/features/logarithmic_sliders_docking

POTENTIAL TRANSITION PLAN

If it works, the change will come with:

  • Copious amount of comments in Changelog, API Breaking Changes section, and around related functions.
  • #define IMGUI_VERSION_NUM 17703 will be increased so in the case of code that needs to be shared across versions this may be used to use #ifdef blocks.
  • We will aim at removing redirection functions wrapper sooner than the typical calendar of full obsolescence. Because they are asserting with power!=1.0f they are not of as much use and mainly they are polluting the namespace. So I’m hoping we could get rid of them in 3~12 months if people agree that the transition has been very smooth. If we do that it’ll come with copying amount of commentary.

*EDIT* EPILOGUE This has been merged, still open to feedback. Also added the following new flags for Slider and Drag:

// Flags for SliderFloat(), SliderInt() etc.
enum ImGuiSliderFlags_
{
    ImGuiSliderFlags_None                   = 0,
    ImGuiSliderFlags_ClampOnInput           = 1 << 4,       // Clamp value to min/max bounds when input manually with CTRL+Click. By default CTRL+Click allows going out of bounds.
    ImGuiSliderFlags_Logarithmic            = 1 << 5,       // Make the widget logarithmic (linear otherwise). Consider using ImGuiSliderFlags_NoRoundToFormat with this if using a format-string with small amount of digits.
    ImGuiSliderFlags_NoRoundToFormat        = 1 << 6,       // Disable rounding underlying value to match precision of the display format string (e.g. %.3f values are rounded to those 3 digits)
    ImGuiSliderFlags_NoInput                = 1 << 7        // Disable CTRL+Click or Enter key allowing to input text directly into the widget
};

You can version check with #if IMGUI_VERSION_NUM >= 17704

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 21
  • Comments: 26 (12 by maintainers)

Commits related to this issue

Most upvoted comments

See the comment in that enum:

ImGuiSliderFlags_InvalidMask_ = 0x7000000F    // [Internal] We treat using those bits as being potentially a 'float power' argument from the previous API that has got miscast to this enum, and will trigger an assert if needed.

Not using those bits for valid flags allows to assert for a few invalid uses of API and ABI.

This is now merged in master.

@torkeldanielsson Thanks for reporting this, fixed by 4c201994d421089493a7a996978e8239ad619a20.

(I didn’t add the check in ClampBehaviorT() because that (v_min < v_max) check in DragBehavior (which could be a v_min == v_max) has been pretty inconsistent with other widgets and is mostly a legacy thing that was initially designed to facilitate default value to DragFloat(). Using -FLT_MAX, +FLT_MAX as default bound would be preferable down the line.)

I ran into an issue with ImGuiSliderFlags_ClampOnInput and using min=max=0.0f to signal no limit of range: the ctrl-click input value is clamped to 0.0f. (Dragging the value works as expected.)

Looking briefly at the code I see that DragBehaviorT has “const bool is_clamped = (v_min < v_max);” which seems to be what triggers the non-clamp in the normal drag flow. For the input I found no similar check in “ClampBehaviorT”. I can make a PR adding a check similar to is_clamped to ClampBehaviorT, but I get a feeling there might be some details in how these functions interact with other things so let me know if you want that 😃