From 57487875b54127fb663f879e2f49b2d7e1eca3db Mon Sep 17 00:00:00 2001 From: Michael Anderson Date: Tue, 13 Oct 2026 07:47:54 -0500 Subject: [PATCH] Never crash: Cap the spot radius so huge spot sizes render (#1519) Sidecar spot sizes aren't limited to the slider range. Count at most two long edges. --- crates/engine/src/tests.rs | 21 +++++++++++++++++++++ crates/pipeline/src/spots.rs | 18 +++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6aad89b31..6ba14fe3a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1178,3 +1178,24 @@ fn hdr_edits_render_with_their_hdr_histogram() { assert!(r.histogram.above_sdr() > 0.0); assert!(r.deep.is_none(), "the preview itself stays the 8-bit SDR rendition"); } + +/// A Heal or Remove spot whose size (from a sidecar, which isn't held to the slider's range) is far past the photo +/// made the render's radius infinite: its heal blur and the source it reads went with it, overflowing the blur's box +/// radii and the bilinear sampler (issue #1519). Such spots render, quickly, as a spot over the whole frame. +#[test] +fn oversized_spots_render() { + use lightcraft_develop::{Spot, SpotMode}; + for size in [10.0, 1e30, f64::MAX, f64::INFINITY, f64::NAN] { + for mode in [SpotMode::Heal, SpotMode::Remove, SpotMode::Clone] { + let mut s = demo(); + let id = s.active().unwrap(); + let mut dd = (*s.develop_of(id).unwrap_or_default()).clone(); + dd.spots.push(Spot { mode, points: vec![lightcraft_geom::Point::new(0.5, 0.5)], size, ..Default::default() }); + s.set_develop(id, dd, "Remove").unwrap(); + let t = std::time::Instant::now(); + let r = s.render_now(id, 300, 200).unwrap(); + assert!(r.image.width >= 1 && r.image.height >= 1, "{size} {mode:?}"); + assert!(t.elapsed().as_secs_f64() < 60.0, "{size} {mode:?}: {:?}", t.elapsed()); + } + } +} diff --git a/crates/pipeline/src/spots.rs b/crates/pipeline/src/spots.rs index 3bb775df4..1e1c5dabd 100644 --- a/crates/pipeline/src/spots.rs +++ b/crates/pipeline/src/spots.rs @@ -22,6 +22,18 @@ fn smooth(e0: f32, e1: f32, x: f32) -> f32 { t * t * (3.0 - 2.0 * t) } +/// Most a spot's size counts for, in long edges: a size read from a sidecar isn't held to the slider's range, and one +/// far past the picture (or infinite) made the radius, the blur it heals with and the patch it reads from unbounded +/// (issue #1519). Two long edges cover the whole frame from anywhere in it. +const MAX_SPOT_SIZE: f64 = 2.0; + +/// A spot's radius in output pixels, `ppl` output pixels to the long edge: at least a pixel (a size that is not a +/// number too), at most [`MAX_SPOT_SIZE`] long edges. +fn radius_px(size: f64, ppl: f64) -> f64 { + let size = if size.is_nan() { 0.0 } else { size.min(MAX_SPOT_SIZE) }; + (size * ppl).max(1.0) +} + /// Mean colour of `img` in a disc/annulus. fn ring_stats(img: &Rgb32f, c: (f32, f32), r0: f32, r1: f32) -> Option> { let mut out = Vec::new(); @@ -99,7 +111,7 @@ pub fn pick_source( let img = frame.sample(src, w, h); let (to_out, to_norm) = (frame.norm_to_out(w, h), frame.out_to_norm(w, h)); let t = to_out.apply(*spot.points.first()?); - let r = (spot.size * frame.px_per_long(w)).max(1.0) as f32; + let r = radius_px(spot.size, frame.px_per_long(w)) as f32; let avoid = avoid.map(|o| { let (a, b) = (to_out.apply(Point::new(0.0, 0.0)), to_out.apply(o)); ((b.x - a.x) as f32, (b.y - a.y) as f32) @@ -165,7 +177,7 @@ pub fn window_for_reads_checked( let boxes: Vec<([f64; 4], [f64; 4])> = spots .iter() .filter_map(|spot| { - let r = (spot.size * ppl).max(1.0); + let r = radius_px(spot.size, ppl); let pts: Vec<_> = spot.points.iter().map(|p| to_out.apply(*p)).collect(); let first = pts.first()?; let (mut x0, mut y0, mut x1, mut y1) = (first.x, first.y, first.x, first.y); @@ -261,7 +273,7 @@ pub fn apply(img: &mut Rgb32f, spots: &[Spot], frame: &Frame, ppl: f64) { let (w, h) = (img.width, img.height); let to_out = frame.norm_to_out(w, h); for spot in spots { - let r = (spot.size * ppl).max(1.0) as f32; + let r = radius_px(spot.size, ppl) as f32; let pts: Vec<(f32, f32)> = spot .points .iter()