Fix `DragValue` range clamping (#5118)

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
* https://github.com/emilk/egui/pull/4728
* https://github.com/emilk/egui/issues/4881
* https://github.com/emilk/egui/pull/4882
This commit is contained in:
Emil Ernerfeldt 2024-09-17 14:44:20 +02:00 committed by GitHub
parent 89da356b79
commit 7d6c83b37f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 60 additions and 22 deletions

View File

@ -41,7 +41,7 @@ pub struct DragValue<'a> {
prefix: String, prefix: String,
suffix: String, suffix: String,
range: RangeInclusive<f64>, range: RangeInclusive<f64>,
clamp_to_range: bool, clamp_existing_to_range: bool,
min_decimals: usize, min_decimals: usize,
max_decimals: Option<usize>, max_decimals: Option<usize>,
custom_formatter: Option<NumFormatter<'a>>, custom_formatter: Option<NumFormatter<'a>>,
@ -72,7 +72,7 @@ impl<'a> DragValue<'a> {
prefix: Default::default(), prefix: Default::default(),
suffix: Default::default(), suffix: Default::default(),
range: f64::NEG_INFINITY..=f64::INFINITY, range: f64::NEG_INFINITY..=f64::INFINITY,
clamp_to_range: true, clamp_existing_to_range: true,
min_decimals: 0, min_decimals: 0,
max_decimals: None, max_decimals: None,
custom_formatter: None, custom_formatter: None,
@ -93,35 +93,75 @@ impl<'a> DragValue<'a> {
/// Sets valid range for the value. /// Sets valid range for the value.
/// ///
/// By default all values are clamped to this range, even when not interacted with. /// 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"] #[deprecated = "Use `range` instead"]
#[inline] #[inline]
pub fn clamp_range<Num: emath::Numeric>(mut self, range: RangeInclusive<Num>) -> Self { pub fn clamp_range<Num: emath::Numeric>(self, range: RangeInclusive<Num>) -> Self {
self.range = range.start().to_f64()..=range.end().to_f64(); self.range(range)
self
} }
/// Sets valid range for dragging the value. /// Sets valid range for dragging the value.
/// ///
/// By default all values are clamped to this range, even when not interacted with. /// 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] #[inline]
pub fn range<Num: emath::Numeric>(mut self, range: RangeInclusive<Num>) -> Self { pub fn range<Num: emath::Numeric>(mut self, range: RangeInclusive<Num>) -> Self {
self.range = range.start().to_f64()..=range.end().to_f64(); self.range = range.start().to_f64()..=range.end().to_f64();
self 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. /// If `false`, only values entered by the user (via dragging or text editing)
/// Dragging will be restricted to the range regardless of this setting. /// will be clamped to the range.
/// Default: `true`. ///
/// ### 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] #[inline]
pub fn clamp_to_range(mut self, clamp_to_range: bool) -> Self { pub fn clamp_existing_to_range(mut self, clamp_existing_to_range: bool) -> Self {
self.clamp_to_range = clamp_to_range; self.clamp_existing_to_range = clamp_existing_to_range;
self 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: " /// Show a prefix before the number, e.g. "x: "
#[inline] #[inline]
pub fn prefix(mut self, prefix: impl ToString) -> Self { pub fn prefix(mut self, prefix: impl ToString) -> Self {
@ -392,7 +432,7 @@ impl<'a> Widget for DragValue<'a> {
mut get_set_value, mut get_set_value,
speed, speed,
range, range,
clamp_to_range, clamp_existing_to_range,
prefix, prefix,
suffix, suffix,
min_decimals, min_decimals,
@ -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()); value = clamp_value_to_range(value, range.clone());
} }
@ -501,9 +541,8 @@ impl<'a> Widget for DragValue<'a> {
// Make sure we applied the last text value: // Make sure we applied the last text value:
let parsed_value = parse(&custom_parser, &value_text); let parsed_value = parse(&custom_parser, &value_text);
if let Some(mut parsed_value) = parsed_value { if let Some(mut parsed_value) = parsed_value {
if clamp_to_range { // User edits always clamps:
parsed_value = clamp_value_to_range(parsed_value, range.clone()); parsed_value = clamp_value_to_range(parsed_value, range.clone());
}
set(&mut get_set_value, parsed_value); set(&mut get_set_value, parsed_value);
} }
} }
@ -537,9 +576,8 @@ impl<'a> Widget for DragValue<'a> {
if update { if update {
let parsed_value = parse(&custom_parser, &value_text); let parsed_value = parse(&custom_parser, &value_text);
if let Some(mut parsed_value) = parsed_value { if let Some(mut parsed_value) = parsed_value {
if clamp_to_range { // User edits always clamps:
parsed_value = clamp_value_to_range(parsed_value, range.clone()); parsed_value = clamp_value_to_range(parsed_value, range.clone());
}
set(&mut get_set_value, parsed_value); set(&mut get_set_value, parsed_value);
} }
} }

View File

@ -822,7 +822,7 @@ impl<'a> Slider<'a> {
let mut dv = DragValue::new(&mut value) let mut dv = DragValue::new(&mut value)
.speed(speed) .speed(speed)
.range(self.range.clone()) .range(self.range.clone())
.clamp_to_range(self.clamp_to_range) .clamp_existing_to_range(self.clamp_to_range)
.min_decimals(self.min_decimals) .min_decimals(self.min_decimals)
.max_decimals_opt(self.max_decimals) .max_decimals_opt(self.max_decimals)
.suffix(self.suffix.clone()) .suffix(self.suffix.clone())