From d1b9bc2b3dba6cb53ca337da39c55c3ec1ca1dce Mon Sep 17 00:00:00 2001 From: Muhammad Saad Date: Sat, 24 Oct 2026 10:51:11 +0400 Subject: [PATCH 1/3] Pattern Brush: keep the tile's curves on a straight run (#3926) Bending flattens every path it maps: warp_path runs kurbo::flatten and emits SubPath::polyline, so a circle tile is drawn as a polygon. That shows in the live stroke, not only after Expand Brush Strokes, and it happens even on a perfectly straight path where the tile needs nothing more than an affine. Bending is only non-linear where the track bends. On a straight run the arc-length map and the normal offset are both linear, so affine_fit derives the affine from three corners of the art and then checks it against a five by five grid, including those corners. Within the tolerance the flattening would have used, the art is transformed instead, which keeps its Beziers. Outside it, nothing changes: the old path runs and the result is what it was. Strokes are scaled by the same factor the bent path uses rather than by the affine's own, so curves are the only thing this changes. Corner tiles already took this route, which is why they kept their curves while the tiles between them did not. Two existing tests moved with it, both because they read anchor counts that only held while straight runs were flattened: pattern_places_corner_tiles told corner tiles from side tiles by "four anchors", true only because the side rectangles arrived as polylines. It now tells them apart by a kept handle, which is what actually distinguishes an ellipse from a rectangle, and additionally asserts the side tiles keep their own four anchors. pattern_output_is_capped_across_subpaths_not_only_per_run counted on sixteen tracks passing the #3594 anchor budget. A tile is about four times cheaper in anchors now, so the budget engages later; the test derives how many tracks it needs from what one costs. --- crates/brush/src/tests.rs | 96 +++++++++++++++++++++++++++++++-------- crates/brush/src/warp.rs | 60 ++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 18 deletions(-) diff --git a/crates/brush/src/tests.rs b/crates/brush/src/tests.rs index 069e1bf3c..aeec892db 100644 --- a/crates/brush/src/tests.rs +++ b/crates/brush/src/tests.rs @@ -273,15 +273,21 @@ fn pattern_tile_count_by_fit() { fn pattern_places_corner_tiles() { let sq = shapes::rectangle(Rect::new(0.0, 0.0, 100.0, 100.0)).to_bezpath(); let out = stroke_pieces(&Document::new(1.0, 1.0), &pattern_brush(PatternFit::Stretch, true), &sq, &stroke(1.0)); - let corners = out - .iter() - .filter( - |n| matches!(&n.kind, NodeKind::Path { path, .. } if path.subpaths[0].anchors.len() == 4 && n.geometric_bounds().unwrap().width() < 7.0), - ) - .count(); - assert_eq!(corners, 4); + // The corner tile is an ellipse and the side tile a rectangle, so a kept handle tells them + // apart. Anchor count no longer does: the side tiles keep their own four anchors now that a + // straight run is not flattened (#3926), where they used to arrive as many-anchor polylines. + let curved = |n: &Node| { + matches!(&n.kind, NodeKind::Path { path, .. } + if path.subpaths.iter().any(|s| s.anchors.iter().any(|a| a.h_in != a.p || a.h_out != a.p))) + }; + let corners = out.iter().filter(|n| curved(n)).count(); + assert_eq!(corners, 4, "the four corner ellipses, still curves"); // Sides lose half a corner tile at each end: 94 pt per side → 9 tiles each. assert_eq!(out.len() - corners, 36); + for n in out.iter().filter(|n| !curved(n)) { + let NodeKind::Path { path, .. } = &n.kind else { continue }; + assert_eq!(path.subpaths[0].anchors.len(), 4, "a rectangle tile on a straight run stays four anchors"); + } } /// The 10 × 4 pt side tile of [`pattern_brush`] with the given corner sources and no corner art. @@ -1141,19 +1147,22 @@ fn pattern_output_is_capped_across_subpaths_not_only_per_run() { let a1 = anchors(&one); assert!(a1 < 2_000_000, "one track stays under the total budget: {a1}"); - // Sixteen tracks would be sixteen times that without a total budget. - // Four tracks are 4 x 320,000 anchors, still inside the budget, so nothing is cut: the - // budget must not bite before the per-factor caps have actually multiplied past it. - let four = stroke_pieces(&Document::new(1.0, 1.0), &brush, &many_long_subpaths(4), &stroke(1.0)); - assert_eq!(four.len(), 4 * 20_000, "four tracks are under the budget and keep every piece"); - assert_eq!(anchors(&four), 4 * a1); + // Two tracks are well inside the budget, so nothing is cut: it must not bite before the + // per-factor caps have actually multiplied past it. + let two = stroke_pieces(&Document::new(1.0, 1.0), &brush, &many_long_subpaths(2), &stroke(1.0)); + assert_eq!(two.len(), 2 * 20_000, "two tracks are under the budget and keep every piece"); + assert_eq!(anchors(&two), 2 * a1); - // Sixteen would be 5,120,000 anchors. The budget is what stops it, not the per-run caps. - let many = stroke_pieces(&Document::new(1.0, 1.0), &brush, &many_long_subpaths(16), &stroke(1.0)); + // Enough tracks that the per-run caps alone would pass the budget. Derived from what one + // track costs rather than written down, because how many anchors a tile holds is not fixed: + // a tile on a straight run keeps its own anchors instead of being flattened (#3926), which + // moved this by a factor of four. + let tracks = 2_000_000 / a1 + 2; + let many = stroke_pieces(&Document::new(1.0, 1.0), &brush, &many_long_subpaths(tracks), &stroke(1.0)); let an = anchors(&many); - assert!(an <= 2_000_000, "sixteen subpaths must not exceed the budget: {an}"); - assert!(an > anchors(&four), "but more is drawn than four tracks' worth: {an}"); - assert!(many.len() < 16 * 20_000, "and fewer pieces than the per-run caps alone would allow: {}", many.len()); + assert!(an <= 2_000_000, "{tracks} subpaths must not exceed the budget: {an}"); + assert!(an > anchors(&two), "but more is drawn than two tracks' worth: {an}"); + assert!(many.len() < tracks * 20_000, "and fewer pieces than the per-run caps alone would allow: {}", many.len()); } /// The same for scatter, whose per-track guard is also 20,000 copies. @@ -1184,3 +1193,54 @@ fn an_ordinary_stroke_is_unaffected_by_the_budget() { assert_eq!(out.len(), 40, "ten tiles on each of the four sides, as before the budget"); assert!(anchors(&out) < 2_000_000); } + +/// Anchors under `n`, and how many of them keep a handle (so the segment is a curve). +fn anchors_and_curved(n: &Node) -> (usize, usize) { + let NodeKind::Path { path, .. } = &n.kind else { return (0, 0) }; + let all: Vec<_> = path.subpaths.iter().flat_map(|s| s.anchors.iter()).collect(); + (all.len(), all.iter().filter(|a| a.h_in != a.p || a.h_out != a.p).count()) +} + +/// #3926: a Pattern Brush on a straight path only moves, turns and scales its tile, so the +/// tile's curves survive. They used to be flattened into chords by the bending code, which is +/// visible in the stroke itself and leaves no handles after Expand Brush Strokes. +#[test] +fn a_pattern_tile_keeps_its_curves_on_a_straight_path() { + 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 (tile_anchors, tile_curved) = anchors_and_curved(&circle); + assert_eq!((tile_anchors, tile_curved), (4, 4), "the tile is a 4-anchor smooth circle"); + + let brush = Brush { + name: "p".into(), + kind: BrushKind::Pattern(PatternBrush { side: circle, fit: PatternFit::Stretch, colorization: Colorization::None, ..Default::default() }), + }; + let out = stroke_pieces(&Document::new(1.0, 1.0), &brush, &line(0.0, 0.0, 480.0, 0.0), &stroke(1.0)); + assert!(!out.is_empty(), "the brush drew something"); + for n in &out { + assert_eq!(anchors_and_curved(n), (4, 4), "each circle is still a 4-anchor curve, not a polygon"); + } + // The ink is in the same place: a circle tile still covers the path it was laid along. + let b = bounds(&out); + assert!(near(b.width(), 480.0, 1.0), "the tiles still span the path: {}", b.width()); + assert!(near(b.height(), 40.0, 1.0), "and still stand the tile's height: {}", b.height()); +} + +/// The other half of the same behaviour: where the path really does bend, the tile is still +/// subdivided, because an affine cannot reproduce a bend. The fit is checked before it is used, +/// so a bent run falls back to exactly what it did before. +#[test] +fn a_pattern_tile_on_a_bent_path_is_still_subdivided() { + 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 brush = Brush { + name: "p".into(), + kind: BrushKind::Pattern(PatternBrush { side: circle, fit: PatternFit::Stretch, colorization: Colorization::None, ..Default::default() }), + }; + // A circle to run along: every tile sits on a curving section. + let round = shapes::ellipse(Rect::new(0.0, 0.0, 400.0, 400.0)).to_bezpath(); + let out = stroke_pieces(&Document::new(1.0, 1.0), &brush, &round, &stroke(1.0)); + assert!(!out.is_empty()); + 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()); +} diff --git a/crates/brush/src/warp.rs b/crates/brush/src/warp.rs index 066797176..f5e150fb3 100644 --- a/crates/brush/src/warp.rs +++ b/crates/brush/src/warp.rs @@ -161,6 +161,58 @@ impl AlongMap { } } +/// The affine that reproduces `f` over `b` to within `tol`, if one does. +/// +/// Bending is only non-linear where the track bends. On a straight run the arc-length map and the +/// normal offset are both linear, so the art is just moved, turned and scaled, and flattening it +/// into chords throws its curves away for nothing (#3926). +/// +/// Derived from three corners and then *checked* against a grid: it is used only where it +/// reproduces the real map at least as closely as the flattening would have, so where it does not +/// hold the result is exactly what it was before. +fn affine_fit(f: &dyn Fn(Point) -> Point, b: Rect, tol: f64) -> Option { + let (w, h) = (b.width(), b.height()); + if !(w > 1e-9 && h > 1e-9 && tol.is_finite() && tol > 0.0) { + return None; + } + let origin = Point::new(b.x0, b.y0); + let q0 = f(origin); + // Images of the unit x and y directions. + let u = (f(Point::new(b.x1, b.y0)) - q0) / w; + let v = (f(Point::new(b.x0, b.y1)) - q0) / h; + if ![q0.x, q0.y, u.x, u.y, v.x, v.y].iter().all(|k| k.is_finite()) { + return None; + } + let a = Affine::translate(q0.to_vec2()) * Affine::new([u.x, u.y, v.x, v.y, 0.0, 0.0]) * Affine::translate(-origin.to_vec2()); + // Five by five including the corners the fit came from, so a map that merely agrees at those + // three is rejected. + for i in 0..=4 { + for j in 0..=4 { + let p = Point::new(b.x0 + w * i as f64 / 4.0, b.y0 + h * j as f64 / 4.0); + let (want, got) = (f(p), a * p); + if !(want.x.is_finite() && want.y.is_finite()) || want.distance(got) > tol { + return None; + } + } + } + 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(); + } +} + /// Bend `art` (already oriented, bounds `b`) along `t` using `along` and cross scale `cross`. fn bend(t: &Track, art: &Node, b: Rect, along: AlongMap, cross: f64) -> Option { let cy = b.center().y; @@ -168,6 +220,14 @@ fn bend(t: &Track, art: &Node, b: Rect, along: AlongMap, cross: f64) -> Option Date: Mon, 26 Oct 2026 10:53:27 +0400 Subject: [PATCH 2/3] 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 Date: Tue, 27 Oct 2026 19:48:58 +0400 Subject: [PATCH 3/3] Pattern Brush: decide straightness from the track, not from samples (#3926 review) Messhias is right that a sampled check cannot promise a run is straight, and my own write-up overstated what the grid did: I said the fit was "used only where it reproduces the real map", which five sample positions cannot establish. A bend narrower than the sample spacing passes between the samples, and the grid then approves an affine that is simply wrong. It is not a theoretical gap. A zigzag of amplitude 4 pt and half-period 20 pt, turning 11.3 degrees (under CORNER_DEG, so the vertices are not corners and their tangents blend flat), lands its y = 0 vertices exactly on the five sample positions. All 25 grid points then agree with the affine to the last digit while the art between them is thrown 4.000 pt off, against the 0.0613 pt tolerance the grid believed it was enforcing: 65 times over, measured, not estimated. `Track::is_straight(s0, s1)` reads the answer off the track instead. A bend in the span is two segment directions that differ, so no bend can hide in it whatever its size. It also checks the vertex tangents, because `frame` blends them across a segment unless the vertex is a corner, so a neighbour outside the span can bend the tangent inside it: directions alone would call `d, d, e` straight over the first two segments and be wrong. It fails closed on a closed track's seam, a non-finite bound and a track with no segments. `AlongMap::is_linear` covers the other half: Between Guides bends the arc-length map itself, which no amount of looking at the track would reveal. The grid stays, demoted to a second opinion that can reject and never admit, and `affine_fit`'s doc comment now says that rather than claiming to be the gate. Tests. `a_bend_shorter_than_the_sample_grid_is_still_a_bend` builds the zigzag above, asserts the sampled check accepts it, measures how far out the approved affine is, and then asserts the track-based gate refuses it; end to end through `bend` it asserts the aliased bend is subdivided while a genuinely straight run of the same span keeps the tile's four anchors. Restoring the sampled gate reds it with `the aliased bend is subdivided, not affine-mapped: 4 anchors`, so the old code really did lay a flat tile along a waving track. `straightness_is_read_off_the_track_not_sampled` covers the predicate directly: collinear segments and their extrapolation, a 0.2 pt detour, a span around it and a span before it, the blended-tangent case above, the closed seam and a NaN bound. Writing it caught a bug in my own guard: `f64::min` discards a NaN rather than propagating it, so `is_straight(NAN, 10.0)` was reporting straight until the finiteness check moved ahead of the min/max. --- crates/brush/src/track.rs | 100 ++++++++++++++++++++++++++++++ crates/brush/src/warp.rs | 124 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 220 insertions(+), 4 deletions(-) diff --git a/crates/brush/src/track.rs b/crates/brush/src/track.rs index 85ccbd990..7d8799186 100644 --- a/crates/brush/src/track.rs +++ b/crates/brush/src/track.rs @@ -29,6 +29,12 @@ fn unit(v: Vec2) -> Vec2 { if l < 1e-12 { Vec2::new(1.0, 0.0) } else { v / l } } +/// Whether two unit vectors point the same way, to within the slack a unit vector carries after +/// a few multiplications (a bend of even a thousandth of a degree fails this). +fn aligned(a: Vec2, b: Vec2) -> bool { + a.dot(b) > 1.0 - 1e-12 +} + /// Left-hand normal in y-down document space (for a path running +x it points +y). pub fn normal(t: Vec2) -> Vec2 { Vec2::new(-t.y, t.x) @@ -154,6 +160,59 @@ impl Track { let (p, t) = self.frame(s); p + normal(t) * d } + + /// Whether [`Self::frame`] holds one constant tangent over arc lengths `s0..s1`, so mapping + /// through it is an affine of the art and nothing more. + /// + /// Read off the track, not sampled. A bend in the span is two segments with different + /// directions, however short the bent part is, so this cannot miss one; a sampled check can, + /// because a wiggle narrower than the sample spacing passes between the samples (#3926). + /// + /// Fails closed: a span that needs reasoning this does not do (a closed track's seam, a + /// non-finite bound, a track with no segments) is reported as not straight, and the caller + /// bends it the long way. + pub fn is_straight(&self, s0: f64, s1: f64) -> bool { + // Checked before the min/max, which quietly drop a NaN rather than propagate it. + let n = self.segments(); + if !(s0.is_finite() && s1.is_finite()) || n == 0 { + return false; + } + let (lo, hi) = (s0.min(s1), s0.max(s1)); + let l = self.len(); + // A closed track wraps, so a span reaching past either end covers segments twice. + if self.closed && (lo < -1e-9 || hi > l + 1e-9) { + return false; + } + // Past an open track's ends `frame` extrapolates along the end segment's direction, so + // those segments are part of the span too. + let ends = [(!self.closed && lo < 0.0).then_some(0), (!self.closed && hi > l).then(|| n - 1)]; + let touched = (0..n).filter(|&i| { + let (a, b) = (self.cum.get(i), self.cum.get(i + 1)); + matches!((a, b), (Some(a), Some(b)) if *a < hi + 1e-9 && *b > lo - 1e-9) + }); + let mut dir: Option = None; + let mut any = false; + for i in touched.chain(ends.into_iter().flatten()) { + let (Some(&d), Some(&t0), Some(&t1), Some(&c0), Some(&c1)) = + (self.dirs.get(i), self.tangents.get(i), self.tangents.get(i + 1), self.corner.get(i), self.corner.get(i + 1)) + else { + return false; + }; + if dir.is_some_and(|first: Vec2| !aligned(first, d)) { + return false; + } + dir = Some(d); + any = true; + // `frame` blends the two vertex tangents across the segment unless the vertex is a + // corner, where it uses the segment's own direction. So a neighbour outside the span + // can still bend the tangent inside it, and checking the directions alone would miss + // that: segments d, d, e leave vertex 2's tangent at unit(d + e). + if !(c0 || aligned(t0, d)) || !(c1 || aligned(t1, d)) { + return false; + } + } + any + } } /// The most flattened elements of a path kept. @@ -253,6 +312,47 @@ pub fn seed_of(bp: &BezPath, salt: &str) -> u64 { mod tests { use super::*; + /// A straight run is straight; a bend in the span is not, however short it is. This is the + /// property a sampled check cannot offer, so it is worth pinning on its own (#3926 review). + #[test] + fn straightness_is_read_off_the_track_not_sampled() { + let line = |pts: &[(f64, f64)], closed: bool| { + let mut bp = BezPath::new(); + bp.move_to(pts[0]); + for p in &pts[1..] { + bp.line_to(*p); + } + if closed { + bp.close_path(); + } + tracks(&bp, 0.1).remove(0) + }; + + let straight = line(&[(0.0, 0.0), (100.0, 0.0), (200.0, 0.0)], false); + assert!(straight.is_straight(0.0, 200.0), "two collinear segments are one straight run"); + // Open tracks extrapolate along the end directions, which is still the same line. + assert!(straight.is_straight(-50.0, 250.0), "and so is the extrapolation past both ends"); + + // A detour of a fifth of a point, 0.1% of the run: far below any sampling grid this code + // would choose, and still a bend. + let wiggle = line(&[(0.0, 0.0), (99.9, 0.0), (100.0, 0.2), (100.1, 0.0), (200.0, 0.0)], false); + assert!(!wiggle.is_straight(0.0, 200.0), "a 0.2 pt detour is a bend"); + assert!(!wiggle.is_straight(99.0, 101.0), "and is caught when the span is only around it"); + assert!(wiggle.is_straight(0.0, 50.0), "while a span before it is straight"); + + // `frame` blends vertex tangents, so the segment before a bend is not straight all the way + // to the vertex: the tangent there is already turning. Pinning this because checking the + // segment directions alone would call that span straight and be wrong. + let kink = line(&[(0.0, 0.0), (100.0, 0.0), (160.0, 60.0)], false); + assert!(!kink.is_straight(0.0, 100.0), "the last segment's tangent is already turning at its far vertex"); + assert!(kink.is_straight(0.0, 50.0), "a span clear of the turning vertex is straight"); + + // Fails closed where it does not reason: a closed track's seam, and nonsense bounds. + let ring = line(&[(0.0, 0.0), (100.0, 0.0), (100.0, 100.0), (0.0, 100.0)], true); + assert!(!ring.is_straight(-10.0, 10.0), "a span across a closed track's seam is refused"); + assert!(ring.is_straight(10.0, 90.0), "a span inside one of its sides is straight"); + assert!(!straight.is_straight(f64::NAN, 10.0), "a non-finite bound is refused"); + } #[test] fn straight_track_frames() { let mut bp = BezPath::new(); diff --git a/crates/brush/src/warp.rs b/crates/brush/src/warp.rs index 088128e7f..95632ad49 100644 --- a/crates/brush/src/warp.rs +++ b/crates/brush/src/warp.rs @@ -155,6 +155,32 @@ impl AlongMap { let w = self.xs[i + 1] - self.xs[i]; if w <= 1e-12 { 1.0 } else { (self.ss[i + 1] - self.ss[i]) / w } } + /// Whether `at` is one linear function across the art, so it adds no bend of its own. + /// + /// [`AlongMap::linear`] always is: its outer two pieces have no width and the middle one + /// spans everything. Between Guides is not, once its three pieces really do differ, which is + /// the whole point of it. + fn is_linear(&self) -> bool { + let mut slope: Option = None; + for i in 0..3 { + // A piece with no width constrains nothing: no art x lands inside it. + if self.xs[i + 1] - self.xs[i] <= 1e-12 { + continue; + } + let s = self.slope(i); + if slope.is_some_and(|first: f64| (first - s).abs() > 1e-9 * first.abs().max(1.0)) { + return false; + } + slope = Some(s); + } + slope.is_some() + } + + /// The arc lengths the art spans, low to high. + fn span(&self) -> (f64, f64) { + (self.ss[0].min(self.ss[3]), self.ss[0].max(self.ss[3])) + } + /// Largest along-scale (arc length per art unit) of the pieces. fn max_scale(&self) -> f64 { (0..3).map(|i| self.slope(i).abs()).fold(1e-6, f64::max) @@ -167,9 +193,9 @@ impl AlongMap { /// normal offset are both linear, so the art is just moved, turned and scaled, and flattening it /// into chords throws its curves away for nothing (#3926). /// -/// Derived from three corners and then *checked* against a grid: it is used only where it -/// reproduces the real map at least as closely as the flattening would have, so where it does not -/// hold the result is exactly what it was before. +/// Derived from three corners and then checked against a grid. The check is a second opinion, not +/// the gate: `bend` only calls this once the track and the along map have been read and found +/// straight, because sampling can reject a bend and cannot promise there is none (#3926 review). fn affine_fit(f: &dyn Fn(Point) -> Point, b: Rect, tol: f64) -> Option { let (w, h) = (b.width(), b.height()); if !(w > 1e-9 && h > 1e-9 && tol.is_finite() && tol > 0.0) { @@ -242,7 +268,17 @@ fn bend(t: &Track, art: &Node, b: Rect, along: AlongMap, cross: f64) -> Option Vec } out.out } + +#[cfg(test)] +mod tests { + use super::*; + use crate::track::tracks; + + /// #3926 review: the five by five grid cannot promise a run is straight, only that it looks + /// straight where it was asked. A zigzag whose vertices land on the sample positions is + /// invisible to it: every sample sits on a y = 0 vertex whose blended tangent is flat, so all + /// 25 points agree with the affine exactly, while the art between them is thrown 4 pt off. + /// + /// This is why the gate is `Track::is_straight` and the grid is only a second opinion. + #[test] + fn a_bend_shorter_than_the_sample_grid_is_still_a_bend() { + // Half-period 20 pt, amplitude 4 pt. The 11.3 degree turn is under `CORNER_DEG`, so the + // vertices are not corners and their tangents blend flat, which is what hides them. + let (d, a) = (20.0, 4.0); + let mut bp = BezPath::new(); + bp.move_to((0.0, 0.0)); + for i in 1..=16 { + bp.line_to((d * i as f64, if i % 2 == 1 { a } else { 0.0 })); + } + let t = tracks(&bp, 0.01).remove(0); + assert_eq!(t.corner.iter().filter(|c| **c).count(), 0, "no vertex is a corner, so every tangent is blended"); + + // A span over four full periods, held clear of both ends so no sample hits the + // extrapolation branch: the five samples land on vertices 4, 6, 8, 10 and 12, all at y = 0. + let seglen = (d * d + a * a).sqrt(); + let (s0, s1) = (4.0 * seglen, 12.0 * seglen); + let b = Rect::new(0.0, -20.0, 40.0, 20.0); + let along = AlongMap::linear(b.width(), s0, s1); + let (cy, x0, cross) = (b.center().y, b.x0, 1.0); + let f = |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; + + // The grid is satisfied, and it should not be. + let fit = affine_fit(&f, b, flat_tol.max(1e-9)).expect("the sampled check accepts this bend"); + + // How wrong it is: an art x landing on a peak of the zigzag, which no sample reaches. + let peak_x = x0 + (5.0 * seglen - s0) / (s1 - s0) * b.width(); + let at_peak = Point::new(peak_x, cy); + let off = (f(at_peak) - fit * at_peak).hypot(); + assert!( + off > 3.0 && off > flat_tol * 20.0, + "the affine the grid approved is {off:.3} pt out at the peak, against a {flat_tol:.4} pt tolerance" + ); + + // Reading the track instead refuses it, and still admits the straight run it is made of. + assert!(!t.is_straight(s0, s1), "a bend between the samples is still a bend"); + let straight = tracks( + &{ + let mut bp = BezPath::new(); + bp.move_to((0.0, 0.0)); + bp.line_to((320.0, 0.0)); + bp + }, + 0.01, + ) + .remove(0); + assert!(straight.is_straight(s0, s1), "and the same span of an actually straight track is straight"); + + // End to end through `bend`: the aliased bend is subdivided, the straight run keeps its + // curves. This is the gate itself, not the two predicates it is made of. + let anchors = |n: &Node| match &n.kind { + NodeKind::Path { path, .. } => path.subpaths.iter().map(|s| s.anchors.len()).sum::(), + _ => 0, + }; + let circle = Node::path( + vectorcraft_doc::NodeId(0), + vectorcraft_geom::shapes::ellipse(b), + vectorcraft_doc::Appearance::basic(vectorcraft_color::Paint::solid(vectorcraft_color::Color::BLACK), vectorcraft_color::Paint::None, 0.0), + ); + assert_eq!(anchors(&circle), 4, "the art is a 4-anchor circle to begin with"); + let bent = bend(&t, &circle, b, along, cross).expect("the zigzag still draws something"); + assert!(anchors(&bent) > 8, "the aliased bend is subdivided, not affine-mapped: {} anchors", anchors(&bent)); + let moved = bend(&straight, &circle, b, along, cross).expect("the straight run still draws something"); + assert_eq!(anchors(&moved), 4, "the straight run keeps the tile's curves"); + } +}