Merge pull request #5832 from storytold/perf/1280-hover-memo-check

Selection: a hover checks the bounds memo once, not once per object

crates/engine/src/tests/previewbounds.rs

@@ -878,6 +878,31 @@ fn a_handle_drag_keeps_the_opposite_edge_of_the_offset_shape() {
     }
 }
 
+#[test]
+fn a_hover_checks_the_memo_as_often_however_many_objects_are_selected() {
+    let checks = |objects: u32| {
+        let mut s = session();
+        for i in 0..objects {
+            rect(&mut s, f64::from(i), 1.0, 0.0);
+        }
+        run(&mut s, "select.all", json!({}));
+        let v = ViewInfo::default();
+        s.select_tool("selection", v).unwrap();
+        // Off the art, so the hover walks every selected object and finds no anchor or center.
+        let hover = |s: &mut Session| {
+            s.pointer(&PointerEvent::new(PointerKind::Move, 350.0, 350.0), v).unwrap();
+            s.cursor(Point::new(350.0, 350.0), Mods::default(), v)
+        };
+        hover(&mut s);
+        let before = s.preview_memo.checks();
+        hover(&mut s);
+        s.preview_memo.checks() - before
+    };
+    let few = checks(3);
+    assert!(few > 0, "a hover asks the memo");
+    assert_eq!(checks(60), few, "a check compares the whole selection: one per object is quadratic");
+}
+
 #[test]
 fn hovering_over_an_unchanged_document_measures_nothing_again() {
     // A selected square with Offset Path 10, drawn from 90 to 210, with Use Preview Bounds and Smart

crates/tools/src/select.rs

@@ -379,6 +379,8 @@ struct MemoState {
     entry: Option<Measured>,
     /// How many measurements the memo has made.
     measurements: u64,
+    /// How many times the memo has checked its key.
+    checks: u64,
 }
 
 /// The measurements for one key; a slot holds `None` until it is measured.
@@ -404,8 +406,15 @@ impl PreviewMemo {
         self.0.try_borrow().map_or(0, |st| st.measurements)
     }
 
+    /// How many times the memo has checked its key against the document. A check compares the whole
+    /// selection, so a hover has to make the same few however many objects are selected.
+    pub fn checks(&self) -> u64 {
+        self.0.try_borrow().map_or(0, |st| st.checks)
+    }
+
     /// The measurements for the state `cx` shows, after the ones for any other state are dropped.
     fn current<'m>(st: &'m mut MemoState, cx: &ToolContext) -> &'m mut Measured {
+        st.checks += 1;
         let fresh = || Measured {
             revision: cx.revision,
             objects: cx.selection.objects.clone(),
@@ -466,7 +475,27 @@ fn shown_box(cx: &ToolContext) -> Option<OrientedBox> {
 
 /// The bounds of object `id`, measured as [`selection_bounds`] measures the selection.
 fn object_bounds(cx: &ToolContext, id: NodeId) -> Option<Rect> {
-    remembered(cx, |m| m.by_object.entry(id).or_insert(None), || vectorcraft_effects::preview::bounds_of(cx.doc, &[id], cx.preview_bounds))
+    with_object_bounds(cx, |bounds| bounds(id))
+}
+
+/// Runs `walk` with a lookup of [`object_bounds`]. The lookups share one check of the memo's key,
+/// which compares the whole selection: a walk of every selected object checking it each time takes
+/// time in the square of their number.
+fn with_object_bounds<R>(cx: &ToolContext, walk: impl FnOnce(&mut dyn FnMut(NodeId) -> Option<Rect>) -> R) -> R {
+    let mut measure = |id: NodeId| vectorcraft_effects::preview::bounds_of(cx.doc, &[id], cx.preview_bounds);
+    let Some(mut st) = cx.preview_memo.and_then(|memo| memo.0.try_borrow_mut().ok()) else {
+        return walk(&mut measure);
+    };
+    let mut measured = 0;
+    let by_object = &mut PreviewMemo::current(&mut st, cx).by_object;
+    let out = walk(&mut |id| {
+        *by_object.entry(id).or_insert(None).get_or_insert_with(|| {
+            measured += 1;
+            measure(id)
+        })
+    });
+    st.measurements += measured;
+    out
 }
 
 /// The objects of `ids` that lie entirely inside marquee `r`, edges included, measured by their
@@ -560,15 +589,17 @@ impl SelectionTool {
         };
         // A press on an unselected object selects it alone: the selected ones stay behind.
         let selected = if under.is_none_or(|u| cx.selection.contains(u)) { &cx.selection.objects[..] } else { &[] };
-        under.into_iter().chain(selected.iter().copied()).find_map(|id| {
-            let n = cx.doc.node(id)?;
-            let anchor = n.path_data().and_then(|pd| nearest(&mut pd.subpaths.iter().flat_map(|sp| sp.anchors.iter().map(|a| a.p))));
-            anchor.map(|q| (q, "anchor")).or_else(|| {
-                if !cx.smart_guides {
-                    return None;
-                }
-                let c = object_bounds(cx, id)?.center();
-                (c.distance(p) <= reach).then_some((c, "center"))
+        with_object_bounds(cx, |bounds| {
+            under.into_iter().chain(selected.iter().copied()).find_map(|id| {
+                let n = cx.doc.node(id)?;
+                let anchor = n.path_data().and_then(|pd| nearest(&mut pd.subpaths.iter().flat_map(|sp| sp.anchors.iter().map(|a| a.p))));
+                anchor.map(|q| (q, "anchor")).or_else(|| {
+                    if !cx.smart_guides {
+                        return None;
+                    }
+                    let c = bounds(id)?.center();
+                    (c.distance(p) <= reach).then_some((c, "center"))
+                })
             })
         })
     }