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

SliderFloat clamping to zero too aggressively with logarithms #642

Closed
josiahmanson opened this issue May 9, 2016 · 21 comments
Closed

SliderFloat clamping to zero too aggressively with logarithms #642

josiahmanson opened this issue May 9, 2016 · 21 comments

Comments

@josiahmanson
Copy link
Contributor

josiahmanson commented May 9, 2016

I ran into a problem with the logarithmic slider. I am trying to control some weighting parameters that can be very small. I display in scientific format with only a few decimal places (for example "%.3g"), but need to support values in the range 1e-9 to 1, for example. The current behavior clamps the slider to the precision of the format string, which causes a huge jump at low values. Basically, there is a connection between display format and clamping currently that doesn't make much sense to me.

I got rid of the rounding by commenting out the line shown below in ImGui::SliderBehavior. I'm only taking logs of positive numbers, so I don't need clamping (min value is sufficient). You clearly want support negative numbers also, so a more general solution would maybe be to only allow clamping for non-logarithmic sliders? Maybe pass the clamp value as a separate parameter from the display format?

// Round past decimal precision
new_value = RoundScalar(new_value, decimal_precision);
@josiahmanson
Copy link
Contributor Author

josiahmanson commented May 10, 2016

There is also a problem with the minimum value that I pass in not being all the way on the left side of the slider. If you try the code below, you will notice that the minimum value is in the middle of the slider instead of on the left. The maximum value is correctly located on the right though.

float v;
ImGui::SliderFloat( "Test", &v, .1, 1, "%.3g", 10 );

@ocornut
Copy link
Owner

ocornut commented May 10, 2016

there is a connection between display format and clamping currently that doesn't make much sense to me.
Maybe pass the clamp value as a separate parameter from the display format?

It makes sense for many use cases! It's nice to have those rounded values and not always get "1.2673476" every time one click on a typical slicker. And we actually have display matching the real value. Adding extra parameters to those functions not really trivial (API explosion issue).

So while I agree that we should allow to disable that clamping I haven't yet figured out what is the best path toward making it happen. Probably need to play a little more with that sort of range too.

The later (second message) is very definitively a bug. Should look into it.

This is closely related to your other issue #643 that the internal of those functions probably need a reoverhaul.

@josiahmanson
Copy link
Contributor Author

josiahmanson commented May 10, 2016

I understand the problem now. You are doing the clamp because you want the value that is stored in the float to match what is displayed on the screen. I was thinking it was to avoid the issue that log(0) == -infinity, which could be handled with a fixed sized epsilon that is not tied to the display format.

The most general, easiest way to get the behavior you want is probably not to create your own parsing code to find decimal_precision and make that general to handle the random format strings that can be used (e.g. %.3f, %.3e, %.3g are all a little different). Just use the parser in stdlib to do the heavy lifting for you.

As a proof of concept, I routed the format string through SliderBehavior and replaced your rounding function with the code below. It worked great!

float ImGui::RoundScalar( float value, const char *fmt )
{
    char buf[64];
    sprintf( buf, fmt, value );
    return (float)atof( buf );
}

I noticed you had a comment. // FIXME: Investigate better rounding methods FIXED!

clipboard01

@ocornut
Copy link
Owner

ocornut commented May 11, 2016

You are doing the clamp because you want the value that is stored in the float to match what is displayed on the screen

That, and if you are authoring data interactively (say: level designer or game designer tweaking stuff) and those data are meant to be seen by developers, it is just nicer to have round number like 0.001f and not float with lots of significant digits everywhere, when that precision isn't required. So the user can explicitly request more precision if they want it.

FYI other less obvious but very useful use of those format strings include:

  • Including prefixes or suffixes to the display (which is why we parse it ourselves)
  • Actually omitting a format at all, which can be used to just feed strings and have mini-enums style labels within the slider or drag widgets. It makes much more sense with integers than float obviously.
  • If you don't mind one frame of lag with what the display shows (it won't affect interactions) you can create the full the label yourself and also omit using % at all.
  • Also note bug ImGui::SliderInt without %.0f display format behaves diffrently... #467 which is related to this discussion.

