Separation & improved book-keeping of entity -> visualizer assignment

.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"
     },

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<VisualizableEntities>,
+        indicator_matching_entities_per_visualizer: &PerVisualizer<IndicatorMatchingEntities>,
     ) -> DataQueryResult;
 }

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<VisualizableEntities>,
+        indicator_matching_entities_per_visualizer: &PerVisualizer<IndicatorMatchingEntities>,
     ) -> DataQueryResult {
         re_tracing::profile_function!();
 
         let mut data_results = SlotMap::<DataResultHandle, DataResultNode>::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<VisualizableEntities>,
+    indicator_matching_entities_per_visualizer:
+        &'a IntMap<ViewSystemIdentifier, IndicatorMatchingEntities>,
     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<VisualizableEntities>,
+        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<EntityPathHash>`
+                        //   -> 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::<VisualizableEntities>::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::<IndicatorMatchingEntities>(
+                    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| {

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",

crates/re_space_view_spatial/src/parts/boxes2d.rs

@@ -1,15 +1,14 @@
-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,
     components::{HalfSizes2D, Position2D, Text},
     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<EntityPathHash>,
-        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(

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<EntityPathHash>,
-        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(