From b814fcf689cb1c584d09c5f1e28f697464877660 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 5 Nov 2026 02:35:32 +0700 Subject: [PATCH] Pathfinder: holes that touch at a point stay holes of their region The Live Paint faces were right; the Pathfinder regions over-counted. Two small circles inside a big one, touching each other at one point, gave the big circle's region only one of its two holes, and the other hole came out as a region of its own. linesweeper walks the boundary of the two touching holes as one loop and, when the walk comes back to the shared point, cuts the second hole off as a child of the first and flags it outer, as if it were an island inside it. face_groups trusted that flag, so the hole started a face. The same cut goes wrong the other way when an outer contour touches its own hole: the outer contour comes out as an island inside the hole. Such a loop winds the same way as the contour it was cut from, against its flag. face_groups now checks every contour's flag against its winding; a pass where any disagrees is grouped by geometry instead (each hole goes with the smallest outer contour of its mask around it). Passes where they all agree keep the sweep's tree as before, at the cost of one area per contour. Adds deterministic tests for both shapes of the bug and checks in the proptest regression seed. --- .../proptest-regressions/planar/tests.txt | 7 ++ crates/pathops/src/pathfinder.rs | 79 ++++++++++++++++++- crates/pathops/src/planar/tests.rs | 19 +++++ 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 crates/pathops/proptest-regressions/planar/tests.txt diff --git a/crates/pathops/proptest-regressions/planar/tests.txt b/crates/pathops/proptest-regressions/planar/tests.txt new file mode 100644 index 000000000..2587908e1 --- /dev/null +++ b/crates/pathops/proptest-regressions/planar/tests.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 799771bbae95181944a44d3a40559928606eb57d3d6a3cf4916ed37bc9bf2c15 # shrinks to shapes = [Shape { path: PathData { subpaths: [SubPath { anchors: [Anchor { p: (20.0, 85.0), h_in: (20.0, 87.76142374915396), h_out: (20.0, 82.23857625084604), kind: Smooth }, Anchor { p: (25.0, 80.0), h_in: (22.23857625084603, 80.0), h_out: (27.76142374915397, 80.0), kind: Smooth }, Anchor { p: (30.0, 85.0), h_in: (30.0, 82.23857625084604), h_out: (30.0, 87.76142374915396), kind: Smooth }, Anchor { p: (25.0, 90.0), h_in: (27.76142374915397, 90.0), h_out: (22.23857625084603, 90.0), kind: Smooth }], closed: true }] }, rule: EvenOdd, key: 0 }, Shape { path: PathData { subpaths: [SubPath { anchors: [Anchor { p: (10.0, 80.0), h_in: (10.0, 91.04569499661588), h_out: (10.0, 68.95430500338412), kind: Smooth }, Anchor { p: (30.0, 60.0), h_in: (18.95430500338413, 60.0), h_out: (41.04569499661587, 60.0), kind: Smooth }, Anchor { p: (50.0, 80.0), h_in: (50.0, 68.95430500338412), h_out: (50.0, 91.04569499661588), kind: Smooth }, Anchor { p: (30.0, 100.0), h_in: (41.04569499661587, 100.0), h_out: (18.95430500338413, 100.0), kind: Smooth }], closed: true }] }, rule: EvenOdd, key: 0 }, Shape { path: PathData { subpaths: [SubPath { anchors: [Anchor { p: (30.0, 85.0), h_in: (30.0, 87.76142374915396), h_out: (30.0, 82.23857625084604), kind: Smooth }, Anchor { p: (35.0, 80.0), h_in: (32.23857625084603, 80.0), h_out: (37.76142374915397, 80.0), kind: Smooth }, Anchor { p: (40.0, 85.0), h_in: (40.0, 82.23857625084604), h_out: (40.0, 87.76142374915396), kind: Smooth }, Anchor { p: (35.0, 90.0), h_in: (37.76142374915397, 90.0), h_out: (32.23857625084603, 90.0), kind: Smooth }], closed: true }] }, rule: EvenOdd, key: 0 }] diff --git a/crates/pathops/src/pathfinder.rs b/crates/pathops/src/pathfinder.rs index 13476fbbc..5e6f55135 100644 --- a/crates/pathops/src/pathfinder.rs +++ b/crates/pathops/src/pathfinder.rs @@ -330,7 +330,18 @@ impl CoverIds { /// enclosing contour of its mask, as in that mask's own pass; an outer contour always starts a face /// of its own, even inside a hole of its mask (an island left in a cutout is a separate face, not /// part of the frame around it). -fn face_groups(c: &linesweeper::topology::Contours, owner: &[usize]) -> Vec<(usize, Vec)> { +/// +/// The sweep's tree of contours is wrong where it cut a loop off a contour at a point the contour +/// visits twice and the loop isn't nested in it: two holes touching at a point (the second comes +/// out an island inside the first), or an outer contour touching its own hole (it comes out an +/// island inside the hole). Such a loop winds the same way as the contour it was cut from, against +/// its flag ([`Contour::outer`](linesweeper::topology::Contour::outer)), and the pass is then +/// grouped by where the contours lie instead ([`face_groups_by_winding`]). +fn face_groups(c: &Contours, owner: &[usize]) -> Vec<(usize, Vec)> { + let areas: Vec = c.contours().map(|ct| ct.path.area()).collect(); + if c.contours().zip(&areas).any(|(ct, &a)| ct.outer != (a > 0.0)) { + return face_groups_by_winding(c, owner, &areas); + } let mut children = vec![Vec::new(); owner.len()]; let mut roots = Vec::new(); for (i, &m) in owner.iter().enumerate() { @@ -358,6 +369,47 @@ fn face_groups(c: &linesweeper::topology::Contours, owner: &[usize]) -> Vec<(usi .collect() } +/// [`face_groups`] without the sweep's tree: each hole (a contour whose area in `areas` isn't +/// positive) goes with the smallest outer contour of its mask around it, and each outer contour, +/// or hole with none around it, starts a face. Faces come in the order of their first contour, +/// each with its contours in pass order. +fn face_groups_by_winding(c: &Contours, owner: &[usize], areas: &[f64]) -> Vec<(usize, Vec)> { + let boxes: Vec = c.contours().map(|ct| ct.path.bounding_box()).collect(); + let outer = |i: usize| areas.get(i).is_some_and(|&a| a > 0.0); + let mut root: Vec = (0..owner.len()).collect(); + for (i, ct) in c.contours().enumerate() { + // Contours of one mask share no edge, so the middle of a hole's first piece is strictly + // inside or outside each outer contour of its mask. + let (false, Some(p), Some(b)) = (outer(i), ct.path.segments().next().map(|s| s.eval(0.5)), boxes.get(i)) else { + continue; + }; + let around = c + .contours() + .enumerate() + .filter(|&(j, o)| { + outer(j) && owner.get(j) == owner.get(i) && boxes.get(j).is_some_and(|bj| bj.union(*b) == *bj) && o.path.winding(p) != 0 + }) + .min_by(|(j, _), (k, _)| areas.get(*j).unwrap_or(&f64::INFINITY).total_cmp(areas.get(*k).unwrap_or(&f64::INFINITY))) + .map(|(j, _)| j); + if let (Some(j), Some(r)) = (around, root.get_mut(i)) { + *r = j; + } + } + let mut slot: Vec> = vec![None; owner.len()]; + let mut groups: Vec<(usize, Vec)> = Vec::new(); + for (i, &r) in root.iter().enumerate() { + let (Some(s), Some(&m)) = (slot.get_mut(r), owner.get(r)) else { continue }; + let k = *s.get_or_insert_with(|| { + groups.push((m, Vec::new())); + groups.len() - 1 + }); + if let Some((_, g)) = groups.get_mut(k) { + g.push(ContourIdx(i)); + } + } + groups +} + /// A point as a map key (the arrangement's vertices are exact). fn vertex_key(p: Point) -> (u64, u64) { (p.x.to_bits(), p.y.to_bits()) @@ -1020,6 +1072,31 @@ mod tests { .prop_map(|(p, even_odd, key)| Shape::new(p, if even_odd { FillRule::EvenOdd } else { FillRule::NonZero }, key)) } + /// Contours that touch at a point, which the sweep's tree of contours gets wrong + /// ([`face_groups`]): each face keeps its holes, and no hole is a face of its own. + #[test] + fn contours_touching_at_a_point_keep_their_faces() { + let sorted = |shapes: &[Shape]| { + let mut v: Vec<(Vec, f64)> = + regions(shapes).iter().map(|r| (r.sources.clone(), crate::area(&r.path, FillRule::NonZero))).collect(); + v.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.total_cmp(&b.1))); + v + }; + // A frame whose cutout corner (60,110) touches its hole: the sweep makes the frame an + // island inside the hole. + let mut frame = rect(50.0, 100.0, 80.0, 130.0); + frame.subpaths.extend(rect(60.0, 110.0, 65.0, 115.0).subpaths); + let s = [Shape::new(rect(20.0, 70.0, 60.0, 110.0), FillRule::NonZero, 0), Shape::new(frame, FillRule::EvenOdd, 0)]; + assert_eq!(sorted(&s), [(vec![0], 1500.0), (vec![0, 1], 100.0), (vec![1], 775.0)]); + // Two holes touching at (30,50): the sweep makes the second an island inside the first. + let s = [ + Shape::new(rect(0.0, 0.0, 100.0, 100.0), FillRule::NonZero, 0), + Shape::new(rect(20.0, 40.0, 30.0, 50.0), FillRule::NonZero, 0), + Shape::new(rect(30.0, 50.0, 40.0, 60.0), FillRule::NonZero, 0), + ]; + assert_eq!(sorted(&s), [(vec![0], 9800.0), (vec![0, 1], 100.0), (vec![0, 2], 100.0)]); + } + #[test] fn crossing_strips_match_one_mask_at_a_time() { let mut v = Vec::new(); diff --git a/crates/pathops/src/planar/tests.rs b/crates/pathops/src/planar/tests.rs index d1d05d4df..654d7d5cb 100644 --- a/crates/pathops/src/planar/tests.rs +++ b/crates/pathops/src/planar/tests.rs @@ -1075,3 +1075,22 @@ fn many_lines_stay_quick() { assert!(faces.len() > 500, "{}", faces.len()); assert!(started.elapsed().as_secs_f64() < 5.0, "{:?}", started.elapsed()); } + +#[test] +fn holes_touching_at_a_point_stay_holes() { + // Two small circles inside a big one, touching each other at (30,85): the big circle's face + // has both as holes, and every face and region is one of the three. + let circle = |cx: f64, cy: f64, r: f64| Shape::new(shapes::ellipse(Rect::new(cx - r, cy - r, cx + r, cy + r)), FillRule::EvenOdd, 0); + let closed = [circle(25.0, 85.0, 5.0), circle(30.0, 80.0, 20.0), circle(35.0, 85.0, 5.0)]; + let (big, small) = (400.0 * std::f64::consts::PI, 25.0 * std::f64::consts::PI); + let (faces, _) = live_paint(&closed); + let regions = crate::regions(&closed); + for found in [&faces, ®ions] { + let mut sizes: Vec<(Vec, f64)> = found.iter().map(|r| (r.sources.clone(), area(r))).collect(); + sizes.sort_by(|a, b| a.0.cmp(&b.0)); + assert_eq!(sizes.iter().map(|s| s.0.clone()).collect::>(), [vec![0, 1], vec![1], vec![1, 2]], "{sizes:?}"); + for ((_, a), want) in sizes.iter().zip([small, big - 2.0 * small, small]) { + assert!((a - want).abs() < 0.5, "{a} vs {want}: {sizes:?}"); + } + } +}