diff --git a/crates/re_data_ui/src/item_ui.rs b/crates/re_data_ui/src/item_ui.rs index ab3767286cb5..f95062d9c590 100644 --- a/crates/re_data_ui/src/item_ui.rs +++ b/crates/re_data_ui/src/item_ui.rs @@ -309,15 +309,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 1e6c1b03a68e..6b14e880220b 100644 --- a/crates/re_viewer/src/ui/selection_panel.rs +++ b/crates/re_viewer/src/ui/selection_panel.rs @@ -65,13 +65,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()); } }); @@ -588,7 +589,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 2832bceba134..708147174bb4 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 24543471654c..710bb888a5ae 100644 --- a/crates/re_viewer_context/src/viewer_context.rs +++ b/crates/re_viewer_context/src/viewer_context.rs @@ -53,17 +53,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. @@ -77,7 +75,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| { @@ -93,10 +91,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()