Merge pull request #5493 from storytold/fix/sec-effects-paint-dash-budget

Effects: bound 3D paint copies, dash restarts and trap candidate pairs

crates/effects/src/revolve.rs

@@ -25,17 +25,33 @@ const MAX_OVERLAP_TESTS: usize = 1 << 22;
 /// The width of the stroke around each face that covers the seams between faces.
 const SEAM_WIDTH: f64 = 0.75;
 
-/// About the bytes one copy of `paint` takes.
+/// About the bytes one copy of `paint` takes, with the strings it owns (swatch and pattern names).
 fn paint_bytes(paint: &Paint) -> usize {
     use std::mem::size_of;
     use vectorcraft_doc::color::{FreeformPoint, GradientStop};
     let own = size_of::<Paint>();
-    let Paint::Gradient(g) = paint else { 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>());
-    own + size_of::<vectorcraft_doc::color::GradientPaint>() + g.gradient.stops.len() * size_of::<GradientStop>() + freeform
+    let name = |s: &Option<String>| s.as_ref().map_or(0, String::len);
+    let g = match paint {
+        Paint::Solid { swatch, .. } => return own + name(swatch),
+        Paint::Pattern { pattern, .. } => return own + pattern.len(),
+        Paint::Gradient(g) => g,
+        _ => return own,
+    };
+    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
+}
+
+/// Add `copies` copies of `paint` to the running byte total `used`; whether it still fits within
+/// [`MAX_PAINT_COPIES`] (for the solid surfaces, whose items' 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

@@ -484,6 +484,21 @@ fn paint_of(item: &AppearanceItem, member: f32) -> (Paint, f32, BlendMode) {
     }
 }
 
+/// `item`'s paint, borrowed (for checks that must not copy it).
+fn paint_ref(item: &AppearanceItem) -> &Paint {
+    match item {
+        AppearanceItem::Fill(f) => &f.paint,
+        AppearanceItem::Stroke(s) => &s.paint,
+    }
+}
+
+/// Paint copies `surface` makes for `facets`: two per facet (fill and seam stroke), six when colored
+/// lights give each facet derived paints (`lighting::solid_surface`).
+fn facet_copies(facets: &[Facet], paint: &Paint, colored: bool) -> usize {
+    let derived = colored && paint.color().is_some() && facets.iter().any(|f| f.rgb_gradient.is_some());
+    facets.len().saturating_mul(if derived { 6 } else { 2 })
+}
+
 /// The area `item` paints on `path`, and the rule that fills it: the path itself for a fill, the
 /// stroke's outline (whose contours nest as its holes do) for a stroke.
 fn region(path: &PathData, rule: FillRule, item: &AppearanceItem) -> (PathData, FillRule) {
@@ -543,6 +558,8 @@ fn art_for(n: &Node, fx: &Effect, shapes: &[Profile]) -> Option<Vec<Arc<Node>>>
     let classic = fx.id == CLASSIC_EXTRUDE;
     let colored = !classic && view_options(&p).has_colored_lights();
     let masks = has_masks(shapes);
+    // The paint copies of every solid together: past the allowance the source path is drawn.
+    let mut paint_bytes = 0usize;
     if fx.id == ROTATE {
         // Every part turns about the centre of the object's own bounds (a group's members about
         // the centre of theirs), so they stay together.
@@ -552,6 +569,9 @@ fn art_for(n: &Node, fx: &Effect, shapes: &[Profile]) -> Option<Vec<Arc<Node>>>
                 let (paint, opacity, blend) = paint_of(item, s.opacity);
                 let (area, rule) = region(&s.path, s.rule, item);
                 let f = facets(ROTATE, &area, rule, b, &p).ok()?;
+                if !crate::revolve::paint_copies_add(&mut paint_bytes, facet_copies(&f, &paint, colored), &paint) {
+                    return None;
+                }
                 out.push((s.scopes.as_slice(), Arc::new(surface(n, f, &paint, opacity, blend, colored))));
             }
         }
