From 39daf465f51a6cb9cecad409c4c35c402661a820 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 19 Oct 2026 21:25:19 +0200 Subject: [PATCH] Keep an island inside a hole a separate Shape Builder region Fixes #3132 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/pathops/src/pathfinder.rs | 60 ++++++++++++++++++++------------ crates/pathops/tests/ops.rs | 20 +++++++++++ 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/crates/pathops/src/pathfinder.rs b/crates/pathops/src/pathfinder.rs index fbd4ddfcc..aa63f0381 100644 --- a/crates/pathops/src/pathfinder.rs +++ b/crates/pathops/src/pathfinder.rs @@ -236,32 +236,13 @@ fn regions_of(arr: &Arrangement) -> Vec { // 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 { 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)> { + 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()) diff --git a/crates/pathops/tests/ops.rs b/crates/pathops/tests/ops.rs index cc1da584c..52d1af8c8 100644 --- a/crates/pathops/tests/ops.rs +++ b/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);