From a713897d876da6a7bbef2c48c950f8e33aff77dc Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Thu, 22 Oct 2026 09:54:27 +0200 Subject: [PATCH] M6: SVG import fills open subpaths the way SVG does SVG fills every subpath as if it were closed; the import filled only subpaths that close themselves (a z, or an end within 1e-6 of the start). Logos exported from PDF round relative coordinates to three decimals, so their outlines end about 0.001 short of where they started and most of their letters came in empty. A filled path now has its open subpaths closed before it becomes a vector network: an end within 1/1000 of the path's size snaps onto the start, any other open subpath of two or more segments gets a closing segment. Stroke-only paths are unchanged. A filled and stroked path with a really open subpath becomes a fill layer (closed) under a stroke layer (open), so the stroke stays open as SVG draws it. --- crates/svg/src/import/mod.rs | 100 +++++++++++++++++++--- crates/svg/tests/corpus.rs | 18 ++++ crates/svg/tests/corpus/open_subpaths.svg | 18 ++++ 3 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 crates/svg/tests/corpus/open_subpaths.svg diff --git a/crates/svg/src/import/mod.rs b/crates/svg/src/import/mod.rs index fdae7156..521146fb 100644 --- a/crates/svg/src/import/mod.rs +++ b/crates/svg/src/import/mod.rs @@ -377,15 +377,38 @@ fn path(cx: &mut Ctx, p: &usvg::Path, parent: NodeId, base: Affine) { return; } path.apply_affine(Affine::translate((-bb.x0, -bb.y0))); - let mut network = VectorNetwork::from_path(&path); let rule = if fill.is_some_and(|f| f.rule() == usvg::FillRule::EvenOdd) { WindingRule::EvenOdd } else { WindingRule::NonZero }; - for r in &mut network.regions { - r.winding = rule; + let vector = |path: &BezPath, filled: bool| { + let mut network = VectorNetwork::from_path(path); + for r in &mut network.regions { + r.winding = rule; + } + if !filled { + network.regions.clear(); + } + NodeKind::Vector { network } + }; + let at = Affine::translate((bb.x0, bb.y0)); + // Fill closes every subpath; a stroke only those that close themselves. A filled + // and stroked path with a really open subpath becomes a fill layer and a stroke layer. + let tol = CLOSE_TOLERANCE * bb.width().max(bb.height()); + let (snapped, closed) = (closed_for_fill(&path, tol, false), closed_for_fill(&path, tol, true)); + if let (Some(_), Some(_)) = (fill, stroke) + && snapped != closed + { + let fill_only = + Node::new(cx.doc.alloc_id(), vector(&closed, true), format!("{} fill", layer_name(p.id(), "Vector")), bb.width(), bb.height()); + let stroke_only = Node::new(cx.doc.alloc_id(), vector(&snapped, false), layer_name(p.id(), "Vector"), bb.width(), bb.height()); + let mut layers = [paint_path(cx, p, fill_only, at, world, true, false), paint_path(cx, p, stroke_only, at, world, false, true)]; + if p.paint_order() == usvg::PaintOrder::StrokeAndFill { + layers.reverse(); + } + for n in layers { + cx.add(n, parent); + } + return; } - if fill.is_none() { - network.regions.clear(); - } - (NodeKind::Vector { network }, Affine::translate((bb.x0, bb.y0)), bb.width(), bb.height(), 0.0) + (vector(if fill.is_some() { &closed } else { &path }, fill.is_some()), at, bb.width(), bb.height(), 0.0) } }; let fallback = match kind { @@ -395,12 +418,18 @@ fn path(cx: &mut Ctx, p: &usvg::Path, parent: NodeId, base: Affine) { _ => "Vector", }; let mut n = Node::new(cx.doc.alloc_id(), kind, layer_name(p.id(), fallback), w, h); - n.transform = transform; n.corners.radii = [radius; 4]; - let target = Target { to_node: transform.inverse() * world, size: (w, h) }; - n.fills = fill.and_then(|f| paint::paint(cx, f.paint(), f.opacity().get(), target)).into_iter().collect(); + let n = paint_path(cx, p, n, transform, world, true, true); + cx.add(n, parent); +} + +/// Layer `n` of path `p` placed at `transform`, with the path's fill and / or stroke. +fn paint_path(cx: &mut Ctx, p: &usvg::Path, mut n: Node, transform: Affine, world: Affine, with_fill: bool, with_stroke: bool) -> Node { + n.transform = transform; + let target = Target { to_node: transform.inverse() * world, size: (n.width, n.height) }; + n.fills = p.fill().filter(|_| with_fill).and_then(|f| paint::paint(cx, f.paint(), f.opacity().get(), target)).into_iter().collect(); n.strokes.clear(); - if let Some(s) = stroke { + if let Some(s) = p.stroke().filter(|_| with_stroke) { n.strokes = paint::paint(cx, s.paint(), s.opacity().get(), target).into_iter().collect(); n.stroke = stroke_props(s, length_scale(world)); // A stroke painted under the fill shows only its outer half. @@ -409,7 +438,54 @@ fn path(cx: &mut Ctx, p: &usvg::Path, parent: NodeId, base: Affine) { n.stroke.weight /= 2.0; } } - cx.add(n, parent); + n +} + +/// How far (as a share of the path's larger side) a subpath may end from its start and still be +/// closed onto it: exporters that round coordinates leave closed outlines this far apart. +const CLOSE_TOLERANCE: f64 = 1e-3; + +/// `path` with the subpaths that end within `tol` of their start closed onto it and, with `all` +/// (SVG fills every subpath as if closed), every other subpath of two or more segments closed +/// with a straight segment. +fn closed_for_fill(path: &BezPath, tol: f64, all: bool) -> BezPath { + use figcraft_geom::kurbo::PathEl; + let mut out = BezPath::new(); + let mut sub: Vec = Vec::new(); + let flush = |sub: &mut Vec, out: &mut BezPath| { + let start = match sub.first() { + Some(PathEl::MoveTo(p)) => *p, + _ => { + sub.clear(); + return; + } + }; + let open = !matches!(sub.last(), Some(PathEl::ClosePath)); + let segments = sub.len() - 1; + if open && segments >= 2 { + let near = sub.last().and_then(|e| e.end_point()).is_some_and(|end| (end - start).hypot() <= tol); + if near && let Some(last) = sub.last_mut() { + match last { + PathEl::LineTo(p) | PathEl::QuadTo(_, p) | PathEl::CurveTo(_, _, p) => *p = start, + PathEl::MoveTo(_) | PathEl::ClosePath => {} + } + } + if near || all { + sub.push(PathEl::ClosePath); + } + } + for e in sub.drain(..) { + out.push(e); + } + }; + for e in path.elements() { + if matches!(e, PathEl::MoveTo(_)) { + flush(&mut sub, &mut out); + } + sub.push(*e); + } + flush(&mut sub, &mut out); + out } /// A recognised primitive placed by `world`: its kind, transform, size and corner radius. `None` diff --git a/crates/svg/tests/corpus.rs b/crates/svg/tests/corpus.rs index 35f6f9f4..ac0c38dd 100644 --- a/crates/svg/tests/corpus.rs +++ b/crates/svg/tests/corpus.rs @@ -243,6 +243,24 @@ fn groups_keep_their_ids_and_transforms_bake_into_shapes() { assert_eq!(layer(&d, f, "d").kind.type_name(), "VECTOR"); } +#[test] +fn fills_close_open_subpaths() { + let (d, f) = imported("open_subpaths"); + let loops = |n: &figcraft_doc::Node| match &n.kind { + NodeKind::Vector { network } => network.regions.iter().map(|r| r.loops.len()).sum::(), + _ => 0, + }; + // One filled layer: all four subpaths are fill loops, the 0.001 gap and the triangle included. + let filled = layer(&d, f, "filled"); + assert_eq!((filled.kind.type_name(), loops(filled), filled.strokes.len()), ("VECTOR", 4, 0)); + // Filled and stroked with an open triangle: a fill layer under the stroke, which stays open. + let (fill, stroke) = (layer(&d, f, "stroked fill"), layer(&d, f, "stroked")); + assert_eq!((loops(fill), fill.fills.len(), fill.strokes.len()), (2, 1, 0)); + assert_eq!((loops(stroke), stroke.fills.len(), stroke.strokes.len()), (0, 0, 1)); + let order: Vec<&str> = d.node(f).unwrap().children.iter().filter_map(|c| d.node(*c)).map(|n| n.name.as_str()).collect(); + assert_eq!(order, ["filled", "stroked fill", "stroked"], "the fill paints first, as SVG paints fill then stroke"); +} + #[test] fn basic_shapes_stay_rectangles_ellipses_and_lines() { let (d, f) = imported("primitives"); diff --git a/crates/svg/tests/corpus/open_subpaths.svg b/crates/svg/tests/corpus/open_subpaths.svg new file mode 100644 index 00000000..2d944942 --- /dev/null +++ b/crates/svg/tests/corpus/open_subpaths.svg @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + +