Merge pull request #1374

Render: skip shapes far larger than any screen instead of aborting (#1165)

# Conflicts:
#	crates/render/src/renderer.rs
This commit is contained in:
Brandon Thomas committed 2026-10-30 15:42:21 -04:00
commit c389f62b33
3 files changed
+81 -1

No files matched your search

+18
View File
@@ -199,3 +199,21 @@ pub(crate) fn scale_of(m: Affine) -> f64 {
pub(crate) fn blur(sigma: f64) -> Filter {
Filter::from_primitive(FilterPrimitive::GaussianBlur { std_deviation: sigma.clamp(0.0, 500.0) as f32, edge_mode: EdgeMode::None })
}
/// The widest a shape may reach on screen, in pixels, and still be drawn: ten thousand screens
/// across, far past any real deck, and still cheap for vello to flatten (#1165).
const MAX_DEVICE_EXTENT: f64 = 1e7;
/// Whether a shape's geometry can go to vello. Geometry far beyond any screen (a size at the EMU
/// limit, group scales compounding, custom-geometry guides) makes vello's flattener size a buffer
/// by its on-screen length, and the allocation aborts the process (#1165). Each path is checked
/// for finite points first: a bounding box drops NaN, and a union with the frame would hide it.
/// The extent is the control box, which always holds the curve: the tight bounding box can miss
/// a curve's extremum when finding it overflows.
pub(crate) fn drawable(geo: &Geometry, m: Affine, w: f64, h: f64) -> bool {
if !geo.paths.iter().all(|p| p.path.is_finite()) {
return false;
}
let dev = m.transform_rect_bbox(geo.outline().control_box().union(Rect::new(0.0, 0.0, w, h)));
dev.is_finite() && dev.width().max(dev.height()) <= MAX_DEVICE_EXTENT
}
+7 -1
View File
@@ -1,4 +1,6 @@
use super::geometry::{Caster, blur, border_outside, camera_turn, color, fill_opacity, outside_line, scale_of, shadow_local, shape_geometry};
use super::geometry::{
Caster, blur, border_outside, camera_turn, color, drawable, fill_opacity, outside_line, scale_of, shadow_local, shape_geometry,
};
use super::opts::{RenderOpts, ShapeState, SlideFields};
use super::raster::Image;
use super::slide::{SlidePart, needs_filters, output_size};
@@ -585,6 +587,10 @@ impl Renderer {
Some(pm) => self.morphs.geometry(pm, s, w, h),
None => (self.scaled_geometry(s, m, depth, w, h), Vec::new()),
};
if !drawable(&geo, m, w, h) {
log::warn!("render: shape {} skipped: its outline is not finite or far larger than any screen", s.id.0);
return;
}
let (fill, fill_ph) = resolve::fill(rctx, s);
let (line, line_ph) = resolve::line(rctx, s);
let (effects, fx_ph) = resolve::effects(rctx, s);
+56
View File
@@ -2951,3 +2951,59 @@ fn pie_data_labels_take_the_chart_text_colour_when_it_has_one() {
assert!(count(&coloured, &red) > 30, "the labels take the chart's red: {} red pixels", count(&coloured, &red));
assert!(count(&plain, &white) > count(&coloured, &white), "the labels were white before");
}
/// #1165: an ellipse sized at the EMU limit, crossing the slide, made vello's flattener allocate a
/// buffer from its on-screen length and abort. It is skipped; the rest of the slide still draws.
#[test]
fn a_shape_far_larger_than_any_screen_is_skipped_not_flattened() {
let huge = Shape {
id: ShapeId(501),
geom: Geom::preset("ellipse"),
xfrm: Some(Xfrm::new(-5e17, -5e17, 1e18, 1e18)),
style: Some(ShapeStyle::accent(deckcraft_color::SchemeSlot::Accent2)),
..Default::default()
};
let normal = Shape {
id: ShapeId(502),
xfrm: Some(Xfrm::new(100.0, 100.0, 200.0, 100.0)),
style: Some(ShapeStyle::accent(deckcraft_color::SchemeSlot::Accent1)),
..Default::default()
};
let p = deck_with(vec![huge, normal]);
let img = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() });
let accent = p.masters[0].theme.colors.get(deckcraft_color::SchemeSlot::Accent1);
let px = img.pixel(200, 150);
assert!((px[0] as i32 - accent.r as i32).abs() <= 2, "{px:?} vs {accent:?}");
assert_eq!(img.pixel(50, 50), [255, 255, 255, 255], "the huge ellipse is not drawn");
}
/// #1165: a path with a NaN point is not drawn, even when the shape's frame is finite and small
/// (a bounding box, or a union with the frame, would hide the NaN).
#[test]
fn a_shape_whose_outline_is_not_finite_is_not_drawable() {
let geo = |pts: &[(f64, f64)]| {
let mut path = kurbo::BezPath::new();
path.move_to(pts[0]);
for p in &pts[1..] {
path.line_to(*p);
}
deckcraft_geom::preset::Geometry {
paths: vec![deckcraft_geom::preset::SubPath { path, fill: deckcraft_geom::preset::FillMode::Norm, stroke: true, even_odd: false }],
text_rect: kurbo::Rect::ZERO,
handles: vec![],
sites: vec![],
}
};
let m = kurbo::Affine::IDENTITY;
assert!(crate::geometry::drawable(&geo(&[(0.0, 0.0), (100.0, 50.0)]), m, 100.0, 50.0));
assert!(!crate::geometry::drawable(&geo(&[(0.0, 0.0), (f64::NAN, 50.0)]), m, 100.0, 50.0));
assert!(!crate::geometry::drawable(&geo(&[(0.0, 0.0), (f64::INFINITY, 50.0)]), m, 100.0, 50.0));
assert!(!crate::geometry::drawable(&geo(&[(0.0, 0.0), (1e18, 50.0)]), m, 100.0, 50.0));
// A quadratic whose extremum overflows kurbo's bounding box: its control point still counts.
let mut path = kurbo::BezPath::new();
path.move_to((0.0, 0.0));
path.quad_to((1e308, 0.0), (0.0, 0.0));
let mut quad = geo(&[(0.0, 0.0), (1.0, 1.0)]);
quad.paths[0].path = path;
assert!(!crate::geometry::drawable(&quad, m, 100.0, 50.0));
}