Merge pull request #4062 from dotCooCoo/fix/shape-builder-merge-reuses-hover-map

Shape Builder: a click merges over the map the tool built for the hover

crates/engine/src/cmd/buildcmds.rs

@@ -287,9 +287,21 @@ fn sb_merge(s: &mut Session, p: &Value) -> Result<Value> {
         f => f,
     };
     let roots = root_ids(s, p)?;
+    let gap = gap_param(p);
+    // The map the Shape Builder tool already has, which a click or drag of it is made over.
+    let shared = s.tool.shape_builder_map().filter(|(_, g)| *g == gap);
     let label = if erase { "Shape Builder Delete" } else { "Shape Builder" };
     let (ids, merged, touched, split) = s.edit(label, |d, sel| {
-        let (map, leaves) = BuilderMap::with_gap(d, &roots, erase || split_tol.is_some(), open_filled_as_closed, gap_param(p));
+        let (shapes, leaves) = b::shape_builder_shapes(d, &roots, open_filled_as_closed);
+        let built;
+        let map = match &shared {
+            // The tool's map has the outline edges, which erasing and splitting need.
+            Some((m, _)) if m.shapes == shapes => m.as_ref(),
+            _ => {
+                built = BuilderMap::of_shapes(shapes, erase || split_tol.is_some(), gap);
+                &built
+            }
+        };
         let outlines = b::drawn_outlines(d, &leaves);
         let leaves: Vec<Node> = leaves.into_iter().cloned().collect();
         if map.shapes.is_empty() {

crates/engine/src/tests_build.rs

@@ -365,6 +365,32 @@ fn shape_builder_tool_drives_the_command() {
     assert_eq!(layer_children(&s), 1);
 }
 
+/// A click of the Shape Builder merges over the map the tool built for the hover, as does the
+/// command over the same inputs with the same gap; another gap needs a map of its own.
+#[test]
+fn shape_builder_merge_uses_the_map_the_tool_has() {
+    use vectorcraft_tools::builder::maps_built;
+    let mut s = session();
+    two(&mut s);
+    let v = ViewInfo::default();
+    s.select_tool("shapeBuilder", v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Move, 75.0, 50.0), v).unwrap();
+    let built = maps_built();
+    s.pointer(&PointerEvent::new(PointerKind::Down, 75.0, 50.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Up, 75.0, 50.0), v).unwrap();
+    assert_eq!(s.journal.last().unwrap().0, "shapeBuilder.merge");
+    assert_eq!(layer_children(&s), 3, "the overlap and what is left of each square");
+    assert_eq!(maps_built(), built, "the click builds no map");
+    // Undone, the squares are the inputs the tool's map was built from.
+    s.execute("edit.undo", &json!({})).unwrap();
+    let r = s.execute("shapeBuilder.merge", &json!({"points": [[75, 50]]})).unwrap();
+    assert!((area(&s, NodeId(r["merged"].as_u64().unwrap())) - 5000.0).abs() < 1e-6);
+    assert_eq!(maps_built(), built, "the command over the same inputs builds no map");
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("shapeBuilder.merge", &json!({"points": [[75, 50]], "gap": 3})).unwrap();
+    assert_eq!(maps_built(), built + 1, "another gap");
+}
+
 #[test]
 fn shape_builder_tool_merges_the_narrow_shape_it_highlights_in_a_wide_selection() {
     let mut s = session();

crates/tools/src/builder.rs

@@ -243,7 +243,7 @@ fn shapes_of<'a>(doc: &'a Document, roots: &[NodeId], filled_faces: bool) -> (Ve
 /// they cross, and so do filled open paths without `open_filled_as_closed`. A Live Paint group
 /// contributes its filled faces as shapes and its stroked edges as open lines; its unfilled faces
 /// and its edges without a stroke are left out.
-fn shape_builder_shapes<'a>(doc: &'a Document, roots: &[NodeId], open_filled_as_closed: bool) -> (Vec<po::Shape>, Vec<&'a Node>) {
+pub fn shape_builder_shapes<'a>(doc: &'a Document, roots: &[NodeId], open_filled_as_closed: bool) -> (Vec<po::Shape>, Vec<&'a Node>) {
     let (mut shapes, leaves) = shapes_of(doc, roots, true);
     for (s, l) in shapes.iter_mut().zip(&leaves) {
         if named(l, EDGE_NAME) && doc.parent_of(l.id).and_then(|p| doc.node(p)).is_some_and(is_live_paint) {
@@ -276,6 +276,17 @@ fn faces_of(regions: Vec<po::Region>) -> Vec<Face> {
         .collect()
 }
 
+thread_local! {
+    /// The Shape Builder maps built on this thread ([`maps_built`]).
+    static MAPS_BUILT: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+}
+
+/// How many Shape Builder maps ([`BuilderMap::of_shapes`]) this thread has built, for tests that
+/// count them.
+pub fn maps_built() -> usize {
+    MAPS_BUILT.with(std::cell::Cell::get)
+}
+
 /// The Shape Builder's planar map of a selection.
 #[derive(Default)]
 pub struct BuilderMap {
@@ -328,12 +339,13 @@ impl BuilderMap {
         (Self::of_shapes(shapes, with_edges, gap), leaves)
     }
 
-    /// The map of `shapes` (the inputs, back → front), with the outline edges only `with_edges`,
-    /// closing gaps up to `gap`.
-    fn of_shapes(shapes: Vec<po::Shape>, with_edges: bool, gap: f64) -> Self {
+    /// The map of `shapes` (the inputs, back → front, as [`shape_builder_shapes`] gives them), with
+    /// the outline edges only `with_edges`, closing gaps up to `gap`.
+    pub fn of_shapes(shapes: Vec<po::Shape>, with_edges: bool, gap: f64) -> Self {
         if shapes.is_empty() {
             return Self::default();
         }
+        MAPS_BUILT.with(|n| n.set(n.get() + 1));
         let arr = po::shape_builder_with_gap(&shapes, with_edges, gap);
         let bounded = |pieces: Vec<po::Shape>| {
             pieces
@@ -671,7 +683,8 @@ struct Pending {
 #[derive(Default)]
 struct RegionCache {
     key: Option<CacheKey>,
-    map: BuilderMap,
+    /// The map, which the Shape Builder's merge command takes too ([`Tool::shape_builder_map`]).
+    map: Arc<BuilderMap>,
     /// Live Paint faces (every path an edge, filled or not) rather than Shape Builder regions.
     live: bool,
     /// Gap Detection distance; changing tool options invalidates the cache.
@@ -741,7 +754,7 @@ impl RegionCache {
     }
 
     fn install(&mut self, key: CacheKey, map: BuilderMap, outlines: Vec<PathData>) {
-        self.map = map;
+        self.map = Arc::new(map);
         self.outlines = outlines;
         self.key = Some(key);
         #[cfg(test)]
@@ -1257,6 +1270,10 @@ impl Tool for ShapeBuilderTool {
         }
     }
 
+    fn shape_builder_map(&self) -> Option<(Arc<BuilderMap>, f64)> {
+        self.cache.key.is_some().then(|| (Arc::clone(&self.cache.map), self.cache.gap))
+    }
+
     /// With Pick Color From Color Swatches, the Left and Right arrow keys pick the color swatch
     /// before or after the current fill.
     fn claims_key(&self, _cx: &ToolContext, key: ToolKey) -> bool {

crates/tools/src/lib.rs

@@ -928,6 +928,12 @@ pub trait Tool: Send {
     /// Checks on the work [`Tool::waiting`] reports, waiting for it to finish with `wait`. Once it
     /// is done, the tool shows the highlight under the pointer where it last moved.
     fn poll(&mut self, _cx: &ToolContext, _wait: bool) {}
+    /// The Shape Builder's planar map of the selection, with the outline edges, and the gap it
+    /// closes, when the tool has one. `shapeBuilder.merge` uses it in place of a map of its own
+    /// when the map's inputs are the merge's inputs.
+    fn shape_builder_map(&self) -> Option<(std::sync::Arc<builder::BuilderMap>, f64)> {
+        None
+    }
 }
 
 /// Create a tool by id. Unknown or not-yet-implemented tools fall back to a no-op tool that keeps the id.