From f779d8befbdbdf97925def48f6dfc7b5c4a34e12 Mon Sep 17 00:00:00 2001 From: Antoine Beyeler <49431240+abey79@users.noreply.github.com> Date: Tue, 14 Jan 2025 11:02:44 +0100 Subject: [PATCH 1/2] Filter entities in the UI (part 3): Move action to a menu in the blueprint panel and keep default blueprint when using heuristics (#8672) Co-authored-by: Emil Ernerfeldt --- .../re_blueprint_tree/src/blueprint_tree.rs | 148 +++++++++++------- crates/viewer/re_data_ui/src/app_id.rs | 48 +----- crates/viewer/re_ui/src/command.rs | 15 +- crates/viewer/re_ui/src/ui_ext.rs | 10 ++ crates/viewer/re_view_graph/tests/basic.rs | 1 + crates/viewer/re_viewer/src/app.rs | 12 +- crates/viewer/re_viewer/src/app_state.rs | 7 + .../re_viewer_context/src/command_sender.rs | 18 ++- .../re_viewer_context/src/store_context.rs | 3 + .../viewer/re_viewer_context/src/store_hub.rs | 39 +++-- .../re_viewer_context/src/test_context.rs | 3 +- .../viewer/re_viewport_blueprint/src/view.rs | 1 + .../src/view_contents.rs | 1 + 13 files changed, 172 insertions(+), 134 deletions(-) diff --git a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs index 855f058e2bfb..f88326f5ae72 100644 --- a/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs +++ b/crates/viewer/re_blueprint_tree/src/blueprint_tree.rs @@ -49,14 +49,25 @@ impl BlueprintTree { ui: &mut egui::Ui, ) { ui.panel_content(|ui| { - ui.panel_title_bar_with_buttons( - "Blueprint", - Some("The blueprint is where you can configure the Rerun Viewer"), - |ui| { - self.add_new_view_button_ui(ctx, viewport, ui); - reset_blueprint_button_ui(ctx, ui); - }, - ); + ui.full_span_separator(); + 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); + }), + ); + }); + + ui.full_span_separator(); }); // This call is excluded from `panel_content` because it has a ScrollArea, which should not be @@ -602,32 +613,6 @@ impl BlueprintTree { ctx.handle_select_hover_drag_interactions(&response, item, true); } - /// Add a button to trigger the addition of a new view or container. - #[allow(clippy::unused_self)] - fn add_new_view_button_ui( - &self, - ctx: &ViewerContext<'_>, - viewport: &ViewportBlueprint, - ui: &mut egui::Ui, - ) { - if ui - .small_icon_button(&re_ui::icons::ADD) - .on_hover_text("Add a new view or container") - .clicked() - { - // If a single container is selected, we use it as target. Otherwise, we target the - // root container. - let target_container_id = - if let Some(Item::Container(container_id)) = ctx.selection().single_item() { - *container_id - } else { - viewport.root_container - }; - - show_add_view_or_container_modal(target_container_id); - } - } - // ---------------------------------------------------------------------------- // drag and drop support @@ -868,7 +853,35 @@ impl BlueprintTree { // ---------------------------------------------------------------------------- -fn reset_blueprint_button_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { +/// Add a button to trigger the addition of a new view or container. +fn add_new_view_or_container_menu_button( + ctx: &ViewerContext<'_>, + viewport: &ViewportBlueprint, + ui: &mut egui::Ui, +) { + if ui + .add(egui::Button::image_and_text( + &re_ui::icons::ADD, + "Add view or container…", + )) + .clicked() + { + ui.close_menu(); + + // If a single container is selected, we use it as target. Otherwise, we target the + // root container. + let target_container_id = + if let Some(Item::Container(container_id)) = ctx.selection().single_item() { + *container_id + } else { + viewport.root_container + }; + + show_add_view_or_container_modal(target_container_id); + } +} + +fn set_blueprint_to_default_menu_buttons(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { let default_blueprint_id = ctx .store_context .hub @@ -876,38 +889,61 @@ fn reset_blueprint_button_ui(ctx: &ViewerContext<'_>, ui: &mut egui::Ui) { let default_blueprint = default_blueprint_id.and_then(|id| ctx.store_context.bundle.get(id)); - let mut disabled_reason = None; - - if let Some(default_blueprint) = default_blueprint { - let active_is_clone_of_default = Some(default_blueprint.store_id()).as_ref() - == ctx.store_context.blueprint.cloned_from(); - let last_modified_at_the_same_time = - default_blueprint.latest_row_id() == ctx.store_context.blueprint.latest_row_id(); - if active_is_clone_of_default && last_modified_at_the_same_time { - disabled_reason = Some("No modifications have been made"); + let disabled_reason = match default_blueprint { + None => Some("No default blueprint is set for this app"), + Some(default_blueprint) => { + let active_is_clone_of_default = Some(default_blueprint.store_id()).as_ref() + == ctx.store_context.blueprint.cloned_from(); + let last_modified_at_the_same_time = + default_blueprint.latest_row_id() == ctx.store_context.blueprint.latest_row_id(); + if active_is_clone_of_default && last_modified_at_the_same_time { + Some("No modifications have been made") + } else { + None // it is valid to reset to default + } } - } + }; let enabled = disabled_reason.is_none(); - let response = ui.add_enabled(enabled, ui.small_icon_button_widget(&re_ui::icons::RESET)); - - let response = if let Some(disabled_reason) = disabled_reason { - response.on_disabled_hover_text(disabled_reason) - } else { - let hover_text = if default_blueprint_id.is_some() { - "Reset to the default blueprint for this app" - } else { - "Re-populate viewport with automatically chosen views" - }; - response.on_hover_text(hover_text) + let mut response = ui + .add_enabled( + enabled, + egui::Button::image_and_text(&re_ui::icons::RESET, "Reset to default blueprint"), + ) + .on_hover_text("Reset to the default blueprint for this app"); + + if let Some(disabled_reason) = disabled_reason { + response = response.on_disabled_hover_text(disabled_reason); }; if response.clicked() { + ui.close_menu(); ctx.command_sender .send_system(re_viewer_context::SystemCommand::ClearActiveBlueprint); } } +fn set_blueprint_to_auto_menu_button( + ctx: &ViewerContext<'_>, + viewport: &ViewportBlueprint, + ui: &mut egui::Ui, +) { + let enabled = !viewport.auto_layout() || !viewport.auto_views(); + + if ui + .add_enabled( + enabled, + egui::Button::image_and_text(&re_ui::icons::RESET, "Reset to heuristic blueprint"), + ) + .on_hover_text("Re-populate viewport with automatically chosen views") + .clicked() + { + ui.close_menu(); + ctx.command_sender + .send_system(re_viewer_context::SystemCommand::ClearActiveBlueprintAndEnableHeuristics); + } +} + /// Expand all required items and compute which item we should scroll to. fn handle_focused_item( ctx: &ViewerContext<'_>, diff --git a/crates/viewer/re_data_ui/src/app_id.rs b/crates/viewer/re_data_ui/src/app_id.rs index 512289c0b63b..e5c888cc8b0e 100644 --- a/crates/viewer/re_data_ui/src/app_id.rs +++ b/crates/viewer/re_data_ui/src/app_id.rs @@ -2,7 +2,7 @@ use itertools::Itertools as _; use re_entity_db::EntityDb; use re_log_types::ApplicationId; -use re_viewer_context::{SystemCommandSender as _, UiLayout, ViewerContext}; +use re_viewer_context::{UiLayout, ViewerContext}; use crate::item_ui::entity_db_button_ui; @@ -54,51 +54,5 @@ impl crate::DataUi for ApplicationId { } }); } - - // --------------------------------------------------------------------- - // do not show UI code in tooltips - - if ui_layout != UiLayout::Tooltip { - ui.add_space(8.0); - - // --------------------------------------------------------------------- - - // Blueprint section. - let active_blueprint = ctx.store_context.blueprint; - let default_blueprint = ctx.store_context.hub.default_blueprint_for_app(self); - - let button = - egui::Button::image_and_text(&re_ui::icons::RESET, "Reset to default blueprint"); - - let is_same_as_default = default_blueprint.is_some_and(|default_blueprint| { - default_blueprint.latest_row_id() == active_blueprint.latest_row_id() - }); - - if is_same_as_default { - ui.add_enabled(false, button) - .on_disabled_hover_text("No modifications have been made"); - } else if default_blueprint.is_none() { - ui.add_enabled(false, button) - .on_disabled_hover_text("There's no default blueprint"); - } else { - // The active blueprint is different from the default blueprint - if ui - .add(button) - .on_hover_text("Reset to the default blueprint for this app") - .clicked() - { - ctx.command_sender - .send_system(re_viewer_context::SystemCommand::ClearActiveBlueprint); - } - } - - if ui.add(egui::Button::image_and_text( - &re_ui::icons::RESET, - "Reset to heuristic blueprint", - )).on_hover_text("Clear both active and default blueprint, and auto-generate a new blueprint based on heuristics").clicked() { - ctx.command_sender - .send_system(re_viewer_context::SystemCommand::ClearAndGenerateBlueprint); - } - } } } diff --git a/crates/viewer/re_ui/src/command.rs b/crates/viewer/re_ui/src/command.rs index 6781a2978feb..9b5863a79f1e 100644 --- a/crates/viewer/re_ui/src/command.rs +++ b/crates/viewer/re_ui/src/command.rs @@ -32,7 +32,7 @@ pub enum UICommand { OpenRerunDiscord, ResetViewer, - ClearAndGenerateBlueprint, + ClearActiveBlueprintAndEnableHeuristics, #[cfg(not(target_arch = "wasm32"))] OpenProfiler, @@ -121,7 +121,7 @@ impl UICommand { ), Self::CloseAllRecordings => ("Close all recordings", - "Close all open current recording (unsaved data will be lost)"), + "Close all open current recording (unsaved data will be lost)"), Self::Undo => ("Undo", "Undo the last blueprint edit for the open recording"), Self::Redo => ("Redo", "Redo the last undone thing"), @@ -137,12 +137,11 @@ impl UICommand { "Reset the Viewer to how it looked the first time you ran it, forgetting all stored blueprints and UI state", ), - Self::ClearAndGenerateBlueprint => ( - "Clear and generate new blueprint", - "Clear the current blueprint and generate a new one based on heuristics." + Self::ClearActiveBlueprintAndEnableHeuristics => ( + "Reset to heuristic blueprint", + "Re-populate viewport with automatically chosen views" ), - #[cfg(not(target_arch = "wasm32"))] Self::OpenProfiler => ( "Open profiler", @@ -174,7 +173,6 @@ impl UICommand { "View and change global egui style settings", ), - #[cfg(not(target_arch = "wasm32"))] Self::ToggleFullscreen => ( "Toggle fullscreen", @@ -256,7 +254,6 @@ impl UICommand { "Restart with WebGPU", "Reloads the webpage and force WebGPU for rendering. All data will be lost." ), - } } @@ -314,7 +311,7 @@ impl UICommand { Self::Quit => smallvec![cmd(Key::Q)], Self::ResetViewer => smallvec![ctrl_shift(Key::R)], - Self::ClearAndGenerateBlueprint => smallvec![], + Self::ClearActiveBlueprintAndEnableHeuristics => smallvec![], #[cfg(not(target_arch = "wasm32"))] Self::OpenProfiler => smallvec![ctrl_shift(Key::P)], diff --git a/crates/viewer/re_ui/src/ui_ext.rs b/crates/viewer/re_ui/src/ui_ext.rs index a4eaf864eeb9..1a645758ab38 100644 --- a/crates/viewer/re_ui/src/ui_ext.rs +++ b/crates/viewer/re_ui/src/ui_ext.rs @@ -655,6 +655,16 @@ pub trait UiExt { self.ui().painter().add(shadow); } + /// Convenience function to create a [`list_item::list_item_scope`]. + #[inline] + fn list_item_scope( + &mut self, + id_salt: impl std::hash::Hash, + content: impl FnOnce(&mut egui::Ui) -> R, + ) -> R { + list_item::list_item_scope(self.ui_mut(), id_salt, content) + } + /// Convenience function to create a [`list_item::ListItem`]. #[allow(clippy::unused_self)] fn list_item(&self) -> list_item::ListItem { diff --git a/crates/viewer/re_view_graph/tests/basic.rs b/crates/viewer/re_view_graph/tests/basic.rs index 4701354e814c..c34fb9e7fc31 100644 --- a/crates/viewer/re_view_graph/tests/basic.rs +++ b/crates/viewer/re_view_graph/tests/basic.rs @@ -188,6 +188,7 @@ fn run_graph_view_and_save_snapshot( bundle: &Default::default(), caches: &Default::default(), hub: &Default::default(), + should_enable_heuristics: false, }; // Execute the queries for every `View` diff --git a/crates/viewer/re_viewer/src/app.rs b/crates/viewer/re_viewer/src/app.rs index 934705af994b..678a21100709 100644 --- a/crates/viewer/re_viewer/src/app.rs +++ b/crates/viewer/re_viewer/src/app.rs @@ -524,12 +524,10 @@ impl App { } SystemCommand::ResetViewer => self.reset_viewer(store_hub, egui_ctx), - SystemCommand::ClearAndGenerateBlueprint => { + SystemCommand::ClearActiveBlueprintAndEnableHeuristics => { re_log::debug!("Clear and generate new blueprint"); - // By clearing the default blueprint and the active blueprint - // it will be re-generated based on the default auto behavior. - store_hub.clear_default_blueprint(); - store_hub.clear_active_blueprint(); + store_hub.clear_active_blueprint_and_generate(); + egui_ctx.request_repaint(); // Many changes take a frame delay to show up. } SystemCommand::ClearActiveBlueprint => { // By clearing the blueprint the default blueprint will be restored @@ -779,9 +777,9 @@ impl App { } UICommand::ResetViewer => self.command_sender.send_system(SystemCommand::ResetViewer), - UICommand::ClearAndGenerateBlueprint => { + UICommand::ClearActiveBlueprintAndEnableHeuristics => { self.command_sender - .send_system(SystemCommand::ClearAndGenerateBlueprint); + .send_system(SystemCommand::ClearActiveBlueprintAndEnableHeuristics); } #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/viewer/re_viewer/src/app_state.rs b/crates/viewer/re_viewer/src/app_state.rs index e4bc1ad4cee2..a29e202065d2 100644 --- a/crates/viewer/re_viewer/src/app_state.rs +++ b/crates/viewer/re_viewer/src/app_state.rs @@ -282,6 +282,13 @@ impl AppState { drag_and_drop_manager: &drag_and_drop_manager, }; + // enable the heuristics if we must this frame + if store_context.should_enable_heuristics { + viewport_ui.blueprint.set_auto_layout(true, &ctx); + viewport_ui.blueprint.set_auto_views(true, &ctx); + egui_ctx.request_repaint(); + } + // We move the time at the very start of the frame, // so that we always show the latest data when we're in "follow" mode. move_time(&ctx, recording, rx); diff --git a/crates/viewer/re_viewer_context/src/command_sender.rs b/crates/viewer/re_viewer_context/src/command_sender.rs index 1f0fb34acea5..63c7a1b7fc45 100644 --- a/crates/viewer/re_viewer_context/src/command_sender.rs +++ b/crates/viewer/re_viewer_context/src/command_sender.rs @@ -27,11 +27,23 @@ pub enum SystemCommand { /// Reset the `Viewer` to the default state ResetViewer, - /// Reset the `Blueprint` to the default state + /// Clear the active blueprint. + /// + /// This may have two outcomes: + /// - If a default blueprint is set, it will be used. + /// - Otherwise, the heuristics will be enabled. + /// + /// To force using the heuristics, use [`Self::ClearActiveBlueprintAndEnableHeuristics`]. + /// + /// UI note: because of the above ambiguity, controls for this command should only be enabled if + /// a default blueprint is set. ClearActiveBlueprint, - /// Clear the blueprint and generate a new one - ClearAndGenerateBlueprint, + /// Clear the active blueprint and enable heuristics. + /// + /// The final outcome of this is to set the active blueprint to the heuristics. This command + /// does not affect the default blueprint if any was set. + ClearActiveBlueprintAndEnableHeuristics, /// If this is a recording, switch to it. ActivateRecording(StoreId), diff --git a/crates/viewer/re_viewer_context/src/store_context.rs b/crates/viewer/re_viewer_context/src/store_context.rs index a50405f6e211..659592efd311 100644 --- a/crates/viewer/re_viewer_context/src/store_context.rs +++ b/crates/viewer/re_viewer_context/src/store_context.rs @@ -29,6 +29,9 @@ pub struct StoreContext<'a> { /// The store hub, which keeps track of all the default and active blueprints, among other things. pub hub: &'a StoreHub, + + /// Should we enable the heuristics during this frame? + pub should_enable_heuristics: bool, } impl StoreContext<'_> { diff --git a/crates/viewer/re_viewer_context/src/store_hub.rs b/crates/viewer/re_viewer_context/src/store_hub.rs index 977ac4f3e167..bc01b1c98c29 100644 --- a/crates/viewer/re_viewer_context/src/store_hub.rs +++ b/crates/viewer/re_viewer_context/src/store_hub.rs @@ -45,6 +45,9 @@ pub struct StoreHub { active_blueprint_by_app_id: HashMap, store_bundle: StoreBundle, + /// These applications should enable the heuristics early next frame. + should_enable_heuristics_by_app_id: HashSet, + /// Things that need caching. caches_per_recording: HashMap, @@ -143,6 +146,8 @@ impl StoreHub { active_blueprint_by_app_id: Default::default(), store_bundle, + should_enable_heuristics_by_app_id: Default::default(), + caches_per_recording: Default::default(), blueprint_last_save: Default::default(), blueprint_last_gc: Default::default(), @@ -184,8 +189,11 @@ impl StoreHub { } } - // If there's no active blueprint for this app, try to make the current default one active. - if !self.active_blueprint_by_app_id.contains_key(&app_id) { + // If there's no active blueprint for this app, we must use the default blueprint, UNLESS + // we're about to enable heuristics for this app. + if !self.active_blueprint_by_app_id.contains_key(&app_id) + && !self.should_enable_heuristics_by_app_id.contains(&app_id) + { if let Some(blueprint_id) = self.default_blueprint_by_app_id.get(&app_id).cloned() { self.set_cloned_blueprint_active_for_app(&app_id, &blueprint_id) .unwrap_or_else(|err| { @@ -220,6 +228,7 @@ impl StoreHub { self.active_rec_id = None; } + let should_enable_heuristics = self.should_enable_heuristics_by_app_id.remove(&app_id); let caches = self.active_caches(); Some(StoreContext { @@ -230,6 +239,7 @@ impl StoreHub { bundle: &self.store_bundle, caches: caches.unwrap_or(&EMPTY_CACHES), hub: self, + should_enable_heuristics, }) } @@ -497,15 +507,6 @@ impl StoreHub { Ok(()) } - /// Clear the current default blueprint - pub fn clear_default_blueprint(&mut self) { - if let Some(app_id) = &self.active_application_id { - if let Some(blueprint_id) = self.default_blueprint_by_app_id.remove(app_id) { - self.remove(&blueprint_id); - } - } - } - // --------------------- // Active blueprint @@ -578,6 +579,22 @@ impl StoreHub { } } + /// Clear the currently active blueprint and enable the heuristics to generate a new one. + /// + /// These keeps the default blueprint as is, so the user may reset to the default blueprint + /// afterward. + pub fn clear_active_blueprint_and_generate(&mut self) { + self.clear_active_blueprint(); + + // Simply clearing the default blueprint would trigger a reset to default. Instead, we must + // actively set the blueprint to use the heuristics, so we store a flag so we do that early + // next frame. + if let Some(app_id) = self.active_app() { + self.should_enable_heuristics_by_app_id + .insert(app_id.clone()); + } + } + // --------------------- // Misc operations diff --git a/crates/viewer/re_viewer_context/src/test_context.rs b/crates/viewer/re_viewer_context/src/test_context.rs index 427e84733659..baf122c5c77b 100644 --- a/crates/viewer/re_viewer_context/src/test_context.rs +++ b/crates/viewer/re_viewer_context/src/test_context.rs @@ -281,6 +281,7 @@ impl TestContext { bundle: &Default::default(), caches: &Default::default(), hub: &Default::default(), + should_enable_heuristics: false, }; let drag_and_drop_manager = crate::DragAndDropManager::new(ItemCollection::default()); @@ -398,7 +399,7 @@ impl TestContext { | SystemCommand::AddReceiver(_) | SystemCommand::ResetViewer | SystemCommand::ClearActiveBlueprint - | SystemCommand::ClearAndGenerateBlueprint + | SystemCommand::ClearActiveBlueprintAndEnableHeuristics | SystemCommand::ActivateRecording(_) | SystemCommand::CloseStore(_) | SystemCommand::UndoBlueprint { .. } diff --git a/crates/viewer/re_viewport_blueprint/src/view.rs b/crates/viewer/re_viewport_blueprint/src/view.rs index be58f4f85116..e040eadcf27a 100644 --- a/crates/viewer/re_viewport_blueprint/src/view.rs +++ b/crates/viewer/re_viewport_blueprint/src/view.rs @@ -760,6 +760,7 @@ mod tests { bundle: &Default::default(), caches: &Default::default(), hub: &re_viewer_context::StoreHub::test_hub(), + should_enable_heuristics: false, }; let mut query_result = view.contents.execute_query( diff --git a/crates/viewer/re_viewport_blueprint/src/view_contents.rs b/crates/viewer/re_viewport_blueprint/src/view_contents.rs index ef96719299b7..d2d9068ac9e6 100644 --- a/crates/viewer/re_viewport_blueprint/src/view_contents.rs +++ b/crates/viewer/re_viewport_blueprint/src/view_contents.rs @@ -668,6 +668,7 @@ mod tests { bundle: &Default::default(), caches: &Default::default(), hub: &StoreHub::test_hub(), + should_enable_heuristics: false, }; struct Scenario { From 63012cd9a28245cd0572068ca74ec20c1dd92f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Proch=C3=A1zka?= Date: Tue, 14 Jan 2025 11:46:58 +0100 Subject: [PATCH 2/2] Fix unused imports (#8678) --- .../store/re_log_encoding/src/protobuf_conversions.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/store/re_log_encoding/src/protobuf_conversions.rs b/crates/store/re_log_encoding/src/protobuf_conversions.rs index 4ad6580d2950..f3193575907a 100644 --- a/crates/store/re_log_encoding/src/protobuf_conversions.rs +++ b/crates/store/re_log_encoding/src/protobuf_conversions.rs @@ -1,7 +1,3 @@ -use re_protos::missing_field; - -use crate::codec::CodecError; - impl From for crate::Compression { fn from(value: re_protos::log_msg::v0::Compression) -> Self { match value { @@ -24,9 +20,12 @@ impl From for re_protos::log_msg::v0::Compression { pub fn log_msg_from_proto( message: re_protos::log_msg::v0::LogMsg, ) -> Result { - use crate::codec::arrow::decode_arrow; + use crate::codec::{arrow::decode_arrow, CodecError}; use crate::decoder::DecodeError; - use re_protos::log_msg::v0::{log_msg::Msg, Encoding}; + use re_protos::{ + log_msg::v0::{log_msg::Msg, Encoding}, + missing_field, + }; match message.msg { Some(Msg::SetStoreInfo(set_store_info)) => Ok(re_log_types::LogMsg::SetStoreInfo(