Mentioning those because we probably need to have the full scope in mind if changing this code.

@ocornut
Copy link
Owner

ocornut commented May 16, 2016

@josiahmanson

There is also a problem with the minimum value that I pass in not being all the way on the left side of the slider. If you try the code below, you will notice that the minimum value is in the middle of the slider instead of on the left. The maximum value is correctly located on the right though.

float v;
ImGui::SliderFloat( "Test", &v, .1, 1, "%.3g", 10 );

I am not sure this is a bug per se. ^10 curve is very flat in small number before going very steep. With 3 decimals of precision it takes moving to the right for a big portion of that range until we hit a value that is bigger >= 0.101f. You can either increase the precision or reduce the power curve steepness.

@josiahmanson
Copy link
Contributor Author

In my view, not having the ends of the slider map to the min and max values in the specified input range is a bug. You can argue that the impact of the bug is minimal and not worth addressing (maybe that is what you mean by an enhancement). It is a valid point that this is unlikely to affect most users. I made a contrived example to make the problem easily reproducible, but I originally noticed the problem as a hitch at the bottom range of the slider with a much smaller minimum value.

I don't see how using a log10 slider is at all unreasonable to support. Ten is a large power, but through an accident of history and biology, that is the basis for our number system.

@josiahmanson
Copy link
Contributor Author

Now that I think about it, I guess this isn't a logarithmic slider, because a log is a log is a log. The spacing shouldn't change based on power. I don't know what the slider is doing or maybe I'm confusing myself. Anyhow, the ends not matching up is weird. Is there a logarithmic slider in ImGui that I didn't notice? I just assumed that was what the power parameter affected. A log is what I really want. :-)

@ocornut
Copy link
Owner

ocornut commented May 17, 2016

In my view, not having the ends of the slider map to the min and max values in the specified input range is a bug.

If you plot x^10 from 0 to 1, about half on the left side would be below 0.101.
Therefore with three digits only, the range from 0.100 to 0.101 should cover so much space and be represented by the same values. It is very odd indeed! This relates to your request where you were surprised that the format string is guiding precision of the floating point value. If you need to manipulate values between 0.100 and 0.101 then you need more digits of precision.

I'm not sure what you mean exactly by logarithmic slider here. Not acting dumb but everybody may have a slightly different perception at how non-linear sliders may work. The behavior I'm using for SliderFloat() may be totally inadequate! I haven't used scientific/maths apps so I may be totally off your habits. You can play with simpler range and powers (like 2.0f) to have a feel for it.

ImGui::SliderFloat( "Test", &v, 0.0f, 4.0f, "%.3f", 2 );

At 50% of the slider we are at 1
At ~71% of the slider we are at 2
At 100% of the slider we are at 4

The SliderBehavior code for positive values/ranges does: (some unrelated code omited)

// value from 0.0 (left-most) to 1.0 (right-most)
float normalized_pos = ImClamp((mouse_abs_pos - slider_usable_pos_min) / slider_usable_sz, 0.0f, 1.0f);

float new_value;
if (is_non_linear)
{
        // Positive: rescale to the positive range before powering
        float a;
        if (fabsf(linear_zero_pos - 1.0f) > 1.e-6f)
            a = (normalized_pos - linear_zero_pos) / (1.0f - linear_zero_pos);
        else
            a = normalized_pos;
        a = powf(a, power);
        new_value = ImLerp(ImMax(v_min,0.0f), v_max, a);
}

It may be odd to counter-intuitive for you. I am not against changing it.

@AndrewBelt
Copy link

AndrewBelt commented Sep 6, 2017

I've spent a while trying to get my mind around the power argument for sliders. There seems to be no way to make a logarithmic slider other than using a linear variable and fake the text on the slider. A logarithmic slider simply means value = base^position with rescaling of course. For example, a slider with a minimum of 0.01 and maximum of 100.0 would look like this

[0.01========0.1========1.0========10.0========100.0]

