Clean up TODOs to reference #4377

rerun-io · Nov 29, 2023 · 6135a38 · 6135a38
1 parent daf7fe7
commit 6135a38
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 18 deletions.
diff --git a/crates/re_viewer/src/ui/selection_history_ui.rs b/crates/re_viewer/src/ui/selection_history_ui.rs
@@ -177,10 +177,7 @@ fn item_to_string(blueprint: &ViewportBlueprint<'_>, item: &Item) -> String {
             }
         }
         Item::InstancePath(_, entity_path) => entity_path.to_string(),
-        Item::DataBlueprintGroup(_sid, _qid, entity_path) => {
-            // TODO(jleibs): How do we access the query in this context?
-            entity_path.to_string()
-        }
+        Item::DataBlueprintGroup(_sid, _qid, entity_path) => entity_path.to_string(),
         Item::ComponentPath(path) => {
             format!("{} {}", path.entity_path, path.component_name.short_name(),)
         }

diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs
@@ -308,7 +308,7 @@ fn list_existing_data_blueprints(
     entity_path: &EntityPath,
     blueprint: &ViewportBlueprint<'_>,
 ) {
-    let space_views_with_path = blueprint.space_views_containing_entity_path(entity_path);
+    let space_views_with_path = blueprint.space_views_containing_entity_path(ctx, entity_path);
 
     if space_views_with_path.is_empty() {
         ui.weak("(Not shown in any Space View)");

diff --git a/crates/re_viewport/src/space_view.rs b/crates/re_viewport/src/space_view.rs
@@ -161,7 +161,7 @@ impl SpaceViewBlueprint {
 
         let query_result = ctx.lookup_query_result(self.query_id()).clone();
 
-        // TODO(jleibs): Use PerSystemDataResults?
+        // TODO(#4377): Use PerSystemDataResults
         let mut per_system_entities = PerSystemEntities::default();
         {
             re_tracing::profile_scope!("per_system_data_results");

diff --git a/crates/re_viewport/src/space_view_entity_picker.rs b/crates/re_viewport/src/space_view_entity_picker.rs
@@ -177,6 +177,7 @@ fn add_entities_line_ui(
     _space_view: &mut SpaceViewBlueprint,
     _entities_add_info: &IntMap<EntityPath, EntityAddInfo>,
 ) {
+    // TODO(#4377): Reformulate this in terms of modifying query expressions
     ui.label("Not implemented");
     /*
     ui.horizontal(|ui| {
@@ -322,6 +323,7 @@ fn create_entity_add_info(
         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_name(), entity_path, heuristic_context_per_entity, &ctx.current_query()) {
+                // TODO(#4377): Reformulate this in terms of modifying query expressions
                 CanAddToSpaceView::No {
                     reason: "Not implemented".to_owned(),
                 }

diff --git a/crates/re_viewport/src/space_view_heuristics.rs b/crates/re_viewport/src/space_view_heuristics.rs
@@ -108,6 +108,8 @@ pub fn all_possible_space_views(
             entities_used_by_any_part_system_of_class
                 .iter()
                 .filter_map(|(class_name, _entities_used_by_any_part_system)| {
+                    // 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_name,
                         std::iter::once(&EntityPathExpr::Recursive(candidate_space_path.clone())),
@@ -242,7 +244,7 @@ pub fn default_created_space_views(
             continue;
         };
 
-        // TODO(jleibs): Can spawn heuristics consume the query_result directly?
+        // TODO(#4377): Can spawn heuristics consume the query_result directly?
         let mut per_system_entities = PerSystemEntities::default();
         {
             re_tracing::profile_scope!("per_system_data_results");

diff --git a/crates/re_viewport/src/space_view_highlights.rs b/crates/re_viewport/src/space_view_highlights.rs
@@ -41,7 +41,7 @@ pub fn highlights_for_space_view(
             Item::ComponentPath(_) | Item::SpaceView(_) => {}
 
             Item::DataBlueprintGroup(_space_view_id, _query_id, _entity_path) => {
-                // TODO(jleibs): Fix DataBlueprintGroup
+                // TODO(#4377): Fix DataBlueprintGroup
                 /*
                 if *group_space_view_id == space_view_id {
                     if let Some(space_view) = space_views.get(group_space_view_id) {
@@ -118,7 +118,7 @@ pub fn highlights_for_space_view(
             Item::ComponentPath(_) | Item::SpaceView(_) => {}
 
             Item::DataBlueprintGroup(_space_view_id, _query_id, _entity_path) => {
-                // TODO(jleibs): Fix DataBlueprintGroup
+                // TODO(#4377): Fix DataBlueprintGroup
                 /*
                 // Unlike for selected objects/data we are more picky for data blueprints with our hover highlights
                 // since they are truly local to a space view.

diff --git a/crates/re_viewport/src/viewport_blueprint.rs b/crates/re_viewport/src/viewport_blueprint.rs
@@ -227,21 +227,26 @@ impl<'a> ViewportBlueprint<'a> {
     }
 
     #[allow(clippy::unused_self)]
-    pub fn space_views_containing_entity_path(&self, _path: &EntityPath) -> Vec<SpaceViewId> {
-        // TODO(jleibs): Need to search for entity path in query-results
-        /*
+    pub fn space_views_containing_entity_path(
+        &self,
+        ctx: &ViewerContext<'_>,
+        path: &EntityPath,
+    ) -> Vec<SpaceViewId> {
         self.space_views
             .iter()
             .filter_map(|(space_view_id, space_view)| {
-                if space_view.contents.contains_entity(path) {
+                let query_result = ctx.lookup_query_result(space_view.query_id());
+                if query_result
+                    .tree
+                    .lookup_result_by_path_and_group(path, false)
+                    .is_some()
+                {
                     Some(*space_view_id)
                 } else {
                     None
                 }
             })
             .collect()
-            */
-        vec![]
     }
 
     /// Compares the before and after snapshots and sends any necessary deltas to the store.
@@ -424,7 +429,6 @@ pub fn sync_space_view(
 
         add_delta_from_single_component(deltas, &space_view.entity_path(), &timepoint, component);
 
-        // TODO(jleibs): Query-removal logic
         for query in &space_view.queries {
             add_delta_from_single_component(
                 deltas,

diff --git a/crates/re_viewport/src/viewport_blueprint_ui.rs b/crates/re_viewport/src/viewport_blueprint_ui.rs
@@ -311,7 +311,7 @@ impl ViewportBlueprint<'_> {
                         let response =
                             remove_button_ui(re_ui, ui, "Remove Entity from the Space View");
                         if response.clicked() {
-                            // TODO(jleibs): Fix removal
+                            // TODO(#4377): Fix entity removal
                             //space_view.contents.remove_entity(entity_path);
                             space_view.entities_determined_by_user = true;
                         }
@@ -374,7 +374,7 @@ impl ViewportBlueprint<'_> {
                     });
 
                 if remove_group {
-                    // TODO(jleibs): Fix removal
+                    // TODO(#4377): Fix group removal
                     /*
                     if let Some(group_handle) = space_view
                         .contents