From 959cd19af39321842803ba96b806ed690d0a2042 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 17 Sep 2024 15:04:48 +0200 Subject: [PATCH 1/2] Add `Slider::clamping` for precise clamp control --- crates/egui/src/response.rs | 6 ++ crates/egui/src/widgets/drag_value.rs | 3 +- crates/egui/src/widgets/mod.rs | 2 +- crates/egui/src/widgets/slider.rs | 127 +++++++++++++++++++---- crates/egui_demo_lib/src/demo/sliders.rs | 21 ++-- 5 files changed, 129 insertions(+), 30 deletions(-) diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index b2fdd5e283e6..b358fc3be78f 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -116,6 +116,9 @@ pub struct Response { /// /// e.g. the slider was dragged, text was entered in a [`TextEdit`](crate::TextEdit) etc. /// Always `false` for something like a [`Button`](crate::Button). + /// + /// Note that this can be `true` even if the user did not interact with the widget, + /// for instance if an existing slider value was clamped to the given range. #[doc(hidden)] pub changed: bool, } @@ -496,6 +499,9 @@ impl Response { /// /// This is not set if the *view* of the data was changed. /// For instance, moving the cursor in a [`TextEdit`](crate::TextEdit) does not set this to `true`. + /// + /// Note that this can be `true` even if the user did not interact with the widget, + /// for instance if an existing slider value was clamped to the given range. #[inline(always)] pub fn changed(&self) -> bool { self.changed diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 2ae3585f77dc..14d1d8c45b46 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -737,7 +737,8 @@ fn default_parser(text: &str) -> Option { text.parse().ok() } -fn clamp_value_to_range(x: f64, range: RangeInclusive) -> f64 { +/// Clamp the given value with careful handling of negative zero, and other corner cases. +pub(crate) fn clamp_value_to_range(x: f64, range: RangeInclusive) -> f64 { let (mut min, mut max) = (*range.start(), *range.end()); if min.total_cmp(&max) == Ordering::Greater { diff --git a/crates/egui/src/widgets/mod.rs b/crates/egui/src/widgets/mod.rs index c6ab94ee82c9..e75a232cfd36 100644 --- a/crates/egui/src/widgets/mod.rs +++ b/crates/egui/src/widgets/mod.rs @@ -37,7 +37,7 @@ pub use self::{ radio_button::RadioButton, selected_label::SelectableLabel, separator::Separator, - slider::{Slider, SliderOrientation}, + slider::{Slider, SliderClamping, SliderOrientation}, spinner::Spinner, text_edit::{TextBuffer, TextEdit}, }; diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index f06d5924c041..c75c79e5a50a 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -8,6 +8,8 @@ use crate::{ TextWrapMode, Ui, Vec2, Widget, WidgetInfo, WidgetText, MINUS_CHAR_STR, }; +use super::drag_value::clamp_value_to_range; + // ---------------------------------------------------------------------------- type NumFormatter<'a> = Box) -> String>; @@ -44,11 +46,35 @@ struct SliderSpec { } /// Specifies the orientation of a [`Slider`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub enum SliderOrientation { Horizontal, Vertical, } +/// Specifies how values in a [`Slider`] are clamped. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] +pub enum SliderClamping { + /// Values are not clamped. + /// + /// This means editing the value with the keyboard, + /// or dragging the number next to the slider will always work. + /// + /// The actual slider part is always clamped though. + Never, + + /// Users cannot enter new values that are outside the range. + /// + /// Existing values remain intact though. + Edits, + + /// Always clamp values, even existing ones. + #[default] + Always, +} + /// Control a number with a slider. /// /// The slider range defines the values you get when pulling the slider to the far edges. @@ -73,7 +99,7 @@ pub struct Slider<'a> { get_set_value: GetSetValue<'a>, range: RangeInclusive, spec: SliderSpec, - clamp_to_range: bool, + clamping: SliderClamping, smart_aim: bool, show_value: bool, orientation: SliderOrientation, @@ -95,6 +121,9 @@ pub struct Slider<'a> { impl<'a> Slider<'a> { /// Creates a new horizontal slider. + /// + /// The `value` given will be clamped to the `range`, + /// unless you change this behavior with [`Self::slider_clamping`]. pub fn new(value: &'a mut Num, range: RangeInclusive) -> Self { let range_f64 = range.start().to_f64()..=range.end().to_f64(); let slf = Self::from_get_set(range_f64, move |v: Option| { @@ -123,7 +152,7 @@ impl<'a> Slider<'a> { smallest_positive: 1e-6, largest_finite: f64::INFINITY, }, - clamp_to_range: true, + clamping: SliderClamping::default(), smart_aim: true, show_value: true, orientation: SliderOrientation::Horizontal, @@ -218,14 +247,59 @@ impl<'a> Slider<'a> { self } - /// If set to `true`, all incoming and outgoing values will be clamped to the slider range. - /// Default: `true`. + /// Controls when the values will be clamped to the range. + /// + /// ### With `.clamping(SliderClamping::Always)` (default) + /// ``` + /// # egui::__run_test_ui(|ui| { + /// let mut my_value: f32 = 1337.0; + /// ui.add(egui::Slider::new(&mut my_value, 0.0..=1.0)); + /// assert!(0.0 <= my_value && my_value <= 1.0, "Existing value should be clamped"); + /// # }); + /// ``` + /// + /// ### With `.clamping(SliderClamping::Edits)` + /// ``` + /// # egui::__run_test_ui(|ui| { + /// let mut my_value: f32 = 1337.0; + /// let response = ui.add( + /// egui::Slider::new(&mut my_value, 0.0..=1.0) + /// .clamping(egui::SliderClamping::Edits) + /// ); + /// if response.dragged() { + /// // The user edited the value, so it should now be clamped to the range + /// assert!(0.0 <= my_value && my_value <= 1.0); + /// } + /// # }); + /// ``` + /// + /// ### With `.clamping(SliderClamping::Never)` + /// ``` + /// # egui::__run_test_ui(|ui| { + /// let mut my_value: f32 = 1337.0; + /// let response = ui.add( + /// egui::Slider::new(&mut my_value, 0.0..=1.0) + /// .clamping(egui::SliderClamping::Never) + /// ); + /// // The user could have set the value to anything + /// # }); + /// ``` #[inline] - pub fn clamp_to_range(mut self, clamp_to_range: bool) -> Self { - self.clamp_to_range = clamp_to_range; + pub fn clamping(mut self, clamping: SliderClamping) -> Self { + self.clamping = clamping; self } + #[inline] + #[deprecated = "Use `slider.clamping(…) instead"] + pub fn clamp_to_range(self, clamp_to_range: bool) -> Self { + self.clamping(if clamp_to_range { + SliderClamping::Always + } else { + SliderClamping::Never + }) + } + /// Turn smart aim on/off. Default is ON. /// There is almost no point in turning this off. #[inline] @@ -531,21 +605,18 @@ impl<'a> Slider<'a> { fn get_value(&mut self) -> f64 { let value = get(&mut self.get_set_value); - if self.clamp_to_range { - let start = *self.range.start(); - let end = *self.range.end(); - value.clamp(start.min(end), start.max(end)) + if self.clamping == SliderClamping::Always { + clamp_value_to_range(value, self.range.clone()) } else { value } } fn set_value(&mut self, mut value: f64) { - if self.clamp_to_range { - let start = *self.range.start(); - let end = *self.range.end(); - value = value.clamp(start.min(end), start.max(end)); + if self.clamping != SliderClamping::Never { + value = clamp_value_to_range(value, self.range.clone()); } + if let Some(step) = self.step { let start = *self.range.start(); value = start + ((value - start) / step).round() * step; @@ -821,12 +892,21 @@ impl<'a> Slider<'a> { let response = ui.add({ let mut dv = DragValue::new(&mut value) .speed(speed) - .range(self.range.clone()) - .clamp_existing_to_range(self.clamp_to_range) .min_decimals(self.min_decimals) .max_decimals_opt(self.max_decimals) .suffix(self.suffix.clone()) .prefix(self.prefix.clone()); + + match self.clamping { + SliderClamping::Never => {} + SliderClamping::Edits => { + dv = dv.range(self.range.clone()).clamp_existing_to_range(false); + } + SliderClamping::Always => { + dv = dv.range(self.range.clone()).clamp_existing_to_range(true); + } + } + if let Some(fmt) = &self.custom_formatter { dv = dv.custom_formatter(fmt); }; @@ -855,6 +935,10 @@ impl<'a> Slider<'a> { fn add_contents(&mut self, ui: &mut Ui) -> Response { let old_value = self.get_value(); + if self.clamping == SliderClamping::Always { + self.set_value(old_value); + } + let thickness = ui .text_style_height(&TextStyle::Body) .at_least(ui.spacing().interact_size.y); @@ -875,10 +959,10 @@ impl<'a> Slider<'a> { } builder.add_action(Action::SetValue); - let clamp_range = if self.clamp_to_range { - self.range() - } else { + let clamp_range = if self.clamping == SliderClamping::Never { f64::NEG_INFINITY..=f64::INFINITY + } else { + self.range() }; if value < *clamp_range.end() { builder.add_action(Action::Increment); @@ -1090,6 +1174,9 @@ fn logarithmic_zero_cutoff(min: f64, max: f64) -> f64 { }; let cutoff = min_magnitude / (min_magnitude + max_magnitude); - debug_assert!(0.0 <= cutoff && cutoff <= 1.0); + debug_assert!( + 0.0 <= cutoff && cutoff <= 1.0, + "Bad cutoff {cutoff:?} for min {min:?} and max {max:?}" + ); cutoff } diff --git a/crates/egui_demo_lib/src/demo/sliders.rs b/crates/egui_demo_lib/src/demo/sliders.rs index 1d435d4819bf..d15d9aa31006 100644 --- a/crates/egui_demo_lib/src/demo/sliders.rs +++ b/crates/egui_demo_lib/src/demo/sliders.rs @@ -1,4 +1,4 @@ -use egui::{style::HandleShape, Slider, SliderOrientation, Ui}; +use egui::{style::HandleShape, Slider, SliderClamping, SliderOrientation, Ui}; use std::f64::INFINITY; /// Showcase sliders @@ -9,7 +9,7 @@ pub struct Sliders { pub min: f64, pub max: f64, pub logarithmic: bool, - pub clamp_to_range: bool, + pub clamping: SliderClamping, pub smart_aim: bool, pub step: f64, pub use_steps: bool, @@ -26,7 +26,7 @@ impl Default for Sliders { min: 0.0, max: 10000.0, logarithmic: true, - clamp_to_range: false, + clamping: SliderClamping::Always, smart_aim: true, step: 10.0, use_steps: false, @@ -61,7 +61,7 @@ impl crate::View for Sliders { min, max, logarithmic, - clamp_to_range, + clamping, smart_aim, step, use_steps, @@ -97,7 +97,7 @@ impl crate::View for Sliders { ui.add( Slider::new(&mut value_i32, (*min as i32)..=(*max as i32)) .logarithmic(*logarithmic) - .clamp_to_range(*clamp_to_range) + .clamping(*clamping) .smart_aim(*smart_aim) .orientation(orientation) .text("i32 demo slider") @@ -110,7 +110,7 @@ impl crate::View for Sliders { ui.add( Slider::new(value, (*min)..=(*max)) .logarithmic(*logarithmic) - .clamp_to_range(*clamp_to_range) + .clamping(*clamping) .smart_aim(*smart_aim) .orientation(orientation) .text("f64 demo slider") @@ -188,9 +188,14 @@ impl crate::View for Sliders { ui.label("Logarithmic sliders can include infinity and zero."); ui.add_space(8.0); - ui.checkbox(clamp_to_range, "Clamp to range"); + ui.horizontal(|ui| { + ui.label("Clamping:"); + ui.selectable_value(clamping, SliderClamping::Never, "Never"); + ui.selectable_value(clamping, SliderClamping::Edits, "Edits"); + ui.selectable_value(clamping, SliderClamping::Always, "Always"); + }); ui.label("If true, the slider will clamp incoming and outgoing values to the given range."); - ui.label("If false, the slider can shows values outside its range, and you can manually enter values outside the range."); + ui.label("If false, the slider can show values outside its range, and you cannot enter new values outside the range."); ui.add_space(8.0); ui.checkbox(smart_aim, "Smart Aim"); From f5a57b14af72e6e8c10945f17bc168fdaa43ca44 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 17 Sep 2024 15:16:44 +0200 Subject: [PATCH 2/2] Fix doclink --- crates/egui/src/widgets/slider.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/widgets/slider.rs b/crates/egui/src/widgets/slider.rs index c75c79e5a50a..7b0cb074ea54 100644 --- a/crates/egui/src/widgets/slider.rs +++ b/crates/egui/src/widgets/slider.rs @@ -123,7 +123,7 @@ impl<'a> Slider<'a> { /// Creates a new horizontal slider. /// /// The `value` given will be clamped to the `range`, - /// unless you change this behavior with [`Self::slider_clamping`]. + /// unless you change this behavior with [`Self::clamping`]. pub fn new(value: &'a mut Num, range: RangeInclusive) -> Self { let range_f64 = range.start().to_f64()..=range.end().to_f64(); let slf = Self::from_get_set(range_f64, move |v: Option| {