From 79edef98e0e5b8855395f625909ca849369903fe Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 7 Jun 2024 18:32:02 -0400 Subject: [PATCH 1/5] New UI for setting defaults --- crates/re_selection_panel/src/defaults_ui.rs | 241 ++++++++++++++++++ crates/re_selection_panel/src/lib.rs | 1 + .../re_selection_panel/src/selection_panel.rs | 26 +- 3 files changed, 260 insertions(+), 8 deletions(-) create mode 100644 crates/re_selection_panel/src/defaults_ui.rs diff --git a/crates/re_selection_panel/src/defaults_ui.rs b/crates/re_selection_panel/src/defaults_ui.rs new file mode 100644 index 000000000000..5e7be83b9045 --- /dev/null +++ b/crates/re_selection_panel/src/defaults_ui.rs @@ -0,0 +1,241 @@ +use std::collections::{BTreeMap, BTreeSet}; + +use itertools::Itertools; + +use re_data_store::LatestAtQuery; +use re_log_types::{DataCell, DataRow, EntityPath, RowId}; +use re_types_core::ComponentName; +use re_ui::UiExt as _; +use re_viewer_context::{ + blueprint_timeline, ComponentUiTypes, QueryContext, SystemCommand, SystemCommandSender as _, + ViewContext, ViewSystemIdentifier, +}; +use re_viewport_blueprint::SpaceViewBlueprint; + +pub fn defaults_ui(ctx: &ViewContext<'_>, space_view: &SpaceViewBlueprint, ui: &mut egui::Ui) { + let db = ctx.viewer_ctx.blueprint_db(); + let query = ctx.viewer_ctx.blueprint_query; + let resolver = Default::default(); + + // Cleared components should act as unset, so we filter out everything that's empty, + // even if they are listed in `all_components`. + let active_defaults = ctx + .blueprint_db() + .store() + .all_components(&blueprint_timeline(), &space_view.defaults_path) + .unwrap_or_default() + .into_iter() + .filter(|c| { + db.query_caches() + .latest_at(db.store(), query, &space_view.defaults_path, [*c]) + .components + .get(c) + .and_then(|data| data.resolved(&resolver).ok()) + .map_or(false, |data| !data.is_empty()) + }) + .collect::>(); + + // It only makes sense to set defaults for components that are used by a system in the view. + let mut component_to_vis: BTreeMap = Default::default(); + + // Accumulate the components across all visualizers and track which visualizer + // each component came from so we can use it for fallbacks later. + // + // If two visualizers have the same component, the first one wins. + // TODO(jleibs): We can do something fancier in the future such as presenting both + // options once we have a motivating use-case. + for (id, vis) in ctx.visualizer_collection.iter_with_identifiers() { + for component in vis.visualizer_query_info().queried { + component_to_vis.entry(component).or_insert_with(|| id); + } + } + + add_new_default( + ctx, + query, + ui, + &component_to_vis, + &active_defaults, + &space_view.defaults_path, + ); + + let sorted_overrides = active_defaults.iter().sorted(); + + let query_context = QueryContext { + viewer_ctx: ctx.viewer_ctx, + target_entity_path: &space_view.defaults_path, + archetype_name: None, + query, + view_state: ctx.view_state, + view_ctx: Some(ctx), + }; + + re_ui::list_item::list_item_scope(ui, "defaults", |ui| { + ui.spacing_mut().item_spacing.y = 0.0; + for component_name in sorted_overrides { + let Some(visualizer_identifier) = component_to_vis.get(component_name) else { + continue; + }; + let Ok(visualizer) = ctx + .visualizer_collection + .get_by_identifier(*visualizer_identifier) + else { + re_log::warn!( + "Failed to resolve visualizer identifier {visualizer_identifier}, to a visualizer implementation" + ); + continue; + }; + + // TODO(jleibs): We're already doing this query above as part of the filter. This is kind of silly to do it again. + // Change the structure to avoid this. + let component_data = db + .query_caches() + .latest_at( + db.store(), + query, + &space_view.defaults_path, + [*component_name], + ) + .components + .get(component_name) + .cloned(); /* arc */ + + if let Some(component_data) = component_data { + let value_fn = |ui: &mut egui::Ui| { + ctx.viewer_ctx.component_ui_registry.singleline_edit_ui( + &query_context, + ui, + db, + &space_view.defaults_path, + *component_name, + &component_data, + visualizer.as_fallback_provider(), + ); + }; + + ui.list_item() + .interactive(false) + .show_flat( + ui, + re_ui::list_item::PropertyContent::new(component_name.short_name()) + .min_desired_width(150.0) + .action_button(&re_ui::icons::CLOSE, || { + ctx.save_empty_blueprint_component_by_name( + &space_view.defaults_path, + *component_name, + ); + }) + .value_fn(|ui, _| value_fn(ui)), + ) + .on_hover_text(component_name.full_name()); + } + } + }); +} + +#[allow(clippy::too_many_arguments)] +pub fn add_new_default( + ctx: &ViewContext<'_>, + query: &LatestAtQuery, + ui: &mut egui::Ui, + component_to_vis: &BTreeMap, + active_overrides: &BTreeSet, + defaults_path: &EntityPath, +) { + let remaining_components = component_to_vis + .keys() + .filter(|c| !active_overrides.contains(*c)) + .collect::>(); + + let enabled = !remaining_components.is_empty(); + + ui.add_enabled_ui(enabled, |ui| { + let mut opened = false; + let menu = ui + .menu_button("Add", |ui| { + opened = true; + ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); + + let query_context = QueryContext { + viewer_ctx: ctx.viewer_ctx, + target_entity_path: defaults_path, + archetype_name: None, + query, + view_state: ctx.view_state, + view_ctx: Some(ctx), + }; + + // Present the option to add new components for each component that doesn't + // already have an active override. + for (component, viz) in component_to_vis { + if active_overrides.contains(component) { + continue; + } + + // If there is no registered editor, don't let the user create an override + // TODO(andreas): Can only handle single line editors right now. + if !ctx + .viewer_ctx + .component_ui_registry + .registered_ui_types(*component) + .contains(ComponentUiTypes::SingleLineEditor) + { + continue; + } + + if ui.button(component.short_name()).clicked() { + // We are creating a new override. We need to decide what initial value to give it. + // - First see if there's an existing splat in the recording. + // - Next see if visualizer system wants to provide a value. + // - Finally, fall back on the default value from the component registry. + + // TODO(jleibs): Is this the right place for fallbacks to come from? + let Some(mut initial_data) = ctx + .visualizer_collection + .get_by_identifier(*viz) + .ok() + .and_then(|sys| { + sys.fallback_for(&query_context, *component) + .map(|fallback| DataCell::from_arrow(*component, fallback)) + .ok() + }) + else { + re_log::warn!("Could not identify an initial value for: {}", component); + return; + }; + + initial_data.compute_size_bytes(); + + match DataRow::from_cells( + RowId::new(), + ctx.blueprint_timepoint_for_writes(), + defaults_path.clone(), + [initial_data], + ) { + Ok(row) => { + ctx.viewer_ctx.command_sender.send_system( + SystemCommand::UpdateBlueprint( + ctx.blueprint_db().store_id().clone(), + vec![row], + ), + ); + } + Err(err) => { + re_log::warn!( + "Failed to create DataRow for blueprint component: {}", + err + ); + } + } + + ui.close_menu(); + } + } + }) + .response + .on_disabled_hover_text("No additional components available."); + if !opened { + menu.on_hover_text("Choose a component to specify an override value.".to_owned()); + } + }); +} diff --git a/crates/re_selection_panel/src/lib.rs b/crates/re_selection_panel/src/lib.rs index 3375d15ed792..b6906336f32b 100644 --- a/crates/re_selection_panel/src/lib.rs +++ b/crates/re_selection_panel/src/lib.rs @@ -1,5 +1,6 @@ //! The UI for the selection panel. +mod defaults_ui; mod override_ui; mod query_range_ui; mod selection_history_ui; diff --git a/crates/re_selection_panel/src/selection_panel.rs b/crates/re_selection_panel/src/selection_panel.rs index e39169908605..866e32cbe616 100644 --- a/crates/re_selection_panel/src/selection_panel.rs +++ b/crates/re_selection_panel/src/selection_panel.rs @@ -30,8 +30,11 @@ use re_viewport_blueprint::{ ui::show_add_space_view_or_container_modal, SpaceViewBlueprint, ViewportBlueprint, }; -use crate::override_ui::{override_ui, override_visualizer_ui}; use crate::space_view_entity_picker::SpaceViewEntityPicker; +use crate::{ + defaults_ui::defaults_ui, + override_ui::{override_ui, override_visualizer_ui}, +}; use crate::{ query_range_ui::query_range_ui_data_result, query_range_ui::query_range_ui_space_view, selection_history_ui::SelectionHistoryUi, @@ -237,13 +240,13 @@ impl SelectionPanel { blueprint: &ViewportBlueprint, view_states: &mut ViewStates, ui: &mut Ui, - space_view_id: SpaceViewId, + view_id: SpaceViewId, ) { - if let Some(space_view) = blueprint.space_view(&space_view_id) { + if let Some(space_view) = blueprint.space_view(&view_id) { if let Some(new_entity_path_filter) = self.entity_path_filter_ui( ctx, ui, - space_view_id, + view_id, &space_view.contents.entity_path_filter, &space_view.space_origin, ) { @@ -262,7 +265,7 @@ impl SelectionPanel { ) .clicked() { - if let Some(new_space_view_id) = blueprint.duplicate_space_view(&space_view_id, ctx) { + if let Some(new_space_view_id) = blueprint.duplicate_space_view(&view_id, ctx) { ctx.selection_state() .set_selection(Item::SpaceView(new_space_view_id)); blueprint.mark_user_interaction(ctx); @@ -273,10 +276,10 @@ impl SelectionPanel { ui.full_span_separator(); ui.add_space(ui.spacing().item_spacing.y / 2.0); - if let Some(space_view) = blueprint.space_view(&space_view_id) { + if let Some(space_view) = blueprint.space_view(&view_id) { let class_identifier = space_view.class_identifier(); - let space_view_state = view_states.get_mut( + let view_state = view_states.get_mut( ctx.space_view_class_registry, space_view.id, class_identifier, @@ -293,7 +296,7 @@ impl SelectionPanel { if let Err(err) = space_view_class.selection_ui( ctx, ui, - space_view_state.view_state.as_mut(), + view_state.view_state.as_mut(), &space_view.space_origin, space_view.id, &mut props, @@ -308,6 +311,13 @@ impl SelectionPanel { if props_before != props { space_view.save_legacy_properties(ctx, props); } + + let view_ctx = + space_view.bundle_context_with_state(ctx, view_state.view_state.as_ref()); + + ui.large_collapsing_header("Component Defaults", true, |ui| { + defaults_ui(&view_ctx, space_view, ui); + }); } } From e62e5df26975a1763045d7d49edd4f2ac926babb Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Mon, 10 Jun 2024 20:13:56 -0400 Subject: [PATCH 2/5] Add default-handling to get_instance_with_fallback --- crates/re_space_view/src/query.rs | 14 +++++++++++++- crates/re_space_view/src/results_ext.rs | 11 ++++++++++- .../src/space_view/view_context.rs | 1 + crates/re_viewport_blueprint/src/space_view.rs | 14 ++++++++++++-- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/crates/re_space_view/src/query.rs b/crates/re_space_view/src/query.rs index f7ad00de8670..4a4fce6a9a3b 100644 --- a/crates/re_space_view/src/query.rs +++ b/crates/re_space_view/src/query.rs @@ -69,12 +69,24 @@ pub fn latest_at_with_overrides<'a>( ctx.viewer_ctx.recording_store(), latest_at_query, &data_result.entity_path, - component_set, + component_set.iter().copied(), + ); + + // TODO(jleibs): This doesn't work when the component set contains empty results. + // This means we over-query for defaults that will never be used. + // component_set.retain(|component| !results.components.contains_key(component)); + + let defaults = ctx.viewer_ctx.blueprint_db().query_caches().latest_at( + ctx.viewer_ctx.store_context.blueprint.store(), + ctx.viewer_ctx.blueprint_query, + &ctx.defaults_path, + component_set.iter().copied(), ); HybridLatestAtResults { overrides, results, + defaults, ctx, query: latest_at_query.clone(), data_result, diff --git a/crates/re_space_view/src/results_ext.rs b/crates/re_space_view/src/results_ext.rs index fa7302b45c6b..8499391eec88 100644 --- a/crates/re_space_view/src/results_ext.rs +++ b/crates/re_space_view/src/results_ext.rs @@ -18,6 +18,7 @@ use crate::DataResultQuery as _; pub struct HybridLatestAtResults<'a> { pub(crate) overrides: LatestAtResults, pub(crate) results: LatestAtResults, + pub(crate) defaults: LatestAtResults, pub ctx: &'a ViewContext<'a>, pub query: LatestAtQuery, pub data_result: &'a DataResult, @@ -116,7 +117,10 @@ impl<'a> HybridLatestAtResults<'a> { self.get_instance(0) } - /// Utility for retrieving a single instance of a component with fallback + /// Utility for retrieving a single instance of a component with fallback. + /// + /// If the space view specifies a default, that will be used first, otherwise + /// the fallback provider will be queried. #[inline] pub fn get_instance_with_fallback( &self, @@ -124,6 +128,11 @@ impl<'a> HybridLatestAtResults<'a> { ) -> T { self.get(T::name()) .and_then(|r| r.try_instance::(&self.resolver, index)) + .or_else(|| { + self.defaults + .get(T::name()) + .and_then(|r| r.try_instance::(&self.resolver, 0)) + }) .or_else(|| { self.try_fallback_raw(T::name()) .and_then(|raw| T::from_arrow(raw.as_ref()).ok()) diff --git a/crates/re_viewer_context/src/space_view/view_context.rs b/crates/re_viewer_context/src/space_view/view_context.rs index 0b99118c5915..c009a6727b2d 100644 --- a/crates/re_viewer_context/src/space_view/view_context.rs +++ b/crates/re_viewer_context/src/space_view/view_context.rs @@ -17,6 +17,7 @@ pub struct ViewContext<'a> { pub viewer_ctx: &'a crate::ViewerContext<'a>, pub view_id: SpaceViewId, pub view_state: &'a dyn crate::SpaceViewState, + pub defaults_path: &'a EntityPath, pub visualizer_collection: Arc, } diff --git a/crates/re_viewport_blueprint/src/space_view.rs b/crates/re_viewport_blueprint/src/space_view.rs index d59c3c1c2dd9..3f1639d3336c 100644 --- a/crates/re_viewport_blueprint/src/space_view.rs +++ b/crates/re_viewport_blueprint/src/space_view.rs @@ -51,6 +51,9 @@ pub struct SpaceViewBlueprint { /// True if this space view is visible in the UI. pub visible: bool, + /// Path where these space_views defaults can be found. + pub defaults_path: EntityPath, + /// Pending blueprint writes for nested components from duplicate. pending_writes: Vec, } @@ -73,6 +76,7 @@ impl SpaceViewBlueprint { space_origin: recommended.origin, contents: SpaceViewContents::new(id, space_view_class, recommended.query_filter), visible: true, + defaults_path: id.as_entity_path().join(&"defaults".into()), pending_writes: Default::default(), } } @@ -165,6 +169,7 @@ impl SpaceViewBlueprint { &space_env, ); let visible = visible.map_or(true, |v| v.0); + let defaults_path = id.as_entity_path().join(&"defaults".into()); Some(Self { id, @@ -173,6 +178,7 @@ impl SpaceViewBlueprint { space_origin, contents: content, visible, + defaults_path, pending_writes: Default::default(), }) } @@ -193,6 +199,7 @@ impl SpaceViewBlueprint { space_origin, contents, visible, + defaults_path: _, pending_writes, } = self; @@ -287,6 +294,7 @@ impl SpaceViewBlueprint { space_origin: self.space_origin.clone(), contents, visible: self.visible, + defaults_path: self.defaults_path.clone(), pending_writes, } } @@ -466,7 +474,7 @@ impl SpaceViewBlueprint { } pub fn bundle_context_with_states<'a>( - &self, + &'a self, ctx: &'a ViewerContext<'a>, view_states: &'a mut ViewStates, ) -> ViewContext<'a> { @@ -483,12 +491,13 @@ impl SpaceViewBlueprint { viewer_ctx: ctx, view_id: self.id, view_state, + defaults_path: &self.defaults_path, visualizer_collection: self.visualizer_collection(ctx), } } pub fn bundle_context_with_state<'a>( - &self, + &'a self, ctx: &'a ViewerContext<'a>, view_state: &'a dyn SpaceViewState, ) -> ViewContext<'a> { @@ -496,6 +505,7 @@ impl SpaceViewBlueprint { viewer_ctx: ctx, view_id: self.id, view_state, + defaults_path: &self.defaults_path, visualizer_collection: self.visualizer_collection(ctx), } } From a98a3013c0512b80f2e065b744b36d5cdce0864b Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Mon, 10 Jun 2024 21:03:20 -0400 Subject: [PATCH 3/5] Add default lookups for HybridResults --- crates/re_query/src/range/results.rs | 13 ++++ crates/re_space_view/src/query.rs | 25 ++++++-- crates/re_space_view/src/results_ext.rs | 60 ++++++++++++++++++- .../src/visualizers/entity_iterator.rs | 2 +- .../src/line_visualizer_system.rs | 2 +- .../src/point_visualizer_system.rs | 2 +- 6 files changed, 94 insertions(+), 10 deletions(-) diff --git a/crates/re_query/src/range/results.rs b/crates/re_query/src/range/results.rs index 3e32ef1acf6f..7641cd819b5e 100644 --- a/crates/re_query/src/range/results.rs +++ b/crates/re_query/src/range/results.rs @@ -339,6 +339,19 @@ impl<'a, T: 'static> RangeData<'a, T> { start_index..end_index } + + pub fn is_empty(&self) -> bool { + if let Some(indices) = self.indices.as_ref() { + indices.is_empty() + } else if let Some(data) = self.data.as_ref() { + match data { + Data::Owned(data) => data.dyn_num_values() == 0, + Data::Cached(data) => data.num_values() == 0, + } + } else { + true + } + } } impl RangeComponentResults { diff --git a/crates/re_space_view/src/query.rs b/crates/re_space_view/src/query.rs index 4a4fce6a9a3b..e14a616f2d8d 100644 --- a/crates/re_space_view/src/query.rs +++ b/crates/re_space_view/src/query.rs @@ -17,7 +17,7 @@ use crate::results_ext::{HybridLatestAtResults, HybridRangeResults}; /// Data should be accessed via the [`crate::RangeResultsExt`] trait which is implemented for /// [`crate::HybridResults`]. pub fn range_with_overrides( - ctx: &ViewerContext<'_>, + ctx: &ViewContext<'_>, _annotations: Option<&re_viewer_context::Annotations>, range_query: &RangeQuery, data_result: &re_viewer_context::DataResult, @@ -27,7 +27,7 @@ pub fn range_with_overrides( let mut component_set = component_names.into_iter().collect::>(); - let overrides = query_overrides(ctx, data_result, component_set.iter()); + let overrides = query_overrides(ctx.viewer_ctx, data_result, component_set.iter()); // No need to query for components that have overrides. component_set.retain(|component| !overrides.components.contains_key(component)); @@ -36,10 +36,25 @@ pub fn range_with_overrides( ctx.recording_store(), range_query, &data_result.entity_path, - component_set, + component_set.iter().copied(), ); - HybridRangeResults { overrides, results } + // TODO(jleibs): This doesn't work when the component set contains empty results. + // This means we over-query for defaults that will never be used. + // component_set.retain(|component| !results.components.contains_key(component)); + + let defaults = ctx.viewer_ctx.blueprint_db().query_caches().latest_at( + ctx.viewer_ctx.store_context.blueprint.store(), + ctx.viewer_ctx.blueprint_query, + ctx.defaults_path, + component_set.iter().copied(), + ); + + HybridRangeResults { + overrides, + results, + defaults, + } } /// Queries for the given `component_names` using latest-at semantics with override support. @@ -79,7 +94,7 @@ pub fn latest_at_with_overrides<'a>( let defaults = ctx.viewer_ctx.blueprint_db().query_caches().latest_at( ctx.viewer_ctx.store_context.blueprint.store(), ctx.viewer_ctx.blueprint_query, - &ctx.defaults_path, + ctx.defaults_path, component_set.iter().copied(), ); diff --git a/crates/re_space_view/src/results_ext.rs b/crates/re_space_view/src/results_ext.rs index 8499391eec88..6a400964abe8 100644 --- a/crates/re_space_view/src/results_ext.rs +++ b/crates/re_space_view/src/results_ext.rs @@ -33,6 +33,7 @@ pub struct HybridLatestAtResults<'a> { pub struct HybridRangeResults { pub(crate) overrides: LatestAtResults, pub(crate) results: RangeResults, + pub(crate) defaults: LatestAtResults, } impl<'a> HybridLatestAtResults<'a> { @@ -362,7 +363,34 @@ impl RangeResultsExt for HybridRangeResults { Ok(data) } else { - self.results.get_or_empty_dense(resolver) + let data = self.results.get_or_empty_dense(resolver); + + // If the data is not empty, return it. + if let Ok(data) = data { + if !data.is_empty() { + return Ok(data); + } + }; + + // Otherwise try to use the default data. + + let results = self.defaults.get_or_empty(C::name()); + // Because this is an default from the blueprint we always re-index the data as static + let data = + RangeData::from_latest_at(resolver, results, Some((TimeInt::STATIC, RowId::ZERO))); + + // TODO(#5607): what should happen if the promise is still pending? + let (front_status, back_status) = data.status(); + match front_status { + PromiseResult::Error(err) => return Err(re_query::QueryError::Other(err.into())), + PromiseResult::Pending | PromiseResult::Ready(_) => {} + } + match back_status { + PromiseResult::Error(err) => return Err(re_query::QueryError::Other(err.into())), + PromiseResult::Pending | PromiseResult::Ready(_) => {} + } + + Ok(data) } } } @@ -411,6 +439,7 @@ impl<'a> RangeResultsExt for HybridLatestAtResults<'a> { if self.overrides.contains(component_name) { let results = self.overrides.get_or_empty(C::name()); + // Because this is an override we always re-index the data as static let data = RangeData::from_latest_at(resolver, results, Some((TimeInt::STATIC, RowId::ZERO))); @@ -428,7 +457,34 @@ impl<'a> RangeResultsExt for HybridLatestAtResults<'a> { Ok(data) } else { - self.results.get_or_empty_dense(resolver) + let data = self.results.get_or_empty_dense(resolver); + + // If the data is not empty, return it. + if let Ok(data) = data { + if !data.is_empty() { + return Ok(data); + } + }; + + // Otherwise try to use the default data. + + let results = self.defaults.get_or_empty(C::name()); + // Because this is an default from the blueprint we always re-index the data as static + let data = + RangeData::from_latest_at(resolver, results, Some((TimeInt::STATIC, RowId::ZERO))); + + // TODO(#5607): what should happen if the promise is still pending? + let (front_status, back_status) = data.status(); + match front_status { + PromiseResult::Error(err) => return Err(re_query::QueryError::Other(err.into())), + PromiseResult::Pending | PromiseResult::Ready(_) => {} + } + match back_status { + PromiseResult::Error(err) => return Err(re_query::QueryError::Other(err.into())), + PromiseResult::Pending | PromiseResult::Ready(_) => {} + } + + Ok(data) } } } diff --git a/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs b/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs index 0a2fdd057b70..d83f350faf29 100644 --- a/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs +++ b/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs @@ -56,7 +56,7 @@ pub fn query_archetype_with_history<'a, A: Archetype>( ), ); let results = range_with_overrides( - ctx.viewer_ctx, + ctx, None, &range_query, data_result, diff --git a/crates/re_space_view_time_series/src/line_visualizer_system.rs b/crates/re_space_view_time_series/src/line_visualizer_system.rs index b70d0ee96b3b..3da56980be6a 100644 --- a/crates/re_space_view_time_series/src/line_visualizer_system.rs +++ b/crates/re_space_view_time_series/src/line_visualizer_system.rs @@ -193,7 +193,7 @@ impl SeriesLineSystem { let query = re_data_store::RangeQuery::new(view_query.timeline, time_range); let results = range_with_overrides( - ctx.viewer_ctx, + ctx, None, &query, data_result, diff --git a/crates/re_space_view_time_series/src/point_visualizer_system.rs b/crates/re_space_view_time_series/src/point_visualizer_system.rs index c6060f797559..a8593bad79df 100644 --- a/crates/re_space_view_time_series/src/point_visualizer_system.rs +++ b/crates/re_space_view_time_series/src/point_visualizer_system.rs @@ -148,7 +148,7 @@ impl SeriesPointSystem { let query = re_data_store::RangeQuery::new(view_query.timeline, time_range); let results = range_with_overrides( - ctx.viewer_ctx, + ctx, None, &query, data_result, From 59ed23ad438d8ee05f866d79eecb975bced37d17 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Mon, 10 Jun 2024 21:33:21 -0400 Subject: [PATCH 4/5] Fix is_empty check --- crates/re_query/src/range/results.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/re_query/src/range/results.rs b/crates/re_query/src/range/results.rs index 7641cd819b5e..0f0c86a8216d 100644 --- a/crates/re_query/src/range/results.rs +++ b/crates/re_query/src/range/results.rs @@ -341,9 +341,7 @@ impl<'a, T: 'static> RangeData<'a, T> { } pub fn is_empty(&self) -> bool { - if let Some(indices) = self.indices.as_ref() { - indices.is_empty() - } else if let Some(data) = self.data.as_ref() { + if let Some(data) = self.data.as_ref() { match data { Data::Owned(data) => data.dyn_num_values() == 0, Data::Cached(data) => data.num_values() == 0, From c8293794b929eac9353c5b1821ecccea4f2e35da Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Tue, 11 Jun 2024 11:49:25 -0400 Subject: [PATCH 5/5] Names and comments --- crates/re_space_view/src/lib.rs | 4 ++- crates/re_space_view/src/query.rs | 26 ++++++++++++------- .../src/visualizers/entity_iterator.rs | 8 +++--- .../src/line_visualizer_system.rs | 4 +-- .../src/point_visualizer_system.rs | 4 +-- 5 files changed, 29 insertions(+), 17 deletions(-) diff --git a/crates/re_space_view/src/lib.rs b/crates/re_space_view/src/lib.rs index 96310303fea0..11b5d7fbbec5 100644 --- a/crates/re_space_view/src/lib.rs +++ b/crates/re_space_view/src/lib.rs @@ -11,7 +11,9 @@ mod screenshot; mod view_property_ui; pub use heuristics::suggest_space_view_for_each_entity; -pub use query::{latest_at_with_overrides, range_with_overrides, DataResultQuery}; +pub use query::{ + latest_at_with_blueprint_resolved_data, range_with_blueprint_resolved_data, DataResultQuery, +}; pub use results_ext::{HybridResults, RangeResultsExt}; pub use screenshot::ScreenshotMode; pub use view_property_ui::view_property_ui; diff --git a/crates/re_space_view/src/query.rs b/crates/re_space_view/src/query.rs index e14a616f2d8d..65b7a5e165d1 100644 --- a/crates/re_space_view/src/query.rs +++ b/crates/re_space_view/src/query.rs @@ -9,14 +9,18 @@ use crate::results_ext::{HybridLatestAtResults, HybridRangeResults}; // --- -/// Queries for the given `component_names` using range semantics with override support. +/// Queries for the given `component_names` using range semantics with blueprint support. /// -/// If the `DataResult` contains a specified override from the blueprint, that values -/// will be used instead of the range query. +/// Data will be resolved, in order of priority: +/// - Data overrides from the blueprint +/// - Data from the recording +/// - Default data from the blueprint +/// - Fallback from the visualizer +/// - Placeholder from the component. /// /// Data should be accessed via the [`crate::RangeResultsExt`] trait which is implemented for /// [`crate::HybridResults`]. -pub fn range_with_overrides( +pub fn range_with_blueprint_resolved_data( ctx: &ViewContext<'_>, _annotations: Option<&re_viewer_context::Annotations>, range_query: &RangeQuery, @@ -57,14 +61,18 @@ pub fn range_with_overrides( } } -/// Queries for the given `component_names` using latest-at semantics with override support. +/// Queries for the given `component_names` using latest-at semantics with blueprint support. /// -/// If the `DataResult` contains a specified override from the blueprint, that values -/// will be used instead of the latest-at query. +/// Data will be resolved, in order of priority: +/// - Data overrides from the blueprint +/// - Data from the recording +/// - Default data from the blueprint +/// - Fallback from the visualizer +/// - Placeholder from the component. /// /// Data should be accessed via the [`crate::RangeResultsExt`] trait which is implemented for /// [`crate::HybridResults`]. -pub fn latest_at_with_overrides<'a>( +pub fn latest_at_with_blueprint_resolved_data<'a>( ctx: &'a ViewContext<'a>, _annotations: Option<&'a re_viewer_context::Annotations>, latest_at_query: &LatestAtQuery, @@ -183,7 +191,7 @@ impl DataResultQuery for DataResult { ctx: &'a ViewContext<'a>, latest_at_query: &'a LatestAtQuery, ) -> HybridLatestAtResults<'a> { - latest_at_with_overrides( + latest_at_with_blueprint_resolved_data( ctx, None, latest_at_query, diff --git a/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs b/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs index d83f350faf29..acc89521d612 100644 --- a/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs +++ b/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs @@ -3,7 +3,9 @@ use re_data_store::{LatestAtQuery, RangeQuery}; use re_entity_db::EntityProperties; use re_log_types::{EntityPath, TimeInt, Timeline}; use re_renderer::DepthOffset; -use re_space_view::{latest_at_with_overrides, range_with_overrides, HybridResults}; +use re_space_view::{ + latest_at_with_blueprint_resolved_data, range_with_blueprint_resolved_data, HybridResults, +}; use re_types::Archetype; use re_viewer_context::{ IdentifiedViewSystem, QueryRange, SpaceViewClass, SpaceViewSystemExecutionError, ViewContext, @@ -55,7 +57,7 @@ pub fn query_archetype_with_history<'a, A: Archetype>( timeline_cursor, ), ); - let results = range_with_overrides( + let results = range_with_blueprint_resolved_data( ctx, None, &range_query, @@ -66,7 +68,7 @@ pub fn query_archetype_with_history<'a, A: Archetype>( } QueryRange::LatestAt => { let latest_query = LatestAtQuery::new(*timeline, timeline_cursor); - let results = latest_at_with_overrides( + let results = latest_at_with_blueprint_resolved_data( ctx, None, &latest_query, diff --git a/crates/re_space_view_time_series/src/line_visualizer_system.rs b/crates/re_space_view_time_series/src/line_visualizer_system.rs index 3da56980be6a..766d2c617615 100644 --- a/crates/re_space_view_time_series/src/line_visualizer_system.rs +++ b/crates/re_space_view_time_series/src/line_visualizer_system.rs @@ -1,6 +1,6 @@ use itertools::Itertools as _; use re_query::{PromiseResult, QueryError}; -use re_space_view::range_with_overrides; +use re_space_view::range_with_blueprint_resolved_data; use re_types::archetypes; use re_types::{ archetypes::SeriesLine, @@ -192,7 +192,7 @@ impl SeriesLineSystem { let entity_path = &data_result.entity_path; let query = re_data_store::RangeQuery::new(view_query.timeline, time_range); - let results = range_with_overrides( + let results = range_with_blueprint_resolved_data( ctx, None, &query, diff --git a/crates/re_space_view_time_series/src/point_visualizer_system.rs b/crates/re_space_view_time_series/src/point_visualizer_system.rs index a8593bad79df..0b380f223e13 100644 --- a/crates/re_space_view_time_series/src/point_visualizer_system.rs +++ b/crates/re_space_view_time_series/src/point_visualizer_system.rs @@ -1,7 +1,7 @@ use itertools::Itertools as _; use re_query::{PromiseResult, QueryError}; -use re_space_view::range_with_overrides; +use re_space_view::range_with_blueprint_resolved_data; use re_types::{ archetypes::{self, SeriesPoint}, components::{Color, MarkerShape, MarkerSize, Name, Scalar}, @@ -147,7 +147,7 @@ impl SeriesPointSystem { let entity_path = &data_result.entity_path; let query = re_data_store::RangeQuery::new(view_query.timeline, time_range); - let results = range_with_overrides( + let results = range_with_blueprint_resolved_data( ctx, None, &query,