Add clamp_to_range_on_drag option to DragValue and clamp_to_range_on_value_drag option to Slider

[ishbosamiya]

Added egui::DragValue::clamp_to_range_on_drag and egui::Slider::clamp_to_range_on_value_drag to allow the user to determine if the value should be clamped while dragging.

The default for egui::DragValue::clamp_to_range_on_drag and egui::Slider::clamp_to_range_on_value_drag is set to true. This means the default behavior mimics 0.28.

If the default behavior must match 0.27, egui::DragValue::clamp_to_range_on_drag should be true and egui::Slider::clamp_to_range_on_value_drag should be false.

• Closes Regression: dragging value of slider no longer supports out of range values #4881
• I have followed the instructions in the PR template

egui_slider_clamp_to_range_on_value_drag.mp4

[emilk]

Do we really need both this options?
It seems to me that clamp_to_range should clamp both drags and keyboard edits instead.

[ishbosamiya]

I don't think both these options are required. This PR was made assuming that the regression was intentional, it seems like it is not.

I will update the PR soon to use just clamp_to_range.

[ishbosamiya]

With the latest update, the behavior now mimics 0.27 where clamp_to_range is used to determine if the value should be clamped while dragging instead of always clamping.

The PR title will need to be updated, but I am not sure about the guidelines for this in egui, so I am leaving it up to you to edit when merging.

[emilk]

I'm trying to refresh my memory of the last PR: #4728 by @Wumpf

It seems like we want is three modes:

A) no clamping
B) clamping only user input (dragging or editing text), but leave existing value intact
C) always clamp

The difference between mode B and C is:

let mut x = 42.0;
ui.add(DragValue::new(&mut x).range(0.0..=1.0));
// What will `x` be here?

I think the intention of #4728 was that you get the different modes via:

• A): don't call .range()
• B) call .range() and .clamp_to_range(false)
• C) call .range()

At least, that's how I think both Slider and DragValue should work (it seems like the least surprising behavior). But obviously that's not how DragValue currently works, because it checks clamp_to_range too much. I believe the correct fix is to remove these ifs:

egui/crates/egui/src/widgets/drag_value.rs

Line 504 in a48b894

if clamp_to_range {

egui/crates/egui/src/widgets/drag_value.rs

Line 540 in a48b894

if clamp_to_range {

Ping @Wumpf

[Wumpf]

Went through it again and can confirm the behavior of the Slider as described by your comment. It is also reflected in the doc strings just fine.

On how to fix it:
The effect of clamping programmatic input if clamp_to_range is false is done early on here

egui/crates/egui/src/widgets/drag_value.rs

Line 473 in a48b894

if clamp_to_range {

So afterwards, all clamping is about user input. Since we earlier asserted that user input is always clamped against the set range (which may be infinite in either direction, thus not clamping!), we have to apply clamping to range indiscriminately to user input, but not if no user input occurred. Both pieces of code linked by @emilk are about keyboard user input and indeed this means that there we should apply clamping always.
The last piece where we have to clamp to range is when dragging. That happens here

egui/crates/egui/src/widgets/drag_value.rs

Line 615 in a48b894

let rounded_new_value = clamp_value_to_range(rounded_new_value, range.clone());

and already does the right thing

So, yes! What @emilk said :)

Furthermore, documentation on clamping should be equalized between Slider and DragValue, as in both cases the semantics should be the exact same after this change.

[emilk]

Then I'll make a new PR to get us the desired behavior