From f12f10249725834558d84dbf63597aca57f0a2f9 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 19:46:03 +0000 Subject: [PATCH] address review: validate finiteness before segment-limit fallback, loop raster grid budget reduction --- crates/prepress/src/flatten/compose.rs | 20 ++++++++++++++++++++ crates/prepress/src/flatten/geom.rs | 7 +++++++ crates/prepress/src/flatten/raster.rs | 15 +++++++++++++-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/prepress/src/flatten/compose.rs b/crates/prepress/src/flatten/compose.rs index 2c98db88..fc5f2ef4 100644 --- a/crates/prepress/src/flatten/compose.rs +++ b/crates/prepress/src/flatten/compose.rs @@ -189,6 +189,7 @@ fn flatten_group( let segments = b.shapes.iter().map(|s| s.path.elements().len()).chain(b.clips.iter().map(|c| c.path.elements().len())).fold(0_usize, usize::saturating_add); if segments > MAX_GROUP_SEGMENTS { + geom::check_finite(&b.shapes, &b.clips)?; let mut path = BezPath::new(); for &i in members { if !bounds[i].is_nan() { @@ -1128,6 +1129,25 @@ mod tests { assert_eq!(f.report.regions, 0, "the arrangement was not built"); } + #[test] + fn a_group_with_too_many_segments_still_rejects_non_finite_points() { + let mut path = BezPath::new(); + path.move_to(Point::new(0.0, 0.0)); + for i in 0..=MAX_GROUP_SEGMENTS { + path.line_to(Point::new((i % 100) as f64, (i / 100) as f64)); + } + path.line_to(Point::new(f64::NAN, 0.0)); + path.close_path(); + let crowded = Item::Fill(FillItem { + path, + rule: FillRule::NonZero, + paint: Paint::Solid(InkColor::Tints(vec![(C, 1.0)])), + overprint: Overprint::default(), + }); + let l = list(InkSet::new(), vec![Item::Group(group(0.5, BlendMode::Normal, vec![crowded]))]); + assert!(flatten(&l, &FlattenOptions::default()).is_err()); + } + #[test] fn kept_isolated_and_knockout_groups_come_back_plain() { // A placed page: an isolated group of opaque art, away from any transparency. diff --git a/crates/prepress/src/flatten/geom.rs b/crates/prepress/src/flatten/geom.rs index 75b51a08..0504c2c7 100644 --- a/crates/prepress/src/flatten/geom.rs +++ b/crates/prepress/src/flatten/geom.rs @@ -113,6 +113,13 @@ pub fn regions_with(shapes: &[Shape], clips: &[ClipPath], sliver: f64) -> Result Ok(Arrangement::new(shapes, clips, sliver)?.regions()) } +/// The finiteness check [`regions_with`] makes: every point of every shape path and of every clip +/// path a shape names. For callers that skip the arrangement but must still reject bad input. +pub fn check_finite(shapes: &[Shape], clips: &[ClipPath]) -> Result<(), GeomError> { + let used = shapes.iter().flat_map(|s| &s.clips).filter_map(|&c| clips.get(c)).map(|c| &c.path); + eps_for(shapes.iter().map(|s| &s.path).chain(used)).map(|_| ()) +} + /// Box comparisons [`overlap_groups`] makes before it folds the boxes still open into one group: /// many boxes sharing a horizontal span would otherwise cost one comparison per pair. const MAX_OVERLAP_COMPARISONS: usize = 4_000_000; diff --git a/crates/prepress/src/flatten/raster.rs b/crates/prepress/src/flatten/raster.rs index ae75fb47..9b2516b3 100644 --- a/crates/prepress/src/flatten/raster.rs +++ b/crates/prepress/src/flatten/raster.rs @@ -323,8 +323,11 @@ impl Grid { return Some((grid, false)); } // Scaling both sides down once is not enough for a thin region, whose short side stays - // at its minimum pixels: the second pass scales by the pixel ratio itself. - for linear in [false, true] { + // at its minimum pixels: later passes scale by the pixel ratio itself. A grid whose + // coordinates hit the clamp reports fewer pixels than it spans, so a few passes may be + // needed; each shrinks the scale, and 8 bound the work. + for pass in 0..8 { + let linear = pass > 0; let ratio = MAX_RASTER_PIXELS as f64 / grid.pixels() as f64; scale *= if linear { ratio } else { ratio.sqrt() } * 0.999; grid = Self::new(bounds, page, scale); @@ -424,4 +427,12 @@ mod tests { let (grid, _) = Grid::within_budget(page, page, 600.0 / 72.0).unwrap(); assert!(grid.pixels() <= MAX_RASTER_PIXELS && grid.pixels() > MAX_RASTER_PIXELS / 2, "{} × {}", grid.width(), grid.height()); } + + #[test] + fn a_clamped_grid_still_fits_the_pixel_budget() { + // Both sides past the coordinate clamp: the pixel count saturates, so one pass is not enough. + let page = Rect::new(0.0, 0.0, 1e15, 1e15); + let (grid, lowered) = Grid::within_budget(page, page, 600.0 / 72.0).unwrap(); + assert!(lowered && grid.pixels() <= MAX_RASTER_PIXELS, "{} × {}", grid.width(), grid.height()); + } }