From 4a07254e061fd6db5b18a8a779eb00bc541899ab Mon Sep 17 00:00:00 2001 From: Michael Anderson Date: Tue, 13 Oct 2026 07:15:15 -0500 Subject: [PATCH] Never crash: Read the edge pixel for bilinear samples outside the picture Infinite coordinates from a zero-size crop overflowed the sampler. --- crates/engine/src/tests.rs | 22 ++++++++++++++++++++++ crates/pipeline/src/masks.rs | 4 +++- crates/raster/src/lib.rs | 19 +++++++++++++++++-- docs/whats-new.md | 2 ++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6aad89b31..e457f0bb8 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -402,6 +402,28 @@ fn paste_selected_settings_pastes_only_chosen_copied_groups() { assert_eq!(s.develop_of(ids[1]).unwrap().color.vibrance, 30.0); } +/// A crop of no size (or a hair wide), as `crop.set` or a sidecar can set it, made the render's +/// samplers read at infinite coordinates and its blurs ask for sigmas of millions of pixels: with a +/// healing spot and masks on, render workers panicked (an i32 square in the blur, a bilinear +/// neighbour past isize::MAX). Such a photo renders, quickly. +#[test] +fn degenerate_crops_render_with_spots_and_masks() { + for rect in [[0.5, 0.5, 0.5, 0.5], [0.0, 0.0, 1e-9, 1.0]] { + let mut s = demo(); + let id = s.active().unwrap(); + s.execute("spot.add", &json!({"points": [[0.5, 0.5]], "size": 0.2, "mode": "heal"})).unwrap(); + s.execute("mask.add", &json!({"kind": "radial", "rx": 0.3, "ry": 0.3})).unwrap(); + s.execute("mask.add", &json!({"kind": "linear"})).unwrap(); + s.execute("develop.set", &json!({"control": "effects.clarity", "value": 80})).unwrap(); + s.execute("develop.set", &json!({"control": "effects.dehaze", "value": 50})).unwrap(); + s.execute("crop.set", &json!({"rect": rect, "angle": 30})).unwrap(); + let t = std::time::Instant::now(); + let r = s.render_now(id, 700, 450).unwrap(); + assert!(r.image.width >= 1 && r.image.height >= 1, "{rect:?}"); + assert!(t.elapsed().as_secs_f64() < 60.0, "{rect:?}: {:?}", t.elapsed()); + } +} + #[test] fn masks_reorder_and_duplicate_and_invert() { let mut s = demo(); diff --git a/crates/pipeline/src/masks.rs b/crates/pipeline/src/masks.rs index 0628a6a6f..14fa558f8 100644 --- a/crates/pipeline/src/masks.rs +++ b/crates/pipeline/src/masks.rs @@ -398,7 +398,9 @@ fn box_down(m: &Image, f: usize) -> Plane { /// Bilinear sample at continuous pixel coordinates (pixel centres at +0.5), edges extended. fn bilinear(p: &Plane, x: f32, y: f32) -> f32 { - let (fx, fy) = (x - 0.5, y - 0.5); + // (held a pixel outside, which reads the same edge: an infinite coordinate reads the edge + // instead of a NaN weight and a neighbour index past isize::MAX) + let (fx, fy) = (x.clamp(-1.0, p.width as f32 + 1.0) - 0.5, y.clamp(-1.0, p.height as f32 + 1.0) - 0.5); let (x0, y0) = (fx.floor(), fy.floor()); let (tx, ty) = (fx - x0, fy - y0); let (x0, y0) = (x0 as isize, y0 as isize); diff --git a/crates/raster/src/lib.rs b/crates/raster/src/lib.rs index 41ae75af8..19fe1f7ff 100644 --- a/crates/raster/src/lib.rs +++ b/crates/raster/src/lib.rs @@ -206,8 +206,11 @@ impl Rgb32f { /// Bilinear sample at continuous pixel coordinates (pixel centres at +0.5). #[inline] pub fn sample_bilinear(&self, x: f32, y: f32) -> [f32; 3] { - let fx = x - 0.5; - let fy = y - 0.5; + // a pixel past the edge reads the edge pixel, so a coordinate held a pixel outside gives + // the same sample; an infinite one (a crop of no size's inverse mapping) then reads the + // edge too, instead of a NaN weight and a neighbour index past isize::MAX + let fx = x.clamp(-1.0, self.width as f32 + 1.0) - 0.5; + let fy = y.clamp(-1.0, self.height as f32 + 1.0) - 0.5; let x0 = fx.floor(); let y0 = fy.floor(); let tx = fx - x0; @@ -258,6 +261,18 @@ mod tests { use super::*; use lightcraft_geom::Orientation; + /// A sample at an infinite coordinate (the inverse mapping of a crop of no size) cast to + /// isize::MAX, and its right / lower neighbour overflowed (a panic where overflow is checked): + /// the neighbours saturate and the edge pixel comes back. + #[test] + fn bilinear_samples_at_infinite_coordinates_read_the_edge() { + let img = Rgb32f::from_fn(3, 2, |x, y| [x as f32, y as f32, 1.0]); + assert_eq!(img.sample_bilinear(f32::INFINITY, f32::INFINITY), [2.0, 1.0, 1.0]); + assert_eq!(img.sample_bilinear(f32::MAX, 0.0), [2.0, 0.0, 1.0]); + assert_eq!(img.sample_bilinear(f32::NEG_INFINITY, f32::NEG_INFINITY), [0.0, 0.0, 1.0]); + assert_eq!(img.sample_bilinear(1.5, 0.5), [1.0, 0.0, 1.0], "an ordinary sample is as before"); + } + #[test] fn crop_and_rotate() { let img = Image::::from_fn(4, 3, |x, y| (y * 10 + x) as u32); diff --git a/docs/whats-new.md b/docs/whats-new.md index 972ef5f5c..bf16c9d5c 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -5,6 +5,8 @@ ### Reliability - A crop a hair wide (or of no size), set by an agent or read from a sidecar, no longer crashes or hangs the render with Clarity, Dehaze, Texture or Highlights / Shadows on: their blurs are held to the picture's size. +- A crop of no size with healing spots or masks no longer crashes the render: samples outside the picture read its + edge. ### Slider drags on large raws - Dragging a slider no longer re-renders the edited photo's filmstrip thumbnail on every frame; it follows once you