From 21861c2f142dfed8722ae56a8041e60a0abec5ad Mon Sep 17 00:00:00 2001 From: Atmik Shetty Date: Tue, 13 Oct 2026 16:14:19 +0530 Subject: [PATCH] LR-EDIT-DETAIL-NR: noise reduction works in the original's pixels, so previews match the full-size render scaled down (fixes #960) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Grid thumbnails looked much more denoised than the loupe. `nr_params` sized the radii from the output's scale relative to the source buffer it was given, not the original: a 256-px cell drawn from the 512-px thumbnail level used half the full-size radius on pixels that each average ~23 sensor pixels, and below 0.4x (0.35x for colour) the radius stopped shrinking at all. The guided filter's eps (the variance treated as noise) also stayed at its full-size value although downscaling divides the noise variance by 1/scale², so texture in small previews was flattened as noise. - `SourceInfo::native_long` (filled by the engine from the catalog's photo size, kept when the decoder supplies its own facts, carried to web workers) and `Plan::detail_scale` = output pixels per original pixel (from `px_per_long`, so a crop no longer changes it). - `nr_params(s, scale)`: radii x scale with no floor, eps x scale². Below 1 px the smallest blur the box Gaussian makes is blended in by the ratio of variances (`blur::box_sigma`); a fade under 1% skips the filter. At scale 1 the parameters are unchanged: 1:1 views and full-size exports render as before. The GPU takes the same parameters. - RENDER_CACHE_VERSION 63: cached thumbnails are redrawn. Measured on a 1536x1024 noisy synthetic scene (tests_nr_scale.rs), error of the preview against the full-size render scaled down, as a fraction of leaving NR off, luminance NR 50 at preview scales 0.5 / 0.33 / 0.25 / 0.167 / 0.1 / 0.05: before 0.53 / 0.70 / 1.04 / 2.26 / 3.87 / 6.07, after 0.28 / 0.37 / 0.66 / 0.90 / 0.99 / 1.00. A 256-px render from the 512-px level now matches one from the original (mean 0.07 LSB vs 0.71 before). --- apps/lightcraft-web/src/wire.rs | 6 + crates/engine/src/files.rs | 4 +- crates/engine/src/media.rs | 32 +++- crates/gpu/src/render.rs | 2 +- crates/gpu/tests/equivalence.rs | 20 +++ crates/pipeline/src/lib.rs | 26 +++- crates/pipeline/src/local.rs | 60 ++++++-- crates/pipeline/src/tests_nr_scale.rs | 211 ++++++++++++++++++++++++++ crates/raster/src/blur.rs | 24 +++ docs/parity-checklist.md | 7 +- docs/whats-new.md | 7 + 11 files changed, 375 insertions(+), 24 deletions(-) create mode 100644 crates/pipeline/src/tests_nr_scale.rs diff --git a/apps/lightcraft-web/src/wire.rs b/apps/lightcraft-web/src/wire.rs index b3d685f5f..08f9e898d 100644 --- a/apps/lightcraft-web/src/wire.rs +++ b/apps/lightcraft-web/src/wire.rs @@ -41,6 +41,9 @@ pub struct WireJob { pub as_shot_temp: f64, pub as_shot_tint: f64, pub lens: Option, + /// The original's long edge (`SourceInfo::native_long`). + #[serde(default)] + pub native_long: Option, pub settings: Arc, pub max_w: usize, pub max_h: usize, @@ -80,6 +83,7 @@ impl WireJob { as_shot_temp: job.info.as_shot_temp, as_shot_tint: job.info.as_shot_tint, lens: job.info.lens, + native_long: job.info.native_long, settings: job.settings.clone(), max_w: job.request.max_w, max_h: job.request.max_h, @@ -101,6 +105,7 @@ impl WireJob { lens: self.lens, relative_wb: self.relative_wb, legacy_relative_wb: self.legacy_relative_wb, + native_long: self.native_long, ..Default::default() } } @@ -469,6 +474,7 @@ mod tests { as_shot_temp: 6500.0, as_shot_tint: 0.0, lens: None, + native_long: None, settings: Arc::new(DevelopSettings::default()), max_w: 32, max_h: 32, diff --git a/crates/engine/src/files.rs b/crates/engine/src/files.rs index dc79b8b31..2374eafde 100644 --- a/crates/engine/src/files.rs +++ b/crates/engine/src/files.rs @@ -495,6 +495,8 @@ fn load_bytes_now( camera_tone, mattes, local_tone, + // (the catalog knows the original's size: `media::source_info`) + native_long: None, }, )); } @@ -2077,7 +2079,7 @@ mod tests { assert_eq!(p.kind, MediaKind::Raw, "still a raw file (filters, Convert to DNG…)"); assert!(!p.develops_raw()); // rendered like the JPEG it is: relative white balance, display tone curve, no raw defaults - assert_eq!(crate::media::source_info(&p), SourceInfo::default()); + assert_eq!(SourceInfo { native_long: None, ..crate::media::source_info(&p) }, SourceInfo::default()); assert_eq!(*p.develop, lightcraft_develop::DevelopSettings::default()); assert!(s.render_job(id, 48, 48, false, true).unwrap().run().rendered.is_ok()); // agents see it diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index cf17c3264..9e25e5d6f 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -62,7 +62,7 @@ impl SettingsHashes { } /// Bump when the pipeline's output changes, to invalidate cached thumbnails. -pub const RENDER_CACHE_VERSION: u64 = 62; +pub const RENDER_CACHE_VERSION: u64 = 63; /// Thumbnails render at one of these long edges (so window/cell size changes reuse the cache). pub const THUMB_SIZES: [usize; 4] = [128, 256, 384, 512]; @@ -214,9 +214,13 @@ impl DecodedSource { } /// What to render these pixels against: the decoder's facts, else `header` (the catalog's) - /// with any stored camera tone curve. + /// with any stored camera tone curve. The original's size comes from `header` either way: the + /// decoder only sees the level it decoded. pub fn info_or(&self, header: SourceInfo) -> SourceInfo { - self.info.clone().unwrap_or_else(|| SourceInfo { camera_tone: self.camera_tone.or(header.camera_tone), ..header }) + match &self.info { + Some(info) => SourceInfo { native_long: info.native_long.or(header.native_long), ..info.clone() }, + None => SourceInfo { camera_tone: self.camera_tone.or(header.camera_tone), ..header }, + } } } @@ -1096,9 +1100,16 @@ pub fn content_key(p: &Photo) -> String { pub fn source_info(p: &Photo) -> SourceInfo { // Procedural demo scenes are scene-referred HDR (like raw files): use the filmic tone map. + // (They are drawn afresh at every size, so they have no original size.) if matches!(p.source, Source::Demo { .. }) { return SourceInfo { raw: true, ..Default::default() }; } + // the thumbnail and preview levels are smaller copies: noise reduction works in the original's pixels (#960) + let native_long = Some(p.width.max(p.height) as usize).filter(|&n| n > 0); + SourceInfo { native_long, ..header_info(p) } +} + +fn header_info(p: &Photo) -> SourceInfo { // A raw shown from its embedded preview is a rendered (display-referred) JPEG: relative white // balance and the display tone curve, like any other rendered file. if p.develops_raw() { @@ -1695,6 +1706,21 @@ mod thumbnail_hash_tests { mod tests { use super::*; + #[test] + fn renders_know_the_original_size_whichever_level_they_come_from() { + // issue #960: noise reduction works in the original's pixels, so a grid cell drawn from the + // 512-px thumbnail level denoises like a scaled-down export + let p = Photo::new(PhotoId(1), Source::File { path: "/photos/a.ARW".into() }, "a.ARW", "ARW", 6000, 4000, ""); + assert_eq!(source_info(&p).native_long, Some(6000)); + let decoded = SourceInfo { raw: true, as_shot_temp: 4800.0, ..Default::default() }; + let thumb = DecodedSource::new(Arc::new(Rgb32f::new(8, 8)), Some(decoded.clone())); + let info = thumb.info_or(source_info(&p)); + assert_eq!(info, SourceInfo { native_long: Some(6000), ..decoded }, "the decoder's facts, the catalog's size"); + let unknown = Photo::new(PhotoId(2), Source::File { path: "/photos/b.jpg".into() }, "b.jpg", "JPEG", 0, 0, ""); + assert_eq!(source_info(&unknown).native_long, None); + assert_eq!(source_info(&Photo::new(PhotoId(3), Source::Demo { scene: 0 }, "d", "ARW", 600, 400, "")).native_long, None); + } + #[test] fn denoise_mix_is_reserved_before_the_source_enters_the_cache() { let mut plain = Rgb32f::new(8, 8); diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 04ae60958..3c7f56b18 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -823,7 +823,7 @@ fn linear(cx: &mut Cx<'_>, sampled: &Buf, info: &SourceInfo, plan: &Plan<'_>, ho fn denoise(cx: &mut Cx<'_>, img: Buf, plan: &Plan<'_>) -> Buf { let (w, h) = (plan.w, plan.h); let n = w * h; - let (lum, col) = local::nr_params(&plan.settings, plan.src_long, plan.frame.output_long(w, h)); + let (lum, col) = local::nr_params(&plan.settings, plan.detail_scale); let mut img = img; if let Some(nr) = lum { let l = cx.gpu.buffer(n); diff --git a/crates/gpu/tests/equivalence.rs b/crates/gpu/tests/equivalence.rs index 96771a971..fcf222191 100644 --- a/crates/gpu/tests/equivalence.rs +++ b/crates/gpu/tests/equivalence.rs @@ -93,6 +93,26 @@ fn typical(s: &mut DevelopSettings) { s.detail.sharpen_amount = 40.0; } +/// Issue #960: a preview's noise reduction works in the original's pixels (a smaller radius, a +/// smaller `eps`, faded below one pixel). The GPU takes the same parameters (`local::nr_params`). +#[test] +fn preview_noise_reduction_matches() { + if !gpu() { + return; + } + let src = scene(3, 640, 480); + let mut s = DevelopSettings::default(); + s.detail.nr_luminance = 70.0; + s.detail.nr_detail = 30.0; + s.detail.nr_color = 60.0; + s.detail.nr_color_smoothness = 50.0; + // a preview level of a 6000-px original: loupe-, grid- and tiny-thumbnail-sized outputs + let info = SourceInfo { raw: true, native_long: Some(6000), ..Default::default() }; + for (name, edge) in [("NR preview 640 of 6000", 640), ("NR preview 320 of 6000", 320), ("NR preview 128 of 6000", 128)] { + check(name, &src, &info, &s, &RenderRequest::fit(edge, edge)); + } +} + /// A render big enough for the per-pixel stage to run in several bands of rows (and the render /// in many submissions), with grain (position-dependent) and masks. #[test] diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index d94d3a2fc..8553c23f4 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -92,6 +92,11 @@ pub struct SourceInfo { /// The raw's own local tone mapping (DNG `ProfileGainTableMap`), rendered only when the /// photo's Profile option asks for it ([`local_tone`]). pub local_tone: Option>, + /// Long edge (px) of the original photo, when the source handed to the renderer is a smaller + /// copy of it (the app's thumbnail and preview levels); `None`: the source is the original. + /// Noise reduction sizes its radii in the original's pixels ([`Plan::detail_scale`]), so a + /// preview looks like the full-size render scaled down (issue #960). + pub native_long: Option, } impl Default for SourceInfo { @@ -107,6 +112,7 @@ impl Default for SourceInfo { camera_tone: None, mattes: None, local_tone: None, + native_long: None, } } } @@ -360,6 +366,9 @@ pub struct Plan<'a> { pub px_per_long: f64, /// Long edge of the source buffer (px). pub src_long: usize, + /// Output pixels per pixel of the original photo ([`SourceInfo::native_long`], else the source + /// buffer), at most 1: the scale of noise reduction, whose radii are in the original's pixels. + pub detail_scale: f32, /// Key of the resampled source (together with the source buffer's identity). pub geo: u64, /// Key of the white-balanced, retouched, denoised image (and of the planes computed from it). @@ -423,6 +432,9 @@ pub fn plan<'a>(src: &Rgb32f, info: &SourceInfo, s: &'a DevelopSettings, req: &R None => (frame, full_w, full_h), }; let src_long = src.width.max(src.height); + // (an original reported smaller than the source is the source) + let native_long = info.native_long.unwrap_or(0).max(src_long).max(1); + let detail_scale = local::detail_scale(px_per_long, native_long); let geo = hash_of((format!("{frame:?}"), w, h)); let (wb_t, wb_tint) = local::effective_wb(info, s); let eyes = redeye::resolve(src, &s.red_eye, s.orientation, &frame, w, h, px_per_long); @@ -438,8 +450,12 @@ pub fn plan<'a>(src: &Rgb32f, info: &SourceInfo, s: &'a DevelopSettings, req: &R local_tone::enabled(info, s), src_long, )); - let lin_key = hash_of((pre_nr_key, [d.nr_luminance, d.nr_detail, d.nr_color, d.nr_color_detail, d.nr_color_smoothness].map(f64::to_bits))); - Plan { settings, frame, w, h, px_per_long, src_long, geo, lin_key, pre_nr_key, eyes, fixed_air, keep, mattes: info.mattes.clone() } + let lin_key = hash_of(( + pre_nr_key, + [d.nr_luminance, d.nr_detail, d.nr_color, d.nr_color_detail, d.nr_color_smoothness].map(f64::to_bits), + detail_scale.to_bits(), + )); + Plan { settings, frame, w, h, px_per_long, src_long, detail_scale, geo, lin_key, pre_nr_key, eyes, fixed_air, keep, mattes: info.mattes.clone() } } /// Whether the scene-linear stage needs work only the CPU does (defringe, spot removal). @@ -538,7 +554,7 @@ fn render_impl(src: Src<'_>, info: &SourceInfo, s: &DevelopSettings, req: &Rende Src::Shared(a) => a, }; let plan = plan(src_img, info, s, req); - let Plan { ref frame, w, h, px_per_long, src_long, geo, lin_key, .. } = plan; + let Plan { ref frame, w, h, px_per_long, detail_scale, geo, lin_key, .. } = plan; let s = &*plan.settings; let shared = match (&src, cache) { @@ -558,7 +574,7 @@ fn render_impl(src: Src<'_>, info: &SourceInfo, s: &DevelopSettings, req: &Rende // Without a cache the resampled buffer is ours: work on it in place. let mut img = if shared.is_some() { (*sampled).clone() } else { Arc::unwrap_or_clone(sampled.clone()) }; lin_cpu(&mut img, info, &plan); - local::denoise(&mut img, s, src_long, frame.output_long(w, h)); + local::denoise(&mut img, s, detail_scale); Arc::new(img) } }; @@ -713,6 +729,8 @@ mod tests_hdr; #[cfg(test)] mod tests_local; #[cfg(test)] +mod tests_nr_scale; +#[cfg(test)] mod tests_tone; #[cfg(test)] mod tests_window; diff --git a/crates/pipeline/src/local.rs b/crates/pipeline/src/local.rs index ce3994cd8..89d5d51b1 100644 --- a/crates/pipeline/src/local.rs +++ b/crates/pipeline/src/local.rs @@ -306,28 +306,64 @@ pub struct NrColor { pub t: f32, } -/// Noise-reduction parameters at an output long edge of `out_long` px (see [`denoise`]). -pub fn nr_params(s: &DevelopSettings, src_long: usize, out_long: usize) -> (Option, Option) { +/// Output pixels per pixel of the original photo for an output of `px_per_long` pixels per unit +/// of the long edge of an original `native_long` pixels long, at most 1 ([`crate::Plan::detail_scale`]). +pub fn detail_scale(px_per_long: f64, native_long: usize) -> f32 { + let k = (px_per_long / native_long.max(1) as f64) as f32; + if k.is_finite() { k.clamp(0.0, 1.0) } else { 1.0 } +} + +/// Smallest radius (px) noise reduction filters a preview with: [`gaussian`] makes no blur below +/// about 0.58 px and one 3-px box from there ([`lightcraft_raster::blur::box_sigma`]). +const NR_MIN_SIGMA: f32 = 1.0; + +/// Below this fade a preview's noise reduction is not drawn at all (less than 1 % of a 1-px blur). +const NR_MIN_BLEND: f32 = 0.01; + +/// Radius and blend weight that stand in for a blur of `sigma` px: below [`NR_MIN_SIGMA`] the +/// smallest blur the filter makes, blended in by the ratio of their variances, which keeps the +/// smoothing's second moment (how much it softens the finest detail) that of `sigma`. +fn nr_radius(sigma: f32) -> (f32, f32) { + if sigma >= NR_MIN_SIGMA { + return (sigma, 1.0); + } + let made = lightcraft_raster::blur::box_sigma(NR_MIN_SIGMA); + (NR_MIN_SIGMA, (sigma / made).powi(2).min(1.0)) +} + +/// Noise-reduction parameters at `scale` output pixels per pixel of the original +/// ([`crate::Plan::detail_scale`]; see [`denoise`]). Noise lives at the original's pixels, so the +/// radii are the full-size radii times `scale`, and the guided filter's `eps` (the variance it +/// treats as noise) follows the noise variance, which averaging `1/scale × 1/scale` pixels into one +/// multiplies by `scale²`. A radius below [`NR_MIN_SIGMA`] fades instead ([`nr_radius`]): previews look like +/// the full-size render scaled down (issue #960). At `scale` 1 these are the full-size parameters. +pub fn nr_params(s: &DevelopSettings, scale: f32) -> (Option, Option) { let lum = (s.detail.nr_luminance / 100.0) as f32; let col = (s.detail.nr_color / 100.0) as f32; - let scale = (out_long as f32 / src_long.max(1) as f32).clamp(0.05, 1.0); - let l = (lum > 0.0).then(|| { + let scale = if scale.is_finite() { scale.clamp(0.0, 1.0) } else { 1.0 }; + let l = (lum > 0.0).then(|| nr_radius((1.0 + 2.5 * lum) * scale)).filter(|&(_, w)| w >= NR_MIN_BLEND).map(|(sigma, w)| { let detail = (s.detail.nr_detail / 100.0) as f32; - NrLum { sigma: (1.0 + 2.5 * lum) * scale.max(0.4), eps: 0.002 + lum * lum * 0.25 * (1.0 - 0.7 * detail), k: lum.sqrt() } + let eps = ((0.002 + lum * lum * 0.25 * (1.0 - 0.7 * detail)) * scale * scale).max(1e-7); + NrLum { sigma, eps, k: lum.sqrt() * w } }); - let c = (col > 0.0).then(|| { - let sigma = (1.5 + 6.0 * col) * scale.max(0.35) * (1.0 + (s.detail.nr_color_smoothness / 100.0) as f32); + let smooth = 1.0 + (s.detail.nr_color_smoothness / 100.0) as f32; + let c = (col > 0.0).then(|| nr_radius((1.5 + 6.0 * col) * scale * smooth)).filter(|&(_, w)| w >= NR_MIN_BLEND).map(|(sigma, w)| { let keep = (s.detail.nr_color_detail / 100.0) as f32 * 0.5; - NrColor { sigma, t: col * (1.0 - keep) } + NrColor { sigma, t: col * (1.0 - keep) * w } }); (l, c) } /// Noise reduction at output resolution: luminance via an edge-aware self-guided filter on /// log-luminance, colour by blurring chromaticity (rgb / Y) and re-applying the original luminance. -/// Radii scale with how much the source was downsampled (preview noise is already averaged out). -pub fn denoise(img: &mut Rgb32f, s: &DevelopSettings, src_long: usize, out_long: usize) { - let (lum, col) = nr_params(s, src_long, out_long); +/// `scale`: output pixels per pixel of the original ([`nr_params`]). +pub fn denoise(img: &mut Rgb32f, s: &DevelopSettings, scale: f32) { + let (lum, col) = nr_params(s, scale); + apply_nr(img, lum, col); +} + +/// Noise reduction with explicit parameters (see [`denoise`]). +pub fn apply_nr(img: &mut Rgb32f, lum: Option, col: Option) { let w = img.width; if let Some(nr) = lum { let _t = crate::profiling().then(std::time::Instant::now); @@ -641,7 +677,7 @@ mod nr_tests { let (v0, m0) = var(&img); let mut s = DevelopSettings::default(); s.detail.nr_luminance = 80.0; - denoise(&mut img, &s, 64, 64); + denoise(&mut img, &s, 1.0); let (v1, m1) = var(&img); assert!(v1 < v0 * 0.5, "{v0} -> {v1}"); assert!((m1 - m0).abs() < 0.01); diff --git a/crates/pipeline/src/tests_nr_scale.rs b/crates/pipeline/src/tests_nr_scale.rs new file mode 100644 index 000000000..3247422fa --- /dev/null +++ b/crates/pipeline/src/tests_nr_scale.rs @@ -0,0 +1,211 @@ +//! Noise reduction is resolution independent (issue #960): a preview looks like the full-size +//! render scaled down, whatever the preview's size and whichever source level it is drawn from. + +use crate::local::{denoise, log_lum, nr_params}; +use crate::{RenderRequest, SourceInfo, plan, render}; +use lightcraft_develop::DevelopSettings; +use lightcraft_raster::resample::{Filter, resize}; +use lightcraft_raster::{Rgb32f, Rgba8}; + +const W: usize = 1536; +const H: usize = 1024; + +/// Deterministic standard normal samples (xorshift + Box-Muller). +struct Rng(u64); + +impl Rng { + fn uniform(&mut self) -> f32 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 7; + self.0 ^= self.0 << 17; + ((self.0 >> 40) as f32 + 0.5) / (1u64 << 24) as f32 + } + fn normal(&mut self) -> f32 { + let (u, v) = (self.uniform(), self.uniform()); + (-2.0 * u.ln()).sqrt() * (std::f32::consts::TAU * v).cos() + } +} + +/// A `W × H` linear scene: a gradient, an edge and texture from 3 to 750 px wavelength, with white +/// luminance noise of `noise` stops and colour noise. +fn noisy_scene(noise: f32) -> Rgb32f { + let mut rng = Rng(0x9e37_79b9_7f4a_7c15); + let n: Vec<[f32; 4]> = (0..W * H).map(|_| [rng.normal(), rng.normal(), rng.normal(), rng.normal()]).collect(); + Rgb32f::from_fn(W, H, |x, y| { + let r = n[y * W + x]; + let (xf, yf) = (x as f32, y as f32); + let tex = 0.25 * (xf / 120.0).sin() * (yf / 90.0).cos() + + 0.12 * ((xf + yf) / 16.0).sin() + + 0.08 * (xf / 6.0).sin() * (yf / 7.0).sin() + + 0.05 * ((xf - 2.0 * yf) / 3.0).sin(); + let edge = if x > 900 { 1.2 } else { 0.0 }; + let y0 = 0.18 * (-1.5 + 1.5 * yf / H as f32 + tex + edge + noise * r[0]).exp2(); + let c = |v: f32| y0 * (1.0 + 0.3 * noise * v).max(0.05); + [c(r[1]), c(r[2]), c(r[3])] + }) +} + +fn scaled(img: &Rgb32f, s: f32) -> Rgb32f { + resize(img, (W as f32 * s).round() as usize, (H as f32 * s).round() as usize, Filter::Mitchell) +} + +/// RMS difference of log2 luminance (stops). +fn rms_log(a: &Rgb32f, b: &Rgb32f) -> f32 { + let n = a.data.len() as f32; + (a.data.iter().zip(&b.data).map(|(p, q)| (log_lum(*p) - log_lum(*q)).powi(2)).sum::() / n).sqrt() +} + +/// RMS difference of chromaticity (r and b over r + g + b). +fn rms_chroma(a: &Rgb32f, b: &Rgb32f) -> f32 { + let ch = |p: [f32; 3]| { + let s = (p[0] + p[1] + p[2]).max(1e-6); + [p[0] / s, p[2] / s] + }; + let d = |p: [f32; 3], q: [f32; 3]| (ch(p)[0] - ch(q)[0]).powi(2) + (ch(p)[1] - ch(q)[1]).powi(2); + (a.data.iter().zip(&b.data).map(|(p, q)| d(*p, *q)).sum::() / a.data.len() as f32).sqrt() +} + +/// For each preview scale `k`: (k, how far the preview is from the full-size render scaled down, +/// how far it would be with noise reduction left off). +fn preview_error(s: &DevelopSettings, scales: &[f32], metric: fn(&Rgb32f, &Rgb32f) -> f32) -> Vec<(f32, f32, f32)> { + let src = noisy_scene(0.15); + let mut full = src.clone(); + denoise(&mut full, s, 1.0); + scales + .iter() + .map(|&k| { + let reference = scaled(&full, k); + let plain = scaled(&src, k); + let mut preview = plain.clone(); + denoise(&mut preview, s, k); + (k, metric(&preview, &reference), metric(&plain, &reference)) + }) + .collect() +} + +const SCALES: [f32; 6] = [0.5, 0.33, 0.25, 0.167, 0.1, 0.05]; + +#[test] +fn luminance_nr_preview_matches_full_size_scaled_down() { + let mut s = DevelopSettings::default(); + s.detail.nr_luminance = 50.0; + s.detail.nr_detail = 50.0; + s.detail.nr_color = 0.0; + for (k, err, off) in preview_error(&s, &SCALES, rms_log) { + let ratio = err / off; + eprintln!("luminance NR, preview at {k}: {err:.4} stops from the full-size render scaled down ({ratio:.2} of NR off)"); + // Before #960 the radius stopped shrinking at 0.4×: 2.3× (at 0.167) to 6× (at 0.05) further + // off than no noise reduction at all, and 0.53 / 0.70 of it at 0.5 / 0.33. + assert!(ratio <= 1.0, "preview at {k} is further off than no noise reduction: {err} vs {off}"); + assert!(err <= 0.01, "preview at {k} is {err} stops off"); + if k >= 0.33 { + assert!(ratio <= 0.4, "preview at {k}: {ratio}"); + } + } +} + +#[test] +fn colour_nr_preview_matches_full_size_scaled_down() { + let mut s = DevelopSettings::default(); + s.detail.nr_luminance = 0.0; + s.detail.nr_color = 50.0; + s.detail.nr_color_detail = 50.0; + s.detail.nr_color_smoothness = 50.0; + for (k, err, off) in preview_error(&s, &SCALES, rms_chroma) { + let ratio = err / off; + eprintln!("colour NR, preview at {k}: {err:.6} from the full-size render scaled down ({ratio:.2} of NR off)"); + // before #960: 0.00020 (0.31 of NR off) at 0.167, 0.00022 (0.75) at 0.1 + assert!(err <= 1.2e-4, "preview at {k} is {err} off"); + if k >= 0.1 { + assert!(ratio <= 0.4, "preview at {k}: {ratio}"); + } + } +} + +#[test] +fn full_size_parameters_unchanged() { + // full-size renders and exports keep their look: these are the parameters from before #960 + let mut s = DevelopSettings::default(); + s.detail.nr_luminance = 50.0; + s.detail.nr_detail = 50.0; + s.detail.nr_color = 25.0; + s.detail.nr_color_detail = 50.0; + s.detail.nr_color_smoothness = 50.0; + let (l, c) = nr_params(&s, 1.0); + let (l, c) = (l.unwrap(), c.unwrap()); + assert_eq!(l.sigma, 1.0 + 2.5 * 0.5); + assert_eq!(l.eps, 0.002 + 0.5 * 0.5 * 0.25 * (1.0 - 0.7 * 0.5)); + assert_eq!(l.k, 0.5f32.sqrt()); + assert_eq!(c.sigma, (1.5 + 6.0 * 0.25) * 1.5); + assert_eq!(c.t, 0.25 * (1.0 - 0.25)); +} + +#[test] +fn tiny_previews_skip_noise_reduction_and_hostile_scales_are_safe() { + let mut s = DevelopSettings::default(); + s.detail.nr_luminance = 100.0; + s.detail.nr_color = 100.0; + // a 128-px thumbnail of a 6000-px photo: the luminance radius is a few hundredths of a pixel, + // the colour radius (wider) a quarter of one, so that fades to a tenth + let (l, c) = nr_params(&s, 128.0 / 6000.0); + assert_eq!(l, None); + let (c, full) = (c.unwrap(), nr_params(&s, 1.0).1.unwrap()); + assert!(c.t < full.t * 0.1 && c.t > 0.0, "{c:?}"); + assert_eq!(nr_params(&s, 0.0), (None, None)); + for k in [f32::NAN, f32::INFINITY, -1.0, 7.0] { + let (l, c) = nr_params(&s, k); + let vals = [l.map(|l| [l.sigma, l.eps, l.k]), c.map(|c| [c.sigma, c.t, 1.0])]; + for v in vals.into_iter().flatten().flatten() { + assert!(v.is_finite() && v >= 0.0, "{k}: {v}"); + } + } +} + +fn mean_abs_diff(a: &Rgba8, b: &Rgba8) -> f32 { + let px = |p: &[u8; 4], q: &[u8; 4]| (0..3).map(|i| (f32::from(p[i]) - f32::from(q[i])).abs()).sum::() / 3.0; + a.data.iter().zip(&b.data).map(|(p, q)| px(p, q)).sum::() / a.data.len() as f32 +} + +#[test] +fn preview_from_a_smaller_source_level_matches_one_from_the_original() { + // The app draws a grid cell from the 512-px thumbnail level and the loupe from the 2560-px + // preview level. Told the original's size, a render from the smaller copy denoises like one from + // the original at the same output size; before #960 its radii were in the copy's pixels. + let mut s = DevelopSettings::default(); + s.detail.nr_luminance = 60.0; + s.detail.nr_color = 40.0; + let original = noisy_scene(0.15); + let thumb_level = scaled(&original, 512.0 / W as f32); + let req = RenderRequest::fit(256, 256); + let raw = SourceInfo { raw: true, ..Default::default() }; + let told = SourceInfo { native_long: Some(W), ..raw.clone() }; + assert!((plan(&thumb_level, &told, &s, &req).detail_scale - 256.0 / W as f32).abs() < 1e-4); + assert!((plan(&original, &raw, &s, &req).detail_scale - 256.0 / W as f32).abs() < 1e-4); + assert!((plan(&thumb_level, &raw, &s, &req).detail_scale - 0.5).abs() < 1e-4, "original unknown: the source's pixels"); + + let mut plain = s.clone(); + plain.detail.nr_luminance = 0.0; + plain.detail.nr_color = 0.0; + let from_original = render(&original, &raw, &s, &req).image; + let told_diff = mean_abs_diff(&render(&thumb_level, &told, &s, &req).image, &from_original); + let untold_diff = mean_abs_diff(&render(&thumb_level, &raw, &s, &req).image, &from_original); + // (the two sources differ a little anyway: the thumbnail level is resampled twice) + let resampling = mean_abs_diff(&render(&thumb_level, &told, &plain, &req).image, &render(&original, &raw, &plain, &req).image); + eprintln!("mean |Δ| (8-bit) from the original's render: told {told_diff:.3}, not told {untold_diff:.3}, NR off {resampling:.3}"); + assert!(told_diff <= resampling + 0.1, "{told_diff} vs {resampling}"); + assert!(untold_diff > told_diff + 0.3, "the copy's own pixels change the result: {untold_diff} vs {told_diff}"); +} + +#[test] +fn crop_does_not_change_the_noise_reduction_scale() { + // a 1:1 render of a crop is at the original's pixels, like a 1:1 render of the whole photo + let s = DevelopSettings::default(); + let src = Rgb32f::filled(400, 200, [0.2; 3]); + let info = SourceInfo::default(); + assert!((plan(&src, &info, &s, &RenderRequest::fit(400, 200)).detail_scale - 1.0).abs() < 1e-6); + let mut cropped = s.clone(); + cropped.crop.geometry.rect = lightcraft_geom::Rect::new(0.25, 0.25, 0.75, 0.75); + let p = plan(&src, &info, &cropped, &RenderRequest::fit(200, 100)); + assert_eq!((p.w, p.h), (200, 100)); + assert!((p.detail_scale - 1.0).abs() < 1e-6, "{}", p.detail_scale); +} diff --git a/crates/raster/src/blur.rs b/crates/raster/src/blur.rs index 8deba2d13..9d954a814 100644 --- a/crates/raster/src/blur.rs +++ b/crates/raster/src/blur.rs @@ -17,6 +17,16 @@ pub fn box_radii(sigma: f32) -> [usize; 3] { std::array::from_fn(|i| (((if (i as i32) < m { wl } else { wu }) - 1) / 2).max(0) as usize) } +/// Standard deviation (px) of the blur [`gaussian`] actually makes for `sigma`: its boxes are +/// whole pixels wide, so small radii snap to a few steps (0 below about 0.58 px, then √(2/3) px for one +/// 3-px box per axis, …). +pub fn box_sigma(sigma: f32) -> f32 { + if sigma.is_nan() || sigma <= 0.3 { + return 0.0; + } + box_radii(sigma).iter().map(|&r| (((2 * r + 1) * (2 * r + 1) - 1) as f32) / 12.0).sum::().sqrt() +} + /// One horizontal box pass over `row` (clamped edges), using `tmp` as scratch. #[inline] fn box_row(row: &mut [T], tmp: &mut Vec, r: usize) { @@ -114,6 +124,20 @@ mod tests { assert!((b.get(50, 45) - b.get(50, 55)).abs() < 1e-6); } + #[test] + fn box_sigma_is_the_blur_made() { + assert_eq!(box_sigma(0.3), 0.0); + assert_eq!(box_sigma(0.5), 0.0, "no box wider than 1 px"); + assert!((box_sigma(1.0) - (2.0f32 / 3.0).sqrt()).abs() < 1e-6); + for sigma in [1.0f32, 2.5, 8.0] { + let mut imp = Image::::new(201, 1); + imp.set(100, 0, 1.0); + let b = gaussian(&imp, sigma); + let var: f32 = b.data.iter().enumerate().map(|(i, v)| v * ((i as f32 - 100.0).powi(2))).sum(); + assert!((var.sqrt() - box_sigma(sigma)).abs() < 1e-3, "{sigma}: {} vs {}", var.sqrt(), box_sigma(sigma)); + } + } + #[test] fn variance_close_to_sigma_squared() { let mut imp = Image::::new(201, 1); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 76d91ba3e..a15e8cc92 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (LR-EDIT-DETAIL-NR, LR-EDIT-DETAIL-CNR: noise reduction no longer stronger in the grid than in the loupe (#960); LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -204,8 +204,8 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-EFFECTS-VIGNETTE | Post-crop vignette | P0 | ✅ | `ctl:vignette.*`, `crates/pipeline/src/finish.rs`, `crates/ui-egui/src/panels/edit.rs` | style picker (Highlight / Color / Paint) in the Effects section | | LR-EDIT-EFFECTS-GRAIN | Grain | P1 | ✅ | `ctl:grain.*` | | | LR-EDIT-DETAIL-SHARPEN | Sharpening | P0 | ✅ | `ctl:detail.sharpenAmount`, `ctl:detail.sharpenRadius`, `ctl:detail.sharpenDetail`, `ctl:detail.sharpenMasking` | no Alt-drag mask preview | -| LR-EDIT-DETAIL-NR | Luminance noise reduction | P0 | ✅ | `ctl:detail.nrLuminance`, `ctl:detail.nrDetail`, `ctl:detail.nrContrast` | | -| LR-EDIT-DETAIL-CNR | Colour noise reduction | P0 | ✅ | `ctl:detail.nrColor`, `ctl:detail.nrColorDetail`, `ctl:detail.nrColorSmoothness` | | +| LR-EDIT-DETAIL-NR | Luminance noise reduction | P0 | ✅ | `ctl:detail.nrLuminance`, `ctl:detail.nrDetail`, `ctl:detail.nrContrast` | radii in the original's pixels, so grid, loupe and smaller exports look like the full-size render scaled down (`local::nr_params`, `Plan::detail_scale`, #960) | +| LR-EDIT-DETAIL-CNR | Colour noise reduction | P0 | ✅ | `ctl:detail.nrColor`, `ctl:detail.nrColorDetail`, `ctl:detail.nrColorSmoothness` | preview-size independent like luminance NR (#960) | | LR-EDIT-DETAIL-DENOISE | AI denoise | P2 | 🟡 | cmd:denoise.toggle, cmd:denoise.models.install, cmd:denoise.queue, ctl:enhance.denoise, `crates/denoise/`, `docs/denoise.md` | non-destructive Bayer RAW, opt-in weights, pure-Rust CPU/GPU; GPU convolution uses independently written shared scalars (Metal equivalence regression, issue #479); tiles can also run on another computer's graphics card (`lightcraft-cli denoise-serve`, `crates/denoise/src/remote.rs`); X-Trans/linear RGB, model policy and Lightroom fidelity remain; closing the model setup without installing turns the switch back off (#922) | | LR-EDIT-DETAIL-RAWDETAILS | Improved demosaic toggle | P2 | ⬜ | | | | LR-EDIT-DETAIL-SUPERRES | Super resolution | P2 | ⬜ | | | @@ -789,6 +789,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | LR-EDIT-DETAIL-NR, LR-EDIT-DETAIL-CNR: noise-reduction radii in the original's pixels, so previews at any size and smaller exports look like the full-size render scaled down (issue #960); status unchanged | | 2026-10-11 | minor | LR-MERGE-HDR: HDR and panorama merges of raws without colour matrices (CR2, NEF, ARW…) write the colour they open with as DNG tags (issue #965); status unchanged | | 2026-10-12 | minor | LR-META-XMP: Read Metadata from File no longer auto-writes the sidecar it has just read, which added `lc:settings` and hid another app's later `crs:` edits (#934); status unchanged (✅) | | 2026-10-12 | patch | LR-IMP-CAMERA-COVERAGE: CR2 white level from the active area, not the saturated masked rows above it (issue #1272); status unchanged (🟡) | diff --git a/docs/whats-new.md b/docs/whats-new.md index b5c2ac68c..35ac52f5f 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,13 @@ ## 2026-10-13 +### Noise reduction looks the same at every size +- Luminance and colour noise reduction no longer look much stronger in grid thumbnails and the filmstrip than in + the loupe (issue #960). Their radii are now set in the original photo's pixels, so a thumbnail, the fitted loupe + and a smaller export look like the full-size result scaled down; 1:1 views and full-size exports are unchanged. + Like in Lightroom, judge noise reduction at 1:1: at small sizes the noise it removes is mostly averaged away. + Cached thumbnails are redrawn. + ### 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