From e38e4196f58ce668e3026726335ccab50fd1b12c Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 14 Jan 2025 11:51:17 +0100 Subject: [PATCH 01/13] Filter widget ui return response for title (if shown) --- crates/viewer/re_ui/src/filter_widget.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_ui/src/filter_widget.rs b/crates/viewer/re_ui/src/filter_widget.rs index b75d8d553804..fd5f658954cc 100644 --- a/crates/viewer/re_ui/src/filter_widget.rs +++ b/crates/viewer/re_ui/src/filter_widget.rs @@ -76,7 +76,11 @@ impl FilterState { /// /// Note: this uses [`egui::Ui::available_width`] to determine the location of the right-aligned /// search button, as usual for [`list_item::ListItem`]-based widgets. - pub fn ui(&mut self, ui: &mut egui::Ui, section_title: impl Into) { + pub fn ui( + &mut self, + ui: &mut egui::Ui, + section_title: impl Into, + ) -> Option { let mut button_clicked = false; let icon = if self.inner_state.is_none() { @@ -95,6 +99,8 @@ impl FilterState { ); let text_width = galley.size().x; + let mut title_response = None; + list_item::list_item_scope(ui, ui.next_auto_id(), |ui| { ui.list_item() .interactive(false) @@ -126,7 +132,7 @@ impl FilterState { response.request_focus(); } } else { - ui.label(galley); + title_response = Some(ui.label(galley)); } }) .with_content_width(text_width) @@ -145,6 +151,8 @@ impl FilterState { self.inner_state = None; } } + + title_response } } From 7848cad938e3eda3f0aacedac34d26ceecdbbdc8 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 14 Jan 2025 11:51:25 +0100 Subject: [PATCH 02/13] WIP: widget filter in selection panel --- .../re_blueprint_tree/src/blueprint_tree.rs | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index f88326f5ae72..104e808377cc 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -7,7 +7,9 @@ use re_context_menu::{context_menu_ui_for_item, SelectionUpdateBehavior}; use re_entity_db::InstancePath; use re_log_types::EntityPath; use re_types::blueprint::components::Visible; -use re_ui::{drag_and_drop::DropTarget, list_item, ContextExt as _, DesignTokens, UiExt as _}; +use re_ui::{ + drag_and_drop::DropTarget, filter_widget, list_item, ContextExt as _, DesignTokens, UiExt as _, +}; use re_viewer_context::{ contents_name_style, icon_for_container_kind, CollapseScope, Contents, DataResultNodeOrPath, DragAndDropFeedback, DragAndDropPayload, SystemCommandSender, @@ -38,6 +40,8 @@ pub struct BlueprintTree { /// /// We double-buffer this value to deal with ordering constraints. next_candidate_drop_parent_container_id: Option, + + filter_state: filter_widget::FilterState, } impl BlueprintTree { @@ -53,18 +57,30 @@ impl BlueprintTree { ui.add_space(-1.); ui.list_item_scope("blueprint_section_title", |ui| { - ui.list_item_flat_noninteractive( - list_item::CustomContent::new(|ui, _| { - ui.strong("Blueprint").on_hover_text( - "The blueprint is where you can configure the Rerun Viewer", - ); - }) - .menu_button(&re_ui::icons::MORE, |ui| { - add_new_view_or_container_menu_button(ctx, viewport, ui); - set_blueprint_to_default_menu_buttons(ctx, ui); - set_blueprint_to_auto_menu_button(ctx, viewport, ui); - }), - ); + //TODO: move height to design token + //TODO: make it fit 24 using no expand + 1.0 selection + ui.list_item() + .interactive(false) + .with_height(30.0) + .show_flat( + ui, + list_item::CustomContent::new(|ui, _| { + let title_response = self + .filter_state + .ui(ui, egui::RichText::new("Blueprint").strong()); + + if let Some(title_response) = title_response { + title_response.on_hover_text( + "The blueprint is where you can configure the Rerun Viewer", + ); + } + }) + .menu_button(&re_ui::icons::MORE, |ui| { + add_new_view_or_container_menu_button(ctx, viewport, ui); + set_blueprint_to_default_menu_buttons(ctx, ui); + set_blueprint_to_auto_menu_button(ctx, viewport, ui); + }), + ); }); ui.full_span_separator(); From 1790dcd7d9d4cc110fb21e92100d1a2affe73e3e Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 14 Jan 2025 14:53:59 +0100 Subject: [PATCH 03/13] Integrate filter widget to blueprint / Update streams pixels / Updated/added tests for streams --- .../re_blueprint_tree/src/blueprint_tree.rs | 43 ++++----- crates/viewer/re_time_panel/src/lib.rs | 9 +- .../tests/snapshots/time_panel_dense_data.png | 4 +- ...time_panel_filter_test_active_no_query.png | 3 + .../time_panel_filter_test_active_query.png | 3 + .../time_panel_filter_test_inactive.png | 3 + .../snapshots/time_panel_two_sections.png | 4 +- .../re_time_panel/tests/time_panel_tests.rs | 79 ++++++++++++++-- crates/viewer/re_ui/src/filter_widget.rs | 90 +++++++++++-------- 9 files changed, 162 insertions(+), 76 deletions(-) create mode 100644 crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_no_query.png create mode 100644 crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_query.png create mode 100644 crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_inactive.png diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index 104e808377cc..6926624338f6 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -57,30 +57,25 @@ impl BlueprintTree { ui.add_space(-1.); ui.list_item_scope("blueprint_section_title", |ui| { - //TODO: move height to design token - //TODO: make it fit 24 using no expand + 1.0 selection - ui.list_item() - .interactive(false) - .with_height(30.0) - .show_flat( - ui, - list_item::CustomContent::new(|ui, _| { - let title_response = self - .filter_state - .ui(ui, egui::RichText::new("Blueprint").strong()); - - if let Some(title_response) = title_response { - title_response.on_hover_text( - "The blueprint is where you can configure the Rerun Viewer", - ); - } - }) - .menu_button(&re_ui::icons::MORE, |ui| { - add_new_view_or_container_menu_button(ctx, viewport, ui); - set_blueprint_to_default_menu_buttons(ctx, ui); - set_blueprint_to_auto_menu_button(ctx, viewport, ui); - }), - ); + ui.list_item().interactive(false).show_flat( + ui, + list_item::CustomContent::new(|ui, _| { + let title_response = self + .filter_state + .ui(ui, egui::RichText::new("Blueprint").strong()); + + if let Some(title_response) = title_response { + title_response.on_hover_text( + "The blueprint is where you can configure the Rerun Viewer", + ); + } + }) + .menu_button(&re_ui::icons::MORE, |ui| { + add_new_view_or_container_menu_button(ctx, viewport, ui); + set_blueprint_to_default_menu_buttons(ctx, ui); + set_blueprint_to_auto_menu_button(ctx, viewport, ui); + }), + ); }); ui.full_span_separator(); diff --git a/crates/viewer/re_time_panel/src/lib.rs b/crates/viewer/re_time_panel/src/lib.rs index fafc93f5287f..0f247d47e4b6 100644 --- a/crates/viewer/re_time_panel/src/lib.rs +++ b/crates/viewer/re_time_panel/src/lib.rs @@ -183,6 +183,11 @@ impl TimePanel { } } + /// Activates the search filter (for e.g. test purposes). + pub fn activate_filter(&mut self, query: &str) { + self.filter_state.activate(query); + } + #[allow(clippy::too_many_arguments)] pub fn show_panel( &mut self, @@ -446,7 +451,7 @@ impl TimePanel { ui.add_space(-4.0); // hack to vertically center the text - let size = egui::vec2(self.prev_col_width, 28.0); + let size = egui::vec2(self.prev_col_width, 27.0); ui.allocate_ui_with_layout(size, egui::Layout::top_down(egui::Align::LEFT), |ui| { ui.set_min_size(size); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); @@ -463,8 +468,6 @@ impl TimePanel { .strong(), ); }); - - ui.add_space(2.0); // hack to vertically center the text }) .response .on_hover_text( diff --git a/crates/viewer/re_time_panel/tests/snapshots/time_panel_dense_data.png b/crates/viewer/re_time_panel/tests/snapshots/time_panel_dense_data.png index 2d30e99a20b8..ae073d311745 100644 --- a/crates/viewer/re_time_panel/tests/snapshots/time_panel_dense_data.png +++ b/crates/viewer/re_time_panel/tests/snapshots/time_panel_dense_data.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:da37f484f89f6e385fadfc6ffcbd8d4119dcbc6236bf5f91d16cc711074d5189 -size 25965 +oid sha256:116c174b99d52b3527bb7aab3fa1f62b0cc3f952991e30e43ff231db6436de29 +size 25861 diff --git a/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_no_query.png b/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_no_query.png new file mode 100644 index 000000000000..7e2d91fc12c6 --- /dev/null +++ b/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_no_query.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d44fe5a5f525ac01203f631b6d75023323f384fc3d7fde4d2ff8f1a72264f072 +size 17468 diff --git a/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_query.png b/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_query.png new file mode 100644 index 000000000000..8c19583c7280 --- /dev/null +++ b/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_active_query.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:17c40bafbc465fe68335737b608569126c437c29319b42d49e7b38e87f027ee1 +size 27073 diff --git a/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_inactive.png b/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_inactive.png new file mode 100644 index 000000000000..7ee285c452b5 --- /dev/null +++ b/crates/viewer/re_time_panel/tests/snapshots/time_panel_filter_test_inactive.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4ff3d2333fdc5fd0b5fd2bef3d1529ebbc793dc185672560f432103c771de2f0 +size 25678 diff --git a/crates/viewer/re_time_panel/tests/snapshots/time_panel_two_sections.png b/crates/viewer/re_time_panel/tests/snapshots/time_panel_two_sections.png index ef2c4cc7a969..a4d2d049e6c9 100644 --- a/crates/viewer/re_time_panel/tests/snapshots/time_panel_two_sections.png +++ b/crates/viewer/re_time_panel/tests/snapshots/time_panel_two_sections.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:7e2086ecac13e41e291aa7780e006faf05a9cc9e9f26c3410f67af152603c963 -size 32001 +oid sha256:0e7dee1c865d86504e1136ead5afd939f84e6c66976f10f14a1d3ebb39e612f7 +size 31820 diff --git a/crates/viewer/re_time_panel/tests/time_panel_tests.rs b/crates/viewer/re_time_panel/tests/time_panel_tests.rs index 15e7d5dfd754..9bf24f3863b8 100644 --- a/crates/viewer/re_time_panel/tests/time_panel_tests.rs +++ b/crates/viewer/re_time_panel/tests/time_panel_tests.rs @@ -33,7 +33,11 @@ pub fn time_panel_two_sections_should_match_snapshot() { .unwrap(); } - run_time_panel_and_save_snapshot(test_context, "time_panel_two_sections"); + run_time_panel_and_save_snapshot( + test_context, + TimePanel::default(), + "time_panel_two_sections", + ); } #[test] @@ -69,12 +73,75 @@ pub fn time_panel_dense_data_should_match_snapshot() { .add_chunk(&Arc::new(builder.build().unwrap())) .unwrap(); - run_time_panel_and_save_snapshot(test_context, "time_panel_dense_data"); + run_time_panel_and_save_snapshot(test_context, TimePanel::default(), "time_panel_dense_data"); } -fn run_time_panel_and_save_snapshot(mut test_context: TestContext, _snapshot_name: &str) { - let mut panel = TimePanel::default(); +#[test] +pub fn time_panel_filter_test_inactive_should_match_snapshot() { + run_time_panel_filter_tests(false, "", "time_panel_filter_test_inactive"); +} + +#[test] +pub fn time_panel_filter_test_active_no_query_should_match_snapshot() { + run_time_panel_filter_tests(true, "", "time_panel_filter_test_active_no_query"); +} + +#[test] +pub fn time_panel_filter_test_active_query_should_match_snapshot() { + run_time_panel_filter_tests(true, "ath", "time_panel_filter_test_active_query"); +} + +#[allow(clippy::unwrap_used)] +pub fn run_time_panel_filter_tests(filter_active: bool, query: &str, snapshot_name: &str) { + TimePanel::ensure_registered_subscribers(); + let mut test_context = TestContext::default(); + + let points1 = MyPoint::from_iter(0..1); + for i in 0..2 { + let entity_path = EntityPath::from(format!("/entity/{i}")); + let mut builder = Chunk::builder(entity_path.clone()); + + builder = builder.with_sparse_component_batches( + RowId::new(), + [build_frame_nr(1)], + [(MyPoint::descriptor(), Some(&points1 as _))], + ); + + test_context + .recording_store + .add_chunk(&Arc::new(builder.build().unwrap())) + .unwrap(); + } + + for i in 0..2 { + let entity_path = EntityPath::from(format!("/path/{i}")); + let mut builder = Chunk::builder(entity_path.clone()); + + builder = builder.with_sparse_component_batches( + RowId::new(), + [build_frame_nr(1)], + [(MyPoint::descriptor(), Some(&points1 as _))], + ); + + test_context + .recording_store + .add_chunk(&Arc::new(builder.build().unwrap())) + .unwrap(); + } + + let mut time_panel = TimePanel::default(); + if filter_active { + time_panel.activate_filter(query); + } + + run_time_panel_and_save_snapshot(test_context, time_panel, snapshot_name); +} +fn run_time_panel_and_save_snapshot( + mut test_context: TestContext, + mut time_panel: TimePanel, + snapshot_name: &str, +) { //TODO(ab): this contains a lot of boilerplate which should be provided by helpers let mut harness = test_context .setup_kittest_for_rendering() @@ -88,7 +155,7 @@ fn run_time_panel_and_save_snapshot(mut test_context: TestContext, _snapshot_nam let mut time_ctrl = viewer_ctx.rec_cfg.time_ctrl.read().clone(); - panel.show_expanded_with_header( + time_panel.show_expanded_with_header( viewer_ctx, &blueprint, viewer_ctx.recording(), @@ -103,5 +170,5 @@ fn run_time_panel_and_save_snapshot(mut test_context: TestContext, _snapshot_nam }); harness.run(); - harness.snapshot(_snapshot_name); + harness.snapshot(snapshot_name); } diff --git a/crates/viewer/re_ui/src/filter_widget.rs b/crates/viewer/re_ui/src/filter_widget.rs index fd5f658954cc..9fcda368a396 100644 --- a/crates/viewer/re_ui/src/filter_widget.rs +++ b/crates/viewer/re_ui/src/filter_widget.rs @@ -52,6 +52,17 @@ pub struct FilterState { } impl FilterState { + /// Activate the filter. + /// + /// This is the same as clicking the "loupe" icon button. + pub fn activate(&mut self, query: &str) { + self.inner_state = Some(InnerState { + filter_query: query.to_owned(), + ..Default::default() + }); + self.request_focus = true; + } + /// Return the filter if any. /// /// Returns `None` if the filter is disabled. Returns `Some(query)` if the filter is enabled @@ -102,51 +113,52 @@ impl FilterState { let mut title_response = None; list_item::list_item_scope(ui, ui.next_auto_id(), |ui| { - ui.list_item() - .interactive(false) - .with_height(30.0) - .show_flat( - ui, - list_item::CustomContent::new(|ui, _| { - if self.inner_state.is_some() - && ui.input_mut(|i| { - i.consume_key(egui::Modifiers::NONE, egui::Key::Escape) - }) - { - self.inner_state = None; + ui.list_item().interactive(false).show_flat( + ui, + list_item::CustomContent::new(|ui, _| { + if self.inner_state.is_some() + && ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) + { + self.inner_state = None; + } + + if let Some(inner_state) = self.inner_state.as_mut() { + // we add additional spacing for aesthetic reasons (active text edits have a + // fat border) + ui.spacing_mut().text_edit_width = + (ui.max_rect().width() - 10.0).at_least(0.0); + + // TODO(ab): ideally _all_ text edit would be styled this way, but we requre egui + // support for that (https://github.com/emilk/egui/issues/3284) + ui.visuals_mut().widgets.hovered.expansion = 0.0; + ui.visuals_mut().widgets.active.expansion = 0.0; + ui.visuals_mut().widgets.open.expansion = 0.0; + ui.visuals_mut().widgets.active.fg_stroke.width = 1.0; + ui.visuals_mut().selection.stroke.width = 1.0; + + let response = egui::TextEdit::singleline(&mut inner_state.filter_query) + .lock_focus(true) + .ui(ui); + + if self.request_focus { + self.request_focus = false; + response.request_focus(); } - - if let Some(inner_state) = self.inner_state.as_mut() { - // we add additional spacing for aesthetic reasons (active text edits have a - // fat border) - ui.spacing_mut().text_edit_width = - (ui.max_rect().width() - 10.0).at_least(0.0); - - let response = - egui::TextEdit::singleline(&mut inner_state.filter_query) - .lock_focus(true) - .ui(ui); - - if self.request_focus { - self.request_focus = false; - response.request_focus(); - } - } else { - title_response = Some(ui.label(galley)); - } - }) - .with_content_width(text_width) - .action_button(icon, || { - button_clicked = true; - }), - ); + } else { + title_response = Some(ui.label(galley)); + } + }) + .with_content_width(text_width) + .action_button(icon, || { + button_clicked = true; + }), + ); }); // defer button handling because we can't mutably borrow `self` in both closures above if button_clicked { if self.inner_state.is_none() { - self.inner_state = Some(InnerState::default()); - self.request_focus = true; + self.activate(""); } else { self.inner_state = None; } From 32022cd9c8ccd561981f0183b3dda2d4861f09cc Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 14 Jan 2025 15:09:18 +0100 Subject: [PATCH 04/13] mein gott these method names got weird over the years... --- .../re_blueprint_tree/src/blueprint_tree.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index 6926624338f6..4a474ef4c3ca 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -110,7 +110,7 @@ impl BlueprintTree { .and_then(|item| handle_focused_item(ctx, viewport, ui, item)); list_item::list_item_scope(ui, "blueprint tree", |ui| { - self.root_container_tree_ui(ctx, viewport, ui); + self.root_container_ui(ctx, viewport, ui); }); let empty_space_response = @@ -160,10 +160,10 @@ impl BlueprintTree { ) { match contents { Contents::Container(container_id) => { - self.container_tree_ui(ctx, viewport, ui, container_id, parent_visible); + self.container_ui(ctx, viewport, ui, container_id, parent_visible); } Contents::View(view_id) => { - self.view_entry_ui(ctx, viewport, ui, view_id, parent_visible); + self.view_ui(ctx, viewport, ui, view_id, parent_visible); } }; } @@ -172,7 +172,7 @@ impl BlueprintTree { /// /// The root container is different from other containers in that it cannot be removed or dragged, and it cannot be /// collapsed, so it's drawn without a collapsing triangle. - fn root_container_tree_ui( + fn root_container_ui( &mut self, ctx: &ViewerContext<'_>, viewport: &ViewportBlueprint, @@ -223,7 +223,7 @@ impl BlueprintTree { ); } - fn container_tree_ui( + fn container_ui( &mut self, ctx: &ViewerContext<'_>, viewport: &ViewportBlueprint, @@ -308,7 +308,7 @@ impl BlueprintTree { ); } - fn view_entry_ui( + fn view_ui( &mut self, ctx: &ViewerContext<'_>, viewport: &ViewportBlueprint, @@ -371,7 +371,7 @@ impl BlueprintTree { .force_hovered(is_item_hovered) .show_hierarchical_with_children(ui, id, default_open, item_content, |ui| { // Always show the origin hierarchy first. - self.view_entity_hierarchy_ui( + self.data_result_ui( ctx, viewport, ui, @@ -402,7 +402,7 @@ impl BlueprintTree { ); for projection in projections { - self.view_entity_hierarchy_ui( + self.data_result_ui( ctx, viewport, ui, @@ -446,7 +446,7 @@ impl BlueprintTree { } #[allow(clippy::too_many_arguments)] - fn view_entity_hierarchy_ui( + fn data_result_ui( &self, ctx: &ViewerContext<'_>, viewport: &ViewportBlueprint, @@ -577,7 +577,7 @@ impl BlueprintTree { continue; }; - self.view_entity_hierarchy_ui( + self.data_result_ui( ctx, viewport, ui, From 41761968f4d12259d3936ce84851b1c555548d9f Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Tue, 14 Jan 2025 17:50:25 +0100 Subject: [PATCH 05/13] "Naive" version of filtering implemented Also: updated/added tree walking apis for `DataResultTree` and `ViewportBlueprint` Two problems: - matches `$origin` when it shouldn't - matches in-origin-tree entities for parts that are _above or at_ origin Both of these are not displayed, and thus should not be matched. --- .../re_blueprint_tree/src/blueprint_tree.rs | 148 +++++++++++++++++- .../src/actions/collapse_expand_all.rs | 14 +- .../re_viewer_context/src/query_context.rs | 41 +++++ .../src/viewport_blueprint.rs | 38 +++-- 4 files changed, 216 insertions(+), 25 deletions(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index 4a474ef4c3ca..a1fc0ee500e8 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -99,6 +99,8 @@ impl BlueprintTree { self.candidate_drop_parent_container_id = self.next_candidate_drop_parent_container_id; self.next_candidate_drop_parent_container_id = None; + let filter_matcher = self.filter_state.filter(); + egui::ScrollArea::both() .id_salt("blueprint_tree_scroll_area") .auto_shrink([true, false]) @@ -110,7 +112,7 @@ impl BlueprintTree { .and_then(|item| handle_focused_item(ctx, viewport, ui, item)); list_item::list_item_scope(ui, "blueprint tree", |ui| { - self.root_container_ui(ctx, viewport, ui); + self.root_container_ui(ctx, viewport, ui, &filter_matcher); }); let empty_space_response = @@ -157,13 +159,21 @@ impl BlueprintTree { ui: &mut egui::Ui, contents: &Contents, parent_visible: bool, + filter_matcher: &filter_widget::FilterMatcher, ) { match contents { Contents::Container(container_id) => { - self.container_ui(ctx, viewport, ui, container_id, parent_visible); + self.container_ui( + ctx, + viewport, + ui, + container_id, + parent_visible, + filter_matcher, + ); } Contents::View(view_id) => { - self.view_ui(ctx, viewport, ui, view_id, parent_visible); + self.view_ui(ctx, viewport, ui, view_id, parent_visible, filter_matcher); } }; } @@ -177,8 +187,12 @@ impl BlueprintTree { ctx: &ViewerContext<'_>, viewport: &ViewportBlueprint, ui: &mut egui::Ui, + filter_matcher: &filter_widget::FilterMatcher, ) { let container_id = viewport.root_container; + if !match_container(ctx, viewport, &container_id, filter_matcher) { + return; + } let Some(container_blueprint) = viewport.container(&container_id) else { re_log::warn!("Failed to find root container {container_id} in ViewportBlueprint"); @@ -201,7 +215,7 @@ impl BlueprintTree { ); for child in &container_blueprint.contents { - self.contents_ui(ctx, viewport, ui, child, true); + self.contents_ui(ctx, viewport, ui, child, true, filter_matcher); } context_menu_ui_for_item( @@ -230,7 +244,12 @@ impl BlueprintTree { ui: &mut egui::Ui, container_id: &ContainerId, parent_visible: bool, + filter_matcher: &filter_widget::FilterMatcher, ) { + if !match_container(ctx, viewport, container_id, filter_matcher) { + return; + } + let item = Item::Container(*container_id); let content = Contents::Container(*container_id); @@ -277,7 +296,7 @@ impl BlueprintTree { .drop_target_style(self.is_candidate_drop_parent_container(container_id)) .show_hierarchical_with_children(ui, id, default_open, item_content, |ui| { for child in &container_blueprint.contents { - self.contents_ui(ctx, viewport, ui, child, container_visible); + self.contents_ui(ctx, viewport, ui, child, container_visible, filter_matcher); } }); @@ -315,7 +334,12 @@ impl BlueprintTree { ui: &mut egui::Ui, view_id: &ViewId, container_visible: bool, + filter_matcher: &filter_widget::FilterMatcher, ) { + if !match_view(ctx, view_id, filter_matcher) { + return; + } + let Some(view) = viewport.view(view_id) else { re_log::warn_once!("Bug: asked to show a UI for a view that doesn't exist"); return; @@ -380,6 +404,7 @@ impl BlueprintTree { view, view_visible, false, + filter_matcher, ); // Show 'projections' if there's any items that weren't part of the tree under origin but are directly included. @@ -411,6 +436,7 @@ impl BlueprintTree { view, view_visible, true, + filter_matcher, ); } } @@ -456,9 +482,51 @@ impl BlueprintTree { view: &ViewBlueprint, view_visible: bool, projection_mode: bool, + filter_matcher: &filter_widget::FilterMatcher, ) { let entity_path = node_or_path.path(); + // We traverse and display this data result only if contains a matching entity part + 'early_exit: { + if filter_matcher.matches_nothing() { + return; + } + + // Filter is inactive or otherwise whitelisting everything. + if filter_matcher.matches_everything() { + break 'early_exit; + } + + // The current path is a match. + if entity_path + .iter() + .any(|entity_part| filter_matcher.matches(&entity_part.ui_string())) + { + break 'early_exit; + } + + // Our subtree contains a match. + if let Some(node) = node_or_path.data_result_node() { + if query_result + .tree + .find_node_by(Some(node), |node| { + node.data_result + .entity_path + .last() + .is_some_and(|entity_part| { + filter_matcher.matches(&entity_part.ui_string()) + }) + }) + .is_some() + { + break 'early_exit; + } + } + + // No match to be found, so nothing to display. + return; + } + if projection_mode && entity_path == &view.space_origin { if ui .list_item() @@ -502,9 +570,11 @@ impl BlueprintTree { .map_or("unknown".to_owned(), |e| e.ui_string()) }; let item_label = if ctx.recording().is_known_entity(entity_path) { - egui::RichText::new(item_label) + filter_matcher + .matches_formatted(ui.ctx(), &item_label) + .unwrap_or_else(|| item_label.into()) } else { - ui.ctx().warning_text(item_label) + ui.ctx().warning_text(item_label).into() }; let subdued = !view_visible || !visible; @@ -586,6 +656,7 @@ impl BlueprintTree { view, view_visible, projection_mode, + filter_matcher, ); } }) @@ -864,6 +935,67 @@ impl BlueprintTree { // ---------------------------------------------------------------------------- +fn match_container( + ctx: &ViewerContext<'_>, + viewport: &ViewportBlueprint, + container_id: &ContainerId, + filter_matcher: &filter_widget::FilterMatcher, +) -> bool { + if filter_matcher.matches_everything() { + return true; + } + + if filter_matcher.matches_nothing() { + return false; + } + + viewport + .find_contents_in_container_by( + &|content| { + // dont recurse infinitely + if content == &Contents::Container(*container_id) { + return false; + } + + match content { + Contents::Container(container_id) => { + match_container(ctx, viewport, container_id, filter_matcher) + } + Contents::View(view_id) => match_view(ctx, view_id, filter_matcher), + } + }, + container_id, + ) + .is_some() +} + +/// Does this view match the provided filter? +fn match_view( + ctx: &ViewerContext<'_>, + view_id: &ViewId, + filter_matcher: &filter_widget::FilterMatcher, +) -> bool { + if filter_matcher.matches_everything() { + return true; + } + + if filter_matcher.matches_nothing() { + return false; + }; + + let query_result = ctx.lookup_query_result(*view_id); + let result_tree = &query_result.tree; + + result_tree + .find_node_by(None, |node| { + node.data_result + .entity_path + .last() + .is_some_and(|entity_part| filter_matcher.matches(&entity_part.ui_string())) + }) + .is_some() +} + /// Add a button to trigger the addition of a new view or container. fn add_new_view_or_container_menu_button( ctx: &ViewerContext<'_>, @@ -1026,6 +1158,7 @@ fn expand_all_contents_until( expend_contents(&Contents::Container(*parent)); } } + true }); } @@ -1044,6 +1177,7 @@ fn list_views_with_entity( view_ids.push(*view_id); } } + true }); view_ids } diff --git a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs index e92c6d11fa32..64c911f9ed7f 100644 --- a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs +++ b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs @@ -51,11 +51,15 @@ impl ContextMenuAction for CollapseExpandAllAction { fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) { ctx.viewport_blueprint - .visit_contents_in_container(container_id, &mut |contents, _| match contents { - Contents::Container(container_id) => CollapseScope::BlueprintTree - .container(*container_id) - .set_open(&ctx.egui_context, self.open()), - Contents::View(view_id) => self.process_view(ctx, view_id), + .visit_contents_in_container(container_id, &mut |contents, _| { + match contents { + Contents::Container(container_id) => CollapseScope::BlueprintTree + .container(*container_id) + .set_open(&ctx.egui_context, self.open()), + Contents::View(view_id) => self.process_view(ctx, view_id), + } + + true }); } diff --git a/crates/viewer/re_viewer_context/src/query_context.rs b/crates/viewer/re_viewer_context/src/query_context.rs index e1ad601eb026..42a11d69753a 100644 --- a/crates/viewer/re_viewer_context/src/query_context.rs +++ b/crates/viewer/re_viewer_context/src/query_context.rs @@ -163,6 +163,47 @@ impl DataResultTree { } } + /// Depth-first traversal of the tree, calling `visitor` on each result, starting from a + /// specific node. + /// + /// Stops traversing a branch if `visitor` returns `false`. + pub fn visit_from_node<'a>( + &'a self, + node: &DataResultNode, + visitor: &mut impl FnMut(&'a DataResultNode) -> bool, + ) { + if let Some(handle) = self + .data_results_by_path + .get(&node.data_result.entity_path.hash()) + { + self.visit_recursive(*handle, visitor); + } + } + + /// Depth-first search of a node based on the provided predicate. + /// + /// If a `staring_node` is provided, the search starts at that node. Otherwise, it starts at the + /// root node. + pub fn find_node_by( + &self, + starting_node: Option<&DataResultNode>, + predicate: impl Fn(&DataResultNode) -> bool, + ) -> Option<&DataResultNode> { + let mut result = None; + + starting_node.or_else(|| self.root_node()).and_then(|node| { + self.visit_from_node(node, &mut |node| { + if predicate(node) { + result = Some(node); + false + } else { + true + } + }); + result + }) + } + /// Look up a [`DataResult`] in the tree based on its handle. #[inline] pub fn lookup_result(&self, handle: DataResultHandle) -> Option<&DataResult> { diff --git a/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs b/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs index 86a0f8e38dd5..23656137d068 100644 --- a/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs +++ b/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs @@ -429,7 +429,10 @@ impl ViewportBlueprint { /// Walk the entire [`Contents`] tree, starting from the root container. /// /// See [`Self::visit_contents_in_container`] for details. - pub fn visit_contents(&self, visitor: &mut impl FnMut(&Contents, &SmallVec<[ContainerId; 4]>)) { + pub fn visit_contents( + &self, + visitor: &mut impl FnMut(&Contents, &SmallVec<[ContainerId; 4]>) -> bool, + ) { self.visit_contents_in_container(&self.root_container, visitor); } @@ -437,13 +440,14 @@ impl ViewportBlueprint { /// [`Contents`]. /// /// Note: + /// - Stops traversing if `visitor` returns `false`. /// - `visitor` is first called for the container passed in argument /// - `visitor`'s second argument contains the hierarchy leading to the visited contents, from /// (and including) the container passed in argument pub fn visit_contents_in_container( &self, container_id: &ContainerId, - visitor: &mut impl FnMut(&Contents, &SmallVec<[ContainerId; 4]>), + visitor: &mut impl FnMut(&Contents, &SmallVec<[ContainerId; 4]>) -> bool, ) { let mut hierarchy = SmallVec::new(); self.visit_contents_in_container_impl(container_id, &mut hierarchy, visitor); @@ -453,21 +457,29 @@ impl ViewportBlueprint { &self, container_id: &ContainerId, hierarchy: &mut SmallVec<[ContainerId; 4]>, - visitor: &mut impl FnMut(&Contents, &SmallVec<[ContainerId; 4]>), + visitor: &mut impl FnMut(&Contents, &SmallVec<[ContainerId; 4]>) -> bool, ) { - visitor(&Contents::Container(*container_id), hierarchy); - if let Some(container) = self.container(container_id) { - hierarchy.push(*container_id); - for contents in &container.contents { - visitor(contents, hierarchy); - match contents { - Contents::Container(container_id) => { - self.visit_contents_in_container_impl(container_id, hierarchy, visitor); + if visitor(&Contents::Container(*container_id), hierarchy) { + if let Some(container) = self.container(container_id) { + hierarchy.push(*container_id); + for contents in &container.contents { + if visitor(contents, hierarchy) { + match contents { + Contents::Container(container_id) => { + self.visit_contents_in_container_impl( + container_id, + hierarchy, + visitor, + ); + } + Contents::View(_) => {} + } + } else { + return; } - Contents::View(_) => {} } + hierarchy.pop(); } - hierarchy.pop(); } } From 378848776eaf8bd943631ca8e368edf941ce3b86 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Wed, 15 Jan 2025 14:17:01 +0100 Subject: [PATCH 06/13] WIP less naive version of filtering / TODO: don't show `Projections:` when there are none --- .../re_blueprint_tree/src/blueprint_tree.rs | 130 ++++++++++++------ crates/viewer/re_ui/src/filter_widget.rs | 5 + .../re_viewer_context/src/query_context.rs | 6 +- .../src/viewport_blueprint.rs | 2 +- 4 files changed, 98 insertions(+), 45 deletions(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index a1fc0ee500e8..e20c50307e72 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -420,6 +420,7 @@ impl BlueprintTree { false // We found a projection, stop recursing as everything below is now included in the projections. } }); + //TODO: this must consider the filter as well if !projections.is_empty() { ui.list_item().interactive(false).show_flat( ui, @@ -484,50 +485,19 @@ impl BlueprintTree { projection_mode: bool, filter_matcher: &filter_widget::FilterMatcher, ) { - let entity_path = node_or_path.path(); - - // We traverse and display this data result only if contains a matching entity part - 'early_exit: { - if filter_matcher.matches_nothing() { - return; - } - - // Filter is inactive or otherwise whitelisting everything. - if filter_matcher.matches_everything() { - break 'early_exit; - } - - // The current path is a match. - if entity_path - .iter() - .any(|entity_part| filter_matcher.matches(&entity_part.ui_string())) - { - break 'early_exit; - } - - // Our subtree contains a match. - if let Some(node) = node_or_path.data_result_node() { - if query_result - .tree - .find_node_by(Some(node), |node| { - node.data_result - .entity_path - .last() - .is_some_and(|entity_part| { - filter_matcher.matches(&entity_part.ui_string()) - }) - }) - .is_some() - { - break 'early_exit; - } - } - - // No match to be found, so nothing to display. + if !self.match_data_result( + query_result, + node_or_path, + view, + projection_mode, + filter_matcher, + ) { return; } - if projection_mode && entity_path == &view.space_origin { + let entity_path = node_or_path.path(); + let display_origin_placeholder = projection_mode && entity_path == &view.space_origin; + if display_origin_placeholder { if ui .list_item() .show_hierarchical( @@ -695,6 +665,82 @@ impl BlueprintTree { ctx.handle_select_hover_drag_interactions(&response, item, true); } + /// Should this data result (and its children) be displayed? + fn match_data_result( + &self, + query_result: &DataQueryResult, + node_or_path: &DataResultNodeOrPath<'_>, + view: &ViewBlueprint, + projection_mode: bool, + filter_matcher: &filter_widget::FilterMatcher, + ) -> bool { + let entity_path = node_or_path.path(); + let display_origin_placeholder = projection_mode && entity_path == &view.space_origin; + + if filter_matcher.matches_nothing() { + return false; + } + + // Filter is inactive or otherwise whitelisting everything. + if filter_matcher.matches_everything() { + return true; + } + + // We never display the origin placeholder if the filter is active, because if the + // `$origin` subtree matched something, it would be visible in the non-projected + // data-results. + if display_origin_placeholder && self.filter_state.is_active() { + return false; + } + + // Is the current path a match? + // + // This is subtle! If we are in projection mode, we check for all parts starting at the + // root for a match. However, if we are _not_ in projection mode, we skip checking the + // origin part (which is always the first part of the entity path in this case), because + // that part is not displayed and, as such, should not trigger a match. + if entity_path + .iter() + .skip( + if !projection_mode && entity_path.starts_with(&view.space_origin) { + view.space_origin.len() + } else { + 0 + }, + ) + .any(|entity_part| filter_matcher.matches(&entity_part.ui_string())) + { + return true; + } + + // Our subtree contains a match. + if let Some(node) = node_or_path.data_result_node() { + if query_result + .tree + .find_node_by(Some(node), |node| { + // If are in projection mode, we must reject anything that is the origin or + // its child. If there was a match there, then it would be displayed in the + // non-projected data results. + if projection_mode + && node.data_result.entity_path.starts_with(&view.space_origin) + { + return false; + } + + node.data_result + .entity_path + .last() + .is_some_and(|entity_part| filter_matcher.matches(&entity_part.ui_string())) + }) + .is_some() + { + return true; + } + } + + false + } + // ---------------------------------------------------------------------------- // drag and drop support @@ -996,6 +1042,8 @@ fn match_view( .is_some() } +// ---------------------------------------------------------------------------- + /// Add a button to trigger the addition of a new view or container. fn add_new_view_or_container_menu_button( ctx: &ViewerContext<'_>, diff --git a/crates/viewer/re_ui/src/filter_widget.rs b/crates/viewer/re_ui/src/filter_widget.rs index 9fcda368a396..91e7cae64130 100644 --- a/crates/viewer/re_ui/src/filter_widget.rs +++ b/crates/viewer/re_ui/src/filter_widget.rs @@ -63,6 +63,11 @@ impl FilterState { self.request_focus = true; } + /// Is the filter currently active? + pub fn is_active(&self) -> bool { + self.inner_state.is_some() + } + /// Return the filter if any. /// /// Returns `None` if the filter is disabled. Returns `Some(query)` if the filter is enabled diff --git a/crates/viewer/re_viewer_context/src/query_context.rs b/crates/viewer/re_viewer_context/src/query_context.rs index 42a11d69753a..b0968b20050b 100644 --- a/crates/viewer/re_viewer_context/src/query_context.rs +++ b/crates/viewer/re_viewer_context/src/query_context.rs @@ -195,10 +195,10 @@ impl DataResultTree { self.visit_from_node(node, &mut |node| { if predicate(node) { result = Some(node); - false - } else { - true } + + // keep recursing until we find something + result.is_none() }); result }) diff --git a/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs b/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs index 23656137d068..e95bb97ffdd7 100644 --- a/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs +++ b/crates/viewer/re_viewport_blueprint/src/viewport_blueprint.rs @@ -440,7 +440,7 @@ impl ViewportBlueprint { /// [`Contents`]. /// /// Note: - /// - Stops traversing if `visitor` returns `false`. + /// - Returns as soon as `visitor` returns `false`. /// - `visitor` is first called for the container passed in argument /// - `visitor`'s second argument contains the hierarchy leading to the visited contents, from /// (and including) the container passed in argument From 9110bf5f8e1cc2e9b5948075b60d6eca3b183b0d Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 16 Jan 2025 10:25:11 +0100 Subject: [PATCH 07/13] hide "projection" label when not needed --- .../re_blueprint_tree/src/blueprint_tree.rs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index e20c50307e72..d57b56afec8e 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -416,11 +416,25 @@ impl BlueprintTree { } else if node.data_result.tree_prefix_only { true // Keep recursing until we find a projection. } else { - projections.push(node); - false // We found a projection, stop recursing as everything below is now included in the projections. + // We found a projection, but we must check if it is ruled out by the + // filter. + if self.match_data_result( + query_result, + &DataResultNodeOrPath::DataResultNode(node), + view, + true, + filter_matcher, + ) { + projections.push(node); + } + + // No further recursion needed in this branch, everything below is included + // in the projection (or shouldn't be included if the projection has already + // been filtered out). + false } }); - //TODO: this must consider the filter as well + if !projections.is_empty() { ui.list_item().interactive(false).show_flat( ui, From 5873380277b85847659355b707f2ecdb9046439a Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 16 Jan 2025 11:22:27 +0100 Subject: [PATCH 08/13] deal with collapsedness + related contextual menus --- .../re_blueprint_tree/src/blueprint_tree.rs | 276 ++++++++++-------- .../src/actions/collapse_expand_all.rs | 133 ++++++--- crates/viewer/re_time_panel/src/lib.rs | 2 +- crates/viewer/re_view_spatial/src/ui_3d.rs | 4 +- .../re_viewer_context/src/collapsed_id.rs | 3 + .../re_viewer_context/src/selection_state.rs | 7 + 6 files changed, 267 insertions(+), 158 deletions(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index d57b56afec8e..824a4d2671d8 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -1,9 +1,9 @@ use egui::{Response, Ui}; use itertools::Itertools; -use re_data_ui::item_ui::guess_instance_path_icon; use smallvec::SmallVec; -use re_context_menu::{context_menu_ui_for_item, SelectionUpdateBehavior}; +use re_context_menu::{context_menu_ui_for_item_with_context, SelectionUpdateBehavior}; +use re_data_ui::item_ui::guess_instance_path_icon; use re_entity_db::InstancePath; use re_log_types::EntityPath; use re_types::blueprint::components::Visible; @@ -11,14 +11,13 @@ use re_ui::{ drag_and_drop::DropTarget, filter_widget, list_item, ContextExt as _, DesignTokens, UiExt as _, }; use re_viewer_context::{ - contents_name_style, icon_for_container_kind, CollapseScope, Contents, DataResultNodeOrPath, - DragAndDropFeedback, DragAndDropPayload, SystemCommandSender, + contents_name_style, icon_for_container_kind, CollapseScope, ContainerId, Contents, + DataQueryResult, DataResultNode, DataResultNodeOrPath, DragAndDropFeedback, DragAndDropPayload, + HoverHighlight, Item, ItemContext, SystemCommandSender, ViewId, ViewerContext, }; -use re_viewer_context::{ - ContainerId, DataQueryResult, DataResultNode, HoverHighlight, Item, ViewId, ViewerContext, +use re_viewport_blueprint::{ + ui::show_add_view_or_container_modal, ViewBlueprint, ViewportBlueprint, }; -use re_viewport_blueprint::ui::show_add_view_or_container_modal; -use re_viewport_blueprint::{ViewBlueprint, ViewportBlueprint}; /// Holds the state of the blueprint tree UI. #[derive(Default)] @@ -109,7 +108,7 @@ impl BlueprintTree { self.blueprint_tree_scroll_to_item = ctx .focused_item .as_ref() - .and_then(|item| handle_focused_item(ctx, viewport, ui, item)); + .and_then(|item| self.handle_focused_item(ctx, viewport, ui, item)); list_item::list_item_scope(ui, "blueprint tree", |ui| { self.root_container_ui(ctx, viewport, ui, &filter_matcher); @@ -218,10 +217,14 @@ impl BlueprintTree { self.contents_ui(ctx, viewport, ui, child, true, filter_matcher); } - context_menu_ui_for_item( + context_menu_ui_for_item_with_context( ctx, viewport, &item, + // expand/collapse context menu actions need this information + ItemContext::BlueprintTree { + filter_session_id: self.filter_state.session_id(), + }, &item_response, SelectionUpdateBehavior::UseSelection, ); @@ -283,7 +286,7 @@ impl BlueprintTree { // Globally unique id - should only be one of these in view at one time. // We do this so that we can support "collapse/expand all" command. - let id = egui::Id::new(CollapseScope::BlueprintTree.container(*container_id)); + let id = egui::Id::new(self.collapse_scope().container(*container_id)); let list_item::ShowCollapsingResponse { item_response: response, @@ -305,10 +308,14 @@ impl BlueprintTree { container_blueprint.container_kind )); - context_menu_ui_for_item( + context_menu_ui_for_item_with_context( ctx, viewport, &item, + // expand/collapse context menu actions need this information + ItemContext::BlueprintTree { + filter_session_id: self.filter_state.session_id(), + }, &response, SelectionUpdateBehavior::UseSelection, ); @@ -356,7 +363,8 @@ impl BlueprintTree { let root_node = result_tree.root_node(); // empty views should display as open by default to highlight the fact that they are empty - let default_open = root_node.map_or(true, Self::default_open_for_data_result); + let default_open = self.filter_state.is_active() + || root_node.map_or(true, Self::default_open_for_data_result); let is_item_hovered = ctx.selection_state().highlight_for_ui_element(&item) == HoverHighlight::Hovered; @@ -382,7 +390,7 @@ impl BlueprintTree { // Globally unique id - should only be one of these in view at one time. // We do this so that we can support "collapse/expand all" command. - let id = egui::Id::new(CollapseScope::BlueprintTree.view(*view_id)); + let id = egui::Id::new(self.collapse_scope().view(*view_id)); let list_item::ShowCollapsingResponse { item_response: response, @@ -463,10 +471,14 @@ impl BlueprintTree { viewport.focus_tab(view.id); } - context_menu_ui_for_item( + context_menu_ui_for_item_with_context( ctx, viewport, &item, + // expand/collapse context menu actions need this information + ItemContext::BlueprintTree { + filter_session_id: self.filter_state.session_id(), + }, &response, SelectionUpdateBehavior::UseSelection, ); @@ -609,13 +621,15 @@ impl BlueprintTree { let has_children = data_result_node.is_some_and(|n| !n.children.is_empty()); let response = if let (true, Some(node)) = (has_children, data_result_node) { // Don't default open projections. - let default_open = entity_path.starts_with(&view.space_origin) - && Self::default_open_for_data_result(node); + let default_open = self.filter_state.is_active() + || (entity_path.starts_with(&view.space_origin) + && Self::default_open_for_data_result(node)); // Globally unique id - should only be one of these in view at one time. // We do this so that we can support "collapse/expand all" command. let id = egui::Id::new( - CollapseScope::BlueprintTree.data_result(view.id, entity_path.clone()), + self.collapse_scope() + .data_result(view.id, entity_path.clone()), ); list_item @@ -668,10 +682,14 @@ impl BlueprintTree { } }); - context_menu_ui_for_item( + context_menu_ui_for_item_with_context( ctx, viewport, &item, + // expand/collapse context menu actions need this information + ItemContext::BlueprintTree { + filter_session_id: self.filter_state.session_id(), + }, &response, SelectionUpdateBehavior::UseSelection, ); @@ -991,6 +1009,130 @@ impl BlueprintTree { fn is_candidate_drop_parent_container(&self, container_id: &ContainerId) -> bool { self.candidate_drop_parent_container_id.as_ref() == Some(container_id) } + + fn collapse_scope(&self) -> CollapseScope { + match self.filter_state.session_id() { + None => CollapseScope::BlueprintTree, + Some(session_id) => CollapseScope::BlueprintTreeFiltered { session_id }, + } + } + + // --- + + /// Expand all required items and compute which item we should scroll to. + fn handle_focused_item( + &self, + ctx: &ViewerContext<'_>, + viewport: &ViewportBlueprint, + ui: &egui::Ui, + focused_item: &Item, + ) -> Option { + match focused_item { + Item::AppId(_) | Item::DataSource(_) | Item::StoreId(_) => None, + + Item::Container(container_id) => { + self.expand_all_contents_until( + viewport, + ui.ctx(), + &Contents::Container(*container_id), + ); + Some(focused_item.clone()) + } + Item::View(view_id) => { + self.expand_all_contents_until(viewport, ui.ctx(), &Contents::View(*view_id)); + ctx.focused_item.clone() + } + Item::DataResult(view_id, instance_path) => { + self.expand_all_contents_until(viewport, ui.ctx(), &Contents::View(*view_id)); + self.expand_all_data_results_until( + ctx, + ui.ctx(), + view_id, + &instance_path.entity_path, + ); + + ctx.focused_item.clone() + } + Item::InstancePath(instance_path) => { + let view_ids = list_views_with_entity(ctx, viewport, &instance_path.entity_path); + + // focus on the first matching data result + let res = view_ids + .first() + .map(|id| Item::DataResult(*id, instance_path.clone())); + + for view_id in view_ids { + self.expand_all_contents_until(viewport, ui.ctx(), &Contents::View(view_id)); + self.expand_all_data_results_until( + ctx, + ui.ctx(), + &view_id, + &instance_path.entity_path, + ); + } + + res + } + Item::ComponentPath(component_path) => self.handle_focused_item( + ctx, + viewport, + ui, + &Item::InstancePath(InstancePath::entity_all(component_path.entity_path.clone())), + ), + } + } + + /// Expand all containers until reaching the provided content. + fn expand_all_contents_until( + &self, + viewport: &ViewportBlueprint, + egui_ctx: &egui::Context, + focused_contents: &Contents, + ) { + //TODO(ab): this could look nicer if `Contents` was declared in re_view_context :) + let expend_contents = |contents: &Contents| match contents { + Contents::Container(container_id) => self + .collapse_scope() + .container(*container_id) + .set_open(egui_ctx, true), + Contents::View(view_id) => self + .collapse_scope() + .view(*view_id) + .set_open(egui_ctx, true), + }; + + viewport.visit_contents(&mut |contents, hierarchy| { + if contents == focused_contents { + expend_contents(contents); + for parent in hierarchy { + expend_contents(&Contents::Container(*parent)); + } + } + true + }); + } + + /// Expand data results of the provided view all the way to the provided entity. + fn expand_all_data_results_until( + &self, + ctx: &ViewerContext<'_>, + egui_ctx: &egui::Context, + view_id: &ViewId, + entity_path: &EntityPath, + ) { + let result_tree = &ctx.lookup_query_result(*view_id).tree; + if result_tree.lookup_node_by_path(entity_path).is_some() { + if let Some(root_node) = result_tree.root_node() { + EntityPath::incremental_walk(Some(&root_node.data_result.entity_path), entity_path) + .chain(std::iter::once(root_node.data_result.entity_path.clone())) + .for_each(|entity_path| { + self.collapse_scope() + .data_result(*view_id, entity_path) + .set_open(egui_ctx, true); + }); + } + } + } } // ---------------------------------------------------------------------------- @@ -1149,81 +1291,6 @@ fn set_blueprint_to_auto_menu_button( } } -/// Expand all required items and compute which item we should scroll to. -fn handle_focused_item( - ctx: &ViewerContext<'_>, - viewport: &ViewportBlueprint, - ui: &egui::Ui, - focused_item: &Item, -) -> Option { - match focused_item { - Item::AppId(_) | Item::DataSource(_) | Item::StoreId(_) => None, - - Item::Container(container_id) => { - expand_all_contents_until(viewport, ui.ctx(), &Contents::Container(*container_id)); - Some(focused_item.clone()) - } - Item::View(view_id) => { - expand_all_contents_until(viewport, ui.ctx(), &Contents::View(*view_id)); - ctx.focused_item.clone() - } - Item::DataResult(view_id, instance_path) => { - expand_all_contents_until(viewport, ui.ctx(), &Contents::View(*view_id)); - expand_all_data_results_until(ctx, ui.ctx(), view_id, &instance_path.entity_path); - - ctx.focused_item.clone() - } - Item::InstancePath(instance_path) => { - let view_ids = list_views_with_entity(ctx, viewport, &instance_path.entity_path); - - // focus on the first matching data result - let res = view_ids - .first() - .map(|id| Item::DataResult(*id, instance_path.clone())); - - for view_id in view_ids { - expand_all_contents_until(viewport, ui.ctx(), &Contents::View(view_id)); - expand_all_data_results_until(ctx, ui.ctx(), &view_id, &instance_path.entity_path); - } - - res - } - Item::ComponentPath(component_path) => handle_focused_item( - ctx, - viewport, - ui, - &Item::InstancePath(InstancePath::entity_all(component_path.entity_path.clone())), - ), - } -} - -/// Expand all containers until reaching the provided content. -fn expand_all_contents_until( - viewport: &ViewportBlueprint, - egui_ctx: &egui::Context, - focused_contents: &Contents, -) { - //TODO(ab): this could look nicer if `Contents` was declared in re_view_context :) - let expend_contents = |contents: &Contents| match contents { - Contents::Container(container_id) => CollapseScope::BlueprintTree - .container(*container_id) - .set_open(egui_ctx, true), - Contents::View(view_id) => CollapseScope::BlueprintTree - .view(*view_id) - .set_open(egui_ctx, true), - }; - - viewport.visit_contents(&mut |contents, hierarchy| { - if contents == focused_contents { - expend_contents(contents); - for parent in hierarchy { - expend_contents(&Contents::Container(*parent)); - } - } - true - }); -} - /// List all views that have the provided entity as data result. #[inline] fn list_views_with_entity( @@ -1244,27 +1311,6 @@ fn list_views_with_entity( view_ids } -/// Expand data results of the provided view all the way to the provided entity. -fn expand_all_data_results_until( - ctx: &ViewerContext<'_>, - egui_ctx: &egui::Context, - view_id: &ViewId, - entity_path: &EntityPath, -) { - let result_tree = &ctx.lookup_query_result(*view_id).tree; - if result_tree.lookup_node_by_path(entity_path).is_some() { - if let Some(root_node) = result_tree.root_node() { - EntityPath::incremental_walk(Some(&root_node.data_result.entity_path), entity_path) - .chain(std::iter::once(root_node.data_result.entity_path.clone())) - .for_each(|entity_path| { - CollapseScope::BlueprintTree - .data_result(*view_id, entity_path) - .set_open(egui_ctx, true); - }); - } - } -} - fn remove_button_ui(ui: &mut Ui, tooltip: &str) -> Response { ui.small_icon_button(&re_ui::icons::REMOVE) .on_hover_text(tooltip) diff --git a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs index 64c911f9ed7f..65250d27c88f 100644 --- a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs +++ b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs @@ -5,9 +5,9 @@ use re_viewer_context::{CollapseScope, ContainerId, Contents, Item, ItemContext, use crate::{ContextMenuAction, ContextMenuContext}; /// Collapse or expand all items in the selection. -// TODO(ab): the current implementation makes strong assumptions of which CollapseScope to use based -// on the item type. This is brittle and will not scale if/when we add more trees to the UI. When -// that happens, we will have to pass the scope to `context_menu_ui_for_item` and use it here. +/// +/// Note: this makes _heavy_ use of [`ItemContext`] to determine the correct scope to +/// collapse/expand. pub(crate) enum CollapseExpandAllAction { CollapseAll, ExpandAll, @@ -50,13 +50,17 @@ impl ContextMenuAction for CollapseExpandAllAction { } fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) { + let scope = blueprint_collapse_scope(ctx, &Item::Container(*container_id)); + ctx.viewport_blueprint .visit_contents_in_container(container_id, &mut |contents, _| { match contents { - Contents::Container(container_id) => CollapseScope::BlueprintTree + Contents::Container(container_id) => scope .container(*container_id) .set_open(&ctx.egui_context, self.open()), - Contents::View(view_id) => self.process_view(ctx, view_id), + + // IMPORTANT: don't call process_view() here, or the scope information would be lost + Contents::View(view_id) => self.process_view_impl(ctx, view_id, scope), } true @@ -64,19 +68,9 @@ impl ContextMenuAction for CollapseExpandAllAction { } fn process_view(&self, ctx: &ContextMenuContext<'_>, view_id: &ViewId) { - CollapseScope::BlueprintTree - .view(*view_id) - .set_open(&ctx.egui_context, self.open()); + let scope = blueprint_collapse_scope(ctx, &Item::View(*view_id)); - let query_result = ctx.viewer_context.lookup_query_result(*view_id); - let result_tree = &query_result.tree; - if let Some(root_node) = result_tree.root_node() { - self.process_data_result( - ctx, - view_id, - &InstancePath::entity_all(root_node.data_result.entity_path.clone()), - ); - } + self.process_view_impl(ctx, view_id, scope); } fn process_data_result( @@ -85,22 +79,10 @@ impl ContextMenuAction for CollapseExpandAllAction { view_id: &ViewId, instance_path: &InstancePath, ) { - //TODO(ab): here we should in principle walk the DataResult tree instead of the entity tree - // but the current API isn't super ergonomic. - let Some(subtree) = ctx - .viewer_context - .recording() - .tree() - .subtree(&instance_path.entity_path) - else { - return; - }; + let scope = + blueprint_collapse_scope(ctx, &Item::DataResult(*view_id, instance_path.clone())); - subtree.visit_children_recursively(|entity_path| { - CollapseScope::BlueprintTree - .data_result(*view_id, entity_path.clone()) - .set_open(&ctx.egui_context, self.open()); - }); + self.process_data_result_impl(ctx, view_id, instance_path, scope); } fn process_instance_path(&self, ctx: &ContextMenuContext<'_>, instance_path: &InstancePath) { @@ -108,11 +90,6 @@ impl ContextMenuAction for CollapseExpandAllAction { .selection .context_for_item(&Item::InstancePath(instance_path.clone())) { - Some(&ItemContext::StreamsTree { - store_kind: StoreKind::Recording, - filter_session_id: None, - }) => (ctx.viewer_context.recording(), CollapseScope::StreamsTree), - Some(&ItemContext::StreamsTree { store_kind: StoreKind::Recording, filter_session_id: Some(session_id), @@ -137,10 +114,18 @@ impl ContextMenuAction for CollapseExpandAllAction { CollapseScope::BlueprintStreamsTreeFiltered { session_id }, ), - // default to recording if we don't have more specific information - Some(&ItemContext::TwoD { .. } | &ItemContext::ThreeD { .. }) | None => { - (ctx.viewer_context.recording(), CollapseScope::StreamsTree) - } + // default to recording if the item context explicitly points to it or if we don't have + // any relevant context + Some( + &ItemContext::StreamsTree { + store_kind: StoreKind::Recording, + filter_session_id: None, + } + | &ItemContext::TwoD { .. } + | &ItemContext::ThreeD { .. } + | &ItemContext::BlueprintTree { .. }, + ) + | None => (ctx.viewer_context.recording(), CollapseScope::StreamsTree), }; let Some(subtree) = db.tree().subtree(&instance_path.entity_path) else { @@ -162,4 +147,70 @@ impl CollapseExpandAllAction { Self::ExpandAll => true, } } + + fn process_view_impl( + &self, + ctx: &ContextMenuContext<'_>, + view_id: &ViewId, + scope: CollapseScope, + ) { + scope + .view(*view_id) + .set_open(&ctx.egui_context, self.open()); + + let query_result = ctx.viewer_context.lookup_query_result(*view_id); + let result_tree = &query_result.tree; + if let Some(root_node) = result_tree.root_node() { + self.process_data_result_impl( + ctx, + view_id, + &InstancePath::entity_all(root_node.data_result.entity_path.clone()), + scope, + ); + } + } + + fn process_data_result_impl( + &self, + ctx: &ContextMenuContext<'_>, + view_id: &ViewId, + instance_path: &InstancePath, + scope: CollapseScope, + ) { + //TODO(ab): here we should in principle walk the DataResult tree instead of the entity tree + // but the current API isn't super ergonomic. + let Some(subtree) = ctx + .viewer_context + .recording() + .tree() + .subtree(&instance_path.entity_path) + else { + return; + }; + + subtree.visit_children_recursively(|entity_path| { + scope + .data_result(*view_id, entity_path.clone()) + .set_open(&ctx.egui_context, self.open()); + }); + } +} + +/// Determine the [`CollapseScope`] to use for items in the blueprint tree. +fn blueprint_collapse_scope(ctx: &ContextMenuContext<'_>, item: &Item) -> CollapseScope { + match ctx.selection.context_for_item(item) { + Some(&ItemContext::BlueprintTree { + filter_session_id: Some(session_id), + }) => CollapseScope::BlueprintTreeFiltered { session_id }, + + None + | Some( + &ItemContext::BlueprintTree { + filter_session_id: None, + } + | &ItemContext::StreamsTree { .. } + | &ItemContext::TwoD { .. } + | &ItemContext::ThreeD { .. }, + ) => CollapseScope::BlueprintTree, + } } diff --git a/crates/viewer/re_time_panel/src/lib.rs b/crates/viewer/re_time_panel/src/lib.rs index 0f247d47e4b6..6dacd912ea0e 100644 --- a/crates/viewer/re_time_panel/src/lib.rs +++ b/crates/viewer/re_time_panel/src/lib.rs @@ -752,7 +752,7 @@ impl TimePanel { let id = collapse_scope.entity(tree.path.clone()).into(); let is_short_path = tree.path.len() <= 1 && !tree.is_leaf(); - let default_open = self.filter_state.session_id().is_some() || is_short_path; + let default_open = self.filter_state.is_active() || is_short_path; let list_item::ShowCollapsingResponse { item_response: response, diff --git a/crates/viewer/re_view_spatial/src/ui_3d.rs b/crates/viewer/re_view_spatial/src/ui_3d.rs index ce9baf90c197..a5e1b6d8321a 100644 --- a/crates/viewer/re_view_spatial/src/ui_3d.rs +++ b/crates/viewer/re_view_spatial/src/ui_3d.rs @@ -907,7 +907,9 @@ fn show_projections_from_2d_space( } } } - ItemContext::ThreeD { .. } | ItemContext::StreamsTree { .. } => {} + ItemContext::ThreeD { .. } + | ItemContext::StreamsTree { .. } + | ItemContext::BlueprintTree { .. } => {} } } diff --git a/crates/viewer/re_viewer_context/src/collapsed_id.rs b/crates/viewer/re_viewer_context/src/collapsed_id.rs index 2eec7eb4c3d2..3471c1532926 100644 --- a/crates/viewer/re_viewer_context/src/collapsed_id.rs +++ b/crates/viewer/re_viewer_context/src/collapsed_id.rs @@ -25,6 +25,9 @@ pub enum CollapseScope { /// Blueprint tree from the blueprint panel (left panel) BlueprintTree, + + /// Blueprint tree from the blueprint panel (left panel), when the filter is active + BlueprintTreeFiltered { session_id: egui::Id }, } impl CollapseScope { diff --git a/crates/viewer/re_viewer_context/src/selection_state.rs b/crates/viewer/re_viewer_context/src/selection_state.rs index 14c9009370aa..f3719d424d49 100644 --- a/crates/viewer/re_viewer_context/src/selection_state.rs +++ b/crates/viewer/re_viewer_context/src/selection_state.rs @@ -41,6 +41,13 @@ pub enum ItemContext { /// Which store does this streams tree correspond to? store_kind: StoreKind, + /// The current entity filter session id, if any. + filter_session_id: Option, + }, + + /// Hovering/selecting in the blueprint tree. + BlueprintTree { + /// The current entity filter session id, if any. filter_session_id: Option, }, } From a115a6710c4cd63ab3a60cacf233c98ba6e79404 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 16 Jan 2025 11:35:48 +0100 Subject: [PATCH 09/13] typo --- crates/viewer/re_ui/src/filter_widget.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/viewer/re_ui/src/filter_widget.rs b/crates/viewer/re_ui/src/filter_widget.rs index 91e7cae64130..16b72f1e5d41 100644 --- a/crates/viewer/re_ui/src/filter_widget.rs +++ b/crates/viewer/re_ui/src/filter_widget.rs @@ -133,8 +133,8 @@ impl FilterState { ui.spacing_mut().text_edit_width = (ui.max_rect().width() - 10.0).at_least(0.0); - // TODO(ab): ideally _all_ text edit would be styled this way, but we requre egui - // support for that (https://github.com/emilk/egui/issues/3284) + // TODO(ab): ideally _all_ text edits would be styled this way, but we + // require egui support for that (https://github.com/emilk/egui/issues/3284) ui.visuals_mut().widgets.hovered.expansion = 0.0; ui.visuals_mut().widgets.active.expansion = 0.0; ui.visuals_mut().widgets.open.expansion = 0.0; From a476bf612cea6ba8b729ab8bc6878cae45275f9f Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Thu, 16 Jan 2025 14:37:43 +0100 Subject: [PATCH 10/13] add per-app-id invalidation (same as for time panel) --- .../re_blueprint_tree/src/blueprint_tree.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index 824a4d2671d8..341191ba6145 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -5,7 +5,7 @@ use smallvec::SmallVec; use re_context_menu::{context_menu_ui_for_item_with_context, SelectionUpdateBehavior}; use re_data_ui::item_ui::guess_instance_path_icon; use re_entity_db::InstancePath; -use re_log_types::EntityPath; +use re_log_types::{ApplicationId, EntityPath}; use re_types::blueprint::components::Visible; use re_ui::{ drag_and_drop::DropTarget, filter_widget, list_item, ContextExt as _, DesignTokens, UiExt as _, @@ -40,7 +40,14 @@ pub struct BlueprintTree { /// We double-buffer this value to deal with ordering constraints. next_candidate_drop_parent_container_id: Option, + /// State of the entity filter widget. filter_state: filter_widget::FilterState, + + /// The store id the filter widget relates to. + /// + /// Used to invalidate the filter state (aka deactivate it) when the user switches to a + /// recording with a different application id. + filter_state_app_id: Option, } impl BlueprintTree { @@ -51,6 +58,12 @@ impl BlueprintTree { viewport: &ViewportBlueprint, ui: &mut egui::Ui, ) { + // Invalidate the filter widget if the store id has changed. + if self.filter_state_app_id.as_ref() != Some(&ctx.store_context.app_id) { + self.filter_state = Default::default(); + self.filter_state_app_id = Some(ctx.store_context.app_id.clone()); + } + ui.panel_content(|ui| { ui.full_span_separator(); ui.add_space(-1.); From 4820455ab1c103d6951db47b7a9891ff4863f037 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 17 Jan 2025 17:08:59 +0100 Subject: [PATCH 11/13] Move all the logic for collapsing/expanding in a separate, easier to reuse module. --- .../src/actions/collapse_expand_all.rs | 97 +++++-------------- .../re_context_menu/src/collapse_expand.rs | 88 +++++++++++++++++ crates/viewer/re_context_menu/src/lib.rs | 1 + 3 files changed, 114 insertions(+), 72 deletions(-) create mode 100644 crates/viewer/re_context_menu/src/collapse_expand.rs diff --git a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs index 65250d27c88f..0a14e4a44507 100644 --- a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs +++ b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs @@ -1,7 +1,15 @@ +//! Context menu action to expand and collapse various trees in the UI. +//! +//! Note: the actual collapse/expand logic is in [`crate::collapse_expand`]. + use re_entity_db::InstancePath; use re_log_types::StoreKind; -use re_viewer_context::{CollapseScope, ContainerId, Contents, Item, ItemContext, ViewId}; +use re_viewer_context::{CollapseScope, ContainerId, Item, ItemContext, ViewId}; +use crate::collapse_expand::{ + collapse_expand_container, collapse_expand_data_result, collapse_expand_instance_path, + collapse_expand_view, +}; use crate::{ContextMenuAction, ContextMenuContext}; /// Collapse or expand all items in the selection. @@ -52,25 +60,19 @@ impl ContextMenuAction for CollapseExpandAllAction { fn process_container(&self, ctx: &ContextMenuContext<'_>, container_id: &ContainerId) { let scope = blueprint_collapse_scope(ctx, &Item::Container(*container_id)); - ctx.viewport_blueprint - .visit_contents_in_container(container_id, &mut |contents, _| { - match contents { - Contents::Container(container_id) => scope - .container(*container_id) - .set_open(&ctx.egui_context, self.open()), - - // IMPORTANT: don't call process_view() here, or the scope information would be lost - Contents::View(view_id) => self.process_view_impl(ctx, view_id, scope), - } - - true - }); + collapse_expand_container( + ctx.viewer_context, + ctx.viewport_blueprint, + container_id, + scope, + self.open(), + ); } fn process_view(&self, ctx: &ContextMenuContext<'_>, view_id: &ViewId) { let scope = blueprint_collapse_scope(ctx, &Item::View(*view_id)); - self.process_view_impl(ctx, view_id, scope); + collapse_expand_view(ctx.viewer_context, view_id, scope, self.open()); } fn process_data_result( @@ -82,7 +84,13 @@ impl ContextMenuAction for CollapseExpandAllAction { let scope = blueprint_collapse_scope(ctx, &Item::DataResult(*view_id, instance_path.clone())); - self.process_data_result_impl(ctx, view_id, instance_path, scope); + collapse_expand_data_result( + ctx.viewer_context, + view_id, + instance_path, + scope, + self.open(), + ); } fn process_instance_path(&self, ctx: &ContextMenuContext<'_>, instance_path: &InstancePath) { @@ -128,15 +136,7 @@ impl ContextMenuAction for CollapseExpandAllAction { | None => (ctx.viewer_context.recording(), CollapseScope::StreamsTree), }; - let Some(subtree) = db.tree().subtree(&instance_path.entity_path) else { - return; - }; - - subtree.visit_children_recursively(|entity_path| { - scope - .entity(entity_path.clone()) - .set_open(&ctx.egui_context, self.open()); - }); + collapse_expand_instance_path(&ctx.viewer_context, db, instance_path, scope, self.open()); } } @@ -147,53 +147,6 @@ impl CollapseExpandAllAction { Self::ExpandAll => true, } } - - fn process_view_impl( - &self, - ctx: &ContextMenuContext<'_>, - view_id: &ViewId, - scope: CollapseScope, - ) { - scope - .view(*view_id) - .set_open(&ctx.egui_context, self.open()); - - let query_result = ctx.viewer_context.lookup_query_result(*view_id); - let result_tree = &query_result.tree; - if let Some(root_node) = result_tree.root_node() { - self.process_data_result_impl( - ctx, - view_id, - &InstancePath::entity_all(root_node.data_result.entity_path.clone()), - scope, - ); - } - } - - fn process_data_result_impl( - &self, - ctx: &ContextMenuContext<'_>, - view_id: &ViewId, - instance_path: &InstancePath, - scope: CollapseScope, - ) { - //TODO(ab): here we should in principle walk the DataResult tree instead of the entity tree - // but the current API isn't super ergonomic. - let Some(subtree) = ctx - .viewer_context - .recording() - .tree() - .subtree(&instance_path.entity_path) - else { - return; - }; - - subtree.visit_children_recursively(|entity_path| { - scope - .data_result(*view_id, entity_path.clone()) - .set_open(&ctx.egui_context, self.open()); - }); - } } /// Determine the [`CollapseScope`] to use for items in the blueprint tree. diff --git a/crates/viewer/re_context_menu/src/collapse_expand.rs b/crates/viewer/re_context_menu/src/collapse_expand.rs new file mode 100644 index 000000000000..61886f095fed --- /dev/null +++ b/crates/viewer/re_context_menu/src/collapse_expand.rs @@ -0,0 +1,88 @@ +//! Logic that implements the collapse/expand functionality. +//! +//! This is separated from the corresponding context menu action, so it may be reused directly, in +//! particular in tests. + +use re_entity_db::{EntityDb, InstancePath}; +use re_viewer_context::{CollapseScope, ContainerId, Contents, ViewId, ViewerContext}; +use re_viewport_blueprint::ViewportBlueprint; + +pub fn collapse_expand_container( + ctx: &ViewerContext<'_>, + blueprint: &ViewportBlueprint, + container_id: &ContainerId, + scope: CollapseScope, + expand: bool, +) { + blueprint.visit_contents_in_container(container_id, &mut |contents, _| { + match contents { + Contents::Container(container_id) => scope + .container(*container_id) + .set_open(ctx.egui_ctx, expand), + + // IMPORTANT: don't call process_view() here, or the scope information would be lost + Contents::View(view_id) => collapse_expand_view(ctx, view_id, scope, expand), + } + + true + }); +} + +pub fn collapse_expand_view( + ctx: &ViewerContext<'_>, + view_id: &ViewId, + scope: CollapseScope, + expand: bool, +) { + scope.view(*view_id).set_open(&ctx.egui_ctx, expand); + + let query_result = ctx.lookup_query_result(*view_id); + let result_tree = &query_result.tree; + if let Some(root_node) = result_tree.root_node() { + collapse_expand_data_result( + ctx, + view_id, + &InstancePath::entity_all(root_node.data_result.entity_path.clone()), + scope, + expand, + ); + } +} + +pub fn collapse_expand_data_result( + ctx: &ViewerContext<'_>, + view_id: &ViewId, + instance_path: &InstancePath, + scope: CollapseScope, + expand: bool, +) { + //TODO(ab): here we should in principle walk the DataResult tree instead of the entity tree + // but the current API isn't super ergonomic. + let Some(subtree) = ctx.recording().tree().subtree(&instance_path.entity_path) else { + return; + }; + + subtree.visit_children_recursively(|entity_path| { + scope + .data_result(*view_id, entity_path.clone()) + .set_open(&ctx.egui_ctx, expand); + }); +} + +pub fn collapse_expand_instance_path( + ctx: &ViewerContext<'_>, + db: &EntityDb, + instance_path: &InstancePath, + scope: CollapseScope, + expand: bool, +) { + let Some(subtree) = db.tree().subtree(&instance_path.entity_path) else { + return; + }; + + subtree.visit_children_recursively(|entity_path| { + scope + .entity(entity_path.clone()) + .set_open(&ctx.egui_ctx, expand); + }); +} diff --git a/crates/viewer/re_context_menu/src/lib.rs b/crates/viewer/re_context_menu/src/lib.rs index 1d921964449b..69e244bf937f 100644 --- a/crates/viewer/re_context_menu/src/lib.rs +++ b/crates/viewer/re_context_menu/src/lib.rs @@ -9,6 +9,7 @@ use re_viewer_context::{ use re_viewport_blueprint::{ContainerBlueprint, ViewportBlueprint}; mod actions; +pub mod collapse_expand; mod sub_menu; use actions::{ From df8dd4e1a57b8794aa132124ebffeeaf385b55d1 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 17 Jan 2025 17:15:11 +0100 Subject: [PATCH 12/13] update invalidated test snapshot --- crates/viewer/re_ui/tests/snapshots/filter_widget.png | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_ui/tests/snapshots/filter_widget.png b/crates/viewer/re_ui/tests/snapshots/filter_widget.png index d6a3bc390738..3f20ea110903 100644 --- a/crates/viewer/re_ui/tests/snapshots/filter_widget.png +++ b/crates/viewer/re_ui/tests/snapshots/filter_widget.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:73136b6632393d094da0165a6983c42068275c6b92bc69030c0bfb081db4ce64 +oid sha256:ee052e2bae78fd1725f5425194ab6c1e0b781d25d847288f51a59a44e26428f9 size 10226 From 522b57d4f3fe1a63532ac1f38f52d4d9490e2438 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler Date: Fri, 17 Jan 2025 17:40:00 +0100 Subject: [PATCH 13/13] lints --- .../re_context_menu/src/actions/collapse_expand_all.rs | 2 +- crates/viewer/re_context_menu/src/collapse_expand.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs index 0a14e4a44507..4af8a95927c9 100644 --- a/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs +++ b/crates/viewer/re_context_menu/src/actions/collapse_expand_all.rs @@ -136,7 +136,7 @@ impl ContextMenuAction for CollapseExpandAllAction { | None => (ctx.viewer_context.recording(), CollapseScope::StreamsTree), }; - collapse_expand_instance_path(&ctx.viewer_context, db, instance_path, scope, self.open()); + collapse_expand_instance_path(ctx.viewer_context, db, instance_path, scope, self.open()); } } diff --git a/crates/viewer/re_context_menu/src/collapse_expand.rs b/crates/viewer/re_context_menu/src/collapse_expand.rs index 61886f095fed..4fddc53b9e21 100644 --- a/crates/viewer/re_context_menu/src/collapse_expand.rs +++ b/crates/viewer/re_context_menu/src/collapse_expand.rs @@ -34,7 +34,7 @@ pub fn collapse_expand_view( scope: CollapseScope, expand: bool, ) { - scope.view(*view_id).set_open(&ctx.egui_ctx, expand); + scope.view(*view_id).set_open(ctx.egui_ctx, expand); let query_result = ctx.lookup_query_result(*view_id); let result_tree = &query_result.tree; @@ -65,7 +65,7 @@ pub fn collapse_expand_data_result( subtree.visit_children_recursively(|entity_path| { scope .data_result(*view_id, entity_path.clone()) - .set_open(&ctx.egui_ctx, expand); + .set_open(ctx.egui_ctx, expand); }); } @@ -83,6 +83,6 @@ pub fn collapse_expand_instance_path( subtree.visit_children_recursively(|entity_path| { scope .entity(entity_path.clone()) - .set_open(&ctx.egui_ctx, expand); + .set_open(ctx.egui_ctx, expand); }); }