Fix multi-entity drag-and-drop only adding one entity

crates/viewer/re_viewport/src/viewport_ui.rs

@@ -6,7 +6,7 @@ use ahash::HashMap;
 use egui_tiles::{Behavior as _, EditAction};
 
 use re_context_menu::{context_menu_ui_for_item, SelectionUpdateBehavior};
-use re_log_types::{EntityPath, EntityPathRule};
+use re_log_types::{EntityPath, EntityPathRule, RuleEffect};
 use re_ui::{design_tokens, ContextExt as _, DesignTokens, Icon, UiExt as _};
 use re_viewer_context::{
     blueprint_id_to_tile_id, icon_for_container_kind, Contents, DragAndDropFeedback,
@@ -277,14 +277,27 @@ impl ViewportUi {
         if ctx.egui_ctx.input(|i| i.pointer.any_released()) {
             egui::DragAndDrop::clear_payload(ctx.egui_ctx);
 
-            for entity in entities {
-                if can_entity_be_added(entity) {
-                    view_blueprint.contents.raw_add_entity_inclusion(
-                        ctx,
-                        EntityPathRule::including_subtree(entity.clone()),
-                    );
-                }
-            }
+            view_blueprint
+                .contents
+                .mutate_entity_path_filter(ctx, |filter| {
+                    for entity in entities {
+                        if can_entity_be_added(entity) {
+                            filter.add_rule(
+                                RuleEffect::Include,
+                                EntityPathRule::including_subtree(entity.clone()),
+                            );
+                        }
+                    }
+                });
+
+            // for entity in entities {
+            //     if can_entity_be_added(entity) {
+            //         view_blueprint.contents.raw_add_entity_inclusion(
+            //             ctx,
+            //             EntityPathRule::including_subtree(entity.clone()),
+            //         );
+            //     }
+            // }
 
             ctx.selection_state()
                 .set_selection(Item::View(view_blueprint.id));

crates/viewer/re_viewport_blueprint/src/view_contents.rs

@@ -142,6 +142,9 @@ impl ViewContents {
         );
     }
 
+    /// Set the entity path filter. WARNING: a single mutation is allowed per frame, or data will be
+    /// lost.
+    //TODO(#8518): address this massive foot-gun.
     pub fn set_entity_path_filter(
         &self,
         ctx: &ViewerContext<'_>,
@@ -184,40 +187,65 @@ impl ViewContents {
         }
     }
 
-    /// Remove a subtree and any existing rules that it would match.
+    /// Perform arbitrary mutation on the entity path filter. WARNING: a single mutation is allowed
+    /// per frame, or data will be lost.
+    ///
+    /// This method exists because of the single mutation per frame limitation. It currently is the
+    /// only way to perform multiple mutations on the entity path filter in a single frame.
+    //TODO(#8518): address this massive foot-gun.
+    pub fn mutate_entity_path_filter(
+        &self,
+        ctx: &ViewerContext<'_>,
+        f: impl FnOnce(&mut EntityPathFilter),
+    ) {
+        let mut new_entity_path_filter = self.entity_path_filter.clone();
+        f(&mut new_entity_path_filter);
+        self.set_entity_path_filter(ctx, &new_entity_path_filter);
+    }
+
+    /// Remove a subtree and any existing rules that it would match. WARNING: a single mutation is
+    /// allowed per frame, or data will be lost.
     ///
     /// Because most-specific matches win, if we only add a subtree exclusion
     /// it can still be overridden by existing inclusions. This method ensures
     /// that not only do we add a subtree exclusion, but clear out any existing
     /// inclusions or (now redundant) exclusions that would match the subtree.
+    //TODO(#8518): address this massive foot-gun.
     pub fn remove_subtree_and_matching_rules(&self, ctx: &ViewerContext<'_>, path: EntityPath) {
         let mut new_entity_path_filter = self.entity_path_filter.clone();
         new_entity_path_filter.remove_subtree_and_matching_rules(path);
         self.set_entity_path_filter(ctx, &new_entity_path_filter);
     }
 
-    /// Directly add an exclusion rule to the [`EntityPathFilter`].
+    /// Directly add an exclusion rule to the [`EntityPathFilter`]. WARNING: a single mutation is
+    /// allowed per frame, or data will be lost.
     ///
     /// This is a direct modification of the filter and will not do any simplification
     /// related to overlapping or conflicting rules.
     ///
     /// If you are trying to remove an entire subtree, prefer using [`Self::remove_subtree_and_matching_rules`].
+    //TODO(#8518): address this massive foot-gun.
     pub fn raw_add_entity_exclusion(&self, ctx: &ViewerContext<'_>, rule: EntityPathRule) {
         let mut new_entity_path_filter = self.entity_path_filter.clone();
         new_entity_path_filter.add_rule(RuleEffect::Exclude, rule);
         self.set_entity_path_filter(ctx, &new_entity_path_filter);
     }
 
-    /// Directly add an inclusion rule to the [`EntityPathFilter`].
+    /// Directly add an inclusion rule to the [`EntityPathFilter`]. WARNING: a single mutation is
+    /// allowed per frame, or data will be lost.
     ///
     /// This is a direct modification of the filter and will not do any simplification
     /// related to overlapping or conflicting rules.
+    //TODO(#8518): address this massive foot-gun.
     pub fn raw_add_entity_inclusion(&self, ctx: &ViewerContext<'_>, rule: EntityPathRule) {
         let mut new_entity_path_filter = self.entity_path_filter.clone();
         new_entity_path_filter.add_rule(RuleEffect::Include, rule);
         self.set_entity_path_filter(ctx, &new_entity_path_filter);
     }
 
+    /// Remove rules for a given entity. WARNING: a single mutation is allowed per frame, or data
+    /// will be lost.
+    //TODO(#8518): address this massive foot-gun.
     pub fn remove_filter_rule_for(&self, ctx: &ViewerContext<'_>, ent_path: &EntityPath) {
         let mut new_entity_path_filter = self.entity_path_filter.clone();
         new_entity_path_filter.remove_rule_for(ent_path);

tests/python/release_checklist/check_multi_entity_drag_and_drop.py

@@ -0,0 +1,60 @@
+from __future__ import annotations
+
+import os
+from argparse import Namespace
+from uuid import uuid4
+
+import numpy as np
+import rerun as rr
+import rerun.blueprint as rrb
+
+README = """\
+# Multi-entity drag-and-drop
+
+This test checks that dragging multiple entities to a view correctly adds all entities.
+
+1. Multi-select `cos_curve` and `line_curve` entities in the streams tree.
+2. Drag them to the PLOT view.
+3. _Expect_: both entities are visible in the plot view and each are listed in the view's entity path filter.
+"""
+
+
+def log_readme() -> None:
+    rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True)
+
+
+def blueprint() -> rrb.BlueprintLike:
+    return rrb.Vertical(
+        rrb.TextDocumentView(origin="readme"),
+        rrb.TimeSeriesView(origin="/", contents=[], name="PLOT"),
+    )
+
+
+def log_some_scalar_entities() -> None:
+    times = np.arange(100)
+    curves = [
+        ("cos_curve", np.cos(times / 100 * 2 * np.pi)),
+        ("line_curve", times / 100 + 0.2),
+    ]
+
+    time_column = rr.TimeSequenceColumn("frame", times)
+
+    for path, curve in curves:
+        rr.send_columns(path, times=[time_column], components=[rr.components.ScalarBatch(curve)])
+
+
+def run(args: Namespace) -> None:
+    rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4())
+    rr.send_blueprint(blueprint(), make_default=True, make_active=True)
+
+    log_readme()
+    log_some_scalar_entities()
+
+
+if __name__ == "__main__":
+    import argparse
+
+    parser = argparse.ArgumentParser(description="Interactive release checklist")
+    rr.script_add_args(parser)
+    args = parser.parse_args()
+    run(args)