diff --git a/crates/re_entity_db/src/entity_properties.rs b/crates/re_entity_db/src/entity_properties.rs index 4e7e0d1e2d38..02f6e29325a1 100644 --- a/crates/re_entity_db/src/entity_properties.rs +++ b/crates/re_entity_db/src/entity_properties.rs @@ -94,7 +94,6 @@ impl FromIterator<(EntityPath, EntityProperties)> for EntityPropertyMap { #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] #[cfg_attr(feature = "serde", serde(default))] pub struct EntityProperties { - pub visible: bool, pub visible_history: re_query::ExtraQueryHistory, pub interactive: bool, @@ -144,7 +143,6 @@ pub struct EntityProperties { impl Default for EntityProperties { fn default() -> Self { Self { - visible: true, visible_history: re_query::ExtraQueryHistory::default(), interactive: true, color_mapper: EditableAutoValue::default(), @@ -166,7 +164,6 @@ impl EntityProperties { /// Multiply/and these together. pub fn with_child(&self, child: &Self) -> Self { Self { - visible: self.visible && child.visible, visible_history: self.visible_history.with_child(&child.visible_history), interactive: self.interactive && child.interactive, @@ -211,7 +208,6 @@ impl EntityProperties { /// loaded from the Blueprint store where the Auto values are not up-to-date. pub fn merge_with(&self, other: &Self) -> Self { Self { - visible: other.visible, visible_history: self.visible_history.with_child(&other.visible_history), interactive: other.interactive, @@ -250,7 +246,6 @@ impl EntityProperties { /// Determine whether this `EntityProperty` has user-edits relative to another `EntityProperty` pub fn has_edits(&self, other: &Self) -> bool { let Self { - visible, visible_history, interactive, color_mapper, @@ -265,8 +260,7 @@ impl EntityProperties { time_series_aggregator, } = self; - visible != &other.visible - || visible_history != &other.visible_history + visible_history != &other.visible_history || interactive != &other.interactive || color_mapper.has_edits(&other.color_mapper) || pinhole_image_plane_distance.has_edits(&other.pinhole_image_plane_distance) diff --git a/crates/re_space_view/src/data_query.rs b/crates/re_space_view/src/data_query.rs index ea9771817509..7cf3fcb141fd 100644 --- a/crates/re_space_view/src/data_query.rs +++ b/crates/re_space_view/src/data_query.rs @@ -1,9 +1,10 @@ -use ahash::HashMap; +use nohash_hasher::IntMap; use re_entity_db::{external::re_data_store::LatestAtQuery, EntityProperties, EntityPropertyMap}; -use re_log_types::{EntityPath, StoreKind}; use re_types::ComponentName; -use re_viewer_context::{DataQueryResult, PerVisualizer, StoreContext, VisualizableEntities}; +use re_viewer_context::{ + DataQueryResult, OverridePath, PerVisualizer, StoreContext, VisualizableEntities, +}; pub struct EntityOverrideContext { pub root: EntityProperties, @@ -11,7 +12,7 @@ pub struct EntityOverrideContext { pub recursive: EntityPropertyMap, /// Base component overrides that are inherited by all entities. - pub root_component_overrides: HashMap, + pub root_component_overrides: IntMap, } /// Trait for resolving properties needed by most implementations of [`DataQuery`] diff --git a/crates/re_space_view/src/space_view.rs b/crates/re_space_view/src/space_view.rs index c30be095628e..821f62597b96 100644 --- a/crates/re_space_view/src/space_view.rs +++ b/crates/re_space_view/src/space_view.rs @@ -446,7 +446,7 @@ impl SpaceViewBlueprint { accumulated_properties, individual_properties, recursive_properties: Default::default(), - component_overrides: Default::default(), + resolved_component_overrides: Default::default(), recursive_override_path: entity_path.clone(), individual_override_path: entity_path, }), @@ -464,8 +464,8 @@ mod tests { }; use re_types::{archetypes::Points3D, ComponentBatch, ComponentName, Loggable as _}; use re_viewer_context::{ - blueprint_timeline, IndicatedEntities, PerVisualizer, SpaceViewClassRegistry, StoreContext, - VisualizableEntities, + blueprint_timeline, IndicatedEntities, OverridePath, PerVisualizer, SpaceViewClassRegistry, + StoreContext, VisualizableEntities, }; use std::collections::HashMap; @@ -582,9 +582,9 @@ mod tests { ); } - // Now, override visibility on parent individually. + // Now, override interactive on parent individually. let mut overrides = parent.individual_properties().cloned().unwrap_or_default(); - overrides.visible = false; + overrides.interactive = false; save_override( overrides, @@ -593,7 +593,7 @@ mod tests { ); } - // Parent is not visible, but children are + // Parent is not interactive, but children are { let ctx = StoreContext { app_id: re_log_types::ApplicationId::unknown(), @@ -622,18 +622,18 @@ mod tests { .lookup_result_by_path(&EntityPath::from("parent/skip/child2")) .unwrap(); - assert!(!parent.accumulated_properties().visible); + assert!(!parent.accumulated_properties().interactive); for result in [child1, child2] { - assert!(result.accumulated_properties().visible); + assert!(result.accumulated_properties().interactive); } - // Override visibility on parent recursively. + // Override interactivity on parent recursively. let mut overrides = parent_group .individual_properties() .cloned() .unwrap_or_default(); - overrides.visible = false; + overrides.interactive = false; save_override( overrides, @@ -642,7 +642,7 @@ mod tests { ); } - // Nobody is visible + // Nobody is interactive { let ctx = StoreContext { app_id: re_log_types::ApplicationId::unknown(), @@ -668,11 +668,11 @@ mod tests { .unwrap(); for result in [parent, child1, child2] { - assert!(!result.accumulated_properties().visible); + assert!(!result.accumulated_properties().interactive); } } - // Override visible range on root + // Override interactive range on root { let root = space_view.root_data_result( &StoreContext { @@ -694,7 +694,7 @@ mod tests { ); } - // Everyone has visible history + // Everyone has interactive history { let ctx = StoreContext { app_id: re_log_types::ApplicationId::unknown(), @@ -736,7 +736,7 @@ mod tests { ); } - // Child2 has its own visible history + // Child2 has its own interactive history { let ctx = StoreContext { app_id: re_log_types::ApplicationId::unknown(), @@ -1042,13 +1042,13 @@ mod tests { query_result.tree.visit(&mut |node| { let result = &node.data_result; if let Some(property_overrides) = &result.property_overrides { - if !property_overrides.component_overrides.is_empty() { + if !property_overrides.resolved_component_overrides.is_empty() { visited.insert( result.entity_path.clone(), property_overrides - .component_overrides + .resolved_component_overrides .iter() - .map(|(component_name, (store_kind, path))| { + .map(|(component_name, OverridePath { store_kind, path })| { assert_eq!(store_kind, &StoreKind::Blueprint); (*component_name, path.clone()) }) diff --git a/crates/re_space_view/src/space_view_contents.rs b/crates/re_space_view/src/space_view_contents.rs index 773a59e71517..c999d337e276 100644 --- a/crates/re_space_view/src/space_view_contents.rs +++ b/crates/re_space_view/src/space_view_contents.rs @@ -1,4 +1,4 @@ -use ahash::HashMap; +use nohash_hasher::IntMap; use slotmap::SlotMap; use smallvec::SmallVec; @@ -6,7 +6,7 @@ use re_entity_db::{ external::re_data_store::LatestAtQuery, EntityDb, EntityProperties, EntityPropertiesComponent, EntityPropertyMap, EntityTree, }; -use re_log_types::{path::RuleEffect, EntityPath, EntityPathFilter, EntityPathRule, StoreKind}; +use re_log_types::{path::RuleEffect, EntityPath, EntityPathFilter, EntityPathRule}; use re_types::{ blueprint::{archetypes as blueprint_archetypes, components::QueryExpression}, Archetype as _, @@ -14,8 +14,8 @@ use re_types::{ use re_types_core::{components::VisualizerOverrides, ComponentName}; use re_viewer_context::{ DataQueryResult, DataResult, DataResultHandle, DataResultNode, DataResultTree, - IndicatedEntities, PerVisualizer, PropertyOverrides, SpaceViewClassIdentifier, SpaceViewId, - StoreContext, ViewerContext, VisualizableEntities, + IndicatedEntities, OverridePath, PerVisualizer, PropertyOverrides, SpaceViewClassIdentifier, + SpaceViewId, StoreContext, ViewerContext, VisualizableEntities, }; use crate::{ @@ -340,7 +340,7 @@ impl DataQueryPropertyResolver<'_> { .space_view .root_data_result(ctx, query) .property_overrides - .map(|p| (p.accumulated_properties, p.component_overrides)) + .map(|p| (p.accumulated_properties, p.resolved_component_overrides)) .unwrap_or_default(); for prefix in &self.default_stack { @@ -416,7 +416,7 @@ impl DataQueryPropertyResolver<'_> { query_result: &mut DataQueryResult, override_context: &EntityOverrideContext, accumulated: &EntityProperties, - recursive_property_overrides: &HashMap, + recursive_property_overrides: &IntMap, handle: DataResultHandle, ) { if let Some((child_handles, accumulated, recursive_property_overrides)) = @@ -493,7 +493,7 @@ impl DataQueryPropertyResolver<'_> { if !component_data.is_empty() { recursive_property_overrides.to_mut().insert( *component, - (StoreKind::Blueprint, recursive_override_path.clone()), + OverridePath::blueprint_path(recursive_override_path.clone()), ); } } @@ -502,7 +502,7 @@ impl DataQueryPropertyResolver<'_> { // Then, gather individual overrides - these may override the recursive ones again, // but recursive overrides are still inherited to children. - let mut component_overrides = (*recursive_property_overrides).clone(); + let mut resolved_component_overrides = (*recursive_property_overrides).clone(); if let Some(individual_override_subtree) = ctx.blueprint.tree().subtree(&individual_override_path) { @@ -514,9 +514,9 @@ impl DataQueryPropertyResolver<'_> { .and_then(|(_, _, cells)| cells[0].clone()) { if !component_data.is_empty() { - component_overrides.insert( + resolved_component_overrides.insert( *component, - (StoreKind::Blueprint, individual_override_path.clone()), + OverridePath::blueprint_path(individual_override_path.clone()), ); } } @@ -527,7 +527,7 @@ impl DataQueryPropertyResolver<'_> { accumulated_properties, individual_properties: individual_properties.cloned(), recursive_properties: recursive_properties.cloned(), - component_overrides, + resolved_component_overrides, recursive_override_path, individual_override_path, }); diff --git a/crates/re_space_view_bar_chart/src/visualizer_system.rs b/crates/re_space_view_bar_chart/src/visualizer_system.rs index 834a29dd3c9e..ef93f0024656 100644 --- a/crates/re_space_view_bar_chart/src/visualizer_system.rs +++ b/crates/re_space_view_bar_chart/src/visualizer_system.rs @@ -50,7 +50,7 @@ impl VisualizerSystem for BarChartVisualizerSystem { let store = ctx.entity_db.store(); - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { let query = LatestAtQuery::new(query.timeline, query.latest_at); let tensor = store.query_latest_component::( &data_result.entity_path, diff --git a/crates/re_space_view_dataframe/src/space_view_class.rs b/crates/re_space_view_dataframe/src/space_view_class.rs index 99e1c0b75fb3..82454150c679 100644 --- a/crates/re_space_view_dataframe/src/space_view_class.rs +++ b/crates/re_space_view_dataframe/src/space_view_class.rs @@ -81,7 +81,7 @@ impl SpaceViewClass for DataframeSpaceView { // These are the entity paths whose content we must display. let sorted_entity_paths: BTreeSet<_> = query .iter_all_data_results() - .filter(|data_result| data_result.accumulated_properties().visible) + .filter(|data_result| data_result.is_visible(ctx)) .map(|data_result| &data_result.entity_path) .cloned() .collect(); diff --git a/crates/re_space_view_spatial/src/contexts/depth_offsets.rs b/crates/re_space_view_spatial/src/contexts/depth_offsets.rs index b70714c07252..736192b302dd 100644 --- a/crates/re_space_view_spatial/src/contexts/depth_offsets.rs +++ b/crates/re_space_view_spatial/src/contexts/depth_offsets.rs @@ -48,7 +48,7 @@ impl ViewContextSystem for EntityDepthOffsets { // Use a BTreeSet for entity hashes to get a stable order. let mut entities_per_draw_order = BTreeMap::>::new(); - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { if let Some(draw_order) = store .query_latest_component::(&data_result.entity_path, &ctx.current_query()) { diff --git a/crates/re_space_view_spatial/src/visualizers/cameras.rs b/crates/re_space_view_spatial/src/visualizers/cameras.rs index 002ab4949852..f9e79bc524b9 100644 --- a/crates/re_space_view_spatial/src/visualizers/cameras.rs +++ b/crates/re_space_view_spatial/src/visualizers/cameras.rs @@ -213,7 +213,7 @@ impl VisualizerSystem for CamerasVisualizer { let mut line_builder = re_renderer::LineDrawableBuilder::new(ctx.render_ctx); line_builder.radius_boost_in_ui_points_for_outlines(SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES); - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { let time_query = re_data_store::LatestAtQuery::new(query.timeline, query.latest_at); if let Some(pinhole) = query_pinhole(store, &time_query, &data_result.entity_path) { 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 4f407245832b..986c40c7db18 100644 --- a/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs +++ b/crates/re_space_view_spatial/src/visualizers/entity_iterator.rs @@ -42,7 +42,7 @@ where let annotations = view_ctx.get::()?; let counter = view_ctx.get::()?; - for data_result in query.iter_visible_data_results(System::identifier()) { + for data_result in query.iter_visible_data_results(ctx, System::identifier()) { // The transform that considers pinholes only makes sense if this is a 3D space-view let world_from_entity = if view_ctx.space_view_class_identifier() == SpatialSpaceView3D::identifier() { @@ -151,7 +151,7 @@ macro_rules! impl_process_archetype { let annotations = view_ctx.get::()?; let counter = view_ctx.get::()?; - for data_result in query.iter_visible_data_results(S::identifier()) { + for data_result in query.iter_visible_data_results(ctx, S::identifier()) { // The transform that considers pinholes only makes sense if this is a 3D space-view let world_from_entity = if view_ctx.space_view_class_identifier() == SpatialSpaceView3D::identifier() { transforms.reference_from_entity(&data_result.entity_path) @@ -256,7 +256,7 @@ pub fn count_instances_in_archetype_views< let mut num_instances = 0; - for data_result in query.iter_visible_data_results(System::identifier()) { + for data_result in query.iter_visible_data_results(ctx, System::identifier()) { match query_archetype_with_history::( ctx.entity_db.store(), &query.timeline, diff --git a/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs b/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs index be5bc096f315..bcb11e63061c 100644 --- a/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs +++ b/crates/re_space_view_spatial/src/visualizers/transform3d_arrows.rs @@ -61,7 +61,7 @@ impl VisualizerSystem for Transform3DArrowsVisualizer { let mut line_builder = re_renderer::LineDrawableBuilder::new(ctx.render_ctx); line_builder.radius_boost_in_ui_points_for_outlines(SIZE_BOOST_IN_POINTS_FOR_LINE_OUTLINES); - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { if !*data_result.accumulated_properties().transform_3d_visible { continue; } diff --git a/crates/re_space_view_tensor/src/visualizer_system.rs b/crates/re_space_view_tensor/src/visualizer_system.rs index e690df2e84d1..55559fc1797b 100644 --- a/crates/re_space_view_tensor/src/visualizer_system.rs +++ b/crates/re_space_view_tensor/src/visualizer_system.rs @@ -48,7 +48,7 @@ impl VisualizerSystem for TensorSystem { re_tracing::profile_function!(); let store = ctx.entity_db.store(); - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { let timeline_query = LatestAtQuery::new(query.timeline, query.latest_at); if let Some(tensor) = store diff --git a/crates/re_space_view_text_document/src/visualizer_system.rs b/crates/re_space_view_text_document/src/visualizer_system.rs index f121ff40fcab..b0f55e9cf04f 100644 --- a/crates/re_space_view_text_document/src/visualizer_system.rs +++ b/crates/re_space_view_text_document/src/visualizer_system.rs @@ -44,7 +44,7 @@ impl VisualizerSystem for TextDocumentSystem { let timeline_query = LatestAtQuery::new(query.timeline, query.latest_at); - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { // TODO(#3320): this match can go away once the issue is resolved match query_archetype::( store, diff --git a/crates/re_space_view_text_log/src/visualizer_system.rs b/crates/re_space_view_text_log/src/visualizer_system.rs index 42fe24acf1ee..192d051668f6 100644 --- a/crates/re_space_view_text_log/src/visualizer_system.rs +++ b/crates/re_space_view_text_log/src/visualizer_system.rs @@ -53,7 +53,7 @@ impl VisualizerSystem for TextLogSystem { let query_caches = ctx.entity_db.query_caches(); let store = ctx.entity_db.store(); - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { re_tracing::profile_scope!("primary", &data_result.entity_path.to_string()); // We want everything, for all times: diff --git a/crates/re_space_view_time_series/src/legacy_visualizer_system.rs b/crates/re_space_view_time_series/src/legacy_visualizer_system.rs index 4d4b6d65b85a..6d71a991ed03 100644 --- a/crates/re_space_view_time_series/src/legacy_visualizer_system.rs +++ b/crates/re_space_view_time_series/src/legacy_visualizer_system.rs @@ -10,7 +10,7 @@ use re_viewer_context::{ }; use crate::{ - overrides::{initial_override_color, lookup_override}, + overrides::initial_override_color, util::{determine_plot_bounds_and_time_per_pixel, determine_time_range, points_to_series}, PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind, ScatterAttrs, }; @@ -57,7 +57,7 @@ impl VisualizerSystem for LegacyTimeSeriesSystem { ctx, &query.latest_at_query(), query - .iter_visible_data_results(Self::identifier()) + .iter_visible_data_results(ctx, Self::identifier()) .map(|data| &data.entity_path), ); @@ -102,7 +102,7 @@ impl LegacyTimeSeriesSystem { let (plot_bounds, time_per_pixel) = determine_plot_bounds_and_time_per_pixel(ctx, query); // TODO(cmc): this should be thread-pooled in case there are a gazillon series in the same plot… - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { let annotations = self.annotation_map.find(&data_result.entity_path); let annotation_info = annotations .resolved_class_description(None) @@ -111,11 +111,14 @@ impl LegacyTimeSeriesSystem { const DEFAULT_RADIUS: f32 = 0.75; - let override_color = lookup_override::(data_result, ctx).map(|c| c.to_array()); - let override_label = lookup_override::(data_result, ctx).map(|t| t.0); - let override_scattered = - lookup_override::(data_result, ctx).map(|s| s.0); - let override_radius = lookup_override::(data_result, ctx).map(|r| r.0); + let override_color = data_result + .lookup_override::(ctx) + .map(|c| c.to_array()); + let override_label = data_result.lookup_override::(ctx).map(|t| t.0); + let override_scattered = data_result + .lookup_override::(ctx) + .map(|s| s.0); + let override_radius = data_result.lookup_override::(ctx).map(|r| r.0); // All the default values for a `PlotPoint`, accounting for both overrides and default // values. 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 92b1dba52471..5672f84770f5 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 @@ -15,7 +15,7 @@ use crate::overrides::initial_override_color; use crate::util::{ determine_plot_bounds_and_time_per_pixel, determine_time_range, points_to_series, }; -use crate::{overrides::lookup_override, PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind}; +use crate::{PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind}; /// The system for rendering [`SeriesLine`] archetypes. #[derive(Default, Debug)] @@ -56,7 +56,7 @@ impl VisualizerSystem for SeriesLineSystem { ctx, &query.latest_at_query(), query - .iter_visible_data_results(Self::identifier()) + .iter_visible_data_results(ctx, Self::identifier()) .map(|data| &data.entity_path), ); @@ -98,7 +98,7 @@ impl SeriesLineSystem { let (plot_bounds, time_per_pixel) = determine_plot_bounds_and_time_per_pixel(ctx, query); - let data_results = query.iter_visible_data_results(Self::identifier()); + let data_results = query.iter_visible_data_results(ctx, Self::identifier()); let parallel_loading = false; // TODO(emilk): enable parallel loading when it is faster, because right now it is often slower. if parallel_loading { @@ -162,9 +162,11 @@ fn load_series( .resolved_class_description(None) .annotation_info(); let default_color = DefaultColor::EntityPath(&data_result.entity_path); - let override_color = lookup_override::(data_result, ctx).map(|c| c.to_array()); - let override_series_name = lookup_override::(data_result, ctx).map(|t| t.0); - let override_stroke_width = lookup_override::(data_result, ctx).map(|r| r.0); + let override_color = data_result + .lookup_override::(ctx) + .map(|c| c.to_array()); + let override_series_name = data_result.lookup_override::(ctx).map(|t| t.0); + let override_stroke_width = data_result.lookup_override::(ctx).map(|r| r.0); // All the default values for a `PlotPoint`, accounting for both overrides and default // values. diff --git a/crates/re_space_view_time_series/src/overrides.rs b/crates/re_space_view_time_series/src/overrides.rs index 397d764915f5..d2f0afc9e8fd 100644 --- a/crates/re_space_view_time_series/src/overrides.rs +++ b/crates/re_space_view_time_series/src/overrides.rs @@ -1,28 +1,6 @@ -use re_log_types::{EntityPath, StoreKind}; -use re_types::{components::Color, Component}; -use re_viewer_context::{DefaultColor, ResolvedAnnotationInfo, ViewerContext}; - -pub fn lookup_override( - data_result: &re_viewer_context::DataResult, - ctx: &ViewerContext<'_>, -) -> Option { - data_result - .property_overrides - .as_ref() - .and_then(|p| p.component_overrides.get(&C::name())) - .and_then(|(store_kind, path)| match store_kind { - StoreKind::Blueprint => ctx - .store_context - .blueprint - .store() - .query_latest_component::(path, ctx.blueprint_query), - StoreKind::Recording => ctx - .entity_db - .store() - .query_latest_component::(path, &ctx.current_query()), - }) - .map(|c| c.value) -} +use re_log_types::EntityPath; +use re_types::components::Color; +use re_viewer_context::{DefaultColor, ResolvedAnnotationInfo}; pub fn initial_override_color(entity_path: &EntityPath) -> Color { let default_color = DefaultColor::EntityPath(entity_path); 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 90ee4ce60e9d..f0b612798773 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 @@ -16,7 +16,7 @@ use crate::util::{ determine_plot_bounds_and_time_per_pixel, determine_time_range, points_to_series, }; use crate::ScatterAttrs; -use crate::{overrides::lookup_override, PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind}; +use crate::{PlotPoint, PlotPointAttrs, PlotSeries, PlotSeriesKind}; /// The system for rendering [`SeriesPoint`] archetypes. #[derive(Default, Debug)] @@ -60,7 +60,7 @@ impl VisualizerSystem for SeriesPointSystem { ctx, &query.latest_at_query(), query - .iter_visible_data_results(Self::identifier()) + .iter_visible_data_results(ctx, Self::identifier()) .map(|data| &data.entity_path), ); @@ -106,17 +106,19 @@ impl SeriesPointSystem { let (plot_bounds, time_per_pixel) = determine_plot_bounds_and_time_per_pixel(ctx, query); // TODO(cmc): this should be thread-pooled in case there are a gazillon series in the same plot… - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { let annotations = self.annotation_map.find(&data_result.entity_path); let annotation_info = annotations .resolved_class_description(None) .annotation_info(); let default_color = DefaultColor::EntityPath(&data_result.entity_path); - let override_color = lookup_override::(data_result, ctx).map(|c| c.to_array()); - let override_series_name = lookup_override::(data_result, ctx).map(|t| t.0); - let override_marker_size = lookup_override::(data_result, ctx).map(|r| r.0); - let override_marker = lookup_override::(data_result, ctx); + let override_color = data_result + .lookup_override::(ctx) + .map(|c| c.to_array()); + let override_series_name = data_result.lookup_override::(ctx).map(|t| t.0); + let override_marker_size = data_result.lookup_override::(ctx).map(|r| r.0); + let override_marker = data_result.lookup_override::(ctx); // All the default values for a `PlotPoint`, accounting for both overrides and default // values. diff --git a/crates/re_types/definitions/rerun/blueprint/components/visible.fbs b/crates/re_types/definitions/rerun/blueprint/components/visible.fbs index 992d11f53f6b..ee11f7651d5d 100644 --- a/crates/re_types/definitions/rerun/blueprint/components/visible.fbs +++ b/crates/re_types/definitions/rerun/blueprint/components/visible.fbs @@ -14,7 +14,7 @@ struct Visible ( "attr.arrow.transparent", "attr.rerun.scope": "blueprint", "attr.python.aliases": "bool", - "attr.rust.derive": "Copy, Default, PartialEq, Eq, PartialOrd, Ord", + "attr.rust.derive": "Copy, PartialEq, Eq, PartialOrd, Ord", "attr.rust.repr": "transparent", "attr.rust.tuple_struct" ) { diff --git a/crates/re_types/src/blueprint/components/mod.rs b/crates/re_types/src/blueprint/components/mod.rs index 554e1ae416dc..0b873668d40b 100644 --- a/crates/re_types/src/blueprint/components/mod.rs +++ b/crates/re_types/src/blueprint/components/mod.rs @@ -14,6 +14,7 @@ mod space_view_class; mod space_view_origin; mod viewer_recommendation_hash; mod visible; +mod visible_ext; pub use self::active_tab::ActiveTab; pub use self::background3d_kind::Background3DKind; diff --git a/crates/re_types/src/blueprint/components/visible.rs b/crates/re_types/src/blueprint/components/visible.rs index 7c671219a95e..20887b57cb2a 100644 --- a/crates/re_types/src/blueprint/components/visible.rs +++ b/crates/re_types/src/blueprint/components/visible.rs @@ -22,7 +22,7 @@ use ::re_types_core::{ComponentBatch, MaybeOwnedComponentBatch}; use ::re_types_core::{DeserializationError, DeserializationResult}; /// **Component**: Whether the container, space view, entity or instance is currently visible. -#[derive(Clone, Debug, Copy, Default, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, Debug, Copy, PartialEq, Eq, PartialOrd, Ord)] #[repr(transparent)] pub struct Visible(pub bool); diff --git a/crates/re_types/src/blueprint/components/visible_ext.rs b/crates/re_types/src/blueprint/components/visible_ext.rs new file mode 100644 index 000000000000..777ab528f2c0 --- /dev/null +++ b/crates/re_types/src/blueprint/components/visible_ext.rs @@ -0,0 +1,7 @@ +use super::Visible; + +impl Default for Visible { + fn default() -> Self { + Self(true) + } +} diff --git a/crates/re_ui/src/lib.rs b/crates/re_ui/src/lib.rs index 55b8d179050f..18cd4abf257d 100644 --- a/crates/re_ui/src/lib.rs +++ b/crates/re_ui/src/lib.rs @@ -65,7 +65,7 @@ pub enum LabelStyle { use crate::list_item::ListItem; use egui::emath::{Rangef, Rot2}; use egui::epaint::util::FloatOrd; -use egui::{pos2, Align2, CollapsingResponse, Color32, Mesh, NumExt, Rect, Shape, Vec2}; +use egui::{pos2, Align2, CollapsingResponse, Color32, Mesh, NumExt, Rect, Shape, Vec2, Widget}; #[derive(Clone)] pub struct ReUi { @@ -431,6 +431,17 @@ impl ReUi { ui: &mut egui::Ui, selected: &mut bool, text: impl Into, + ) -> egui::Response { + self.checkbox_indeterminate(ui, selected, text, false) + } + + #[allow(clippy::unused_self)] + pub fn checkbox_indeterminate( + &self, + ui: &mut egui::Ui, + selected: &mut bool, + text: impl Into, + indeterminate: bool, ) -> egui::Response { ui.scope(|ui| { ui.visuals_mut().widgets.hovered.expansion = 0.0; @@ -438,7 +449,9 @@ impl ReUi { ui.visuals_mut().widgets.open.expansion = 0.0; // NOLINT - ui.checkbox(selected, text) + egui::Checkbox::new(selected, text) + .indeterminate(indeterminate) + .ui(ui) }) .inner } diff --git a/crates/re_viewer/src/ui/override_ui.rs b/crates/re_viewer/src/ui/override_ui.rs index 3b3f2b711035..ff055faf55b4 100644 --- a/crates/re_viewer/src/ui/override_ui.rs +++ b/crates/re_viewer/src/ui/override_ui.rs @@ -13,8 +13,8 @@ use re_types_core::{ ComponentName, }; use re_viewer_context::{ - blueprint_timepoint_for_writes, DataResult, SystemCommand, SystemCommandSender as _, - UiVerbosity, ViewSystemIdentifier, ViewerContext, + blueprint_timepoint_for_writes, DataResult, OverridePath, SystemCommand, + SystemCommandSender as _, UiVerbosity, ViewSystemIdentifier, ViewerContext, }; pub fn override_ui( @@ -48,7 +48,7 @@ pub fn override_ui( let active_overrides: BTreeSet = data_result .property_overrides .as_ref() - .map(|props| props.component_overrides.keys().cloned().collect()) + .map(|props| props.resolved_component_overrides.keys().cloned().collect()) .unwrap_or_default(); let view_systems = ctx @@ -95,7 +95,7 @@ pub fn override_ui( }; let components: Vec<_> = overrides - .component_overrides + .resolved_component_overrides .into_iter() .sorted_by_key(|(c, _)| *c) .filter(|(c, _)| component_to_vis.contains_key(c) && is_component_visible_in_ui(c)) @@ -111,7 +111,7 @@ pub fn override_ui( re_ui::ReUi::setup_table_body(&mut body); let row_height = re_ui::ReUi::table_line_height(); body.rows(row_height, components.len(), |mut row| { - if let Some((component_name, (store_kind, entity_path))) = + if let Some((component_name, OverridePath { store_kind, path })) = components.get(row.index()) { // Remove button @@ -124,7 +124,6 @@ pub fn override_ui( // Note: need to use the blueprint store since the data might // not exist in the recording store. ctx.save_empty_blueprint_component_name( - ctx.store_context.blueprint.store(), &overrides.individual_override_path, *component_name, ); @@ -143,19 +142,11 @@ pub fn override_ui( StoreKind::Blueprint => { let store = ctx.store_context.blueprint.store(); let query = ctx.blueprint_query; - get_component_with_instances( - store, - query, - entity_path, - *component_name, - ) + get_component_with_instances(store, query, path, *component_name) + } + StoreKind::Recording => { + get_component_with_instances(store, &query, path, *component_name) } - StoreKind::Recording => get_component_with_instances( - store, - &query, - entity_path, - *component_name, - ), }; if let Some((_, _, component_data)) = component_data { @@ -165,7 +156,7 @@ pub fn override_ui( UiVerbosity::Small, &query, store, - entity_path, + path, &overrides.individual_override_path, &component_data, instance_key, diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index ac19c50af1c8..8151e04222bd 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -17,8 +17,8 @@ use re_types::{ use re_ui::list_item::ListItem; use re_ui::{ReUi, SyntaxHighlighting as _}; use re_viewer_context::{ - gpu_bridge::colormap_dropdown_button_ui, ContainerId, HoverHighlight, Item, SpaceViewClass, - SpaceViewClassIdentifier, SpaceViewId, UiVerbosity, ViewerContext, + gpu_bridge::colormap_dropdown_button_ui, ContainerId, DataQueryResult, HoverHighlight, Item, + SpaceViewClass, SpaceViewClassIdentifier, SpaceViewId, UiVerbosity, ViewerContext, }; use re_viewport::{ context_menu_ui_for_item, icon_for_container_kind, space_view_name_style, Contents, @@ -911,8 +911,9 @@ fn blueprint_ui_for_data_result( entity_props_ui( ctx, ui, + ctx.lookup_query_result(*space_view_id), &space_view_class, - Some(entity_path), + entity_path, &mut props, data_result.accumulated_properties(), ); @@ -1068,13 +1069,45 @@ The last rule matching `/world/house` is `+ /world/**`, so it is included. fn entity_props_ui( ctx: &ViewerContext<'_>, ui: &mut egui::Ui, + query_result: &DataQueryResult, space_view_class: &SpaceViewClassIdentifier, - entity_path: Option<&EntityPath>, + entity_path: &EntityPath, entity_props: &mut EntityProperties, resolved_entity_props: &EntityProperties, ) { + use re_types::blueprint::components::Visible; + use re_types::Loggable as _; + let re_ui = ctx.re_ui; - re_ui.checkbox(ui, &mut entity_props.visible, "Visible"); + let Some(data_result) = query_result.tree.lookup_result_by_path(entity_path) else { + return; + }; + + { + let visible_before = data_result.lookup_override_or_default::(ctx); + let mut visible = visible_before; + + let override_source = + data_result.component_override_source(&query_result.tree, &Visible::name()); + let is_inherited = + override_source.is_some() && override_source.as_ref() != Some(entity_path); + + ui.horizontal(|ui| { + re_ui.checkbox(ui, &mut visible.0, "Visible"); + if is_inherited { + ui.label("(inherited)"); + } + }); + + if visible_before != visible { + data_result.save_recursive_override_or_clear_if_redundant( + ctx, + &query_result.tree, + &visible, + ); + } + } + re_ui .checkbox(ui, &mut entity_props.interactive, "Interactive") .on_hover_text("If disabled, the entity will not react to any mouse interaction"); @@ -1084,7 +1117,7 @@ fn entity_props_ui( ui, space_view_class, false, - entity_path, + Some(entity_path), &mut entity_props.visible_history, &resolved_entity_props.visible_history, ); @@ -1094,11 +1127,9 @@ fn entity_props_ui( .show(ui, |ui| { // TODO(wumpf): It would be nice to only show pinhole & depth properties in the context of a 3D view. // if *view_state.state_spatial.nav_mode.get() == SpatialNavigationMode::ThreeD { - if let Some(entity_path) = entity_path { - pinhole_props_ui(ctx, ui, entity_path, entity_props); - depth_props_ui(ctx, ui, entity_path, entity_props); - transform3d_visualization_ui(ctx, ui, entity_path, entity_props); - } + pinhole_props_ui(ctx, ui, entity_path, entity_props); + depth_props_ui(ctx, ui, entity_path, entity_props); + transform3d_visualization_ui(ctx, ui, entity_path, entity_props); }); } diff --git a/crates/re_viewer_context/src/blueprint_helpers.rs b/crates/re_viewer_context/src/blueprint_helpers.rs index 4af906b6bad1..a23caa52f9d4 100644 --- a/crates/re_viewer_context/src/blueprint_helpers.rs +++ b/crates/re_viewer_context/src/blueprint_helpers.rs @@ -59,6 +59,13 @@ impl ViewerContext<'_> { let num_instances = components.num_instances() as u32; let timepoint = blueprint_timepoint_for_writes(); + re_log::trace!( + "Writing {} components of type {:?} to {:?}", + num_instances, + components.name(), + entity_path + ); + let data_row_result = if num_instances == 1 { let mut splat_cell: DataCell = [InstanceKey::SPLAT].into(); splat_cell.compute_size_bytes(); @@ -105,11 +112,11 @@ impl ViewerContext<'_> { /// Helper to save a component to the blueprint store. pub fn save_empty_blueprint_component_name( &self, - store: &re_data_store::DataStore, entity_path: &EntityPath, component_name: ComponentName, ) { - let Some(datatype) = store.lookup_datatype(&component_name) else { + let blueprint = &self.store_context.blueprint; + let Some(datatype) = blueprint.store().lookup_datatype(&component_name) else { re_log::error_once!( "Tried to clear a component with unknown type: {}", component_name @@ -130,7 +137,7 @@ impl ViewerContext<'_> { Ok(row) => self .command_sender .send_system(SystemCommand::UpdateBlueprint( - self.store_context.blueprint.store_id().clone(), + blueprint.store_id().clone(), vec![row], )), Err(err) => { diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index 868c5d2fa7e6..a4f40d6ea3f7 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -45,14 +45,15 @@ pub use selection_state::{ Selection, SelectionHighlight, }; pub use space_view::{ - DataResult, IdentifiedViewSystem, PerSystemDataResults, PerSystemEntities, PropertyOverrides, - RecommendedSpaceView, SmallVisualizerSet, SpaceViewClass, SpaceViewClassIdentifier, - SpaceViewClassLayoutPriority, SpaceViewClassRegistry, SpaceViewClassRegistryError, - SpaceViewEntityHighlight, SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewSpawnHeuristics, - SpaceViewState, SpaceViewStateExt, SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, - SystemExecutionOutput, ViewContextCollection, ViewContextSystem, ViewQuery, - ViewSystemIdentifier, VisualizableFilterContext, VisualizerAdditionalApplicabilityFilter, - VisualizerCollection, VisualizerQueryInfo, VisualizerSystem, + DataResult, IdentifiedViewSystem, OverridePath, PerSystemDataResults, PerSystemEntities, + PropertyOverrides, RecommendedSpaceView, SmallVisualizerSet, SpaceViewClass, + SpaceViewClassIdentifier, SpaceViewClassLayoutPriority, SpaceViewClassRegistry, + SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights, + SpaceViewOutlineMasks, SpaceViewSpawnHeuristics, SpaceViewState, SpaceViewStateExt, + SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, SystemExecutionOutput, + ViewContextCollection, ViewContextSystem, ViewQuery, ViewSystemIdentifier, + VisualizableFilterContext, VisualizerAdditionalApplicabilityFilter, VisualizerCollection, + VisualizerQueryInfo, VisualizerSystem, }; pub use store_context::StoreContext; pub use tensor::{TensorDecodeCache, TensorStats, TensorStatsCache}; diff --git a/crates/re_viewer_context/src/space_view/mod.rs b/crates/re_viewer_context/src/space_view/mod.rs index f5d557acd190..df6bc0a3cff1 100644 --- a/crates/re_viewer_context/src/space_view/mod.rs +++ b/crates/re_viewer_context/src/space_view/mod.rs @@ -29,7 +29,8 @@ pub use spawn_heuristics::{RecommendedSpaceView, SpaceViewSpawnHeuristics}; pub use system_execution_output::SystemExecutionOutput; pub use view_context_system::{ViewContextCollection, ViewContextSystem}; pub use view_query::{ - DataResult, PerSystemDataResults, PropertyOverrides, SmallVisualizerSet, ViewQuery, + DataResult, OverridePath, PerSystemDataResults, PropertyOverrides, SmallVisualizerSet, + ViewQuery, }; pub use visualizer_entity_subscriber::VisualizerAdditionalApplicabilityFilter; pub use visualizer_system::{VisualizerCollection, VisualizerQueryInfo, VisualizerSystem}; diff --git a/crates/re_viewer_context/src/space_view/view_query.rs b/crates/re_viewer_context/src/space_view/view_query.rs index 07d09d3da24f..9847cc516536 100644 --- a/crates/re_viewer_context/src/space_view/view_query.rs +++ b/crates/re_viewer_context/src/space_view/view_query.rs @@ -1,19 +1,37 @@ use std::collections::BTreeMap; -use ahash::HashMap; use itertools::Itertools; +use nohash_hasher::IntMap; use once_cell::sync::Lazy; + use re_data_store::LatestAtQuery; use re_entity_db::{EntityPath, EntityProperties, EntityPropertiesComponent, TimeInt, Timeline}; -use re_log_types::{DataCell, DataRow, RowId, StoreKind}; -use re_types::{ComponentName, Loggable}; +use re_log_types::StoreKind; +use re_types::ComponentName; use smallvec::SmallVec; use crate::{ - blueprint_timepoint_for_writes, SpaceViewHighlights, SpaceViewId, SystemCommand, - SystemCommandSender as _, ViewSystemIdentifier, ViewerContext, + DataResultTree, SpaceViewHighlights, SpaceViewId, ViewSystemIdentifier, ViewerContext, }; +/// Path to a specific entity in a specific store used for overrides. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct OverridePath { + // NOTE: StoreKind is easier to work with than a `StoreId`` or full `DataStore` but + // might still be ambiguous when we have multiple stores active at a time. + pub store_kind: StoreKind, + pub path: EntityPath, +} + +impl OverridePath { + pub fn blueprint_path(path: EntityPath) -> Self { + Self { + store_kind: StoreKind::Blueprint, + path, + } + } +} + #[derive(Clone, Debug, PartialEq)] pub struct PropertyOverrides { /// The accumulated properties (including any hierarchical flattening) to apply. @@ -28,10 +46,14 @@ pub struct PropertyOverrides { pub recursive_properties: Option, /// An alternative store and entity path to use for the specified component. - // NOTE: StoreKind is easier to work with than a `StoreId`` or full `DataStore` but - // might still be ambiguous when we have multiple stores active at a time. + /// + /// These are resolved overrides, i.e. the result of recursive override propagation + individual overrides. // TODO(jleibs): Consider something like `tinymap` for this. - pub component_overrides: HashMap, + // TODO(andreas): Should be a `Cow` to not do as many clones. + // TODO(andreas): Track recursive vs resolved (== individual + recursive) overrides. + // Recursive here meaning inherited + own recursive, i.e. not just what's on the path. + // What is logged on *this* entity can be inferred from walking up the tree. + pub resolved_component_overrides: IntMap, /// `EntityPath` in the Blueprint store where updated overrides should be written back /// for properties that apply recursively. @@ -91,6 +113,64 @@ impl DataResult { .map(|p| &p.individual_override_path) } + /// Saves a recursive override OR clears both (!) individual & recursive overrides if the override is due to a parent recursive override or a default value. + // TODO(andreas): Does not take individual overrides into account yet. + // TODO(andreas): This should have a unit test, but the delayed override write makes it hard to test. + pub fn save_recursive_override_or_clear_if_redundant( + &self, + ctx: &ViewerContext<'_>, + data_result_tree: &DataResultTree, + desired_override: &C, + ) { + re_tracing::profile_function!(); + + // TODO(jleibs): Make it impossible for this to happen with different type structure + // This should never happen unless we're doing something with a partially processed + // query. + let (Some(recursive_override_path), Some(individual_override_path)) = ( + self.recursive_override_path(), + self.individual_override_path(), + ) else { + re_log::warn!( + "Tried to save override for {:?} but it has no override path", + self.entity_path + ); + return; + }; + + if let Some(current_resolved_override) = self.lookup_override::(ctx) { + // Do nothing if the resolved override is already the same as the new override. + if ¤t_resolved_override == desired_override { + return; + } + + // TODO(andreas): Assumes this is a recursive override + let parent_recursive_override = self + .entity_path + .parent() + .and_then(|parent_path| data_result_tree.lookup_result_by_path(&parent_path)) + .and_then(|data_result| data_result.lookup_override::(ctx)); + + // If the parent has a recursive override that is the same as the new override, + // clear both individual and recursive override at the current path. + // (at least one of them has to be set, otherwise the current resolved override would be the same as the desired override) + // + // Another case for clearing + if parent_recursive_override.as_ref() == Some(desired_override) + || (parent_recursive_override.is_none() && desired_override == &C::default()) + { + // TODO(andreas): It might be that only either of these two are necessary, in that case we shouldn't clear both. + ctx.save_empty_blueprint_component::(recursive_override_path); + ctx.save_empty_blueprint_component::(individual_override_path); + } else { + ctx.save_blueprint_component(recursive_override_path, desired_override); + } + } else { + // No override at all so far, simply set it. + ctx.save_blueprint_component(recursive_override_path, desired_override); + } + } + /// Write the [`EntityProperties`] for this result back to the Blueprint store on the recursive override. /// /// Setting `new_recursive_props` to `None` will always clear the override. @@ -145,44 +225,20 @@ impl DataResult { return; }; - let cell = match new_individual_props { + match new_individual_props { None => { - re_log::debug!("Clearing {:?}", override_path); - - Some(DataCell::from_arrow_empty( - EntityPropertiesComponent::name(), - EntityPropertiesComponent::arrow_datatype(), - )) + ctx.save_empty_blueprint_component::(override_path); } Some(props) => { // A value of `None` in the data store means "use the default value", so if // the properties are `None`, we only must save if `props` is different // from the default. if props.has_edits(properties.unwrap_or(&DEFAULT_PROPS)) { - re_log::debug!("Overriding {:?} with {:?}", override_path, props); - let component = EntityPropertiesComponent(props); - - Some(DataCell::from([component])) - } else { - None + ctx.save_blueprint_component(override_path, &component); } } }; - - if let Some(cell) = cell { - let timepoint = blueprint_timepoint_for_writes(); - - let row = - DataRow::from_cells1_sized(RowId::new(), override_path.clone(), timepoint, 1, cell) - .unwrap(); - - ctx.command_sender - .send_system(SystemCommand::UpdateBlueprint( - ctx.store_context.blueprint.store_id().clone(), - vec![row], - )); - } } #[inline] @@ -214,6 +270,87 @@ impl DataResult { .as_ref() .and_then(|p| p.individual_properties.as_ref()) } + + pub fn lookup_override(&self, ctx: &ViewerContext<'_>) -> Option { + self.property_overrides + .as_ref() + .and_then(|p| p.resolved_component_overrides.get(&C::name())) + .and_then(|OverridePath { store_kind, path }| match store_kind { + StoreKind::Blueprint => ctx + .store_context + .blueprint + .store() + .query_latest_component::(path, ctx.blueprint_query), + StoreKind::Recording => ctx + .entity_db + .store() + .query_latest_component::(path, &ctx.current_query()), + }) + .map(|c| c.value) + } + + #[inline] + pub fn lookup_override_or_default( + &self, + ctx: &ViewerContext<'_>, + ) -> C { + self.lookup_override(ctx).unwrap_or_default() + } + + /// Returns from which entity path an override originates from. + /// + /// Returns None if there was no override at all. + /// Note that if this returns the current path, the override might be either an individual or recursive override. + #[inline] + pub fn component_override_source( + &self, + result_tree: &DataResultTree, + component_name: &ComponentName, + ) -> Option { + re_tracing::profile_function!(); + + // If we don't have a resolved override, clearly nothing overrode this. + let active_override = self + .property_overrides + .as_ref() + .and_then(|p| p.resolved_component_overrides.get(component_name))?; + + // Walk up the tree to find the highest ancestor which has a matching override. + // This must be the ancestor we inherited the override from. Note that `active_override` + // is a `(StoreKind, EntityPath)`, not a value. + let mut override_source = self.entity_path.clone(); + while let Some(parent_path) = override_source.parent() { + if result_tree + .lookup_result_by_path(&parent_path) + .and_then(|data_result| data_result.property_overrides.as_ref()) + .map_or(true, |property_overrides| { + // TODO(andreas): Assumes all overrides are recursive which is not true, + // This should access `recursive_component_overrides` instead. + property_overrides + .resolved_component_overrides + .get(component_name) + != Some(active_override) + }) + { + break; + } + + override_source = parent_path; + } + + Some(override_source) + } + + /// Shorthand for checking for visibility on data overrides. + /// + /// Note that this won't check if the data store has visibility logged. + // TODO(andreas): Should this be possible? + // TODO(andreas): Should the result be cached, this might be a very common operation? + #[inline] + pub fn is_visible(&self, ctx: &ViewerContext<'_>) -> bool { + self.lookup_override_or_default::(ctx) + .0 + } } pub type PerSystemDataResults<'a> = BTreeMap>; @@ -244,17 +381,21 @@ pub struct ViewQuery<'s> { impl<'s> ViewQuery<'s> { /// Iter over all of the currently visible [`DataResult`]s for a given `ViewSystem` - pub fn iter_visible_data_results( - &self, + pub fn iter_visible_data_results<'a>( + &'a self, + ctx: &'a ViewerContext<'a>, system: ViewSystemIdentifier, - ) -> impl Iterator { + ) -> impl Iterator + where + 's: 'a, + { self.per_system_data_results.get(&system).map_or( itertools::Either::Left(std::iter::empty()), |results| { itertools::Either::Right( results .iter() - .filter(|result| result.accumulated_properties().visible) + .filter(|result| result.is_visible(ctx)) .copied(), ) }, diff --git a/crates/re_viewport/src/context_menu/actions/show_hide.rs b/crates/re_viewport/src/context_menu/actions/show_hide.rs index f3319805ab55..b4488c42eb32 100644 --- a/crates/re_viewport/src/context_menu/actions/show_hide.rs +++ b/crates/re_viewport/src/context_menu/actions/show_hide.rs @@ -125,11 +125,7 @@ fn data_result_visible( query_result .tree .lookup_result_by_path(&instance_path.entity_path) - .map(|data_result| { - data_result - .recursive_properties() - .map_or(true, |prop| prop.visible) - }) + .map(|data_result| data_result.is_visible(ctx.viewer_context)) }) .flatten() } @@ -140,17 +136,18 @@ fn set_data_result_visible( instance_path: &InstancePath, visible: bool, ) { - let query_result = ctx.viewer_context.lookup_query_result(*space_view_id); - if let Some(data_result) = query_result - .tree - .lookup_result_by_path(&instance_path.entity_path) - { - let mut recursive_properties = data_result - .recursive_properties() - .cloned() - .unwrap_or_default(); - recursive_properties.visible = visible; - - data_result.save_recursive_override(ctx.viewer_context, Some(recursive_properties)); + if let Some(query_result) = ctx.viewer_context.query_results.get(space_view_id) { + if let Some(data_result) = query_result + .tree + .lookup_result_by_path(&instance_path.entity_path) + { + data_result.save_recursive_override_or_clear_if_redundant( + ctx.viewer_context, + &query_result.tree, + &re_types::blueprint::components::Visible(visible), + ); + } + } else { + re_log::error!("No query available for space view {:?}", space_view_id); } } diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index 293eb6863ee4..588fefd8c047 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -7,6 +7,7 @@ use re_entity_db::InstancePath; use re_log_types::EntityPath; use re_log_types::EntityPathRule; use re_space_view::{SpaceViewBlueprint, SpaceViewName}; +use re_types::blueprint::components::Visible; use re_ui::{drag_and_drop::DropTarget, list_item::ListItem, ReUi}; use re_viewer_context::{CollapseScope, DataResultTree}; use re_viewer_context::{ @@ -539,12 +540,7 @@ impl Viewport<'_, '_> { let is_item_hovered = ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered; - let visible = - data_result_node.map_or(false, |n| n.data_result.accumulated_properties().visible); - let mut recursive_properties = data_result_node - .and_then(|n| n.data_result.recursive_properties()) - .cloned() - .unwrap_or_default(); + let visible = data_result_node.map_or(false, |n| n.data_result.is_visible(ctx)); let item_label = if entity_path.is_root() { "/ (root)".to_owned() @@ -573,12 +569,20 @@ impl Viewport<'_, '_> { .subdued(subdued) .force_hovered(is_item_hovered) .with_buttons(|re_ui: &_, ui: &mut egui::Ui| { - let vis_response = visibility_button_ui( - re_ui, - ui, - space_view_visible, - &mut recursive_properties.visible, - ); + let mut visible_after = visible; + let vis_response = + visibility_button_ui(re_ui, ui, space_view_visible, &mut visible_after); + if visible_after != visible { + if let Some(data_result_node) = data_result_node { + data_result_node + .data_result + .save_recursive_override_or_clear_if_redundant( + ctx, + &query_result.tree, + &Visible(visible_after), + ); + } + } let response = remove_button_ui( re_ui, @@ -602,7 +606,7 @@ impl Viewport<'_, '_> { let default_open = entity_path.starts_with(&space_view.space_origin) && Self::default_open_for_data_result(node); - let response = list_item + list_item .show_collapsing( ui, CollapseScope::BlueprintTree.data_result(space_view.id, entity_path.clone()), @@ -634,12 +638,7 @@ impl Viewport<'_, '_> { } }, ) - .item_response; - - node.data_result - .save_recursive_override(ctx, Some(recursive_properties)); - - response + .item_response } else { list_item.show(ui) }; diff --git a/examples/rust/custom_space_view/src/color_coordinates_visualizer_system.rs b/examples/rust/custom_space_view/src/color_coordinates_visualizer_system.rs index 690af975e98a..4d072495bf04 100644 --- a/examples/rust/custom_space_view/src/color_coordinates_visualizer_system.rs +++ b/examples/rust/custom_space_view/src/color_coordinates_visualizer_system.rs @@ -54,7 +54,7 @@ impl VisualizerSystem for InstanceColorSystem { _view_ctx: &ViewContextCollection, ) -> Result, SpaceViewSystemExecutionError> { // For each entity in the space view that should be displayed with the `InstanceColorSystem`… - for data_result in query.iter_visible_data_results(Self::identifier()) { + for data_result in query.iter_visible_data_results(ctx, Self::identifier()) { // …gather all colors and their instance ids. if let Ok(arch_view) = query_archetype::( ctx.entity_db.store(),