aleleba/vectorcraft
1
0
Fork 0
mirror of https://github.com/storytold/vectorcraft.git synced 2026-10-31 13:17:19 -06:00
Code Issues Packages Projects Releases Wiki Activity

Pattern Brush: decide straightness from the track, not from samples (#3926 review)

Browse Source 
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.
This commit is contained in:
Muhammad Saad
2026-10-29 13:00:45 +07:00
committed by bflatastic
parent 955660bee0
commit 18e916ddf5
2 changed files with 220 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/brush/src/track.rs
+100
View File
@@ -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<Vec2> = 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();
crates/brush/src/warp.rs
+120 -4
View File
@@ -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<f64> = 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<Affine> {
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<N
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.
if let Some(a) = affine_fit(&f, b, flat_tol.max(1e-9)) {
//
// Straightness is read off the track and the along map, not sampled. A bend anywhere in the
// span is two segment directions that differ, so this cannot miss one however short the bent
// part is; the five by five grid below could, since a wiggle narrower than its spacing passes
// between the samples (#3926 review). The grid now only checks the affine that structure has
// already said exists, so it can reject and never admit.
let (s_lo, s_hi) = along.span();
if along.is_linear()
&& t.is_straight(s_lo, s_hi)
&& let Some(a) = affine_fit(&f, b, flat_tol.max(1e-9))
{
return affine_node(art, a, cross.abs());
}
warp_node(art, &f, flat_tol, MAX_SEG / along.max_scale(), cross.abs())
@@ -806,3 +842,83 @@ pub(crate) fn pattern(pb: &PatternBrush, bp: &BezPath, weight: f64) -> Vec<Node>
}
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::<usize>(),
_ => 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");
}
}