From 535220cf34b7ffd6287d8b6d10308e92b953910e Mon Sep 17 00:00:00 2001 From: bflatastic Date: Mon, 5 Oct 2026 23:17:05 +0700 Subject: [PATCH] Fix #139: SVG import paints paint-order="stroke" text strokes under the letters The text import reads from the XML and never looked at paint-order, so character strokes always painted over the fill. A text whose characters all share one stroke painted first now gets that stroke as the object's own stroke below the Characters row (as the canvas, SVG and PDF export already draw it), with its gradient or pattern resolved in the document; characters with differing strokes keep them and the import warns. Co-Authored-By: Claude Opus 5.5 --- crates/svg/src/import.rs | 35 ++++++---- crates/svg/src/import/text.rs | 43 ++++++++++++- crates/svg/tests/text_paint_order.rs | 96 ++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 crates/svg/tests/text_paint_order.rs diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index 98a48019..1506d395 100644 --- a/crates/svg/src/import.rs +++ b/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 = (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 = (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 { diff --git a/crates/svg/src/import/text.rs b/crates/svg/src/import/text.rs index c056f83a..b6ae4bd2 100644 --- a/crates/svg/src/import/text.rs +++ b/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, /// 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, /// 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)>, /// 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) -> Option<(StrokeLayer, Option)> { + 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 } diff --git a/crates/svg/tests/text_paint_order.rs b/crates/svg/tests/text_paint_order.rs new file mode 100644 index 00000000..da003875 --- /dev/null +++ b/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##" + + Ab +"##; + +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##" + + Ab + "##; + 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##" + Ab + "##; + 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)); +}