Selection state is now fully double buffered and has interior mutability

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());
         }
     }
 }

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)
 }

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();
                 }
             }
         }

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,87 +79,95 @@ 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<SelectionHistory>,
+
+    /// 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<ItemCollection>,
 
     /// What objects are hovered? Read from this.
     #[serde(skip)]
     hovered_previous_frame: ItemCollection,
 
     /// What objects are hovered? Write to this.
     #[serde(skip)]
-    hovered_this_frame: ItemCollection,
+    hovered_this_frame: Mutex<ItemCollection>,
 
     /// What space is the pointer hovering over? Read from this.
     #[serde(skip)]
     hovered_space_previous_frame: HoveredSpace,
 
     /// What space is the pointer hovering over? Write to this.
     #[serde(skip)]
-    hovered_space_this_frame: HoveredSpace,
+    hovered_space_this_frame: Mutex<HoveredSpace>,
 }
 
 impl SelectionState {
     /// Called at the start of each frame
     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<Item = Item>) -> ItemCollection {
+    pub fn set_selection(&self, items: impl Iterator<Item = Item>) {
         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,20 +176,20 @@ impl SelectionState {
     }
 
     /// Set the hovered objects. Will be in [`Self::hovered`] on the next frame.
-    pub fn set_hovered(&mut self, items: impl Iterator<Item = Item>) {
-        self.hovered_this_frame = ItemCollection::new(items);
+    pub fn set_hovered(&self, items: impl Iterator<Item = Item>) {
+        *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<Item>) {
+    pub fn toggle_selection(&self, toggle_items: Vec<Item>) {
         re_tracing::profile_function!();
 
         // Make sure we preserve the order - old items kept in same order, new items added to the end.
 
         // All the items to toggle. If an was already selected, it will be removed from this.
         let mut toggle_items_set: HashSet<Item> = 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 {

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<Item = &'b Item>) {
+    pub fn set_hovered<'b>(&self, hovered: impl Iterator<Item = &'b Item>) {
         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()