Skip to content

Commit

Permalink
Fix DragValue range clamping (#5118)
Browse files Browse the repository at this point in the history
When using a `DragValue`, there are three common modes of range clamping
that the user may want:

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:

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

With this PR, we now can get the three behaviors with:

* A): don't call `.range()` (or use `-Inf..=Inf`)
* B) call `.range()` and `.clamp_existing_to_range(false)`
* C) call `.range()`

## Slider clamping
Slider clamping is slightly different, since a slider always has a
range.

For a slider, there are these three cases to consider:

A) no clamping
B) clamp any value that the user enters, but leave existing values
intact
C) always clamp all values

Out of this, C should probably be the default.

I'm not sure what the best API is for this yet. Maybe an `enum` 🤔 


I'll take a pass on that in a future PR.

## Related
* #4728
* #4881
* #4882
  • Loading branch information
emilk authored Sep 17, 2024
1 parent 89da356 commit 7d6c83b
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 22 deletions.
80 changes: 59 additions & 21 deletions crates/egui/src/widgets/drag_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub struct DragValue<'a> {
prefix: String,
suffix: String,
range: RangeInclusive<f64>,
clamp_to_range: bool,
clamp_existing_to_range: bool,
min_decimals: usize,
max_decimals: Option<usize>,
custom_formatter: Option<NumFormatter<'a>>,
Expand Down Expand Up @@ -72,7 +72,7 @@ impl<'a> DragValue<'a> {
prefix: Default::default(),
suffix: Default::default(),
range: f64::NEG_INFINITY..=f64::INFINITY,
clamp_to_range: true,
clamp_existing_to_range: true,
min_decimals: 0,
max_decimals: None,
custom_formatter: None,
Expand All @@ -93,35 +93,75 @@ impl<'a> DragValue<'a> {
/// Sets valid range for the value.
///
/// By default all values are clamped to this range, even when not interacted with.
/// You can change this behavior by passing `false` to [`crate::Slider::clamp_to_range`].
/// You can change this behavior by passing `false` to [`Self::clamp_existing_to_range`].
#[deprecated = "Use `range` instead"]
#[inline]
pub fn clamp_range<Num: emath::Numeric>(mut self, range: RangeInclusive<Num>) -> Self {
self.range = range.start().to_f64()..=range.end().to_f64();
self
pub fn clamp_range<Num: emath::Numeric>(self, range: RangeInclusive<Num>) -> Self {
self.range(range)
}

/// Sets valid range for dragging the value.
///
/// By default all values are clamped to this range, even when not interacted with.
/// You can change this behavior by passing `false` to [`crate::Slider::clamp_to_range`].
/// You can change this behavior by passing `false` to [`Self::clamp_existing_to_range`].
#[inline]
pub fn range<Num: emath::Numeric>(mut self, range: RangeInclusive<Num>) -> Self {
self.range = range.start().to_f64()..=range.end().to_f64();
self
}

/// If set to `true`, all incoming and outgoing values will be clamped to the sliding [`Self::range`] (if any).
/// If set to `true`, existing values will be clamped to [`Self::range`].
///
/// If set to `false`, a value outside of the range that is set programmatically or by user input will not be changed.
/// Dragging will be restricted to the range regardless of this setting.
/// Default: `true`.
/// If `false`, only values entered by the user (via dragging or text editing)
/// will be clamped to the range.
///
/// ### Without calling `range`
/// ```
/// # egui::__run_test_ui(|ui| {
/// let mut my_value: f32 = 1337.0;
/// ui.add(egui::DragValue::new(&mut my_value));
/// assert_eq!(my_value, 1337.0, "No range, no clamp");
/// # });
/// ```
///
/// ### With `.clamp_existing_to_range(true)` (default)
/// ```
/// # egui::__run_test_ui(|ui| {
/// let mut my_value: f32 = 1337.0;
/// ui.add(egui::DragValue::new(&mut my_value).range(0.0..=1.0));
/// assert!(0.0 <= my_value && my_value <= 1.0, "Existing values should be clamped");
/// # });
/// ```
///
/// ### With `.clamp_existing_to_range(false)`
/// ```
/// # egui::__run_test_ui(|ui| {
/// let mut my_value: f32 = 1337.0;
/// let response = ui.add(
/// egui::DragValue::new(&mut my_value).range(0.0..=1.0)
/// .clamp_existing_to_range(false)
/// );
/// if response.dragged() {
/// // The user edited the value, so it should be clamped to the range
/// assert!(0.0 <= my_value && my_value <= 1.0);
/// } else {
/// // The user didn't edit, so our original value should still be here:
/// assert_eq!(my_value, 1337.0);
/// }
/// # });
/// ```
#[inline]
pub fn clamp_to_range(mut self, clamp_to_range: bool) -> Self {
self.clamp_to_range = clamp_to_range;
pub fn clamp_existing_to_range(mut self, clamp_existing_to_range: bool) -> Self {
self.clamp_existing_to_range = clamp_existing_to_range;
self
}

#[inline]
#[deprecated = "Renamed clamp_existing_to_range"]
pub fn clamp_to_range(self, clamp_to_range: bool) -> Self {
self.clamp_existing_to_range(clamp_to_range)
}

/// Show a prefix before the number, e.g. "x: "
#[inline]
pub fn prefix(mut self, prefix: impl ToString) -> Self {
Expand Down Expand Up @@ -392,7 +432,7 @@ impl<'a> Widget for DragValue<'a> {
mut get_set_value,
speed,
range,
clamp_to_range,
clamp_existing_to_range,
prefix,
suffix,
min_decimals,
Expand Down Expand Up @@ -470,7 +510,7 @@ impl<'a> Widget for DragValue<'a> {
});
}

if clamp_to_range {
if clamp_existing_to_range {
value = clamp_value_to_range(value, range.clone());
}

Expand Down Expand Up @@ -501,9 +541,8 @@ impl<'a> Widget for DragValue<'a> {
// Make sure we applied the last text value:
let parsed_value = parse(&custom_parser, &value_text);
if let Some(mut parsed_value) = parsed_value {
if clamp_to_range {
parsed_value = clamp_value_to_range(parsed_value, range.clone());
}
// User edits always clamps:
parsed_value = clamp_value_to_range(parsed_value, range.clone());
set(&mut get_set_value, parsed_value);
}
}
Expand Down Expand Up @@ -537,9 +576,8 @@ impl<'a> Widget for DragValue<'a> {
if update {
let parsed_value = parse(&custom_parser, &value_text);
if let Some(mut parsed_value) = parsed_value {
if clamp_to_range {
parsed_value = clamp_value_to_range(parsed_value, range.clone());
}
// User edits always clamps:
parsed_value = clamp_value_to_range(parsed_value, range.clone());
set(&mut get_set_value, parsed_value);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/widgets/slider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ impl<'a> Slider<'a> {
let mut dv = DragValue::new(&mut value)
.speed(speed)
.range(self.range.clone())
.clamp_to_range(self.clamp_to_range)
.clamp_existing_to_range(self.clamp_to_range)
.min_decimals(self.min_decimals)
.max_decimals_opt(self.max_decimals)
.suffix(self.suffix.clone())
Expand Down

0 comments on commit 7d6c83b

Please sign in to comment.