diff --git a/Cargo.lock b/Cargo.lock index b071cb0aacb1..1e04517c3353 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -59,7 +59,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f47983a1084940ba9a39c077a8c63e55c619388be5476ac04c804cfbd1e63459" dependencies = [ "accesskit", - "hashbrown 0.15.1", + "hashbrown 0.15.2", "immutable-chunkmap", ] @@ -71,7 +71,7 @@ checksum = "7329821f3bd1101e03a7d2e03bd339e3ac0dc64c70b4c9f9ae1949e3ba8dece1" dependencies = [ "accesskit", "accesskit_consumer 0.26.0", - "hashbrown 0.15.1", + "hashbrown 0.15.2", "objc2", "objc2-app-kit", "objc2-foundation", @@ -103,7 +103,7 @@ checksum = "24fcd5d23d70670992b823e735e859374d694a3d12bfd8dd32bd3bd8bedb5d81" dependencies = [ "accesskit", "accesskit_consumer 0.26.0", - "hashbrown 0.15.1", + "hashbrown 0.15.2", "paste", "static_assertions", "windows 0.58.0", @@ -2998,9 +2998,9 @@ dependencies = [ [[package]] name = "hashbrown" -version = "0.15.1" +version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3a9bfc1af68b1726ea47d3d5109de126281def866b33970e10fbab11b5dafab3" +checksum = "bf151400ff0baff5465007dd2f3e717f3fe502074ca563069ce3a6629d07b289" dependencies = [ "allocator-api2", "equivalent", @@ -3409,7 +3409,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "707907fe3c25f5424cce2cb7e1cbcafee6bdbe735ca90ef77c29e84591e5b9da" dependencies = [ "equivalent", - "hashbrown 0.15.1", + "hashbrown 0.15.2", "serde", ] @@ -3826,7 +3826,7 @@ version = "0.12.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "234cf4f4a04dc1f57e24b96cc0cd600cf2af460d4161ac5ecdd0af8e1f3b2a38" dependencies = [ - "hashbrown 0.15.1", + "hashbrown 0.15.2", ] [[package]] diff --git a/crates/viewer/re_selection_panel/src/lib.rs b/crates/viewer/re_selection_panel/src/lib.rs index 4517a2783bb3..02b267f6ea28 100644 --- a/crates/viewer/re_selection_panel/src/lib.rs +++ b/crates/viewer/re_selection_panel/src/lib.rs @@ -2,7 +2,6 @@ mod defaults_ui; mod item_title; -mod selection_history_ui; mod selection_panel; mod space_view_entity_picker; mod space_view_space_origin_ui; diff --git a/crates/viewer/re_selection_panel/src/selection_history_ui.rs b/crates/viewer/re_selection_panel/src/selection_history_ui.rs deleted file mode 100644 index 2d985e5cab73..000000000000 --- a/crates/viewer/re_selection_panel/src/selection_history_ui.rs +++ /dev/null @@ -1,216 +0,0 @@ -use egui::RichText; - -use re_ui::{UICommand, UiExt as _}; -use re_viewer_context::{Item, ItemCollection, SelectionHistory}; -use re_viewport_blueprint::ViewportBlueprint; - -// --- - -#[derive(Default, serde::Deserialize, serde::Serialize)] -#[serde(default)] -pub struct SelectionHistoryUi {} - -impl SelectionHistoryUi { - pub(crate) fn selection_ui( - &self, - ui: &mut egui::Ui, - viewport: &ViewportBlueprint, - history: &mut SelectionHistory, - ) -> Option { - let next = self.next_button_ui(ui, viewport, history); - let prev = self.prev_button_ui(ui, viewport, history); - prev.or(next) - } - - fn prev_button_ui( - &self, - ui: &mut egui::Ui, - viewport: &ViewportBlueprint, - history: &mut SelectionHistory, - ) -> Option { - // undo selection - if let Some(previous) = history.previous() { - let response = ui - .small_icon_button(&re_ui::icons::ARROW_LEFT) - .on_hover_text(format!( - "Go to previous selection{}:\n\ - {}\n\ - \n\ - Right-click for more.", - UICommand::SelectionPrevious.format_shortcut_tooltip_suffix(ui.ctx()), - selection_to_string(viewport, &previous.selection), - )); - - let mut return_current = false; - response.context_menu(|ui| { - // undo: newest on top, oldest on bottom - let cur = history.current; - for i in (0..history.current).rev() { - self.history_item_ui(viewport, ui, i, history); - } - return_current = cur != history.current; - }); - if return_current { - return history.current().map(|sel| sel.selection); - } - - // TODO(cmc): using the keyboard shortcut should highlight the associated - // button or something (but then again it, it'd make more sense to do that - // at the egui level rather than specifically here). - if response.clicked() { - return history.select_previous(); - } - } else { - ui.add_enabled_ui(false, |ui| { - ui.small_icon_button(&re_ui::icons::ARROW_LEFT) - .on_disabled_hover_text("No past selections found"); - }); - } - - None - } - - fn next_button_ui( - &self, - ui: &mut egui::Ui, - viewport: &ViewportBlueprint, - history: &mut SelectionHistory, - ) -> Option { - // redo selection - if let Some(next) = history.next() { - let response = ui - .small_icon_button(&re_ui::icons::ARROW_RIGHT) - .on_hover_text(format!( - "Go to next selection{}:\n\ - {}\n\ - \n\ - Right-click for more.", - UICommand::SelectionNext.format_shortcut_tooltip_suffix(ui.ctx()), - selection_to_string(viewport, &next.selection), - )); - - let mut return_current = false; - response.context_menu(|ui| { - // redo: oldest on top, most recent on bottom - let cur = history.current; - for i in (history.current + 1)..history.stack.len() { - self.history_item_ui(viewport, ui, i, history); - } - return_current = cur != history.current; - }); - if return_current { - return history.current().map(|sel| sel.selection); - } - - // TODO(cmc): using the keyboard shortcut should highlight the associated - // button or something (but then again it, it'd make more sense to do that - // at the egui level rather than specifically here). - if response.clicked() { - return history.select_next(); - } - } else { - ui.add_enabled_ui(false, |ui| { - ui.small_icon_button(&re_ui::icons::ARROW_RIGHT) - .on_disabled_hover_text("No future selections found"); - }); - } - - None - } - - #[allow(clippy::unused_self)] - fn history_item_ui( - &self, - viewport: &ViewportBlueprint, - ui: &mut egui::Ui, - index: usize, - history: &mut SelectionHistory, - ) { - if let Some(sel) = history.stack.get(index) { - ui.horizontal(|ui| { - { - // borrow checker workaround - let sel = selection_to_string(viewport, sel); - if ui - .selectable_value(&mut history.current, index, sel) - .clicked() - { - ui.close_menu(); - } - } - if sel.iter_items().count() == 1 { - if let Some(item) = sel.iter_items().next() { - item_kind_ui(ui, item); - } - } - }); - } - } -} - -// Different kinds of selections can share the same path in practice! We need to -// differentiate those in the UI to avoid confusion. -fn item_kind_ui(ui: &mut egui::Ui, sel: &Item) { - ui.weak(RichText::new(format!("({})", sel.kind()))); -} - -fn selection_to_string(viewport: &ViewportBlueprint, selection: &ItemCollection) -> String { - debug_assert!( - !selection.is_empty(), - "History should never contain empty selections." - ); - if selection.len() == 1 { - if let Some(item) = selection.iter_items().next() { - item_to_string(viewport, item) - } else { - // All items got removed or weren't there to begin with. - debug_assert!( - selection.iter_space_context().next().is_some(), - "History should never keep selections that have both an empty item & context list." - ); - "".to_owned() - } - } else if let Some(kind) = selection.are_all_items_same_kind() { - format!("{}x {}s", selection.len(), kind) - } else { - "".to_owned() - } -} - -fn item_to_string(viewport: &ViewportBlueprint, item: &Item) -> String { - match item { - Item::AppId(app_id) => app_id.to_string(), - Item::DataSource(data_source) => data_source.to_string(), - Item::StoreId(store_id) => store_id.to_string(), - Item::SpaceView(space_view_id) => { - // TODO(#4678): unnamed space views should have their label formatted accordingly (subdued) - if let Some(space_view) = viewport.view(space_view_id) { - space_view.display_name_or_default().as_ref().to_owned() - } else { - "".to_owned() - } - } - Item::InstancePath(instance_path) => instance_path.to_string(), - Item::DataResult(space_view_id, instance_path) => { - // TODO(#4678): unnamed space views should have their label formatted accordingly (subdued) - let space_view_display_name = if let Some(space_view) = viewport.view(space_view_id) { - space_view.display_name_or_default().as_ref().to_owned() - } else { - "".to_owned() - }; - - format!("{instance_path} in {space_view_display_name}") - } - Item::ComponentPath(path) => { - format!("{}:{}", path.entity_path, path.component_name.short_name(),) - } - Item::Container(container_id) => { - // TODO(#4678): unnamed container should have their label formatted accordingly (subdued) - if let Some(container) = viewport.container(container_id) { - container.display_name_or_default().as_ref().to_owned() - } else { - "".to_owned() - } - } - } -} diff --git a/crates/viewer/re_selection_panel/src/selection_panel.rs b/crates/viewer/re_selection_panel/src/selection_panel.rs index f5997b3db199..0d6145e76564 100644 --- a/crates/viewer/re_selection_panel/src/selection_panel.rs +++ b/crates/viewer/re_selection_panel/src/selection_panel.rs @@ -19,12 +19,11 @@ use re_viewer_context::{ use re_viewport_blueprint::{ui::show_add_space_view_or_container_modal, ViewportBlueprint}; use crate::{defaults_ui::view_components_defaults_section_ui, visualizer_ui::visualizer_ui}; +use crate::{space_view_entity_picker::SpaceViewEntityPicker, ItemTitle}; use crate::{ - selection_history_ui::SelectionHistoryUi, visible_time_range_ui::visible_time_range_ui_for_data_result, visible_time_range_ui::visible_time_range_ui_for_view, }; -use crate::{space_view_entity_picker::SpaceViewEntityPicker, ItemTitle}; // --- fn default_selection_panel_width(screen_width: f32) -> f32 { @@ -35,8 +34,6 @@ fn default_selection_panel_width(screen_width: f32) -> f32 { #[derive(Default, serde::Deserialize, serde::Serialize)] #[serde(default)] pub struct SelectionPanel { - selection_state_ui: SelectionHistoryUi, - #[serde(skip)] /// State for the "Add entity" modal. space_view_entity_modal: SpaceViewEntityPicker, @@ -70,15 +67,7 @@ impl SelectionPanel { ui.panel_content(|ui| { let hover = "The selection view contains information and options about \ the currently selected object(s)"; - ui.panel_title_bar_with_buttons("Selection", Some(hover), |ui| { - let mut history = ctx.selection_state().history.lock(); - if let Some(selection) = - self.selection_state_ui - .selection_ui(ui, viewport, &mut history) - { - ctx.selection_state().set_selection(selection); - } - }); + ui.panel_title_bar("Selection", Some(hover)); }); // move the vertical spacing between the title and the content to _inside_ the scroll diff --git a/crates/viewer/re_ui/src/command.rs b/crates/viewer/re_ui/src/command.rs index 21109f45cc7f..f42fc755dde1 100644 --- a/crates/viewer/re_ui/src/command.rs +++ b/crates/viewer/re_ui/src/command.rs @@ -59,9 +59,6 @@ pub enum UICommand { #[cfg(not(target_arch = "wasm32"))] ZoomReset, - SelectionPrevious, - SelectionNext, - ToggleCommandPalette, // Playback: @@ -199,8 +196,6 @@ impl UICommand { "Resets the UI zoom level to the operating system's default value", ), - Self::SelectionPrevious => ("Previous selection", "Go to previous selection"), - Self::SelectionNext => ("Next selection", "Go to next selection"), Self::ToggleCommandPalette => ("Command palette…", "Toggle the Command Palette"), Self::PlaybackTogglePlayPause => { @@ -346,8 +341,6 @@ impl UICommand { #[cfg(not(target_arch = "wasm32"))] Self::ZoomReset => Some(egui::gui_zoom::kb_shortcuts::ZOOM_RESET), - Self::SelectionPrevious => Some(ctrl_shift(Key::ArrowLeft)), - Self::SelectionNext => Some(ctrl_shift(Key::ArrowRight)), Self::ToggleCommandPalette => Some(cmd(Key::P)), Self::PlaybackTogglePlayPause => Some(key(Key::Space)), diff --git a/crates/viewer/re_viewer/src/app.rs b/crates/viewer/re_viewer/src/app.rs index 21e939406064..39b3e4c02b2e 100644 --- a/crates/viewer/re_viewer/src/app.rs +++ b/crates/viewer/re_viewer/src/app.rs @@ -825,12 +825,6 @@ impl App { egui_ctx.set_zoom_factor(1.0); } - UICommand::SelectionPrevious => { - self.state.selection_state.select_previous(); - } - UICommand::SelectionNext => { - self.state.selection_state.select_next(); - } UICommand::ToggleCommandPalette => { self.cmd_palette.toggle(); } diff --git a/crates/viewer/re_viewer_context/src/lib.rs b/crates/viewer/re_viewer_context/src/lib.rs index 221a532e4fd4..c0b0359c1d3b 100644 --- a/crates/viewer/re_viewer_context/src/lib.rs +++ b/crates/viewer/re_viewer_context/src/lib.rs @@ -18,7 +18,6 @@ mod item; mod maybe_mut_ref; mod query_context; mod query_range; -mod selection_history; mod selection_state; mod space_view; mod store_context; @@ -59,7 +58,6 @@ pub use self::{ DataQueryResult, DataResultHandle, DataResultNode, DataResultTree, QueryContext, }, query_range::QueryRange, - selection_history::SelectionHistory, selection_state::{ ApplicationSelectionState, HoverHighlight, InteractionHighlight, ItemCollection, ItemSpaceContext, SelectionHighlight, diff --git a/crates/viewer/re_viewer_context/src/selection_history.rs b/crates/viewer/re_viewer_context/src/selection_history.rs deleted file mode 100644 index 99b26391f7fb..000000000000 --- a/crates/viewer/re_viewer_context/src/selection_history.rs +++ /dev/null @@ -1,120 +0,0 @@ -use crate::selection_state::ItemCollection; - -use super::Item; - -/// A `Selection` and its index into the historical stack. -#[derive(Debug, Clone)] -pub struct HistoricalSelection { - pub index: usize, - pub selection: ItemCollection, -} - -impl From<(usize, ItemCollection)> for HistoricalSelection { - fn from((index, selection): (usize, ItemCollection)) -> Self { - Self { index, selection } - } -} - -const MAX_SELECTION_HISTORY_LENGTH: usize = 100; - -// --- - -/// A stack of `Selection`s, used to implement "undo/redo"-like semantics for selections. -#[derive(Clone, Default, Debug)] -pub struct SelectionHistory { - /// Index into [`Self::stack`]. - pub current: usize, - - /// Oldest first. - pub stack: Vec, -} - -impl SelectionHistory { - /// Retains all elements that fulfill a certain condition. - pub fn retain(&mut self, f: &impl Fn(&Item) -> bool) { - re_tracing::profile_function!(); - - let mut i = 0; - self.stack.retain_mut(|selection| { - selection.retain(|item, _| f(item)); - let retain = !selection.is_empty(); - if !retain && i <= self.current { - self.current = self.current.saturating_sub(1); - } - i += 1; - retain - }); - - // In case `self.current` was bad going in to this function: - self.current = self.current.min(self.stack.len().saturating_sub(1)); - } - - pub fn current(&self) -> Option { - self.stack - .get(self.current) - .cloned() - .map(|s| (self.current, s).into()) - } - - pub fn previous(&self) -> Option { - let prev_index = self.current.checked_sub(1)?; - let prev = self.stack.get(prev_index)?; - Some((prev_index, prev.clone()).into()) - } - - pub fn next(&self) -> Option { - self.stack - .get(self.current + 1) - .map(|sel| (self.current + 1, sel.clone()).into()) - } - - #[must_use] - pub fn select_previous(&mut self) -> Option { - if let Some(previous) = self.previous() { - if previous.index != self.current { - self.current = previous.index; - return self.current().map(|s| s.selection); - } - } - None - } - - #[must_use] - pub fn select_next(&mut self) -> Option { - if let Some(next) = self.next() { - if next.index != self.current { - self.current = next.index; - return self.current().map(|s| s.selection); - } - } - None - } - - pub fn update_selection(&mut self, selection: &ItemCollection) { - // Selecting nothing is irrelevant from a history standpoint. - if selection.is_empty() { - return; - } - - // Do not grow the history if the thing being selected is equal to the value that the - // current history cursor points to. - if self.current().as_ref().map(|c| &c.selection) == Some(selection) { - return; - } - - // Make sure to clear the entire redo history past this point: we are engaging in a - // diverging timeline! - self.stack.truncate(self.current + 1); - - self.stack.push(selection.clone()); - - // Keep size under a certain maximum. - if self.stack.len() > MAX_SELECTION_HISTORY_LENGTH { - self.stack - .drain((self.stack.len() - MAX_SELECTION_HISTORY_LENGTH)..self.stack.len()); - } - - // Update current index last so it points to something valid! - self.current = self.stack.len() - 1; - } -} diff --git a/crates/viewer/re_viewer_context/src/selection_state.rs b/crates/viewer/re_viewer_context/src/selection_state.rs index 51ff5d20e027..202427cac510 100644 --- a/crates/viewer/re_viewer_context/src/selection_state.rs +++ b/crates/viewer/re_viewer_context/src/selection_state.rs @@ -6,7 +6,7 @@ use re_entity_db::EntityPath; use crate::{item::resolve_mono_instance_path_item, ViewerContext}; -use super::{Item, SelectionHistory}; +use super::Item; /// Context information that a space view might attach to an item from [`ItemCollection`] and useful /// for how a selection might be displayed and interacted with. @@ -205,10 +205,6 @@ impl ItemCollection { #[derive(Default, serde::Deserialize, serde::Serialize)] #[serde(default)] pub struct ApplicationSelectionState { - /// History of selections (what was selected previously). - #[serde(skip)] - pub history: Mutex, - /// Selection of the previous frame. Read from this. selection_previous_frame: ItemCollection, @@ -235,10 +231,6 @@ impl ApplicationSelectionState { // Use a different name so we don't get a collision in puffin. re_tracing::profile_scope!("SelectionState::on_frame_start"); - // Purge history of invalid items. - let history = self.history.get_mut(); - history.retain(&item_retain_condition); - // Purge selection of invalid items. let selection_this_frame = self.selection_this_frame.get_mut(); selection_this_frame.retain(|item, _| item_retain_condition(item)); @@ -253,25 +245,10 @@ impl ApplicationSelectionState { // Selection in contrast, is sticky! if selection_this_frame != &self.selection_previous_frame { - history.update_selection(selection_this_frame); self.selection_previous_frame = selection_this_frame.clone(); } } - /// Selects the previous element in the history if any. - pub fn select_previous(&self) { - if let Some(selection) = self.history.lock().select_previous() { - *self.selection_this_frame.lock() = selection; - } - } - - /// Selections the next element in the history if any. - pub fn select_next(&self) { - if let Some(selection) = self.history.lock().select_next() { - *self.selection_this_frame.lock() = selection; - } - } - /// Clears the current selection out. pub fn clear_selection(&self) { self.set_selection(ItemCollection::default());