@@ -569,6 +589,7 @@ fn art_for(n: &Node, fx: &Effect, shapes: &[Profile]) -> Option<Vec<Arc<Node>>>
     for ((item, opacity, _, _, scopes), f) in parts.iter().zip(built) {
         let item: &AppearanceItem = item;
         match f {
+            Ok(f) if !crate::revolve::paint_copies_add(&mut paint_bytes, facet_copies(&f, paint_ref(item), colored), paint_ref(item)) => return None,
             Ok(f) => solids.push((item, *opacity, *scopes, 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

@@ -14,6 +14,10 @@ use super::{ARCLEN_ACCURACY, kink, push_seg, segments, subpaths, tangent};
 
 /// More dashes than this (a tiny pattern on a long path) draws the line solid instead.
 const MAX_DASHES: f64 = 200_000.0;
+/// Pattern entries walked in all (every subpath and every corner restart starts the pattern over, and
+/// zero-length entries cost no path length, so a long pattern is walked in full even on a short
+/// run); past this the line is drawn solid. About 100x what real art needs.
+const MAX_STEPS: f64 = 20_000_000.0;
 /// A pattern whose longest entry is shorter than this can't advance along the path (each entry
 /// ends within the generator's tolerance of where it starts), so it draws the line solid.
 const MIN_ENTRY: f64 = 1e-6;
@@ -80,6 +84,10 @@ pub fn dash_kept(bp: &BezPath, d: &Dash, keep: &dyn Fn(&PathSeg) -> bool) -> Opt
     if plans.iter().map(|p| p.periods).sum::<f64>() * pat.len() as f64 > MAX_DASHES {
         return None;
     }
+    let restarts = |p: &Plan| p.restart.iter().filter(|r| **r).count() as f64;
+    if plans.iter().map(|p| (p.periods.ceil().max(1.0) + restarts(p)) * pat.len() as f64).sum::<f64>() > MAX_STEPS {
+        return None;
+    }
     // Where an exact pattern starts: `offset` into it.
     let mut off = d.offset.rem_euclid(period);
     let mut ix0 = 0;
@@ -89,8 +97,13 @@ pub fn dash_kept(bp: &BezPath, d: &Dash, keep: &dyn Fn(&PathSeg) -> bool) -> Opt
     }
     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, keep);
+        if !dash_subpath(&mut out, plan, &pat, phase, keep, &mut budget) {
+            return None;
+        }
     }
     Some(out)
 }
@@ -183,7 +196,8 @@ impl Piece {
     }
 }
 
-fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase, keep: &dyn Fn(&PathSeg) -> bool) {
+/// 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, keep: &dyn Fn(&PathSeg) -> bool, 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.
@@ -244,6 +258,8 @@ fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase, keep:
                     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();
@@ -273,7 +289,7 @@ fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase, keep:
             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.
@@ -291,6 +307,7 @@ fn dash_subpath(out: &mut Dashed, plan: &Plan, pat: &[f64], mut ph: Phase, keep:
         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/stroke/tests_fit.rs

@@ -222,3 +222,17 @@ fn smooth_points_of_curves_stay_smooth_with_any_join() {
         assert!((a - want).abs() / want < 0.005, "{join:?}: {a} vs {want}");
     }
 }
+
+#[test]
+fn a_long_pattern_restarted_on_many_short_subpaths_draws_solid() {
+    // Each subpath walks the zero-length entries in full although it is far shorter than a period.
+    let mut pattern = vec![0.0; 2000];
+    pattern[0] = 1.0;
+    let mut b = BezPath::new();
+    for i in 0..20_000 {
+        b.move_to((i as f64 * 2.0, 0.0));
+        b.line_to((i as f64 * 2.0 + 0.01, 0.0));
+    }
+    let d = Dash { pattern, offset: 0.0, align_corners: false };
+    assert!(dash(&b, &d).is_none());
+}

crates/effects/src/trap.rs

@@ -218,34 +218,66 @@ fn visible(sources: &[TrapSource]) -> Vec<PathData> {
 /// The pairs `(i, j)`, `i < j`, in ascending order, of `rects` (`None`: left out) that come within
 /// `reach_x` of each other across x and `reach_y` across y. A sweep over the rects sorted by their
 /// left edge, so shapes spread over the page cost about one check each rather than one per pair.
-fn near_pairs(rects: &[Option<Rect>], reach_x: f64, reach_y: f64) -> Vec<(usize, usize)> {
+/// `None` when there are more than [`MAX_NEAR_PAIRS`] (many shapes all within reach of each other
+/// make the pair list grow with the square of their count) or the sweep examines more than
+/// `MAX_NEAR_SCAN` pairs (many shapes sharing an x range, far apart in y).
+fn near_pairs(rects: &[Option<Rect>], reach_x: f64, reach_y: f64) -> Option<Vec<(usize, usize)>> {
+    near_pairs_within(rects, reach_x, reach_y, MAX_NEAR_SCAN)
+}
+
+fn near_pairs_within(rects: &[Option<Rect>], reach_x: f64, reach_y: f64, max_scan: usize) -> Option<Vec<(usize, usize)>> {
     let mut order: Vec<(usize, Rect)> = rects.iter().enumerate().filter_map(|(i, r)| r.map(|r| (i, r))).collect();
     order.sort_unstable_by(|a, b| a.1.x0.total_cmp(&b.1.x0).then(a.0.cmp(&b.0)));
     let mut pairs = vec![];
+    let mut scanned = 0usize;
     for (k, &(i, a)) in order.iter().enumerate() {
         // Later rects start at or right of `a`, so only how far right they start can rule them out.
         for &(j, b) in order[k + 1..].iter().take_while(|(_, b)| b.x0 <= a.x1 + reach_x) {
+            // Rects sharing an x range but far apart in y are examined without being stored.
+            scanned += 1;
+            if scanned > max_scan {
+                return None;
+            }
             if b.y0 <= a.y1 + reach_y && a.y0 <= b.y1 + reach_y {
+                if pairs.len() >= MAX_NEAR_PAIRS {
+                    return None;
+                }
                 pairs.push((i.min(j), i.max(j)));
             }
         }
     }
     pairs.sort_unstable();
-    pairs
+    Some(pairs)
 }
 
+/// Most pairs the sweep in `near_pairs` examines, stored or not (a plain bounds check each).
+const MAX_NEAR_SCAN: usize = 1 << 27;
+
+/// 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.
+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 or the pair scan is too long (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 or the scan for
+/// them is too long (`MAX_NEAR_SCAN` pairs examined), 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
@@ -257,7 +289,8 @@ 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![];
-    for (i, j) in near_pairs(&rects, w + hair, w * r + hair) {
+    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) {
             continue;
@@ -276,7 +309,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.
@@ -379,10 +412,19 @@ mod tests {
         assert!(near(po::area(&t[0].path, FillRule::NonZero), 22.0 * 22.0 - 400.0));
     }
 
+    #[test]
+    fn the_pair_scan_is_bounded_even_when_no_pair_is_near() {
+        // Eleven rects share an x range but lie far apart in y: 55 pairs are examined, none stored.
+        let rects: Vec<Option<Rect>> = (0..11).map(|k| Some(Rect::new(0.0, k as f64 * 1000.0, 10.0, k as f64 * 1000.0 + 10.0))).collect();
+        assert_eq!(near_pairs_within(&rects, 1.0, 1.0, 100), Some(vec![]));
+        assert_eq!(near_pairs_within(&rects, 1.0, 1.0, 54), None);
+    }
+
     #[test]
     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();
@@ -431,10 +473,13 @@ mod tests {
     fn only_shapes_within_reach_are_paired() {
         let s = many_apart_and_one_abutting();
         let rects: Vec<Option<Rect>> = s.iter().map(|t| t.path.bounds()).collect();
-        assert_eq!(near_pairs(&rects, 1.0, 1.0), vec![(0, 2000)], "the squares 10 pt apart are never paired");
+        assert_eq!(near_pairs(&rects, 1.0, 1.0), Some(vec![(0, 2000)]), "the squares 10 pt apart are never paired");
         // Reaching 10 pt, neighbours in a row pair, in ascending order; left-out rects never do.
         let row: Vec<Option<Rect>> = [3, 1, 0, 2].iter().map(|&k| s[k].path.bounds()).chain([None]).collect();
-        assert_eq!(near_pairs(&row, 10.0, 10.0), vec![(0, 3), (1, 2), (1, 3)]);
+        assert_eq!(near_pairs(&row, 10.0, 10.0), Some(vec![(0, 3), (1, 2), (1, 3)]));
+        // Past the allowance the pair list stops growing instead of being built.
+        let packed = vec![Some(Rect::new(0.0, 0.0, 1.0, 1.0)); 1500];
+        assert_eq!(near_pairs(&packed, 1.0, 1.0), None);
         let t = traps(&s, &TrapOptions { thickness: 1.0, ..Default::default() });
         assert_eq!(t.len(), 1, "only the abutting pair traps");
         assert_eq!((t[0].from, t[0].into), (2000, 0));

crates/engine/src/cmd/addons/pathtools.rs

@@ -774,8 +774,9 @@ fn divide_dashes(s: &mut Session, p: &Value) -> Result<Value> {
                 continue;
             }
             let only = (segs.len() < sp.segment_count()).then_some(&segs);
-            let out = pt::divide_dashes(sp, only, spec, MAX_PIECES.saturating_sub(made))
-                .ok_or_else(|| bad(C, "that makes too many pieces (at most 100000): make the dashes longer"))?;
+            let out = pt::divide_dashes(sp, only, spec, MAX_PIECES.saturating_sub(made)).ok_or_else(|| {
+                bad(C, "that makes too many pieces (at most 100000) or dash steps: simplify the dash pattern or make the dashes longer")
+            })?;
             made += out.len();
             for (piece, kind) in out.into_iter().filter(|(_, k)| !(remove_gaps && *k == pt::Piece::Gap)) {
                 dashes += usize::from(kind == pt::Piece::Dash);
@@ -825,7 +826,7 @@ fn dashed_paths(d: &Document, roots: &[NodeId]) -> Vec<NodeId> {
 
 fn expand_dashes(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "addon.pathtools.expandDashes";
-    let too_many = || bad(C, "that makes too many pieces (at most 100000): make the dashes longer");
+    let too_many = || bad(C, "that makes too many pieces (at most 100000) or dash steps: simplify the dash pattern or make the dashes longer");
     let roots = checked_ids_or_selection(s, p, C)?;
     let d = &s.doc()?.doc;
     // Each dashed path and its dashes, as the canvas draws them.

crates/engine/src/cmd/pathops.rs

@@ -719,7 +719,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) = length_given(p, "thickness", ID)? {
@@ -758,7 +758,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, or too many objects share a horizontal range; 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()));
         }