From 446c3f071ffe5d06c18ed29d878458f8ffd98f5c Mon Sep 17 00:00:00 2001 From: Don Omondi <282319807+mockydevs@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:12:16 -0500 Subject: [PATCH] SVG import: a clipPath's children clip to their union, each under its own clip-rule (#5840) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A clipPath with several children was imported as one compound path under the first child's fill rule, so children wound in opposite directions cancelled where they overlapped, and an even-odd child after a non-zero one lost its hole. SVG 1.1 §14.3.5 defines the clip as the union of the children's silhouettes, each computed on its own. A clipPath with several children now comes in as a group of them as the clip group's clipping path, each child keeping its own rule; a group clips to the union of its members. One child imports as before. --- crates/svg/src/import/importer.rs | 36 +++++++++++++++++-------- crates/svg/tests/it/import_details.rs | 39 +++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/crates/svg/src/import/importer.rs b/crates/svg/src/import/importer.rs index 0a0372e3f..826fed099 100644 --- a/crates/svg/src/import/importer.rs +++ b/crates/svg/src/import/importer.rs @@ -7,6 +7,7 @@ use super::{ StrokeLayer, Vec2, fnv1a, fx, shapes, spread, text, vector_effect, }; use crate::SvgError; +use kurbo::BezPath; /// How many clip paths deep (``) an object's clip is read. const MAX_CLIP_NEST: usize = 16; @@ -446,19 +447,32 @@ impl Importer { } fn clip_node(&mut self, cp: &usvg::ClipPath, ts: Affine) -> Option { - let shapes = clip_shapes(cp, ts); - if shapes.is_empty() { + let mut shapes = clip_shapes(cp, ts); + let id = cp.id().to_string(); + if shapes.len() == 1 { + let (bp, r) = shapes.pop()?; + let kind = self.clip_shape(&bp, r, true)?; + return self.named(&id, kind); + } + // SVG 1.1 §14.3.5: the clip is the union of the children's silhouettes, each under its own + // clip-rule, so they don't cancel or share one rule: a group of them, which clips to that + // union (`Node::clip_shapes`). + let mut children = Vec::with_capacity(shapes.len()); + for (bp, r) in &shapes { + let kind = self.clip_shape(bp, *r, false)?; + children.push(Arc::new(Node::new(self.alloc_id()?, kind))); + } + if children.is_empty() { return None; } - let r = shapes[0].1; - let mut subs = Vec::new(); - for (bp, _) in &shapes { - subs.extend(PathData::from_bezpath(bp).subpaths); - } - let id = cp.id().to_string(); + self.named(&id, NodeKind::Group { children, clip: false }) + } + + /// One clip path child: a path, or a compound path of its subpaths when it has several. + fn clip_shape(&mut self, bp: &BezPath, r: FillRule, clipping: bool) -> Option { + let subs = PathData::from_bezpath(bp).subpaths; if subs.len() == 1 { - let path = PathData::new(subs); - return self.named(&id, NodeKind::Path { path, rule: r, live: None, clipping: true, guide: false }); + return Some(NodeKind::Path { path: PathData::new(subs), rule: r, live: None, clipping, guide: false }); } let mut children = Vec::with_capacity(subs.len()); for sp in subs { @@ -466,7 +480,7 @@ impl Importer { children .push(Arc::new(Node::new(nid, NodeKind::Path { path: PathData::single(sp), rule: r, live: None, clipping: false, guide: false }))); } - self.named(&id, NodeKind::Compound { children, rule: r }) + Some(NodeKind::Compound { children, rule: r }) } /// A usvg paint mapped by `m`, for art covering `area` (after `m`: where a reflected or diff --git a/crates/svg/tests/it/import_details.rs b/crates/svg/tests/it/import_details.rs index 8b3645860..967963a6f 100644 --- a/crates/svg/tests/it/import_details.rs +++ b/crates/svg/tests/it/import_details.rs @@ -213,6 +213,45 @@ fn nested_clips_intersect() { assert_ne!(mine.over_white(90, 60), [255, 255, 255]); } +/// SVG 1.1 §14.3.5: a clip path clips to the union of its children's silhouettes, each under its +/// own `clip-rule` (https://www.w3.org/TR/SVG11/masking.html#EstablishingANewClippingPath). +#[test] +fn clip_path_children_unite_under_their_own_rules() { + // Two squares wound in opposite directions overlap at 60–100: the overlap stays clipped in. + let opposite = r##" + "##; + // A small non-zero square first, then an even-odd ring: the ring's hole stays out. + let ring = r##" + + "##; + let page = |body: &str| format!(r#"{body}"#); + for body in [opposite, ring] { + let (d, w) = import_with_report(&page(body)).unwrap(); + assert!(w.is_empty(), "{w:?}"); + let mine = render_artboard(&d); + assert_similar(&mine, &resvg_render(&page(body), 200, 200), 24.0, 0.003); + // Written back out, the clip is still the union. + let out = vectorcraft_svg::export(&d, &Default::default()); + assert_similar(&render_artboard(&vectorcraft_svg::import(&out).unwrap()), &mine, 24.0, 0.003); + } + let (d, _) = import_with_report(&page(opposite)).unwrap(); + let mine = render_artboard(&d); + assert_eq!(mine.over_white(80, 80), [255, 0, 0], "the overlap is clipped in"); + assert_eq!(mine.over_white(120, 120), [255, 0, 0]); + assert_eq!(mine.over_white(130, 30), [255, 255, 255]); + let (d, _) = import_with_report(&page(ring)).unwrap(); + let mine = render_artboard(&d); + assert_eq!(mine.over_white(100, 80), [255, 255, 255], "the even-odd hole stays out"); + assert_eq!(mine.over_white(50, 80), [255, 0, 0]); + assert_eq!(mine.over_white(5, 5), [255, 0, 0]); + // Each child keeps its own rule in the clipping group, still named after the clip path. + let clip = &art(&d)[1]; + assert_eq!(clip.name.as_deref(), Some("c")); + let NodeKind::Group { children, clip: false } = &clip.kind else { panic!("{clip:?}") }; + assert!(matches!(children[0].kind, NodeKind::Path { rule: vectorcraft_geom::FillRule::NonZero, .. })); + assert!(matches!(children[1].kind, NodeKind::Compound { rule: vectorcraft_geom::FillRule::EvenOdd, .. })); +} + #[test] fn undisplayed_objects_import_hidden() { let body = r##"