Merge pull request #147 from storytold/fix/issue-139

Fix #139: SVG import paints paint-order="stroke" text strokes under the letters

crates/svg/src/import.rs

@@ -676,19 +676,18 @@ 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, non_scaling, .. } = self.slots.texts.get(i)?.clone();
+        let text::PendingText { name, mut obj, servers, paints, non_scaling, under, .. } = 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();
-        let resolved: Vec<Paint> = (0..paints)
-            .map(|k| match kids.get(main + 1 + k) {
-                Some(usvg::Node::Path(p)) => p.fill().map_or(Paint::None, |f| {
-                    let area = kurbo::Shape::bounding_box(&bezpath(p.data(), to_text));
-                    self.paint(f.paint(), to_text, Some(area))
-                }),
-                _ => Paint::None,
-            })
-            .collect();
+        let slot_paint = |im: &mut Self, k: usize, m: Affine| match kids.get(main + 1 + k) {
+            Some(usvg::Node::Path(p)) => p.fill().map_or(Paint::None, |f| {
+                let area = kurbo::Shape::bounding_box(&bezpath(p.data(), m));
+                im.paint(f.paint(), m, Some(area))
+            }),
+            _ => Paint::None,
+        };
+        let resolved: Vec<Paint> = (0..paints).map(|k| slot_paint(self, k, to_text)).collect();
         let server = |k: usize| resolved.get(k).cloned().unwrap_or(Paint::None);
         for (run, (fill, stroke)) in obj.runs.iter_mut().zip(servers) {
             if let Some(k) = fill {
@@ -703,7 +702,21 @@ impl Importer {
             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))))])
+        // A stroke under the characters is the object's own: it paints in the document.
+        let under = under.map(|(mut sl, k)| {
+            if let Some(k) = k {
+                sl.paint = slot_paint(self, k, ts);
+            }
+            let mut ap = Appearance { items: vec![AppearanceItem::Stroke(sl)], ..Default::default() };
+            ap.scale_strokes(if non_scaling { self.screen } else { obj.xf.determinant().abs().sqrt() });
+            ap.set_contents_at(1);
+            ap
+        });
+        let mut n = self.named(&name, NodeKind::Text(Box::new(obj)));
+        if let Some(ap) = under {
+            n.appearance = ap;
+        }
+        Some(vec![Arc::new(n)])
     }
 
     fn group_node(&mut self, g: &usvg::Group, acc: Affine) -> Option<Node> {

crates/svg/src/import/text.rs

@@ -26,7 +26,7 @@ use std::str::FromStr;
 
 use usvg::roxmltree;
 use vectorcraft_color::{Color, Paint};
-use vectorcraft_doc::{CharStyle, Dash, Justify, LineCap, LineJoin, TextKind, TextObject, TextRun};
+use vectorcraft_doc::{CharStyle, Dash, Justify, LineCap, LineJoin, StrokeLayer, TextKind, TextObject, TextRun};
 use vectorcraft_geom::kurbo::ParamCurveArclen;
 use vectorcraft_geom::{Affine, BezPath, PathData, Point, Rect};
 use vectorcraft_text::{FontDb, TextLayout};
@@ -72,6 +72,8 @@ struct ElemStyle {
     stroke: Option<String>,
     /// Extra space after each space character (`word-spacing`).
     word_spacing: f64,
+    /// `paint-order` paints the stroke before the fill.
+    stroke_first: bool,
 }
 
 /// One character of a text and the adjustments its position needs.
@@ -82,6 +84,8 @@ struct Cell {
     stroke: Option<usize>,
     /// Space added after the character, in points (becomes manual kerning).
     kern: f64,
+    /// Its stroke paints before its fill (`paint-order`).
+    stroke_first: bool,
 }
 
 impl Cell {
@@ -125,6 +129,9 @@ pub(super) struct PendingText {
     pub paints: usize,
     /// `vector-effect="non-scaling-stroke"`: the character strokes keep their width on screen.
     pub non_scaling: bool,
+    /// The character stroke `paint-order` puts under the characters (in text space; it becomes
+    /// the object's own stroke below the Characters row) and the index of its `url(#…)` paint.
+    pub under: Option<(StrokeLayer, Option<usize>)>,
     /// The placeholder rectangle in the element's user space.
     marker: Rect,
 }
@@ -412,7 +419,10 @@ impl Ctx {
             (st.fill, st.stroke, fill_url, stroke_url) = (Paint::None, Paint::None, None, None);
         }
         let word_spacing = css.prop(n, "word-spacing").and_then(|v| self.length(&v, size, size)).unwrap_or(0.0);
-        let e = Rc::new(ElemStyle { style: st, fill: fill_url, stroke: stroke_url, word_spacing });
+        let order = css.prop(n, "paint-order").and_then(|v| svgtypes::PaintOrder::from_str(&v).ok()).unwrap_or_default().order;
+        let at = |k| order.iter().position(|o| *o == k);
+        let stroke_first = at(svgtypes::PaintOrderKind::Stroke) < at(svgtypes::PaintOrderKind::Fill);
+        let e = Rc::new(ElemStyle { style: st, fill: fill_url, stroke: stroke_url, word_spacing, stroke_first });
         self.elems.borrow_mut().insert(n.id(), e.clone());
         e
     }
@@ -536,9 +546,11 @@ impl Ctx {
                     fill: paint_index(&e.fill),
                     stroke: paint_index(&e.stroke),
                     kern: if *ch == ' ' { e.word_spacing } else { 0.0 },
+                    stroke_first: e.stroke_first,
                 }
             })
             .collect();
