Fix #569: Shape Builder merges leave no stray edges in their outlines

Merging regions united the faces' outlines with a boolean union. Each
face had been refit on its own, so two faces' shared edge could come out
a hair apart: the union then kept the edge between them, or a spur of
it, and a stroked merged shape (unfilled art) showed a stray line inside.
Subtracting the merged shape from the others left slivers along it too.

pathops::FaceMerger merges faces from the planar arrangement itself,
made once per merge: their contours share their edges exactly, so the
edges between merged faces vanish. The Shape Builder builds the merged
shape that way, and what is left of each shape from its faces not merged
(rather than subtracting the merged shape from it). Selections with open
paths keep the old union.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/buildcmds.rs

@@ -250,7 +250,8 @@ fn sb_merge(s: &mut Session, p: &Value) -> Result<Value> {
         // Edges come back from the planar map within a few hundredths of the outlines they follow.
         let cut_tol = step * 0.2;
         let picked: Vec<&po::Region> = touched.regions.iter().filter_map(|&i| map.regions.get(i).map(|r| &r.0)).collect();
-        let union = if picked.is_empty() { PathData::default() } else { po::merge_regions(&picked) };
+        let faces = po::FaceMerger::new(&map.shapes);
+        let union = if picked.is_empty() { PathData::default() } else { faces.merge(&picked) };
         // The merged shape takes the look of the front-most object over the first region, or (for an
         // area only lines enclose) of the front-most object and the current fill.
         let covered = picked.iter().find_map(|r| r.top());
@@ -278,7 +279,17 @@ fn sb_merge(s: &mut Session, p: &Value) -> Result<Value> {
                 // its outline, shared edges from every shape along them).
                 let in_union = picked.iter().any(|r| r.sources.contains(&i));
                 let mut rest = if in_union {
-                    po::boolean(&sh.path, sh.rule, &union, FillRule::NonZero, po::BoolOp::Difference)
+                    // What is left of it: its faces not merged or deleted, united. They share their
+                    // edges with the merged faces exactly, where subtracting the merged shape left
+                    // slivers of the outline along them (#569).
+                    let kept: Vec<&po::Region> = map
+                        .regions
+                        .iter()
+                        .enumerate()
+                        .filter(|(k, (r, ..))| r.sources.contains(&i) && !touched.regions.contains(k))
+                        .map(|(_, (r, ..))| r)
+                        .collect();
+                    if kept.is_empty() { PathData::default() } else { faces.merge(&kept) }
                 } else {
                     b::node_outline(leaf).map_or_else(|| sh.path.clone(), |o| o.0)
                 };

crates/engine/src/tests_build.rs

@@ -704,3 +704,67 @@ fn image_trace_object_remembers_its_settings() {
     let id = st.selection.in_paint_order(&st.doc)[0];
     assert!(st.doc.node(id).unwrap().trace.is_none());
 }
+
+/// #569: merging the regions of a rosette of unfilled ellipses (each off the centre, petals
+/// overlapping) merges every region the drags touch into one shape whose outline is only the
+/// merged area's border: no edge between merged regions stays in, as a stray line or a spur.
+#[test]
+fn shape_builder_merges_rosette_rings_cleanly() {
+    let mut s = session();
+    s.execute("paint.setFill", &json!({"none": true})).unwrap();
+    let c = [400.0, 320.0];
+    let mut petals = vec![];
+    for k in 0..8 {
+        let id =
+            s.execute("shape.ellipse", &json!({"x": c[0] - 60.0, "y": c[1] - 172.0, "width": 120, "height": 324})).unwrap()["id"].as_u64().unwrap();
+        s.execute("select.set", &json!({"ids": [id]})).unwrap();
+        s.execute("object.rotate", &json!({"angle": 45.0 * f64::from(k), "origin": c})).unwrap();
+        petals.push(id);
+    }
+    s.execute("select.set", &json!({"ids": petals})).unwrap();
+    // Drags around the ring of petals, from its inside out; the outer ones miss the petals.
+    let mut merged = None;
+    for radius in [110.0, 150.0, 190.0, 230.0] {
+        let pts: Vec<[f64; 2]> = (0..=120)
+            .map(|i| {
+                let a = std::f64::consts::TAU * f64::from(i) / 120.0;
+                [c[0] + radius * a.cos(), c[1] + radius * a.sin()]
+            })
+            .collect();
+        if let Ok(r) = s.execute("shapeBuilder.merge", &json!({"points": pts})) {
+            merged = r["merged"].as_u64().map(NodeId);
+        }
+    }
+    let (ring, rule) = outline(&node(&s, merged.expect("the drags merged")));
+    let inside = |p: kurbo::Point| kurbo::Shape::winding(&ring.to_bezpath(), p) != 0;
+    for k in 0..8 {
+        let a = std::f64::consts::FRAC_PI_4 * f64::from(k) - std::f64::consts::FRAC_PI_2;
+        let tip = kurbo::Point::new(c[0] + 165.0 * a.cos(), c[1] + 165.0 * a.sin());
+        assert!(inside(tip), "petal {k}'s tip at {tip:?} is in the merged shape");
+    }
+    // What is left of the petals stays out of the merged area (a piece left over it is a stray
+    // outline inside it).
+    let merged = merged.unwrap();
+    for id in s.doc().unwrap().selection.objects.clone().into_iter().filter(|id| *id != merged) {
+        let (p, r) = outline(&node(&s, id));
+        let over = vectorcraft_pathops::boolean(&p, r, &ring, rule, vectorcraft_pathops::BoolOp::Intersect);
+        let a = vectorcraft_pathops::area(&over, FillRule::NonZero);
+        assert!(a < 1.0, "{id:?} overlaps the merged shape by {a} pt²");
+    }
+    // No outline is a sliver or carries a spur (2 × area / length: its mean width).
+    for id in s.doc().unwrap().selection.objects.clone() {
+        let (p, r) = outline(&node(&s, id));
+        for sp in &p.subpaths {
+            let one = PathData::new(vec![sp.clone()]);
+            let width = 2.0 * vectorcraft_pathops::area(&one, r) / one.length();
+            assert!(width > 1.0, "{id:?} has an outline {width:.3} pt wide on average, {:.1} pt long", one.length());
+        }
+    }
+    // The ring is one outline around the centre (the regions the drags missed aside): the
+    // petals' tips are no outlines of their own beside it, apart by a hairline.
+    let big = ring.subpaths.iter().filter(|sp| vectorcraft_pathops::area(&PathData::new(vec![(*sp).clone()]), rule) > 100.0).count();
+    assert_eq!(big, 2, "{:?}", ring.subpaths.iter().map(|sp| vectorcraft_pathops::area(&PathData::new(vec![sp.clone()]), rule)).collect::<Vec<_>>());
+    // An inner edge would be a second outline over the merged area: normalising would drop it.
+    let clean = vectorcraft_pathops::normalize(&ring, rule);
+    assert!((ring.length() - clean.length()).abs() < 1.0, "{} pt of outline, {} without inner edges", ring.length(), clean.length());
+}

crates/pathops/src/lib.rs

@@ -19,7 +19,7 @@ pub use edit::{
     split_into_grid,
 };
 pub use offset::{Cap, Join, offset_path, outline_stroke, stroke_region};
-pub use pathfinder::{PathfinderOp, Region, Shape, merge_regions, pathfinder, region_at, regions};
+pub use pathfinder::{FaceMerger, PathfinderOp, Region, Shape, merge_regions, pathfinder, region_at, regions};
 pub use planar::{BuilderArrangement, SHAPE_BUILDER_MAX_SEGMENTS, cut_out, encloses_area, interior_point, live_paint, shape_builder};
 
 /// Errors from fallible operations.

crates/pathops/src/pathfinder.rs

@@ -7,7 +7,7 @@ use kurbo::{BezPath, ParamCurve, ParamCurveNearest, Point, Shape as _};
 use linesweeper::topology::ContourIdx;
 use vectorcraft_geom::{FillRule, PathData};
 
-use crate::boolean::{Arrangement, Multi, Seg, all_contours_to_path, contours_to_path, fill_bezpath, segs_to_subpath, unite_all};
+use crate::boolean::{Arrangement, Multi, Seg, all_contours_to_path, contours_to_path, fill_bezpath, normalize_bez, segs_to_subpath, unite_all};
 
 /// A filled shape in a Pathfinder stack. `key` identifies its paint (e.g. a hashed fill colour);
 /// results carry the key of the object whose paint they keep.
@@ -264,6 +264,63 @@ pub fn merge_regions(regions: &[&Region]) -> PathData {
     unite_all(&v)
 }
 
+/// Merges faces of the planar arrangement of a set of shapes (as [`regions`] gives them) into one
+/// path, from the arrangement itself, made once for every merge of the set: the faces' contours
+/// share their edges exactly, so the edges between merged faces vanish. Uniting the faces' tidied
+/// outlines ([`merge_regions`]) could leave those edges in, as a hairline gap or a spur, where
+/// neighbouring faces' outlines had been refit apart. Shapes with open paths (faces lines cut)
+/// fall back to [`merge_regions`].
+pub struct FaceMerger {
+    /// The arrangement (`None`: open paths, or the sweep failed) and the number of shapes.
+    arr: Option<Arrangement>,
+    shapes: usize,
+}
+
+impl FaceMerger {
+    pub fn new(shapes: &[Shape]) -> Self {
+        let has_open = shapes.iter().flat_map(|s| &s.path.subpaths).any(|sp| !sp.closed && sp.anchors.len() >= 2);
+        Self { arr: if has_open { None } else { arrangement(shapes) }, shapes: shapes.len() }
+    }
+
+    /// The union of `faces`.
+    pub fn merge(&self, faces: &[&Region]) -> PathData {
+        self.exact(faces).filter(|p| !p.is_empty()).unwrap_or_else(|| merge_regions(faces))
+    }
+
+    fn exact(&self, faces: &[&Region]) -> Option<PathData> {
+        let arr = self.arr.as_ref()?;
+        // Each face by its coverage and a point inside it.
+        let mut picks: Vec<(Vec<bool>, Point)> = Vec::with_capacity(faces.len());
+        for r in faces {
+            let mut mask = vec![false; self.shapes];
+            for &s in &r.sources {
+                *mask.get_mut(s)? = true;
+            }
+            picks.push((mask, crate::planar::interior_point(&r.path)?));
+        }
+        let mut raw = BezPath::new();
+        let mut done: Vec<&[bool]> = vec![];
+        for (mask, _) in &picks {
+            if done.contains(&mask.as_slice()) {
+                continue;
+            }
+            done.push(mask);
+            let c = arr.contours(|k| k == mask.as_slice());
+            for group in c.grouped() {
+                let mut face = BezPath::new();
+                for i in group {
+                    face.extend(c[i].path.iter());
+                }
+                if picks.iter().any(|(m, p)| m == mask && face.winding(*p) != 0) {
+                    raw.extend(face.iter());
+                }
+            }
+        }
+        let merged = normalize_bez(&raw, FillRule::NonZero).ok()?;
+        Some(all_contours_to_path(&merged, &arr.tidy))
+    }
+}
+
 /// Pathfinder Outline: split every shape's boundary at junctions of the arrangement.
 fn outline_edges(arr: &Arrangement, shapes: &[Shape]) -> Vec<Shape> {
     let junctions = arr.junctions();