From 79c5218f8ad6287e6a7668196dda2ab2eb94e164 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 18:59:49 -0800 Subject: [PATCH] Fix #5161: Shape Builder merges across a line under a merged shape's edge Where a line lies under the outline of a closed path in front of it (such as a shape an earlier merge made, which takes the place of the art in front of the lines), the planar map gave that stretch only to the closed path, so the line had no piece there. A merge or an erase across it left the line whole, and the next map split the new shape along it into two regions. - pathops: each planar edge records every input it lies on, and BuilderArrangement::lines_under holds the lines' pieces under a closed outline in front of them. lines, edges and Live Paint are unchanged. - tools: BuilderMap adds those pieces to its lines, so a merge or an erase removes the stretch between the regions it takes, as it does elsewhere. An Alt-click where a line lies under an edge hits the one in front. --- crates/engine/src/tests/build.rs | 45 ++++++++++++++++ crates/pathops/src/planar/graph.rs | 11 ++-- crates/pathops/src/planar/live_paint.rs | 2 +- crates/pathops/src/planar/map.rs | 63 ++++++++++++++-------- crates/pathops/src/planar/shape_builder.rs | 20 +++++-- crates/pathops/src/planar/tests.rs | 19 +++++++ crates/tools/src/builder/map.rs | 12 +++-- crates/tools/src/builder/tests.rs | 20 +++++++ 8 files changed, 158 insertions(+), 34 deletions(-) diff --git a/crates/engine/src/tests/build.rs b/crates/engine/src/tests/build.rs index bd9bd02a2..b0d839220 100644 --- a/crates/engine/src/tests/build.rs +++ b/crates/engine/src/tests/build.rs @@ -785,6 +785,51 @@ fn shape_builder_merge_across_a_line_removes_the_piece_between() { assert_eq!(layer_children(&s), 2); } +/// A line from (100, 200) to (300, 200) and one from (200, 100) to (200, 300), with an unfilled +/// square from (100, 100) to (300, 300) in front of them, selected; the top row merged, which puts +/// the merged shape in front of the lines with its bottom edge along the first line. Returns the +/// first line and the merged shape. +fn lines_under_a_merged_row(s: &mut Session) -> (NodeId, NodeId) { + let h = line(s, (100.0, 200.0), (300.0, 200.0)); + let v = line(s, (200.0, 100.0), (200.0, 300.0)); + let sq = rect(s, 100.0, 100.0, 200.0, 200.0); + s.edit("t", |d, _| { + for id in [h, v, sq] { + d.node_mut(id).unwrap().appearance.set_fill(Paint::None); + } + Ok(()) + }) + .unwrap(); + s.execute("select.set", &json!({"ids": [h.0, v.0, sq.0]})).unwrap(); + let r = s.execute("shapeBuilder.merge", &json!({"points": [[150, 150], [250, 150]]})).unwrap(); + (h, NodeId(r["merged"].as_u64().unwrap())) +} + +/// The left half of `h` (from x = 100 to 200) is gone. +fn left_half_gone(s: &Session, h: NodeId) -> bool { + s.doc().unwrap().doc.node(h).is_none_or(|n| outline(n).0.bounds().is_none_or(|b| b.x0 > 199.0)) +} + +#[test] +fn shape_builder_drag_from_a_merged_shape_over_a_line_under_its_edge_makes_one_region() { + let mut s = session(); + let (h, m1) = lines_under_a_merged_row(&mut s); + let r = s.execute("shapeBuilder.merge", &json!({"points": [[250, 150], [150, 250]]})).unwrap(); + let merged = NodeId(r["merged"].as_u64().unwrap()); + assert!(s.doc().unwrap().doc.node(m1).is_none(), "the merged row is part of the new shape"); + assert!((area(&s, merged) - 30000.0).abs() < 1.0); + assert!(left_half_gone(&s, h), "the line's stretch between the merged regions is removed"); + let r = s.execute("shapeBuilder.regions", &json!({})).unwrap(); + let ours = r["regions"].as_array().unwrap().iter().filter(|x| x["sources"] == json!([merged.0])).count(); + assert_eq!(ours, 1, "{r}"); + + // Erasing from the merged row into the cell below takes the line's stretch between them too. + let mut s = session(); + let (h, _) = lines_under_a_merged_row(&mut s); + s.execute("shapeBuilder.merge", &json!({"points": [[250, 150], [150, 250]], "erase": true})).unwrap(); + assert!(left_half_gone(&s, h)); +} + #[test] fn shape_builder_alt_deletes_a_region_or_a_line_piece() { let mut s = session(); diff --git a/crates/pathops/src/planar/graph.rs b/crates/pathops/src/planar/graph.rs index a260953f7..dca4c99e9 100644 --- a/crates/pathops/src/planar/graph.rs +++ b/crates/pathops/src/planar/graph.rs @@ -19,11 +19,14 @@ impl Outlines { Self { segs: RectIndex::with_footprint(segs, |r, (s, _), cell| segment_footprint(r, s, cell)), tol: 16.0 * eps } } - /// The front-most input whose outline passes through `p`. - pub(super) fn source_at(&self, p: Point) -> Option { + /// The inputs whose outlines pass through `p`, back to front (none when no outline does). + pub(super) fn sources_at(&self, p: Point) -> Vec { use kurbo::ParamCurveNearest; let tol = self.tol; - self.segs.at(p, tol).filter(|(s, _)| s.nearest(p, 1e-9).distance_sq <= tol * tol).map(|(_, i)| *i).max() + let mut out: Vec = self.segs.at(p, tol).filter(|(s, _)| s.nearest(p, 1e-9).distance_sq <= tol * tol).map(|(_, i)| *i).collect(); + out.sort_unstable(); + out.dedup(); + out } } @@ -54,6 +57,8 @@ pub(super) struct Edge { pub(super) ends: [usize; 2], /// The input it came from (the front-most, where inputs share it). pub(super) source: usize, + /// Every input it lies on, back to front (`source` last). + pub(super) sources: Vec, } /// `segs` (a chain) moved to start at `a` and end at `b`, the handles next to each moving with it. diff --git a/crates/pathops/src/planar/live_paint.rs b/crates/pathops/src/planar/live_paint.rs index 92c126e57..1b9d5b617 100644 --- a/crates/pathops/src/planar/live_paint.rs +++ b/crates/pathops/src/planar/live_paint.rs @@ -8,7 +8,7 @@ use super::*; /// wherever they meet another path, keyed by the input they came from (the front-most where /// inputs share one). Empty when the paths can't be swept (non-finite or too degenerate). pub fn live_paint(shapes: &[Shape]) -> (Vec, Vec) { - build(shapes, DEFAULT_PRECISION).unwrap_or_default() + build(shapes, DEFAULT_PRECISION).map(|(faces, edges, _)| (faces, edges)).unwrap_or_default() } /// [`live_paint`] with Gap Detection: the faces are those of `shapes` with a line across each gap diff --git a/crates/pathops/src/planar/map.rs b/crates/pathops/src/planar/map.rs index 2b673c3a3..ec8b0b76c 100644 --- a/crates/pathops/src/planar/map.rs +++ b/crates/pathops/src/planar/map.rs @@ -303,8 +303,10 @@ fn sweep_graph(inputs: &[(BezPath, usize)], eps: f64, precision: f64) -> Option< // coordinates: merging them (a union) then cancels their common edge, which it can't when // each face carries its own rounding of the corner (#893). pin_ends(&mut segs, p0, p1); - let Some(source) = mid(&segs).and_then(|m| outlines.source_at(m)) else { continue }; - // Coincident pieces the sweep left apart are one edge (the front-most input's). + let sources = mid(&segs).map(|m| outlines.sources_at(m)).unwrap_or_default(); + let Some(&source) = sources.last() else { continue }; + // Coincident pieces the sweep left apart are one edge (the front-most input's), lying on + // the inputs of both. let key = (ends[0].min(ends[1]), ends[0].max(ends[1])); let here = mid(&segs); let twin_of = between.get(&key).and_then(|v| { @@ -314,15 +316,16 @@ fn sweep_graph(inputs: &[(BezPath, usize)], eps: f64, precision: f64) -> Option< }) }); if let Some(e) = twin_of { - if let Some(e) = edges.get_mut(e) - && source > e.source - { - e.source = source; + if let Some(e) = edges.get_mut(e) { + e.source = e.source.max(source); + e.sources.extend(sources); + e.sources.sort_unstable(); + e.sources.dedup(); } continue; } between.entry(key).or_default().push(edges.len()); - edges.push(Edge { segs, ends, source }); + edges.push(Edge { segs, ends, source, sources }); } Some((points, edges)) } @@ -414,7 +417,10 @@ impl Map { } } -pub(super) fn build(shapes: &[Shape], precision: f64) -> Option<(Vec, Vec)> { +/// Faces and edges of `shapes`, and the pieces of the open paths (those with no closed subpath) +/// that lie under the outline of a closed path in front of them, whose edges are that closed +/// path's. +pub(super) fn build(shapes: &[Shape], precision: f64) -> Option<(Vec, Vec, Vec)> { let m = map_with(shapes, false, precision)?; let Map { edges, around, faces, holes, face_holes, tidy, anchors, snap, .. } = &m; let snap = *snap; @@ -441,18 +447,38 @@ pub(super) fn build(shapes: &[Shape], precision: f64) -> Option<(Vec, Ve .collect(); // Edges: chains of segments through vertices where nothing else meets. - let degree: Vec = around.iter().map(Vec::len).collect(); + let to_shapes = |chains: Vec<(Vec, bool, usize)>| -> Vec { + chains + .into_iter() + .filter_map(|(segs, closed, source)| { + let sp = segs_to_subpath(&tidy_segments(segs, closed, tidy), closed)?; + Some(Shape::new(unshear(PathData::new(vec![sp]), anchors, snap), FillRule::NonZero, shapes.get(source)?.key)) + }) + .collect() + }; + let edge_shapes = to_shapes(chains_by(edges, around, |e| Some(e.source))); + let is_line = |i: usize| shapes.get(i).is_some_and(|s| !s.path.subpaths.iter().any(|sp| sp.closed)); + let under = to_shapes(chains_by(edges, around, |e| if is_line(e.source) { None } else { e.sources.iter().rev().copied().find(|&i| is_line(i)) })); + Some((regions, edge_shapes, under)) +} + +/// The edges that `key` gives an input, joined into chains through the vertices where only two +/// edges of the same input meet, each with that input. A chain is closed when it comes back round +/// to where it started. +fn chains_by(edges: &[Edge], around: &[Vec], key: impl Fn(&Edge) -> Option) -> Vec<(Vec, bool, usize)> { + let keys: Vec> = edges.iter().map(&key).collect(); + let key_of = |h: usize| keys.get(h / 2).copied().flatten(); let passes = |v: usize| -> Option<(usize, usize)> { let list = around.get(v)?; let (&a, &b) = (list.first()?, list.get(1)?); - (degree.get(v) == Some(&2) && edges.get(a / 2)?.source == edges.get(b / 2)?.source).then_some((a, b)) + (list.len() == 2 && key_of(a).is_some() && key_of(a) == key_of(b)).then_some((a, b)) }; - let mut used = vec![false; edges.len()]; + // Edges with no input to give are never walked. + let mut used: Vec = keys.iter().map(Option::is_none).collect(); let mut chains: Vec<(Vec, bool, usize)> = Vec::new(); - // A chain from half-edge `h0` up to the next vertex where it doesn't pass straight through; - // closed when it comes back round to where it started. + // A chain from half-edge `h0` up to the next vertex where it doesn't pass straight through. let walk = |h0: usize, used: &mut Vec| -> Option<(Vec, bool, usize)> { - let source = edges.get(h0 / 2)?.source; + let source = key_of(h0)?; let mut segs = Vec::new(); let mut h = h0; loop { @@ -487,14 +513,7 @@ pub(super) fn build(shapes: &[Shape], precision: f64) -> Option<(Vec, Ve chains.push(c); } } - let edge_shapes = chains - .into_iter() - .filter_map(|(segs, closed, source)| { - let sp = segs_to_subpath(&tidy_segments(segs, closed, tidy), closed)?; - Some(Shape::new(unshear(PathData::new(vec![sp]), anchors, snap), FillRule::NonZero, shapes.get(source)?.key)) - }) - .collect(); - Some((regions, edge_shapes)) + chains } /// A point strictly inside a filled path, well away from its edges where it can be (for re-finding diff --git a/crates/pathops/src/planar/shape_builder.rs b/crates/pathops/src/planar/shape_builder.rs index 1ed84be79..290bc270d 100644 --- a/crates/pathops/src/planar/shape_builder.rs +++ b/crates/pathops/src/planar/shape_builder.rs @@ -18,6 +18,10 @@ pub struct BuilderArrangement { /// The pieces of the other paths' outlines, keyed by input index (the front-most where inputs /// share one); only when asked for. pub edges: Vec, + /// The pieces of the paths with no closed subpath that lie under the outline of a closed path + /// in front of them, keyed by input index. No piece of `lines` covers them; there the edge is + /// the closed path's (in `edges`, when asked for). + pub lines_under: Vec, } /// Does any of `shapes` have an open subpath (a cutting edge for the Shape Builder)? @@ -74,15 +78,16 @@ pub(crate) fn builder(shapes: &[Shape], edges: bool, gap: f64, precision: f64) - .then(|| { let keyed: Vec = shapes.iter().enumerate().map(|(i, s)| Shape::new(s.path.clone(), s.rule, i as u64)).collect(); if has_open && gap.is_finite() && gap > 0.0 { - let (faces, pieces) = build(&gap_inputs(&keyed, gap), precision).unwrap_or_default(); + let (faces, pieces, under) = build(&gap_inputs(&keyed, gap), precision).unwrap_or_default(); // Gap targets split original outlines too, but the bridges themselves aren't art. - (faces, pieces.into_iter().filter(|p| p.key < shapes.len() as u64).collect()) + let art = |p: &Shape| p.key < shapes.len() as u64; + (faces, pieces.into_iter().filter(art).collect(), under.into_iter().filter(art).collect()) } else { build(&keyed, precision).unwrap_or_default() } }) - .filter(|(faces, pieces)| !faces.is_empty() || !pieces.is_empty()); - let Some((faces, pieces)) = planar else { + .filter(|(faces, pieces, _)| !faces.is_empty() || !pieces.is_empty()); + let Some((faces, pieces, under)) = planar else { return BuilderArrangement { regions: filled_regions(shapes, precision), ..Default::default() }; }; let is_line = |i: u64| shapes.get(i as usize).is_some_and(|s| !s.path.subpaths.iter().any(|sp| sp.closed)); @@ -95,7 +100,12 @@ pub(crate) fn builder(shapes: &[Shape], edges: bool, gap: f64, precision: f64) - regions.extend(faces.into_iter().filter(|f| f.sources.is_empty())); regions }; - BuilderArrangement { regions, lines: if has_open { lines } else { Vec::new() }, edges: if edges { outline } else { Vec::new() } } + BuilderArrangement { + regions, + lines: if has_open { lines } else { Vec::new() }, + edges: if edges { outline } else { Vec::new() }, + lines_under: if has_open { under } else { Vec::new() }, + } } /// The regions of the shapes' filled areas. A shape that fills nothing (open paths that enclose no diff --git a/crates/pathops/src/planar/tests.rs b/crates/pathops/src/planar/tests.rs index bf9340189..3ca7a4814 100644 --- a/crates/pathops/src/planar/tests.rs +++ b/crates/pathops/src/planar/tests.rs @@ -383,6 +383,25 @@ fn shape_builder_lines_cut_regions() { assert_eq!(lines.len(), 9); } +#[test] +fn shape_builder_keeps_the_pieces_of_a_line_under_an_outline_in_front() { + // A line whose left half lies under the bottom edge of a rectangle in front of it. + let s = [line((0.0, 50.0), (200.0, 50.0), 0), rect(0.0, 0.0, 100.0, 50.0, 1)]; + let BuilderArrangement { lines, edges, lines_under, .. } = shape_builder(&s, true); + let span = |l: &Shape| l.path.bounds().map(|b| (b.x0, b.x1)); + assert_eq!(lines.iter().map(span).collect::>(), [Some((100.0, 200.0))], "{lines:?}"); + assert_eq!(lines_under.iter().map(span).collect::>(), [Some((0.0, 100.0))], "{lines_under:?}"); + assert!(lines.iter().chain(&lines_under).all(|l| l.key == 0)); + // The rectangle's outline keeps its edge there. + assert!(!edges.is_empty() && edges.iter().all(|e| e.key == 1 && e.path.bounds() == Some(Rect::new(0.0, 0.0, 100.0, 50.0))), "{edges:?}"); + // Live Paint keeps one edge there, the front-most input's. + let (_, pieces) = live_paint(&s); + assert!(!pieces.iter().any(|p| p.key == 0 && p.path.bounds().is_some_and(|b| b.x1 <= 100.0)), "{pieces:?}"); + // A line in front of the outline has no pieces under it. + let s = [rect(0.0, 0.0, 100.0, 50.0, 0), line((0.0, 50.0), (200.0, 50.0), 1)]; + assert!(shape_builder(&s, true).lines_under.is_empty()); +} + #[test] fn shape_builder_merges_adjacent_gap_closed_faces_without_keeping_bridges() { let outline = Shape::new( diff --git a/crates/tools/src/builder/map.rs b/crates/tools/src/builder/map.rs index cf9327dd4..e217d6980 100644 --- a/crates/tools/src/builder/map.rs +++ b/crates/tools/src/builder/map.rs @@ -81,7 +81,10 @@ impl BuilderMap { }) .collect() }; - Self { shapes, regions: faces_of(arr.regions), lines: bounded(arr.lines), edges: bounded(arr.edges) } + // A line keeps its pieces under the outline of a closed path in front of it, so a merge + // or an erase removes them as it does the rest of the line. + let lines = arr.lines.into_iter().chain(arr.lines_under).collect(); + Self { shapes, regions: faces_of(arr.regions), lines: bounded(lines), edges: bounded(arr.edges) } } /// The region containing `p`. @@ -117,10 +120,13 @@ impl BuilderMap { } /// The line piece or edge within `piece_tol` of `p` that [`Self::hit`] gives (None without - /// `piece_tol`). + /// `piece_tol`). Where a line lies under an edge, the one in front is hit. fn piece_hit(&self, p: Point, piece_tol: Option) -> Option { + let key = |pieces: &[(po::Shape, Rect)], i: usize| pieces.get(i).map(|(s, _)| s.key); piece_tol.and_then(|t| match (piece_near(&self.lines, p, t), piece_near(&self.edges, p, t)) { - (Some(l), Some(e)) if e.0 < l.0 => Some(Hit::Edge(e.1)), + (Some(l), Some(e)) if e.0 < l.0 - 1e-6 * t || (e.0 <= l.0 + 1e-6 * t && key(&self.edges, e.1) > key(&self.lines, l.1)) => { + Some(Hit::Edge(e.1)) + } (Some(l), _) => Some(Hit::Line(l.1)), (None, e) => e.map(|e| Hit::Edge(e.1)), }) diff --git a/crates/tools/src/builder/tests.rs b/crates/tools/src/builder/tests.rs index da77c9d11..acaac0ec8 100644 --- a/crates/tools/src/builder/tests.rs +++ b/crates/tools/src/builder/tests.rs @@ -343,6 +343,26 @@ fn map_of(d: &Document, s: &Selection) -> BuilderMap { BuilderMap::new(d, &sorted_roots(d, &s.objects), true, true).0 } +#[test] +fn shape_builder_keeps_a_line_under_an_edge_in_front_and_alt_click_there_takes_the_edge() { + // A line from (0, 50) to (200, 50), then a rectangle in front of it whose bottom edge lies + // along its left half. + let mut d = Document::new(500.0, 500.0); + let l = d.layers[0].id; + let mut s = Selection::default(); + let line = PathData::new(vec![SubPath::polyline(&[Point::new(0.0, 50.0), Point::new(200.0, 50.0)], false)]); + for (k, path) in [line, shapes::rectangle(Rect::new(0.0, 0.0, 100.0, 50.0))].into_iter().enumerate() { + let id = d.alloc_id(); + d.insert(Some(l), k, Node::path(id, path, Appearance::default_art())).unwrap(); + s.add(id); + } + let map = map_of(&d, &s); + let under = map.lines.iter().any(|(piece, b)| piece.key == 0 && b.x1 <= 100.5 && (b.y0 - 50.0).abs() < 1e-6 && b.width() > 99.0); + assert!(under, "the line keeps its piece under the rectangle's edge: {:?}", map.lines.iter().map(|(_, b)| b).collect::>()); + assert!(matches!(map.hit(Point::new(50.0, 51.0), Some(4.0)), Some(Hit::Edge(_))), "the rectangle in front is hit"); + assert!(matches!(map.hit(Point::new(150.0, 51.0), Some(4.0)), Some(Hit::Line(_)))); +} + #[test] fn shape_builder_erase_drag_across_an_overlap_narrower_than_twice_the_tolerance_touches_it() { let (d, s) = sliver_rects();