Merge pull request #146 from storytold/fix/issue-131

Fix #131: SVG import keeps non-scaling strokes at their screen width

crates/svg/src/import.rs

@@ -25,6 +25,7 @@ mod files;
 mod fx;
 mod spread;
 mod text;
+mod vector_effect;
 
 pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<String>), SvgError> {
     let (svg, found) = prepass(svg, opts);
@@ -62,6 +63,8 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<S
         symbols: HashMap::new(),
         blends: fx::blends(&xml),
         files,
+        non_scaling: found.non_scaling,
+        screen: (kx * ky).sqrt(),
     };
     if im.files.nested_text() {
         im.warn("text inside an SVG image isn't imported".into());
@@ -191,6 +194,11 @@ struct Importer {
     blends: HashMap<String, String>,
     /// The files `<image>` elements link to.
     files: files::Files,
+    /// Ids of the shapes whose strokes don't scale ([`vector_effect`]).
+    non_scaling: HashSet<String>,
+    /// Points per screen pixel (the root's user unit before its `viewBox`): what a non-scaling
+    /// stroke's width is measured in.
+    screen: f64,
 }
 
 /// A `<symbol>` made a symbol: its name, its art with no object ids (what each `<use>` has to show
@@ -296,6 +304,8 @@ struct Found {
     uses: HashMap<String, String>,
     /// The files `<image>` elements link to.
     files: files::Files,
+    /// Ids of the shapes whose strokes don't scale ([`vector_effect`]).
+    non_scaling: HashSet<String>,
     warnings: Vec<String>,
 }
 
@@ -361,11 +371,12 @@ impl Edits {
 ///   ([`MADE_UP_ID`]…), recorded with its URL or symbol;
 /// * undisplayed objects (`display: none`, as Save writes hidden layers) are shown and recorded,
 ///   to come back hidden; one something links to (a `<use>` template) stays as it is;
+/// * shapes with a non-scaling stroke get an id when they have none, recorded ([`vector_effect`]);
 /// * linked files are read ([`files`]).
 fn prepass<'s>(svg: &'s str, opts: &ImportOptions) -> (Cow<'s, str>, Found) {
     let mut found = Found::default();
     // (`:` for prefixed elements such as `<svg:image>`.)
-    if !["<a", "display", "<use", ":use", "<image", ":image"].iter().any(|t| svg.contains(t)) {
+    if !["<a", "display", "<use", ":use", "<image", ":image", vector_effect::NON_SCALING].iter().any(|t| svg.contains(t)) {
         return (svg.into(), found);
     }
     let Ok(xml) = roxmltree::Document::parse_with_options(svg, roxmltree::ParsingOptions { allow_dtd: true, ..Default::default() }) else {
@@ -384,8 +395,15 @@ fn prepass<'s>(svg: &'s str, opts: &ImportOptions) -> (Cow<'s, str>, Found) {
             found.uses.insert(edits.id(svg, u), sym.to_string());
         }
     }
-    if svg.contains("display") {
-        hidden_objects(svg, &xml, &mut edits, &mut found.hidden);
+    let (display, non_scaling) = (svg.contains("display"), svg.contains(vector_effect::NON_SCALING));
+    if display || non_scaling {
+        let css = css::Styles::new(&xml);
+        if display {
+            hidden_objects(svg, &xml, &css, &mut edits, &mut found.hidden);
+        }
+        if non_scaling {
+            vector_effect::find(svg, &xml, &css, &mut edits, &mut found.non_scaling, &mut found.warnings);
+        }
     }
     found.files = files::Files::read(svg, &xml, opts, &mut edits, &mut found.warnings);
     (edits.apply(svg), found)
@@ -395,8 +413,7 @@ fn prepass<'s>(svg: &'s str, opts: &ImportOptions) -> (Cow<'s, str>, Found) {
 const OBJECTS: &[&str] = &["g", "a", "path", "rect", "circle", "ellipse", "line", "polyline", "polygon", "image", "text", "use"];
 
 /// Show the undisplayed objects outside `<defs>` (and the like) for usvg, recording their ids.
-fn hidden_objects(svg: &str, xml: &roxmltree::Document, edits: &mut Edits, ids: &mut HashSet<String>) {
-    let css = css::Styles::new(xml);
+fn hidden_objects(svg: &str, xml: &roxmltree::Document, css: &css::Styles, edits: &mut Edits, ids: &mut HashSet<String>) {
     let linked: HashSet<&str> = xml.descendants().filter_map(href).filter_map(|h| h.strip_prefix('#')).collect();
     let mut todo: Vec<XNode> = xml.root_element().children().filter(XNode::is_element).collect();
     while let Some(n) = todo.pop() {
@@ -659,7 +676,7 @@ impl Importer {
         if !matches!(kids.get(main), Some(usvg::Node::Path(p)) if p.is_visible()) {
             return Some(vec![]);
         }
-        let text::PendingText { name, mut obj, servers, paints, .. } = self.slots.texts.get(i)?.clone();
+        let text::PendingText { name, mut obj, servers, paints, non_scaling, .. } = self.slots.texts.get(i)?.clone();
         // usvg resolved the `url(#…)` paints (the paths after the main one) in the element's user
         // space; runs paint in text space.
         let to_text = obj.xf.inverse();
@@ -682,6 +699,10 @@ impl Importer {
             }
         }
         obj.xf = ts * obj.xf;
+        if non_scaling && !vector_effect::unscale_text(&mut obj, self.screen) {
+            let label = if name.is_empty() || made_up(&name) { "a text".to_string() } else { format!("text '{name}'") };
+            self.warn(format!("non-scaling stroke on {label} approximated: the text is stretched or skewed"));
+        }
         Some(vec![Arc::new(self.named(&name, NodeKind::Text(Box::new(obj))))])
     }
 
@@ -930,7 +951,9 @@ impl Importer {
         }
         let mut stroke = None;
         if let Some(s) = p.stroke() {
-            let scale = m.determinant().abs().sqrt();
+            // A non-scaling stroke is as wide on screen whatever `m` does: the path takes `m`, so a
+            // stroke of that width in the document is exact.
+            let scale = if self.non_scaling.contains(p.id()) { self.screen } else { m.determinant().abs().sqrt() };
             // The stroke reaches past the path (miters further: a generous margin).
             let reach = s.width().get() as f64 * scale * (s.miterlimit().get() as f64).max(1.0);
             let paint = self.paint(s.paint(), m, area.map(|a| a.inflate(reach, reach)));
@@ -1025,10 +1048,15 @@ impl Importer {
     /// An SVG shown as an image (a data URL or a linked file) → its art, `acc` mapping its size.
     /// Its ids are its own: they mean nothing in the SVG around it.
     fn svg_image(&mut self, tree: &usvg::Tree, acc: Affine) -> Option<Node> {
-        let outer =
-            (std::mem::take(&mut self.links), std::mem::take(&mut self.hidden), std::mem::take(&mut self.uses), std::mem::take(&mut self.labels));
+        let outer = (
+            std::mem::take(&mut self.links),
+            std::mem::take(&mut self.hidden),
+            std::mem::take(&mut self.uses),
+            std::mem::take(&mut self.labels),
+            std::mem::take(&mut self.non_scaling),
+        );
         let art = self.group_node(tree.root(), acc);
-        (self.links, self.hidden, self.uses, self.labels) = outer;
+        (self.links, self.hidden, self.uses, self.labels, self.non_scaling) = outer;
         art
     }
 }

crates/svg/src/import/text.rs

@@ -32,7 +32,7 @@ use vectorcraft_geom::{Affine, BezPath, PathData, Point, Rect};
 use vectorcraft_text::{FontDb, TextLayout};
 
 use super::css::{Styles, XNode};
-use super::{DEFAULT_FONT_SIZE, PT_PER_IN, href};
+use super::{DEFAULT_FONT_SIZE, PT_PER_IN, href, vector_effect};
 use crate::xml_escape;
 
 const SVG_NS: &str = "http://www.w3.org/2000/svg";
@@ -123,6 +123,8 @@ pub(super) struct PendingText {
     pub servers: Vec<(Option<usize>, Option<usize>)>,
     /// Number of `url(#…)` paints (the placeholder paths after the main one, in order).
     pub paints: usize,
+    /// `vector-effect="non-scaling-stroke"`: the character strokes keep their width on screen.
+    pub non_scaling: bool,
     /// The placeholder rectangle in the element's user space.
     marker: Rect,
 }
@@ -602,7 +604,8 @@ impl Ctx {
         let b = if paints.is_empty() { obj.bounds() } else { Some(obj.xf.transform_rect_bbox(measure(&obj).bounds)) }.unwrap_or_default();
         let marker = Rect::new(b.x0, b.y0, b.x1.max(b.x0 + 1.0), b.y1.max(b.y0 + 1.0));
         let name = t.attribute("id").unwrap_or("").to_string();
-        Some((PendingText { name, obj, servers, paints: paints.len(), marker }, paints))
+        let non_scaling = vector_effect::is_non_scaling(&self.css, t);
+        Some((PendingText { name, obj, servers, paints: paints.len(), non_scaling, marker }, paints))
     }
 }
 

crates/svg/src/import/vector_effect.rs

@@ -0,0 +1,79 @@
+//! `vector-effect="non-scaling-stroke"`: the stroke keeps its width (and dashes) in screen space
+//! whatever the transforms above it. usvg knows the property but drops it, so [`find`] records
+//! the elements that ask for it before usvg runs. Their transforms are baked into the path, so a
+//! document-space stroke of the screen width is exact; type and the art usvg drops ids from
+//! (patterns, markers, `<use>` copies) can't always keep it, and a warning says so.
+
+use std::collections::HashSet;
+
+use usvg::roxmltree;
+use vectorcraft_doc::TextObject;
+use vectorcraft_geom::Affine;
+
+use super::css::{Styles, XNode};
+use super::{Edits, href};
+
+/// The property value this module reads.
+pub(super) const NON_SCALING: &str = "non-scaling-stroke";
+
+/// The shapes the property applies to (`<text>` is read by the text import).
+const SHAPES: &[&str] = &["path", "rect", "circle", "ellipse", "line", "polyline", "polygon"];
+
+/// Does element `n` ask for a non-scaling stroke?
+pub(super) fn is_non_scaling(css: &Styles, n: XNode) -> bool {
+    css.own(n, "vector-effect").is_some_and(|v| v.trim() == NON_SCALING)
+}
+
+/// Record the ids of the shapes with a non-scaling stroke (made up for those without one) in
+/// `ids`, and warn about those shown where usvg drops ids: their strokes scale like others.
+pub(super) fn find(svg: &str, xml: &roxmltree::Document, css: &Styles, edits: &mut Edits, ids: &mut HashSet<String>, warnings: &mut Vec<String>) {
+    let reused: HashSet<&str> =
+        xml.descendants().filter(|n| n.tag_name().name() == "use").filter_map(href).filter_map(|h| h.trim().strip_prefix('#')).collect();
+    for n in xml.descendants().filter(|n| n.is_element() && SHAPES.contains(&n.tag_name().name()) && is_non_scaling(css, *n)) {
+        // usvg drops the ids of marker and `<use>` copies; a pattern's paint scales its art.
+        let placed = n.ancestors().any(|a| matches!(a.tag_name().name(), "pattern" | "marker" | "symbol"));
+        if placed || n.ancestors().any(|a| a.attribute("id").is_some_and(|id| reused.contains(id))) {
+            let label = n.attribute("id").map_or_else(|| format!("a {}", n.tag_name().name()), |id| format!("'{id}'"));
+            let w = format!("non-scaling stroke on {label} in a pattern, marker or reused element: it scales with the transform there");
+            if !warnings.contains(&w) {
+                warnings.push(w);
+            }
+        }
+        if !placed {
+            ids.insert(edits.id(svg, n));
+        }
+    }
+}
+
+/// Is `m` free of non-uniform scaling and skew (a uniform scale, rotation and reflection)?
+pub(super) fn conformal(m: Affine) -> bool {
+    let [a, b, c, d, _, _] = m.as_coeffs();
+    let tol = 1e-9 * (a.abs() + b.abs() + c.abs() + d.abs()).max(1e-12);
+    ((a - d).abs() <= tol && (b + c).abs() <= tol) || ((a + d).abs() <= tol && (b - c).abs() <= tol)
+}
+
+/// Give the character strokes of `t` (whose `xf` maps text space to the document) the widths and
+/// dashes they have in screen space, `screen` points per screen pixel. `false` when the text is
+/// stretched or skewed, so its strokes can only be approximated.
+pub(super) fn unscale_text(t: &mut TextObject, screen: f64) -> bool {
+    let k = screen / t.xf.determinant().abs().sqrt();
+    if !k.is_finite() || k <= 0.0 {
+        return false;
+    }
+    t.scale_char_strokes(k);
+    conformal(t.xf) || !t.runs.iter().any(|r| r.style.has_stroke())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn conformal_transforms_keep_circles_round() {
+        assert!(conformal(Affine::scale(3.0)));
+        assert!(conformal(Affine::rotate(0.7) * Affine::scale(2.0)));
+        assert!(conformal(Affine::scale_non_uniform(-2.0, 2.0)));
+        assert!(!conformal(Affine::scale_non_uniform(2.0, 1.0)));
+        assert!(!conformal(Affine::skew(0.5, 0.0)));
+    }
+}

crates/svg/tests/non_scaling_stroke.rs

@@ -0,0 +1,94 @@
+//! `vector-effect="non-scaling-stroke"` on import: the stroke keeps its screen width and dashes
+//! whatever the transforms above it, on shapes and type; a warning where it can't be kept.
+// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness).
+#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
+
+use vectorcraft_doc::{Document, Node, NodeKind, TextObject};
+use vectorcraft_svg::import_with_report;
+
+/// The stroke width and dash pattern of the object named `name`.
+fn stroke(d: &Document, name: &str) -> (f64, Option<Vec<f64>>) {
+    let mut found = None;
+    d.walk(|n: &Node| {
+        if n.name.as_deref() == Some(name) {
+            let s = n.appearance.stroke().expect("stroked");
+            found = Some((s.width, s.dash.as_ref().map(|d| d.pattern.clone())));
+        }
+    });
+    found.unwrap_or_else(|| panic!("no object '{name}'"))
+}
+
+fn text(d: &Document) -> TextObject {
+    let mut found = None;
+    d.walk(|n: &Node| {
+        if let NodeKind::Text(t) = &n.kind {
+            found = Some((**t).clone());
+        }
+    });
+    found.expect("a text")
+}
+
+fn close(a: f64, b: f64) -> bool {
+    (a - b).abs() < 1e-6
+}
+
+#[test]
+fn a_non_scaling_stroke_keeps_its_width_under_a_scale() {
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200" viewBox="0 0 200 200">
+      <g transform="scale(3)">
+        <path id="red" d="M10 15 H55" stroke="#e33" stroke-width="4"/>
+        <path id="green" d="M10 35 H55" stroke="#3a3" stroke-width="4" vector-effect="non-scaling-stroke"/>
+      </g>
+    </svg>"##;
+    let (d, warnings) = import_with_report(svg).unwrap();
+    assert!(close(stroke(&d, "red").0, 12.0));
+    assert!(close(stroke(&d, "green").0, 4.0), "{:?}", stroke(&d, "green"));
+    assert!(warnings.is_empty(), "{warnings:?}");
+}
+
+#[test]
+fn widths_and_dashes_are_in_screen_pixels_past_the_view_box() {
+    // The viewBox makes a user unit 2 px; a rule asks for the non-scaling stroke.
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" width="200" height="100" viewBox="0 0 100 50">
+      <style>.thin { vector-effect: non-scaling-stroke }</style>
+      <rect id="box" class="thin" x="10" y="10" width="50" height="20" fill="none" stroke="#000" stroke-width="2" stroke-dasharray="6 3"/>
+      <circle id="ring" cx="80" cy="25" r="10" fill="none" stroke="#000" stroke-width="2"/>
+    </svg>"##;
+    let (d, _) = import_with_report(svg).unwrap();
+    let (w, dash) = stroke(&d, "box");
+    assert!(close(w, 2.0) && dash == Some(vec![6.0, 3.0]), "{w} {dash:?}");
+    assert!(close(stroke(&d, "ring").0, 4.0));
+
+    // In a root sized in millimetres a CSS pixel is 0.75 pt.
+    let mm = svg.replace(r#"width="200" height="100""#, r#"width="200mm" height="100mm""#);
+    let (d, _) = import_with_report(&mm).unwrap();
+    assert!(close(stroke(&d, "box").0, 1.5), "a 2 px stroke is 1.5 pt: {:?}", stroke(&d, "box"));
+}
+
+#[test]
+fn a_reused_non_scaling_stroke_warns() {
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
+      <defs><path id="tick" d="M0 0 H20" stroke="#000" stroke-width="2" vector-effect="non-scaling-stroke"/></defs>
+      <use xlink:href="#tick" transform="scale(4)"/>
+    </svg>"##;
+    let (_, warnings) = import_with_report(svg).unwrap();
+    assert!(warnings.iter().any(|w| w.contains("non-scaling stroke on 'tick'")), "{warnings:?}");
+}
+
+#[test]
+fn type_keeps_its_non_scaling_stroke_width() {
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" width="300" height="200">
+      <text x="10" y="40" transform="scale(2)" font-size="20" stroke="#000" stroke-width="3" stroke-dasharray="4 2"
+            vector-effect="non-scaling-stroke">Ab</text>
+    </svg>"##;
+    let (d, warnings) = import_with_report(svg).unwrap();
+    let t = text(&d);
+    let st = &t.runs[0].style;
+    // Text space is scaled 2×: half the width there is the width on screen.
+    assert!(close(st.stroke_width * t.xf.determinant().sqrt(), 3.0), "{} {:?}", st.stroke_width, t.xf);
+    assert_eq!(st.stroke_dash.as_ref().map(|d| d.pattern.clone()), Some(vec![2.0, 1.0]));
+    assert!(warnings.is_empty(), "{warnings:?}");
+
+    let (_, warnings) = import_with_report(&svg.replace("scale(2)", "scale(2 1)")).unwrap();
+    assert!(warnings.iter().any(|w| w.contains("non-scaling stroke on a text approximated")), "{warnings:?}");
+}