From c4a05513bb138523b1efda365afd0b2b349a2f27 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Wed, 29 Nov 2023 12:09:46 +0100 Subject: [PATCH] selection state is now fully double buffered and has interior mutability --- crates/re_data_ui/src/item_ui.rs | 7 +- crates/re_space_view_spatial/src/ui.rs | 2 +- crates/re_viewer/src/ui/selection_panel.rs | 7 +- .../re_viewer_context/src/selection_state.rs | 83 ++++++++++--------- .../re_viewer_context/src/viewer_context.rs | 14 +--- 5 files changed, 58 insertions(+), 55 deletions(-) diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index 347e00c71d08..dccff820a242 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -308,15 +308,14 @@ pub fn select_hovered_on_click( re_tracing::profile_function!(); if response.hovered() { - ctx.selection_state_mut().set_hovered(items.iter().cloned()); + ctx.set_hovered(items.iter()); } if response.clicked() { if response.ctx.input(|i| i.modifiers.command) { - ctx.selection_state_mut().toggle_selection(items.to_vec()); + ctx.selection_state().toggle_selection(items.to_vec()); } else { - ctx.selection_state_mut() - .set_selection(items.iter().cloned()); + ctx.selection_state().set_selection(items.iter().cloned()); } } } diff --git a/crates/re_space_view_spatial/src/ui.rs b/crates/re_space_view_spatial/src/ui.rs index 8abf34b03c12..bacd1509d0bf 100644 --- a/crates/re_space_view_spatial/src/ui.rs +++ b/crates/re_space_view_spatial/src/ui.rs @@ -657,7 +657,7 @@ pub fn picking( } } }; - ctx.selection_state_mut().set_hovered_space(hovered_space); + ctx.selection_state().set_hovered_space(hovered_space); Ok(response) } diff --git a/crates/re_viewer/src/ui/selection_panel.rs b/crates/re_viewer/src/ui/selection_panel.rs index 004423568ea2..d6e94b6adb5e 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -64,13 +64,14 @@ impl SelectionPanel { currently selected object(s)"; ctx.re_ui .panel_title_bar_with_buttons(ui, "Selection", Some(hover), |ui| { + let mut history = ctx.selection_state().history.lock(); if let Some(selection) = self.selection_state_ui.selection_ui( ctx.re_ui, ui, &viewport.blueprint, - &mut ctx.selection_state_mut().history, + &mut history, ) { - ctx.selection_state_mut() + ctx.selection_state() .set_selection(selection.iter().cloned()); } }); @@ -547,7 +548,7 @@ fn blueprint_ui( ); data_result.save_override(Some(props), ctx); } else { - ctx.selection_state_mut().clear_current(); + ctx.selection_state().clear_current(); } } } diff --git a/crates/re_viewer_context/src/selection_state.rs b/crates/re_viewer_context/src/selection_state.rs index 7c7a1cc7e109..318f86c609f1 100644 --- a/crates/re_viewer_context/src/selection_state.rs +++ b/crates/re_viewer_context/src/selection_state.rs @@ -1,4 +1,5 @@ use ahash::HashSet; +use parking_lot::Mutex; use re_data_store::EntityPath; @@ -78,19 +79,23 @@ impl InteractionHighlight { } } -/// Selection and hover state +/// Selection and hover state. +/// +/// Both hover and selection are double buffered: +/// Changes from one frame are only visible in the next frame. #[derive(Default, serde::Deserialize, serde::Serialize)] #[serde(default)] pub struct SelectionState { - /// Currently selected things. - /// - /// Do not access this field directly! Use the helper methods instead, which will make sure - /// to properly maintain the undo/redo history. - selection: ItemCollection, - /// History of selections (what was selected previously). #[serde(skip)] - pub history: SelectionHistory, + pub history: Mutex, + + /// Selection of the previous frame. Read from this. + selection_previous_frame: ItemCollection, + + /// Selection of the current frame. Write to this. + #[serde(skip)] + selection_this_frame: Mutex, /// What objects are hovered? Read from this. #[serde(skip)] @@ -98,7 +103,7 @@ pub struct SelectionState { /// What objects are hovered? Write to this. #[serde(skip)] - hovered_this_frame: ItemCollection, + hovered_this_frame: Mutex, /// What space is the pointer hovering over? Read from this. #[serde(skip)] @@ -106,7 +111,7 @@ pub struct SelectionState { /// What space is the pointer hovering over? Write to this. #[serde(skip)] - hovered_space_this_frame: HoveredSpace, + hovered_space_this_frame: Mutex, } impl SelectionState { @@ -114,51 +119,55 @@ impl SelectionState { pub fn on_frame_start(&mut self, item_retain_condition: impl Fn(&Item) -> bool) { re_tracing::profile_function!(); - self.history.retain(&item_retain_condition); + let history = self.history.get_mut(); + history.retain(&item_retain_condition); + // Hovering needs to be refreshed every frame: If it wasn't hovered last frame, it's no longer hovered! + self.hovered_previous_frame = std::mem::take(self.hovered_this_frame.get_mut()); self.hovered_space_previous_frame = - std::mem::replace(&mut self.hovered_space_this_frame, HoveredSpace::None); - self.hovered_previous_frame = std::mem::take(&mut self.hovered_this_frame); + std::mem::replace(self.hovered_space_this_frame.get_mut(), HoveredSpace::None); + + // Selection in contrast, is sticky! + let selection_this_frame = self.selection_this_frame.get_mut(); + 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(&mut self) { - if let Some(selection) = self.history.select_previous() { - self.selection = selection; + 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(&mut self) { - if let Some(selection) = self.history.select_next() { - self.selection = selection; + 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_current(&mut self) { - self.selection = ItemCollection::default(); + pub fn clear_current(&self) { + *self.selection_this_frame.lock() = ItemCollection::default(); } /// Sets a single selection, updating history as needed. - /// - /// Returns the previous selection. - pub fn set_single_selection(&mut self, item: Item) -> ItemCollection { - self.set_selection(std::iter::once(item)) + pub fn set_single_selection(&self, item: Item) { + self.set_selection(std::iter::once(item)); } /// Sets several objects to be selected, updating history as needed. - /// - /// Returns the previous selection. - pub fn set_selection(&mut self, items: impl Iterator) -> ItemCollection { + pub fn set_selection(&self, items: impl Iterator) { let new_selection = ItemCollection::new(items); - self.history.update_selection(&new_selection); - std::mem::replace(&mut self.selection, new_selection) + *self.selection_this_frame.lock() = new_selection; } /// Returns the current selection. pub fn current(&self) -> &ItemCollection { - &self.selection + &self.selection_previous_frame } /// Returns the currently hovered objects. @@ -167,12 +176,12 @@ impl SelectionState { } /// Set the hovered objects. Will be in [`Self::hovered`] on the next frame. - pub fn set_hovered(&mut self, items: impl Iterator) { - self.hovered_this_frame = ItemCollection::new(items); + pub fn set_hovered(&self, items: impl Iterator) { + *self.hovered_this_frame.lock() = ItemCollection::new(items); } /// Select currently hovered objects unless already selected in which case they get unselected. - pub fn toggle_selection(&mut self, toggle_items: Vec) { + pub fn toggle_selection(&self, toggle_items: Vec) { re_tracing::profile_function!(); // Make sure we preserve the order - old items kept in same order, new items added to the end. @@ -180,7 +189,7 @@ impl SelectionState { // All the items to toggle. If an was already selected, it will be removed from this. let mut toggle_items_set: HashSet = toggle_items.iter().cloned().collect(); - let mut new_selection = self.selection.to_vec(); + let mut new_selection = self.selection_previous_frame.to_vec(); new_selection.retain(|item| !toggle_items_set.remove(item)); // Add the new items, unless they were toggling out existing items: @@ -197,8 +206,8 @@ impl SelectionState { &self.hovered_space_previous_frame } - pub fn set_hovered_space(&mut self, space: HoveredSpace) { - self.hovered_space_this_frame = space; + pub fn set_hovered_space(&self, space: HoveredSpace) { + *self.hovered_space_this_frame.lock() = space; } pub fn highlight_for_ui_element(&self, test: &Item) -> HoverHighlight { diff --git a/crates/re_viewer_context/src/viewer_context.rs b/crates/re_viewer_context/src/viewer_context.rs index 0a7a17a9a40e..71e8aa4a47b0 100644 --- a/crates/re_viewer_context/src/viewer_context.rs +++ b/crates/re_viewer_context/src/viewer_context.rs @@ -48,17 +48,15 @@ pub struct ViewerContext<'a> { } impl<'a> ViewerContext<'a> { - /// Sets a single selection, updating history as needed. - /// - /// Returns the previous selection. - pub fn set_single_selection(&mut self, item: &Item) -> ItemCollection { + /// Sets a single selection on the next frame, updating history as needed. + pub fn set_single_selection(&self, item: &Item) { self.rec_cfg .selection_state .set_single_selection(resolve_mono_instance_path_item( &self.rec_cfg.time_ctrl.current_query(), self.store_db.store(), item, - )) + )); } /// Returns the current selection. @@ -72,7 +70,7 @@ impl<'a> ViewerContext<'a> { } /// Set the hovered objects. Will be in [`Self::hovered`] on the next frame. - pub fn set_hovered<'b>(&mut self, hovered: impl Iterator) { + pub fn set_hovered<'b>(&self, hovered: impl Iterator) { self.rec_cfg .selection_state .set_hovered(hovered.map(|item| { @@ -88,10 +86,6 @@ impl<'a> ViewerContext<'a> { &self.rec_cfg.selection_state } - pub fn selection_state_mut(&mut self) -> &mut SelectionState { - &mut self.rec_cfg.selection_state - } - /// The current time query, based on the current time control. pub fn current_query(&self) -> re_arrow_store::LatestAtQuery { self.rec_cfg.time_ctrl.current_query()