address review: count freeform line headers, share paint budget across items, bound dash walk, report trap pair limit

crates/effects/src/revolve.rs

@@ -29,17 +29,26 @@ pub(crate) fn paint_bytes(paint: &Paint) -> usize {
         Paint::Gradient(g) => g,
         _ => return own,
     };
-    let freeform = g
-        .freeform
-        .as_ref()
-        .map_or(0, |f| f.points.len() * size_of::<FreeformPoint>() + f.lines.iter().map(|l| l.len() * size_of::<usize>()).sum::<usize>());
+    let freeform = g.freeform.as_ref().map_or(0, |f| {
+        // Each line is a Vec: its header too, not only its indices.
+        f.points.len() * size_of::<FreeformPoint>()
+            + f.lines.len() * size_of::<Vec<usize>>()
+            + f.lines.iter().map(|l| l.len() * size_of::<usize>()).sum::<usize>()
+    });
     let stops: usize = g.gradient.stops.iter().map(|s| size_of::<GradientStop>() + name(&s.swatch)).sum();
     own + size_of::<vectorcraft_doc::color::GradientPaint>() + name(&g.swatch) + stops + freeform
 }
 
 /// Whether `copies` copies of `paint` stay within [`MAX_PAINT_COPIES`].
 pub(crate) fn paint_copies_fit(copies: usize, paint: &Paint) -> bool {
-    copies.saturating_mul(paint_bytes(paint)) <= MAX_PAINT_COPIES
+    paint_copies_add(&mut 0, copies, paint)
+}
+
+/// Add `copies` copies of `paint` to the running byte total `used`; whether it still fits within
+/// [`MAX_PAINT_COPIES`] (for several items whose copies count together).
+pub(crate) fn paint_copies_add(used: &mut usize, copies: usize, paint: &Paint) -> bool {
+    *used = used.saturating_add(copies.saturating_mul(paint_bytes(paint)));
+    *used <= MAX_PAINT_COPIES
 }
 
 fn visible(fx: &[Effect]) -> bool {

crates/effects/src/solids.rs

@@ -256,12 +256,14 @@ fn art_for(n: &Node, fx: &Effect, path: &PathData, rule: FillRule, items: &[&App
     if fx.id == ROTATE {
         // Every part turns about the centre of the object's own bounds, so they stay together.
         let mut out = vec![];
+        let mut paint_bytes = 0usize;
         for item in items {
             let (paint, opacity, blend) = paint_of(item);
             let (area, rule) = region(path, rule, item);
             let f = facets(ROTATE, &area, rule, b, &p).ok()?;
-            // Each facet and its seam stroke copy the paint; past the allowance the source path is drawn.
-            if !crate::revolve::paint_copies_fit(f.len().saturating_mul(2), &paint) {
+            // Each facet and its seam stroke copy the paint; past the allowance (all items together)
+            // the source path is drawn.
+            if !crate::revolve::paint_copies_add(&mut paint_bytes, f.len().saturating_mul(2), &paint) {
                 return None;
             }
             out.push(Arc::new(surface(n, f, &paint, opacity, blend, colored)));
@@ -289,10 +291,11 @@ fn art_for(n: &Node, fx: &Effect, path: &PathData, rule: FillRule, items: &[&App
         parts.iter().map(|(_, area, rule)| facets(&fx.id, area, *rule, bounds, &p)).collect()
     };
     let mut solids = vec![];
+    let mut paint_bytes = 0usize;
     for ((item, _, _), f) in parts.iter().zip(built) {
         let item: &AppearanceItem = item;
         match f {
-            Ok(f) if !crate::revolve::paint_copies_fit(f.len().saturating_mul(2), &paint_of(item).0) => return None,
+            Ok(f) if !crate::revolve::paint_copies_add(&mut paint_bytes, f.len().saturating_mul(2), &paint_of(item).0) => return None,
             Ok(f) => solids.push((item, f)),
             // A stroke's outline past the limits is left out; the fill's solid still draws.
             Err(_) if matches!(item, AppearanceItem::Stroke(_)) => {}

crates/effects/src/stroke/dash.rs

@@ -89,8 +89,13 @@ pub fn dash(bp: &BezPath, d: &Dash) -> Option<Dashed> {
     }
     let phase = Phase { ix: ix0, rem: pat[ix0] - off, on: ix0 % 2 == 0 };
     let mut out = Dashed::default();
+    // The estimates above trust the fitted scales; this counts the pattern entries actually walked,
+    // so entries that fit down to nothing can't keep a subpath looping.
+    let mut budget = MAX_STEPS as usize;
     for plan in &plans {
-        dash_subpath(&mut out, plan, &pat, phase);
+        if !dash_subpath(&mut out, plan, &pat, phase, &mut budget) {
+            return None;
+        }
     }
     Some(out)
 }
@@ -183,7 +188,8 @@ impl Piece {
     }
 }
 
-fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase) {
+/// Dash one subpath; `false` when `budget` (pattern entries left to walk, shared by all subpaths) runs out.
+fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase, budget: &mut usize) -> bool {
     let first_dot = out.dots.len();
     let total: f64 = plan.lens.iter().sum();
     // Fraction of the subpath's length at `x` along the plan.
@@ -235,6 +241,8 @@ fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase) {
                     push_dot(out, first_dot, Dot { at: seg.eval(t), dir: tangent(seg, t), t: frac(acc + end) });
                 }
             }
+            let Some(rest) = budget.checked_sub(1) else { return false };
+            *budget = rest;
             cuts += 1;
             s = end;
             ph.ix = (ph.ix + 1) % pat.len();
@@ -263,7 +271,7 @@ fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase) {
             write(&mut out.path, &d.segs);
             out.path.close_path();
             out.spans.push((frac(0.0), frac(0.0) + 1.0));
-            return;
+            return true;
         }
         if starts_on && ends_on && dashes.len() > 1 {
             // The dash running through the start point is one dash.
@@ -281,6 +289,7 @@ fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase) {
         let t0 = frac(d.start);
         out.spans.push((t0, if total > 0.0 { t0 + (d.end - d.start) / total } else { t0 }));
     }
+    true
 }
 
 /// Add `dot` unless the subpath's last dot (from index `first`) is already there: a run ending on

crates/effects/src/trap.rs

@@ -241,20 +241,27 @@ fn near_pairs(rects: &[Option<Rect>], reach_x: f64, reach_y: f64) -> Option<Vec<
 
 /// Most candidate pairs one trap evaluation keeps; past it no traps are made (the art draws as is).
 /// Each pair costs a boolean trap band, so real art stays far below this.
-const MAX_NEAR_PAIRS: usize = 1 << 20;
+pub const MAX_NEAR_PAIRS: usize = 1 << 20;
 
 /// The traps between `sources` (back to front) with options `o`: for each two sources of
 /// different solid colours whose visible parts meet, the strip of the darker one's visible part
 /// next to the lighter one's (the other way round with [`TrapOptions::reverse`]), painted per
-/// [`trap_paint`]. Empty when no visible edges of different colours meet.
+/// [`trap_paint`]. Empty when no visible edges of different colours meet, or when there are more
+/// than [`MAX_NEAR_PAIRS`] candidate pairs (see [`try_traps`] to tell those apart).
 pub fn traps(sources: &[TrapSource], o: &TrapOptions) -> Vec<TrapPiece> {
+    try_traps(sources, o).unwrap_or_default()
+}
+
+/// [`traps`], but `None` when there are more than [`MAX_NEAR_PAIRS`] candidate pairs, as opposed to
+/// `Some` of an empty list when no edges meet.
+pub fn try_traps(sources: &[TrapSource], o: &TrapOptions) -> Option<Vec<TrapPiece>> {
     let colors: Vec<Option<Color>> = sources.iter().map(|s| trap_color(&s.fill)).collect();
     if colors.iter().flatten().count() < 2 {
-        return vec![];
+        return Some(vec![]);
     }
     let (w, r) = (o.thickness, o.height_width);
     if !(w.is_finite() && w > 0.0 && r.is_finite() && r > 0.0) {
-        return vec![]; // `trap_band` cuts nothing with these.
+        return Some(vec![]); // `trap_band` cuts nothing with these.
     }
     let vis = visible(sources);
     // Only pairs whose visible parts come within the trap's reach can trap (`trap_band` rejects
@@ -266,7 +273,7 @@ pub fn traps(sources: &[TrapSource], o: &TrapOptions) -> Vec<TrapPiece> {
     let extent = rects.iter().flatten().flat_map(|b| [b.x0, b.y0, b.x1, b.y1]).fold(0.0_f64, |m, v| m.max(v.abs()));
     let hair = 1e-9 * (1.0 + extent + w * r.max(1.0));
     let mut out = vec![];
-    let Some(pairs) = near_pairs(&rects, w + hair, w * r + hair) else { return vec![] };
+    let pairs = near_pairs(&rects, w + hair, w * r + hair)?;
     for (i, j) in pairs {
         let (Some(ci), Some(cj)) = (colors[i], colors[j]) else { continue };
         if vis[i].is_empty() || vis[j].is_empty() || same_color(&sources[i].fill, &sources[j].fill, ci, cj) {
@@ -286,7 +293,7 @@ pub fn traps(sources: &[TrapSource], o: &TrapOptions) -> Vec<TrapPiece> {
         let Some(color) = colors[from] else { continue };
         out.push(TrapPiece { path: band, paint: trap_paint(&sources[from].fill, color, o), from, into });
     }
-    out
+    Some(out)
 }
 
 /// A trap's appearance: one overprinting fill of `paint`, no stroke.
@@ -393,6 +400,7 @@ mod tests {
     fn no_trap_without_adjoining_edges_of_different_colours() {
         let apart = vec![src(Rect::new(0.0, 0.0, 10.0, 10.0), yellow()), src(Rect::new(30.0, 0.0, 40.0, 10.0), blue())];
         assert!(traps(&apart, &TrapOptions::default()).is_empty());
+        assert_eq!(try_traps(&apart, &TrapOptions::default()).map(|t| t.len()), Some(0), "no edges meet: empty, not over the cap");
         let same = vec![src(Rect::new(0.0, 0.0, 10.0, 10.0), blue()), src(Rect::new(10.0, 0.0, 20.0, 10.0), blue())];
         assert!(traps(&same, &TrapOptions::default()).is_empty());
         let mut unfilled = abutting();

crates/engine/src/cmd/pathops.rs

@@ -683,7 +683,7 @@ fn run_pf(s: &mut Session, p: &Value, op: PathfinderOp) -> Result<Value> {
 /// Pathfinder panel › Trap ([`vectorcraft_render::effects::trap`]): the traps between the selected
 /// objects' leaf shapes, as one group of overprinting paths above the topmost selected object.
 fn trap(s: &mut Session, p: &Value) -> Result<Value> {
-    use vectorcraft_render::effects::trap::{TRAP_LABEL, TrapOptions, TrapSource, trap_appearance, traps};
+    use vectorcraft_render::effects::trap::{TRAP_LABEL, TrapOptions, TrapSource, trap_appearance, try_traps};
     const ID: &str = "object.pathfinder.trap";
     let mut q = p.clone();
     if let Some(t) = len_param(p, "thickness") {
@@ -721,7 +721,12 @@ fn trap(s: &mut Session, p: &Value) -> Result<Value> {
         if sources.iter().filter(|s| s.fill.color().is_some()).count() < 2 {
             return Err(EngineError::Other("Trap: select two or more objects filled with solid colours".into()));
         }
-        let made = traps(&sources, &options);
+        let Some(made) = try_traps(&sources, &options) else {
+            return Err(EngineError::Other(format!(
+                "Trap: more than {} pairs of objects lie close enough to trap; trap fewer objects at a time",
+                vectorcraft_render::effects::trap::MAX_NEAR_PAIRS
+            )));
+        };
         if made.is_empty() {
             return Err(EngineError::Other("Trap: no edges of different colours meet among the selected objects".into()));
         }