Merge pull request #5411 from storytold/fix/sec-tools-bounded-loops

Tools: Shape Builder hatch lines are counted; Liquify simplification is linear

crates/tools/src/builder/map.rs

@@ -448,9 +448,15 @@ pub fn hatch(path: &BezPath, gap: f64) -> BezPath {
         y1 = y1.max(a.y.max(c.y));
     }
     let gap = gap.max((y1 - y0) / 400.0).max(1e-6);
-    let mut y = (y0 / gap).floor() * gap + gap * 0.5;
+    let start = (y0 / gap).floor() * gap + gap * 0.5;
     let mut xs: Vec<f64> = vec![];
-    while y < y1 {
+    // Counted, not summed: far from the origin `y + gap` can round back to `y`. The gap is at least
+    // a 400th of the height, so 402 lines cover it.
+    for i in 0..402u32 {
+        let y = start + f64::from(i) * gap;
+        if y.partial_cmp(&y1) != Some(std::cmp::Ordering::Less) {
+            break;
+        }
         xs.clear();
         for (a, c) in &edges {
             if (a.y <= y) != (c.y <= y) {
@@ -462,7 +468,6 @@ pub fn hatch(path: &BezPath, gap: f64) -> BezPath {
             out.move_to(unrot(Point::new(pair[0], y)));
             out.line_to(unrot(Point::new(pair[1], y)));
         }
-        y += gap;
     }
     out
 }

crates/tools/src/builder/tests.rs

@@ -713,6 +713,13 @@ fn hatch_stays_inside_region() {
     assert!(h.elements().len() > 10);
 }
 
+#[test]
+fn hatch_far_from_the_origin_ends() {
+    // At 1e17 a 4-point gap is below the coordinates' precision: adding it doesn't move the line.
+    let bp = shapes::rectangle(Rect::new(0.0, 1e17, 64.0, 1e17 + 64.0)).to_bezpath();
+    assert!(hatch(&bp, 4.0).elements().len() <= 2 * 402);
+}
+
 /// An empty hidden sources group for a Live Paint group made in a test.
 fn empty_sources(d: &mut Document) -> Arc<Node> {
     let mut g = Node::group(d.alloc_id(), vec![]);

crates/tools/src/distort/liquify.rs

@@ -508,31 +508,46 @@ fn simplify(sp: &mut SubPath, tol: f64, region: Rect) {
     if tol <= 0.0 {
         return;
     }
+    let min = if sp.closed { 4 } else { 3 };
+    let src = std::mem::take(&mut sp.anchors);
+    let n = src.len();
+    // Compacted into `out`, each anchor moved once: removing them from the middle one by one
+    // shifts the rest every time.
+    let mut out: Vec<Anchor> = Vec::with_capacity(n);
+    let mut src = src.into_iter().peekable();
+    out.extend(src.next());
+    let last = if sp.closed { n } else { n.saturating_sub(1) };
     let mut i = 1;
-    loop {
-        let n = sp.anchors.len();
-        let min = if sp.closed { 4 } else { 3 };
-        if n < min {
-            return;
-        }
-        let last = if sp.closed { n } else { n - 1 };
-        if i >= last {
-            return;
-        }
-        let (pa, k, pb) = (sp.anchors[i - 1], sp.anchors[i], sp.anchors[(i + 1) % n]);
-        let flat = region.contains(k.p)
+    while let Some(k) = src.next() {
+        // Anchors left if `k` stays: those kept, `k`, and those not yet looked at.
+        let left = out.len() + 1 + src.len();
+        let Some(&pa) = out.last() else { break };
+        let pb = match src.peek() {
+            Some(pb) => *pb,
+            None => out[0],
+        };
+        let flat = i < last
+            && left >= min
+            && region.contains(k.p)
             && [k.p, k.h_in, k.h_out, pa.h_out, pb.h_in].iter().all(|q| dist_seg(*q, pa.p, pb.p) <= tol)
             && (k.p - pa.p).dot(pb.p - k.p) > 0.0;
-        if flat {
-            let lab = pa.p.distance(pb.p);
-            let (lak, lkb) = (pa.p.distance(k.p).max(1e-9), k.p.distance(pb.p).max(1e-9));
-            sp.anchors[i - 1].h_out = pa.p + (pa.h_out - pa.p) * (lab / lak);
-            sp.anchors[(i + 1) % n].h_in = pb.p + (pb.h_in - pb.p) * (lab / lkb);
-            sp.anchors.remove(i);
-        } else {
-            i += 1;
+        i += 1;
+        if !flat {
+            out.push(k);
+            continue;
+        }
+        let lab = pa.p.distance(pb.p);
+        let (lak, lkb) = (pa.p.distance(k.p).max(1e-9), k.p.distance(pb.p).max(1e-9));
+        if let Some(a) = out.last_mut() {
+            a.h_out = pa.p + (pa.h_out - pa.p) * (lab / lak);
+        }
+        let h_in = pb.p + (pb.h_in - pb.p) * (lab / lkb);
+        match src.peek_mut() {
+            Some(b) => b.h_in = h_in,
+            None => out[0].h_in = h_in,
         }
     }
+    sp.anchors = out;
 }
 
 /// Apply a liquify stroke (its dabs, at the brush's intensity) to `path`. `salt` decorrelates
@@ -888,6 +903,21 @@ mod tests {
         LiquifyParams { intensity: 1.0, ..LiquifyParams::new(kind) }
     }
 
+    #[test]
+    fn simplify_drops_flat_anchors_and_keeps_the_ends() {
+        let pts: Vec<Point> = (0..200_000).map(|i| Point::new(i as f64, 0.0)).collect();
+        let mut open = SubPath::polyline(&pts, false);
+        let start = std::time::Instant::now();
+        simplify(&mut open, 0.5, Rect::new(-1.0, -1.0, 1e6, 1.0));
+        assert!(start.elapsed() < std::time::Duration::from_secs(5), "{:?}", start.elapsed());
+        assert_eq!(open.anchors.iter().map(|a| a.p).collect::<Vec<_>>(), [pts[0], pts[199_999]]);
+        // A closed square with a flat midpoint on each side keeps its four corners.
+        let ring = [(0.0, 0.0), (5.0, 0.0), (10.0, 0.0), (10.0, 5.0), (10.0, 10.0), (5.0, 10.0), (0.0, 10.0), (0.0, 5.0)];
+        let mut closed = SubPath::polyline(&ring.map(|(x, y)| Point::new(x, y)), true);
+        simplify(&mut closed, 0.5, Rect::new(-1.0, -1.0, 11.0, 11.0));
+        assert_eq!(closed.anchors.iter().map(|a| (a.p.x, a.p.y)).collect::<Vec<_>>(), [(0.0, 0.0), (10.0, 0.0), (10.0, 10.0), (0.0, 10.0)]);
+    }
+
     #[test]
     fn dabs_are_evenly_spaced() {
         let d = dabs(&[Point::new(0.0, 0.0), Point::new(100.0, 0.0)], 10.0);