From 70103a7e55f7e4499835ee1dc6097c230ae46af9 Mon Sep 17 00:00:00 2001 From: radialmonster Date: Sat, 10 Oct 2026 03:11:08 -0400 Subject: [PATCH] LR-EDIT-LIGHT-HIGHLIGHTS: keep local contrast under Highlights/Shadows, as process V2 (#632) The highlights/shadows base (fast guided filter of log luminance, eps 0.35) followed thin high-contrast detail, so the per-pixel shift flattened twigs, lines and texture in bright areas (and dark ones under Shadows); Exposure kept them. This changes how every existing edit with Highlights or Shadows renders, so it ships as a new rendering process (docs/process-versions.md): - Process::V2 (version 2) is added and made LATEST. New photos and Reset get it; existing edits stay on V1 until Update to Current Process. Settings saved without the field still load as V1. - From V2 the base is the same guided filter of a separable median of log luminance (radius 0.15 % of the long edge, <= 24 px): fine detail takes its surroundings' shift and keeps its contrast, region steps stay put (no new halos). V1's base is untouched: local::tone_base_detail is 0 under V1 and tone_base then is exactly guided_fast(l, sigma, BASE_EPS), as before. - CPU (local::prepare) and GPU prepare (median_h / median_v kernels, bit for bit with the CPU median) both branch on it via PlaneSigmas::base_detail. - The base plane's cache key is now (sigma, process, median radius) (local::BaseKey), so a warm cache never serves one process the other's base. plane_at is shared with the GPU crate. - finish::base_tone: V2 uses V1's tone map. Tests: V2 keeps twig/local contrast (>= 0.9 of Exposure's) with no halo; V1 plane sigmas have no median and its base equals guided_fast bit for bit; V1 and V2 render alike without Highlights/Shadows; warm-cache renders across V1/V2 switches equal fresh ones (fails if the key drops the process); GPU matches CPU on both processes at 720 and 2400 px, cached and not. Pentax 645D 2401 at 4000 px: main and V1 renders bit-identical (plain, Highlights -100, Shadows +100). Highlights -100 bright rel. detail 0.095 (V1) -> 0.147 (V2), Exposure -1: 0.124; Shadows +100 dark rel. detail 0.126 -> 0.148. RENDER_CACHE_VERSION 54 (adjust at merge). --- crates/develop/src/settings.rs | 8 +- crates/engine/src/media.rs | 2 +- crates/gpu/src/ctx.rs | 6 +- crates/gpu/src/render.rs | 47 ++++-- crates/gpu/src/wgsl/blur.wgsl | 55 +++++++ crates/gpu/tests/equivalence.rs | 46 +++++- crates/pipeline/src/finish.rs | 3 +- crates/pipeline/src/lib.rs | 2 + crates/pipeline/src/local.rs | 124 +++++++++++++-- crates/pipeline/src/tests_tone.rs | 245 ++++++++++++++++++++++++++++++ docs/parity.md | 4 +- docs/process-versions.md | 5 +- docs/whats-new.md | 7 + 13 files changed, 516 insertions(+), 38 deletions(-) create mode 100644 crates/pipeline/src/tests_tone.rs diff --git a/crates/develop/src/settings.rs b/crates/develop/src/settings.rs index 54577bffc..530994bda 100644 --- a/crates/develop/src/settings.rs +++ b/crates/develop/src/settings.rs @@ -81,16 +81,20 @@ impl ProcessVersion { pub enum Process { /// The rendering of LightCraft before process versions existed (2026-10). V1, + /// Highlights and Shadows keep the local contrast of the tones they move: their edge-aware base + /// is taken of a fine median of log luminance (issue #632). + V2, } impl Process { /// Every process, oldest first. - pub const ALL: [Process; 1] = [Process::V1]; - pub const LATEST: Process = Process::V1; + pub const ALL: [Process; 2] = [Process::V1, Process::V2]; + pub const LATEST: Process = Process::V2; pub const fn version(self) -> ProcessVersion { match self { Process::V1 => ProcessVersion(1), + Process::V2 => ProcessVersion(2), } } } diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 6d5b548f1..9c1566d53 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 = 42; +pub const RENDER_CACHE_VERSION: u64 = 54; /// 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]; diff --git a/crates/gpu/src/ctx.rs b/crates/gpu/src/ctx.rs index 2f4abd4d2..03838199d 100644 --- a/crates/gpu/src/ctx.rs +++ b/crates/gpu/src/ctx.rs @@ -29,7 +29,11 @@ const MODULES: &[Module] = &[ ], entries: &["main"], }, - Module { src: include_str!("wgsl/blur.wgsl"), bindings: &[("src", false, "f32"), ("dst", true, "f32")], entries: &["box_h", "box_v"] }, + Module { + src: include_str!("wgsl/blur.wgsl"), + bindings: &[("src", false, "f32"), ("dst", true, "f32")], + entries: &["box_h", "box_v", "median_h", "median_v"], + }, Module { src: include_str!("wgsl/resize.wgsl"), bindings: &[("src", false, "f32"), ("table", false, "u32"), ("dst", true, "f32")], diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 45a5a5f6d..3f8403570 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -8,6 +8,7 @@ use lightcraft_develop::DevelopSettings; use lightcraft_geom::Orientation; use lightcraft_pipeline::finish::{FinishParams, MASK_TERMS, mask_terms}; use lightcraft_pipeline::geometry::SampleMode; +use lightcraft_pipeline::local::plane_at; use lightcraft_pipeline::{Plan, RenderRequest, Rendered, SourceInfo, local}; use lightcraft_raster::resample::Filter; use lightcraft_raster::{Histogram, Plane, Rgb32f, Rgba8}; @@ -40,7 +41,7 @@ struct Entry { struct Planes { key: u64, log_l: Option>, - base: Option<(u32, Arc)>, + base: Option<(local::BaseKey, Arc)>, clarity: Option<(u32, Arc)>, texture: Option<(u32, Arc)>, dark: Option<(u32, Arc, f32)>, @@ -312,6 +313,28 @@ fn guided_coeffs(cx: &mut Cx<'_>, p: &Buf, w: usize, h: usize, sigma: f32, eps: gaussian(cx, &ab, w, h, 2, sigma) } +/// Separable median of a plane (`local::median_hv`). +fn median_hv(cx: &mut Cx<'_>, p: &Buf, w: usize, h: usize, r: usize) -> Buf { + if r == 0 || w * h == 0 { + return cx.copy(p); + } + let pr = [w as u32, h as u32, 1, r.min(local::BASE_DETAIL_MAX) as u32]; + let tmp = cx.gpu.buffer(w * h); + cx.run("median_h", &pr, &[Some(p), Some(&tmp)], groups2(w, h, [16, 16])); + let out = cx.gpu.buffer(w * h); + cx.run("median_v", &pr, &[Some(&tmp), Some(&out)], groups2(w, h, [16, 16])); + out +} + +/// The highlights/shadows base (`local::tone_base`; `detail` 0: V1, no median). +fn tone_base(cx: &mut Cx<'_>, l: &Buf, w: usize, h: usize, sigma: f32, detail: usize) -> Buf { + if detail == 0 { + return guided_fast(cx, l, w, h, sigma, local::BASE_EPS); + } + let m = median_hv(cx, l, w, h, detail); + guided_fast(cx, &m, w, h, sigma, local::BASE_EPS) +} + /// Fast guided filter (`local::guided_fast`): coefficients on a subsampled grid. fn guided_fast(cx: &mut Cx<'_>, p: &Buf, w: usize, h: usize, sigma: f32, eps: f32) -> Buf { let s = local::guided_fast_step(sigma); @@ -805,17 +828,6 @@ struct Prep { air: f32, } -fn plane_at(slot: &mut Option<(u32, Arc)>, sigma: f32, f: impl FnOnce() -> Buf) -> Arc { - match slot { - Some((k, p)) if *k == sigma.to_bits() => p.clone(), - _ => { - let p = Arc::new(f()); - *slot = Some((sigma.to_bits(), p.clone())); - p - } - } -} - /// The spatial planes (`local::prepare`), reusing those in `planes`. fn prepare(cx: &mut Cx<'_>, lin: &Buf, plan: &Plan<'_>, req: &RenderRequest, planes: &mut Planes) -> Prep { let (w, h) = (plan.w, plan.h); @@ -832,11 +844,14 @@ fn prepare(cx: &mut Cx<'_>, lin: &Buf, plan: &Plan<'_>, req: &RenderRequest, pla } }; let base = match sig.base { - Some(sg) => plane_at(&mut planes.base, sg, || guided_fast(cx, &log_l, w, h, sg, local::BASE_EPS)), + Some(sg) => { + let key = local::base_key(&plan.settings, sg, sig.base_detail); + plane_at(&mut planes.base, key, || tone_base(cx, &log_l, w, h, sg, sig.base_detail)) + } None => log_l.clone(), }; - let clarity = sig.clarity.map(|sg| plane_at(&mut planes.clarity, sg, || guided_fast(cx, &log_l, w, h, sg, local::CLARITY_EPS))); - let texture = sig.texture.map(|sg| plane_at(&mut planes.texture, sg, || gaussian(cx, &log_l, w, h, 1, sg))); + let clarity = sig.clarity.map(|sg| plane_at(&mut planes.clarity, sg.to_bits(), || guided_fast(cx, &log_l, w, h, sg, local::CLARITY_EPS))); + let texture = sig.texture.map(|sg| plane_at(&mut planes.texture, sg.to_bits(), || gaussian(cx, &log_l, w, h, 1, sg))); let (dark, air) = match sig.dark { None => (None, 1.0), Some(sg) => match &planes.dark { @@ -858,7 +873,7 @@ fn prepare(cx: &mut Cx<'_>, lin: &Buf, plan: &Plan<'_>, req: &RenderRequest, pla }, }; let chroma = sig.chroma.map(|sg| { - plane_at(&mut planes.chroma, sg, || { + plane_at(&mut planes.chroma, sg.to_bits(), || { let ch = cx.gpu.buffer(n * 3); map(cx, "chroma_k", n, &[], [Some(lin), None, None], &ch); gaussian(cx, &ch, w, h, 3, sg) diff --git a/crates/gpu/src/wgsl/blur.wgsl b/crates/gpu/src/wgsl/blur.wgsl index 576aff09a..b0e28370d 100644 --- a/crates/gpu/src/wgsl/blur.wgsl +++ b/crates/gpu/src/wgsl/blur.wgsl @@ -78,3 +78,58 @@ fn box_v(@builtin(global_invocation_id) g: vec3) { acc = acc + ld(u32(min(i32(y) + r + 1, last)) * w + x, nc) - ld(u32(max(i32(y) - r, 0)) * w + x, nc); } } + +// Separable median of a single-channel plane (`lightcraft_pipeline::local::median_hv`): 2r+1 +// taps (r ≤ 24, clamped edges) along a row (median_h) or a column (median_v), one thread per +// pixel. The median is a selection, so the result matches the CPU's bit for bit. +// P: w, h, nc (unused), r. +fn median_of(n: u32, r: u32, v: ptr>) -> f32 { + // partial selection sort up to the middle + for (var i = 0u; i <= r; i++) { + var m = i; + for (var j = i + 1u; j < n; j++) { + if ((*v)[j] < (*v)[m]) { + m = j; + } + } + let t = (*v)[i]; + (*v)[i] = (*v)[m]; + (*v)[m] = t; + } + return (*v)[r]; +} + +@compute @workgroup_size(16, 16) +fn median_h(@builtin(global_invocation_id) g: vec3) { + let w = pu(0u); + let h = pu(1u); + let r = min(pu(3u), 24u); + if (g.x >= w || g.y >= h) { + return; + } + let n = 2u * r + 1u; + var v: array; + let row = g.y * w; + for (var k = 0u; k < n; k++) { + let x = clamp(i32(g.x + k) - i32(r), 0, i32(w) - 1); + v[k] = src[row + u32(x)]; + } + dst[row + g.x] = median_of(n, r, &v); +} + +@compute @workgroup_size(16, 16) +fn median_v(@builtin(global_invocation_id) g: vec3) { + let w = pu(0u); + let h = pu(1u); + let r = min(pu(3u), 24u); + if (g.x >= w || g.y >= h) { + return; + } + let n = 2u * r + 1u; + var v: array; + for (var k = 0u; k < n; k++) { + let y = clamp(i32(g.y + k) - i32(r), 0, i32(h) - 1); + v[k] = src[u32(y) * w + g.x]; + } + dst[g.y * w + g.x] = median_of(n, r, &v); +} diff --git a/crates/gpu/tests/equivalence.rs b/crates/gpu/tests/equivalence.rs index b10836c37..2f33430fd 100644 --- a/crates/gpu/tests/equivalence.rs +++ b/crates/gpu/tests/equivalence.rs @@ -6,7 +6,9 @@ use std::sync::Arc; -use lightcraft_develop::{BrushStroke, DevelopSettings, Mask, MaskComponent, MaskOp, MaskShape, Spot, Treatment, VignetteStyle, WbMode, Wheel}; +use lightcraft_develop::{ + BrushStroke, DevelopSettings, Mask, MaskComponent, MaskOp, MaskShape, Process, Spot, Treatment, VignetteStyle, WbMode, Wheel, +}; use lightcraft_geom::{Orientation, Point, Rect}; use lightcraft_pipeline::{Quality, RenderRequest, SourceInfo, StageCache, render}; use lightcraft_raster::{Rgb32f, Rgba8}; @@ -550,6 +552,48 @@ fn cached_renders_match_uncached() { } } +/// Issue #632: the Highlights / Shadows base of each process (V1: the edge-aware filter of log +/// luminance; V2: of its fine median, a radius that grows with the render) matches the CPU's, and +/// a warm cache never hands one process the other's base. +#[test] +fn highlights_shadows_base_matches_on_every_process() { + if !gpu() { + return; + } + let raw = SourceInfo { raw: true, ..Default::default() }; + let src = scene(0, 2400, 1600); + let cache = StageCache::default(); + for &(w, h) in &[(720, 480), (2400, 1600)] { + let req = RenderRequest::fit(w, h); + for p in Process::ALL { + let mut s = DevelopSettings { process: p.version(), ..DevelopSettings::default() }; + s.light.highlights = -100.0; + s.light.shadows = 60.0; + check(&format!("{p:?} highlights/shadows {w}"), &src, &raw, &s, &req); + typical(&mut s); + check(&format!("{p:?} typical {w}"), &src, &raw, &s, &req); + s.masks = vec![Mask { + components: vec![MaskComponent { + name: None, + op: MaskOp::Add, + invert: false, + shape: MaskShape::Linear { start: Point::new(0.5, 0.0), end: Point::new(0.5, 0.9) }, + }], + adjust: lightcraft_develop::LocalAdjustments { highlights: -70.0, shadows: 40.0, ..Default::default() }, + ..Default::default() + }]; + check(&format!("{p:?} local highlights {w}"), &src, &raw, &s, &req); + } + for p in [Process::V1, Process::V2, Process::V1, Process::V2] { + let mut s = DevelopSettings { process: p.version(), ..DevelopSettings::default() }; + s.light.highlights = -100.0; + let warm = lightcraft_gpu::render(&src, &raw, &s, &req, Some(&cache)).expect("gpu"); + let fresh = lightcraft_gpu::render(&src, &raw, &s, &req, None).expect("gpu"); + assert_eq!(warm.image, fresh.image, "{p:?} at {w} px, cached"); + } + } +} + #[test] fn overlays_match() { if !gpu() { diff --git a/crates/pipeline/src/finish.rs b/crates/pipeline/src/finish.rs index fce3b8324..c58b99e86 100644 --- a/crates/pipeline/src/finish.rs +++ b/crates/pipeline/src/finish.rs @@ -279,7 +279,8 @@ impl FinishParams { pub fn base_tone(s: &DevelopSettings, info: &SourceInfo, peak: f32) -> ToneMap { let l = &s.light; match s.process.process() { - Process::V1 => match info.camera_tone.as_ref().filter(|_| info.raw && peak <= 1.0) { + // V2 changes the Highlights/Shadows base (`local::tone_base`), not the tone map + Process::V1 | Process::V2 => match info.camera_tone.as_ref().filter(|_| info.raw && peak <= 1.0) { Some(curve) => ToneMap::camera(curve, l.contrast, l.whites, l.blacks), None if info.raw => ToneMap::hdr(l.contrast, l.whites, l.blacks, peak), None => ToneMap::display_hdr(l.contrast, l.whites, l.blacks, peak), diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index f7c33ee9e..9c068e2b7 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -645,4 +645,6 @@ mod tests_hdr; #[cfg(test)] mod tests_local; #[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 fd7fefd34..f51d75cb3 100644 --- a/crates/pipeline/src/local.rs +++ b/crates/pipeline/src/local.rs @@ -3,7 +3,7 @@ use lightcraft_color::cct::{temp_tint_to_xy, wb_matrix}; use lightcraft_color::{REC2020, luminance_2020}; -use lightcraft_develop::DevelopSettings; +use lightcraft_develop::{DevelopSettings, Process}; use lightcraft_raster::blur::gaussian; use std::sync::Arc; @@ -120,6 +120,86 @@ pub fn guided_fast(p: &Plane, sigma: f32, eps: f32) -> Plane { guided_apply(p, &up(&ma), &up(&mb)) } +/// Separable median (`2r+1` px along each row, then each column; clamped edges). It keeps the +/// steps between regions where they are and removes features thinner than about `r` px. +pub fn median_hv(p: &Plane, r: usize) -> Plane { + if r == 0 || p.data.is_empty() { + return p.clone(); + } + let rows = |src: &Plane| { + let mut out = Plane::new(src.width, src.height); + for_rows(&mut out.data, src.width, |y, row| median_row(src.row(y), row, r)); + out + }; + transpose(&rows(&transpose(&rows(p)))) +} + +/// Running median of `src` over `2r+1` taps (clamped edges) into `out`: a sorted window that +/// drops the leaving value and inserts the entering one. +fn median_row(src: &[f32], out: &mut [f32], r: usize) { + let n = src.len(); + if n == 0 { + return; + } + let at = |i: isize| src[i.clamp(0, n as isize - 1) as usize]; + let r_ = r as isize; + let mut win: Vec = (-r_..=r_).map(at).collect(); + win.sort_unstable_by(f32::total_cmp); + for (x, o) in out.iter_mut().enumerate() { + *o = win[r]; + let (old, new) = (at(x as isize - r_), at(x as isize + r_ + 1)); + if old.to_bits() != new.to_bits() { + let i = win.binary_search_by(|v| v.total_cmp(&old)).unwrap_or_else(|i| i.min(win.len() - 1)); + win.remove(i); + let j = win.binary_search_by(|v| v.total_cmp(&new)).unwrap_or_else(|j| j); + win.insert(j, new); + } + } +} + +fn transpose(p: &Plane) -> Plane { + let (w, h) = (p.width, p.height); + let mut t = Plane::new(h, w); + for_rows(&mut t.data, h, |x, row| { + for (y, v) in row.iter_mut().enumerate() { + *v = p.data[y * w + x]; + } + }); + t +} + +/// Detail scale of the highlights/shadows base from process V2, as a fraction of the long edge: +/// the base is the edge-aware filter of the [`median_hv`] of log luminance at this radius, so +/// twigs, lines and fine texture brighter or darker than their surroundings take their +/// surroundings' shift (and keep their contrast, as with Exposure) while region boundaries stay +/// sharp (no halos). V1's base is the edge-aware filter of log luminance itself (issue #632). +pub const BASE_DETAIL: f64 = 0.0015; +/// Largest [`base_detail_radius`] (px). +pub const BASE_DETAIL_MAX: usize = 24; + +/// Median radius (px) of the highlights/shadows base at `px_per_long` (see [`BASE_DETAIL`]). +pub fn base_detail_radius(px_per_long: f64) -> usize { + ((BASE_DETAIL * px_per_long).round().max(0.0) as usize).min(BASE_DETAIL_MAX) +} + +/// Median radius (px) of the highlights/shadows base of `s`'s rendering process at `px_per_long`: +/// 0 (no median) under V1, [`base_detail_radius`] from V2. +pub fn tone_base_detail(s: &DevelopSettings, px_per_long: f64) -> usize { + match s.process.process() { + Process::V1 => 0, + Process::V2 => base_detail_radius(px_per_long), + } +} + +/// The highlights/shadows base layer of log luminance `l`: the fast guided filter ([`BASE_EPS`]) +/// of its [`median_hv`] at `detail` px ([`tone_base_detail`]); at 0, of `l` itself (V1). +pub fn tone_base(l: &Plane, sigma: f32, detail: usize) -> Plane { + if detail == 0 { + return guided_fast(l, sigma, BASE_EPS); + } + guided_fast(&median_hv(l, detail), sigma, BASE_EPS) +} + /// Subsampling step of [`guided_fast`] for `sigma` (1 = no subsampling: the plain guided filter). pub fn guided_fast_step(sigma: f32) -> usize { (sigma / 3.0).floor().clamp(1.0, 16.0) as usize @@ -211,7 +291,8 @@ pub fn log_lum(c: [f32; 3]) -> f32 { pub(crate) struct Planes { pub key: u64, pub log_l: Option>, - pub base: Option<(u32, Arc)>, + /// Keyed by its radius, process and median radius ([`BaseKey`]). + pub base: Option<(BaseKey, Arc)>, pub clarity: Option<(u32, Arc)>, pub texture: Option<(u32, Arc)>, /// Dark channel and its airlight. @@ -220,13 +301,24 @@ pub(crate) struct Planes { pub chroma: Option<(u32, Arc)>, } -/// Reuse `slot` if it was computed at `sigma`, else compute and store it. -fn plane_at(slot: &mut Option<(u32, Arc)>, sigma: f32, f: impl FnOnce() -> Plane) -> Arc { +/// What the highlights/shadows base plane was computed with: its radius (`f32` bits), the +/// rendering process and the median radius ([`tone_base_detail`]). The base differs between +/// processes, so a photo moved to another one (Update to Current Process, undo) recomputes it. +pub type BaseKey = (u32, Process, usize); + +/// The [`BaseKey`] of a base at `sigma` for `s` (`detail`: [`PlaneSigmas::base_detail`]). +pub fn base_key(s: &DevelopSettings, sigma: f32, detail: usize) -> BaseKey { + (sigma.to_bits(), s.process.process(), detail) +} + +/// Reuse `slot` if it was computed with `key` (a radius's `f32` bits, or a [`BaseKey`]), else +/// compute and store it. +pub fn plane_at(slot: &mut Option<(K, Arc

)>, key: K, f: impl FnOnce() -> P) -> Arc

