From b6422e5ba1411b1d012d7f6fc68f43d6d90c4be6 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:23:26 +0100 Subject: [PATCH] Keep compound path holes when using the Eraser and Knife Fixes #4495 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/draw2/common.rs | 27 ++++++++++++++++ crates/engine/src/cmd/draw2/cut.rs | 9 +++--- crates/engine/src/tests/draw2.rs | 46 +++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/draw2/common.rs b/crates/engine/src/cmd/draw2/common.rs index 2812b2fdc..b8c598a2d 100644 --- a/crates/engine/src/cmd/draw2/common.rs +++ b/crates/engine/src/cmd/draw2/common.rs @@ -168,6 +168,33 @@ pub(super) fn target_paths(doc: &Document, sel: &Selection, area: Rect) -> Vec Result> { + let closed_compound = |n: &Node| { + matches!(&n.kind, NodeKind::Compound { children, .. } + if children.iter().all(|c| matches!(&c.kind, NodeKind::Path { path, .. } if path.is_closed()))) + }; + let editable = doc.editable(); + let mut out: Vec<(NodeId, PathData, FillRule)> = vec![]; + for id in target_paths(doc, sel, area) { + let compound = doc.parent_of(id).and_then(|p| doc.node(p)).filter(|p| closed_compound(p) && editable(p.id)); + let target = compound.map_or(id, |c| c.id); + if out.iter().any(|t| t.0 == target) { + continue; + } + let (pd, rule) = match compound.map(|c| &c.kind) { + Some(NodeKind::Compound { children, rule }) => { + (PathData::new(children.iter().filter_map(|c| c.path_data()).flat_map(|p| p.subpaths.clone()).collect()), *rule) + } + _ => path_of(doc, id)?, + }; + out.push((target, pd, rule)); + } + Ok(out) +} + /// The members of a compound path with geometry `pd`: one path per subpath, with fresh ids. fn compound_members(d: &mut Document, pd: PathData) -> Vec> { pd.subpaths.into_iter().map(|sp| Arc::new(Node::path(d.alloc_id(), PathData::single(sp), Appearance::default()))).collect() diff --git a/crates/engine/src/cmd/draw2/cut.rs b/crates/engine/src/cmd/draw2/cut.rs index 4747ef783..e3a566f17 100644 --- a/crates/engine/src/cmd/draw2/cut.rs +++ b/crates/engine/src/cmd/draw2/cut.rs @@ -1,5 +1,6 @@ use super::common::{ - dist_to_polyline, ids_json, path_mut, path_of, points_param, poly_bounds, replace_with_pieces, sibling_with, target_paths, usize_opt, + dist_to_polyline, fill_targets, ids_json, path_mut, path_of, points_param, poly_bounds, replace_with_pieces, sibling_with, target_paths, + usize_opt, }; use super::{ Anchor, AnchorKind, Beside, BezPath, BoolOp, Cap, EngineError, FillRule, Join, NodeId, ParamCurve, PathData, Point, Result, Session, SubPath, @@ -236,8 +237,7 @@ pub(super) fn knife(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; let strip = po::outline_stroke(&PathData::single(SubPath::polyline(&pts, false)), KNIFE_GAP, Cap::Butt, Join::Round, 4.0); let mut plan: Vec<(NodeId, Vec)> = vec![]; - for id in target_paths(&st.doc, &st.selection, poly_bounds(&pts, 1.0)) { - let (pd, rule) = path_of(&st.doc, id)?; + for (id, pd, rule) in fill_targets(&st.doc, &st.selection, poly_bounds(&pts, 1.0))? { if !pd.is_closed() { continue; } @@ -281,8 +281,7 @@ pub(super) fn erase_region(s: &mut Session, p: &Value) -> Result { let squash = eraser_tip(angle, roundness).inverse(); let round: Vec = pts.iter().map(|q| squash * *q).collect(); let mut plan: Vec<(NodeId, Vec)> = vec![]; - for id in target_paths(&st.doc, &st.selection, poly_bounds(&pts, r)) { - let (pd, rule) = path_of(&st.doc, id)?; + for (id, pd, rule) in fill_targets(&st.doc, &st.selection, poly_bounds(&pts, r))? { if pd.is_closed() { if po::area(&po::boolean(&pd, rule, ®ion, FillRule::NonZero, BoolOp::Intersect), FillRule::NonZero) <= 1e-6 { continue; diff --git a/crates/engine/src/tests/draw2.rs b/crates/engine/src/tests/draw2.rs index c43362c0b..6a4537666 100644 --- a/crates/engine/src/tests/draw2.rs +++ b/crates/engine/src/tests/draw2.rs @@ -809,6 +809,52 @@ fn eraser_removes_small_shape_entirely() { assert!(paths(&s).is_empty()); } +/// #4495: the Eraser cuts a compound path as one shape: its hole stays empty, the erased area is +/// empty, and the rest stays filled, with either fill rule. A notch inside the ring keeps one +/// compound path; a drag through the whole ring leaves compound paths whose holes stay holes. +#[test] +fn eraser_keeps_the_holes_of_a_compound_path() { + for rule in ["nonZero", "evenOdd"] { + for (drag, erased) in [([[200, 110], [200, 120]], (200.0, 115.0)), ([[200, 80], [200, 320]], (200.0, 120.0))] { + let mut s = session(); + let outer = rect(&mut s, 100.0, 100.0, 200.0, 200.0); + let inner = rect(&mut s, 150.0, 150.0, 100.0, 100.0); + s.execute("select.set", &json!({"ids": [outer.0, inner.0]})).unwrap(); + s.execute("object.compoundPath.make", &json!({})).unwrap(); + s.execute("path.setFillRule", &json!({"rule": rule})).unwrap(); + s.execute("select.none", &json!({})).unwrap(); + s.execute("path.eraseRegion", &json!({"points": drag, "size": 10})).unwrap(); + // Whether any top-level shape fills `p`, a compound path with all its members at once. + let filled = |x: f64, y: f64| { + let doc = &s.doc().unwrap().doc; + let mut hit = false; + for layer in &doc.layers { + for n in layer.children().unwrap() { + let (subs, rule) = match &n.kind { + NodeKind::Path { path, rule, .. } => (path.subpaths.clone(), *rule), + NodeKind::Compound { children, rule } => { + (children.iter().flat_map(|c| c.path_data().unwrap().subpaths.clone()).collect(), *rule) + } + _ => continue, + }; + hit |= vectorcraft_geom::hit::fill_contains(&PathData::new(subs).to_bezpath(), rule, Point::new(x, y)); + } + } + hit + }; + assert!(!filled(170.0, 200.0), "{rule} {drag:?}: the hole stays empty"); + assert!(!filled(erased.0, erased.1), "{rule} {drag:?}: the erased area is empty"); + assert!(filled(120.0, 200.0) && filled(280.0, 200.0), "{rule} {drag:?}: the ring stays filled"); + if drag[1][1] == 120 { + let doc = &s.doc().unwrap().doc; + let tops: Vec<_> = doc.layers.iter().flat_map(|l| l.children().unwrap().iter().cloned()).collect(); + assert_eq!(tops.len(), 1, "{rule}: one object"); + assert!(matches!(tops[0].kind, NodeKind::Compound { .. }), "{rule}: still a compound path"); + } + } + } +} + /// The Eraser's tip is an ellipse `size` across its long axis, at `angle` counter-clockwise from /// horizontal, with a short axis `roundness` percent as long: a vertical drag across a square /// removes a band as wide as the tip is across, and a press on a horizontal line removes the