From c8d6edf59bd496b523a5dbf9506d11c6951743a6 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Fri, 15 Mar 2024 10:33:29 +0100 Subject: [PATCH] Allow hiding/showing entity subtrees under shown/hidden parent tree (#5508) ### What * Fixes #5464 * using the design we agreed upon that doesn't use tristate buttons in the blueprint panel * Next step on #5463 * Fixes #5396 * Fixes #3194 The title describes the main effect this has on what Rerun can do compared to the previous release. But the real star of the show is that visible is now a component, not part of `EntityProperties`. @ reviewer: Please take your time and explore the behavior both on blueprint panel and selection panel, there's a surprising amount of nuance in here. For instance there's extra logic for allowing you to go back to a clean slate with ui interactions - for instance hiding & unhiding an item that doesn't receive any parent overrides will remove the visibility override completely as the default value for visibility is 'true'. You can now hide/show under a shown/hidden parent tree: https://github.com/rerun-io/rerun/assets/1220815/f412cac5-2db6-43d0-9b02-d2269cb60824 We're able to keep track of where an override comes from, this is exposed in the selection panels' tooltip: ![image](https://github.com/rerun-io/rerun/assets/1220815/ef7794ac-07c8-4930-ba92-11a45f91f6fe) ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested the web demo (if applicable): * Using newly built examples: [app.rerun.io](https://app.rerun.io/pr/5508/index.html) * Using examples from latest `main` build: [app.rerun.io](https://app.rerun.io/pr/5508/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json) * Using full set of examples from `nightly` build: [app.rerun.io](https://app.rerun.io/pr/5508/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json) * [x] The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG * [x] If applicable, add a new check to the [release checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)! - [PR Build Summary](https://build.rerun.io/pr/5508) - [Docs preview](https://rerun.io/preview/a09f77a2233fd101a4c5a94703f7726a920a18c6/docs) - [Examples preview](https://rerun.io/preview/a09f77a2233fd101a4c5a94703f7726a920a18c6/examples) - [Recent benchmark results](https://build.rerun.io/graphs/crates.html) - [Wasm size tracking](https://build.rerun.io/graphs/sizes.html) --- crates/re_entity_db/src/entity_properties.rs | 8 +- crates/re_space_view/src/data_query.rs | 9 +- crates/re_space_view/src/space_view.rs | 36 +-- .../re_space_view/src/space_view_contents.rs | 22 +- .../src/visualizer_system.rs | 2 +- .../src/space_view_class.rs | 2 +- .../src/contexts/depth_offsets.rs | 2 +- .../src/visualizers/cameras.rs | 2 +- .../src/visualizers/entity_iterator.rs | 6 +- .../src/visualizers/transform3d_arrows.rs | 2 +- .../src/visualizer_system.rs | 2 +- .../src/visualizer_system.rs | 2 +- .../src/visualizer_system.rs | 2 +- .../src/legacy_visualizer_system.rs | 19 +- .../src/line_visualizer_system.rs | 14 +- .../src/overrides.rs | 28 +-- .../src/point_visualizer_system.rs | 16 +- .../rerun/blueprint/components/visible.fbs | 2 +- .../re_types/src/blueprint/components/mod.rs | 1 + .../src/blueprint/components/visible.rs | 2 +- .../src/blueprint/components/visible_ext.rs | 7 + crates/re_ui/src/lib.rs | 17 +- crates/re_viewer/src/ui/override_ui.rs | 29 +-- crates/re_viewer/src/ui/selection_panel.rs | 53 ++++- .../src/blueprint_helpers.rs | 13 +- crates/re_viewer_context/src/lib.rs | 17 +- .../re_viewer_context/src/space_view/mod.rs | 3 +- .../src/space_view/view_query.rs | 219 ++++++++++++++---- .../src/context_menu/actions/show_hide.rs | 31 ++- .../re_viewport/src/viewport_blueprint_ui.rs | 37 ++- .../color_coordinates_visualizer_system.rs | 2 +- 31 files changed, 388 insertions(+), 219 deletions(-) create mode 100644 crates/re_types/src/blueprint/components/visible_ext.rs 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(),