{ match slot { - Some((k, p)) if *k == sigma.to_bits() => p.clone(), + Some((k, p)) if *k == key => p.clone(), _ => { let p = Arc::new(f()); - *slot = Some((sigma.to_bits(), p.clone())); + *slot = Some((key, p.clone())); p } } @@ -248,8 +340,11 @@ pub const CHROMA_SIGMA: f32 = 0.004; /// Radii (px) of the spatial planes the settings need (`None` = not needed). #[derive(Clone, Copy, Debug, Default, PartialEq)] pub struct PlaneSigmas { - /// Edge-aware base for highlights/shadows (fast guided filter, [`BASE_EPS`]). + /// Edge-aware base for highlights/shadows ([`tone_base`]: fast guided filter, [`BASE_EPS`]; from + /// process V2 of the [`BASE_DETAIL`] median). pub base: Option, + /// Median radius (px) of that base ([`tone_base_detail`]: 0 under V1). + pub base_detail: usize, /// Clarity band (fast guided filter, [`CLARITY_EPS`]). pub clarity: Option, /// Texture / sharpening band (Gaussian). @@ -265,7 +360,9 @@ pub fn plane_sigmas(s: &DevelopSettings, px_per_long: f64, q: Quality) -> PlaneS let local_any = |f: fn(&lightcraft_develop::LocalAdjustments) -> f64| s.masks.iter().any(|m| f(&m.adjust) != 0.0); let tone_active = s.light.highlights != 0.0 || s.light.shadows != 0.0 || s.masks.iter().any(|m| m.adjust.highlights != 0.0 || m.adjust.shadows != 0.0); - // Edge-aware base at ~1.5% of the long edge (EV² epsilon: edges of > ~0.6 EV are preserved). + // Edge-aware base at ~1.5% of the long edge (EV² epsilon: edges of > ~0.6 EV are preserved) + // From V2 it is taken of the median at 0.15%: lines and texture finer than that are detail, + // not tone (#632). let base = tone_active.then(|| { let sigma = (0.015 * ppl).max(1.0); if q == Quality::Draft { sigma.min(24.0) } else { sigma } @@ -281,7 +378,8 @@ pub fn plane_sigmas(s: &DevelopSettings, px_per_long: f64, q: Quality) -> PlaneS .then(|| (0.0018 * ppl).max(0.6)); let dark = (s.effects.dehaze != 0.0 || local_any(|a| a.dehaze)).then(|| (0.02 * ppl).max(1.0)); let chroma = (local_any(|a| a.moire) || s.masks.iter().any(|m| m.adjust.noise > 0.0)).then(|| (CHROMA_SIGMA * ppl).max(1.0)); - PlaneSigmas { base, clarity, texture, dark, chroma } + let base_detail = if base.is_some() { tone_base_detail(s, px_per_long) } else { 0 }; + PlaneSigmas { base, base_detail, clarity, texture, dark, chroma } } /// The spatial planes the per-pixel stage needs for `img` (white-balanced, before exposure), @@ -298,7 +396,7 @@ pub(crate) fn prepare( mattes: Option<&masks::Mattes>, ) -> Prepared { let log_l = planes.log_l.get_or_insert_with(|| Arc::new(timed("log_l", || img.map(log_lum)))).clone(); - let PlaneSigmas { base: base_sigma, clarity: clarity_sigma, texture: texture_sigma, dark: dark_sigma, chroma: chroma_sigma } = + let PlaneSigmas { base: base_sigma, base_detail, clarity: clarity_sigma, texture: texture_sigma, dark: dark_sigma, chroma: chroma_sigma } = plane_sigmas(s, px_per_long, q); let Planes { base: sb, clarity: sc, texture: st, dark: sd, chroma: sch, .. } = planes; @@ -312,13 +410,13 @@ pub(crate) fn prepare( }); let l = &log_l; let (base, (clarity_blur, (texture_blur, dark))) = par_join( - || base_sigma.map(|sg| plane_at(sb, sg, || timed("base", || guided_fast(l, sg, BASE_EPS)))), + || base_sigma.map(|sg| plane_at(sb, base_key(s, sg, base_detail), || timed("base", || tone_base(l, sg, base_detail)))), || { par_join( - || clarity_sigma.map(|sg| plane_at(sc, sg, || timed("clarity", || guided_fast(l, sg, CLARITY_EPS)))), + || clarity_sigma.map(|sg| plane_at(sc, sg.to_bits(), || timed("clarity", || guided_fast(l, sg, CLARITY_EPS)))), || { par_join( - || texture_sigma.map(|sg| plane_at(st, sg, || timed("texture", || gaussian(l, sg)))), + || texture_sigma.map(|sg| plane_at(st, sg.to_bits(), || timed("texture", || gaussian(l, sg)))), || { dark_sigma.map(|sg| match sd { Some((k, d, air)) if *k == sg.to_bits() => (d.clone(), *air), diff --git a/crates/pipeline/src/tests_tone.rs b/crates/pipeline/src/tests_tone.rs new file mode 100644 index 000000000..a6ec70eb5 --- /dev/null +++ b/crates/pipeline/src/tests_tone.rs @@ -0,0 +1,245 @@ +//! Highlights / Shadows keep the local contrast of the tones they move (issue #632), from process +//! V2; V1 keeps the base it always had (`docs/process-versions.md`). + +use std::sync::Arc; + +use lightcraft_develop::{DevelopSettings, Process, ProcessVersion, controls}; +use lightcraft_raster::{Plane, Rgb32f, Rgba8}; + +use crate::{Quality, RenderRequest, SourceInfo, StageCache, local, render, render_cached}; + +const W: usize = 1200; +const H: usize = 600; + +/// Left half: a bright sky crossed by 1 px twigs (≈ 2.5 EV darker) and a little texture; right +/// half: a dark ground with 1 px light stalks. The two halves meet at x = W/2 (a strong edge). +fn scene() -> Rgb32f { + Rgb32f::from_fn(W, H, |x, y| { + let t = 1.0 + 0.06 * (((x * 7 + y * 13) % 5) as f32 - 2.0) / 2.0; + let line = on_line(x, y); + let v = if x < W / 2 { + if line { 0.14 } else { 0.85 } + } else if line { + 0.06 + } else { + 0.011 + }; + [v * t; 3] + }) +} + +/// Thin lines in a chequer of 48 px cells: twigs where a branch crosses the sky, open sky between. +fn on_line(x: usize, y: usize) -> bool { + (x / 48 + y / 48).is_multiple_of(2) && (x % 4 == 1 || y % 5 == 2) +} + +/// log2 of (mean luma off the lines / mean luma on them) over columns `x0..x1`: the twigs' (or +/// stalks') contrast against their surroundings. +fn line_contrast(img: &Rgba8, x0: usize, x1: usize) -> f32 { + let (mut on, mut off) = ((0.0f64, 0.0f64), (0.0f64, 0.0f64)); + for y in 40..H - 40 { + for x in x0..x1 { + let v = luma(img.data[y * W + x]) as f64; + let acc = if on_line(x, y) { &mut on } else { &mut off }; + acc.0 += v; + acc.1 += 1.0; + } + } + ((off.0 / off.1) / (on.0 / on.1)).log2().abs() as f32 +} + +fn shot(s: &DevelopSettings) -> Rgba8 { + render(&scene(), &SourceInfo::default(), s, &RenderRequest::fit(W, H)).image +} + +/// Settings on `process`. +fn on(process: Process) -> DevelopSettings { + DevelopSettings { process: process.version(), ..DevelopSettings::default() } +} + +/// V2 settings with control `id` at `v`. +fn with(id: &str, v: f64) -> DevelopSettings { + with_on(Process::V2, id, v) +} + +fn with_on(process: Process, id: &str, v: f64) -> DevelopSettings { + let mut s = on(process); + controls::set(&mut s, id, v); + s +} + +fn luma(p: [u8; 4]) -> f32 { + 0.2126 * p[0] as f32 + 0.7152 * p[1] as f32 + 0.0722 * p[2] as f32 +} + +/// (mean, rel. local contrast: std of luma minus its 9×9 mean, over the mean) of the block +/// `x0..x1 × 40..H-40`. +fn stats(img: &Rgba8, x0: usize, x1: usize) -> (f32, f32) { + let l = |x: usize, y: usize| luma(img.data[y * W + x]); + let (mut sum, mut hp, mut n) = (0.0f64, Vec::new(), 0.0f64); + for y in 40..H - 40 { + for x in x0..x1 { + let mut m = 0.0; + for dy in 0..9 { + for dx in 0..9 { + m += l(x + dx - 4, y + dy - 4); + } + } + sum += l(x, y) as f64; + hp.push(l(x, y) - m / 81.0); + n += 1.0; + } + } + let mean = (sum / n) as f32; + let var = hp.iter().map(|v| v * v).sum::() / hp.len() as f32; + (mean, var.sqrt() / mean) +} + +#[test] +fn highlights_keep_the_local_contrast_of_bright_areas() { + let (plain, hl, exp) = (shot(&on(Process::V2)), shot(&with("light.highlights", -100.0)), shot(&with("light.exposure", -1.0))); + let (bright, flat_dark) = ((40, 540), (700, 1160)); + let ((m0, c0), (mh, ch), (_, ce)) = (stats(&plain, bright.0, bright.1), stats(&hl, bright.0, bright.1), stats(&exp, bright.0, bright.1)); + let (k0, kh, ke) = (line_contrast(&plain, bright.0, bright.1), line_contrast(&hl, bright.0, bright.1), line_contrast(&exp, bright.0, bright.1)); + // the sky comes down as much as with −1 EV or more… + assert!(mh < m0 - 25.0, "highlights −100 mean {mh} vs {m0}"); + // …and keeps its twigs as −1 EV does (the edge-aware base alone followed them: 0.73 of + // Exposure's twig contrast and 0.79 of its local contrast were left) + assert!(kh > 0.9 * ke && kh > 0.9 * k0, "highlights −100 twig contrast {kh} EV vs exposure −1 {ke}, plain {k0}"); + assert!(ch > 0.9 * ce, "highlights −100 local contrast {ch} vs exposure −1 {ce}, plain {c0}"); + // the dark half is not touched + let (d0, dh) = (stats(&plain, flat_dark.0, flat_dark.1).0, stats(&hl, flat_dark.0, flat_dark.1).0); + assert!((dh - d0).abs() < 1.5, "dark half {dh} vs {d0}"); +} + +#[test] +fn shadows_keep_the_local_contrast_of_dark_areas() { + let (plain, sh, exp) = (shot(&on(Process::V2)), shot(&with("light.shadows", 100.0)), shot(&with("light.exposure", 1.0))); + let dark = (640, 1160); + let ((m0, _), (ms, cs), (_, ce)) = (stats(&plain, dark.0, dark.1), stats(&sh, dark.0, dark.1), stats(&exp, dark.0, dark.1)); + let (k0, ks, ke) = (line_contrast(&plain, dark.0, dark.1), line_contrast(&sh, dark.0, dark.1), line_contrast(&exp, dark.0, dark.1)); + assert!(ms > m0 + 10.0, "shadows +100 mean {ms} vs {m0}"); + // (the edge-aware base alone: 0.75 of Exposure's stalk contrast, 0.72 of its local contrast) + assert!(ks > 0.9 * ke, "shadows +100 stalk contrast {ks} EV vs exposure +1 {ke}, plain {k0}"); + assert!(cs > 0.9 * ce, "shadows +100 local contrast {cs} vs exposure +1 {ce}"); + // the bright half is not lifted + let (b0, bs) = (stats(&plain, 40, 540).0, stats(&sh, 40, 540).0); + assert!((bs - b0).abs() < 1.5, "bright half {bs} vs {b0}"); +} + +#[test] +fn highlights_add_no_halo_at_a_strong_edge() { + // a flat bright field against a flat dark one: the bright pixels next to the edge come down + // (nearly) as much as those far from it, the dark ones stay where they are (the guided + // filter's own softness leaves up to 4 levels within a few px of the edge, as before) + let src = Rgb32f::from_fn(W, H, |x, _| if x < W / 2 { [0.85; 3] } else { [0.011; 3] }); + let shot = |s: &DevelopSettings| render(&src, &SourceInfo::default(), s, &RenderRequest::fit(W, H)).image; + let (plain, hl) = (shot(&on(Process::V2)), shot(&with("light.highlights", -100.0))); + let at = |img: &Rgba8, x: usize| luma(img.data[(H / 2) * W + x]); + let far = at(&hl, 100) - at(&plain, 100); + for x in (W / 2 - 40..W / 2).chain(W / 2..W / 2 + 40) { + let d = at(&hl, x) - at(&plain, x); + let want = if x < W / 2 { far } else { 0.0 }; + assert!((d - want).abs() <= 5.0, "x {x}: change {d}, far from the edge {want}"); + } +} + +#[test] +fn zero_highlights_and_shadows_render_without_the_base() { + let mut s = on(Process::V2); + s.light.highlights = 0.0; + s.light.shadows = 0.0; + controls::set(&mut s, "effects.clarity", 40.0); + // no base plane is computed or read (the per-pixel stage sees log luminance): the render is + // the one it always was + assert_eq!(local::plane_sigmas(&s, 1200.0, Quality::Full).base, None); + assert!(local::plane_sigmas(&with("light.shadows", 1.0), 1200.0, Quality::Full).base.is_some()); +} + +#[test] +fn median_keeps_steps_and_drops_thin_lines() { + let p = Plane::from_fn(40, 30, |x, y| if x < 20 { -2.0 } else { 2.0 } + if y == 15 || x == 8 { -3.0 } else { 0.0 }); + let m = local::median_hv(&p, 2); + for y in 0..30 { + for x in 0..40 { + let want = if x < 20 { -2.0 } else { 2.0 }; + assert_eq!(m.get(x, y), want, "({x}, {y})"); + } + } + assert_eq!(local::median_hv(&p, 0).data, p.data); + assert_eq!(local::base_detail_radius(5568.0), 8); + assert_eq!(local::base_detail_radius(300.0), 0); + assert_eq!(local::base_detail_radius(1e9), local::BASE_DETAIL_MAX); +} + +#[test] +fn median_matches_a_plain_per_pixel_median() { + let p = + Plane::from_fn(37, 23, |x, y| (((x * 31 + y * 17) * 2_654_435_761usize) % 1000) as f32 / 250.0 - 2.0 + if x % 5 == 0 { 0.5 } else { 0.0 }); + for r in [1, 2, 4, 9] { + let med = |v: &mut Vec| { + v.sort_by(f32::total_cmp); + v[r] + }; + let rows = Plane::from_fn(37, 23, |x, y| med(&mut (0..=2 * r).map(|k| p.get((x + k).saturating_sub(r).min(36), y)).collect())); + let want = Plane::from_fn(37, 23, |x, y| med(&mut (0..=2 * r).map(|k| rows.get(x, (y + k).saturating_sub(r).min(22))).collect())); + assert_eq!(local::median_hv(&p, r).data, want.data, "r {r}"); + } +} + +#[test] +fn v1_keeps_the_base_it_always_had() { + // V1's base is the edge-aware filter of log luminance itself, as before process V2: no median + // radius, and `tone_base` at 0 is that filter bit for bit + let v1 = with_on(Process::V1, "light.highlights", -100.0); + assert_eq!(ProcessVersion::legacy().process(), Process::V1); + for ppl in [300.0, 1200.0, 5568.0, 1e6] { + for q in [Quality::Full, Quality::Draft] { + let sig = local::plane_sigmas(&v1, ppl, q); + assert!(sig.base.is_some()); + assert_eq!(sig.base_detail, 0, "V1 at {ppl} px"); + assert_eq!(local::tone_base_detail(&v1, ppl), 0); + } + } + let l = + Plane::from_fn(301, 157, |x, y| (((x * 31 + y * 17) * 2_654_435_761usize) % 1000) as f32 / 250.0 - 2.0 + if x % 5 == 0 { 0.5 } else { 0.0 }); + for sg in [1.0f32, 4.5, 18.0] { + assert_eq!(local::tone_base(&l, sg, 0).data, local::guided_fast(&l, sg, local::BASE_EPS).data, "sigma {sg}"); + } + // V2 takes the median wherever the image is big enough for one + let v2 = with("light.highlights", -100.0); + assert_eq!(local::plane_sigmas(&v2, 5568.0, Quality::Full).base_detail, local::base_detail_radius(5568.0)); + assert_eq!(local::plane_sigmas(&v2, 300.0, Quality::Full).base_detail, 0); +} + +#[test] +fn only_v2_keeps_the_twigs_under_highlights() { + let bright = (40, 540); + let (p1, p2) = (shot(&on(Process::V1)), shot(&on(Process::V2))); + // without Highlights / Shadows the processes render alike + assert!(p1 == p2, "plain renders differ between V1 and V2"); + let (h1, h2, e) = + (shot(&with_on(Process::V1, "light.highlights", -100.0)), shot(&with("light.highlights", -100.0)), shot(&with("light.exposure", -1.0))); + let (k1, k2, ke) = (line_contrast(&h1, bright.0, bright.1), line_contrast(&h2, bright.0, bright.1), line_contrast(&e, bright.0, bright.1)); + // V1 is the base from before (≈ 0.73 of Exposure's twig contrast), V2 keeps them + assert!(k1 < 0.85 * ke && k2 > 0.9 * ke, "twig contrast V1 {k1} EV, V2 {k2} EV, exposure −1 {ke} EV"); +} + +#[test] +fn the_base_plane_is_cached_per_process() { + let s1 = with_on(Process::V1, "light.highlights", -100.0); + let s2 = with("light.highlights", -100.0); + // moving a photo between processes (Update to Current Process, then undo) with warm caches + // renders what a fresh render does + let src = Arc::new(scene()); + let (info, req) = (SourceInfo::default(), RenderRequest::fit(W, H)); + let cache = StageCache::default(); + for s in [&s1, &s2, &s1, &s2] { + let warm = render_cached(&src, &info, s, &req, &cache).image; + assert!(warm == render(&src, &info, s, &req).image, "process {:?}", s.process); + } + let sig = local::plane_sigmas(&s2, W as f64, Quality::Full); + let sg = sig.base.unwrap(); + assert_ne!(local::base_key(&s1, sg, 0), local::base_key(&s2, sg, 0), "the process is part of the key"); + assert_ne!(local::base_key(&s2, sg, 0), local::base_key(&s2, sg, sig.base_detail)); +} diff --git a/docs/parity.md b/docs/parity.md index d0bdc35c2..8ce93a287 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -244,8 +244,8 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-EDIT-HDR-MODE | HDR editing | P2 | ✅ | `cmd:develop.hdr`, `ctl:hdr.maxEv`, `crates/ui-egui/src/panels/edit.rs` (`hdr_row`) | HDR switch in the Light panel; see Q. HDR | | LR-EDIT-LIGHT-EXPOSURE | Exposure | P0 | ✅ | `ctl:light.exposure` | | | LR-EDIT-LIGHT-CONTRAST | Contrast | P0 | ✅ | `ctl:light.contrast` | | -| LR-EDIT-LIGHT-HIGHLIGHTS | Highlights | P0 | ✅ | `ctl:light.highlights`, `crates/raw/src/highlight.rs` | sensor-clipped highlights stay neutral when Highlights (or Exposure, Whites) darkens them: fully clipped content is rebuilt to the neutral of the colour model applied after it, partly clipped content takes its colour from reliable unclipped content nearby (not from dark objects, lens fringes or another surface beside it) and white where there is none, and pale partly clipped content (a white sky) turns white on its way to clipping (#523); binned previews and thumbnails decide clipping per sample before averaging (`develop_binned_masked`, `highlight::reconstruct_masked`), so a block with one clipped specular sample keeps its mean instead of jumping to the clip level, which reconstruction turned into green speckles at 500 px (#548) | -| LR-EDIT-LIGHT-SHADOWS | Shadows | P0 | ✅ | `ctl:light.shadows` | | +| LR-EDIT-LIGHT-HIGHLIGHTS | Highlights | P0 | ✅ | `ctl:light.highlights`, `crates/raw/src/highlight.rs` | sensor-clipped highlights stay neutral when Highlights (or Exposure, Whites) darkens them: fully clipped content is rebuilt to the neutral of the colour model applied after it, partly clipped content takes its colour from reliable unclipped content nearby (not from dark objects, lens fringes or another surface beside it) and white where there is none, and pale partly clipped content (a white sky) turns white on its way to clipping (#523); binned previews and thumbnails decide clipping per sample before averaging (`develop_binned_masked`, `highlight::reconstruct_masked`), so a block with one clipped specular sample keeps its mean instead of jumping to the clip level, which reconstruction turned into green speckles at 500 px (#548) From process V2 the Highlights/Shadows base is the edge-aware filter of the 0.15 %-of-long-edge median of log luminance: twigs and fine texture take their surroundings' shift, so bright areas keep their local contrast as with Exposure (V1's base kept ~0.6 of it at −100, #632); region edges stay sharp | +| LR-EDIT-LIGHT-SHADOWS | Shadows | P0 | ✅ | `ctl:light.shadows` | From process V2, the same base as Highlights: dark areas keep their local contrast when lifted (#632) | | LR-EDIT-LIGHT-WHITES | Whites | P0 | ✅ | `ctl:light.whites` | | | LR-EDIT-LIGHT-BLACKS | Blacks | P0 | ✅ | `ctl:light.blacks` | | | LR-EDIT-LIGHT-CURVE-PARAM | Parametric curve | P0 | ✅ | `ctl:curve.highlights`, `ctl:curve.lights`, `ctl:curve.darks`, `ctl:curve.shadows`, `ctl:curve.split*` | | diff --git a/docs/process-versions.md b/docs/process-versions.md index f9831b7a5..74125078f 100644 --- a/docs/process-versions.md +++ b/docs/process-versions.md @@ -5,7 +5,10 @@ Every photo's develop settings record the **rendering process** they are interpr this the process version. It lets the look of LightCraft improve (a new default tone curve, issue #146; stronger Light sliders, issue #196) without changing a single photo someone has already edited. -Today there is one process, **V1**: LightCraft's rendering from before process versions existed. +| Process | What it changes | +|---|---| +| **V1** | LightCraft's rendering from before process versions existed. Frozen. | +| **V2** (the latest) | Highlights and Shadows keep fine detail (issue #632): their base layer (`local::tone_base`) is the edge-aware filter of a separable median of log luminance (radius 0.15 % of the long edge, at most 24 px, `local::BASE_DETAIL`) instead of log luminance itself, so twigs, lines and texture take their surroundings' shift and keep their contrast; region edges stay sharp. CPU (`local::prepare`) and GPU prepare branch on `local::tone_base_detail` (0 = no median under V1); the base plane is cached per process (`local::BaseKey`). Renders without Highlights or Shadows (global or local) are the same as V1's. Tone map unchanged. | ## The rule diff --git a/docs/whats-new.md b/docs/whats-new.md index fb1fd3165..95ae5827d 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -119,6 +119,13 @@ into the display's own gamut; thumbnails and other previews are converted to it. Histograms, the preview caches and exports don't change. Agents use `app.displayProfile`. See [display profiles](display-profiles.md). +### Highlights and Shadows (process 2) +- Highlights and Shadows keep the fine detail of the tones they move (issue #632): pulling Highlights down on a bright + sky no longer flattens the twigs, lines and texture in it, and lifting Shadows keeps the texture of dark areas, much + as Exposure would; the edges between bright and dark regions stay as sharp as before. This is rendering process 2: + new photos (and Reset) get it, photos you already edited keep their look until you choose Photo ▸ Update to Current + Process. See [process versions](process-versions.md). + ### Masking - Drag anywhere inside the selected radial gradient's ellipse to move it, including rotated ellipses and radial components in compound masks. Edge and rotation handles keep their existing