Note that 1.0 is precisely in the middle because log_10 1 is the midpoint of log_10 0.01 and log_10 100.
A slider with minimum 1 and maximum 1024 would look like this

[1==2==4==8==16==32==64==128==256==512==1024]

The current slider implementation seems to be polynomial with position = value^1/power, not logarithmic. I'm not sure why this would be useful as it's a strange mathematical relationship. It would break code to change this behavior, but I'm in favor of it, since I see no application of the existing 1/power polynomial relationship and thus no sane code should be using it.

EDIT: Scaling is done by value = min_value * (max_value / min_value)^p where p is position remapped to the range [0, 1]. We could use power = 0.0 (which would result in a divide-by-zero error in the current code) to signify actual logarithmic scaling, to remain backwards compatible.

@ocornut
Copy link
Owner

ocornut commented Sep 6, 2017

@AndrewBelt As stated above I don't mind changing that. That code is something I stupidly grabbed from another piece of code I wrote in 2009 and I didn't give it much though back then. Also, please, nobody should trust me on anything related to maths :)
Do you have a patch proposal?

@AndrewBelt
Copy link

Working on one! (See my edit if you missed it)

@ocornut
Copy link
Owner

ocornut commented Sep 6, 2017

I'm not sure why this would be useful as it's a strange mathematical relationship. It would break code to change this behavior, but I'm in favor of it, since I see no application of the existing 1/power polynomial relationship and thus no sane code should be using it.

@AndrewBelt As per your PR and comment above: I don't think backward compatibility is most important there as long as we explain the change. What would be your preferred design/api if backward compatibility was ignored?

@AndrewBelt
Copy link

Existing:

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

Proposed:

SliderFloat(const char* label, float* v, float v_min, float v_max, const char* display_format = "%.3f", bool logarithmic = false);

To clean up the code, the if (is_non_linear) blocks in imgui.cpp should be removed.

@ocornut
Copy link
Owner

ocornut commented Sep 6, 2017

Do you think base-e natural logarithm is the only base that would be useful?
I'd prefer to avoid a bool parameter, experience told me they always become an issue. I wonder if this could be incorporated in a flag set, allowing for other extensions of sliders.

How would power be patched for the DragFloat function?

@AndrewBelt
Copy link

The beautiful thing about logarithms is that a change of base means a simple rescaling log_b(x) = log(x) / log(b), and since the scaling factors are based on v_min and v_max, all bases would give the same number for

logf(v_clamped / v_min) / logf(v_max / v_min)

since the log(b) of the top log would cancel with the bottom log.

Boolean arguments are indeed controversial, but the recommended alternative is always a new function with a different name suggesting its different behavior. Following that logic, there should be a SliderFloatLog function, but since that would be extremely redundant code, you would want to implement internally it with a boolean argument in SliderFloat anyway and call SliderFloat(..., true) from the new hypothetical SliderFloatLog function.

If you want to keep power != 1.0, you could call the existing nonlinear scaling "polynomial" or "power" scaling because maybe position = value^2 could still be useful to someone. In that case, linear is actually a specific case (since position = value^1) so that doesn't need to be implemented differently.

@AndrewBelt
Copy link

DragFloat is possible, I'll look at that. Instead of adding some delta_v to the value, you'd multiply it by some number, like v *= mouse.x * some_constant and then of course clamp it.

@ocornut
Copy link
Owner

ocornut commented Sep 6, 2017

Boolean arguments are indeed controversial, but the recommended alternative is always a new function with a different name suggesting its different behavior.

I think SliderFloatLog may be better there, if we assume that SliderFloat2/3/4 aren't of much use with logarithmic sliders? And internally we can have a SliderFloatEx() with the bool that's fine.

