From 34e6973b505a87719d853ac669d5216025f46dfe Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 03:11:07 +0000 Subject: [PATCH 1/3] effects: count paint strings in 3D copies, charge dash restarts, cap trap pairs --- crates/effects/src/revolve.rs | 22 +++++++++++++++++----- crates/effects/src/solids.rs | 5 +++++ crates/effects/src/stroke/dash.rs | 8 ++++++++ crates/effects/src/stroke/tests_fit.rs | 14 ++++++++++++++ crates/effects/src/trap.rs | 23 ++++++++++++++++++----- 5 files changed, 62 insertions(+), 10 deletions(-) diff --git a/crates/effects/src/revolve.rs b/crates/effects/src/revolve.rs index b2528818b..dfd85fa16 100644 --- a/crates/effects/src/revolve.rs +++ b/crates/effects/src/revolve.rs @@ -17,17 +17,29 @@ pub const REVOLVE: &str = "threeD.revolve"; /// thousands of faces took gigabytes; past this the source path is drawn. const MAX_PAINT_COPIES: usize = 256 << 20; -/// About the bytes one copy of `paint` takes. -fn paint_bytes(paint: &Paint) -> usize { +/// About the bytes one copy of `paint` takes, with the strings it owns (swatch and pattern names). +pub(crate) fn paint_bytes(paint: &Paint) -> usize { use std::mem::size_of; use vectorcraft_doc::color::{FreeformPoint, GradientStop}; let own = size_of::(); - let Paint::Gradient(g) = paint else { return own }; + let name = |s: &Option| 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| f.points.len() * size_of::() + f.lines.iter().map(|l| l.len() * size_of::()).sum::()); - own + size_of::() + g.gradient.stops.len() * size_of::() + freeform + let stops: usize = g.gradient.stops.iter().map(|s| size_of::() + name(&s.swatch)).sum(); + own + size_of::() + 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 } fn visible(fx: &[Effect]) -> bool { @@ -112,7 +124,7 @@ fn surface( let colored = options.light_color != [1.0; 3] || options.lights.is_some(); let faces = vectorcraft_three_d::revolve(path, b, options).ok()?; // A face's fill, its shadow and their seam strokes: up to four copies of the paint. - if faces.len().saturating_mul(4).saturating_mul(paint_bytes(&paint)) > MAX_PAINT_COPIES { + if !paint_copies_fit(faces.len().saturating_mul(4), &paint) { return None; } // Visibility is an expansion option, never a change to live geometry or export baking. diff --git a/crates/effects/src/solids.rs b/crates/effects/src/solids.rs index 1165340bc..34649a4c3 100644 --- a/crates/effects/src/solids.rs +++ b/crates/effects/src/solids.rs @@ -260,6 +260,10 @@ fn art_for(n: &Node, fx: &Effect, path: &PathData, rule: FillRule, items: &[&App 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) { + return None; + } out.push(Arc::new(surface(n, f, &paint, opacity, blend, colored))); } return Some(out); @@ -288,6 +292,7 @@ fn art_for(n: &Node, fx: &Effect, path: &PathData, rule: FillRule, items: &[&App 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) => 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(_)) => {} diff --git a/crates/effects/src/stroke/dash.rs b/crates/effects/src/stroke/dash.rs index 689c88dae..dd52063d9 100644 --- a/crates/effects/src/stroke/dash.rs +++ b/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; @@ -72,6 +76,10 @@ pub fn dash(bp: &BezPath, d: &Dash) -> Option { if plans.iter().map(|p| p.periods).sum::() * 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::() > MAX_STEPS { + return None; + } // Where an exact pattern starts: `offset` into it. let mut off = d.offset.rem_euclid(period); let mut ix0 = 0; diff --git a/crates/effects/src/stroke/tests_fit.rs b/crates/effects/src/stroke/tests_fit.rs index 91933087d..b427fdbce 100644 --- a/crates/effects/src/stroke/tests_fit.rs +++ b/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()); +} diff --git a/crates/effects/src/trap.rs b/crates/effects/src/trap.rs index f60232bed..5418038c3 100644 --- a/crates/effects/src/trap.rs +++ b/crates/effects/src/trap.rs @@ -218,7 +218,9 @@ fn visible(sources: &[TrapSource]) -> Vec { /// 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], 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). +fn near_pairs(rects: &[Option], reach_x: f64, reach_y: f64) -> Option> { 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![]; @@ -226,14 +228,21 @@ fn near_pairs(rects: &[Option], reach_x: f64, reach_y: f64) -> Vec<(usize, // 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) { 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 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; + /// 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 @@ -257,7 +266,8 @@ pub fn traps(sources: &[TrapSource], o: &TrapOptions) -> Vec { 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 Some(pairs) = near_pairs(&rects, w + hair, w * r + hair) else { return vec![] }; + 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; @@ -431,10 +441,13 @@ mod tests { fn only_shapes_within_reach_are_paired() { let s = many_apart_and_one_abutting(); let rects: Vec> = 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> = [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)); From adecf50ab9e27c76952d5577aa1f27c90c7887d5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 01:00:23 +0000 Subject: [PATCH 2/3] address review: count freeform line headers, share paint budget across items, bound dash walk, report trap pair limit --- crates/effects/src/revolve.rs | 19 ++++++++++++++----- crates/effects/src/solids.rs | 9 ++++++--- crates/effects/src/stroke/dash.rs | 15 ++++++++++++--- crates/effects/src/trap.rs | 20 ++++++++++++++------ crates/engine/src/cmd/pathops.rs | 9 +++++++-- 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/crates/effects/src/revolve.rs b/crates/effects/src/revolve.rs index dfd85fa16..53d3feb07 100644 --- a/crates/effects/src/revolve.rs +++ b/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::() + f.lines.iter().map(|l| l.len() * size_of::()).sum::()); + 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::() + + f.lines.len() * size_of::>() + + f.lines.iter().map(|l| l.len() * size_of::()).sum::() + }); let stops: usize = g.gradient.stops.iter().map(|s| size_of::() + name(&s.swatch)).sum(); own + size_of::() + 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 { diff --git a/crates/effects/src/solids.rs b/crates/effects/src/solids.rs index 34649a4c3..193f47c91 100644 --- a/crates/effects/src/solids.rs +++ b/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(_)) => {} diff --git a/crates/effects/src/stroke/dash.rs b/crates/effects/src/stroke/dash.rs index dd52063d9..657711de8 100644 --- a/crates/effects/src/stroke/dash.rs +++ b/crates/effects/src/stroke/dash.rs @@ -89,8 +89,13 @@ pub fn dash(bp: &BezPath, d: &Dash) -> Option { } 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 diff --git a/crates/effects/src/trap.rs b/crates/effects/src/trap.rs index 5418038c3..98287a3b7 100644 --- a/crates/effects/src/trap.rs +++ b/crates/effects/src/trap.rs @@ -241,20 +241,27 @@ fn near_pairs(rects: &[Option], reach_x: f64, reach_y: f64) -> Option Vec { + 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> { let colors: Vec> = 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 { 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 { 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(); diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index e1fecc6da..04887e291 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -683,7 +683,7 @@ fn run_pf(s: &mut Session, p: &Value, op: PathfinderOp) -> Result { /// 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 { - 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 { 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())); } From 18556f2a843e54898a3f661b414bb2fb2bd27bea Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 05:59:18 +0000 Subject: [PATCH 3/3] address review: one paint allowance across items, derived paints counted, bounded trap pair scan, dash error names both limits --- crates/effects/src/solids.rs | 21 ++++++++++------ crates/effects/src/trap.rs | 30 ++++++++++++++++++++--- crates/engine/src/cmd/addons/pathtools.rs | 7 +++--- crates/engine/src/cmd/pathops.rs | 2 +- 4 files changed, 46 insertions(+), 14 deletions(-) diff --git a/crates/effects/src/solids.rs b/crates/effects/src/solids.rs index 193f47c91..7348c9890 100644 --- a/crates/effects/src/solids.rs +++ b/crates/effects/src/solids.rs @@ -226,6 +226,13 @@ fn surface(n: &Node, facets: Vec, paint: &Paint, opacity: f32, blend: Ble group } +/// 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 }) +} + fn paint_of(item: &AppearanceItem) -> (Paint, f32, BlendMode) { match item { AppearanceItem::Fill(f) => (f.paint.clone(), f.opacity, f.blend), @@ -249,21 +256,20 @@ fn region(path: &PathData, rule: FillRule, item: &AppearanceItem) -> (PathData, /// a surface one composite or masks it, their faces are drawn in one back-to-front order (where /// depths tie, as for the caps turned toward the viewer, in the order of the appearance); /// otherwise each solid is a surface of its own, in that order. -fn art_for(n: &Node, fx: &Effect, path: &PathData, rule: FillRule, items: &[&AppearanceItem]) -> Option>> { +fn art_for(n: &Node, fx: &Effect, path: &PathData, rule: FillRule, items: &[&AppearanceItem], paint_bytes: &mut usize) -> Option>> { let b = path.bounds()?; let p = crate::merged_params(&fx.id, &fx.params); let colored = view_options(&p).has_colored_lights(); 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 (all items together) // the source path is drawn. - if !crate::revolve::paint_copies_add(&mut paint_bytes, f.len().saturating_mul(2), &paint) { + if !crate::revolve::paint_copies_add(paint_bytes, facet_copies(&f, &paint, colored), &paint) { return None; } out.push(Arc::new(surface(n, f, &paint, opacity, blend, colored))); @@ -291,11 +297,10 @@ 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_add(&mut paint_bytes, f.len().saturating_mul(2), &paint_of(item).0) => return None, + Ok(f) if !crate::revolve::paint_copies_add(paint_bytes, facet_copies(&f, &paint_of(item).0, colored), &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(_)) => {} @@ -354,11 +359,13 @@ pub fn solid_art(n: &Node) -> Option { let ctx = GeomContext::of(n); let paints = solid_paints(n); let painted: Vec<&AppearanceItem> = paints.iter().collect(); + // One paint-copy allowance for every item's solid together. + let mut paint_bytes = 0usize; let mut children = vec![]; let remaining: Vec; if let Some((index, fx)) = solid(&n.appearance.effects) { let path = geometry(&path, n.appearance.effects.get(..index)?, &ctx); - children.extend(art_for(n, fx, &path, rule, &painted)?); + children.extend(art_for(n, fx, &path, rule, &painted, &mut paint_bytes)?); // Later geometry effects act on the projected art as a whole. remaining = n .appearance @@ -374,7 +381,7 @@ pub fn solid_art(n: &Node) -> Option { for item in &painted { if let Some((index, fx)) = solid(item.effects()) { let p = geometry(&path, item.effects().get(..index)?, &ctx.item(item)); - for mut art in art_for(n, fx, &p, rule, &[*item])? { + for mut art in art_for(n, fx, &p, rule, &[*item], &mut paint_bytes)? { Arc::make_mut(&mut art).appearance.effects = item.effects().iter().skip(index + 1).filter(|e| !is_three_d(&e.id)).cloned().collect(); children.push(art); diff --git a/crates/effects/src/trap.rs b/crates/effects/src/trap.rs index 98287a3b7..b2721ed0f 100644 --- a/crates/effects/src/trap.rs +++ b/crates/effects/src/trap.rs @@ -219,14 +219,25 @@ fn visible(sources: &[TrapSource]) -> Vec { /// `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. /// `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). +/// 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], reach_x: f64, reach_y: f64) -> Option> { + near_pairs_within(rects, reach_x, reach_y, MAX_NEAR_SCAN) +} + +fn near_pairs_within(rects: &[Option], reach_x: f64, reach_y: f64, max_scan: usize) -> Option> { 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; @@ -239,6 +250,9 @@ fn near_pairs(rects: &[Option], reach_x: f64, reach_y: f64) -> Option Vec { try_traps(sources, o).unwrap_or_default() } -/// [`traps`], but `None` when there are more than [`MAX_NEAR_PAIRS`] candidate pairs, as opposed to +/// [`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> { let colors: Vec> = sources.iter().map(|s| trap_color(&s.fill)).collect(); @@ -396,6 +412,14 @@ 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> = (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())]; diff --git a/crates/engine/src/cmd/addons/pathtools.rs b/crates/engine/src/cmd/addons/pathtools.rs index 5d0e5723f..9ddcc4355 100644 --- a/crates/engine/src/cmd/addons/pathtools.rs +++ b/crates/engine/src/cmd/addons/pathtools.rs @@ -774,8 +774,9 @@ fn divide_dashes(s: &mut Session, p: &Value) -> Result { 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 { fn expand_dashes(s: &mut Session, p: &Value) -> Result { 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. diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index 04887e291..1d701e13f 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -723,7 +723,7 @@ fn trap(s: &mut Session, p: &Value) -> Result { } 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", + "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 ))); };