From f753778f450d87a0d2f695d10b51ad1d9e4ab5f0 Mon Sep 17 00:00:00 2001 From: Michael Anderson Date: Tue, 13 Oct 2026 07:14:58 -0500 Subject: [PATCH] Never crash: Fix blur overflow and hangs on sliver crops Cap the sigma at twice the picture's long side and square the box width in i64. --- crates/gpu/src/render.rs | 5 +-- crates/pipeline/src/tests.rs | 24 +++++++++++ crates/raster/src/blur.rs | 81 +++++++++++++++++++++++++++++++++--- docs/whats-new.md | 4 ++ 4 files changed, 106 insertions(+), 8 deletions(-) diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 04ae60958..b7fa5e12d 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -245,10 +245,9 @@ fn box_pass(cx: &mut Cx<'_>, horizontal: bool, src: &Buf, dst: &Buf, (w, h): (us /// Gaussian blur of a `w × h` image of `nc` interleaved channels: three box passes each way /// (`lightcraft_raster::blur::gaussian`). pub(crate) fn gaussian(cx: &mut Cx<'_>, src: &Buf, w: usize, h: usize, nc: usize, sigma: f32) -> Buf { - if sigma <= 0.3 || w * h == 0 { + let Some(radii) = lightcraft_raster::blur::radii_for(sigma, w, h) else { return cx.copy(src); - } - let radii = lightcraft_raster::blur::box_radii(sigma); + }; let passes: Vec<(bool, usize)> = radii.iter().map(|r| (true, *r)).chain(radii.iter().map(|r| (false, *r))).filter(|p| p.1 > 0).collect(); if passes.is_empty() { return cx.copy(src); diff --git a/crates/pipeline/src/tests.rs b/crates/pipeline/src/tests.rs index 2398feece..00851fa2a 100644 --- a/crates/pipeline/src/tests.rs +++ b/crates/pipeline/src/tests.rs @@ -711,3 +711,27 @@ fn stored_process_numbers_render_with_a_known_process() { } } } + +/// A crop a hair wide (or of no size, as a command or a sidecar can set) shown to fit made the +/// tone, clarity and dehaze stages blur with sigmas of millions of pixels (infinite for no size): +/// the box width's square overflowed (a panic in debug builds, a radius of billions of pixels and +/// a hung render in release ones). The blur is held to twice the picture's size: the render is +/// quick and its pixels are numbers. +#[test] +fn sliver_crops_render_quickly_with_wide_stages_on() { + let src = scene(); + for (rect, angle) in [([0.0, 0.0, 1e-9, 1.0], 45.0), ([0.5, 0.5, 0.5, 0.5], 0.0), ([0.0, 0.0, 1e-5, 1.0], -30.0)] { + let mut s = DevelopSettings::default(); + s.crop.geometry.rect = lightcraft_geom::Rect::new(rect[0], rect[1], rect[2], rect[3]); + s.crop.geometry.angle = angle; + s.effects.clarity = 80.0; + s.effects.dehaze = 50.0; + s.effects.texture = 40.0; + s.light.highlights = -60.0; + s.light.shadows = 60.0; + let t = std::time::Instant::now(); + let r = render(&src, &SourceInfo::default(), &s, &RenderRequest::fit(280, 180)); + assert!(t.elapsed().as_secs_f64() < 20.0, "{rect:?}: {:?}", t.elapsed()); + assert!(r.image.width >= 1 && r.image.height >= 1, "{rect:?}"); + } +} diff --git a/crates/raster/src/blur.rs b/crates/raster/src/blur.rs index 8deba2d13..33816dffa 100644 --- a/crates/raster/src/blur.rs +++ b/crates/raster/src/blur.rs @@ -3,8 +3,16 @@ use crate::resample::Pixel; use crate::{Image, par_rows}; -/// Box radii approximating a Gaussian of `sigma` with 3 passes (Kovesi / Wells). +/// The largest blur sigma (pixels) the box radii are made for. A box wider than any image (the +/// encoders' limit is 65535 px) blurs to the same flat result, and a larger sigma made the box +/// width's square overflow (from about 23 170 px: a panic where overflow is checked, a garbage +/// radius of billions of pixels, i.e. a hung render, where it isn't). +pub const MAX_SIGMA: f32 = 65_536.0; + +/// Box radii approximating a Gaussian of `sigma` with 3 passes (Kovesi / Wells). A sigma that is +/// not a number counts as 0 (no blur); one above [`MAX_SIGMA`] (infinity too) as `MAX_SIGMA`. pub fn box_radii(sigma: f32) -> [usize; 3] { + let sigma = if sigma.is_nan() { 0.0 } else { sigma.clamp(0.0, MAX_SIGMA) }; let n = 3.0f32; let w_ideal = (12.0 * sigma * sigma / n + 1.0).sqrt(); let mut wl = w_ideal.floor() as i32; @@ -12,7 +20,8 @@ pub fn box_radii(sigma: f32) -> [usize; 3] { wl -= 1; } let wu = wl + 2; - let m_ideal = (12.0 * sigma * sigma - n * (wl * wl) as f32 - 4.0 * n * wl as f32 - 3.0 * n) / (-4.0 * wl as f32 - 4.0); + // (the square in i64: it passes i32 from a box 46 341 px wide) + let m_ideal = (12.0 * sigma * sigma - n * (i64::from(wl) * i64::from(wl)) as f32 - 4.0 * n * wl as f32 - 3.0 * n) / (-4.0 * wl as f32 - 4.0); let m = m_ideal.round() as i32; std::array::from_fn(|i| (((if (i as i32) < m { wl } else { wu }) - 1) / 2).max(0) as usize) } @@ -71,12 +80,24 @@ fn box_v(src: &Image, dst: &mut Image, r: usize) { }); } +/// The box radii of [`gaussian`] on a `w × h` image (the GPU renderer's twin uses them too), or +/// `None` when the blur leaves the image as it is: `sigma` at most 0.3 or not a number, or an +/// empty image. A sigma wider than twice the image's long side counts as that: a crop a hair +/// wide shown to fit asked the tone and dehaze stages for sigmas of millions of pixels (infinite +/// for a crop of no size), which blur no flatter than that but cost boxes that wide on every row. +pub fn radii_for(sigma: f32, w: usize, h: usize) -> Option<[usize; 3]> { + if sigma.is_nan() || sigma <= 0.3 || w == 0 || h == 0 { + return None; + } + let cap = (w.max(h) as f32 * 2.0).max(1.0); + Some(box_radii(sigma.min(cap))) +} + /// Blur with Gaussian `sigma` (pixels). `sigma <= 0.3` returns a copy. pub fn gaussian(img: &Image, sigma: f32) -> Image { - if sigma <= 0.3 || img.is_empty() { + let Some(radii) = radii_for(sigma, img.width, img.height) else { return img.clone(); - } - let radii = box_radii(sigma); + }; let mut a = img.clone(); par_rows(&mut a.data, img.width, |_, row| { let mut tmp = Vec::with_capacity(row.len()); @@ -97,6 +118,56 @@ pub fn gaussian(img: &Image, sigma: f32) -> Image { #[cfg(test)] mod tests { + /// The box radii of the formula as it was, for the check below. + fn box_radii_before(sigma: f32) -> [usize; 3] { + let n = 3.0f32; + let w_ideal = (12.0 * sigma * sigma / n + 1.0).sqrt(); + let mut wl = w_ideal.floor() as i32; + if wl % 2 == 0 { + wl -= 1; + } + let wu = wl + 2; + let m_ideal = (12.0 * sigma * sigma - n * (wl * wl) as f32 - 4.0 * n * wl as f32 - 3.0 * n) / (-4.0 * wl as f32 - 4.0); + let m = m_ideal.round() as i32; + std::array::from_fn(|i| (((if (i as i32) < m { wl } else { wu }) - 1) / 2).max(0) as usize) + } + + /// A blur sigma from a degenerate edit or a deep zoom (found by a UI campaign: a render + /// worker's blur with a huge sigma) squared the box width past i32, which panics in a debug + /// build and gave a radius of billions of pixels in a release one. Huge, infinite and NaN + /// sigmas give bounded radii, and a blur with them is quick and stays finite; every sigma that + /// worked before keeps exactly its radii. + #[test] + fn huge_and_non_finite_sigmas_give_bounded_radii() { + for sigma in [26_800.0, 46_341.0, 1e5, 1e9, f32::MAX, f32::INFINITY, f32::NEG_INFINITY, f32::NAN, -5.0] { + let r = box_radii(sigma); + assert!(r.iter().all(|&r| r <= MAX_SIGMA as usize * 2), "{sigma}: {r:?}"); + } + assert_eq!(box_radii(f32::NAN), [0, 0, 0]); + assert_eq!(box_radii(f32::NEG_INFINITY), [0, 0, 0]); + assert_eq!(box_radii(1e9), box_radii(MAX_SIGMA)); + assert_eq!(box_radii(f32::INFINITY), box_radii(MAX_SIGMA)); + // on an image, the blur is at most twice its long side; NaN and tiny sigmas don't blur + assert_eq!(radii_for(3.1e7, 1400, 900), Some(box_radii(2800.0))); + assert_eq!(radii_for(f32::INFINITY, 1400, 900), Some(box_radii(2800.0))); + assert_eq!(radii_for(40.0, 1400, 900), Some(box_radii(40.0)), "an ordinary sigma is as asked"); + assert_eq!(radii_for(f32::NAN, 1400, 900), None); + assert_eq!(radii_for(0.2, 1400, 900), None); + assert_eq!(radii_for(5.0, 0, 900), None); + let p = crate::Plane { width: 4, height: 3, data: (0..12).map(|i| i as f32).collect() }; + let t = std::time::Instant::now(); + for sigma in [1e9, f32::INFINITY, f32::NAN] { + let b = gaussian(&p, sigma); + assert!(b.data.iter().all(|v| v.is_finite()), "{sigma}"); + } + assert!(t.elapsed().as_secs_f64() < 30.0, "a huge sigma's blur is bounded"); + let mut s = 0.0f32; + while s < 23_000.0 { + assert_eq!(box_radii(s), box_radii_before(s), "sigma {s}"); + s = s * 1.013 + 0.07; + } + } + use super::*; #[test] diff --git a/docs/whats-new.md b/docs/whats-new.md index b5c2ac68c..972ef5f5c 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,10 @@ ## 2026-10-13 +### 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. + ### 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 let go. On a 64 MP raw the first of those renders decoded the whole file again, which held up the loupe's