From 8394dc9bd69d699453d16b08b7dddfef6d9e8cc7 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 10 Oct 2024 11:15:51 +0200 Subject: [PATCH] Add UI for precisely picking an exact sequence time (#7673) ### What * Part of https://github.com/rerun-io/rerun/issues/7653 I only implemented it for sequence timelines for now, since I didn't want to parse timestamps in different formats this early in the morning. Hopefully this can still help @teh-cmc. It also shows how to do it for temporal timelines if someone wants to follow my lead. ![precise-time-control](https://github.com/user-attachments/assets/2577a551-5ce2-4f18-a396-37c4488998a9) ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using examples from latest `main` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7673?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [rerun.io/viewer](https://rerun.io/viewer/pr/7673?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! * [x] If have noted any breaking changes to the log API in `CHANGELOG.md` and the migration guide - [PR Build Summary](https://build.rerun.io/pr/7673) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) To run all checks from `main`, comment on the PR with `@rerun-bot full-check`. --- .../store/re_log_types/src/time_point/mod.rs | 42 ++++++++++++++++++- .../re_log_types/src/time_point/time_int.rs | 16 +++---- crates/utils/re_format/src/lib.rs | 14 +++++++ crates/viewer/re_time_panel/src/lib.rs | 29 +++++++++++-- 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/crates/store/re_log_types/src/time_point/mod.rs b/crates/store/re_log_types/src/time_point/mod.rs index 6049e9602e07..dbb5ea5bda21 100644 --- a/crates/store/re_log_types/src/time_point/mod.rs +++ b/crates/store/re_log_types/src/time_point/mod.rs @@ -127,7 +127,22 @@ impl TimeType { } } - #[inline] + pub fn format_sequence(time_int: TimeInt) -> String { + Self::Sequence.format(time_int, TimeZone::Utc) + } + + pub fn parse_sequence(s: &str) -> Option { + match s { + "" => Some(TimeInt::STATIC), + "−∞" => Some(TimeInt::MIN), + "+∞" => Some(TimeInt::MAX), + _ => { + let s = s.strip_prefix('#').unwrap_or(s); + re_format::parse_i64(s).map(TimeInt::new_temporal) + } + } + } + pub fn format( &self, time_int: impl Into, @@ -136,7 +151,7 @@ impl TimeType { let time_int = time_int.into(); match time_int { TimeInt::STATIC => "".into(), - TimeInt::MIN => "-∞".into(), + TimeInt::MIN => "−∞".into(), TimeInt::MAX => "+∞".into(), _ => match self { Self::Time => Time::from(time_int).format(time_zone_for_timestamps), @@ -221,3 +236,26 @@ impl, const N: usize> From<[(Timeline, T); N]> for TimePoint ) } } + +// ---------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::{TimeInt, TimeType}; + + #[test] + fn test_format_parse() { + let cases = [ + (TimeInt::STATIC, ""), + (TimeInt::MIN, "−∞"), + (TimeInt::MAX, "+∞"), + (TimeInt::new_temporal(-42), "#−42"), + (TimeInt::new_temporal(12345), "#12 345"), + ]; + + for (int, s) in cases { + assert_eq!(TimeType::format_sequence(int), s); + assert_eq!(TimeType::parse_sequence(s), Some(int)); + } + } +} diff --git a/crates/store/re_log_types/src/time_point/time_int.rs b/crates/store/re_log_types/src/time_point/time_int.rs index de51b6da6c1e..46e73dca4721 100644 --- a/crates/store/re_log_types/src/time_point/time_int.rs +++ b/crates/store/re_log_types/src/time_point/time_int.rs @@ -60,8 +60,8 @@ impl TimeInt { pub const ONE: Self = Self(Some(NonMinI64::ONE)); #[inline] - pub fn is_static(&self) -> bool { - *self == Self::STATIC + pub fn is_static(self) -> bool { + self == Self::STATIC } /// Creates a new temporal [`TimeInt`]. @@ -100,7 +100,7 @@ impl TimeInt { /// Returns `i64::MIN` for [`Self::STATIC`]. #[inline] - pub const fn as_i64(&self) -> i64 { + pub const fn as_i64(self) -> i64 { match self.0 { Some(t) => t.get(), None => i64::MIN, @@ -109,7 +109,7 @@ impl TimeInt { /// Returns `f64::MIN` for [`Self::STATIC`]. #[inline] - pub const fn as_f64(&self) -> f64 { + pub const fn as_f64(self) -> f64 { match self.0 { Some(t) => t.get() as _, None => f64::MIN, @@ -119,20 +119,20 @@ impl TimeInt { /// Always returns [`Self::STATIC`] for [`Self::STATIC`]. #[inline] #[must_use] - pub fn inc(&self) -> Self { + pub fn inc(self) -> Self { match self.0 { Some(t) => Self::new_temporal(t.get().saturating_add(1)), - None => *self, + None => self, } } /// Always returns [`Self::STATIC`] for [`Self::STATIC`]. #[inline] #[must_use] - pub fn dec(&self) -> Self { + pub fn dec(self) -> Self { match self.0 { Some(t) => Self::new_temporal(t.get().saturating_sub(1)), - None => *self, + None => self, } } } diff --git a/crates/utils/re_format/src/lib.rs b/crates/utils/re_format/src/lib.rs index 1363bd09a4d3..393be9c5b6cc 100644 --- a/crates/utils/re_format/src/lib.rs +++ b/crates/utils/re_format/src/lib.rs @@ -408,6 +408,20 @@ pub fn parse_f64(text: &str) -> Option { text.parse().ok() } +/// Parses a number, ignoring whitespace (e.g. thousand separators), +/// and treating the special minus character `MINUS` (−) as a minus sign. +pub fn parse_i64(text: &str) -> Option { + let text: String = text + .chars() + // Ignore whitespace (trailing, leading, and thousands separators): + .filter(|c| !c.is_whitespace()) + // Replace special minus character with normal minus (hyphen): + .map(|c| if c == '−' { '-' } else { c }) + .collect(); + + text.parse().ok() +} + /// Pretty format a large number by using SI notation (base 10), e.g. /// /// ``` diff --git a/crates/viewer/re_time_panel/src/lib.rs b/crates/viewer/re_time_panel/src/lib.rs index 146556e2a564..9e521168237a 100644 --- a/crates/viewer/re_time_panel/src/lib.rs +++ b/crates/viewer/re_time_panel/src/lib.rs @@ -22,7 +22,7 @@ use re_data_ui::{item_ui::guess_instance_path_icon, sorted_component_list_for_ui use re_entity_db::{EntityTree, InstancePath}; use re_log_types::{ external::re_types_core::ComponentName, ComponentPath, EntityPath, EntityPathPart, - ResolvedTimeRange, TimeInt, TimeReal, + ResolvedTimeRange, TimeInt, TimeReal, TimeType, }; use re_types::blueprint::components::PanelState; use re_ui::{list_item, ContextExt as _, DesignTokens, UiExt as _}; @@ -1088,10 +1088,33 @@ fn help_button(ui: &mut egui::Ui) { ); } -fn current_time_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, time_ctrl: &TimeControl) { +fn current_time_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, time_ctrl: &mut TimeControl) { if let Some(time_int) = time_ctrl.time_int() { let time_type = time_ctrl.time_type(); - ui.monospace(time_type.format(time_int, ctx.app_options.time_zone)); + match time_type { + re_log_types::TimeType::Time => { + // TODO(#7653): parse time stamps + ui.monospace(time_type.format(time_int, ctx.app_options.time_zone)); + } + re_log_types::TimeType::Sequence => { + // NOTE: egui uses `f64` for all numbers internally, so we get precision problems if the integer gets too big. + if time_int.as_f64() as i64 == time_int.as_i64() { + let mut int = time_int.as_i64(); + let drag_value = egui::DragValue::new(&mut int) + .custom_formatter(|x, _range| { + TimeType::format_sequence(TimeInt::new_temporal(x as i64)) + }) + .custom_parser(|s| TimeType::parse_sequence(s).map(TimeInt::as_f64)); + let response = ui.add(drag_value); + if response.changed() { + time_ctrl.set_time(TimeInt::new_temporal(int)); + } + } else { + // Avoid the precision problems by just displaying the number without the ability to change it (here). + ui.monospace(time_type.format(time_int, ctx.app_options.time_zone)); + } + } + } } }