From 23ca7527973a55ae3a439f40f154f9008a655db6 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 29 Aug 2024 16:05:10 +0200 Subject: [PATCH 1/8] WIP: add PoV entity/component to dataframe view query UI --- Cargo.lock | 1 + .../blueprint/datatypes/time_range_query.fbs | 10 +- .../blueprint/datatypes/time_range_query.rs | 223 ++++++++++++++- .../datatypes/time_range_query_ext.rs | 2 + .../viewer/re_space_view_dataframe/Cargo.toml | 2 + .../src/query_kind_ui.rs | 258 ++++++++++++++---- .../src/space_view_class.rs | 8 +- .../re_space_view_dataframe/src/view_query.rs | 66 ++++- .../examples/re_ui_example/right_panel.rs | 28 ++ .../blueprint/datatypes/time_range_query.cpp | 37 ++- .../blueprint/datatypes/time_range_query.hpp | 7 + .../components/time_range_queries.py | 2 + .../blueprint/datatypes/time_range_query.py | 47 +++- 13 files changed, 607 insertions(+), 84 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c77102fea6f..e8fab705641d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4997,6 +4997,7 @@ version = "0.19.0-alpha.1+dev" dependencies = [ "egui", "egui_extras", + "itertools 0.13.0", "re_chunk_store", "re_data_ui", "re_entity_db", diff --git a/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs b/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs index 6b467209e580..05ea88800b84 100644 --- a/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs +++ b/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs @@ -10,9 +10,15 @@ table TimeRangeQuery ( /// Name of the timeline this applies to. timeline: rerun.datatypes.Utf8 (order: 100); + /// Point-of-view entity. + pov_entity: rerun.datatypes.EntityPath (order: 200); + + /// Point-of-view component. + pov_component: rerun.datatypes.Utf8 (order: 300); + /// Beginning of the time range. - start: rerun.datatypes.TimeInt (order: 200); + start: rerun.datatypes.TimeInt (order: 400); /// End of the time range (inclusive). - end: rerun.datatypes.TimeInt (order: 300); + end: rerun.datatypes.TimeInt (order: 500); } diff --git a/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs b/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs index 59e581545bd7..a12876e9ec9c 100644 --- a/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs +++ b/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs @@ -24,6 +24,12 @@ pub struct TimeRangeQuery { /// Name of the timeline this applies to. pub timeline: crate::datatypes::Utf8, + /// Point-of-view entity. + pub pov_entity: crate::datatypes::EntityPath, + + /// Point-of-view component. + pub pov_component: crate::datatypes::Utf8, + /// Beginning of the time range. pub start: crate::datatypes::TimeInt, @@ -34,12 +40,18 @@ pub struct TimeRangeQuery { impl ::re_types_core::SizeBytes for TimeRangeQuery { #[inline] fn heap_size_bytes(&self) -> u64 { - self.timeline.heap_size_bytes() + self.start.heap_size_bytes() + self.end.heap_size_bytes() + self.timeline.heap_size_bytes() + + self.pov_entity.heap_size_bytes() + + self.pov_component.heap_size_bytes() + + self.start.heap_size_bytes() + + self.end.heap_size_bytes() } #[inline] fn is_pod() -> bool { ::is_pod() + && ::is_pod() + && ::is_pod() && ::is_pod() && ::is_pod() } @@ -65,6 +77,16 @@ impl ::re_types_core::Loggable for TimeRangeQuery { ::arrow_datatype(), false, ), + Field::new( + "pov_entity", + ::arrow_datatype(), + false, + ), + Field::new( + "pov_component", + ::arrow_datatype(), + false, + ), Field::new( "start", ::arrow_datatype(), @@ -135,6 +157,79 @@ impl ::re_types_core::Loggable for TimeRangeQuery { .boxed() } }, + { + let (somes, pov_entity): (Vec<_>, Vec<_>) = data + .iter() + .map(|datum| { + let datum = datum.as_ref().map(|datum| datum.pov_entity.clone()); + (datum.is_some(), datum) + }) + .unzip(); + let pov_entity_bitmap: Option = { + let any_nones = somes.iter().any(|some| !*some); + any_nones.then(|| somes.into()) + }; + { + let offsets = arrow2::offset::Offsets::::try_from_lengths( + pov_entity.iter().map(|opt| { + opt.as_ref().map(|datum| datum.0.len()).unwrap_or_default() + }), + )? + .into(); + let inner_data: arrow2::buffer::Buffer = pov_entity + .into_iter() + .flatten() + .flat_map(|datum| datum.0 .0) + .collect(); + + #[allow(unsafe_code, clippy::undocumented_unsafe_blocks)] + unsafe { + Utf8Array::::new_unchecked( + DataType::Utf8, + offsets, + inner_data, + pov_entity_bitmap, + ) + } + .boxed() + } + }, + { + let (somes, pov_component): (Vec<_>, Vec<_>) = data + .iter() + .map(|datum| { + let datum = datum.as_ref().map(|datum| datum.pov_component.clone()); + (datum.is_some(), datum) + }) + .unzip(); + let pov_component_bitmap: Option = { + let any_nones = somes.iter().any(|some| !*some); + any_nones.then(|| somes.into()) + }; + { + let offsets = arrow2::offset::Offsets::::try_from_lengths( + pov_component.iter().map(|opt| { + opt.as_ref().map(|datum| datum.0.len()).unwrap_or_default() + }), + )? + .into(); + let inner_data: arrow2::buffer::Buffer = pov_component + .into_iter() + .flatten() + .flat_map(|datum| datum.0 .0) + .collect(); + #[allow(unsafe_code, clippy::undocumented_unsafe_blocks)] + unsafe { + Utf8Array::::new_unchecked( + DataType::Utf8, + offsets, + inner_data, + pov_component_bitmap, + ) + } + .boxed() + } + }, { let (somes, start): (Vec<_>, Vec<_>) = data .iter() @@ -269,6 +364,118 @@ impl ::re_types_core::Loggable for TimeRangeQuery { .into_iter() } }; + let pov_entity = { + if !arrays_by_name.contains_key("pov_entity") { + return Err(DeserializationError::missing_struct_field( + Self::arrow_datatype(), + "pov_entity", + )) + .with_context("rerun.blueprint.datatypes.TimeRangeQuery"); + } + let arrow_data = &**arrays_by_name["pov_entity"]; + { + let arrow_data = arrow_data + .as_any() + .downcast_ref::>() + .ok_or_else(|| { + let expected = DataType::Utf8; + let actual = arrow_data.data_type().clone(); + DeserializationError::datatype_mismatch(expected, actual) + }) + .with_context("rerun.blueprint.datatypes.TimeRangeQuery#pov_entity")?; + let arrow_data_buf = arrow_data.values(); + let offsets = arrow_data.offsets(); + arrow2::bitmap::utils::ZipValidity::new_with_validity( + offsets.iter().zip(offsets.lengths()), + arrow_data.validity(), + ) + .map(|elem| { + elem.map(|(start, len)| { + let start = *start as usize; + let end = start + len; + if end > arrow_data_buf.len() { + return Err(DeserializationError::offset_slice_oob( + (start, end), + arrow_data_buf.len(), + )); + } + + #[allow(unsafe_code, clippy::undocumented_unsafe_blocks)] + let data = + unsafe { arrow_data_buf.clone().sliced_unchecked(start, len) }; + Ok(data) + }) + .transpose() + }) + .map(|res_or_opt| { + res_or_opt.map(|res_or_opt| { + res_or_opt.map(|v| { + crate::datatypes::EntityPath(::re_types_core::ArrowString(v)) + }) + }) + }) + .collect::>>>() + .with_context("rerun.blueprint.datatypes.TimeRangeQuery#pov_entity")? + .into_iter() + } + }; + let pov_component = { + if !arrays_by_name.contains_key("pov_component") { + return Err(DeserializationError::missing_struct_field( + Self::arrow_datatype(), + "pov_component", + )) + .with_context("rerun.blueprint.datatypes.TimeRangeQuery"); + } + let arrow_data = &**arrays_by_name["pov_component"]; + { + let arrow_data = arrow_data + .as_any() + .downcast_ref::>() + .ok_or_else(|| { + let expected = DataType::Utf8; + let actual = arrow_data.data_type().clone(); + DeserializationError::datatype_mismatch(expected, actual) + }) + .with_context( + "rerun.blueprint.datatypes.TimeRangeQuery#pov_component", + )?; + let arrow_data_buf = arrow_data.values(); + let offsets = arrow_data.offsets(); + arrow2::bitmap::utils::ZipValidity::new_with_validity( + offsets.iter().zip(offsets.lengths()), + arrow_data.validity(), + ) + .map(|elem| { + elem.map(|(start, len)| { + let start = *start as usize; + let end = start + len; + if end > arrow_data_buf.len() { + return Err(DeserializationError::offset_slice_oob( + (start, end), + arrow_data_buf.len(), + )); + } + + #[allow(unsafe_code, clippy::undocumented_unsafe_blocks)] + let data = + unsafe { arrow_data_buf.clone().sliced_unchecked(start, len) }; + Ok(data) + }) + .transpose() + }) + .map(|res_or_opt| { + res_or_opt.map(|res_or_opt| { + res_or_opt.map(|v| { + crate::datatypes::Utf8(::re_types_core::ArrowString(v)) + }) + }) + }) + .collect::>>>() + .with_context("rerun.blueprint.datatypes.TimeRangeQuery#pov_component")? + .into_iter() + } + }; let start = { if !arrays_by_name.contains_key("start") { return Err(DeserializationError::missing_struct_field( @@ -314,17 +521,27 @@ impl ::re_types_core::Loggable for TimeRangeQuery { .map(|res_or_opt| res_or_opt.map(crate::datatypes::TimeInt)) }; arrow2::bitmap::utils::ZipValidity::new_with_validity( - ::itertools::izip!(timeline, start, end), + ::itertools::izip!(timeline, pov_entity, pov_component, start, end), arrow_data.validity(), ) .map(|opt| { - opt.map(|(timeline, start, end)| { + opt.map(|(timeline, pov_entity, pov_component, start, end)| { Ok(Self { timeline: timeline .ok_or_else(DeserializationError::missing_data) .with_context( "rerun.blueprint.datatypes.TimeRangeQuery#timeline", )?, + pov_entity: pov_entity + .ok_or_else(DeserializationError::missing_data) + .with_context( + "rerun.blueprint.datatypes.TimeRangeQuery#pov_entity", + )?, + pov_component: pov_component + .ok_or_else(DeserializationError::missing_data) + .with_context( + "rerun.blueprint.datatypes.TimeRangeQuery#pov_component", + )?, start: start .ok_or_else(DeserializationError::missing_data) .with_context("rerun.blueprint.datatypes.TimeRangeQuery#start")?, diff --git a/crates/store/re_types/src/blueprint/datatypes/time_range_query_ext.rs b/crates/store/re_types/src/blueprint/datatypes/time_range_query_ext.rs index d8d54d875aeb..b314520b7022 100644 --- a/crates/store/re_types/src/blueprint/datatypes/time_range_query_ext.rs +++ b/crates/store/re_types/src/blueprint/datatypes/time_range_query_ext.rs @@ -5,6 +5,8 @@ impl Default for TimeRangeQuery { fn default() -> Self { Self { timeline: Utf8::from("log_time"), + pov_entity: Default::default(), + pov_component: Default::default(), start: TimeInt::MIN, end: TimeInt::MAX, } diff --git a/crates/viewer/re_space_view_dataframe/Cargo.toml b/crates/viewer/re_space_view_dataframe/Cargo.toml index c3e4af1e708a..0375b5e44a15 100644 --- a/crates/viewer/re_space_view_dataframe/Cargo.toml +++ b/crates/viewer/re_space_view_dataframe/Cargo.toml @@ -35,3 +35,5 @@ re_viewport_blueprint.workspace = true egui_extras.workspace = true egui.workspace = true +itertools.workspace = true + diff --git a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs index b0be3a92908a..1f118f88e558 100644 --- a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs @@ -1,17 +1,29 @@ -use re_log_types::{ResolvedTimeRange, TimeInt, TimeType, TimelineName}; +use re_log_types::{EntityPath, ResolvedTimeRange, TimeInt, TimeType, Timeline}; +use re_types_core::ComponentName; use re_ui::{list_item, UiExt}; use re_viewer_context::{TimeDragValue, ViewerContext}; +use std::collections::BTreeSet; use crate::view_query::QueryKind; /// Helper to handle the UI for the various query kinds are they are shown to the user. /// /// This struct is the "UI equivalent" of the [`QueryKind`] enum. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum UiQueryKind { - LatestAt { time: TimeInt }, - TimeRangeAll, - TimeRange { from: TimeInt, to: TimeInt }, + LatestAt { + time: TimeInt, + }, + TimeRangeAll { + pov_entity: EntityPath, + pov_component: ComponentName, + }, + TimeRange { + pov_entity: EntityPath, + pov_component: ComponentName, + from: TimeInt, + to: TimeInt, + }, } impl UiQueryKind { @@ -21,10 +33,10 @@ impl UiQueryKind { ctx: &ViewerContext<'_>, ui: &mut egui::Ui, time_drag_value: &TimeDragValue, - timeline_name: &TimelineName, - time_type: TimeType, + timeline: &Timeline, + all_entities: &BTreeSet, ) -> bool { - let orig_self = *self; + let orig_self = self.clone(); ui.vertical(|ui| { // @@ -46,7 +58,7 @@ impl UiQueryKind { } .into(); - changed |= match time_type { + changed |= match timeline.typ() { TimeType::Time => time_drag_value .temporal_drag_value_ui( ui, @@ -72,50 +84,148 @@ impl UiQueryKind { // TIME RANGE ALL // - ui.horizontal(|ui| { - let mut is_time_range_all = matches!(self, Self::TimeRangeAll); - if ui - .re_radio_value(&mut is_time_range_all, true, "From –∞ to +∞") - .changed() - && is_time_range_all - { - *self = Self::TimeRangeAll; - } - }); + let mut changed = false; + let mut is_time_range_all = matches!(self, Self::TimeRangeAll { .. }); + changed |= ui + .re_radio_value(&mut is_time_range_all, true, "From –∞ to +∞") + .changed(); // // TIME RANGE CUSTOM // - ui.vertical(|ui| { - let mut is_time_range_custom = matches!(self, Self::TimeRange { .. }); - let mut changed = ui - .re_radio_value(&mut is_time_range_custom, true, "Define time range") - .changed(); + let mut is_time_range_custom = matches!(self, Self::TimeRange { .. }); + if ui + .re_radio_value(&mut is_time_range_custom, true, "Define time range") + .changed() + { + //TODO: fix that ugly hack + is_time_range_all = false; + changed = true; + } - let mut should_display_time_range = false; + // + // EXTRA UI FOR THE TIME RANGE OPTIONS + // - if is_time_range_custom { - ui.spacing_mut().indent = ui.spacing().icon_width + ui.spacing().icon_spacing; - ui.indent("time_range_custom", |ui| { - ui.add_space(-4.0); + if is_time_range_all || is_time_range_custom { + ui.spacing_mut().indent = ui.spacing().icon_width + ui.spacing().icon_spacing; + ui.indent("time_range_custom", |ui| { + ui.add_space(-4.0); - let mut from = if let Self::TimeRange { from, .. } = self { - (*from).into() - } else { - (*time_drag_value.range.start()).into() + list_item::list_item_scope(ui, "time_range", |ui| { + // + // POV ENTITY + // + + let current_entity = match self { + Self::TimeRangeAll { pov_entity, .. } + | Self::TimeRange { pov_entity, .. } => all_entities + .contains(pov_entity) + .then(|| pov_entity.clone()), + Self::LatestAt { .. } => None, }; - let mut to = if let Self::TimeRange { to, .. } = self { - (*to).into() - } else { - (*time_drag_value.range.end()).into() + let mut pov_entity = current_entity + .clone() + .and_then(|entity| all_entities.contains(&entity).then_some(entity)) + .or_else(|| all_entities.iter().next().cloned()) + .unwrap_or_else(|| EntityPath::from("/")); + changed |= Some(&pov_entity) != current_entity.as_ref(); + + // let mut pov_entity = + // current_entity.unwrap_or_else(|| EntityPath::from("/")); + + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("PoV entity").value_fn(|ui, _| { + egui::ComboBox::new("pov_entity", "") + .selected_text(pov_entity.to_string()) + .show_ui(ui, |ui| { + for entity in all_entities { + changed |= ui + .selectable_value( + &mut pov_entity, + entity.clone(), + entity.to_string(), + ) + .changed(); + } + }); + }), + ); + + // + // POV COMPONENT + // + + let all_components = ctx + .recording_store() + .all_components_on_timeline(timeline, &pov_entity) + .unwrap_or_default(); + + let current_component = match self { + Self::TimeRangeAll { pov_component, .. } + | Self::TimeRange { pov_component, .. } => Some(*pov_component), + Self::LatestAt { .. } => None, }; - list_item::list_item_scope(ui, "time_range_custom_scope", |ui| { + // If the currently saved component, we auto-switch it to a reasonable one. + let mut pov_component = current_component + .and_then(|component| { + all_components.contains(&component).then_some(component) + }) + //TODO(ab): we should be smarter here, e.g. take the required component of the detected archetype + .or_else(|| all_components.first().copied()) + .unwrap_or_else(|| ComponentName::from("-")); + changed |= Some(pov_component) != current_component; + + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("PoV component").value_fn(|ui, _| { + egui::ComboBox::new("pov_component", "") + .selected_text(pov_component.short_name()) + .show_ui(ui, |ui| { + for component in &all_components { + changed |= ui + .selectable_value( + &mut pov_component, + *component, + component.short_name(), + ) + .changed(); + } + }); + }), + ); + + // + // TIME RANGE BOUNDARIES + // + + if is_time_range_all { + if changed { + *self = Self::TimeRangeAll { + pov_entity, + pov_component, + }; + } + } else { + let mut should_display_time_range = false; + + let mut from = if let Self::TimeRange { from, .. } = self { + (*from).into() + } else { + (*time_drag_value.range.start()).into() + }; + + let mut to = if let Self::TimeRange { to, .. } = self { + (*to).into() + } else { + (*time_drag_value.range.end()).into() + }; + ui.list_item_flat_noninteractive( list_item::PropertyContent::new("Start").value_fn(|ui, _| { - let response = match time_type { + let response = match timeline.typ() { TimeType::Time => { time_drag_value .temporal_drag_value_ui( @@ -140,7 +250,7 @@ impl UiQueryKind { ui.list_item_flat_noninteractive( list_item::PropertyContent::new("End").value_fn(|ui, _| { - let response = match time_type { + let response = match timeline.typ() { TimeType::Time => { time_drag_value .temporal_drag_value_ui( @@ -162,25 +272,27 @@ impl UiQueryKind { || response.has_focus(); }), ); - }); - if changed { - *self = Self::TimeRange { - from: from.into(), - to: to.into(), - }; - } + if changed { + *self = Self::TimeRange { + pov_entity, + pov_component, + from: from.into(), + to: to.into(), + }; + } - if should_display_time_range { - let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); - if time_ctrl.timeline().name() == timeline_name { - time_ctrl.highlighted_range = - Some(ResolvedTimeRange::new(from, to)); + if should_display_time_range { + let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); + if time_ctrl.timeline() == timeline { + time_ctrl.highlighted_range = + Some(ResolvedTimeRange::new(from, to)); + } } } }); - } - }); + }); + } }); *self != orig_self @@ -192,10 +304,25 @@ impl From for UiQueryKind { match value { QueryKind::LatestAt { time } => Self::LatestAt { time }, QueryKind::Range { + pov_entity, + pov_component, from: TimeInt::MIN, to: TimeInt::MAX, - } => Self::TimeRangeAll, - QueryKind::Range { from, to } => Self::TimeRange { from, to }, + } => Self::TimeRangeAll { + pov_entity: pov_entity.clone(), + pov_component, + }, + QueryKind::Range { + pov_entity, + pov_component, + from, + to, + } => Self::TimeRange { + pov_entity, + pov_component, + from, + to, + }, } } } @@ -204,11 +331,26 @@ impl From for QueryKind { fn from(value: UiQueryKind) -> Self { match value { UiQueryKind::LatestAt { time } => Self::LatestAt { time }, - UiQueryKind::TimeRangeAll => Self::Range { + UiQueryKind::TimeRangeAll { + pov_entity, + pov_component, + } => Self::Range { + pov_entity, + pov_component, from: TimeInt::MIN, to: TimeInt::MAX, }, - UiQueryKind::TimeRange { from, to } => Self::Range { from, to }, + UiQueryKind::TimeRange { + pov_entity, + pov_component, + from, + to, + } => Self::Range { + pov_entity, + pov_component, + from, + to, + }, } } } diff --git a/crates/viewer/re_space_view_dataframe/src/space_view_class.rs b/crates/viewer/re_space_view_dataframe/src/space_view_class.rs index bc62590809f7..010aa5eea610 100644 --- a/crates/viewer/re_space_view_dataframe/src/space_view_class.rs +++ b/crates/viewer/re_space_view_dataframe/src/space_view_class.rs @@ -142,7 +142,13 @@ mode sets the default time range to _everything_. You can override this in the s QueryKind::LatestAt { time } => { latest_at_table_ui(ctx, ui, query, &LatestAtQuery::new(*timeline, time)); } - QueryKind::Range { from, to } => { + QueryKind::Range { + pov_entity: _pov_entity, + pov_component: _pov_component, + from, + to, + } => { + //TODO: use pov entity and component let time_range_table_order = ViewProperty::from_archetype::( ctx.blueprint_db(), diff --git a/crates/viewer/re_space_view_dataframe/src/view_query.rs b/crates/viewer/re_space_view_dataframe/src/view_query.rs index 96aa6f53d886..a6fa1c236d1b 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query.rs @@ -1,25 +1,27 @@ -use re_log_types::{TimeInt, TimelineName}; +use re_log_types::{EntityPath, TimeInt, TimelineName}; use re_types::blueprint::{archetypes, components, datatypes}; -use re_types_core::Loggable as _; +use re_types_core::{ComponentName, Loggable as _}; use re_ui::UiExt as _; use re_viewer_context::{ SpaceViewId, SpaceViewState, SpaceViewSystemExecutionError, TimeDragValue, ViewerContext, }; use re_viewport_blueprint::ViewProperty; +use std::collections::BTreeSet; use crate::query_kind_ui::UiQueryKind; use crate::visualizer_system::EmptySystem; /// The query kind for the dataframe view. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub(crate) enum QueryKind { LatestAt { time: TimeInt, }, Range { + pov_entity: EntityPath, + pov_component: ComponentName, from: TimeInt, to: TimeInt, - //TODO(#7072): add PoV components }, //TODO(#7067): add selected components } @@ -69,15 +71,24 @@ impl Query { QueryKind::LatestAt { time } } components::QueryKind::TimeRange => { - let (from, to) = property + let time_range_queries = property .component_or_empty::()? - .unwrap_or_default() - .query_for_timeline(&timeline) - .map_or((TimeInt::MIN, TimeInt::MAX), |q| { - (q.start.into(), q.end.into()) - }); + .unwrap_or_default(); - QueryKind::Range { from, to } + let Some(time_range_query) = time_range_queries.query_for_timeline(&timeline) + else { + // It's hard to recover from a missing time range query and provide a meaningful + // default, so we just fall back to the latest-at query. + //TODO(ab): should this be an error? + return Ok(Self::FollowTimeline); + }; + + QueryKind::Range { + pov_entity: time_range_query.pov_entity.clone().into(), + pov_component: ComponentName::from(time_range_query.pov_component.as_str()), + from: time_range_query.start.into(), + to: time_range_query.end.into(), + } } }; @@ -103,7 +114,7 @@ impl Query { time: time_ctrl.time_int().unwrap_or(TimeInt::MAX), } } - Self::Override { kind, .. } => *kind, + Self::Override { kind, .. } => kind.clone(), } } @@ -134,13 +145,20 @@ impl Query { property.save_blueprint_component(ctx, &latest_at_queries); property.save_blueprint_component(ctx, &components::QueryKind::LatestAt); } - QueryKind::Range { from, to } => { + QueryKind::Range { + pov_entity, + pov_component, + from, + to, + } => { let mut time_range_queries = property .component_or_empty::()? .unwrap_or_default(); time_range_queries.set_query_for_timeline(datatypes::TimeRangeQuery { timeline: timeline_name.as_str().into(), + pov_entity: pov_entity.into(), + pov_component: pov_component.as_str().into(), start: (*from).into(), end: (*to).into(), }); @@ -271,8 +289,26 @@ fn override_ui( } else { TimeDragValue::from_time_range(0..=0) }; - let changed = - ui_query_kind.ui(ctx, ui, &time_drag_value, timeline.name(), timeline.typ()); + + // Gather all entities that can meaningfully be used as point-of-view: + // - part of this view + // - has any component on the chosen timeline + let mut all_entities = BTreeSet::new(); + ctx.lookup_query_result(space_view_id) + .tree + .visit(&mut |node| { + if !node.data_result.tree_prefix_only { + let comp_for_entity = ctx + .recording_store() + .all_components_on_timeline(&timeline, &node.data_result.entity_path); + if comp_for_entity.is_some_and(|components| !components.is_empty()) { + all_entities.insert(node.data_result.entity_path.clone()); + } + } + true + }); + + let changed = ui_query_kind.ui(ctx, ui, &time_drag_value, &timeline, &all_entities); if changed { Query::save_kind_for_timeline( ctx, diff --git a/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs b/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs index 40983dda3570..3796f717f536 100644 --- a/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs +++ b/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs @@ -153,6 +153,34 @@ impl RightPanel { }), ); + ui.list_item().show_hierarchical( + ui, + list_item::LabelContent::new("Fake radio button").with_icon_fn( + |ui, rect, _visuals| { + let mut ui = ui.child_ui( + rect, + egui::Layout::left_to_right(egui::Align::Center), + None, + ); + ui.re_radio_value(&mut self.boolean, true, ""); + }, + ), + ); + + ui.list_item().show_hierarchical( + ui, + list_item::LabelContent::new("Fake radio button").with_icon_fn( + |ui, rect, _visuals| { + let mut ui = ui.child_ui( + rect, + egui::Layout::left_to_right(egui::Align::Center), + None, + ); + ui.re_radio_value(&mut self.boolean, false, ""); + }, + ), + ); + ui.list_item() .show_hierarchical( ui, diff --git a/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.cpp b/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.cpp index 1ce005eba6b8..725277cc81c9 100644 --- a/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.cpp +++ b/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.cpp @@ -3,6 +3,7 @@ #include "time_range_query.hpp" +#include "../../datatypes/entity_path.hpp" #include "../../datatypes/time_int.hpp" #include "../../datatypes/utf8.hpp" @@ -16,6 +17,16 @@ namespace rerun { Loggable::arrow_datatype() { static const auto datatype = arrow::struct_({ arrow::field("timeline", Loggable::arrow_datatype(), false), + arrow::field( + "pov_entity", + Loggable::arrow_datatype(), + false + ), + arrow::field( + "pov_component", + Loggable::arrow_datatype(), + false + ), arrow::field("start", Loggable::arrow_datatype(), false), arrow::field("end", Loggable::arrow_datatype(), false), }); @@ -70,7 +81,29 @@ namespace rerun { } } { - auto field_builder = static_cast(builder->field_builder(1)); + auto field_builder = static_cast(builder->field_builder(1)); + ARROW_RETURN_NOT_OK(field_builder->Reserve(static_cast(num_elements))); + for (size_t elem_idx = 0; elem_idx < num_elements; elem_idx += 1) { + RR_RETURN_NOT_OK(Loggable::fill_arrow_array_builder( + field_builder, + &elements[elem_idx].pov_entity, + 1 + )); + } + } + { + auto field_builder = static_cast(builder->field_builder(2)); + ARROW_RETURN_NOT_OK(field_builder->Reserve(static_cast(num_elements))); + for (size_t elem_idx = 0; elem_idx < num_elements; elem_idx += 1) { + RR_RETURN_NOT_OK(Loggable::fill_arrow_array_builder( + field_builder, + &elements[elem_idx].pov_component, + 1 + )); + } + } + { + auto field_builder = static_cast(builder->field_builder(3)); ARROW_RETURN_NOT_OK(field_builder->Reserve(static_cast(num_elements))); for (size_t elem_idx = 0; elem_idx < num_elements; elem_idx += 1) { RR_RETURN_NOT_OK(Loggable::fill_arrow_array_builder( @@ -81,7 +114,7 @@ namespace rerun { } } { - auto field_builder = static_cast(builder->field_builder(2)); + auto field_builder = static_cast(builder->field_builder(4)); ARROW_RETURN_NOT_OK(field_builder->Reserve(static_cast(num_elements))); for (size_t elem_idx = 0; elem_idx < num_elements; elem_idx += 1) { RR_RETURN_NOT_OK(Loggable::fill_arrow_array_builder( diff --git a/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp b/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp index 91e71a00acd2..e3d7c067051c 100644 --- a/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp +++ b/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp @@ -3,6 +3,7 @@ #pragma once +#include "../../datatypes/entity_path.hpp" #include "../../datatypes/time_int.hpp" #include "../../datatypes/utf8.hpp" #include "../../result.hpp" @@ -22,6 +23,12 @@ namespace rerun::blueprint::datatypes { /// Name of the timeline this applies to. rerun::datatypes::Utf8 timeline; + /// Point-of-view entity. + rerun::datatypes::EntityPath pov_entity; + + /// Point-of-view component. + rerun::datatypes::Utf8 pov_component; + /// Beginning of the time range. rerun::datatypes::TimeInt start; diff --git a/rerun_py/rerun_sdk/rerun/blueprint/components/time_range_queries.py b/rerun_py/rerun_sdk/rerun/blueprint/components/time_range_queries.py index bb884758bc8b..9ce123e390cb 100644 --- a/rerun_py/rerun_sdk/rerun/blueprint/components/time_range_queries.py +++ b/rerun_py/rerun_sdk/rerun/blueprint/components/time_range_queries.py @@ -64,6 +64,8 @@ def __init__(self) -> None: "item", pa.struct([ pa.field("timeline", pa.utf8(), nullable=False, metadata={}), + pa.field("pov_entity", pa.utf8(), nullable=False, metadata={}), + pa.field("pov_component", pa.utf8(), nullable=False, metadata={}), pa.field("start", pa.int64(), nullable=False, metadata={}), pa.field("end", pa.int64(), nullable=False, metadata={}), ]), diff --git a/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py b/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py index f8ab08cfee48..1008d83adcac 100644 --- a/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py +++ b/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py @@ -33,11 +33,34 @@ def _time_range_query__timeline__special_field_converter_override(x: datatypes.U return datatypes.Utf8(x) +def _time_range_query__pov_entity__special_field_converter_override( + x: datatypes.EntityPathLike, +) -> datatypes.EntityPath: + if isinstance(x, datatypes.EntityPath): + return x + else: + return datatypes.EntityPath(x) + + +def _time_range_query__pov_component__special_field_converter_override(x: datatypes.Utf8Like) -> datatypes.Utf8: + if isinstance(x, datatypes.Utf8): + return x + else: + return datatypes.Utf8(x) + + @define(init=False) class TimeRangeQuery(TimeRangeQueryExt): """**Datatype**: Time range query configuration for a specific timeline.""" - def __init__(self: Any, timeline: datatypes.Utf8Like, start: datatypes.TimeIntLike, end: datatypes.TimeIntLike): + def __init__( + self: Any, + timeline: datatypes.Utf8Like, + pov_entity: datatypes.EntityPathLike, + pov_component: datatypes.Utf8Like, + start: datatypes.TimeIntLike, + end: datatypes.TimeIntLike, + ): """ Create a new instance of the TimeRangeQuery datatype. @@ -45,6 +68,10 @@ def __init__(self: Any, timeline: datatypes.Utf8Like, start: datatypes.TimeIntLi ---------- timeline: Name of the timeline this applies to. + pov_entity: + Point-of-view entity. + pov_component: + Point-of-view component. start: Beginning of the time range. end: @@ -53,13 +80,23 @@ def __init__(self: Any, timeline: datatypes.Utf8Like, start: datatypes.TimeIntLi """ # You can define your own __init__ function as a member of TimeRangeQueryExt in time_range_query_ext.py - self.__attrs_init__(timeline=timeline, start=start, end=end) + self.__attrs_init__(timeline=timeline, pov_entity=pov_entity, pov_component=pov_component, start=start, end=end) timeline: datatypes.Utf8 = field(converter=_time_range_query__timeline__special_field_converter_override) # Name of the timeline this applies to. # # (Docstring intentionally commented out to hide this field from the docs) + pov_entity: datatypes.EntityPath = field(converter=_time_range_query__pov_entity__special_field_converter_override) + # Point-of-view entity. + # + # (Docstring intentionally commented out to hide this field from the docs) + + pov_component: datatypes.Utf8 = field(converter=_time_range_query__pov_component__special_field_converter_override) + # Point-of-view component. + # + # (Docstring intentionally commented out to hide this field from the docs) + start: datatypes.TimeInt = field( converter=TimeRangeQueryExt.start__field_converter_override, # type: ignore[misc] ) @@ -90,6 +127,8 @@ def __init__(self) -> None: self, pa.struct([ pa.field("timeline", pa.utf8(), nullable=False, metadata={}), + pa.field("pov_entity", pa.utf8(), nullable=False, metadata={}), + pa.field("pov_component", pa.utf8(), nullable=False, metadata={}), pa.field("start", pa.int64(), nullable=False, metadata={}), pa.field("end", pa.int64(), nullable=False, metadata={}), ]), @@ -102,7 +141,7 @@ class TimeRangeQueryBatch(BaseBatch[TimeRangeQueryArrayLike]): @staticmethod def _native_to_pa_array(data: TimeRangeQueryArrayLike, data_type: pa.DataType) -> pa.Array: - from rerun.datatypes import TimeIntBatch, Utf8Batch + from rerun.datatypes import EntityPathBatch, TimeIntBatch, Utf8Batch if isinstance(data, TimeRangeQuery): data = [data] @@ -110,6 +149,8 @@ def _native_to_pa_array(data: TimeRangeQueryArrayLike, data_type: pa.DataType) - return pa.StructArray.from_arrays( [ Utf8Batch([x.timeline for x in data]).as_arrow_array().storage, # type: ignore[misc, arg-type] + EntityPathBatch([x.pov_entity for x in data]).as_arrow_array().storage, # type: ignore[misc, arg-type] + Utf8Batch([x.pov_component for x in data]).as_arrow_array().storage, # type: ignore[misc, arg-type] TimeIntBatch([x.start for x in data]).as_arrow_array().storage, # type: ignore[misc, arg-type] TimeIntBatch([x.end for x in data]).as_arrow_array().storage, # type: ignore[misc, arg-type] ], From 987ad98666441a42894ba5c85fd267496cdad26c Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 2 Sep 2024 13:08:33 +0200 Subject: [PATCH 2/8] Better PoV component suggestion --- crates/store/re_types_core/src/reflection.rs | 39 +++++++++++++++++++ .../src/query_kind_ui.rs | 33 ++++++++++++++-- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/crates/store/re_types_core/src/reflection.rs b/crates/store/re_types_core/src/reflection.rs index 82f223ebdf92..319eb0579934 100644 --- a/crates/store/re_types_core/src/reflection.rs +++ b/crates/store/re_types_core/src/reflection.rs @@ -20,6 +20,37 @@ pub struct Reflection { pub archetypes: ArchetypeReflectionMap, } +impl Reflection { + /// Find an [`ArchetypeReflection`] based on its short name. + /// + /// Useful when the only information available is the short name, e.g. when inferring archetype + /// names from an indicator component. + //TODO( #6889): tagged component will contain a fully qualified archetype name, so this function + // will be unnecessary. + pub fn archetype_reflection_from_short_name( + &self, + short_name: &str, + ) -> Option<&ArchetypeReflection> { + // note: this mirrors `ArchetypeName::short_name`'s implementation + self.archetypes + .get(&ArchetypeName::from(short_name)) + .or_else(|| { + self.archetypes.get(&ArchetypeName::from(format!( + "rerun.archetypes.{short_name}" + ))) + }) + .or_else(|| { + self.archetypes.get(&ArchetypeName::from(format!( + "rerun.blueprint.archetypes.{short_name}" + ))) + }) + .or_else(|| { + self.archetypes + .get(&ArchetypeName::from(format!("rerun.{short_name}"))) + }) + } +} + /// Runtime reflection about components. pub type ComponentReflectionMap = nohash_hasher::IntMap; @@ -53,6 +84,14 @@ pub struct ArchetypeReflection { pub fields: Vec, } +impl ArchetypeReflection { + /// Iterate over this archetype's required fields. + #[inline] + pub fn required_fields(&self) -> impl Iterator { + self.fields.iter().filter(|field| field.is_required) + } +} + /// Additional information about an archetype's field. #[derive(Clone, Debug)] pub struct ArchetypeFieldReflection { diff --git a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs index 1f118f88e558..76b22f2f757d 100644 --- a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs @@ -1,8 +1,9 @@ +use std::collections::BTreeSet; + use re_log_types::{EntityPath, ResolvedTimeRange, TimeInt, TimeType, Timeline}; -use re_types_core::ComponentName; +use re_types_core::{ComponentName, ComponentNameSet}; use re_ui::{list_item, UiExt}; use re_viewer_context::{TimeDragValue, ViewerContext}; -use std::collections::BTreeSet; use crate::view_query::QueryKind; @@ -169,13 +170,37 @@ impl UiQueryKind { Self::LatestAt { .. } => None, }; + // The list of suggested components is build as follows: + // - consider all indicator components + // - for the matching archetypes, take all required components + // - keep those that are actually present + let suggested_components = || { + all_components + .iter() + .filter_map(|c| { + c.indicator_component_archetype().and_then( + |archetype_short_name| { + ctx.reflection.archetype_reflection_from_short_name( + &archetype_short_name, + ) + }, + ) + }) + .flat_map(|archetype_reflection| { + archetype_reflection + .required_fields() + .map(|field| field.component_name) + }) + .filter(|c| all_components.contains(c)) + .collect::() + }; + // If the currently saved component, we auto-switch it to a reasonable one. let mut pov_component = current_component .and_then(|component| { all_components.contains(&component).then_some(component) }) - //TODO(ab): we should be smarter here, e.g. take the required component of the detected archetype - .or_else(|| all_components.first().copied()) + .or_else(|| suggested_components().first().copied()) .unwrap_or_else(|| ComponentName::from("-")); changed |= Some(pov_component) != current_component; From ffbba41858ef1167d13ff82e218a277bab39057a Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 2 Sep 2024 13:19:05 +0200 Subject: [PATCH 3/8] Fix deprecated warning --- .../re_ui/examples/re_ui_example/right_panel.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs b/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs index 3796f717f536..62d7989b996e 100644 --- a/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs +++ b/crates/viewer/re_ui/examples/re_ui_example/right_panel.rs @@ -157,11 +157,7 @@ impl RightPanel { ui, list_item::LabelContent::new("Fake radio button").with_icon_fn( |ui, rect, _visuals| { - let mut ui = ui.child_ui( - rect, - egui::Layout::left_to_right(egui::Align::Center), - None, - ); + let mut ui = ui.new_child(egui::UiBuilder::new().max_rect(rect)); ui.re_radio_value(&mut self.boolean, true, ""); }, ), @@ -171,11 +167,7 @@ impl RightPanel { ui, list_item::LabelContent::new("Fake radio button").with_icon_fn( |ui, rect, _visuals| { - let mut ui = ui.child_ui( - rect, - egui::Layout::left_to_right(egui::Align::Center), - None, - ); + let mut ui = ui.new_child(egui::UiBuilder::new().max_rect(rect)); ui.re_radio_value(&mut self.boolean, false, ""); }, ), From 596c1f954d1cfaa491ccdbbfb807f6dbc985fc80 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Mon, 2 Sep 2024 17:50:05 +0200 Subject: [PATCH 4/8] WIP: update UI according to latest design TODO: - fix inf-capable time widget - remove `UiQueryKind` - clean-up stuff --- .../src/query_kind_ui.rs | 535 +++++++++--------- .../re_space_view_dataframe/src/view_query.rs | 96 ++-- 2 files changed, 306 insertions(+), 325 deletions(-) diff --git a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs index 76b22f2f757d..83ba9d4baa6a 100644 --- a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs @@ -1,6 +1,6 @@ use std::collections::BTreeSet; -use re_log_types::{EntityPath, ResolvedTimeRange, TimeInt, TimeType, Timeline}; +use re_log_types::{EntityPath, ResolvedTimeRange, TimeInt, TimeType, TimeZone, Timeline}; use re_types_core::{ComponentName, ComponentNameSet}; use re_ui::{list_item, UiExt}; use re_viewer_context::{TimeDragValue, ViewerContext}; @@ -15,10 +15,6 @@ pub(crate) enum UiQueryKind { LatestAt { time: TimeInt, }, - TimeRangeAll { - pov_entity: EntityPath, - pov_component: ComponentName, - }, TimeRange { pov_entity: EntityPath, pov_component: ComponentName, @@ -39,286 +35,264 @@ impl UiQueryKind { ) -> bool { let orig_self = self.clone(); - ui.vertical(|ui| { - // - // LATEST-AT - // - - ui.horizontal(|ui| { - let mut is_latest_at = matches!(self, Self::LatestAt { .. }); - - let mut changed = ui - .re_radio_value(&mut is_latest_at, true, "Latest-at") - .changed(); - - if is_latest_at { - let mut time = if let Self::LatestAt { time } = self { - *time - } else { - TimeInt::MAX - } - .into(); - - changed |= match timeline.typ() { - TimeType::Time => time_drag_value - .temporal_drag_value_ui( - ui, - &mut time, - true, - None, - ctx.app_options.time_zone, - ) - .0 - .changed(), - TimeType::Sequence => time_drag_value - .sequence_drag_value_ui(ui, &mut time, true, None) - .changed(), - }; - - if changed { - *self = Self::LatestAt { time: time.into() }; - } - } - }); + // + // LATEST-AT + // - // - // TIME RANGE ALL - // + ui.horizontal(|ui| { + let mut is_latest_at = matches!(self, Self::LatestAt { .. }); - let mut changed = false; - let mut is_time_range_all = matches!(self, Self::TimeRangeAll { .. }); - changed |= ui - .re_radio_value(&mut is_time_range_all, true, "From –∞ to +∞") + let mut changed = ui + .re_radio_value(&mut is_latest_at, true, "Latest-at") .changed(); - // - // TIME RANGE CUSTOM - // - - let mut is_time_range_custom = matches!(self, Self::TimeRange { .. }); - if ui - .re_radio_value(&mut is_time_range_custom, true, "Define time range") - .changed() - { - //TODO: fix that ugly hack - is_time_range_all = false; - changed = true; + if is_latest_at { + let mut time = if let Self::LatestAt { time } = self { + *time + } else { + TimeInt::MAX + } + .into(); + + changed |= match timeline.typ() { + TimeType::Time => time_drag_value + .temporal_drag_value_ui( + ui, + &mut time, + true, + None, + ctx.app_options.time_zone, + ) + .0 + .changed(), + TimeType::Sequence => time_drag_value + .sequence_drag_value_ui(ui, &mut time, true, None) + .changed(), + }; + + if changed { + *self = Self::LatestAt { time: time.into() }; + } } + }); - // - // EXTRA UI FOR THE TIME RANGE OPTIONS - // - - if is_time_range_all || is_time_range_custom { - ui.spacing_mut().indent = ui.spacing().icon_width + ui.spacing().icon_spacing; - ui.indent("time_range_custom", |ui| { - ui.add_space(-4.0); - - list_item::list_item_scope(ui, "time_range", |ui| { - // - // POV ENTITY - // - - let current_entity = match self { - Self::TimeRangeAll { pov_entity, .. } - | Self::TimeRange { pov_entity, .. } => all_entities - .contains(pov_entity) - .then(|| pov_entity.clone()), - Self::LatestAt { .. } => None, - }; + // + // TIME RANGE CUSTOM + // - let mut pov_entity = current_entity - .clone() - .and_then(|entity| all_entities.contains(&entity).then_some(entity)) - .or_else(|| all_entities.iter().next().cloned()) - .unwrap_or_else(|| EntityPath::from("/")); - changed |= Some(&pov_entity) != current_entity.as_ref(); + let mut is_time_range_custom = matches!(self, Self::TimeRange { .. }); + let mut changed = ui + .re_radio_value(&mut is_time_range_custom, true, "Define time range") + .changed(); - // let mut pov_entity = - // current_entity.unwrap_or_else(|| EntityPath::from("/")); + // + // EXTRA UI FOR THE TIME RANGE OPTIONS + // - ui.list_item_flat_noninteractive( - list_item::PropertyContent::new("PoV entity").value_fn(|ui, _| { - egui::ComboBox::new("pov_entity", "") - .selected_text(pov_entity.to_string()) - .show_ui(ui, |ui| { - for entity in all_entities { - changed |= ui - .selectable_value( - &mut pov_entity, - entity.clone(), - entity.to_string(), - ) - .changed(); - } - }); - }), - ); + if is_time_range_custom { + ui.spacing_mut().indent = ui.spacing().icon_width + ui.spacing().icon_spacing; + ui.indent("time_range_custom", |ui| { + ui.add_space(-4.0); - // - // POV COMPONENT - // + list_item::list_item_scope(ui, "time_range", |ui| { + // + // TIME RANGE BOUNDARIES + // - let all_components = ctx - .recording_store() - .all_components_on_timeline(timeline, &pov_entity) - .unwrap_or_default(); + let mut should_display_time_range = false; - let current_component = match self { - Self::TimeRangeAll { pov_component, .. } - | Self::TimeRange { pov_component, .. } => Some(*pov_component), - Self::LatestAt { .. } => None, - }; + let mut from = if let Self::TimeRange { from, .. } = self { + (*from).into() + } else { + TimeInt::MIN.into() + }; - // The list of suggested components is build as follows: - // - consider all indicator components - // - for the matching archetypes, take all required components - // - keep those that are actually present - let suggested_components = || { - all_components - .iter() - .filter_map(|c| { - c.indicator_component_archetype().and_then( - |archetype_short_name| { - ctx.reflection.archetype_reflection_from_short_name( - &archetype_short_name, - ) - }, - ) - }) - .flat_map(|archetype_reflection| { - archetype_reflection - .required_fields() - .map(|field| field.component_name) - }) - .filter(|c| all_components.contains(c)) - .collect::() - }; + let mut to = if let Self::TimeRange { to, .. } = self { + (*to).into() + } else { + TimeInt::MAX.into() + }; - // If the currently saved component, we auto-switch it to a reasonable one. - let mut pov_component = current_component - .and_then(|component| { - all_components.contains(&component).then_some(component) - }) - .or_else(|| suggested_components().first().copied()) - .unwrap_or_else(|| ComponentName::from("-")); - changed |= Some(pov_component) != current_component; + // all time boundaries to not be aligned to the pov entity/component + list_item::list_item_scope(ui, "time_range_boundaries", |ui| { + let mut reset_from = false; ui.list_item_flat_noninteractive( - list_item::PropertyContent::new("PoV component").value_fn(|ui, _| { - egui::ComboBox::new("pov_component", "") - .selected_text(pov_component.short_name()) - .show_ui(ui, |ui| { - for component in &all_components { - changed |= ui - .selectable_value( - &mut pov_component, - *component, - component.short_name(), - ) - .changed(); - } - }); - }), - ); - - // - // TIME RANGE BOUNDARIES - // - - if is_time_range_all { - if changed { - *self = Self::TimeRangeAll { - pov_entity, - pov_component, - }; - } - } else { - let mut should_display_time_range = false; - - let mut from = if let Self::TimeRange { from, .. } = self { - (*from).into() - } else { - (*time_drag_value.range.start()).into() - }; - - let mut to = if let Self::TimeRange { to, .. } = self { - (*to).into() - } else { - (*time_drag_value.range.end()).into() - }; - - ui.list_item_flat_noninteractive( - list_item::PropertyContent::new("Start").value_fn(|ui, _| { - let response = match timeline.typ() { - TimeType::Time => { - time_drag_value - .temporal_drag_value_ui( - ui, - &mut from, - true, - None, - ctx.app_options.time_zone, - ) - .0 - } - TimeType::Sequence => time_drag_value - .sequence_drag_value_ui(ui, &mut from, true, None), - }; + list_item::PropertyContent::new("Start") + .action_button(&re_ui::icons::ADD, || { + reset_from = true; + }) + .value_fn(|ui, _| { + let response = time_boundary_ui( + ui, + time_drag_value, + None, + timeline.typ(), + ctx.app_options.time_zone, + &mut from, + ); changed |= response.changed(); should_display_time_range |= response.hovered() || response.dragged() || response.has_focus(); }), - ); - - ui.list_item_flat_noninteractive( - list_item::PropertyContent::new("End").value_fn(|ui, _| { - let response = match timeline.typ() { - TimeType::Time => { - time_drag_value - .temporal_drag_value_ui( - ui, - &mut to, - true, - Some(from), - ctx.app_options.time_zone, - ) - .0 - } - TimeType::Sequence => time_drag_value - .sequence_drag_value_ui(ui, &mut to, true, Some(from)), - }; + ); + + if reset_from { + from = TimeInt::MIN.into(); + changed = true; + } + + let mut reset_to = false; + + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("End") + .action_button(&re_ui::icons::ADD, || { + reset_to = true; + }) + .value_fn(|ui, _| { + let response = time_boundary_ui( + ui, + time_drag_value, + Some(from), + timeline.typ(), + ctx.app_options.time_zone, + &mut to, + ); changed |= response.changed(); should_display_time_range |= response.hovered() || response.dragged() || response.has_focus(); }), - ); - - if changed { - *self = Self::TimeRange { - pov_entity, - pov_component, - from: from.into(), - to: to.into(), - }; - } - - if should_display_time_range { - let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); - if time_ctrl.timeline() == timeline { - time_ctrl.highlighted_range = - Some(ResolvedTimeRange::new(from, to)); - } - } + ); + + if reset_to { + to = TimeInt::MAX.into(); + changed = true; } }); + + if should_display_time_range { + let mut time_ctrl = ctx.rec_cfg.time_ctrl.write(); + if time_ctrl.timeline() == timeline { + time_ctrl.highlighted_range = Some(ResolvedTimeRange::new(from, to)); + } + } + + // + // POV ENTITY + // + + let current_entity = match self { + Self::TimeRange { pov_entity, .. } => all_entities + .contains(pov_entity) + .then(|| pov_entity.clone()), + Self::LatestAt { .. } => None, + }; + + let mut pov_entity = current_entity + .clone() + .and_then(|entity| all_entities.contains(&entity).then_some(entity)) + .or_else(|| all_entities.iter().next().cloned()) + .unwrap_or_else(|| EntityPath::from("/")); + changed |= Some(&pov_entity) != current_entity.as_ref(); + + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("PoV entity").value_fn(|ui, _| { + egui::ComboBox::new("pov_entity", "") + .selected_text(pov_entity.to_string()) + .show_ui(ui, |ui| { + for entity in all_entities { + changed |= ui + .selectable_value( + &mut pov_entity, + entity.clone(), + entity.to_string(), + ) + .changed(); + } + }); + }), + ); + + // + // POV COMPONENT + // + + let all_components = ctx + .recording_store() + .all_components_on_timeline(timeline, &pov_entity) + .unwrap_or_default(); + + let current_component = match self { + Self::TimeRange { pov_component, .. } => Some(*pov_component), + Self::LatestAt { .. } => None, + }; + + // The list of suggested components is build as follows: + // - consider all indicator components + // - for the matching archetypes, take all required components + // - keep those that are actually present + let suggested_components = || { + all_components + .iter() + .filter_map(|c| { + c.indicator_component_archetype() + .and_then(|archetype_short_name| { + ctx.reflection.archetype_reflection_from_short_name( + &archetype_short_name, + ) + }) + }) + .flat_map(|archetype_reflection| { + archetype_reflection + .required_fields() + .map(|field| field.component_name) + }) + .filter(|c| all_components.contains(c)) + .collect::() + }; + + // If the currently saved component, we auto-switch it to a reasonable one. + let mut pov_component = current_component + .and_then(|component| { + all_components.contains(&component).then_some(component) + }) + .or_else(|| suggested_components().first().copied()) + .unwrap_or_else(|| ComponentName::from("-")); + changed |= Some(pov_component) != current_component; + + ui.list_item_flat_noninteractive( + list_item::PropertyContent::new("PoV component").value_fn(|ui, _| { + egui::ComboBox::new("pov_component", "") + .selected_text(pov_component.short_name()) + .show_ui(ui, |ui| { + for component in &all_components { + changed |= ui + .selectable_value( + &mut pov_component, + *component, + component.short_name(), + ) + .changed(); + } + }); + }), + ); + + if changed { + *self = Self::TimeRange { + pov_entity, + pov_component, + from: from.into(), + to: to.into(), + }; + } }); - } - }); + }); + } *self != orig_self } @@ -328,15 +302,6 @@ impl From for UiQueryKind { fn from(value: QueryKind) -> Self { match value { QueryKind::LatestAt { time } => Self::LatestAt { time }, - QueryKind::Range { - pov_entity, - pov_component, - from: TimeInt::MIN, - to: TimeInt::MAX, - } => Self::TimeRangeAll { - pov_entity: pov_entity.clone(), - pov_component, - }, QueryKind::Range { pov_entity, pov_component, @@ -356,15 +321,6 @@ impl From for QueryKind { fn from(value: UiQueryKind) -> Self { match value { UiQueryKind::LatestAt { time } => Self::LatestAt { time }, - UiQueryKind::TimeRangeAll { - pov_entity, - pov_component, - } => Self::Range { - pov_entity, - pov_component, - from: TimeInt::MIN, - to: TimeInt::MAX, - }, UiQueryKind::TimeRange { pov_entity, pov_component, @@ -379,3 +335,38 @@ impl From for QueryKind { } } } + +fn time_boundary_ui( + ui: &mut egui::Ui, + time_drag_value: &TimeDragValue, + low_bound_override: Option, + timeline_typ: TimeType, + time_zone: TimeZone, + time: &mut re_types_core::datatypes::TimeInt, +) -> egui::Response { + if *time == re_types_core::datatypes::TimeInt::MAX { + let response = ui.button("+∞"); + if response.dragged() { + *time = (*time_drag_value.range.end()).into(); + } + response + } else if *time == re_types_core::datatypes::TimeInt::MIN { + let response = ui.button("-∞"); + if response.dragged() { + *time = (*time_drag_value.range.start()).into(); + } + response + } else { + match timeline_typ { + TimeType::Time => { + time_drag_value + .temporal_drag_value_ui(ui, time, true, low_bound_override, time_zone) + .0 + } + + TimeType::Sequence => { + time_drag_value.sequence_drag_value_ui(ui, time, true, low_bound_override) + } + } + } +} diff --git a/crates/viewer/re_space_view_dataframe/src/view_query.rs b/crates/viewer/re_space_view_dataframe/src/view_query.rs index a6fa1c236d1b..4b46c886f29f 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query.rs @@ -265,60 +265,50 @@ fn override_ui( // we don't need to provide a fallback here as the timeline should be present by definition &EmptySystem {}, ); + }); + + let timeline = property + .component_or_empty::()? + .map(|t| t.into()) + .and_then(|timeline_name: TimelineName| { + ctx.recording() + .timelines() + .find(|t| t.name() == &timeline_name) + .copied() + }) + .unwrap_or(*ctx.rec_cfg.time_ctrl.read().timeline()); + let timeline_name = timeline.name(); - ui.end_row(); - - ui.grid_left_hand_label("Showing"); - - let timeline = property - .component_or_empty::()? - .map(|t| t.into()) - .and_then(|timeline_name: TimelineName| { - ctx.recording() - .timelines() - .find(|t| t.name() == &timeline_name) - .copied() - }) - .unwrap_or(*ctx.rec_cfg.time_ctrl.read().timeline()); - let timeline_name = timeline.name(); - - let query = Query::try_from_blueprint(ctx, space_view_id)?; - let mut ui_query_kind: UiQueryKind = query.kind(ctx).into(); - let time_drag_value = if let Some(times) = ctx.recording().time_histogram(&timeline) { - TimeDragValue::from_time_histogram(times) - } else { - TimeDragValue::from_time_range(0..=0) - }; - - // Gather all entities that can meaningfully be used as point-of-view: - // - part of this view - // - has any component on the chosen timeline - let mut all_entities = BTreeSet::new(); - ctx.lookup_query_result(space_view_id) - .tree - .visit(&mut |node| { - if !node.data_result.tree_prefix_only { - let comp_for_entity = ctx - .recording_store() - .all_components_on_timeline(&timeline, &node.data_result.entity_path); - if comp_for_entity.is_some_and(|components| !components.is_empty()) { - all_entities.insert(node.data_result.entity_path.clone()); - } - } - true - }); - - let changed = ui_query_kind.ui(ctx, ui, &time_drag_value, &timeline, &all_entities); - if changed { - Query::save_kind_for_timeline( - ctx, - space_view_id, - timeline_name, - &ui_query_kind.into(), - )?; + let query = Query::try_from_blueprint(ctx, space_view_id)?; + let mut ui_query_kind: UiQueryKind = query.kind(ctx).into(); + let time_drag_value = if let Some(times) = ctx.recording().time_histogram(&timeline) { + TimeDragValue::from_time_histogram(times) + } else { + TimeDragValue::from_time_range(0..=0) + }; + + // Gather all entities that can meaningfully be used as point-of-view: + // - part of this view + // - has any component on the chosen timeline + let mut all_entities = BTreeSet::new(); + ctx.lookup_query_result(space_view_id) + .tree + .visit(&mut |node| { + if !node.data_result.tree_prefix_only { + let comp_for_entity = ctx + .recording_store() + .all_components_on_timeline(&timeline, &node.data_result.entity_path); + if comp_for_entity.is_some_and(|components| !components.is_empty()) { + all_entities.insert(node.data_result.entity_path.clone()); + } } + true + }); - Ok(()) - }) - .inner + let changed = ui_query_kind.ui(ctx, ui, &time_drag_value, &timeline, &all_entities); + if changed { + Query::save_kind_for_timeline(ctx, space_view_id, timeline_name, &ui_query_kind.into())?; + } + + Ok(()) } From d77e6045e41e3ec11f0b54dadc0321da5bc0f216 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 3 Sep 2024 09:17:16 +0200 Subject: [PATCH 5/8] Post merge fixes --- .../src/query_kind_ui.rs | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs index 83ba9d4baa6a..cb9e92a39d50 100644 --- a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs @@ -51,8 +51,7 @@ impl UiQueryKind { *time } else { TimeInt::MAX - } - .into(); + }; changed |= match timeline.typ() { TimeType::Time => time_drag_value @@ -71,7 +70,7 @@ impl UiQueryKind { }; if changed { - *self = Self::LatestAt { time: time.into() }; + *self = Self::LatestAt { time }; } } }); @@ -102,15 +101,15 @@ impl UiQueryKind { let mut should_display_time_range = false; let mut from = if let Self::TimeRange { from, .. } = self { - (*from).into() + *from } else { - TimeInt::MIN.into() + TimeInt::MIN }; let mut to = if let Self::TimeRange { to, .. } = self { - (*to).into() + *to } else { - TimeInt::MAX.into() + TimeInt::MAX }; // all time boundaries to not be aligned to the pov entity/component @@ -140,7 +139,7 @@ impl UiQueryKind { ); if reset_from { - from = TimeInt::MIN.into(); + from = TimeInt::MIN; changed = true; } @@ -169,7 +168,7 @@ impl UiQueryKind { ); if reset_to { - to = TimeInt::MAX.into(); + to = TimeInt::MAX; changed = true; } }); @@ -286,8 +285,8 @@ impl UiQueryKind { *self = Self::TimeRange { pov_entity, pov_component, - from: from.into(), - to: to.into(), + from, + to, }; } }); @@ -339,21 +338,21 @@ impl From for QueryKind { fn time_boundary_ui( ui: &mut egui::Ui, time_drag_value: &TimeDragValue, - low_bound_override: Option, + low_bound_override: Option, timeline_typ: TimeType, time_zone: TimeZone, - time: &mut re_types_core::datatypes::TimeInt, + time: &mut TimeInt, ) -> egui::Response { - if *time == re_types_core::datatypes::TimeInt::MAX { + if *time == TimeInt::MAX { let response = ui.button("+∞"); if response.dragged() { - *time = (*time_drag_value.range.end()).into(); + *time = time_drag_value.max_time(); } response - } else if *time == re_types_core::datatypes::TimeInt::MIN { + } else if *time == TimeInt::MIN { let response = ui.button("-∞"); if response.dragged() { - *time = (*time_drag_value.range.start()).into(); + *time = time_drag_value.min_time(); } response } else { From b75a776025ce9bb6491c3af4190bb1ef58c3d7c8 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 3 Sep 2024 09:55:18 +0200 Subject: [PATCH 6/8] Minor tweak and code refactor/cleaning --- .../viewer/re_space_view_dataframe/src/lib.rs | 2 +- .../src/{query_kind_ui.rs => query_kind.rs} | 95 +++++++------------ .../src/space_view_class.rs | 8 +- .../re_space_view_dataframe/src/view_query.rs | 30 ++---- 4 files changed, 45 insertions(+), 90 deletions(-) rename crates/viewer/re_space_view_dataframe/src/{query_kind_ui.rs => query_kind.rs} (84%) diff --git a/crates/viewer/re_space_view_dataframe/src/lib.rs b/crates/viewer/re_space_view_dataframe/src/lib.rs index b036b03577b4..92e83e1e8ed0 100644 --- a/crates/viewer/re_space_view_dataframe/src/lib.rs +++ b/crates/viewer/re_space_view_dataframe/src/lib.rs @@ -3,7 +3,7 @@ //! A Space View that shows the data contained in entities in a table. mod latest_at_table; -mod query_kind_ui; +mod query_kind; mod space_view_class; mod table_ui; mod time_range_table; diff --git a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs b/crates/viewer/re_space_view_dataframe/src/query_kind.rs similarity index 84% rename from crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs rename to crates/viewer/re_space_view_dataframe/src/query_kind.rs index cb9e92a39d50..91d69e12ab69 100644 --- a/crates/viewer/re_space_view_dataframe/src/query_kind_ui.rs +++ b/crates/viewer/re_space_view_dataframe/src/query_kind.rs @@ -5,25 +5,22 @@ use re_types_core::{ComponentName, ComponentNameSet}; use re_ui::{list_item, UiExt}; use re_viewer_context::{TimeDragValue, ViewerContext}; -use crate::view_query::QueryKind; - -/// Helper to handle the UI for the various query kinds are they are shown to the user. -/// -/// This struct is the "UI equivalent" of the [`QueryKind`] enum. -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) enum UiQueryKind { +/// The query kind for the dataframe view. +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum QueryKind { LatestAt { time: TimeInt, }, - TimeRange { + Range { pov_entity: EntityPath, pov_component: ComponentName, from: TimeInt, to: TimeInt, }, + //TODO(#7067): add selected components } -impl UiQueryKind { +impl QueryKind { /// Show the UI for the query kind selector. pub(crate) fn ui( &mut self, @@ -79,7 +76,7 @@ impl UiQueryKind { // TIME RANGE CUSTOM // - let mut is_time_range_custom = matches!(self, Self::TimeRange { .. }); + let mut is_time_range_custom = matches!(self, Self::Range { .. }); let mut changed = ui .re_radio_value(&mut is_time_range_custom, true, "Define time range") .changed(); @@ -100,13 +97,13 @@ impl UiQueryKind { let mut should_display_time_range = false; - let mut from = if let Self::TimeRange { from, .. } = self { + let mut from = if let Self::Range { from, .. } = self { *from } else { TimeInt::MIN }; - let mut to = if let Self::TimeRange { to, .. } = self { + let mut to = if let Self::Range { to, .. } = self { *to } else { TimeInt::MAX @@ -118,9 +115,13 @@ impl UiQueryKind { ui.list_item_flat_noninteractive( list_item::PropertyContent::new("Start") - .action_button(&re_ui::icons::ADD, || { - reset_from = true; - }) + .action_button_with_enabled( + &re_ui::icons::RESET, + from != TimeInt::MIN, + || { + reset_from = true; + }, + ) .value_fn(|ui, _| { let response = time_boundary_ui( ui, @@ -147,9 +148,13 @@ impl UiQueryKind { ui.list_item_flat_noninteractive( list_item::PropertyContent::new("End") - .action_button(&re_ui::icons::ADD, || { - reset_to = true; - }) + .action_button_with_enabled( + &re_ui::icons::RESET, + to != TimeInt::MAX, + || { + reset_to = true; + }, + ) .value_fn(|ui, _| { let response = time_boundary_ui( ui, @@ -185,7 +190,7 @@ impl UiQueryKind { // let current_entity = match self { - Self::TimeRange { pov_entity, .. } => all_entities + Self::Range { pov_entity, .. } => all_entities .contains(pov_entity) .then(|| pov_entity.clone()), Self::LatestAt { .. } => None, @@ -226,7 +231,7 @@ impl UiQueryKind { .unwrap_or_default(); let current_component = match self { - Self::TimeRange { pov_component, .. } => Some(*pov_component), + Self::Range { pov_component, .. } => Some(*pov_component), Self::LatestAt { .. } => None, }; @@ -282,7 +287,7 @@ impl UiQueryKind { ); if changed { - *self = Self::TimeRange { + *self = Self::Range { pov_entity, pov_component, from, @@ -297,44 +302,6 @@ impl UiQueryKind { } } -impl From for UiQueryKind { - fn from(value: QueryKind) -> Self { - match value { - QueryKind::LatestAt { time } => Self::LatestAt { time }, - QueryKind::Range { - pov_entity, - pov_component, - from, - to, - } => Self::TimeRange { - pov_entity, - pov_component, - from, - to, - }, - } - } -} - -impl From for QueryKind { - fn from(value: UiQueryKind) -> Self { - match value { - UiQueryKind::LatestAt { time } => Self::LatestAt { time }, - UiQueryKind::TimeRange { - pov_entity, - pov_component, - from, - to, - } => Self::Range { - pov_entity, - pov_component, - from, - to, - }, - } - } -} - fn time_boundary_ui( ui: &mut egui::Ui, time_drag_value: &TimeDragValue, @@ -344,15 +311,17 @@ fn time_boundary_ui( time: &mut TimeInt, ) -> egui::Response { if *time == TimeInt::MAX { - let response = ui.button("+∞"); - if response.dragged() { + let mut response = ui.button("+∞").on_hover_text("Click to edit"); + if response.clicked() { *time = time_drag_value.max_time(); + response.mark_changed(); } response } else if *time == TimeInt::MIN { - let response = ui.button("-∞"); - if response.dragged() { + let mut response = ui.button("–∞").on_hover_text("Click to edit"); + if response.clicked() { *time = time_drag_value.min_time(); + response.mark_changed(); } response } else { diff --git a/crates/viewer/re_space_view_dataframe/src/space_view_class.rs b/crates/viewer/re_space_view_dataframe/src/space_view_class.rs index 010aa5eea610..0454eb7591d4 100644 --- a/crates/viewer/re_space_view_dataframe/src/space_view_class.rs +++ b/crates/viewer/re_space_view_dataframe/src/space_view_class.rs @@ -13,10 +13,8 @@ use re_viewer_context::{ use re_viewport_blueprint::ViewProperty; use crate::{ - latest_at_table::latest_at_table_ui, - time_range_table::time_range_table_ui, - view_query::{Query, QueryKind}, - visualizer_system::EmptySystem, + latest_at_table::latest_at_table_ui, query_kind::QueryKind, + time_range_table::time_range_table_ui, view_query::Query, visualizer_system::EmptySystem, }; #[derive(Default)] @@ -148,7 +146,7 @@ mode sets the default time range to _everything_. You can override this in the s from, to, } => { - //TODO: use pov entity and component + //TODO(#7279): use pov entity and component let time_range_table_order = ViewProperty::from_archetype::( ctx.blueprint_db(), diff --git a/crates/viewer/re_space_view_dataframe/src/view_query.rs b/crates/viewer/re_space_view_dataframe/src/view_query.rs index 4b46c886f29f..943b5e9b3f9d 100644 --- a/crates/viewer/re_space_view_dataframe/src/view_query.rs +++ b/crates/viewer/re_space_view_dataframe/src/view_query.rs @@ -1,4 +1,6 @@ -use re_log_types::{EntityPath, TimeInt, TimelineName}; +use std::collections::BTreeSet; + +use re_log_types::{TimeInt, TimelineName}; use re_types::blueprint::{archetypes, components, datatypes}; use re_types_core::{ComponentName, Loggable as _}; use re_ui::UiExt as _; @@ -6,26 +8,10 @@ use re_viewer_context::{ SpaceViewId, SpaceViewState, SpaceViewSystemExecutionError, TimeDragValue, ViewerContext, }; use re_viewport_blueprint::ViewProperty; -use std::collections::BTreeSet; -use crate::query_kind_ui::UiQueryKind; +use crate::query_kind::QueryKind; use crate::visualizer_system::EmptySystem; -/// The query kind for the dataframe view. -#[derive(Debug, Clone)] -pub(crate) enum QueryKind { - LatestAt { - time: TimeInt, - }, - Range { - pov_entity: EntityPath, - pov_component: ComponentName, - from: TimeInt, - to: TimeInt, - }, - //TODO(#7067): add selected components -} - /// Helper for handling the dataframe view query blueprint. pub(crate) enum Query { FollowTimeline, @@ -245,6 +231,7 @@ fn override_ui( space_view_id: SpaceViewId, property: &ViewProperty<'_>, ) -> Result<(), SpaceViewSystemExecutionError> { + ui.add_space(4.0); egui::Grid::new("dataframe_view_query_ui") .num_columns(2) .spacing(egui::vec2(8.0, 10.0)) @@ -266,6 +253,7 @@ fn override_ui( &EmptySystem {}, ); }); + ui.add_space(4.0); let timeline = property .component_or_empty::()? @@ -280,7 +268,7 @@ fn override_ui( let timeline_name = timeline.name(); let query = Query::try_from_blueprint(ctx, space_view_id)?; - let mut ui_query_kind: UiQueryKind = query.kind(ctx).into(); + let mut query_kind = query.kind(ctx); let time_drag_value = if let Some(times) = ctx.recording().time_histogram(&timeline) { TimeDragValue::from_time_histogram(times) } else { @@ -305,9 +293,9 @@ fn override_ui( true }); - let changed = ui_query_kind.ui(ctx, ui, &time_drag_value, &timeline, &all_entities); + let changed = query_kind.ui(ctx, ui, &time_drag_value, &timeline, &all_entities); if changed { - Query::save_kind_for_timeline(ctx, space_view_id, timeline_name, &ui_query_kind.into())?; + Query::save_kind_for_timeline(ctx, space_view_id, timeline_name, &query_kind)?; } Ok(()) From 3578c8e56fbaece3d31d458c5585cbeba186bd86 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 3 Sep 2024 10:09:55 +0200 Subject: [PATCH 7/8] Toml fix --- crates/viewer/re_space_view_dataframe/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/viewer/re_space_view_dataframe/Cargo.toml b/crates/viewer/re_space_view_dataframe/Cargo.toml index 0375b5e44a15..d8154b8bf3d8 100644 --- a/crates/viewer/re_space_view_dataframe/Cargo.toml +++ b/crates/viewer/re_space_view_dataframe/Cargo.toml @@ -36,4 +36,3 @@ re_viewport_blueprint.workspace = true egui_extras.workspace = true egui.workspace = true itertools.workspace = true - From 1767cf0408cf646ad060c3c79a3289bf1395267f Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Wed, 4 Sep 2024 16:07:56 +0200 Subject: [PATCH 8/8] Addressed review comments --- .../rerun/blueprint/datatypes/time_range_query.fbs | 6 ++++++ .../src/blueprint/datatypes/time_range_query.rs | 6 ++++++ .../viewer/re_space_view_dataframe/src/query_kind.rs | 2 ++ .../rerun/blueprint/datatypes/time_range_query.hpp | 6 ++++++ .../rerun/blueprint/datatypes/time_range_query.py | 12 ++++++++++++ 5 files changed, 32 insertions(+) diff --git a/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs b/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs index 05ea88800b84..96ce8024fc90 100644 --- a/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs +++ b/crates/store/re_types/definitions/rerun/blueprint/datatypes/time_range_query.fbs @@ -11,9 +11,15 @@ table TimeRangeQuery ( timeline: rerun.datatypes.Utf8 (order: 100); /// Point-of-view entity. + /// + /// Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + /// in the results returned by the range query. pov_entity: rerun.datatypes.EntityPath (order: 200); /// Point-of-view component. + /// + /// Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + /// in the results returned by the range query. pov_component: rerun.datatypes.Utf8 (order: 300); /// Beginning of the time range. diff --git a/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs b/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs index a12876e9ec9c..f1e2c85e55ed 100644 --- a/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs +++ b/crates/store/re_types/src/blueprint/datatypes/time_range_query.rs @@ -25,9 +25,15 @@ pub struct TimeRangeQuery { pub timeline: crate::datatypes::Utf8, /// Point-of-view entity. + /// + /// Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + /// in the results returned by the range query. pub pov_entity: crate::datatypes::EntityPath, /// Point-of-view component. + /// + /// Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + /// in the results returned by the range query. pub pov_component: crate::datatypes::Utf8, /// Beginning of the time range. diff --git a/crates/viewer/re_space_view_dataframe/src/query_kind.rs b/crates/viewer/re_space_view_dataframe/src/query_kind.rs index 91d69e12ab69..adbda6b50062 100644 --- a/crates/viewer/re_space_view_dataframe/src/query_kind.rs +++ b/crates/viewer/re_space_view_dataframe/src/query_kind.rs @@ -22,6 +22,8 @@ pub(crate) enum QueryKind { impl QueryKind { /// Show the UI for the query kind selector. + /// + /// Return `true` if the query kind was updated (and thus should be saved to blueprint). pub(crate) fn ui( &mut self, ctx: &ViewerContext<'_>, diff --git a/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp b/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp index e3d7c067051c..9930d75945fd 100644 --- a/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp +++ b/rerun_cpp/src/rerun/blueprint/datatypes/time_range_query.hpp @@ -24,9 +24,15 @@ namespace rerun::blueprint::datatypes { rerun::datatypes::Utf8 timeline; /// Point-of-view entity. + /// + /// Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + /// in the results returned by the range query. rerun::datatypes::EntityPath pov_entity; /// Point-of-view component. + /// + /// Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + /// in the results returned by the range query. rerun::datatypes::Utf8 pov_component; /// Beginning of the time range. diff --git a/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py b/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py index 1008d83adcac..15becf69f775 100644 --- a/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py +++ b/rerun_py/rerun_sdk/rerun/blueprint/datatypes/time_range_query.py @@ -70,8 +70,14 @@ def __init__( Name of the timeline this applies to. pov_entity: Point-of-view entity. + + Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + in the results returned by the range query. pov_component: Point-of-view component. + + Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + in the results returned by the range query. start: Beginning of the time range. end: @@ -90,11 +96,17 @@ def __init__( pov_entity: datatypes.EntityPath = field(converter=_time_range_query__pov_entity__special_field_converter_override) # Point-of-view entity. # + # Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + # in the results returned by the range query. + # # (Docstring intentionally commented out to hide this field from the docs) pov_component: datatypes.Utf8 = field(converter=_time_range_query__pov_component__special_field_converter_override) # Point-of-view component. # + # Each non-null value of the point-of-view column (as defined by an entity and a component name) will generate a row + # in the results returned by the range query. + # # (Docstring intentionally commented out to hide this field from the docs) start: datatypes.TimeInt = field(