From e6a0ebac1b9a1ed89400a6ffb8fc41538c36d210 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 2 Nov 2023 11:28:05 +0100 Subject: [PATCH 01/14] WIP: Make visible history more general --- crates/re_data_store/src/entity_properties.rs | 81 ++++++- crates/re_data_store/src/store_db.rs | 4 + crates/re_query/src/util.rs | 9 +- crates/re_time_panel/src/lib.rs | 5 +- crates/re_viewer/src/ui/selection_panel.rs | 219 +++++++++++++++--- 5 files changed, 271 insertions(+), 47 deletions(-) diff --git a/crates/re_data_store/src/entity_properties.rs b/crates/re_data_store/src/entity_properties.rs index 7250006f7a3e..e7a1bfc2d9e2 100644 --- a/crates/re_data_store/src/entity_properties.rs +++ b/crates/re_data_store/src/entity_properties.rs @@ -1,5 +1,6 @@ #[cfg(feature = "serde")] use re_log_types::EntityPath; +use re_log_types::TimeInt; #[cfg(feature = "serde")] use crate::EditableAutoValue; @@ -172,25 +173,91 @@ impl EntityProperties { // ---------------------------------------------------------------------------- +/// One of the boundary of the visible history. +/// +/// The for [`Relative`] and [`Absolute`], the value are either nanos or frames, depending on the +/// type of timeline. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] +pub enum VisibleHistoryBoundary { + /// Boundary is a value relative to the time cursor + Relative(i64), + + /// Boundary is an absolute value + Absolute(i64), + + /// The boundary extends to infinity. + Infinite, +} + +impl VisibleHistoryBoundary { + pub const AT_CURSOR: Self = Self::Relative(0); +} + +impl Default for VisibleHistoryBoundary { + fn default() -> Self { + Self::AT_CURSOR + } +} + +/// Visible history bounds. +#[derive(Clone, Copy, Default, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] +pub struct VisibleHistory { + /// Low time boundary. + pub from: VisibleHistoryBoundary, + + /// High time boundary. + pub to: VisibleHistoryBoundary, +} + +impl VisibleHistory { + /// Value with the visible history feature is disabled. + pub const OFF: Self = Self { + from: VisibleHistoryBoundary::AT_CURSOR, + to: VisibleHistoryBoundary::AT_CURSOR, + }; + + pub fn from(&self, cursor: TimeInt) -> TimeInt { + match self.from { + VisibleHistoryBoundary::Absolute(value) => TimeInt::from(value), + VisibleHistoryBoundary::Relative(value) => cursor + TimeInt::from(value), + VisibleHistoryBoundary::Infinite => TimeInt::MIN, + } + } + + pub fn to(&self, cursor: TimeInt) -> TimeInt { + match self.to { + VisibleHistoryBoundary::Absolute(value) => TimeInt::from(value), + VisibleHistoryBoundary::Relative(value) => cursor + TimeInt::from(value), + VisibleHistoryBoundary::Infinite => TimeInt::MAX, + } + } +} + /// When showing an entity in the history view, add this much history to it. #[derive(Clone, Copy, Default, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[cfg_attr(feature = "serde", serde(default))] pub struct ExtraQueryHistory { - /// Zero = off. - pub nanos: i64, + /// Is the feature enabled? + pub enabled: bool, - /// Zero = off. - pub sequences: i64, + /// Visible history settings for time timelines + pub nanos: VisibleHistory, + + /// Visible history settings for frame timelines + pub sequences: VisibleHistory, } impl ExtraQueryHistory { /// Multiply/and these together. #[allow(dead_code)] fn with_child(&self, child: &Self) -> Self { - Self { - nanos: self.nanos.max(child.nanos), - sequences: self.sequences.max(child.sequences), + if child.enabled { + *child + } else { + *self } } } // ---------------------------------------------------------------------------- diff --git a/crates/re_data_store/src/store_db.rs b/crates/re_data_store/src/store_db.rs index c61708b5bf30..09c0ed99eaaa 100644 --- a/crates/re_data_store/src/store_db.rs +++ b/crates/re_data_store/src/store_db.rs @@ -296,6 +296,10 @@ impl StoreDb { &self.entity_db.times_per_timeline } + pub fn time_histogram(&self, timeline: &Timeline) -> Option<&crate::TimeHistogram> { + self.entity_db().tree.prefix_times.get(timeline) + } + pub fn num_timeless_messages(&self) -> usize { self.entity_db.tree.num_timeless_messages() } diff --git a/crates/re_query/src/util.rs b/crates/re_query/src/util.rs index 1d1da7a755c8..3022e88ace72 100644 --- a/crates/re_query/src/util.rs +++ b/crates/re_query/src/util.rs @@ -1,5 +1,5 @@ use re_arrow_store::{DataStore, LatestAtQuery, RangeQuery, TimeInt, TimeRange, Timeline}; -use re_data_store::ExtraQueryHistory; +use re_data_store::{ExtraQueryHistory, VisibleHistory}; use re_log_types::EntityPath; use re_types_core::Archetype; @@ -17,14 +17,15 @@ pub fn query_archetype_with_history<'a, A: Archetype + 'a, const N: usize>( re_log_types::TimeType::Sequence => history.sequences, }; - if visible_history == 0 { + if !history.enabled || visible_history == VisibleHistory::OFF { let latest_query = LatestAtQuery::new(*timeline, *time); let latest = query_archetype::(store, &latest_query, ent_path)?; Ok(itertools::Either::Left(std::iter::once(latest))) } else { - let min_time = *time - TimeInt::from(visible_history); - let range_query = RangeQuery::new(*timeline, TimeRange::new(min_time, *time)); + let min_time = visible_history.from(*time); + let max_time = visible_history.to(*time); + let range_query = RangeQuery::new(*timeline, TimeRange::new(min_time, max_time)); let range = range_archetype::(store, &range_query, ent_path); diff --git a/crates/re_time_panel/src/lib.rs b/crates/re_time_panel/src/lib.rs index 1f1f61c05a26..4a47ac9e2aed 100644 --- a/crates/re_time_panel/src/lib.rs +++ b/crates/re_time_panel/src/lib.rs @@ -822,10 +822,7 @@ fn initialize_time_ranges_ui( if let Some(times) = ctx .store_db - .entity_db() - .tree - .prefix_times - .get(ctx.rec_cfg.time_ctrl.timeline()) + .time_histogram(ctx.rec_cfg.time_ctrl.timeline()) { // NOTE: `times` can be empty if a GC wiped everything. if !times.is_empty() { diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 6351ce4f4cd4..3a9107e90757 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -1,6 +1,9 @@ -use egui::NumExt as _; +use egui::{NumExt as _, Ui}; +use std::ops::RangeInclusive; -use re_data_store::{ColorMapper, Colormap, EditableAutoValue, EntityPath, EntityProperties}; +use re_data_store::{ + ColorMapper, Colormap, EditableAutoValue, EntityPath, EntityProperties, VisibleHistoryBoundary, +}; use re_data_ui::{image_meaning_for_entity, item_ui, DataUi}; use re_log_types::TimeType; use re_types::{ @@ -8,7 +11,8 @@ use re_types::{ tensor_data::TensorDataMeaning, }; use re_viewer_context::{ - gpu_bridge::colormap_dropdown_button_ui, Item, SpaceViewId, UiVerbosity, ViewerContext, + gpu_bridge::colormap_dropdown_button_ui, Item, SpaceViewClassName, SpaceViewId, UiVerbosity, + ViewerContext, }; use re_viewport::{Viewport, ViewportBlueprint}; @@ -303,9 +307,16 @@ fn blueprint_ui( // TODO(emilk): show the values of this specific instance (e.g. point in the point cloud)! } else { // splat - the whole entity + let space_view_class_name = space_view.class_name().clone(); let data_blueprint = space_view.contents.data_blueprints_individual(); let mut props = data_blueprint.get(&instance_path.entity_path); - entity_props_ui(ctx, ui, Some(&instance_path.entity_path), &mut props); + entity_props_ui( + ctx, + ui, + &space_view_class_name, + Some(&instance_path.entity_path), + &mut props, + ); data_blueprint.set(instance_path.entity_path.clone(), props); } } @@ -321,8 +332,15 @@ fn blueprint_ui( Item::DataBlueprintGroup(space_view_id, data_blueprint_group_handle) => { if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) { + let space_view_class_name = space_view.class_name().clone(); if let Some(group) = space_view.contents.group_mut(*data_blueprint_group_handle) { - entity_props_ui(ctx, ui, None, &mut group.properties_individual); + entity_props_ui( + ctx, + ui, + &space_view_class_name, + None, + &mut group.properties_individual, + ); } else { ctx.selection_state_mut().clear_current(); } @@ -364,6 +382,7 @@ fn list_existing_data_blueprints( fn entity_props_ui( ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui, + space_view_class_name: &SpaceViewClassName, entity_path: Option<&EntityPath>, entity_props: &mut EntityProperties, ) { @@ -373,36 +392,13 @@ fn entity_props_ui( .checkbox(ui, &mut entity_props.interactive, "Interactive") .on_hover_text("If disabled, the entity will not react to any mouse interaction"); + // TODO(ab): this should be displayed only if the entity supports visible history + // TODO(ab): this should run at SV-level for timeseries and text log SV + visible_history_ui(ctx, ui, space_view_class_name, entity_props); + egui::Grid::new("entity_properties") .num_columns(2) .show(ui, |ui| { - ui.label("Visible history"); - let visible_history = &mut entity_props.visible_history; - match ctx.rec_cfg.time_ctrl.timeline().typ() { - TimeType::Time => { - let mut time_sec = visible_history.nanos as f32 / 1e9; - let speed = (time_sec * 0.05).at_least(0.01); - ui.add( - egui::DragValue::new(&mut time_sec) - .clamp_range(0.0..=f32::INFINITY) - .speed(speed) - .suffix("s"), - ) - .on_hover_text("Include this much history of the Entity in the Space View"); - visible_history.nanos = (time_sec * 1e9).round() as _; - } - TimeType::Sequence => { - let speed = (visible_history.sequences as f32 * 0.05).at_least(1.0); - ui.add( - egui::DragValue::new(&mut visible_history.sequences) - .clamp_range(0.0..=f32::INFINITY) - .speed(speed), - ) - .on_hover_text("Include this much history of the Entity in the Space View"); - } - } - ui.end_row(); - // TODO(wumpf): It would be nice to only show pinhole & depth properties in the context of a 3D view. // if *view_state.state_spatial.nav_mode.get() == SpatialNavigationMode::ThreeD { if let Some(entity_path) = entity_path { @@ -413,6 +409,165 @@ fn entity_props_ui( }); } +fn visible_history_ui( + ctx: &mut ViewerContext, + ui: &mut Ui, + space_view_class_name: &SpaceViewClassName, + entity_props: &mut EntityProperties, +) { + if space_view_class_name != "3D" && space_view_class_name != "2D" { + return; + } + + ui.checkbox(&mut entity_props.visible_history.enabled, "Visible history"); + + let time_range = if let Some(times) = ctx + .store_db + .time_histogram(ctx.rec_cfg.time_ctrl.timeline()) + { + times.min_key().unwrap_or_default()..=times.max_key().unwrap_or_default() + } else { + 0..=0 + }; + + let current_time = ctx + .rec_cfg + .time_ctrl + .time_i64() + .unwrap_or_default() + .at_least(*time_range.start()); // accounts for timeless time (TimeInt::BEGINNING) + + let sequence_timeline = match ctx.rec_cfg.time_ctrl.timeline().typ() { + TimeType::Time => false, + TimeType::Sequence => true, + }; + + let visible_history = if sequence_timeline { + &mut entity_props.visible_history.sequences + } else { + &mut entity_props.visible_history.nanos + }; + + ui.add_enabled_ui(entity_props.visible_history.enabled, |ui| { + ui.horizontal(|ui| { + ui.label("From"); + visible_history_boundary_ui( + ui, + &mut visible_history.from, + sequence_timeline, + current_time, + time_range.clone(), + ) + }); + + ui.horizontal(|ui| { + ui.label("To"); + visible_history_boundary_ui( + ui, + &mut visible_history.to, + sequence_timeline, + current_time, + time_range, + ) + }); + }); +} + +fn visible_history_boundary_ui( + ui: &mut egui::Ui, + visible_history_boundary: &mut VisibleHistoryBoundary, + sequence_timeline: bool, + current_time: i64, + mut time_range: RangeInclusive, +) { + let mut infinite: bool; + let mut relative = matches!( + visible_history_boundary, + VisibleHistoryBoundary::Relative(_) + ); + + let span = time_range.end() - time_range.start(); + if relative { + // in relative mode, the range must be wider + time_range = + (time_range.start() - current_time - span)..=(time_range.end() - current_time + span); + } + + match visible_history_boundary { + VisibleHistoryBoundary::Relative(value) | VisibleHistoryBoundary::Absolute(value) => { + if sequence_timeline { + let speed = (span as f32 * 0.005).at_least(1.0); + + ui.add( + egui::DragValue::new(value) + .clamp_range(time_range) + .speed(speed), + ); + } else { + time_drag_value_ui(ui, value, &time_range); + } + + if ui.checkbox(&mut relative, "Relative").changed() { + if relative { + *visible_history_boundary = + VisibleHistoryBoundary::Relative(*value - current_time); + } else { + *visible_history_boundary = + VisibleHistoryBoundary::Absolute(*value + current_time); + } + } + + infinite = false; + } + VisibleHistoryBoundary::Infinite => { + let mut unused = 0.0; + ui.add_enabled( + false, + egui::DragValue::new(&mut unused).custom_formatter(|_, _| "∞".to_owned()), + ); + + let mut unused = false; + ui.add_enabled(false, egui::Checkbox::new(&mut unused, "Relative")); + + infinite = true; + } + } + + if ui.checkbox(&mut infinite, "Infinite").changed() { + if infinite { + *visible_history_boundary = VisibleHistoryBoundary::Infinite; + } else { + *visible_history_boundary = VisibleHistoryBoundary::Relative(0); + } + } +} + +fn time_drag_value_ui(ui: &mut egui::Ui, value: &mut i64, time_range: &RangeInclusive) { + let span = time_range.end() - time_range.start(); + + let (unit, factor) = if span / 1_000_000_000 > 0 { + ("s", 1_000_000_000.) + } else if span / 1_000_000 > 0 { + ("ms", 1_000_000.) + } else if span / 1_000 > 0 { + ("μs", 1_000.) + } else { + ("ns", 1.) + }; + + let mut time_unit = *value as f32 / factor; + let time_range = *time_range.start() as f32 / factor..=*time_range.end() as f32 / factor; + let speed = (time_range.end() - time_range.start()) * 0.005; + + ui.add( + egui::DragValue::new(&mut time_unit) + .clamp_range(time_range) + .speed(speed) + .suffix(unit), + ); + *value = (time_unit * factor).round() as _; +} + fn colormap_props_ui( ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui, From 36058ec1e9b3f3f3342ab5ca6074664abdcf99de Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 2 Nov 2023 17:22:10 +0100 Subject: [PATCH 02/14] Fine-tuning, lints, etc. --- crates/re_viewer/src/ui/selection_panel.rs | 86 ++++++++++++++-------- 1 file changed, 54 insertions(+), 32 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 3a9107e90757..e30d020871e2 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -307,7 +307,7 @@ fn blueprint_ui( // TODO(emilk): show the values of this specific instance (e.g. point in the point cloud)! } else { // splat - the whole entity - let space_view_class_name = space_view.class_name().clone(); + let space_view_class_name = *space_view.class_name(); let data_blueprint = space_view.contents.data_blueprints_individual(); let mut props = data_blueprint.get(&instance_path.entity_path); entity_props_ui( @@ -332,7 +332,7 @@ fn blueprint_ui( Item::DataBlueprintGroup(space_view_id, data_blueprint_group_handle) => { if let Some(space_view) = viewport.blueprint.space_view_mut(space_view_id) { - let space_view_class_name = space_view.class_name().clone(); + let space_view_class_name = *space_view.class_name(); if let Some(group) = space_view.contents.group_mut(*data_blueprint_group_handle) { entity_props_ui( ctx, @@ -410,16 +410,23 @@ fn entity_props_ui( } fn visible_history_ui( - ctx: &mut ViewerContext, + ctx: &mut ViewerContext<'_>, ui: &mut Ui, space_view_class_name: &SpaceViewClassName, entity_props: &mut EntityProperties, ) { + //TODO(#4107): support more space view types. if space_view_class_name != "3D" && space_view_class_name != "2D" { return; } - ui.checkbox(&mut entity_props.visible_history.enabled, "Visible history"); + let re_ui = ctx.re_ui; + + re_ui.checkbox( + ui, + &mut entity_props.visible_history.enabled, + "Visible history", + ); let time_range = if let Some(times) = ctx .store_db @@ -437,10 +444,7 @@ fn visible_history_ui( .unwrap_or_default() .at_least(*time_range.start()); // accounts for timeless time (TimeInt::BEGINNING) - let sequence_timeline = match ctx.rec_cfg.time_ctrl.timeline().typ() { - TimeType::Time => false, - TimeType::Sequence => true, - }; + let sequence_timeline = matches!(ctx.rec_cfg.time_ctrl.timeline().typ(), TimeType::Sequence); let visible_history = if sequence_timeline { &mut entity_props.visible_history.sequences @@ -449,31 +453,50 @@ fn visible_history_ui( }; ui.add_enabled_ui(entity_props.visible_history.enabled, |ui| { - ui.horizontal(|ui| { - ui.label("From"); - visible_history_boundary_ui( - ui, - &mut visible_history.from, - sequence_timeline, - current_time, - time_range.clone(), - ) - }); + egui::Grid::new("visible_history_boundaries") + .num_columns(4) + .show(ui, |ui| { + ui.label("From"); + visible_history_boundary_ui( + re_ui, + ui, + &mut visible_history.from, + sequence_timeline, + current_time, + time_range.clone(), + ); - ui.horizontal(|ui| { - ui.label("To"); - visible_history_boundary_ui( - ui, - &mut visible_history.to, - sequence_timeline, - current_time, - time_range, - ) - }); + ui.end_row(); + + ui.label("To"); + visible_history_boundary_ui( + re_ui, + ui, + &mut visible_history.to, + sequence_timeline, + current_time, + time_range, + ); + + ui.end_row(); + }); }); + ui.add( + egui::Label::new( + egui::RichText::new(if sequence_timeline { + "These settings apply to all sequence timelines." + } else { + "These settings apply to all temporal timelines." + }) + .italics() + .weak(), + ) + .wrap(true), + ); } fn visible_history_boundary_ui( + re_ui: &re_ui::ReUi, ui: &mut egui::Ui, visible_history_boundary: &mut VisibleHistoryBoundary, sequence_timeline: bool, @@ -489,8 +512,7 @@ fn visible_history_boundary_ui( let span = time_range.end() - time_range.start(); if relative { // in relative mode, the range must be wider - time_range = - (time_range.start() - current_time - span)..=(time_range.end() - current_time + span); + time_range = (time_range.start() - span)..=(time_range.end() + span); } match visible_history_boundary { @@ -507,7 +529,7 @@ fn visible_history_boundary_ui( time_drag_value_ui(ui, value, &time_range); } - if ui.checkbox(&mut relative, "Relative").changed() { + if re_ui.checkbox(ui, &mut relative, "Relative").changed() { if relative { *visible_history_boundary = VisibleHistoryBoundary::Relative(*value - current_time); @@ -533,7 +555,7 @@ fn visible_history_boundary_ui( } } - if ui.checkbox(&mut infinite, "Infinite").changed() { + if re_ui.checkbox(ui, &mut infinite, "Infinite").changed() { if infinite { *visible_history_boundary = VisibleHistoryBoundary::Infinite; } else { From adce254256ea924bb09763f9668f2cd69d330ba4 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 2 Nov 2023 18:19:11 +0100 Subject: [PATCH 03/14] Add heuristic to show/hide visible history UI --- crates/re_ui/src/lib.rs | 36 +++++++++++ crates/re_viewer/src/ui/selection_panel.rs | 75 +++++++++++++--------- 2 files changed, 81 insertions(+), 30 deletions(-) diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index 4efd0f77b7ce..86271c055c67 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -15,6 +15,7 @@ pub use command_palette::CommandPalette; pub use design_tokens::DesignTokens; pub use icons::Icon; pub use layout_job_builder::LayoutJobBuilder; +use std::ops::RangeInclusive; pub use toggle_switch::toggle_switch; // --------------------------------------------------------------------------- @@ -445,6 +446,41 @@ impl ReUi { .inner } + /// Shows a `egui::DragValue` for a time in nanoseconds. + /// + /// The value and unit displayed adapts to the provided allowable time range. + #[allow(clippy::unused_self)] + pub fn time_drag_value( + &self, + ui: &mut egui::Ui, + value: &mut i64, + time_range: &RangeInclusive, + ) { + let span = time_range.end() - time_range.start(); + + let (unit, factor) = if span / 1_000_000_000 > 0 { + ("s", 1_000_000_000.) + } else if span / 1_000_000 > 0 { + ("ms", 1_000_000.) + } else if span / 1_000 > 0 { + ("μs", 1_000.) + } else { + ("ns", 1.) + }; + + let mut time_unit = *value as f32 / factor; + let time_range = *time_range.start() as f32 / factor..=*time_range.end() as f32 / factor; + let speed = (time_range.end() - time_range.start()) * 0.005; + + ui.add( + egui::DragValue::new(&mut time_unit) + .clamp_range(time_range) + .speed(speed) + .suffix(unit), + ); + *value = (time_unit * factor).round() as _; + } + pub fn large_button(&self, ui: &mut egui::Ui, icon: &Icon) -> egui::Response { self.large_button_impl(ui, icon, None, None) } diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index e30d020871e2..00869137b5fb 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -10,6 +10,7 @@ use re_types::{ components::{PinholeProjection, Transform3D}, tensor_data::TensorDataMeaning, }; +use re_types_core::ComponentName; use re_viewer_context::{ gpu_bridge::colormap_dropdown_button_ui, Item, SpaceViewClassName, SpaceViewId, UiVerbosity, ViewerContext, @@ -392,9 +393,7 @@ fn entity_props_ui( .checkbox(ui, &mut entity_props.interactive, "Interactive") .on_hover_text("If disabled, the entity will not react to any mouse interaction"); - // TODO(ab): this should be displayed only if the entity supports visible history - // TODO(ab): this should run at SV-level for timeseries and text log SV - visible_history_ui(ctx, ui, space_view_class_name, entity_props); + visible_history_ui(ctx, ui, space_view_class_name, entity_path, entity_props); egui::Grid::new("entity_properties") .num_columns(2) @@ -413,6 +412,7 @@ fn visible_history_ui( ctx: &mut ViewerContext<'_>, ui: &mut Ui, space_view_class_name: &SpaceViewClassName, + entity_path: Option<&EntityPath>, entity_props: &mut EntityProperties, ) { //TODO(#4107): support more space view types. @@ -420,6 +420,10 @@ fn visible_history_ui( return; } + if !should_display_visible_history(ctx, entity_path) { + return; + } + let re_ui = ctx.re_ui; re_ui.checkbox( @@ -495,6 +499,43 @@ fn visible_history_ui( ); } +static VISIBLE_HISTORY_COMPONENT_NAMES: once_cell::sync::Lazy> = + once_cell::sync::Lazy::new(|| { + [ + ComponentName::from("rerun.components.Position2D"), + ComponentName::from("rerun.components.Position3D"), + ComponentName::from("rerun.components.LineStrip2D"), + ComponentName::from("rerun.components.LineStrip3D"), + ComponentName::from("rerun.components.TensorData"), + ComponentName::from("rerun.components.Vector3D"), + ComponentName::from("rerun.components.HalfSizes2D"), + ComponentName::from("rerun.components.HalfSizes3D"), + ] + .into() + }); + +fn should_display_visible_history( + ctx: &mut ViewerContext<'_>, + entity_path: Option<&EntityPath>, +) -> bool { + if let Some(entity_path) = entity_path { + let store = ctx.store_db.store(); + let component_names = store.all_components(ctx.rec_cfg.time_ctrl.timeline(), entity_path); + if let Some(component_names) = component_names { + if !component_names + .iter() + .any(|name| VISIBLE_HISTORY_COMPONENT_NAMES.contains(name)) + { + return false; + } + } else { + return false; + } + } + + true +} + fn visible_history_boundary_ui( re_ui: &re_ui::ReUi, ui: &mut egui::Ui, @@ -526,7 +567,7 @@ fn visible_history_boundary_ui( .speed(speed), ); } else { - time_drag_value_ui(ui, value, &time_range); + re_ui.time_drag_value(ui, value, &time_range); } if re_ui.checkbox(ui, &mut relative, "Relative").changed() { @@ -564,32 +605,6 @@ fn visible_history_boundary_ui( } } -fn time_drag_value_ui(ui: &mut egui::Ui, value: &mut i64, time_range: &RangeInclusive) { - let span = time_range.end() - time_range.start(); - - let (unit, factor) = if span / 1_000_000_000 > 0 { - ("s", 1_000_000_000.) - } else if span / 1_000_000 > 0 { - ("ms", 1_000_000.) - } else if span / 1_000 > 0 { - ("μs", 1_000.) - } else { - ("ns", 1.) - }; - - let mut time_unit = *value as f32 / factor; - let time_range = *time_range.start() as f32 / factor..=*time_range.end() as f32 / factor; - let speed = (time_range.end() - time_range.start()) * 0.005; - - ui.add( - egui::DragValue::new(&mut time_unit) - .clamp_range(time_range) - .speed(speed) - .suffix(unit), - ); - *value = (time_unit * factor).round() as _; -} - fn colormap_props_ui( ctx: &mut ViewerContext<'_>, ui: &mut egui::Ui, From 9c01f7f2b046ec9c2f420672844951491abd592b Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 2 Nov 2023 18:22:38 +0100 Subject: [PATCH 04/14] Fix cargo docs --- crates/re_data_store/src/entity_properties.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/re_data_store/src/entity_properties.rs b/crates/re_data_store/src/entity_properties.rs index e7a1bfc2d9e2..2a09d7dec7cc 100644 --- a/crates/re_data_store/src/entity_properties.rs +++ b/crates/re_data_store/src/entity_properties.rs @@ -175,8 +175,8 @@ impl EntityProperties { /// One of the boundary of the visible history. /// -/// The for [`Relative`] and [`Absolute`], the value are either nanos or frames, depending on the -/// type of timeline. +/// The for [`VisibleHistoryBoundary::Relative`] and [`VisibleHistoryBoundary::Absolute`], the value +/// are either nanos or frames, depending on the type of timeline. #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub enum VisibleHistoryBoundary { From 6f4517e128cebe475c298084809c413f807da762 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 3 Nov 2023 10:01:39 +0100 Subject: [PATCH 05/14] Fix time range for non-zero-based sequence timeline --- crates/re_viewer/src/ui/selection_panel.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 00869137b5fb..0cb20a37b98a 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -552,8 +552,9 @@ fn visible_history_boundary_ui( let span = time_range.end() - time_range.start(); if relative { - // in relative mode, the range must be wider - time_range = (time_range.start() - span)..=(time_range.end() + span); + // the wider range is needed to avoid the DragValue bounds to modify the boundary value when + // the time slider is moved to the timeline's extremes + time_range = -span..=2 * span; } match visible_history_boundary { From 648dcf94cd0605e0190ab7ee3182651ae5941dfc Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 3 Nov 2023 11:25:55 +0100 Subject: [PATCH 06/14] Added some tooltips --- crates/re_viewer/src/ui/selection_panel.rs | 28 +++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 0cb20a37b98a..457f6bed25c7 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -430,7 +430,10 @@ fn visible_history_ui( ui, &mut entity_props.visible_history.enabled, "Visible history", - ); + ).on_hover_text( + "Enable the Visible History feature.\n\nBy default, only the last data received before the \ + current time is shown. By activating the Visible History, all the data received within a \ + time window is shown instead."); let time_range = if let Some(times) = ctx .store_db @@ -468,6 +471,7 @@ fn visible_history_ui( sequence_timeline, current_time, time_range.clone(), + true, ); ui.end_row(); @@ -480,6 +484,7 @@ fn visible_history_ui( sequence_timeline, current_time, time_range, + false, ); ui.end_row(); @@ -543,6 +548,7 @@ fn visible_history_boundary_ui( sequence_timeline: bool, current_time: i64, mut time_range: RangeInclusive, + low_bound: bool, ) { let mut infinite: bool; let mut relative = matches!( @@ -571,7 +577,15 @@ fn visible_history_boundary_ui( re_ui.time_drag_value(ui, value, &time_range); } - if re_ui.checkbox(ui, &mut relative, "Relative").changed() { + if re_ui + .checkbox(ui, &mut relative, "Relative") + .on_hover_text( + "When enabled, the provided value is interpreted as relative to the \ + current time. Otherwise, the provided value is interpreted as an absolute \ + time.", + ) + .changed() + { if relative { *visible_history_boundary = VisibleHistoryBoundary::Relative(*value - current_time); @@ -597,7 +611,15 @@ fn visible_history_boundary_ui( } } - if re_ui.checkbox(ui, &mut infinite, "Infinite").changed() { + if re_ui + .checkbox(ui, &mut infinite, "Infinite") + .on_hover_text(if low_bound { + "Show data from the beginning of the timeline" + } else { + "Show data until the end of the timeline" + }) + .changed() + { if infinite { *visible_history_boundary = VisibleHistoryBoundary::Infinite; } else { From dc39ea7736de482a7bd7ee44c2dca3cbd01cd3d5 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 3 Nov 2023 16:01:06 +0100 Subject: [PATCH 07/14] Added playground test script --- .../python/visible_history_playground/main.py | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 tests/python/visible_history_playground/main.py diff --git a/tests/python/visible_history_playground/main.py b/tests/python/visible_history_playground/main.py new file mode 100644 index 000000000000..1c9b97a7c4c7 --- /dev/null +++ b/tests/python/visible_history_playground/main.py @@ -0,0 +1,61 @@ +"""Playground to test the visible history feature.""" +from __future__ import annotations + +import argparse +import math + +import numpy as np +import rerun as rr + +parser = argparse.ArgumentParser(description=__doc__) +rr.script_add_args(parser) +args = parser.parse_args() +rr.script_setup(args, "rerun_example_visible_history_playground") + + +rr.log("bbox", rr.Boxes2D(centers=[50, 3.5], half_sizes=[50, 4.5], colors=[255, 0, 0]), timeless=True) +rr.log("transform", rr.Transform3D(translation=[0, 0, 0])) +rr.log("some/nested/pinhole", rr.Pinhole(focal_length=3, width=3, height=3), timeless=True) + +rr.log("3dworld/depthimage/pinhole", rr.Pinhole(focal_length=20, width=100, height=10), timeless=True) +rr.log("3dworld/image", rr.Transform3D(translation=[0, 1, 0]), timeless=True) +rr.log("3dworld/image/pinhole", rr.Pinhole(focal_length=20, width=100, height=10), timeless=True) + +for i in range(1, 100): + rr.set_time_seconds("seconds", i) + rr.set_time_seconds("ms", i / 1000) + rr.set_time_seconds("us", i / 1000000) + rr.set_time_sequence("frames_offset", 10000 + i) + rr.set_time_sequence("frames", i) + + rr.log("world/data/nested/point", rr.Points2D([[i, 0], [i, 1]], radii=0.4)) + rr.log("world/data/nested/point2", rr.Points2D([i, 2], radii=0.4)) + rr.log("world/data/nested/box", rr.Boxes2D(centers=[i, 1], half_sizes=[0.5, 0.5])) + rr.log("world/data/nested/arrow", rr.Arrows3D(origins=[i, 4, 0], vectors=[0, 1.7, 0])) + rr.log( + "world/data/nested/linestrip", + rr.LineStrips2D([[[i - 0.4, 6], [i + 0.4, 6], [i - 0.4, 7], [i + 0.4, 7]], [[i - 0.2, 6.5], [i + 0.2, 6.5]]]), + ) + + rr.log("world/data/nested/transformed", rr.Transform3D(translation=[i, 0, 0])) + rr.log("world/data/nested/transformed/point", rr.Boxes2D(centers=[0, 3], half_sizes=[0.5, 0.5])) + + rr.log("text_log", rr.TextLog(f"hello {i}")) + rr.log("scalar", rr.TimeSeriesScalar(math.sin(i / 100 * 2 * math.pi))) + + depth_image = 100 * np.ones((10, 100), dtype=np.float32) + depth_image[:, i] = 50 + rr.log("3dworld/depthimage/pinhole/data", rr.DepthImage(depth_image, meter=100)) + + image = 100 * np.ones((10, 100, 3), dtype=np.uint8) + image[:, i, :] = [255, 0, 0] + rr.log("3dworld/image/pinhole/data", rr.Image(image)) + + x_coord = (i - 50) / 5 + rr.log( + "3dworld/mesh", + rr.Mesh3D( + vertex_positions=[[x_coord, 2, 0], [x_coord, 2, 1], [x_coord, 3, 0]], + vertex_colors=[[0, 0, 255], [0, 255, 0], [255, 0, 0]], + ), + ) From c5b04d9119a31ebeb2415061eae9eac4a3bab81e Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Fri, 3 Nov 2023 17:19:03 +0100 Subject: [PATCH 08/14] Update crates/re_data_store/src/entity_properties.rs Co-authored-by: Andreas Reich --- crates/re_data_store/src/entity_properties.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_data_store/src/entity_properties.rs b/crates/re_data_store/src/entity_properties.rs index deeb662c3455..0b661793eec1 100644 --- a/crates/re_data_store/src/entity_properties.rs +++ b/crates/re_data_store/src/entity_properties.rs @@ -173,7 +173,7 @@ impl EntityProperties { // ---------------------------------------------------------------------------- -/// One of the boundary of the visible history. +/// One of the boundaries of the visible history. /// /// The for [`VisibleHistoryBoundary::Relative`] and [`VisibleHistoryBoundary::Absolute`], the value /// are either nanos or frames, depending on the type of timeline. From 00629fc83f395ec6a5ad0202301062798c1ddc93 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Fri, 3 Nov 2023 17:29:32 +0100 Subject: [PATCH 09/14] Update crates/re_viewer/src/ui/selection_panel.rs Co-authored-by: Andreas Reich --- crates/re_viewer/src/ui/selection_panel.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 457f6bed25c7..bb73f2598bb4 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -431,8 +431,8 @@ fn visible_history_ui( &mut entity_props.visible_history.enabled, "Visible history", ).on_hover_text( - "Enable the Visible History feature.\n\nBy default, only the last data received before the \ - current time is shown. By activating the Visible History, all the data received within a \ + "Enable Visible History.\n\nBy default, only the last state before the \ + current time is shown. By activating Visible History, all data within a \ time window is shown instead."); let time_range = if let Some(times) = ctx From 2d82812acce051dd8afc51b294fe2b4e7a0aab38 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Fri, 3 Nov 2023 17:30:10 +0100 Subject: [PATCH 10/14] Update crates/re_viewer/src/ui/selection_panel.rs Co-authored-by: Andreas Reich --- crates/re_viewer/src/ui/selection_panel.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index bb73f2598bb4..7f8deaae2f13 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -620,11 +620,11 @@ fn visible_history_boundary_ui( }) .changed() { - if infinite { - *visible_history_boundary = VisibleHistoryBoundary::Infinite; + *visible_history_boundary = if infinite { + VisibleHistoryBoundary::Infinite } else { - *visible_history_boundary = VisibleHistoryBoundary::Relative(0); - } + VisibleHistoryBoundary::Relative(0) + }; } } From acd1a0e950366ee9f822791e3b9f59cfe3b0c0be Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Sat, 4 Nov 2023 09:53:40 +0100 Subject: [PATCH 11/14] Updated UI to use a combobox instead of the two checkboxes + some review comments --- crates/re_data_store/src/entity_properties.rs | 31 ++++- crates/re_viewer/src/ui/selection_panel.rs | 111 ++++++++++-------- 2 files changed, 84 insertions(+), 58 deletions(-) diff --git a/crates/re_data_store/src/entity_properties.rs b/crates/re_data_store/src/entity_properties.rs index 0b661793eec1..131bc85dfb65 100644 --- a/crates/re_data_store/src/entity_properties.rs +++ b/crates/re_data_store/src/entity_properties.rs @@ -175,13 +175,13 @@ impl EntityProperties { /// One of the boundaries of the visible history. /// -/// The for [`VisibleHistoryBoundary::Relative`] and [`VisibleHistoryBoundary::Absolute`], the value -/// are either nanos or frames, depending on the type of timeline. +/// For [`VisibleHistoryBoundary::RelativeToTimeCursor`] and [`VisibleHistoryBoundary::Absolute`], +/// the value are either nanos or frames, depending on the type of timeline. #[derive(Clone, Copy, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub enum VisibleHistoryBoundary { /// Boundary is a value relative to the time cursor - Relative(i64), + RelativeToTimeCursor(i64), /// Boundary is an absolute value Absolute(i64), @@ -191,7 +191,26 @@ pub enum VisibleHistoryBoundary { } impl VisibleHistoryBoundary { - pub const AT_CURSOR: Self = Self::Relative(0); + /// UI label to use when [`VisibleHistoryBoundary::RelativeToTimeCursor`] is selected. + pub const RELATIVE_LABEL: &'static str = "Relative"; + + /// UI label to use when [`VisibleHistoryBoundary::Absolute`] is selected. + pub const ABSOLUTE_LABEL: &'static str = "Absolute"; + + /// UI label to use when [`VisibleHistoryBoundary::Infinite`] is selected. + pub const INFINITE_LABEL: &'static str = "Infinite"; + + /// Value when the boundary is set to the current time cursor. + pub const AT_CURSOR: Self = Self::RelativeToTimeCursor(0); + + /// Label to use in the UI. + pub fn label(&self) -> &'static str { + match self { + Self::RelativeToTimeCursor(_) => Self::RELATIVE_LABEL, + Self::Absolute(_) => Self::ABSOLUTE_LABEL, + Self::Infinite => Self::INFINITE_LABEL, + } + } } impl Default for VisibleHistoryBoundary { @@ -221,7 +240,7 @@ impl VisibleHistory { pub fn from(&self, cursor: TimeInt) -> TimeInt { match self.from { VisibleHistoryBoundary::Absolute(value) => TimeInt::from(value), - VisibleHistoryBoundary::Relative(value) => cursor + TimeInt::from(value), + VisibleHistoryBoundary::RelativeToTimeCursor(value) => cursor + TimeInt::from(value), VisibleHistoryBoundary::Infinite => TimeInt::MIN, } } @@ -229,7 +248,7 @@ impl VisibleHistory { pub fn to(&self, cursor: TimeInt) -> TimeInt { match self.to { VisibleHistoryBoundary::Absolute(value) => TimeInt::from(value), - VisibleHistoryBoundary::Relative(value) => cursor + TimeInt::from(value), + VisibleHistoryBoundary::RelativeToTimeCursor(value) => cursor + TimeInt::from(value), VisibleHistoryBoundary::Infinite => TimeInt::MAX, } } diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 7f8deaae2f13..48bc952c29e5 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -426,14 +426,17 @@ fn visible_history_ui( let re_ui = ctx.re_ui; - re_ui.checkbox( - ui, - &mut entity_props.visible_history.enabled, - "Visible history", - ).on_hover_text( - "Enable Visible History.\n\nBy default, only the last state before the \ - current time is shown. By activating Visible History, all data within a \ - time window is shown instead."); + re_ui + .checkbox( + ui, + &mut entity_props.visible_history.enabled, + "Visible history", + ) + .on_hover_text( + "Enable Visible History.\n\nBy default, only the last state before the \ + current time is shown. By activating Visible History, all data within a \ + time window is shown instead.", + ); let time_range = if let Some(times) = ctx .store_db @@ -550,21 +553,19 @@ fn visible_history_boundary_ui( mut time_range: RangeInclusive, low_bound: bool, ) { - let mut infinite: bool; - let mut relative = matches!( - visible_history_boundary, - VisibleHistoryBoundary::Relative(_) - ); - let span = time_range.end() - time_range.start(); - if relative { + if matches!( + visible_history_boundary, + VisibleHistoryBoundary::RelativeToTimeCursor(_) + ) { // the wider range is needed to avoid the DragValue bounds to modify the boundary value when // the time slider is moved to the timeline's extremes time_range = -span..=2 * span; } match visible_history_boundary { - VisibleHistoryBoundary::Relative(value) | VisibleHistoryBoundary::Absolute(value) => { + VisibleHistoryBoundary::RelativeToTimeCursor(value) + | VisibleHistoryBoundary::Absolute(value) => { if sequence_timeline { let speed = (span as f32 * 0.005).at_least(1.0); @@ -576,26 +577,6 @@ fn visible_history_boundary_ui( } else { re_ui.time_drag_value(ui, value, &time_range); } - - if re_ui - .checkbox(ui, &mut relative, "Relative") - .on_hover_text( - "When enabled, the provided value is interpreted as relative to the \ - current time. Otherwise, the provided value is interpreted as an absolute \ - time.", - ) - .changed() - { - if relative { - *visible_history_boundary = - VisibleHistoryBoundary::Relative(*value - current_time); - } else { - *visible_history_boundary = - VisibleHistoryBoundary::Absolute(*value + current_time); - } - } - - infinite = false; } VisibleHistoryBoundary::Infinite => { let mut unused = 0.0; @@ -603,29 +584,55 @@ fn visible_history_boundary_ui( false, egui::DragValue::new(&mut unused).custom_formatter(|_, _| "∞".to_owned()), ); - - let mut unused = false; - ui.add_enabled(false, egui::Checkbox::new(&mut unused, "Relative")); - - infinite = true; } } - if re_ui - .checkbox(ui, &mut infinite, "Infinite") + let (abs_time, rel_time) = match visible_history_boundary { + VisibleHistoryBoundary::RelativeToTimeCursor(value) => (*value + current_time, *value), + VisibleHistoryBoundary::Absolute(value) => (*value, *value - current_time), + VisibleHistoryBoundary::Infinite => (current_time, 0), + }; + + egui::ComboBox::from_id_source(if low_bound { + "time_history_low_bound" + } else { + "time_history_high_bound" + }) + .selected_text(visible_history_boundary.label()) + .show_ui(ui, |ui| { + ui.set_min_width(64.0); + + ui.selectable_value( + visible_history_boundary, + VisibleHistoryBoundary::RelativeToTimeCursor(rel_time), + VisibleHistoryBoundary::RELATIVE_LABEL, + ) + .on_hover_text(if low_bound { + "Show data from a time point relative to the current time." + } else { + "Show data until a time point relative to the current time." + }); + ui.selectable_value( + visible_history_boundary, + VisibleHistoryBoundary::Absolute(abs_time), + VisibleHistoryBoundary::ABSOLUTE_LABEL, + ) + .on_hover_text(if low_bound { + "Show data from an absolute time point." + } else { + "Show data until an absolute time point." + }); + ui.selectable_value( + visible_history_boundary, + VisibleHistoryBoundary::Infinite, + VisibleHistoryBoundary::INFINITE_LABEL, + ) .on_hover_text(if low_bound { "Show data from the beginning of the timeline" } else { "Show data until the end of the timeline" - }) - .changed() - { - *visible_history_boundary = if infinite { - VisibleHistoryBoundary::Infinite - } else { - VisibleHistoryBoundary::Relative(0) - }; - } + }); + }); } fn colormap_props_ui( From 5ddf00acfe33ea4d1fe829872cfd87a15bf6c1f3 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Sat, 4 Nov 2023 10:00:15 +0100 Subject: [PATCH 12/14] Proper constants for 2D/3D space views' names --- crates/re_space_view_spatial/src/space_view_2d.rs | 6 +++++- crates/re_space_view_spatial/src/space_view_3d.rs | 6 +++++- crates/re_viewer/src/ui/selection_panel.rs | 5 ++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/re_space_view_spatial/src/space_view_2d.rs b/crates/re_space_view_spatial/src/space_view_2d.rs index 1feaf4e9b38c..63e126accf7a 100644 --- a/crates/re_space_view_spatial/src/space_view_2d.rs +++ b/crates/re_space_view_spatial/src/space_view_2d.rs @@ -16,11 +16,15 @@ use crate::{ #[derive(Default)] pub struct SpatialSpaceView2D; +impl SpatialSpaceView2D { + pub const NAME: &'static str = "2D"; +} + impl SpaceViewClass for SpatialSpaceView2D { type State = SpatialSpaceViewState; fn name(&self) -> re_viewer_context::SpaceViewClassName { - "2D".into() + Self::NAME.into() } fn icon(&self) -> &'static re_ui::Icon { diff --git a/crates/re_space_view_spatial/src/space_view_3d.rs b/crates/re_space_view_spatial/src/space_view_3d.rs index 93da23aceaed..6873972f31ac 100644 --- a/crates/re_space_view_spatial/src/space_view_3d.rs +++ b/crates/re_space_view_spatial/src/space_view_3d.rs @@ -16,11 +16,15 @@ use crate::{ #[derive(Default)] pub struct SpatialSpaceView3D; +impl SpatialSpaceView3D { + pub const NAME: &'static str = "3D"; +} + impl SpaceViewClass for SpatialSpaceView3D { type State = SpatialSpaceViewState; fn name(&self) -> re_viewer_context::SpaceViewClassName { - "3D".into() + Self::NAME.into() } fn icon(&self) -> &'static re_ui::Icon { diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 48bc952c29e5..39b270031d53 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -6,6 +6,7 @@ use re_data_store::{ }; use re_data_ui::{image_meaning_for_entity, item_ui, DataUi}; use re_log_types::TimeType; +use re_space_view_spatial::{SpatialSpaceView2D, SpatialSpaceView3D}; use re_types::{ components::{PinholeProjection, Transform3D}, tensor_data::TensorDataMeaning, @@ -416,7 +417,9 @@ fn visible_history_ui( entity_props: &mut EntityProperties, ) { //TODO(#4107): support more space view types. - if space_view_class_name != "3D" && space_view_class_name != "2D" { + if space_view_class_name != SpatialSpaceView3D::NAME + && space_view_class_name != SpatialSpaceView2D::NAME + { return; } From bf65bf6ea45b847db3a01e600198e5e23fb24f60 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Sat, 4 Nov 2023 11:46:30 +0100 Subject: [PATCH 13/14] Make it impossible to set "inverted" boundary (low > high) --- crates/re_viewer/src/ui/selection_panel.rs | 31 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 39b270031d53..4faacc7ffc10 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -465,6 +465,9 @@ fn visible_history_ui( &mut entity_props.visible_history.nanos }; + let visible_history_time_range = visible_history.from(current_time.into()).as_i64() + ..=visible_history.to(current_time.into()).as_i64(); + ui.add_enabled_ui(entity_props.visible_history.enabled, |ui| { egui::Grid::new("visible_history_boundaries") .num_columns(4) @@ -478,6 +481,7 @@ fn visible_history_ui( current_time, time_range.clone(), true, + *visible_history_time_range.end(), ); ui.end_row(); @@ -491,6 +495,7 @@ fn visible_history_ui( current_time, time_range, false, + *visible_history_time_range.start(), ); ui.end_row(); @@ -547,6 +552,7 @@ fn should_display_visible_history( true } +#[allow(clippy::too_many_arguments)] fn visible_history_boundary_ui( re_ui: &re_ui::ReUi, ui: &mut egui::Ui, @@ -555,15 +561,34 @@ fn visible_history_boundary_ui( current_time: i64, mut time_range: RangeInclusive, low_bound: bool, + other_boundary_absolute: i64, ) { let span = time_range.end() - time_range.start(); + + // Hot "usability" area! This achieves two things: + // 1) It makes sure the time range in relative mode has enough margin beyond the current + // timeline's span to avoid the boundary value to be modified by changing the current time + // cursor + // 2) It makes sure the two boundaries don't cross in time (i.e. low > high). It does so by + // prioritizing the low boundary. Moving the low boundary against the high boundary will + // displace the high boundary. On the other hand, the high boundary cannot be moved against + // the low boundary. This asymmetry is intentional, and avoids both boundaries fighting each + // other in some corner cases (when the user interacts with the current time cursor). + #[allow(clippy::collapsible_else_if)] // for readability if matches!( visible_history_boundary, VisibleHistoryBoundary::RelativeToTimeCursor(_) ) { - // the wider range is needed to avoid the DragValue bounds to modify the boundary value when - // the time slider is moved to the timeline's extremes - time_range = -span..=2 * span; + if low_bound { + time_range = -span..=2 * span; + } else { + time_range = + (other_boundary_absolute.saturating_sub(current_time)).at_least(-span)..=2 * span; + } + } else { + if !low_bound { + time_range = other_boundary_absolute.at_least(-span)..=*time_range.end(); + } } match visible_history_boundary { From 6bc0ba72dc79b5af782d73cbabb09b670c59b7e3 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Sat, 4 Nov 2023 19:55:04 +0100 Subject: [PATCH 14/14] Added TODO for `should_display_visible_history` --- crates/re_viewer/src/ui/selection_panel.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 4faacc7ffc10..ecbf5c6b4779 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -530,6 +530,8 @@ static VISIBLE_HISTORY_COMPONENT_NAMES: once_cell::sync::Lazy .into() }); +// TODO(#4145): This method is obviously unfortunate. It's a temporary solution until the ViewPart +// system is able to report its ability to handle the visible history feature. fn should_display_visible_history( ctx: &mut ViewerContext<'_>, entity_path: Option<&EntityPath>,