(It will probably end up being exposed as a SliderFlags_XXX in the future, it's just that I cannot think of much Slider flags for now so it's weird exposing it. At minimum, I envision that in the future we'll have Slider variations, with a configurable default, and an style/behavior override through the flags.)

I'll ask around to people who may have been using that power parameter. Don't worry about the API side in the PR for now (I can always merge and rework it). Thanks!

@AndrewBelt
Copy link

AndrewBelt commented Sep 6, 2017

Thinking about it a bit more, it might be best to keep the old API and improve the power parameter to be an actual "bipolar power" relationship. Here's a description.

_20170906_165357

I realized that there are 9 or 10 widgets that use power, so it would be better to encode the different types of nonlinear scaling inside this parameter, since it actually makes sense to use a float with piecewise logic. We're also open to a new method, should someone need something else, by using negative values of power.

@lwiklendt
Copy link

A workaround for those wanting logarithmic input is to sprintf a current value into a string to pass it to the format, then take the log of the value, set the slider range to the log of the desired min and max, then take the exponential of the result.

char val_str[64];
sprintf(val_str, "%.3f", val);
val = log(val);
ImGui::SliderFloat("Example", &val, log(0.01), log(100.0), val_str);
val = exp(val);

@ocornut
Copy link
Owner

ocornut commented Jul 23, 2020

Hello, (repost with missing contents)

EDIT Removed suggestion that rounding to format has been fixed/removed: it has NOT but we are not able to add the option trivially. In fact I just did! See aad61cb.

Currently reopening that dangling topic since we've working on variety of related changes (see #3361)
I realize @josiahmanson that it's been a long time and you may not be working on the same project or on dear imgui even, but if you are it would be nice if you can evaluate the branch in #3361

Quoting Ben:

_"I've pushed some experiments in logarithmic slider behaviour to features/logarithmic_sliders. I haven't done any API refactoring work yet - this is purely about the maths so far, and trying to take the basic concept of #1316 and make it usable for a range of cases (since that PR as-is I felt like has way too many user-facing non-obvious gotchas/bugs - any range where zero or negative numbers was involved would break in weird ways, for example, as would "backwards" ranges).
I think I've got something now that's a reasonably sane mix of "mathematically sensible" and "intuitively does what the user would probably expect when they turn on logarithmic mode (as opposed to what logarithms actually do really!)". In particular it makes use of the extracting precision from the format string trick (which is really cool!) to intuit what sort of lower bound the user likely wants on their range, and does a colossal fudge around slider min/max/zero-crossing points to produce reasonable-looking results.
I've checked in a simple set of test sliders [...], which demonstrate linear vs log sliders for a set of different types of range.
Any feedback on how sensible this feels (even just in the form of a "drag the sliders up/down and watch the values and see how nice/nasty they feel") would be greatly appreciated.
There's still a bit of a question mark over the use of zero in ranges that cross it - I added a fudge that makes it technically possible to select zero, but it requires you to hit T=0.5 exactly on the slider, which is a bit awkward in practice... I'm thinking adding a deadzone of sorts there but does anyone have any better ideas (or a fundamental hatred of the idea of a deadzone in the middle of a slider, for that matter)?

[...]

"The log() stuff in #1316 is fine as long as the range doesn't cross 0, IIRC, but inherently logarithmic stuff kinda blows up at zero so to work with sliders that do (e.g.) -100 to 100 as their range the code has to split that into two logarithmic ranges and handle them separately. Also, ranges to go down to zero without crossing it also need careful clamping to avoid the clamp making the log behaviour blow up (not "blow up" mathematically, as such, but generate unhelpful things like a 0-10 range slider where 95% of the slider pixels are dedicated to the 0-0.01 part of the range)."

ocornut pushed a commit that referenced this issue Aug 17, 2020
ocornut pushed a commit that referenced this issue Aug 17, 2020
ocornut added a commit that referenced this issue Aug 17, 2020
ocornut added a commit that referenced this issue Aug 17, 2020
…rough if power=1.0f, otherwise assert + safe fallback. Remove 3 redirection functions (#3361, #1823, #1316, #642)
ocornut added a commit that referenced this issue Aug 17, 2020
ocornut pushed a commit that referenced this issue Aug 17, 2020
@ocornut
Copy link
Owner

ocornut commented Aug 17, 2020

This is now fixed, added explicit _Logarithmic flags and _NoRoundToFormat flags to decorrelate rounding underlying value from formatted value. 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)
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