address review: validate finiteness before segment-limit fallback, loop raster grid budget reduction

This commit is contained in:
Dylan M. Taylor
2026-11-03 19:46:03 +00:00
parent 9dd821a8b8
commit f12f102497
3 changed files with 40 additions and 2 deletions
+20
View File
@@ -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.
+7
View File
@@ -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;
+13 -2
View File
@@ -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());
}
}