Merge pull request #6008 from dotCooCoo/fix/trace-logo-fit-cap

Image Trace: the logo fitter keeps no piece over its tolerance

crates/trace/src/logo/bezier.rs

@@ -4,9 +4,11 @@
 //! (Graphics Gems, 1990), written from the description there.
 
 use super::geom::{Cubic, bez};
-use kurbo::Vec2;
+use kurbo::{CubicBez, ParamCurveNearest, Vec2};
 use vectorcraft_geom::Point;
 
+/// From this depth a run still over the tolerance is cut in the middle, which bounds the recursion
+/// at this depth plus the logarithm of the run's length.
 const MAX_DEPTH: usize = 14;
 
 fn chord(p: &[Point]) -> Vec<f64> {
@@ -71,6 +73,13 @@ fn max_err(c: &Cubic, p: &[Point], u: &[f64]) -> (f64, usize) {
     best
 }
 
+/// The largest distance from a point of `p` to the nearest point of `c`, which `max_err`'s distance
+/// at a point's parameter can only overstate.
+fn max_distance(c: &Cubic, p: &[Point]) -> f64 {
+    let curve = CubicBez::new(c[0], c[1], c[2], c[3]);
+    p.iter().map(|&q| curve.nearest(q, 1e-6).distance_sq).fold(0.0, f64::max).sqrt()
+}
+
 fn rec(p: &[Point], t1: Vec2, t2: Vec2, tol: f64, depth: usize, out: &mut Vec<Cubic>) {
     let (Some(&a), Some(&b)) = (p.first(), p.last()) else { return };
     if p.len() == 2 {
@@ -96,11 +105,19 @@ fn rec(p: &[Point], t1: Vec2, t2: Vec2, tol: f64, depth: usize, out: &mut Vec<Cu
         }
     }
     let (e, split) = max_err(&c, p, &u);
-    if e < tol || depth >= MAX_DEPTH || p.len() < 6 {
+    let short = p.len() < 6;
+    let capped = depth >= MAX_DEPTH;
+    if e < tol || ((short || capped) && max_distance(&c, p) <= tol) {
         out.push(c);
         return;
     }
-    let s = split.clamp(2, p.len() - 3);
+    let s = if short {
+        split.clamp(1, p.len() - 2)
+    } else if capped {
+        p.len() / 2
+    } else {
+        split.clamp(2, p.len() - 3)
+    };
     let tc = match (p.get(s - 1), p.get(s + 1)) {
         (Some(&x), Some(&y)) if x.distance(y) > 1e-12 => (x - y) / x.distance(y),
         _ => t1,
@@ -182,6 +199,77 @@ mod tests {
         }
     }
 
+    /// The largest distance from a point of `pts` to the nearest point of the fit.
+    fn exact_dev(cs: &[Cubic], pts: &[Point]) -> f64 {
+        pts.iter().map(|&q| cs.iter().map(|c| max_distance(c, &[q])).fold(f64::MAX, f64::min)).fold(0.0, f64::max)
+    }
+
+    /// Fits `pts` and checks that the pieces run from its first point to its last, each within `tol`.
+    fn assert_within(pts: &[Point], tol: f64) -> Vec<Cubic> {
+        let n = pts.len();
+        let cs = fit_cubics(pts, tangent(pts, 0, 4, 1), tangent(pts, n - 1, 4, -1), tol);
+        assert!(exact_dev(&cs, pts) <= tol, "{} points, tolerance {tol}: deviation {}", n, exact_dev(&cs, pts));
+        assert_eq!((cs[0][0], cs[cs.len() - 1][3]), (pts[0], pts[n - 1]));
+        for w in cs.windows(2) {
+            assert!(w[0][3].distance(w[1][0]) < 1e-9);
+        }
+        cs
+    }
+
+    #[test]
+    fn a_short_run_over_the_tolerance_is_cut_until_it_is_within() {
+        let spike = [(0.0, 0.0), (0.12, 0.0), (0.24, 0.5), (0.36, 0.0), (0.48, 0.0)].map(|(x, y)| Point::new(x, y));
+        assert!(assert_within(&spike, 0.1).len() > 1);
+        let four = [(0.0, 0.0), (0.2, 0.6), (0.4, -0.6), (0.6, 0.0)].map(|(x, y)| Point::new(x, y));
+        assert!(assert_within(&four, 0.1).len() > 1);
+        let three = [(0.0, 0.0), (0.2, 0.6), (0.4, 0.0)].map(|(x, y)| Point::new(x, y));
+        assert!(assert_within(&three, 0.1).len() > 1);
+    }
+
+    /// A zigzag that grows towards its end: the largest error is always next to the end, so each
+    /// split takes only a few points off the run.
+    fn growing_zigzag(n: usize) -> Vec<Point> {
+        (0..n).map(|i| Point::new(i as f64 * 0.1, if i % 2 == 0 { 1.0 } else { -1.0 } * 0.02 * 1.02f64.powi(i as i32))).collect()
+    }
+
+    #[test]
+    fn a_run_that_reaches_the_depth_limit_is_still_cut_until_it_is_within() {
+        assert_within(&growing_zigzag(300), 0.05);
+    }
+
+    #[test]
+    fn a_very_long_run_of_small_splits_is_fitted_without_deep_recursion() {
+        let pts: Vec<Point> =
+            (0..200_000).map(|i| Point::new(i as f64 * 0.1, if i % 2 == 0 { 1.0 } else { -1.0 } * (1.0 + i as f64 * 1e-3))).collect();
+        let cs = fit_cubics(&pts, tangent(&pts, 0, 4, 1), tangent(&pts, pts.len() - 1, 4, -1), 0.05);
+        assert_eq!((cs[0][0], cs[cs.len() - 1][3]), (pts[0], pts[pts.len() - 1]));
+    }
+
+    #[test]
+    fn random_runs_stay_within_the_tolerance() {
+        let mut state = 0x9e37_79b9_7f4a_7c15u64;
+        let mut next = move || {
+            state ^= state << 13;
+            state ^= state >> 7;
+            state ^= state << 17;
+            state
+        };
+        for k in 0..300 {
+            let n = 3 + (next() % 200) as usize;
+            let tol = [0.04, 0.08, 0.14, 0.3, 0.8][(next() % 5) as usize];
+            let jitter = [0.02, 0.1, 0.5, 2.0][k % 4];
+            let (mut x, mut y, mut a) = (0.0f64, 0.0f64, 0.0f64);
+            let mut pts = Vec::with_capacity(n);
+            for _ in 0..n {
+                a += ((next() % 2001) as f64 / 1000.0 - 1.0) * [0.05, 0.4, 1.5][k % 3];
+                x += a.cos() * 0.12;
+                y += a.sin() * 0.12 + ((next() % 2001) as f64 / 1000.0 - 1.0) * jitter;
+                pts.push(Point::new(x, y));
+            }
+            assert_within(&pts, tol);
+        }
+    }
+
     #[test]
     fn degenerate_input_gives_nothing_or_a_line_not_a_panic() {
         let t = Vec2::new(1.0, 0.0);

crates/trace/src/logo/outline.rs

@@ -369,6 +369,36 @@ mod tests {
         assert!(segs.is_some_and(|s| !s.is_empty()));
     }
 
+    #[test]
+    fn a_short_run_between_corners_keeps_to_the_tolerance() {
+        let run = [(0.0, 0.0), (0.12, 0.0), (0.24, 0.6), (0.36, 0.0), (0.48, 0.0)].map(|(x, y)| Point::new(x, y));
+        let (st, sg) = (Style::default(), 0.5);
+        let mut segs = Vec::new();
+        fit_run(&run, sg, &st, &mut Stats::default(), &mut segs);
+        let samples = sample_cubics(&segs, 0.001);
+        for q in smooth_open(&run, sg) {
+            let d = samples.iter().map(|s| s.distance(q)).fold(f64::MAX, f64::min);
+            assert!(d <= st.tol + 1e-3, "{q:?} is {d} from the fit");
+        }
+    }
+
+    #[test]
+    fn a_plain_fit_keeps_a_ragged_outline_within_the_tolerance() {
+        let raw: Vec<Point> = (0..120)
+            .map(|i| {
+                let a = i as f64 / 120.0 * std::f64::consts::TAU;
+                let r = 12.0 + if i % 2 == 0 { 1.0 } else { -1.0 } * (0.2 + i as f64 * 0.02);
+                Point::new(25.0 + r * a.cos(), 25.0 + r * a.sin())
+            })
+            .collect();
+        let tol = Style::default().tol;
+        let samples = sample_cubics(&plain_fit(&raw, tol), 0.005);
+        for q in &raw {
+            let d = samples.iter().map(|s| s.distance(*q)).fold(f64::MAX, f64::min);
+            assert!(d <= tol + 1e-2, "{q:?} is {d} from the fit");
+        }
+    }
+
     #[test]
     fn degenerate_outlines_give_none_not_a_panic() {
         let mut st = Stats::default();

crates/trace/src/tests.rs

@@ -1557,7 +1557,7 @@ fn a_traced_pixel_that_straightens_to_no_area_is_left_out_of_fills_and_kept_as_a
         mode: Mode::Color,
         palette: Palette::Limited,
         colors: 6,
-        paths: 90.0,
+        paths: 80.0,
         corners: 60.0,
         noise: 1,
         method: Method::Abutting,

docs/target-app-parity.md

@@ -1032,6 +1032,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
   Pattern editing mode and opacity mask editing exclude each other, by a rule of VectorCraft itself: in pattern editing mode a click or Alt-click on the mask thumbnail does not edit the mask, and Make Mask is off for one object; while a mask is edited, Object › Pattern › Make and Edit Pattern are off.
   Disable, Enable, Unlink and Link Opacity Mask and Opacity Mask Options (`transparency.setOpacityMask`) record no undo step when every mask they act on already has the flags they set.
 - **Image Trace, thin closed outlines:** generated `fit::snap_area_tests` verify nonzero interiors after straightening, opposite closed-boundary winding and Noise controls; see [quality notes](image-trace-quality.md#thin-closed-outlines-with-snap-curves-to-lines). Scores and hours unchanged; open-piece fitting is unchanged.
+- **Image Trace, curve fitting tolerance:** the outline fitter keeps cutting a run while a point stays farther than the tolerance from its curve, so a run of fewer than six points or one cut 14 times no longer keeps a piece over the tolerance. Pieces already within the tolerance do not change, and noisy art can get more anchors. Tests: `logo::bezier::tests`, `logo::outline::tests`. Scores and hours unchanged.
 - **Image Trace, Document Library color groups:** with Document Swatches, the panel's Colors menu and `"colorGroup"` trace with one color group's colors instead of every swatch, as the public panel documentation describes Document Library; code-drawn tests in `crates/engine` and the panel.
 - **Image Trace, Document Library colors:** with Document Swatches, traced fills and strokes use the displayed colors of process, global, spot and tint swatches without links, so editing a swatch leaves the trace unchanged; swatch-library colors are also unlinked; code-drawn tests in `crates/engine`.
 - **Image Trace, flattened slivers:** in a Color, Grayscale or Black and White trace with Fills, a traced region whose outline straightens onto one line (a single pixel, or a one-pixel row along an edge, its pieces shared with the regions beside it) is left out instead of becoming a selectable path with no area, which paints nothing. Snap Curves To Lines straightens more outlines, so it leaves out more of these. Without Fills, the region's 1 px stroke draws it, and it is kept. Code-drawn tests in `crates/trace`.