+        let under = stroke_under(&mut cells, label, warnings);
 
         let anchor = match self.css.prop(cs.chars[0].1, "text-anchor").as_deref() {
             Some("middle") => 0.5,
@@ -605,10 +617,35 @@ impl Ctx {
         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();
         let non_scaling = vector_effect::is_non_scaling(&self.css, t);
-        Some((PendingText { name, obj, servers, paints: paints.len(), non_scaling, marker }, paints))
+        Some((PendingText { name, obj, servers, paints: paints.len(), non_scaling, under, marker }, paints))
     }
 }
 
+/// `paint-order` with the stroke first: the characters' stroke moves under all of them, as the
+/// object's own stroke (returned with its `url(#…)` paint index), and their own strokes go. Only
+/// when every character has that same stroke under a fill; otherwise a warning.
+fn stroke_under(cells: &mut [Cell], label: &str, warnings: &mut Vec<String>) -> Option<(StrokeLayer, Option<usize>)> {
+    let stroked = |c: &Cell| (c.stroke.is_some() || !c.style.stroke.is_none()) && c.style.stroke_width > 0.0;
+    let filled = |c: &Cell| c.fill.is_some() || !c.style.fill.is_none();
+    // Spaces paint nothing.
+    let mut ink = cells.iter().filter(|c| !c.ch.is_whitespace());
+    if !ink.clone().any(|c| c.stroke_first && stroked(c) && filled(c)) {
+        return None;
+    }
+    let key = |c: &Cell| (c.stroke_first && stroked(c)).then(|| (c.style.stroke_layer(), c.stroke));
+    let first = ink.next().and_then(key);
+    if first.is_none() || !ink.all(|c| key(c) == first) {
+        warnings.push(format!("paint-order on {label} ignored: its characters' strokes differ"));
+        return None;
+    }
+    let none = CharStyle::default().stroke_layer();
+    for c in cells.iter_mut() {
+        c.style.set_stroke_layer(&none);
+        c.stroke = None;
+    }
+    first
+}
+
 /// `v`, or 0 when far-off positions added up past f64's range.
 fn finite(v: f64) -> f64 {
     if v.is_finite() { v } else { 0.0 }

crates/svg/tests/text_paint_order.rs

@@ -0,0 +1,96 @@
+//! `paint-order="stroke"` on imported type: the characters' stroke becomes the object's own stroke
+//! below the Characters row, so it paints under the letters as on shapes.
+// 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_color::{Color, Paint};
+use vectorcraft_doc::{AppearanceItem, Document, Node, NodeKind, StrokeLayer, TextObject};
+use vectorcraft_svg::import_with_report;
+use vectorcraft_testkit::raster::render_artboard;
+
+const HALO: &str = r##"<svg xmlns="http://www.w3.org/2000/svg" width="300" height="80">
+  <path d="M10 10 h60 v30 h-60z" fill="#fc0" stroke="#000" stroke-width="10" paint-order="stroke"/>
+  <text x="100" y="45" font-size="36" font-weight="bold" fill="#fc0" stroke="#000" stroke-width="8" stroke-linejoin="round"
+        paint-order="stroke">Ab</text>
+</svg>"##;
+
+fn text(d: &Document) -> (Node, TextObject) {
+    let mut found = None;
+    d.walk(|n: &Node| {
+        if let NodeKind::Text(t) = &n.kind {
+            found = Some((n.clone(), (**t).clone()));
+        }
+    });
+    found.expect("a text")
+}
+
+fn under(n: &Node) -> &StrokeLayer {
+    assert_eq!((n.appearance.items.len(), n.appearance.contents_at()), (1, 1), "one stroke below the characters: {:?}", n.appearance);
+    match &n.appearance.items[0] {
+        AppearanceItem::Stroke(s) => s,
+        i => panic!("{i:?}"),
+    }
+}
+
+/// Pixels of the letters' yellow left showing (a stroke drawn over them hides them).
+fn yellow_letters(d: &Document) -> usize {
+    let img = render_artboard(d);
+    (100..190).flat_map(|x| (0..80).map(move |y| (x, y))).filter(|&(x, y)| img.over_white(x, y) == [0xff, 0xcc, 0]).count()
+}
+
+#[test]
+fn a_stroke_first_text_strokes_under_its_characters() {
+    let (d, warnings) = import_with_report(HALO).unwrap();
+    assert!(warnings.is_empty(), "{warnings:?}");
+    let (n, t) = text(&d);
+    let s = under(&n);
+    assert_eq!((s.paint.clone(), s.width, s.join), (Paint::solid(Color::BLACK), 8.0, vectorcraft_doc::LineJoin::Round));
+    assert!(t.runs.iter().all(|r| !r.style.has_stroke() && r.style.fill == Paint::solid(Color::rgb8(0xff, 0xcc, 0))), "{:?}", t.runs);
+
+    // The letters stay yellow inside their black halo.
+    assert!(yellow_letters(&d) > 150, "{}", yellow_letters(&d));
+}
+
+#[test]
+fn the_stroke_under_scales_with_the_text_and_keeps_its_gradient() {
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" width="300" height="200">
+      <linearGradient id="g" x1="0" y1="0" x2="1" y2="0"><stop offset="0" stop-color="#f00"/><stop offset="1" stop-color="#00f"/></linearGradient>
+      <g transform="scale(2)"><text x="10" y="40" font-size="20" fill="#fc0" stroke="url(#g)" stroke-width="3" paint-order="stroke markers">Ab</text></g>
+    </svg>"##;
+    let (d, warnings) = import_with_report(svg).unwrap();
+    assert!(warnings.is_empty(), "{warnings:?}");
+    let (n, t) = text(&d);
+    let s = under(&n);
+    assert_eq!(s.width, 6.0);
+    let Paint::Gradient(g) = &s.paint else { panic!("{:?}", s.paint) };
+    // The bounding-box gradient spans the text's ink in the document (x from about 20).
+    let geom = g.geom.unwrap();
+    assert!(geom.start.x > 15.0 && geom.end.x > geom.start.x + 40.0, "{geom:?}");
+    assert!(t.runs.iter().all(|r| !r.style.has_stroke()));
+
+    // Non-scaling: the stroke under the characters is as wide as on screen.
+    let svg = svg.replace("paint-order", r#"vector-effect="non-scaling-stroke" paint-order"#);
+    let (d, _) = import_with_report(&svg).unwrap();
+    assert_eq!(under(&text(&d).0).width, 3.0);
+}
+
+#[test]
+fn characters_with_different_strokes_keep_them_and_warn() {
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" width="300" height="80">
+      <text id="mixed" x="10" y="45" font-size="36" fill="#fc0" stroke="#000" stroke-width="8" paint-order="stroke">A<tspan stroke-width="2">b</tspan></text>
+    </svg>"##;
+    let (d, warnings) = import_with_report(svg).unwrap();
+    assert!(warnings.iter().any(|w| w.contains("paint-order on text 'mixed'")), "{warnings:?}");
+    let (n, t) = text(&d);
+    assert!(n.appearance.items.is_empty());
+    assert!(t.runs.iter().all(|r| r.style.has_stroke()));
+}
+
+#[test]
+fn normal_paint_order_leaves_the_character_strokes() {
+    let (d, warnings) = import_with_report(&HALO.replace(r#"paint-order="stroke">Ab"#, ">Ab")).unwrap();
+    assert!(warnings.is_empty(), "{warnings:?}");
+    let (n, t) = text(&d);
+    assert!(n.appearance.items.is_empty() && t.runs.iter().all(|r| r.style.has_stroke()));
+    assert!(yellow_letters(&d) < 20, "{}", yellow_letters(&d));
+}