diff --git a/crates/svg/src/import/mod.rs b/crates/svg/src/import/mod.rs index 521146fb..3aed363a 100644 --- a/crates/svg/src/import/mod.rs +++ b/crates/svg/src/import/mod.rs @@ -446,8 +446,8 @@ fn paint_path(cx: &mut Ctx, p: &usvg::Path, mut n: Node, transform: Affine, worl 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. +/// (SVG fills every subpath as if closed), every other subpath that can enclose an area (two or +/// more segments, or one curve) 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(); @@ -462,7 +462,8 @@ fn closed_for_fill(path: &BezPath, tol: f64, all: bool) -> BezPath { }; let open = !matches!(sub.last(), Some(PathEl::ClosePath)); let segments = sub.len() - 1; - if open && segments >= 2 { + let curve = matches!(sub.last(), Some(PathEl::QuadTo(..) | PathEl::CurveTo(..))); + if open && (segments >= 2 || curve) { 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 { diff --git a/crates/svg/src/lib.rs b/crates/svg/src/lib.rs index 38abc14e..656307d9 100644 --- a/crates/svg/src/lib.rs +++ b/crates/svg/src/lib.rs @@ -155,13 +155,12 @@ fn blend_style(b: figcraft_color::BlendMode) -> String { /// `s` escaped for XML text and attribute values; characters XML 1.0 forbids (control characters /// other than tab, newline and carriage return, U+FFFE and U+FFFF) are left out. fn esc(s: &str) -> String { - s.chars() - .filter(|&c| !matches!(c, '\u{0}'..='\u{8}' | '\u{b}' | '\u{c}' | '\u{e}'..='\u{1f}' | '\u{fffe}' | '\u{ffff}')) - .collect::() - .replace('&', "&") - .replace('<', "<") - .replace('>', ">") - .replace('"', """) + s.chars().filter(|&c| xml_char(c)).collect::().replace('&', "&").replace('<', "<").replace('>', ">").replace('"', """) +} + +/// Whether XML 1.0 allows `c` (see [`esc`]). +fn xml_char(c: char) -> bool { + !matches!(c, '\u{0}'..='\u{8}' | '\u{b}' | '\u{c}' | '\u{e}'..='\u{1f}' | '\u{fffe}' | '\u{ffff}') } fn hex(c: Color) -> String { @@ -562,7 +561,9 @@ impl W<'_> { /// `name` as an `id` not used yet in this file (spaces as underscores; repeats get `_2`…). fn unique_id(&mut self, name: &str) -> String { - let base: String = name.chars().take(200).map(|c| if c.is_whitespace() || c.is_control() { '_' } else { c }).collect(); + // As written: what `esc` leaves out isn't part of it. + let base: String = + name.chars().take(200).map(|c| if c.is_whitespace() || c.is_control() { '_' } else { c }).filter(|&c| xml_char(c)).collect(); let base = if base.is_empty() { "layer".to_string() } else { base }; let next = self.next_suffix.entry(base.clone()).or_insert(1); let id = loop { diff --git a/crates/svg/src/tests.rs b/crates/svg/src/tests.rs index 3936c37c..dedeca3b 100644 --- a/crates/svg/src/tests.rs +++ b/crates/svg/src/tests.rs @@ -229,6 +229,9 @@ fn unique_id_is_linear_for_repeated_names() { assert_eq!(w.unique_id("a_3"), "a_3"); assert_eq!(w.unique_id("a"), "a_2"); assert_eq!(w.unique_id("a"), "a_4"); + // Unique as written, without what XML leaves out. + assert_eq!(w.unique_id("b\u{fffe}"), "b"); + assert_eq!(w.unique_id("b\u{ffff}"), "b_2"); let start = std::time::Instant::now(); for _ in 0..30_000 { w.unique_id("Rectangle"); @@ -247,3 +250,17 @@ fn layer_ids_never_clash_with_generated_ids() { let svg = export_svg(&d, &[f], &SvgOptions { include_ids: true, ..SvgOptions::default() }).unwrap(); assert_eq!(svg.matches("id=\"clip1\"").count(), 1, "{svg}"); } + +#[test] +fn a_filled_open_curve_is_closed_for_its_fill() { + // SVG fills an open subpath as if closed: one curve and the line back enclose an area. + let mut d = Document::new("t"); + let p = d.first_page().unwrap(); + let svg = r##""##; + import_svg(&mut d, svg.as_bytes(), p, 0.0, 0.0, "x").unwrap(); + let loops = |n: &figcraft_doc::Node| match &n.kind { + NodeKind::Vector { network } => network.regions.iter().map(|r| r.loops.len()).sum::(), + _ => 0, + }; + assert_eq!(d.nodes().map(loops).sum::(), 1); +}