diff --git a/.vscode/settings.json b/.vscode/settings.json index 805b04da6452..90ef30e18ae1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -37,6 +37,8 @@ "Tonemapper", "tonemapping", "unsmoothed", + "visualizable", + "visualizability", "voronoi", "vram", "Wgsl" @@ -50,7 +52,7 @@ "--workspace", "--message-format=json", "--all-targets", - "--all-features", // --all-features will set the `__ci` feature flag, which stops crates/re_web_viewer_server/build.rs from building the web viewer + "--all-features" // --all-features will set the `__ci` feature flag, which stops crates/re_web_viewer_server/build.rs from building the web viewer ], "rust-analyzer.cargo.buildScripts.overrideCommand": [ "cargo", @@ -83,12 +85,10 @@ "ruff.lint.args": [ "--config=rerun_py/pyproject.toml" ], - "prettier.configPath": ".prettierrc.toml", "[python]": { - "editor.defaultFormatter": "charliermarsh.ruff", + "editor.defaultFormatter": "charliermarsh.ruff" }, - "[javascript]": { "editor.defaultFormatter": "esbenp.prettier-vscode" }, diff --git a/crates/re_space_view/src/data_query.rs b/crates/re_space_view/src/data_query.rs index 6413f382abe1..f984cf9d40db 100644 --- a/crates/re_space_view/src/data_query.rs +++ b/crates/re_space_view/src/data_query.rs @@ -1,5 +1,7 @@ use re_data_store::{EntityProperties, EntityPropertyMap}; -use re_viewer_context::{DataQueryResult, EntitiesPerSystem, StoreContext}; +use re_viewer_context::{ + DataQueryResult, IndicatorMatchingEntities, PerVisualizer, StoreContext, VisualizableEntities, +}; pub struct EntityOverrideContext { pub root: EntityProperties, @@ -28,6 +30,7 @@ pub trait DataQuery { fn execute_query( &self, ctx: &StoreContext<'_>, - entities_per_system: &EntitiesPerSystem, + visualizable_entities_for_visualizer_systems: &PerVisualizer, + indicator_matching_entities_per_visualizer: &PerVisualizer, ) -> DataQueryResult; } diff --git a/crates/re_space_view/src/data_query_blueprint.rs b/crates/re_space_view/src/data_query_blueprint.rs index 8283b6edaa06..ccdaf4eb70ca 100644 --- a/crates/re_space_view/src/data_query_blueprint.rs +++ b/crates/re_space_view/src/data_query_blueprint.rs @@ -1,3 +1,4 @@ +use nohash_hasher::IntMap; use slotmap::SlotMap; use smallvec::SmallVec; @@ -10,8 +11,9 @@ use re_log_types::{ use re_types_core::archetypes::Clear; use re_viewer_context::{ DataQueryId, DataQueryResult, DataResult, DataResultHandle, DataResultNode, DataResultTree, - EntitiesPerSystem, PropertyOverrides, SpaceViewClassIdentifier, SpaceViewId, StoreContext, - SystemCommand, SystemCommandSender as _, ViewerContext, + IndicatorMatchingEntities, PerVisualizer, PropertyOverrides, SpaceViewClassIdentifier, + SpaceViewId, StoreContext, SystemCommand, SystemCommandSender as _, ViewSystemIdentifier, + ViewerContext, VisualizableEntities, }; use crate::{ @@ -176,13 +178,18 @@ impl DataQuery for DataQueryBlueprint { fn execute_query( &self, ctx: &re_viewer_context::StoreContext<'_>, - entities_per_system: &EntitiesPerSystem, + visualizable_entities_for_visualizer_systems: &PerVisualizer, + indicator_matching_entities_per_visualizer: &PerVisualizer, ) -> DataQueryResult { re_tracing::profile_function!(); let mut data_results = SlotMap::::default(); - let executor = QueryExpressionEvaluator::new(self, entities_per_system); + let executor = QueryExpressionEvaluator::new( + self, + visualizable_entities_for_visualizer_systems, + indicator_matching_entities_per_visualizer, + ); let root_handle = ctx.recording.and_then(|store| { re_tracing::profile_scope!("add_entity_tree_to_data_results_recursive"); @@ -202,19 +209,26 @@ impl DataQuery for DataQueryBlueprint { /// used to efficiently determine if we should continue the walk or switch /// to a pure recursive evaluation. struct QueryExpressionEvaluator<'a> { - per_system_entity_list: &'a EntitiesPerSystem, + visualizable_entities_for_visualizer_systems: &'a PerVisualizer, + indicator_matching_entities_per_visualizer: + &'a IntMap, entity_path_filter: EntityPathFilter, } impl<'a> QueryExpressionEvaluator<'a> { fn new( blueprint: &'a DataQueryBlueprint, - per_system_entity_list: &'a EntitiesPerSystem, + visualizable_entities_for_visualizer_systems: &'a PerVisualizer, + indicator_matching_entities_per_visualizer: &'a IntMap< + ViewSystemIdentifier, + IndicatorMatchingEntities, + >, ) -> Self { re_tracing::profile_function!(); Self { - per_system_entity_list, + visualizable_entities_for_visualizer_systems, + indicator_matching_entities_per_visualizer, entity_path_filter: blueprint.entity_path_filter.clone(), } } @@ -242,11 +256,30 @@ impl<'a> QueryExpressionEvaluator<'a> { // Only populate view_parts if this is a match // Note that allowed prefixes that aren't matches can still create groups let view_parts: SmallVec<_> = if any_match { - self.per_system_entity_list + self.visualizable_entities_for_visualizer_systems .iter() - .filter_map(|(part, ents)| { + .filter_map(|(visualizer, ents)| { if ents.contains(entity_path) { - Some(*part) + // TODO(andreas): + // * not all queries do just heuristic filtering of visualizers, + // some set the visualizer upfront, others should skip this check and visualize all + // * Space view classes should be able to modify this check. + // As of writing this hasn't been done yet in order to simplify things + // * querying the per-visualizer lists every time is silly + // -> at beginning of query squash all visualizers in `visualizable_entities_for_visualizer_systems` + // to a single `IntSet` + // -> consider three steps of query: list entities, list their visualizers, list their properties + if self + .indicator_matching_entities_per_visualizer + .get(visualizer) + .map_or(false, |matching_list| { + matching_list.contains(&entity_path.hash()) + }) + { + Some(*visualizer) + } else { + None + } } else { None } @@ -460,7 +493,7 @@ impl<'a> PropertyResolver for DataQueryPropertyResolver<'a> { mod tests { use re_data_store::StoreDb; use re_log_types::{example_components::MyPoint, DataRow, RowId, StoreId, TimePoint, Timeline}; - use re_viewer_context::StoreContext; + use re_viewer_context::{StoreContext, VisualizableEntities}; use super::*; @@ -487,17 +520,21 @@ mod tests { recording.add_data_row(row).unwrap(); } - let mut entities_per_system = EntitiesPerSystem::default(); + let mut visualizable_entities_for_visualizer_systems = + PerVisualizer::::default(); - entities_per_system + visualizable_entities_for_visualizer_systems + .0 .entry("Points3D".into()) .or_insert_with(|| { - [ - EntityPath::from("parent"), - EntityPath::from("parent/skipped/child1"), - ] - .into_iter() - .collect() + VisualizableEntities( + [ + EntityPath::from("parent"), + EntityPath::from("parent/skipped/child1"), + ] + .into_iter() + .collect(), + ) }); let ctx = StoreContext { @@ -603,7 +640,25 @@ mod tests { entity_path_filter: EntityPathFilter::parse_forgiving(filter), }; - let query_result = query.execute_query(&ctx, &entities_per_system); + let indicator_matching_entities_per_visualizer = + PerVisualizer::( + visualizable_entities_for_visualizer_systems + .iter() + .map(|(id, entities)| { + ( + *id, + IndicatorMatchingEntities( + entities.0.iter().map(|path| path.hash()).collect(), + ), + ) + }) + .collect(), + ); + let query_result = query.execute_query( + &ctx, + &visualizable_entities_for_visualizer_systems, + &indicator_matching_entities_per_visualizer, + ); let mut visited = vec![]; query_result.tree.visit(&mut |handle| { diff --git a/crates/re_space_view_spatial/src/contexts/transform_context.rs b/crates/re_space_view_spatial/src/contexts/transform_context.rs index 0a9213129da5..a0065dc786bf 100644 --- a/crates/re_space_view_spatial/src/contexts/transform_context.rs +++ b/crates/re_space_view_spatial/src/contexts/transform_context.rs @@ -130,7 +130,7 @@ impl ViewContextSystem for TransformContext { let mut encountered_pinhole = None; let mut reference_from_ancestor = glam::Affine3A::IDENTITY; while let Some(parent_path) = current_tree.path.parent() { - let Some(parent_tree) = &entity_tree.subtree(&parent_path) else { + let Some(parent_tree) = entity_tree.subtree(&parent_path) else { // Unlike not having the space path in the hierarchy, this should be impossible. re_log::error_once!( "Path {} is not part of the global Entity tree whereas its child {} is", diff --git a/crates/re_space_view_spatial/src/parts/boxes2d.rs b/crates/re_space_view_spatial/src/parts/boxes2d.rs index 731838e1c9f1..89663616f2b5 100644 --- a/crates/re_space_view_spatial/src/parts/boxes2d.rs +++ b/crates/re_space_view_spatial/src/parts/boxes2d.rs @@ -1,6 +1,4 @@ -use nohash_hasher::IntSet; use re_data_store::{EntityPath, InstancePathHash}; -use re_log_types::EntityPathHash; use re_query::{ArchetypeView, QueryError}; use re_types::{ archetypes::Boxes2D, @@ -8,8 +6,9 @@ use re_types::{ Archetype, ComponentNameSet, }; use re_viewer_context::{ - HeuristicFilterContext, IdentifiedViewSystem, ResolvedAnnotationInfos, + ApplicableEntities, IdentifiedViewSystem, ResolvedAnnotationInfos, SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem, ViewQuery, ViewerContext, + VisualizableEntities, }; use crate::{ @@ -19,8 +18,9 @@ use crate::{ }; use super::{ - entity_iterator::process_archetype_views, picking_id_from_instance_key, process_annotations, - process_colors, process_radii, SpatialViewPartData, + entity_iterator::process_archetype_views, filter_visualizable_2d_entities, + picking_id_from_instance_key, process_annotations, process_colors, process_radii, + SpatialViewPartData, }; pub struct Boxes2DPart { @@ -179,25 +179,13 @@ impl ViewPartSystem for Boxes2DPart { std::iter::once(Boxes2D::indicator().name()).collect() } - fn heuristic_filter( + fn filter_visualizable_entities( &self, - entities_with_matching_indicator: &IntSet, - ent_path: &EntityPath, - ctx: HeuristicFilterContext, - ) -> bool { - if !entities_with_matching_indicator.contains(&ent_path.hash()) { - return false; - } - - // 2D parts can only ever be rendered properly as part of a 3D scene if the - // transform graph includes a pinhole projection. Pinholes only map from 2D children - // to 3D parents, so if the required pinhole exists, it must be an ancestor. - // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. - if ctx.class == "3D" && !ctx.has_ancestor_pinhole { - return false; - } - - true + entities: ApplicableEntities, + context: &dyn std::any::Any, + ) -> VisualizableEntities { + re_tracing::profile_function!(); + filter_visualizable_2d_entities(entities, context) } fn execute( diff --git a/crates/re_space_view_spatial/src/parts/images.rs b/crates/re_space_view_spatial/src/parts/images.rs index e324bf70ac54..47df4065cff3 100644 --- a/crates/re_space_view_spatial/src/parts/images.rs +++ b/crates/re_space_view_spatial/src/parts/images.rs @@ -19,15 +19,15 @@ use re_types::{ Archetype as _, ComponentNameSet, }; use re_viewer_context::{ - gpu_bridge, DefaultColor, HeuristicFilterContext, SpaceViewClass, - SpaceViewSystemExecutionError, TensorDecodeCache, TensorStatsCache, ViewPartSystem, ViewQuery, - ViewerContext, VisualizerAdditionalApplicabilityFilter, + gpu_bridge, ApplicableEntities, DefaultColor, IdentifiedViewSystem, SpaceViewClass, + SpaceViewSystemExecutionError, TensorDecodeCache, TensorStatsCache, ViewContextCollection, + ViewPartSystem, ViewQuery, ViewerContext, VisualizableEntities, + VisualizerAdditionalApplicabilityFilter, }; -use re_viewer_context::{IdentifiedViewSystem, ViewContextCollection}; use crate::{ contexts::{EntityDepthOffsets, SpatialSceneEntityContext, TransformContext}, - parts::SIZE_BOOST_IN_POINTS_FOR_POINT_OUTLINES, + parts::{filter_visualizable_2d_entities, SIZE_BOOST_IN_POINTS_FOR_POINT_OUTLINES}, query_pinhole, view_kind::SpatialSpaceViewKind, SpatialSpaceView2D, SpatialSpaceView3D, @@ -691,25 +691,13 @@ impl ViewPartSystem for ImagesPart { Some(Box::new(ImageVisualizerEntityFilter)) } - fn heuristic_filter( + fn filter_visualizable_entities( &self, - entities_with_matching_indicator: &IntSet, - ent_path: &EntityPath, - ctx: HeuristicFilterContext, - ) -> bool { - if !entities_with_matching_indicator.contains(&ent_path.hash()) { - return false; - } - - // 2D parts can only ever be rendered properly as part of a 3D scene if the - // transform graph includes a pinhole projection. Pinholes only map from 2D children - // to 3D parents, so if the required pinhole exists, it must be an ancestor. - // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. - if ctx.class == "3D" && !ctx.has_ancestor_pinhole { - return false; - } - - true + entities: ApplicableEntities, + context: &dyn std::any::Any, + ) -> VisualizableEntities { + re_tracing::profile_function!(); + filter_visualizable_2d_entities(entities, context) } fn execute( diff --git a/crates/re_space_view_spatial/src/parts/lines2d.rs b/crates/re_space_view_spatial/src/parts/lines2d.rs index 3968ea4ff1df..6ce5a3c0a1bc 100644 --- a/crates/re_space_view_spatial/src/parts/lines2d.rs +++ b/crates/re_space_view_spatial/src/parts/lines2d.rs @@ -1,6 +1,4 @@ -use nohash_hasher::IntSet; use re_data_store::{EntityPath, InstancePathHash}; -use re_log_types::EntityPathHash; use re_query::{ArchetypeView, QueryError}; use re_types::{ archetypes::LineStrips2D, @@ -8,8 +6,9 @@ use re_types::{ Archetype as _, ComponentNameSet, }; use re_viewer_context::{ - HeuristicFilterContext, IdentifiedViewSystem, ResolvedAnnotationInfos, + ApplicableEntities, IdentifiedViewSystem, ResolvedAnnotationInfos, SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem, ViewQuery, ViewerContext, + VisualizableEntities, }; use crate::{ @@ -21,7 +20,10 @@ use crate::{ view_kind::SpatialSpaceViewKind, }; -use super::{picking_id_from_instance_key, process_annotations, SpatialViewPartData}; +use super::{ + filter_visualizable_2d_entities, picking_id_from_instance_key, process_annotations, + SpatialViewPartData, +}; pub struct Lines2DPart { /// If the number of arrows in the batch is > max_labels, don't render point labels. @@ -174,25 +176,13 @@ impl ViewPartSystem for Lines2DPart { std::iter::once(LineStrips2D::indicator().name()).collect() } - fn heuristic_filter( + fn filter_visualizable_entities( &self, - entities_with_matching_indicator: &IntSet, - ent_path: &EntityPath, - ctx: HeuristicFilterContext, - ) -> bool { - if !entities_with_matching_indicator.contains(&ent_path.hash()) { - return false; - } - - // 2D parts can only ever be rendered properly as part of a 3D scene if the - // transform graph includes a pinhole projection. Pinholes only map from 2D children - // to 3D parents, so if the required pinhole exists, it must be an ancestor. - // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. - if ctx.class == "3D" && !ctx.has_ancestor_pinhole { - return false; - } - - true + entities: ApplicableEntities, + context: &dyn std::any::Any, + ) -> VisualizableEntities { + re_tracing::profile_function!(); + filter_visualizable_2d_entities(entities, context) } fn execute( diff --git a/crates/re_space_view_spatial/src/parts/mod.rs b/crates/re_space_view_spatial/src/parts/mod.rs index ea2ded62b542..bf2041b94dcd 100644 --- a/crates/re_space_view_spatial/src/parts/mod.rs +++ b/crates/re_space_view_spatial/src/parts/mod.rs @@ -18,7 +18,7 @@ mod transform3d_arrows; pub use cameras::CamerasPart; pub use images::ImagesPart; pub use images::ViewerImage; -use re_types::components::Text; +use re_viewer_context::ApplicableEntities; pub use spatial_view_part::SpatialViewPartData; pub use transform3d_arrows::{add_axis_arrows, Transform3DArrowsPart}; @@ -28,15 +28,16 @@ pub use points3d::LoadedPoints; use ahash::HashMap; use re_data_store::{EntityPath, InstancePathHash}; -use re_types::components::{Color, InstanceKey}; +use re_types::components::{Color, InstanceKey, Text}; use re_types::datatypes::{KeypointId, KeypointPair}; use re_types::Archetype; -use re_viewer_context::SpaceViewClassRegistryError; use re_viewer_context::{ - auto_color, Annotations, DefaultColor, ResolvedAnnotationInfos, SpaceViewSystemRegistrator, - ViewPartCollection, ViewQuery, + auto_color, Annotations, DefaultColor, ResolvedAnnotationInfos, SpaceViewClassRegistryError, + SpaceViewSystemRegistrator, ViewPartCollection, ViewQuery, VisualizableEntities, }; +use crate::space_view_3d::VisualizableFilterContext3D; + use super::contexts::SpatialSceneEntityContext; /// Collection of keypoints for annotation context. @@ -341,3 +342,22 @@ pub fn image_view_coordinates() -> re_types::components::ViewCoordinates { // - z pointing into the image plane (this is convenient for reading out a depth image which has typically positive z values) re_types::components::ViewCoordinates::RDF } + +/// If 2d object is shown in a 3d space view, it is only then visualizable, if it is under a pinhole camera. +fn filter_visualizable_2d_entities( + entities: ApplicableEntities, + context: &dyn std::any::Any, +) -> VisualizableEntities { + // `VisualizableFilterContext3D` will only be available if we're in a 3D space view. + if let Some(context) = context.downcast_ref::() { + VisualizableEntities( + entities + .0 + .into_iter() + .filter(|ent_path| context.entities_under_pinhole.contains(&ent_path.hash())) + .collect(), + ) + } else { + VisualizableEntities(entities.0) + } +} diff --git a/crates/re_space_view_spatial/src/parts/points2d.rs b/crates/re_space_view_spatial/src/parts/points2d.rs index 2dc490fa057d..95a784a12737 100644 --- a/crates/re_space_view_spatial/src/parts/points2d.rs +++ b/crates/re_space_view_spatial/src/parts/points2d.rs @@ -1,6 +1,4 @@ -use nohash_hasher::IntSet; use re_data_store::{EntityPath, InstancePathHash}; -use re_log_types::EntityPathHash; use re_query::{ArchetypeView, QueryError}; use re_types::{ archetypes::Points2D, @@ -8,8 +6,9 @@ use re_types::{ Archetype, ComponentNameSet, }; use re_viewer_context::{ - HeuristicFilterContext, IdentifiedViewSystem, ResolvedAnnotationInfos, + ApplicableEntities, IdentifiedViewSystem, ResolvedAnnotationInfos, SpaceViewSystemExecutionError, ViewContextCollection, ViewPartSystem, ViewQuery, ViewerContext, + VisualizableEntities, }; use crate::{ @@ -21,7 +20,7 @@ use crate::{ view_kind::SpatialSpaceViewKind, }; -use super::{picking_id_from_instance_key, SpatialViewPartData}; +use super::{filter_visualizable_2d_entities, picking_id_from_instance_key, SpatialViewPartData}; pub struct Points2DPart { /// If the number of points in the batch is > max_labels, don't render point labels. @@ -201,25 +200,13 @@ impl ViewPartSystem for Points2DPart { std::iter::once(Points2D::indicator().name()).collect() } - fn heuristic_filter( + fn filter_visualizable_entities( &self, - entities_with_matching_indicator: &IntSet, - ent_path: &EntityPath, - ctx: HeuristicFilterContext, - ) -> bool { - if !entities_with_matching_indicator.contains(&ent_path.hash()) { - return false; - } - - // 2D parts can only ever be rendered properly as part of a 3D scene if the - // transform graph includes a pinhole projection. Pinholes only map from 2D children - // to 3D parents, so if the required pinhole exists, it must be an ancestor. - // Filter them otherwise to avoid ending up with 2D content mixed into 3D scenes. - if ctx.class == "3D" && !ctx.has_ancestor_pinhole { - return false; - } - - true + entities: ApplicableEntities, + context: &dyn std::any::Any, + ) -> VisualizableEntities { + re_tracing::profile_function!(); + filter_visualizable_2d_entities(entities, context) } fn execute( diff --git a/crates/re_space_view_spatial/src/space_view_3d.rs b/crates/re_space_view_spatial/src/space_view_3d.rs index b30f120993cd..22625f59d5d2 100644 --- a/crates/re_space_view_spatial/src/space_view_3d.rs +++ b/crates/re_space_view_spatial/src/space_view_3d.rs @@ -1,5 +1,7 @@ -use re_data_store::EntityProperties; -use re_log_types::EntityPath; +use nohash_hasher::IntSet; +use re_data_store::{EntityProperties, EntityTree}; +use re_log_types::{EntityPath, EntityPathHash}; +use re_types::{components::PinholeProjection, Loggable as _}; use re_viewer_context::{ AutoSpawnHeuristic, IdentifiedViewSystem as _, PerSystemEntities, SpaceViewClass, SpaceViewClassRegistryError, SpaceViewId, SpaceViewSystemExecutionError, ViewQuery, @@ -14,6 +16,13 @@ use crate::{ view_kind::SpatialSpaceViewKind, }; +// TODO(andreas): This context is used to determine whether a 2D entity has a valid transform +// and is thus visualizable. This should be expanded to cover any invalid transform as non-visualizable. +pub struct VisualizableFilterContext3D { + /// Set of all entities that are under a pinhole camera. + pub entities_under_pinhole: IntSet, +} + #[derive(Default)] pub struct SpatialSpaceView3D; @@ -49,6 +58,54 @@ impl SpaceViewClass for SpatialSpaceView3D { re_viewer_context::SpaceViewClassLayoutPriority::High } + fn visualizable_filter_context( + &self, + space_origin: &EntityPath, + store_db: &re_data_store::StoreDb, + ) -> Box { + re_tracing::profile_function!(); + + // TODO(andreas): Potential optimization: + // We already know all the entities that have a pinhole camera - indirectly today through visualizers, but could also be directly. + // Meaning we don't need to walk until we find a pinhole camera! + // Obviously should skip the whole thing if there are no pinhole cameras under space_origin! + // TODO(jleibs): Component prefix tree on EntityTree would fix this problem nicely. + + let mut entities_under_pinhole = IntSet::default(); + + fn visit_children_recursively( + tree: &EntityTree, + entities_under_pinhole: &mut IntSet, + ) { + if tree + .entity + .components + .contains_key(&PinholeProjection::name()) + { + // This and all children under it are under a pinhole camera! + tree.visit_children_recursively(&mut |ent_path| { + entities_under_pinhole.insert(ent_path.hash()); + }); + } else { + for child in tree.children.values() { + visit_children_recursively(child, entities_under_pinhole); + } + } + } + + let entity_tree = &store_db.tree(); + + // Walk down the tree from the space_origin. + let Some(current_tree) = &entity_tree.subtree(space_origin) else { + return Box::new(()); + }; + visit_children_recursively(current_tree, &mut entities_under_pinhole); + + Box::new(VisualizableFilterContext3D { + entities_under_pinhole, + }) + } + fn auto_spawn_heuristic( &self, ctx: &ViewerContext<'_>, diff --git a/crates/re_viewer/Cargo.toml b/crates/re_viewer/Cargo.toml index 177e326496ce..eb9b5cc795f9 100644 --- a/crates/re_viewer/Cargo.toml +++ b/crates/re_viewer/Cargo.toml @@ -51,7 +51,6 @@ re_log.workspace = true re_memory.workspace = true re_renderer = { workspace = true, default-features = false } re_smart_channel.workspace = true -re_space_view.workspace = true re_space_view_bar_chart.workspace = true re_space_view_dataframe.workspace = true re_space_view_spatial.workspace = true @@ -59,6 +58,7 @@ re_space_view_tensor.workspace = true re_space_view_text_document = { workspace = true, features = ["markdown"] } re_space_view_text_log.workspace = true re_space_view_time_series.workspace = true +re_space_view.workspace = true re_time_panel.workspace = true re_tracing = { workspace = true, features = ["server"] } re_types_core.workspace = true diff --git a/crates/re_viewer/src/app_state.rs b/crates/re_viewer/src/app_state.rs index 3e27b2ce2f01..4c939b3db428 100644 --- a/crates/re_viewer/src/app_state.rs +++ b/crates/re_viewer/src/app_state.rs @@ -9,7 +9,7 @@ use re_viewer_context::{ RecordingConfig, SpaceViewClassRegistry, StoreContext, SystemCommandSender as _, ViewerContext, }; use re_viewport::{ - identify_entities_per_system_per_class, SpaceInfoCollection, Viewport, ViewportBlueprint, + determine_visualizable_entities, SpaceInfoCollection, Viewport, ViewportBlueprint, ViewportState, }; @@ -139,15 +139,10 @@ impl AppState { let rec_cfg = recording_config_entry(recording_configs, store_db.store_id().clone(), store_db); - // Gather all entities that have a matching indicator for a visualizer. - let entities_with_matching_indicator_per_visualizer = space_view_class_registry - .entities_with_matching_indicator_per_visualizer(store_db.store_id()); - - let entities_per_system_per_class = identify_entities_per_system_per_class( - &entities_with_matching_indicator_per_visualizer, - space_view_class_registry, - store_db, - ); + let applicable_entities_per_visualizer = space_view_class_registry + .applicable_entities_for_visualizer_systems(store_db.store_id()); + let indicator_matching_entities_per_visualizer = space_view_class_registry + .indicator_matching_entities_per_visualizer(store_db.store_id()); // Execute the queries for every `SpaceView` let mut query_results = { @@ -157,16 +152,32 @@ impl AppState { .space_views .values() .flat_map(|space_view| { - space_view.queries.iter().filter_map(|query| { - entities_per_system_per_class - .get(&query.space_view_class_identifier) - .map(|entities_per_system| { - ( - query.id, - query.execute_query(store_context, entities_per_system), - ) - }) - }) + // TODO(andreas): This needs to be done in a store subscriber that exists per space view (instance, not class!). + // Note that right now we determine *all* visualizable entities, not just the queried ones. + // In a store subscriber set this is fine, but on a per-frame basis it's wasteful. + let visualizable_entities = determine_visualizable_entities( + &applicable_entities_per_visualizer, + store_db, + &space_view_class_registry + .new_part_collection(*space_view.class_identifier()), + space_view.class(space_view_class_registry), + &space_view.space_origin, + ); + + space_view + .queries + .iter() + .map(|query| { + ( + query.id, + query.execute_query( + store_context, + &visualizable_entities, + &indicator_matching_entities_per_visualizer, + ), + ) + }) + .collect::>() }) .collect::<_>() }; @@ -178,9 +189,8 @@ impl AppState { component_ui_registry, store_db, store_context, - entities_per_system_per_class: &entities_per_system_per_class, - entities_with_matching_indicator_per_visualizer: - &entities_with_matching_indicator_per_visualizer, + applicable_entities_per_visualizer: &applicable_entities_per_visualizer, + indicator_matching_entities_per_visualizer: &indicator_matching_entities_per_visualizer, query_results: &query_results, rec_cfg, re_ui, @@ -197,6 +207,7 @@ impl AppState { { re_tracing::profile_scope!("updated_query_results"); + for space_view in viewport.blueprint.space_views.values() { for query in &space_view.queries { if let Some(query_result) = query_results.get_mut(&query.id) { @@ -217,9 +228,8 @@ impl AppState { component_ui_registry, store_db, store_context, - entities_per_system_per_class: &entities_per_system_per_class, - entities_with_matching_indicator_per_visualizer: - &entities_with_matching_indicator_per_visualizer, + applicable_entities_per_visualizer: &applicable_entities_per_visualizer, + indicator_matching_entities_per_visualizer: &indicator_matching_entities_per_visualizer, query_results: &query_results, rec_cfg, re_ui, diff --git a/crates/re_viewer_context/src/lib.rs b/crates/re_viewer_context/src/lib.rs index d2de61ddfca5..04aa088e9682 100644 --- a/crates/re_viewer_context/src/lib.rs +++ b/crates/re_viewer_context/src/lib.rs @@ -17,6 +17,7 @@ mod space_view; mod store_context; mod tensor; mod time_control; +mod typed_entity_collections; mod utils; mod viewer_context; @@ -41,18 +42,20 @@ pub use selection_state::{ Selection, SelectionHighlight, }; pub use space_view::{ - AutoSpawnHeuristic, DataResult, DynSpaceViewClass, HeuristicFilterContext, - IdentifiedViewSystem, PerSystemDataResults, PerSystemEntities, PropertyOverrides, - SpaceViewClass, SpaceViewClassIdentifier, SpaceViewClassLayoutPriority, SpaceViewClassRegistry, - SpaceViewClassRegistryError, SpaceViewEntityHighlight, SpaceViewHighlights, - SpaceViewOutlineMasks, SpaceViewState, SpaceViewSystemExecutionError, - SpaceViewSystemRegistrator, SystemExecutionOutput, ViewContextCollection, ViewContextSystem, - ViewPartCollection, ViewPartSystem, ViewQuery, ViewSystemIdentifier, - VisualizerAdditionalApplicabilityFilter, + AutoSpawnHeuristic, DataResult, DynSpaceViewClass, IdentifiedViewSystem, PerSystemDataResults, + PerSystemEntities, PropertyOverrides, SpaceViewClass, SpaceViewClassIdentifier, + SpaceViewClassLayoutPriority, SpaceViewClassRegistry, SpaceViewClassRegistryError, + SpaceViewEntityHighlight, SpaceViewHighlights, SpaceViewOutlineMasks, SpaceViewState, + SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, SystemExecutionOutput, + ViewContextCollection, ViewContextSystem, ViewPartCollection, ViewPartSystem, ViewQuery, + ViewSystemIdentifier, VisualizerAdditionalApplicabilityFilter, }; pub use store_context::StoreContext; pub use tensor::{TensorDecodeCache, TensorStats, TensorStatsCache}; pub use time_control::{Looping, PlayState, TimeControl, TimeView}; +pub use typed_entity_collections::{ + ApplicableEntities, IndicatorMatchingEntities, PerVisualizer, VisualizableEntities, +}; pub use utils::{auto_color, level_to_rich_text, DefaultColor}; pub use viewer_context::{RecordingConfig, ViewerContext}; @@ -68,13 +71,6 @@ pub mod external { // --------------------------------------------------------------------------- -use nohash_hasher::{IntMap, IntSet}; -use re_log_types::EntityPath; - -pub type EntitiesPerSystem = IntMap>; - -pub type EntitiesPerSystemPerClass = IntMap; - slotmap::new_key_type! { /// Identifier for a blueprint group. pub struct DataBlueprintGroupHandle; diff --git a/crates/re_viewer_context/src/space_view/dyn_space_view_class.rs b/crates/re_viewer_context/src/space_view/dyn_space_view_class.rs index efa0728492a7..c3438bc12fb9 100644 --- a/crates/re_viewer_context/src/space_view/dyn_space_view_class.rs +++ b/crates/re_viewer_context/src/space_view/dyn_space_view_class.rs @@ -41,6 +41,9 @@ pub enum SpaceViewClassLayoutPriority { /// Each Space View in the viewer's viewport has a single class assigned immutable at its creation time. /// The class defines all aspects of its behavior. /// It determines which entities are queried, how they are rendered, and how the user can interact with them. +/// +/// TODO(andreas): Consider formulating a space view instance context object that is passed to all +/// methods that operate on concrete space views as opposed to be about general information on the class. pub trait DynSpaceViewClass: Send + Sync { /// Identifier of this space view class. /// @@ -90,10 +93,17 @@ pub trait DynSpaceViewClass: Send + Sync { /// Controls how likely this space view will get a large tile in the ui. fn layout_priority(&self) -> SpaceViewClassLayoutPriority; - /// Heuristic used to determine which space view is the best fit for a set of paths. + /// Create context object that is passed to all of this classes visualizers + /// to determine whether they can be visualized /// - /// For each path in `ent_paths`, at least one of the registered [`crate::ViewPartSystem`] for this class - /// returned true when calling [`crate::ViewPartSystem::heuristic_filter`]. + /// See [`crate::ViewPartSystem::filter_visualizable_entities`]. + fn visualizable_filter_context( + &self, + space_origin: &EntityPath, + store_db: &re_data_store::StoreDb, + ) -> Box; + + /// Heuristic used to determine which space view is the best fit for a set of paths. fn auto_spawn_heuristic( &self, _ctx: &ViewerContext<'_>, diff --git a/crates/re_viewer_context/src/space_view/mod.rs b/crates/re_viewer_context/src/space_view/mod.rs index f466377617ed..73355191e2a0 100644 --- a/crates/re_viewer_context/src/space_view/mod.rs +++ b/crates/re_viewer_context/src/space_view/mod.rs @@ -29,7 +29,7 @@ pub use space_view_class_registry::{ }; pub use system_execution_output::SystemExecutionOutput; pub use view_context_system::{ViewContextCollection, ViewContextSystem}; -pub use view_part_system::{HeuristicFilterContext, ViewPartCollection, ViewPartSystem}; +pub use view_part_system::{ViewPartCollection, ViewPartSystem}; pub use view_query::{DataResult, PerSystemDataResults, PropertyOverrides, ViewQuery}; pub use visualizer_entity_subscriber::VisualizerAdditionalApplicabilityFilter; diff --git a/crates/re_viewer_context/src/space_view/named_system.rs b/crates/re_viewer_context/src/space_view/named_system.rs index eacc96b99c7c..0cac915b34e4 100644 --- a/crates/re_viewer_context/src/space_view/named_system.rs +++ b/crates/re_viewer_context/src/space_view/named_system.rs @@ -16,6 +16,9 @@ impl Default for ViewSystemIdentifier { } } +// TODO(andreas): This should likely be `PerVisualizer` instead. +// Implying the missing concept of `SelectedEntities` which is a subset of `VisualizableEntities` +// as selected by the query. pub type PerSystemEntities = BTreeMap>; /// Trait for naming/identifying [`crate::ViewPartSystem`]s & [`crate::ViewContextSystem`]s. diff --git a/crates/re_viewer_context/src/space_view/space_view_class.rs b/crates/re_viewer_context/src/space_view/space_view_class.rs index d042237f930c..8cef66d9e25a 100644 --- a/crates/re_viewer_context/src/space_view/space_view_class.rs +++ b/crates/re_viewer_context/src/space_view/space_view_class.rs @@ -61,10 +61,19 @@ pub trait SpaceViewClass: std::marker::Sized + Send + Sync { /// Controls how likely this space view will get a large tile in the ui. fn layout_priority(&self) -> crate::SpaceViewClassLayoutPriority; - /// Heuristic used to determine which space view is the best fit for a set of paths. + /// Create context object that is passed to all of this classes visualizers + /// to determine whether they can be visualized. /// - /// For each path in `ent_paths`, at least one of the registered [`crate::ViewPartSystem`] for this class - /// returned true when calling [`crate::ViewPartSystem::heuristic_filter`]. + /// See [`crate::ViewPartSystem::filter_visualizable_entities`]. + fn visualizable_filter_context( + &self, + _space_origin: &EntityPath, + _store_db: &re_data_store::StoreDb, + ) -> Box { + Box::new(()) + } + + /// Heuristic used to determine which space view is the best fit for a set of paths. fn auto_spawn_heuristic( &self, _ctx: &ViewerContext<'_>, @@ -173,6 +182,15 @@ impl DynSpaceViewClass for T { self.layout_priority() } + #[inline] + fn visualizable_filter_context( + &self, + space_origin: &EntityPath, + store_db: &re_data_store::StoreDb, + ) -> Box { + self.visualizable_filter_context(space_origin, store_db) + } + #[inline] fn auto_spawn_heuristic( &self, diff --git a/crates/re_viewer_context/src/space_view/space_view_class_registry.rs b/crates/re_viewer_context/src/space_view/space_view_class_registry.rs index e9dc57a35c0b..8a7d535adf92 100644 --- a/crates/re_viewer_context/src/space_view/space_view_class_registry.rs +++ b/crates/re_viewer_context/src/space_view/space_view_class_registry.rs @@ -1,11 +1,10 @@ use ahash::{HashMap, HashSet}; -use nohash_hasher::{IntMap, IntSet}; use re_arrow_store::DataStore; -use re_log_types::{EntityPath, EntityPathHash}; use crate::{ - DynSpaceViewClass, IdentifiedViewSystem, SpaceViewClassIdentifier, ViewContextCollection, - ViewContextSystem, ViewPartCollection, ViewPartSystem, ViewSystemIdentifier, + ApplicableEntities, DynSpaceViewClass, IdentifiedViewSystem, IndicatorMatchingEntities, + PerVisualizer, SpaceViewClassIdentifier, ViewContextCollection, ViewContextSystem, + ViewPartCollection, ViewPartSystem, ViewSystemIdentifier, }; use super::{ @@ -278,46 +277,56 @@ impl SpaceViewClassRegistry { self.space_view_classes.values() } - /// Returns the set of entities that are applicable to the given visualizer. + /// For each visualizer, return the set of entities that is applicable to it. /// - /// The list is kept up to date by a store subscriber. - pub fn applicable_entities_for_visualizer_system( + /// The list is kept up to date by store subscribers. + pub fn applicable_entities_for_visualizer_systems( &self, - visualizer: ViewSystemIdentifier, store_id: &re_log_types::StoreId, - ) -> Option> { - self.visualizers.get(&visualizer).and_then(|entry| { - DataStore::with_subscriber::( - entry.entity_subscriber_handle, - |subscriber| subscriber.applicable_entities(store_id).cloned(), - ) - .flatten() - }) + ) -> PerVisualizer { + re_tracing::profile_function!(); + + PerVisualizer::( + self.visualizers + .iter() + .map(|(id, entry)| { + ( + *id, + DataStore::with_subscriber::( + entry.entity_subscriber_handle, + |subscriber| subscriber.applicable_entities(store_id).cloned(), + ) + .flatten() + .unwrap_or_default(), + ) + }) + .collect(), + ) } /// For each visualizer, the set of entities that have at least one matching indicator component. - pub fn entities_with_matching_indicator_per_visualizer( + pub fn indicator_matching_entities_per_visualizer( &self, store_id: &re_log_types::StoreId, - ) -> IntMap> { - self.visualizers - .iter() - .map(|(id, entry)| { - ( - *id, - DataStore::with_subscriber::( - entry.entity_subscriber_handle, - |subscriber| { - subscriber - .entities_with_matching_indicator(store_id) - .cloned() - }, + ) -> PerVisualizer { + re_tracing::profile_function!(); + + PerVisualizer::( + self.visualizers + .iter() + .map(|(id, entry)| { + ( + *id, + DataStore::with_subscriber::( + entry.entity_subscriber_handle, + |subscriber| subscriber.indicator_matching_entities(store_id).cloned(), + ) + .flatten() + .unwrap_or_default(), ) - .flatten() - .unwrap_or_default(), - ) - }) - .collect() + }) + .collect(), + ) } pub fn new_context_collection( diff --git a/crates/re_viewer_context/src/space_view/view_part_system.rs b/crates/re_viewer_context/src/space_view/view_part_system.rs index 95b138fa5e57..0becabc61f9a 100644 --- a/crates/re_viewer_context/src/space_view/view_part_system.rs +++ b/crates/re_viewer_context/src/space_view/view_part_system.rs @@ -1,43 +1,13 @@ use ahash::HashMap; -use nohash_hasher::IntSet; -use re_log_types::{EntityPath, EntityPathHash}; use re_types::ComponentNameSet; use crate::{ - IdentifiedViewSystem, SpaceViewClassIdentifier, SpaceViewSystemExecutionError, - ViewContextCollection, ViewQuery, ViewSystemIdentifier, ViewerContext, + ApplicableEntities, IdentifiedViewSystem, SpaceViewSystemExecutionError, ViewContextCollection, + ViewQuery, ViewSystemIdentifier, ViewerContext, VisualizableEntities, VisualizerAdditionalApplicabilityFilter, }; -/// This is additional context made available to the `heuristic_filter`. -/// It includes tree-context information such as whether certain components -/// exist in the parent hierarchy which are better computed once than having -/// each entity do their own tree-walk. -#[derive(Clone, Copy, Debug)] -pub struct HeuristicFilterContext { - pub class: SpaceViewClassIdentifier, - pub has_ancestor_pinhole: bool, -} - -impl Default for HeuristicFilterContext { - fn default() -> HeuristicFilterContext { - Self { - class: SpaceViewClassIdentifier::invalid(), - has_ancestor_pinhole: false, - } - } -} - -impl HeuristicFilterContext { - pub fn with_class(&self, class: SpaceViewClassIdentifier) -> Self { - Self { - class, - has_ancestor_pinhole: self.has_ancestor_pinhole, - } - } -} - /// Element of a scene derived from a single archetype query. /// /// Is populated after scene contexts and has access to them. @@ -56,22 +26,17 @@ pub trait ViewPartSystem: Send + Sync + 'static { Default::default() } - /// Implements a filter to heuristically determine whether or not to instantiate the system. - /// - /// If and when the system can be instantiated (i.e. because there is at least one entity that satisfies - /// the minimal set of required components), this method applies an arbitrary filter to determine whether - /// or not the system should be instantiated by default. + /// Filters a set of applicable entities (entities that have all required components), + /// into to a set of visualizable entities. /// - /// Override this method only if a more detailed condition is required to inform heuristics whether or not - /// the given entity is relevant for this system. + /// The context passed in here is generated by [`crate::SpaceViewClass::visualizable_filter_context`]. #[inline] - fn heuristic_filter( + fn filter_visualizable_entities( &self, - entities_with_matching_indicator: &IntSet, - ent_path: &EntityPath, - _ctx: HeuristicFilterContext, - ) -> bool { - entities_with_matching_indicator.contains(&ent_path.hash()) + entities: ApplicableEntities, + _context: &dyn std::any::Any, + ) -> VisualizableEntities { + VisualizableEntities(entities.0) } /// Additional filter for applicability. diff --git a/crates/re_viewer_context/src/space_view/visualizer_entity_subscriber.rs b/crates/re_viewer_context/src/space_view/visualizer_entity_subscriber.rs index 48aec883f543..94e15c73b742 100644 --- a/crates/re_viewer_context/src/space_view/visualizer_entity_subscriber.rs +++ b/crates/re_viewer_context/src/space_view/visualizer_entity_subscriber.rs @@ -1,13 +1,16 @@ use ahash::HashMap; use bit_vec::BitVec; use itertools::Itertools; -use nohash_hasher::{IntMap, IntSet}; +use nohash_hasher::IntMap; use re_arrow_store::StoreSubscriber; -use re_log_types::{EntityPath, EntityPathHash, StoreId}; +use re_log_types::{EntityPathHash, StoreId}; use re_types::{ComponentName, ComponentNameSet}; -use crate::{IdentifiedViewSystem, ViewPartSystem, ViewSystemIdentifier}; +use crate::{ + ApplicableEntities, IdentifiedViewSystem, IndicatorMatchingEntities, ViewPartSystem, + ViewSystemIdentifier, +}; /// A store subscriber that keep track which entities in a store can be /// processed by a single given visualizer type. @@ -73,14 +76,10 @@ struct VisualizerEntityMapping { required_component_and_filter_bitmap_per_entity: IntMap, /// Which entities the visualizer can be applied to. - /// - /// Guaranteed to not have any duplicate entries. - /// Order is not defined. - // TODO(andreas): It would be nice if these were just `EntityPathHash`. - applicable_entities: IntSet, + applicable_entities: ApplicableEntities, /// List of all entities in this store that at some point in time had any of the indicator components. - entities_with_matching_indicator: IntSet, + indicator_matching_entities: IndicatorMatchingEntities, } impl VisualizerEntitySubscriber { @@ -103,7 +102,7 @@ impl VisualizerEntitySubscriber { /// List of entities that are applicable to the visualizer. #[inline] - pub fn applicable_entities(&self, store: &StoreId) -> Option<&IntSet> { + pub fn applicable_entities(&self, store: &StoreId) -> Option<&ApplicableEntities> { self.per_store_mapping .get(store) .map(|mapping| &mapping.applicable_entities) @@ -113,13 +112,13 @@ impl VisualizerEntitySubscriber { /// /// Useful for quickly evaluating basic "should this visualizer apply by default"-heuristic. /// Does *not* imply that any of the given entities is also in the applicable-set! - pub fn entities_with_matching_indicator( + pub fn indicator_matching_entities( &self, store: &StoreId, - ) -> Option<&IntSet> { + ) -> Option<&IndicatorMatchingEntities> { self.per_store_mapping .get(store) - .map(|mapping| &mapping.entities_with_matching_indicator) + .map(|mapping| &mapping.indicator_matching_entities) } } @@ -165,7 +164,8 @@ impl StoreSubscriber for VisualizerEntitySubscriber { .any(|component_name| event.diff.cells.keys().contains(component_name)) { store_mapping - .entities_with_matching_indicator + .indicator_matching_entities + .0 .insert(entity_path_hash); } @@ -207,6 +207,7 @@ impl StoreSubscriber for VisualizerEntitySubscriber { store_mapping .applicable_entities + .0 .insert(entity_path.clone()); } } diff --git a/crates/re_viewer_context/src/typed_entity_collections.rs b/crates/re_viewer_context/src/typed_entity_collections.rs new file mode 100644 index 000000000000..07a47be00178 --- /dev/null +++ b/crates/re_viewer_context/src/typed_entity_collections.rs @@ -0,0 +1,66 @@ +//! Various strongly typed sets of entities to express intent and avoid mistakes. + +use nohash_hasher::{IntMap, IntSet}; +use re_log_types::{EntityPath, EntityPathHash}; + +use crate::ViewSystemIdentifier; + +/// List of entities that are *applicable* to a given visualizer. +/// +/// An entity is applicable if it at any point in time on any timeline has all required components. +#[derive(Default, Clone)] +pub struct ApplicableEntities(pub IntSet); + +impl std::ops::Deref for ApplicableEntities { + type Target = IntSet; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +/// List of entities that match the indicator components of a visualizer. +/// +/// In order to be a match the entity must have at some point in time on any timeline had any of +/// the indicator components specified by the respective visualizer system. +#[derive(Default, Clone)] +pub struct IndicatorMatchingEntities(pub IntSet); + +impl std::ops::Deref for IndicatorMatchingEntities { + type Target = IntSet; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +/// List of entities that can be visualized at some point in time on any timeline +/// by a concrete visualizer in the context of a specific instantiated space view. +/// +/// This is a subset of [`ApplicableEntities`] and differs on a +/// per space view instance base. +#[derive(Default)] +pub struct VisualizableEntities(pub IntSet); + +impl std::ops::Deref for VisualizableEntities { + type Target = IntSet; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +#[derive(Default)] +pub struct PerVisualizer(pub IntMap); + +impl std::ops::Deref for PerVisualizer { + type Target = IntMap; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/crates/re_viewer_context/src/viewer_context.rs b/crates/re_viewer_context/src/viewer_context.rs index 749217837155..f2679ca34c07 100644 --- a/crates/re_viewer_context/src/viewer_context.rs +++ b/crates/re_viewer_context/src/viewer_context.rs @@ -1,14 +1,12 @@ use ahash::HashMap; -use nohash_hasher::{IntMap, IntSet}; use parking_lot::RwLock; use re_data_store::{store_db::StoreDb, EntityTree, TimeHistogramPerTimeline}; -use re_log_types::EntityPathHash; use crate::{ - query_context::DataQueryResult, AppOptions, ApplicationSelectionState, Caches, CommandSender, - ComponentUiRegistry, DataQueryId, EntitiesPerSystemPerClass, Selection, SpaceViewClassRegistry, - StoreContext, TimeControl, ViewSystemIdentifier, + query_context::DataQueryResult, AppOptions, ApplicableEntities, ApplicationSelectionState, + Caches, CommandSender, ComponentUiRegistry, DataQueryId, IndicatorMatchingEntities, + PerVisualizer, Selection, SpaceViewClassRegistry, StoreContext, TimeControl, }; /// Common things needed by many parts of the viewer. @@ -35,11 +33,12 @@ pub struct ViewerContext<'a> { pub store_context: &'a StoreContext<'a>, /// Mapping from class and system to entities for the store - pub entities_per_system_per_class: &'a EntitiesPerSystemPerClass, + /// + /// TODO(andreas): This should have a generation id, allowing to update heuristics(?)/visualizable_entities etc. + pub applicable_entities_per_visualizer: &'a PerVisualizer, /// For each visualizer, the set of entities that have at least one matching indicator component. - pub entities_with_matching_indicator_per_visualizer: - &'a IntMap>, + pub indicator_matching_entities_per_visualizer: &'a PerVisualizer, /// All the query results for this frame pub query_results: &'a HashMap, diff --git a/crates/re_viewport/src/lib.rs b/crates/re_viewport/src/lib.rs index 470936c639b9..061016709699 100644 --- a/crates/re_viewport/src/lib.rs +++ b/crates/re_viewport/src/lib.rs @@ -23,12 +23,8 @@ mod viewport_blueprint_ui; /// Unstable. Used for the ongoing blueprint experimentations. pub mod blueprint; -// Transitive re-imports of blueprint dependencies. -use re_types::datatypes; - pub use space_info::SpaceInfoCollection; pub use space_view::SpaceViewBlueprint; -pub use space_view_heuristics::identify_entities_per_system_per_class; pub use viewport::{Viewport, ViewportState}; pub use viewport_blueprint::ViewportBlueprint; @@ -36,6 +32,14 @@ pub mod external { pub use re_space_view; } +use re_data_store::StoreDb; +use re_log_types::EntityPath; +use re_types::datatypes; + +use re_viewer_context::{ + ApplicableEntities, DynSpaceViewClass, PerVisualizer, VisualizableEntities, +}; + /// Utility for querying a pinhole archetype instance. /// /// TODO(andreas): It should be possible to convert `re_query::ArchetypeView` to its corresponding Archetype for situations like this. @@ -57,3 +61,36 @@ fn query_pinhole( .map(|c| c.value), }) } + +/// Determines the set of visible entities for a given space view. +// TODO(andreas): This should be part of the SpaceView's (non-blueprint) state. +// Updated whenever `applicable_entities_per_visualizer` or the space view blueprint changes. +pub fn determine_visualizable_entities( + applicable_entities_per_visualizer: &PerVisualizer, + store_db: &StoreDb, + visualizers: &re_viewer_context::ViewPartCollection, + class: &dyn DynSpaceViewClass, + space_origin: &EntityPath, +) -> PerVisualizer { + re_tracing::profile_function!(); + + let filter_ctx = class.visualizable_filter_context(space_origin, store_db); + + PerVisualizer::( + visualizers + .iter_with_identifiers() + .map(|(visualizer_identifier, visualizer_system)| { + let entities = if let Some(applicable_entities) = + applicable_entities_per_visualizer.get(&visualizer_identifier) + { + visualizer_system + .filter_visualizable_entities(applicable_entities.clone(), &filter_ctx) + } else { + VisualizableEntities::default() + }; + + (visualizer_identifier, entities) + }) + .collect(), + ) +} diff --git a/crates/re_viewport/src/space_view.rs b/crates/re_viewport/src/space_view.rs index 4709375a5582..d2987b9dd0e6 100644 --- a/crates/re_viewport/src/space_view.rs +++ b/crates/re_viewport/src/space_view.rs @@ -465,7 +465,9 @@ mod tests { use re_log_types::{DataCell, DataRow, EntityPathFilter, RowId, StoreId, TimePoint}; use re_space_view::{DataQuery as _, PropertyResolver as _}; use re_types::archetypes::Points3D; - use re_viewer_context::{EntitiesPerSystem, StoreContext}; + use re_viewer_context::{ + IndicatorMatchingEntities, PerVisualizer, StoreContext, VisualizableEntities, + }; use super::*; @@ -518,17 +520,32 @@ mod tests { let auto_properties = Default::default(); - let mut entities_per_system = EntitiesPerSystem::default(); - entities_per_system + let mut visualizable_entities = PerVisualizer::::default(); + visualizable_entities + .0 .entry("Points3D".into()) .or_insert_with(|| { - [ - EntityPath::from("parent"), - EntityPath::from("parent/skipped/child1"), - ] - .into_iter() - .collect() + VisualizableEntities( + [ + EntityPath::from("parent"), + EntityPath::from("parent/skipped/child1"), + ] + .into_iter() + .collect(), + ) }); + let indicator_matching_entities_per_visualizer = PerVisualizer::( + visualizable_entities + .0 + .iter() + .map(|(id, entities)| { + ( + *id, + IndicatorMatchingEntities(entities.iter().map(|e| e.hash()).collect()), + ) + }) + .collect(), + ); let query = space_view.queries.first().unwrap(); @@ -542,7 +559,11 @@ mod tests { all_recordings: vec![], }; - let mut query_result = query.execute_query(&ctx, &entities_per_system); + let mut query_result = query.execute_query( + &ctx, + &visualizable_entities, + &indicator_matching_entities_per_visualizer, + ); resolver.update_overrides(&ctx, &mut query_result); let parent = query_result @@ -580,7 +601,11 @@ mod tests { all_recordings: vec![], }; - let mut query_result = query.execute_query(&ctx, &entities_per_system); + let mut query_result = query.execute_query( + &ctx, + &visualizable_entities, + &indicator_matching_entities_per_visualizer, + ); resolver.update_overrides(&ctx, &mut query_result); let parent_group = query_result @@ -628,7 +653,11 @@ mod tests { all_recordings: vec![], }; - let mut query_result = query.execute_query(&ctx, &entities_per_system); + let mut query_result = query.execute_query( + &ctx, + &visualizable_entities, + &indicator_matching_entities_per_visualizer, + ); resolver.update_overrides(&ctx, &mut query_result); let parent = query_result @@ -671,7 +700,11 @@ mod tests { all_recordings: vec![], }; - let mut query_result = query.execute_query(&ctx, &entities_per_system); + let mut query_result = query.execute_query( + &ctx, + &visualizable_entities, + &indicator_matching_entities_per_visualizer, + ); resolver.update_overrides(&ctx, &mut query_result); let parent = query_result @@ -709,7 +742,11 @@ mod tests { all_recordings: vec![], }; - let mut query_result = query.execute_query(&ctx, &entities_per_system); + let mut query_result = query.execute_query( + &ctx, + &visualizable_entities, + &indicator_matching_entities_per_visualizer, + ); resolver.update_overrides(&ctx, &mut query_result); let parent = query_result diff --git a/crates/re_viewport/src/space_view_entity_picker.rs b/crates/re_viewport/src/space_view_entity_picker.rs index b1e973780a3e..4bbe6b3baeba 100644 --- a/crates/re_viewport/src/space_view_entity_picker.rs +++ b/crates/re_viewport/src/space_view_entity_picker.rs @@ -7,13 +7,8 @@ use re_log_types::{EntityPathFilter, EntityPathRule}; use re_viewer_context::{DataQueryResult, SpaceViewId, ViewerContext}; use crate::{ - space_info::SpaceInfoCollection, - space_view::SpaceViewBlueprint, - space_view_heuristics::{ - compute_heuristic_context_for_entities, is_entity_processed_by_class, - HeuristicFilterContextPerEntity, - }, - ViewportBlueprint, + determine_visualizable_entities, space_info::SpaceInfoCollection, + space_view::SpaceViewBlueprint, ViewportBlueprint, }; /// Window for adding/removing entities from a space view. @@ -66,18 +61,11 @@ fn add_entities_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui, space_view: &Spac let spaces_info = SpaceInfoCollection::new(ctx.store_db); let tree = &ctx.store_db.tree(); - let heuristic_context_per_entity = compute_heuristic_context_for_entities(ctx.store_db); // TODO(jleibs): Avoid clone let query_result = ctx.lookup_query_result(space_view.query_id()).clone(); let entity_path_filter = space_view.entity_path_filter(); - let entities_add_info = create_entity_add_info( - ctx, - tree, - &heuristic_context_per_entity, - space_view, - &query_result, - &spaces_info, - ); + let entities_add_info = + create_entity_add_info(ctx, tree, space_view, &query_result, &spaces_info); add_entities_tree_ui( ctx, @@ -339,17 +327,28 @@ struct EntityAddInfo { fn create_entity_add_info( ctx: &ViewerContext<'_>, tree: &EntityTree, - heuristic_context_per_entity: &HeuristicFilterContextPerEntity, space_view: &SpaceViewBlueprint, query_result: &DataQueryResult, spaces_info: &SpaceInfoCollection, ) -> IntMap { let mut meta_data: IntMap = IntMap::default(); + // TODO(andreas): This should be state that is already available because it's part of the space view's state. + let class = space_view.class(ctx.space_view_class_registry); + let visualizable_entities = determine_visualizable_entities( + ctx.applicable_entities_per_visualizer, + ctx.store_db, + &ctx.space_view_class_registry + .new_part_collection(class.identifier()), + class, + &space_view.space_origin, + ); + tree.visit_children_recursively(&mut |entity_path| { - let heuristic_context_per_entity = heuristic_context_per_entity.get(entity_path).copied().unwrap_or_default(); let can_add: CanAddToSpaceView = - if is_entity_processed_by_class(ctx, *space_view.class_identifier(), entity_path, heuristic_context_per_entity) { + if visualizable_entities.iter().any(|(_, entities)| entities.contains(entity_path)) { + // TODO(andreas): (topological) reachability should be part of visualizability. + // Yes, this means that once an entity is no longer visualizable (due to pinhole etc.) it stays this way. match spaces_info.is_reachable_by_transform(entity_path, &space_view.space_origin) { Ok(()) => CanAddToSpaceView::Compatible { already_added: query_result.contains_any(entity_path), diff --git a/crates/re_viewport/src/space_view_heuristics.rs b/crates/re_viewport/src/space_view_heuristics.rs index 03c28b5cce1c..1b5452a9b9f7 100644 --- a/crates/re_viewport/src/space_view_heuristics.rs +++ b/crates/re_viewport/src/space_view_heuristics.rs @@ -1,19 +1,19 @@ use ahash::HashMap; use itertools::Itertools; -use nohash_hasher::{IntMap, IntSet}; use re_arrow_store::{LatestAtQuery, Timeline}; -use re_data_store::{EntityPath, EntityTree}; -use re_log_types::{EntityPathFilter, EntityPathHash, TimeInt}; +use re_data_store::EntityPath; +use re_log_types::EntityPathFilter; use re_space_view::{DataQuery as _, DataQueryBlueprint}; use re_types::components::{DisconnectedSpace, TensorData}; use re_viewer_context::{ - AutoSpawnHeuristic, DataQueryResult, EntitiesPerSystem, EntitiesPerSystemPerClass, - HeuristicFilterContext, PerSystemEntities, SpaceViewClassIdentifier, ViewSystemIdentifier, - ViewerContext, + AutoSpawnHeuristic, DataQueryResult, PerSystemEntities, SpaceViewClassIdentifier, ViewerContext, }; -use crate::{query_pinhole, space_info::SpaceInfoCollection, space_view::SpaceViewBlueprint}; +use crate::{ + determine_visualizable_entities, query_pinhole, space_info::SpaceInfoCollection, + space_view::SpaceViewBlueprint, +}; // --------------------------------------------------------------------------- // TODO(#3079): Knowledge of specific space view classes should not leak here. @@ -53,47 +53,59 @@ fn candidate_space_view_paths<'a>( pub fn all_possible_space_views( ctx: &ViewerContext<'_>, spaces_info: &SpaceInfoCollection, - entities_per_system_per_class: &EntitiesPerSystemPerClass, ) -> Vec<(SpaceViewBlueprint, DataQueryResult)> { re_tracing::profile_function!(); // For each candidate, create space views for all possible classes. candidate_space_view_paths(ctx, spaces_info) .flat_map(|candidate_space_path| { - let reachable_entities = - reachable_entities_from_root(candidate_space_path, spaces_info); - if reachable_entities.is_empty() { - return Vec::new(); - } - - entities_per_system_per_class - .iter() - .filter_map(|(class_identifier, entities_per_system)| { - // We only want to run the query if at least one entity is reachable from the candidate. - if entities_per_system.values().all(|entities| { - !entities + ctx.space_view_class_registry + .iter_registry() + .filter_map(|entry| { + // We only want to run the query if there's at least one applicable entity under `candidate_space_path`. + if !entry.visualizer_system_ids.iter().any(|visualizer| { + let Some(entities) = ctx.applicable_entities_per_visualizer.get(visualizer) + else { + return false; + }; + entities .iter() .any(|entity| entity.starts_with(candidate_space_path)) }) { return None; } + let class_identifier = entry.class.identifier(); + let mut entity_path_filter = EntityPathFilter::default(); entity_path_filter.add_subtree(candidate_space_path.clone()); // TODO(#4377): The need to run a query-per-candidate for all possible candidates // is way too expensive. This needs to be optimized significantly. let candidate_query = - DataQueryBlueprint::new(*class_identifier, entity_path_filter); - - let results = - candidate_query.execute_query(ctx.store_context, entities_per_system); + DataQueryBlueprint::new(class_identifier, entity_path_filter); + + let visualizable_entities = determine_visualizable_entities( + ctx.applicable_entities_per_visualizer, + ctx.store_db, + &ctx.space_view_class_registry + .new_part_collection(class_identifier), + entry.class.as_ref(), + candidate_space_path, + ); + + let results = candidate_query.execute_query( + ctx.store_context, + &visualizable_entities, + ctx.indicator_matching_entities_per_visualizer, + ); if !results.is_empty() { Some(( SpaceViewBlueprint::new( - *class_identifier, - ctx.space_view_class_registry.display_name(class_identifier), + entry.class.identifier(), + ctx.space_view_class_registry + .display_name(&class_identifier), candidate_space_path, candidate_query, ), @@ -181,12 +193,11 @@ fn is_interesting_space_view_not_at_root( pub fn default_created_space_views( ctx: &ViewerContext<'_>, spaces_info: &SpaceInfoCollection, - entities_per_system_per_class: &EntitiesPerSystemPerClass, ) -> Vec { re_tracing::profile_function!(); let store = ctx.store_db.store(); - let candidates = all_possible_space_views(ctx, spaces_info, entities_per_system_per_class); + let candidates = all_possible_space_views(ctx, spaces_info); // All queries are "right most" on the log timeline. let query = LatestAtQuery::latest(Timeline::log_time()); @@ -206,13 +217,6 @@ pub fn default_created_space_views( // Main pass through all candidates. // We first check if a candidate is "interesting" and then split it up/modify it further if required. for (candidate, query_result) in candidates { - let Some(_entities_per_system_for_class) = - entities_per_system_per_class.get(candidate.class_identifier()) - else { - // Should never reach this, but if we would there would be no entities in this candidate so skipping makes sense. - continue; - }; - // TODO(#4377): Can spawn heuristics consume the query_result directly? let mut per_system_entities = PerSystemEntities::default(); { @@ -402,168 +406,3 @@ pub fn default_created_space_views( space_views.into_iter().map(|(s, _)| s).collect() } - -pub fn reachable_entities_from_root( - root: &EntityPath, - spaces_info: &SpaceInfoCollection, -) -> Vec { - re_tracing::profile_function!(); - - let mut entities = Vec::new(); - let space_info = spaces_info.get_first_parent_with_info(root); - - if &space_info.path == root { - space_info.visit_descendants_with_reachable_transform(spaces_info, &mut |space_info| { - entities.extend(space_info.descendants_without_transform.iter().cloned()); - }); - } else { - space_info.visit_descendants_with_reachable_transform(spaces_info, &mut |space_info| { - entities.extend( - space_info - .descendants_without_transform - .iter() - .filter(|ent_path| ent_path.starts_with(root)) - .cloned(), - ); - }); - } - - entities -} - -// TODO(andreas): Still used in a bunch of places. Should instead use the global `EntitiesPerSystemPerClass` list. -pub fn is_entity_processed_by_class( - ctx: &ViewerContext<'_>, - class: SpaceViewClassIdentifier, - ent_path: &EntityPath, - heuristic_ctx: HeuristicFilterContext, -) -> bool { - let parts = &ctx.space_view_class_registry.new_part_collection(class); - let heuristic_ctx = heuristic_ctx.with_class(class); - - let empty_entity_set = IntSet::default(); - for (id, visualizer) in parts.iter_with_identifiers() { - let entities_with_matching_indicator = ctx - .entities_with_matching_indicator_per_visualizer - .get(&id) - .unwrap_or(&empty_entity_set); - - if visualizer.heuristic_filter(entities_with_matching_indicator, ent_path, heuristic_ctx) { - return true; - } - } - - false -} - -pub type HeuristicFilterContextPerEntity = IntMap; - -pub fn compute_heuristic_context_for_entities( - store_db: &re_data_store::store_db::StoreDb, -) -> HeuristicFilterContextPerEntity { - let mut heuristic_context = IntMap::default(); - - // Use "right most"/latest available data. - let timeline = Timeline::log_time(); - let query_time = TimeInt::MAX; - let query = LatestAtQuery::new(timeline, query_time); - - let tree = &store_db.tree(); - - fn visit_children_recursively( - has_parent_pinhole: bool, - tree: &EntityTree, - store: &re_arrow_store::DataStore, - query: &LatestAtQuery, - heuristic_context: &mut HeuristicFilterContextPerEntity, - ) { - let has_parent_pinhole = - has_parent_pinhole || query_pinhole(store, query, &tree.path).is_some(); - - heuristic_context.insert( - tree.path.clone(), - HeuristicFilterContext { - class: SpaceViewClassIdentifier::invalid(), - has_ancestor_pinhole: has_parent_pinhole, - }, - ); - - for child in tree.children.values() { - visit_children_recursively(has_parent_pinhole, child, store, query, heuristic_context); - } - } - - visit_children_recursively( - false, - tree, - store_db.data_store(), - &query, - &mut heuristic_context, - ); - heuristic_context -} - -pub fn identify_entities_per_system_per_class( - entities_with_matching_indicator_per_visualizer: &IntMap< - ViewSystemIdentifier, - IntSet, - >, - space_view_class_registry: &re_viewer_context::SpaceViewClassRegistry, - store_db: &re_data_store::store_db::StoreDb, -) -> EntitiesPerSystemPerClass { - re_tracing::profile_function!(); - - let store = store_db.store().id(); - let heuristic_context = compute_heuristic_context_for_entities(store_db); - - let empty_entity_set = IntSet::default(); - - space_view_class_registry - .iter_registry() - .map(|entry| { - let mut entities_per_system = EntitiesPerSystem::default(); - let class_id = entry.class.identifier(); - - // TODO(andreas): Once `heuristic_filter` is no longer applied, we don't need to instantiate the systems anymore. - //for system_id in &entry.visualizer_system_ids { - for (system_id, system) in space_view_class_registry - .new_part_collection(class_id) - .systems - { - let entities_with_matching_indicator = - entities_with_matching_indicator_per_visualizer - .get(&system_id) - .unwrap_or(&empty_entity_set); - - let entities: IntSet = if let Some(entities) = space_view_class_registry - .applicable_entities_for_visualizer_system(system_id, store) - { - // TODO(andreas): Don't apply heuristic_filter here, this should be part of the query! - // Note that once this is done, `EntitiesPerSystemPerClass` becomes just `EntitiesPerSystem` since there's never a need to distinguish per class! - entities - .into_iter() - .filter(|ent_path| { - system.heuristic_filter( - entities_with_matching_indicator, - ent_path, - heuristic_context - .get(ent_path) - .copied() - .unwrap_or_default() - .with_class(class_id), - ) - }) - .collect() - } else { - Default::default() - }; - - entities_per_system.insert(system_id, entities); - } - - // TODO(#4377): Handle context systems but keep them parallel - - (class_id, entities_per_system) - }) - .collect() -} diff --git a/crates/re_viewport/src/viewport.rs b/crates/re_viewport/src/viewport.rs index 513407651527..3808808446a4 100644 --- a/crates/re_viewport/src/viewport.rs +++ b/crates/re_viewport/src/viewport.rs @@ -254,9 +254,7 @@ impl<'a, 'b> Viewport<'a, 'b> { if self.blueprint.auto_space_views { let mut new_space_views = vec![]; - for space_view_candidate in - default_created_space_views(ctx, spaces_info, ctx.entities_per_system_per_class) - { + for space_view_candidate in default_created_space_views(ctx, spaces_info) { if self.should_auto_add_space_view(&new_space_views, &space_view_candidate) { new_space_views.push(space_view_candidate); } diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs index a0e0a1e8ab02..689ac6fb1b2c 100644 --- a/crates/re_viewport/src/viewport_blueprint_ui.rs +++ b/crates/re_viewport/src/viewport_blueprint_ui.rs @@ -423,8 +423,7 @@ impl Viewport<'_, '_> { }; // Space view options proposed by heuristics - let mut possible_space_views = - all_possible_space_views(ctx, spaces_info, ctx.entities_per_system_per_class); + let mut possible_space_views = all_possible_space_views(ctx, spaces_info); possible_space_views .sort_by_key(|(space_view, _)| space_view.space_origin.to_string());