fix previously filtered recommendations showing up later again

crates/re_log_types/src/path/entity_path_filter.rs

@@ -46,6 +46,12 @@ impl std::hash::Hash for EntityPathFilter {
     }
 }
 
+impl std::fmt::Debug for EntityPathFilter {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "EntityPathFilter({:?})", self.formatted())
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct EntityPathRule {
     pub path: EntityPath,

crates/re_types/definitions/rerun/blueprint/archetypes/viewport_blueprint.fbs

@@ -39,7 +39,7 @@ table ViewportBlueprint (
     ///
     /// True if not specified, meaning that if the Viewer deems it necessary to add new Space Views to cover
     /// all logged entities appropriately, it will do so unless they were added previously
-    /// (as identified by `viewer_recommendation_hashes`).
+    /// (as identified by `past_viewer_recommendations`).
     auto_space_views: rerun.blueprint.components.AutoSpaceViews ("attr.rerun.component_optional", nullable, order: 5000);
 
     /// Hashes of all recommended space views the viewer has already added and that should not be added again.
@@ -48,5 +48,5 @@ table ViewportBlueprint (
     /// If you want the viewer from stopping to add space views, you should set `auto_space_views` to `false`.
     ///
     /// The viewer uses this to determine whether it should keep adding space views.
-    viewer_recommendation_hashes: [rerun.blueprint.components.ViewerRecommendationHash] ("attr.rerun.component_optional", nullable, order: 6000);
+    past_viewer_recommendations: [rerun.blueprint.components.ViewerRecommendationHash] ("attr.rerun.component_optional", nullable, order: 6000);
 }

crates/re_viewer_context/src/space_view/spawn_heuristics.rs

@@ -1,7 +1,7 @@
 use re_log_types::{EntityPath, EntityPathFilter};
 
 /// Properties of a space view that as recommended to be spawned by default via space view spawn heuristics.
-#[derive(Hash)]
+#[derive(Hash, Debug)]
 pub struct RecommendedSpaceView {
     pub root: EntityPath,
     pub query_filter: EntityPathFilter,

crates/re_viewport/src/viewport_blueprint.rs

@@ -58,7 +58,7 @@ pub struct ViewportBlueprint {
     auto_space_views: AtomicBool,
 
     /// Hashes of all recommended space views the viewer has already added and that should not be added again.
-    viewer_recommendation_hashes: IntSet<Hash64>,
+    past_viewer_recommendation_hashes: IntSet<Hash64>,
 
     /// Channel to pass Blueprint mutation messages back to the [`crate::Viewport`]
     tree_action_sender: std::sync::mpsc::Sender<TreeAction>,
@@ -79,7 +79,7 @@ impl ViewportBlueprint {
             maximized,
             auto_layout,
             auto_space_views,
-            viewer_recommendation_hashes,
+            past_viewer_recommendations: past_viewer_recommendation_hashes,
         } = match query_archetype(blueprint_db.store(), query, &VIEWPORT_PATH.into())
             .and_then(|arch| arch.to_archetype())
         {
@@ -144,7 +144,7 @@ impl ViewportBlueprint {
             root_container,
         );
 
-        let viewer_recommendation_hashes = viewer_recommendation_hashes
+        let past_viewer_recommendation_hashes = past_viewer_recommendation_hashes
             .unwrap_or_default()
             .into_iter()
             .map(|h| Hash64::from_u64(h.0 .0))
@@ -158,7 +158,7 @@ impl ViewportBlueprint {
             maximized: maximized.map(|id| id.0.into()),
             auto_layout: auto_layout.unwrap_or_default().0.into(),
             auto_space_views: auto_space_views.into(),
-            viewer_recommendation_hashes,
+            past_viewer_recommendation_hashes,
             tree_action_sender,
         }
     }
@@ -296,10 +296,39 @@ impl ViewportBlueprint {
             // Remove all space views that we already spawned via heuristic before.
             recommended_space_views.retain(|recommended_view| {
                 !self
-                    .viewer_recommendation_hashes
+                    .past_viewer_recommendation_hashes
                     .contains(&Hash64::hash(recommended_view))
             });
 
+            // Each of the remaining recommendations would individually be a candidate for spawning if there were
+            // no other space views in the viewport.
+            // In the following steps we further filter this list depending on what's on screen already,
+            // as well as redundancy within the recommendation itself BUT this is an important checkpoint:
+            // All the other views may change due to user interaction, but this does *not* mean
+            // that we should suddenly spawn the views we're filtering out here.
+            // Therefore everything so far needs to be added to `past_viewer_recommendations`,
+            // which marks this as "already processed recommendation".
+            //
+            // Example:
+            // Recommendation contains `/**` and `/camera/**`.
+            // We filter out `/camera/**` because that would be redundant to `/**`.
+            // If now the user edits the space view at `/**` to be `/points/**`, that does *not*
+            // mean we should suddenly add `/camera/**` to the viewport.
+            if !recommended_space_views.is_empty() {
+                let new_viewer_recommendation_hashes = self
+                    .past_viewer_recommendation_hashes
+                    .iter()
+                    .cloned()
+                    .chain(recommended_space_views.iter().map(Hash64::hash))
+                    .map(|hash| ViewerRecommendationHash(hash.hash64().into()))
+                    .collect::<Vec<_>>();
+
+                ctx.save_blueprint_component(
+                    &VIEWPORT_PATH.into(),
+                    &new_viewer_recommendation_hashes,
+                );
+            }
+
             // Remove all space views that have all the entities we already have on screen.
             let existing_path_filters = self
                 .space_views
@@ -326,36 +355,20 @@ impl ViewportBlueprint {
                             i == *j || !other.query_filter.is_superset_of(&candidate.query_filter)
                         })
                 })
-                .map(|(_, recommendation)| recommendation)
-                .collect::<Vec<_>>();
-
-            if !final_recommendations.is_empty() {
-                self.add_space_views(
-                    final_recommendations.iter().map(|recommendation| {
-                        SpaceViewBlueprint::new(
-                            class_id,
-                            &recommendation.root,
-                            recommendation.query_filter.clone(),
-                        )
-                    }),
-                    ctx,
-                    None,
-                    None,
-                );
-
-                // Finally, save the added recommendations to the store.
-                let new_viewer_recommendation_hashes = self
-                    .viewer_recommendation_hashes
-                    .iter()
-                    .cloned()
-                    .chain(final_recommendations.iter().map(Hash64::hash))
-                    .map(|hash| ViewerRecommendationHash(hash.hash64().into()))
-                    .collect::<Vec<_>>();
-                ctx.save_blueprint_component(
-                    &VIEWPORT_PATH.into(),
-                    &new_viewer_recommendation_hashes,
-                );
-            }
+                .map(|(_, recommendation)| recommendation);
+
+            self.add_space_views(
+                final_recommendations.map(|recommendation| {
+                    SpaceViewBlueprint::new(
+                        class_id,
+                        &recommendation.root,
+                        recommendation.query_filter.clone(),
+                    )
+                }),
+                ctx,
+                None,
+                None,
+            );
         }
     }