Pattern Brush: make the straight-run route match warp_node exactly (#3926 review)

FredApps noted a real divergence: the affine route kept whatever the tile held
(it only required bounds) while warp_node drops every node kind but paths,
groups, layers and compound paths, and it did not clear `live` on paths. For a
tile of paths and groups the result was the same, so nothing was visibly wrong,
but "this changes the curves and nothing else" was a wider claim than the code
made: a symbol instance arrived untouched on a straight run and vanished on a
bend.

affine_node now mirrors warp_node's walk through `prepared` and applies one
transform at the end: the same kinds are kept and the rest dropped, `live` is
cleared on every path, an empty path or a group left with no children is
dropped, and strokes are scaled by `width_scale` rather than by the transform's
own factor. The two routes now differ in the tile's curves and in nothing else,
which is what the change was always meant to be.

Test: a_straight_run_keeps_the_same_node_kinds_as_a_bend puts a symbol instance
beside a path in one tile and asserts both routes return the same kinds. On the
old code it reds with `left: ["group", "path", "symbol"] right: ["group",
"path"]`.

crates/brush/src/tests.rs

@@ -1,6 +1,6 @@
 use vectorcraft_color::{Color, Paint};
 use vectorcraft_doc::{Appearance, Document, Node, NodeId, NodeKind, StrokeLayer};
-use vectorcraft_geom::{BezPath, Point, Rect, shapes};
+use vectorcraft_geom::{Affine, BezPath, Point, Rect, shapes};
 
 use super::*;
 
@@ -1244,3 +1244,43 @@ fn a_pattern_tile_on_a_bent_path_is_still_subdivided() {
     let bent = out.iter().filter(|n| anchors_and_curved(n).0 > 8).count();
     assert!(bent > out.len() / 2, "tiles on a bend are still densified: {bent} of {}", out.len());
 }
+
+/// The kind of every node in a piece of brushed art, in paint order.
+fn kinds(n: &Node) -> Vec<&'static str> {
+    let mut out = vec![match &n.kind {
+        NodeKind::Path { .. } => "path",
+        NodeKind::Group { .. } => "group",
+        NodeKind::SymbolInstance { .. } => "symbol",
+        _ => "other",
+    }];
+    for c in n.children().into_iter().flatten() {
+        out.extend(kinds(c));
+    }
+    out
+}
+
+/// The straight-run route differs from the bending route in the tile's curves and in nothing else,
+/// which includes the node kinds it keeps. A tile holding a kind neither route can map (here a
+/// symbol instance, whose art lives in the symbol and not in the node) loses it on a straight run
+/// exactly as it does on a bend: an affine is not a way for unmappable art to arrive untouched.
+#[test]
+fn a_straight_run_keeps_the_same_node_kinds_as_a_bend() {
+    let circle =
+        Node::path(NodeId(0), shapes::ellipse(Rect::new(0.0, -20.0, 40.0, 20.0)), Appearance::basic(Paint::solid(Color::BLACK), Paint::None, 0.0));
+    let sym = Node::new(NodeId(1), NodeKind::SymbolInstance { symbol: "s".into(), xf: Affine::IDENTITY });
+    let tile = Node::group(NodeId(2), vec![Arc::new(circle), Arc::new(sym)]);
+    let brush = Brush {
+        name: "p".into(),
+        kind: BrushKind::Pattern(PatternBrush { side: tile, fit: PatternFit::Stretch, colorization: Colorization::None, ..Default::default() }),
+    };
+    let doc = Document::new(1.0, 1.0);
+    let straight = stroke_pieces(&doc, &brush, &line(0.0, 0.0, 480.0, 0.0), &stroke(1.0));
+    let round = shapes::ellipse(Rect::new(0.0, 0.0, 400.0, 400.0)).to_bezpath();
+    let bent = stroke_pieces(&doc, &brush, &round, &stroke(1.0));
+    assert!(!straight.is_empty() && !bent.is_empty(), "both paths drew something");
+    for (what, out) in [("a straight run", &straight), ("a bend", &bent)] {
+        for n in out {
+            assert_eq!(kinds(n), ["group", "path"], "{what} keeps the path and drops the symbol instance");
+        }
+    }
+}

crates/brush/src/warp.rs

@@ -198,19 +198,40 @@ fn affine_fit(f: &dyn Fn(Point) -> Point, b: Rect, tol: f64) -> Option<Affine> {
     Some(a)
 }
 
-/// Scale every stroke under `n`, as [`warp_node`] does on the way down.
-fn scale_strokes_deep(n: &mut Node, s: f64) {
-    n.appearance.scale_strokes(s);
-    if let NodeKind::Group { children, .. } | NodeKind::Layer { children, .. } | NodeKind::Compound { children, .. } = &mut n.kind {
-        *children = children
-            .iter()
-            .map(|c| {
-                let mut c = (**c).clone();
-                scale_strokes_deep(&mut c, s);
-                Arc::new(c)
-            })
-            .collect();
+/// [`warp_node`] for a map that is affine: the art is transformed rather than flattened, so its
+/// curves survive.
+///
+/// Everything else matches [`warp_node`] deliberately, so curves are the only difference between
+/// the two routes: the same node kinds are kept and the rest dropped, `live` is cleared on every
+/// path, an empty path or a group left with no children is dropped, and strokes are scaled by
+/// `width_scale` at every level rather than by the transform's own factor.
+fn affine_node(n: &Node, a: Affine, width_scale: f64) -> Option<Node> {
+    let mut out = prepared(n, width_scale)?;
+    out.transform(a, false);
+    Some(out)
+}
+
+/// The part of [`affine_node`] that mirrors [`warp_node`]'s walk, before the single transform.
+fn prepared(n: &Node, width_scale: f64) -> Option<Node> {
+    let mut out = n.clone();
+    out.appearance.scale_strokes(width_scale);
+    match &mut out.kind {
+        NodeKind::Path { path, live, .. } => {
+            *live = None;
+            if path.is_empty() {
+                return None;
+            }
+        }
+        NodeKind::Group { children, .. } | NodeKind::Layer { children, .. } | NodeKind::Compound { children, .. } => {
+            let kids: Vec<Arc<Node>> = children.iter().filter_map(|c| prepared(c, width_scale)).map(Arc::new).collect();
+            if kids.is_empty() {
+                return None;
+            }
+            *children = kids;
+        }
+        _ => return None,
     }
+    Some(out)
 }
 
 /// Bend `art` (already oriented, bounds `b`) along `t` using `along` and cross scale `cross`.
@@ -220,13 +241,9 @@ fn bend(t: &Track, art: &Node, b: Rect, along: AlongMap, cross: f64) -> Option<N
     let f = move |p: Point| t.map(along.at(p.x - x0), (p.y - cy) * cross);
     let scale = along.max_scale().max(cross.abs()).max(1e-6);
     let flat_tol = tolerance(b.height().max(b.width()) * scale) / scale;
-    // A straight run needs no bending: move the art and keep its curves. Strokes are scaled the
-    // way the bent path scales them, so this changes the curves and nothing else.
+    // A straight run needs no bending: move the art and keep its curves.
     if let Some(a) = affine_fit(&f, b, flat_tol.max(1e-9)) {
-        let mut out = art.clone();
-        out.transform(a, false);
-        scale_strokes_deep(&mut out, cross.abs());
-        return out.geometric_bounds().is_some().then_some(out);
+        return affine_node(art, a, cross.abs());
     }
     warp_node(art, &f, flat_tol, MAX_SEG / along.max_scale(), cross.abs())
 }