Merge pull request #3882 from dylanmtaylor/fix/audit-geom-pathops

Security: geometry, path edit and tool work bounds

crates/geom/src/arc.rs

@@ -22,7 +22,7 @@ impl ArcPath {
         let mut segs = Vec::new();
         let mut cum = 0.0;
         for s in path.to_bezpath().segments() {
-            let l = s.arclen(ACCURACY);
+            let l = crate::path::measured_arclen(&s, ACCURACY);
             if l.is_finite() && l > 1e-9 {
                 segs.push((s, cum, l));
                 cum += l;
@@ -51,7 +51,8 @@ impl ArcPath {
         let s = if self.closed && self.len > 0.0 { s.rem_euclid(self.len) } else { s.clamp(0.0, self.len) };
         let i = self.segs.partition_point(|(_, c, _)| *c <= s).saturating_sub(1);
         let Some(&(seg, c, l)) = self.segs.get(i) else { return (Point::ZERO, Vec2::new(1.0, 0.0)) };
-        let t = seg.inv_arclen((s - c).min(l), ACCURACY).clamp(0.0, 1.0);
+        // (The solver's precision is relative to the segment's length: a long one needs a looser one.)
+        let t = seg.inv_arclen((s - c).min(l), ACCURACY.max(l * 1e-9)).clamp(0.0, 1.0);
         let p = seg.eval(t);
         let (t0, t1) = ((t - 1e-4).max(0.0), (t + 1e-4).min(1.0));
         let d = seg.eval(t1) - seg.eval(t0);

crates/geom/src/corners.rs

@@ -39,7 +39,11 @@ impl Side {
 
     /// The side's length.
     fn length(&self) -> f64 {
-        if self.straight() { self.curve.p0.distance(self.curve.p3) } else { self.curve.arclen(ARCLEN_ACCURACY) }
+        if self.straight() {
+            self.curve.p0.distance(self.curve.p3)
+        } else {
+            crate::path::measured_arclen(&kurbo::PathSeg::Cubic(self.curve), ARCLEN_ACCURACY)
+        }
     }
 
     /// The parameter `s` along the side from the corner, the point there and the direction the

crates/geom/src/path.rs

@@ -561,10 +561,27 @@ impl PathData {
     }
     /// Total length of all segments.
     pub fn length(&self) -> f64 {
-        self.to_bezpath().segments().map(|s| kurbo::ParamCurveArclen::arclen(&s, 1e-6)).sum()
+        self.to_bezpath().segments().map(|s| measured_arclen(&s, 1e-6)).sum()
     }
 }
 
+/// The largest x or y, in size, of a segment's control points for which its length is measured.
+/// Document coordinates are far inside it; past it (or not finite) the integrator's arithmetic
+/// overflows and it subdivides without end.
+const MEASURE_LIMIT: f64 = 1e9;
+
+/// The length of `seg` to `accuracy`; 0 for a segment whose control points are not finite or past
+/// [`MEASURE_LIMIT`].
+pub(crate) fn measured_arclen(seg: &PathSeg, accuracy: f64) -> f64 {
+    let ok = |p: Point| p.x.is_finite() && p.y.is_finite() && p.x.abs() <= MEASURE_LIMIT && p.y.abs() <= MEASURE_LIMIT;
+    let fits = match seg {
+        PathSeg::Line(l) => ok(l.p0) && ok(l.p1),
+        PathSeg::Quad(q) => ok(q.p0) && ok(q.p1) && ok(q.p2),
+        PathSeg::Cubic(c) => ok(c.p0) && ok(c.p1) && ok(c.p2) && ok(c.p3),
+    };
+    if fits { kurbo::ParamCurveArclen::arclen(seg, accuracy) } else { 0.0 }
+}
+
 /// The largest x or y, in size, of a segment's control points for which [`segment_bounds`]
 /// measures the segment by its extrema. kurbo adds up to eight coordinates together when it finds a
 /// segment's extrema and evaluates the segment there, and a sum of eight coordinates of this size or

crates/pathops/src/edit.rs

@@ -254,7 +254,17 @@ fn rdp(pts: &[Point], tol: f64) -> Vec<Point> {
     keep[0] = true;
     keep[pts.len() - 1] = true;
     let mut stack = vec![(0, pts.len() - 1)];
+    // Distances measured at most: lopsided splits (a shallow zigzag) make it quadratic. Past it,
+    // the points of the parts left are kept as they are.
+    let mut budget = pts.len().saturating_mul(64).saturating_add(1 << 20);
     while let Some((a, b)) = stack.pop() {
+        budget = match budget.checked_sub(b - a) {
+            Some(left) => left,
+            None => {
+                keep[a..=b].fill(true);
+                continue;
+            }
+        };
         let (pa, pb) = (pts[a], pts[b]);
         let chord = pb - pa;
         let len = chord.hypot();
@@ -318,6 +328,23 @@ pub fn remove_redundant_points(path: &PathData, tolerance: f64) -> PathData {
         // Drop zero-length segments.
         segs.retain(|s| !(s.c.p0.distance(s.c.p3) < 1e-9 && s.c.p1.distance(s.c.p0) < 1e-9 && s.c.p2.distance(s.c.p0) < 1e-9));
         let min_segs = if sp.closed { 2 } else { 1 };
+        // Joints are merged along the path in one pass (a merged segment is tried with the one
+        // before it, then the next); the search below then only finds what a closed path's wrap
+        // leaves. (Merging at the first joint of a dense line one at a time shifts the rest each time.)
+        let total = segs.len();
+        let mut merged = 0;
+        let mut done: Vec<Seg> = Vec::with_capacity(total);
+        for s in segs.drain(..) {
+            done.push(s);
+            while done.len() >= 2 && total - merged > min_segs {
+                let n = done.len();
+                let Some(m) = merge_pair(&done[n - 2], &done[n - 1], tol) else { break };
+                done.truncate(n - 1);
+                done[n - 2] = m;
+                merged += 1;
+            }
+        }
+        segs = done;
         let mut changed = true;
         while changed && segs.len() > min_segs {
             changed = false;
@@ -521,9 +548,12 @@ pub fn join(paths: &[PathData], tolerance: f64) -> PathData {
         return PathData::new(closed);
     }
     while open.len() > 1 {
-        // Find the closest pair of endpoints on different subpaths.
+        // Find the closest pair of endpoints on different subpaths. Past this many subpaths every
+        // pair at every merge is too much (cubic): the first one is joined to its nearest.
+        const SEARCH_ALL: usize = 200;
+        let first_only = open.len() > SEARCH_ALL;
         let mut best = (f64::INFINITY, 0, 0, false, false);
-        for i in 0..open.len() {
+        for i in 0..if first_only { 1 } else { open.len() } {
             for j in (i + 1)..open.len() {
                 for ei in [false, true] {
                     for ej in [false, true] {

crates/tools/src/distort/liquify.rs

@@ -388,7 +388,8 @@ fn subdivide(sp: &mut SubPath, prm: &LiquifyParams, c: Point, spacing: f64) {
             continue;
         }
         let len = cub.arclen(1e-3);
-        let k = (len / spacing).ceil() as usize;
+        // (No more pieces than the anchors left under the ceiling.)
+        let k = ((len / spacing).ceil() as usize).min(MAX_ANCHORS.saturating_sub(sp.anchors.len()) + 1);
         if k <= 1 {
             seg += 1;
             continue;

crates/tools/src/distort/perspective.rs

@@ -549,6 +549,11 @@ impl PerspectiveGrid {
         let Some(h) = self.homography(plane) else { return vec![] };
         let d = self.domain(plane);
         let mut out = vec![];
+        // A stored grid is not checked: no spacing, or an extent that isn't a number, has no lines.
+        let (w, h_len) = (d.width(), d.height());
+        if !(self.cell.is_finite() && self.cell > 0.0 && w.is_finite() && h_len.is_finite()) {
+            return out;
+        }
         let step = |len: f64| {
             let mut s = self.cell;
             while len / s > 120.0 {
@@ -556,20 +561,22 @@ impl PerspectiveGrid {
             }
             s
         };
-        let (su, sv) = (step(d.width()), step(d.height()));
-        let mut u = 0.0;
-        while u <= d.width() + 1e-9 {
-            if let (Some(a), Some(b)) = (h.apply(Point::new(u, 0.0)), h.apply(Point::new(u, d.height()))) {
+        let (su, sv) = (step(w), step(h_len));
+        // Counted, not summed: a count in whole lines (at most 121 a way). The fixed 1e-9
+        // tolerance can inflate the count when the whole domain is far smaller than it, so the
+        // bound is capped on its own, before any loop runs.
+        let count = |len: f64, s: f64| ((len + 1e-9) / s).floor().clamp(0.0, 120.0) as usize;
+        for i in 0..=count(w, su) {
+            let u = i as f64 * su;
+            if let (Some(a), Some(b)) = (h.apply(Point::new(u, 0.0)), h.apply(Point::new(u, h_len))) {
                 out.push((a, b));
             }
-            u += su;
         }
-        let mut v = 0.0;
-        while v <= d.height() + 1e-9 {
-            if let (Some(a), Some(b)) = (h.apply(Point::new(0.0, v)), h.apply(Point::new(d.width(), v))) {
+        for i in 0..=count(h_len, sv) {
+            let v = i as f64 * sv;
+            if let (Some(a), Some(b)) = (h.apply(Point::new(0.0, v)), h.apply(Point::new(w, v))) {
                 out.push((a, b));
             }
-            v += sv;
         }
         out
     }

crates/tools/src/distort/perspective/view.rs

@@ -123,6 +123,16 @@ mod tests {
         assert_eq!(g.snap_offset(b, Vec2::new(18.0, 0.0)), Vec2::new(18.0, 0.0), "nothing within reach");
     }
 
+    #[test]
+    fn a_tiny_stored_grid_draws_a_bounded_number_of_lines() {
+        // A stored grid can carry extreme values: the loop bound must not depend on a fixed
+        // tolerance swallowing the whole domain.
+        let g = PerspectiveGrid { cell: 1e-18, extent: 1e-18, height: 1e-18, ..PerspectiveGrid::normal(2, Rect::new(0.0, 0.0, 800.0, 600.0)) };
+        for pl in Plane::ALL {
+            assert!(g.lines(pl).len() <= 121 + 121, "at most 121 lines a way on {pl:?}");
+        }
+    }
+
     #[test]
     fn colours_and_rulers_come_from_the_definition() {
         let mut g = grid();