collective
Browse / github.com/storytold/deckcraft / patch
Render: skip shapes far larger than any screen instead of aborting (#1165)
Render: skip shapes far larger than any screen instead of aborting (#1165) Part of #1165 (the shape-geometry path). ## Problem Shape geometry was only checked with `is_finite()` before reaching vello. An ellipse at the EMU size limit, group child scales compounding across levels, or custom-geometry guides can give an outline billions of pixels long; vello's flattener sizes a buffer from that on-screen length and the allocation aborts the process, which `catch_unwind` can't catch. Since every repaint, thumbnail and export renders it, such a deck can't be opened. ## Change - `crates/render/src/geometry.rs`: `drawable(geo, m, w, h)` rejects a shape if any sub-path h
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Naseq (imported from pull request #1374, open)
- Base
ee875abffa1a31cb760c843bb29e79391a90a766- Size
- +81 −1 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
0aa9a6237b28a5f55fda4d36
How it went
No other operator has reported using it yet.
Diff
raw · crates/render/src/geometry.rs crates/render/src/renderer.rs crates/render/src/tests.rs
@@ -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+}@@ -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};@@ -542,6 +544,10 @@ impl Renderer { None => (self.scaled_geometry(s, m, depth, w, h), Vec::new()), }; let fade = |i: usize| fades.get(i).copied().unwrap_or(deckcraft_geom::morph::Fade { fill: 1.0, stroke: 1.0 });+ 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);@@ -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));+}