Merge pull request #3148 from Silvertree2010/fix/3132-can-t-delete-the-outer-part-of-a

Keep an island inside a hole a separate Shape Builder region

crates/pathops/src/pathfinder.rs

@@ -236,32 +236,13 @@ fn regions_of(arr: &Arrangement) -> Vec<Region> {
             // pass per mask for this batch.
             for (m, mask) in masks.iter().enumerate().filter(|(m, _)| batch[*m] == b) {
                 let c = arr.contours(|k| k == mask.as_slice());
-                faces[m].extend(c.grouped().into_iter().map(|g| contours_to_path(&c, g, &arr.tidy)));
+                let owner = vec![m; c.contours().count()];
+                faces[m].extend(face_groups(&c, &owner).into_iter().map(|(_, g)| contours_to_path(&c, g, &arr.tidy)));
             }
             continue;
         };
-        // A contour belongs under the nearest enclosing contour of the same mask, as in that
-        // mask's own pass.
-        let mut children = vec![Vec::new(); owner.len()];
-        let mut roots = Vec::new();
-        for (i, &m) in owner.iter().enumerate() {
-            let mut up = c[ContourIdx(i)].parent;
-            while let Some(p) = up.filter(|p| owner.get(p.0) != Some(&m)) {
-                up = c[p].parent;
-            }
-            match up {
-                Some(p) => children[p.0].push(i),
-                None => roots.push(i),
-            }
-        }
-        for r in roots {
-            let mut group = Vec::new();
-            let mut stack = vec![r];
-            while let Some(i) = stack.pop() {
-                group.push(ContourIdx(i));
-                stack.extend(children[i].iter().rev());
-            }
-            faces[owner[r]].push(contours_to_path(&c, group, &arr.tidy));
+        for (m, group) in face_groups(&c, &owner) {
+            faces[m].push(contours_to_path(&c, group, &arr.tidy));
         }
     }
     let mut out = Vec::new();
@@ -272,6 +253,39 @@ fn regions_of(arr: &Arrangement) -> Vec<Region> {
     out
 }
 
+/// Splits a contour pass into faces: each face is an outer contour plus the holes of the same mask
+/// directly inside it (`owner` gives each contour's mask). A hole belongs under the nearest
+/// 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<ContourIdx>)> {
+    let mut children = vec![Vec::new(); owner.len()];
+    let mut roots = Vec::new();
+    for (i, &m) in owner.iter().enumerate() {
+        let ct = &c[ContourIdx(i)];
+        let mut up = if ct.outer { None } else { ct.parent };
+        while let Some(p) = up.filter(|p| owner.get(p.0) != Some(&m)) {
+            up = c[p].parent;
+        }
+        match up {
+            Some(p) => children[p.0].push(i),
+            None => roots.push(i),
+        }
+    }
+    roots
+        .into_iter()
+        .map(|r| {
+            let mut group = Vec::new();
+            let mut stack = vec![r];
+            while let Some(i) = stack.pop() {
+                group.push(ContourIdx(i));
+                stack.extend(children[i].iter().rev());
+            }
+            (owner[r], group)
+        })
+        .collect()
+}
+
 /// 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())

crates/pathops/tests/ops.rs

@@ -241,6 +241,26 @@ fn regions_and_region_at() {
     assert!(close(ar(&merge_regions(&refs)), 175.0, 1e-9));
 }
 
+#[test]
+fn regions_split_an_island_inside_a_hole_from_the_frame_around_it() {
+    // A frame (0..200 minus 50..150) with an island (80..120) in its hole, as Minus Front leaves
+    // a rectangle behind a ring (#3132): the frame and the island are separate faces.
+    let mut p = rect(0.0, 0.0, 200.0, 200.0);
+    p.subpaths.extend(rect(50.0, 50.0, 100.0, 100.0).subpaths);
+    p.subpaths.extend(rect(80.0, 80.0, 40.0, 40.0).subpaths);
+    let s = vec![Shape::new(p, FillRule::EvenOdd, 1)];
+    let rs = regions(&s);
+    let mut filled: Vec<(f64, Rect)> = rs.iter().filter(|r| r.sources == vec![0]).map(|r| (ar(&r.path).abs(), r.path.bounds().unwrap())).collect();
+    filled.sort_by(|a, b| b.0.total_cmp(&a.0));
+    assert_eq!(filled.len(), 2, "{filled:?}");
+    assert!(close(filled[0].0, 30000.0, 1e-9), "{filled:?}");
+    assert_eq!(filled[0].1, Rect::new(0.0, 0.0, 200.0, 200.0));
+    assert!(close(filled[1].0, 1600.0, 1e-9), "{filled:?}");
+    assert_eq!(filled[1].1, Rect::new(80.0, 80.0, 120.0, 120.0));
+    let frame = region_at(&s, Point::new(10.0, 10.0)).unwrap();
+    assert!(!frame.contains(Point::new(100.0, 100.0)));
+}
+
 #[test]
 fn offset_circle_grows() {
     let o = offset_path(&circle(0.0, 0.0, 10.0), 3.0, Join::Round, 4.0);