Selection state is now fully double buffered and has interior mutabil…

…ity (#4387)

### What

* A step towards #1325

Hover state was already double buffered but selection was applied
immediately. Not it behaves the same way. This is not only faster but
also more consistent and gives us "safer" behavior

(branch name is a misnomer - had bigger plans and then split it)

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [app.rerun.io](https://app.rerun.io/pr/4387) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4387)
- [Docs
preview](https://rerun.io/preview/27d7c248aef3221c98e1a477433d1f53e72887f7/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/27d7c248aef3221c98e1a477433d1f53e72887f7/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

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

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

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

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

@@ -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<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| {
@@ -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()