From 955660bee0e4d9ba3634316c0fa70a053b944c16 Mon Sep 17 00:00:00 2001 From: Muhammad Saad Date: Mon, 26 Oct 2026 10:53:27 +0400 Subject: [PATCH] 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 | 42 ++++++++++++++++++++++++++++++- crates/brush/src/warp.rs | 53 ++++++++++++++++++++++++++------------- 2 files changed, 76 insertions(+), 19 deletions(-) diff --git a/crates/brush/src/tests.rs b/crates/brush/src/tests.rs index aeec892db..884dd27cf 100644 --- a/crates/brush/src/tests.rs +++ b/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"); + } + } +} diff --git a/crates/brush/src/warp.rs b/crates/brush/src/warp.rs index f5e150fb3..088128e7f 100644 --- a/crates/brush/src/warp.rs +++ b/crates/brush/src/warp.rs @@ -198,19 +198,40 @@ fn affine_fit(f: &dyn Fn(Point) -> Point, b: Rect, tol: f64) -> Option { 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 { + 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 { + 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> = 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