diff --git a/crates/viewer/re_viewport/src/viewport_ui.rs b/crates/viewer/re_viewport/src/viewport_ui.rs index 602c57a7f67b..b145e58fe925 100644 --- a/crates/viewer/re_viewport/src/viewport_ui.rs +++ b/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)); diff --git a/crates/viewer/re_viewport_blueprint/src/view_contents.rs b/crates/viewer/re_viewport_blueprint/src/view_contents.rs index ef4079f1f515..7988a7c9d65a 100644 --- a/crates/viewer/re_viewport_blueprint/src/view_contents.rs +++ b/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); diff --git a/tests/python/release_checklist/check_multi_entity_drag_and_drop.py b/tests/python/release_checklist/check_multi_entity_drag_and_drop.py new file mode 100644 index 000000000000..e541458b3881 --- /dev/null +++ b/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)