From c527082ff18806a1b29f68f2a564e44c0d921b80 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 11:17:48 -0700 Subject: [PATCH] Shape Builder: a click merges over the map the tool built for the hover shapeBuilder.merge built the planar map of the selection again, on the thread that runs the command, although the Shape Builder tool had just built the same map for its hover. The tool now returns its map from a new Tool::shape_builder_map, and the command uses that map when its inputs are the merge's inputs and its gap is the merge's gap; otherwise it builds its own as before. The tool's map has the outline edges, so erasing and splitting a stroke use it too. --- crates/engine/src/cmd/buildcmds.rs | 14 +++++++++++++- crates/engine/src/tests_build.rs | 26 ++++++++++++++++++++++++++ crates/tools/src/builder.rs | 29 +++++++++++++++++++++++------ crates/tools/src/lib.rs | 6 ++++++ 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index 8c8c07225..209339532 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/crates/engine/src/cmd/buildcmds.rs @@ -287,9 +287,21 @@ fn sb_merge(s: &mut Session, p: &Value) -> Result { 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 = leaves.into_iter().cloned().collect(); if map.shapes.is_empty() { diff --git a/crates/engine/src/tests_build.rs b/crates/engine/src/tests_build.rs index c7763d422..9c74ee83f 100644 --- a/crates/engine/src/tests_build.rs +++ b/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(); diff --git a/crates/tools/src/builder.rs b/crates/tools/src/builder.rs index 143ade594..c2bbf5d4c 100644 --- a/crates/tools/src/builder.rs +++ b/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, Vec<&'a Node>) { +pub fn shape_builder_shapes<'a>(doc: &'a Document, roots: &[NodeId], open_filled_as_closed: bool) -> (Vec, 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) -> Vec { .collect() } +thread_local! { + /// The Shape Builder maps built on this thread ([`maps_built`]). + static MAPS_BUILT: std::cell::Cell = 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, 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, 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| { pieces @@ -671,7 +683,8 @@ struct Pending { #[derive(Default)] struct RegionCache { key: Option, - map: BuilderMap, + /// The map, which the Shape Builder's merge command takes too ([`Tool::shape_builder_map`]). + map: Arc, /// 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) { - 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, 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 { diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index a87eb2cb7..026ef055d 100644 --- a/crates/tools/src/lib.rs +++ b/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, f64)> { + None + } } /// Create a tool by id. Unknown or not-yet-implemented tools fall back to a no-op tool that keeps the id.