From 9ac38222e12e9bec2bb171d2a30aa8b69aeb4b9a Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Sun, 11 Oct 2026 17:07:40 +0200 Subject: [PATCH 01/29] Alt/Option-drag previews for clipping and sharpening Holding Alt (Option on macOS) while dragging an Edit slider shows Lightroom's temporary previews: highlight clipping for Exposure, Highlights and Whites, shadow clipping for Shadows and Blacks, greyscale for Sharpening Amount, the detail band for Radius / Detail and the sharpening mask for Masking. They are render overlays (pipeline visualize), drawn the same way after the CPU and GPU renderers; the sharpening views read each renderer's own detail planes. Settings and normal output are unchanged. Fixes #937 Co-Authored-By: Claude Opus 5.5 --- crates/gpu/src/render.rs | 8 + crates/gpu/tests/equivalence.rs | 28 ++++ crates/pipeline/src/finish.rs | 11 +- crates/pipeline/src/lib.rs | 7 +- crates/pipeline/src/local.rs | 7 +- crates/pipeline/src/spots.rs | 4 +- crates/pipeline/src/visualize.rs | 239 +++++++++++++++++++++++++++- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/detail.rs | 16 +- crates/ui-egui/src/state.rs | 5 + crates/ui-egui/src/tests_panels.rs | 37 +++++ docs/parity-checklist.md | 21 +-- docs/ui-parity.md | 11 +- docs/whats-new.md | 11 ++ 14 files changed, 382 insertions(+), 25 deletions(-) diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 44458350f..bb920d4c0 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -747,6 +747,14 @@ pub fn render( lightcraft_pipeline::masks::evaluate_one(m, &plan.frame, w, h, &img, &l, s.light.exposure as f32, plan.mattes.as_deref()) } }); + // a sharpening view: the detail planes this render sharpened with + let overlay_mask = overlay_mask.or_else(|| { + req.overlay.reads_detail().then(|| { + let l = host.log_l.get_or_insert_with(|| Arc::new(cx.read_plane(&prep.log_l, w, h))).clone(); + let tex = prep.texture.as_deref().map(|b| cx.read_plane(b, w, h)); + lightcraft_pipeline::visualize::sharpen_plane(req.overlay, &plan.settings, &l, tex.as_ref(), plan.px_per_long) + })? + }); lightcraft_pipeline::visualize::apply(&mut image, req.overlay, &plan, overlay_mask.as_ref()); // spots grew the window the render worked on: cut it back to the request if let Some(k) = plan.keep { diff --git a/crates/gpu/tests/equivalence.rs b/crates/gpu/tests/equivalence.rs index 96771a971..96d46340b 100644 --- a/crates/gpu/tests/equivalence.rs +++ b/crates/gpu/tests/equivalence.rs @@ -660,6 +660,34 @@ fn overlays_match() { let req = RenderRequest { overlay: Overlay::Mask { id, view, color: [230, 30, 40], opacity: 50 }, ..RenderRequest::fit(640, 640) }; check(&format!("mask {id} overlay {}", view.name()), &src, &raw, &s, &req); } + // Alt-drag previews: clipping and greyscale on the finished image; the sharpening views from + // each renderer's own detail planes (with sharpening on, and off: no texture plane then). + let mut s = DevelopSettings::default(); + (s.light.whites, s.light.blacks, s.detail.sharpen_masking) = (80.0, -80.0, 40.0); + for amount in [50.0, 0.0] { + s.detail.sharpen_amount = amount; + for o in [Overlay::ClipHighlights, Overlay::ClipShadows, Overlay::Grayscale, Overlay::SharpenDetail, Overlay::SharpenMask] { + let req = RenderRequest { overlay: o, ..RenderRequest::fit(640, 640) }; + if matches!(o, Overlay::ClipHighlights | Overlay::ClipShadows) { + // a binary view: a 1-LSB difference at 0 / 255 flips a channel, so compare shares + let cpu = render(&src, &raw, &s, &req).image; + let gpu = lightcraft_gpu::render(&src, &raw, &s, &req, None).expect("gpu").image; + let differ = cpu.data.iter().zip(&gpu.data).filter(|(a, b)| a != b).count() as f64 / cpu.data.len() as f64; + assert!(differ < 0.002, "{o:?} sharpen {amount}: {differ}"); + } else if o == Overlay::Grayscale { + check(&format!("{o:?} preview"), &src, &raw, &s, &req); + } else { + // the sharpening views amplify the detail planes (the mask through a steep + // smoothstep): allow a few larger differences where the planes differ slightly + let cpu = render(&src, &raw, &s, &req).image; + let gpu = lightcraft_gpu::render(&src, &raw, &s, &req, None).expect("gpu").image; + let (mean, max, _) = diff(&cpu, &gpu); + let far = cpu.data.iter().zip(&gpu.data).filter(|(a, b)| a[0].abs_diff(b[0]) > 8).count() as f64 / cpu.data.len() as f64; + eprintln!("{o:?} sharpen {amount:<4} mean {mean:.4} max {max} >8: {:.4}%", far * 100.0); + assert!(mean < 1.0 && far < 0.005, "{o:?} sharpen {amount}: mean {mean:.4}, >8 LSB {far}"); + } + } + } } #[test] diff --git a/crates/pipeline/src/finish.rs b/crates/pipeline/src/finish.rs index eb65d51a2..99c83dc42 100644 --- a/crates/pipeline/src/finish.rs +++ b/crates/pipeline/src/finish.rs @@ -20,6 +20,15 @@ fn smooth(e0: f32, e1: f32, x: f32) -> f32 { t * t * (3.0 - 2.0 * t) } +/// How much of the sharpening a pixel gets (0..1) under Masking `masking` (0..1), from its fine +/// detail `det` (log luminance minus its texture blur): 1 everywhere at 0, only stronger edges +/// as it rises. The GPU kernel (`finish.wgsl`) computes the same; Alt-drag on Masking shows it +/// ([`crate::Overlay::SharpenMask`]). +#[inline] +pub fn sharpen_mask_weight(masking: f32, det: f32) -> f32 { + if masking > 0.0 { smooth(masking * 0.25, masking * 0.25 + 0.15, det.abs()) } else { 1.0 } +} + /// Parametric region curve (encoded domain) composed with the master point curve. fn curve_luts(c: &ToneCurve) -> Option<[Lut1; 3]> { let parametric = c.highlights != 0.0 || c.lights != 0.0 || c.darks != 0.0 || c.shadows != 0.0; @@ -509,7 +518,7 @@ pub(crate) fn finish_with( let tame = 1.0 - 0.6 * smooth(0.4, 1.6, det.abs()); delta += tx * 1.1 * det.clamp(-1.0, 1.0) * tame; if sp != 0.0 { - let m = if sharpen_mask > 0.0 { smooth(sharpen_mask * 0.25, sharpen_mask * 0.25 + 0.15, det.abs()) } else { 1.0 }; + let m = sharpen_mask_weight(sharpen_mask, det); delta += sp * 1.3 * det.clamp(-0.8, 0.8) * m; } } diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index 6fbb156e8..a1d9e2aa5 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -637,9 +637,12 @@ pub fn color_range_sample(src: &Rgb32f, info: &SourceInfo, s: &DevelopSettings, Some(acc.map(|v| v / n)) } -/// The alpha plane a mask overlay shows: the one the render evaluated, or (for a hidden mask) a -/// fresh evaluation. +/// The plane an overlay shows: a sharpening view's detail plane, or the alpha of a mask overlay +/// (the one the render evaluated, or for a hidden mask a fresh evaluation). fn overlay_alpha(o: Overlay, plan: &Plan<'_>, prep: &Prepared) -> Option { + if o.reads_detail() { + return visualize::sharpen_plane(o, &plan.settings, &prep.log_l, prep.texture_blur.as_deref(), plan.px_per_long); + } let m = o.mask(&plan.settings)?; if let Some(e) = prep.masks.iter().find(|e| e.id == m.id) { return Some(e.alpha.clone()); diff --git a/crates/pipeline/src/local.rs b/crates/pipeline/src/local.rs index ce3994cd8..4ddd885cb 100644 --- a/crates/pipeline/src/local.rs +++ b/crates/pipeline/src/local.rs @@ -452,6 +452,11 @@ pub struct PlaneSigmas { pub chroma: Option, } +/// Gaussian sigma (pixels) of the texture / sharpening band at `px_per_long`. +pub fn texture_sigma(px_per_long: f64) -> f32 { + (0.0018 * px_per_long as f32).max(0.6) +} + pub fn plane_sigmas(s: &DevelopSettings, px_per_long: f64, q: Quality) -> PlaneSigmas { let ppl = px_per_long as f32; let local_any = |f: fn(&lightcraft_develop::LocalAdjustments) -> f64| s.masks.iter().any(|m| f(&m.adjust) != 0.0); @@ -472,7 +477,7 @@ pub fn plane_sigmas(s: &DevelopSettings, px_per_long: f64, q: Quality) -> PlaneS || local_any(|a| a.sharpness) || local_any(|a| a.noise) || s.masks.iter().any(|m| m.adjust.defringe > 0.0)) - .then(|| (0.0018 * ppl).max(0.6)); + .then(|| texture_sigma(px_per_long)); 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)); let base_detail = if base.is_some() { tone_base_detail(s, px_per_long) } else { 0 }; diff --git a/crates/pipeline/src/spots.rs b/crates/pipeline/src/spots.rs index 483f83f3c..f8b3fbbdf 100644 --- a/crates/pipeline/src/spots.rs +++ b/crates/pipeline/src/spots.rs @@ -247,8 +247,8 @@ fn fill_box(src: &mut [[f32; 3]], tgt: &[[f32; 3]], alpha: &[f32], bw: usize, bh let flat: Vec = tgt.iter().flatten().copied().collect(); let out = lightcraft_inpaint::complete(bw, bh, 3, &flat, &hole, &lightcraft_inpaint::CompleteParams::default()) .unwrap_or_else(|| lightcraft_inpaint::membrane_fill(bw, bh, 3, &flat, &hole)); - for (d, v) in src.iter_mut().zip(out.chunks_exact(3)) { - *d = [v[0], v[1], v[2]]; + for (d, v) in src.iter_mut().zip(out.as_chunks::<3>().0) { + *d = *v; } } diff --git a/crates/pipeline/src/visualize.rs b/crates/pipeline/src/visualize.rs index fc3d8390f..baa992fd1 100644 --- a/crates/pipeline/src/visualize.rs +++ b/crates/pipeline/src/visualize.rs @@ -9,6 +9,11 @@ //! - **Mask overlay** (Masking): the evaluated alpha of one mask, drawn as a colour tint, a colour //! tint on a black-and-white image, the image on black / white, or the alpha as white on black //! ([`MaskView`]). Both renderers hand the same alpha plane (the one the render used) to [`apply`]. +//! - **Alt/Option-drag previews** (Lightroom's temporary views while a slider is dragged with the +//! modifier held; [`Overlay::for_control`] picks one per control): shadow / highlight clipping +//! (white or black, with the clipped channels shown), greyscale (Sharpening Amount), the fine +//! detail band sharpening works on (Radius / Detail) and the sharpening mask (Masking). The +//! sharpening views read the render's own detail planes ([`sharpen_plane`]). use std::borrow::Cow; @@ -36,6 +41,19 @@ pub enum Overlay { /// Visualize HDR (photos edited in HDR): the SDR image dimmed to grey, with every pixel the HDR /// rendition puts above SDR white painted by how far above it is ([`HDR_BANDS`]). HdrRange, + /// Shadow clipping (Alt-drag on Blacks / Shadows): white, a channel at 0 drops out of the + /// colour, so pixels clipped in every channel are black and partly clipped ones are coloured. + ClipShadows, + /// Highlight clipping (Alt-drag on Exposure / Highlights / Whites): black, a channel at 255 + /// shows in the colour, so pixels clipped in every channel are white. + ClipHighlights, + /// The photo in greyscale (Alt-drag on Sharpening Amount). + Grayscale, + /// The fine detail band sharpening amplifies, as grey ± detail (Alt-drag on Sharpening Radius + /// / Detail). + SharpenDetail, + /// The sharpening mask, white where sharpening applies (Alt-drag on Sharpening Masking). + SharpenMask, } /// [`Overlay::HdrRange`] colours (sRGB) for 0–1, 1–2, 2–3 and ≥ 3 stops above SDR white. @@ -121,6 +139,11 @@ impl Overlay { Overlay::Spots(t) => (2, t as f64), Overlay::Mask { id, view, color, opacity } => (3, pack_mask(id, view, color, opacity) as f64), Overlay::HdrRange => (4, 0.0), + Overlay::ClipShadows => (5, 0.0), + Overlay::ClipHighlights => (6, 0.0), + Overlay::Grayscale => (7, 0.0), + Overlay::SharpenDetail => (8, 0.0), + Overlay::SharpenMask => (9, 0.0), } } @@ -131,6 +154,11 @@ impl Overlay { 2 => Overlay::Spots(v.clamp(0.0, 100.0) as u8), 3 if v.is_finite() && v >= 0.0 => unpack_mask(v as u64), 4 => Overlay::HdrRange, + 5 => Overlay::ClipShadows, + 6 => Overlay::ClipHighlights, + 7 => Overlay::Grayscale, + 8 => Overlay::SharpenDetail, + 9 => Overlay::SharpenMask, _ => Overlay::None, } } @@ -143,9 +171,38 @@ impl Overlay { Overlay::Spots(t) => 0x2000 + t as u64, Overlay::Mask { id, view, color, opacity } => 3 << 60 | pack_mask(id, view, color, opacity), Overlay::HdrRange => 0x4000, + Overlay::ClipShadows => 0x5000, + Overlay::ClipHighlights => 0x5001, + Overlay::Grayscale => 0x5002, + Overlay::SharpenDetail => 0x5003, + Overlay::SharpenMask => 0x5004, } } + /// The temporary preview Lightroom shows while develop control `id` is dragged with Alt + /// (Option on macOS) held, if it has one. + pub fn for_control(id: &str) -> Option { + match id { + "light.exposure" | "light.highlights" | "light.whites" => Some(Overlay::ClipHighlights), + "light.shadows" | "light.blacks" => Some(Overlay::ClipShadows), + "detail.sharpenAmount" => Some(Overlay::Grayscale), + "detail.sharpenRadius" | "detail.sharpenDetail" => Some(Overlay::SharpenDetail), + "detail.sharpenMasking" => Some(Overlay::SharpenMask), + _ => None, + } + } + + /// Whether the overlay draws from the render's detail planes ([`sharpen_plane`]). + pub fn reads_detail(self) -> bool { + matches!(self, Overlay::SharpenDetail | Overlay::SharpenMask) + } + + /// Whether the overlay works pixel by pixel on the finished image, so a window render shows + /// the same as the whole frame would. + pub fn per_pixel(self) -> bool { + matches!(self, Overlay::None | Overlay::Mask { .. } | Overlay::ClipShadows | Overlay::ClipHighlights | Overlay::Grayscale) + } + /// The mask an [`Overlay::Mask`] shows (if it exists in `s` and has components). pub fn mask(self, s: &DevelopSettings) -> Option<&lightcraft_develop::Mask> { match self { @@ -167,9 +224,44 @@ pub fn adjust_settings(o: Overlay, s: &mut Cow<'_, DevelopSettings>) { } } -/// Draw overlay `o` over `img` (rendered with `plan`). `mask` is the evaluated alpha of the mask -/// an [`Overlay::Mask`] shows, at the image's size (see [`Overlay::mask`]). -pub fn apply(img: &mut Rgba8, o: Overlay, plan: &Plan<'_>, mask: Option<&Plane>) { +/// The plane a sharpening view draws ([`Overlay::reads_detail`]), from the render's log luminance +/// `log_l` and its texture blur (`None` when the render didn't need it: blurred here with the same +/// sigma). [`Overlay::SharpenDetail`]: the detail band (log luminance minus its blur, in EV); +/// [`Overlay::SharpenMask`]: the sharpening weight under the photo's Masking. Both renderers call +/// this with their own planes. +pub fn sharpen_plane(o: Overlay, s: &DevelopSettings, log_l: &Plane, texture: Option<&Plane>, px_per_long: f64) -> Option { + if !o.reads_detail() { + return None; + } + let own; + let blur = match texture.filter(|b| (b.width, b.height) == (log_l.width, log_l.height)) { + Some(b) => b, + None => { + own = lightcraft_raster::blur::gaussian(log_l, crate::local::texture_sigma(px_per_long)); + &own + } + }; + let masking = (s.detail.sharpen_masking / 100.0).clamp(0.0, 1.0) as f32; + let mask = o == Overlay::SharpenMask; + let data = log_l + .data + .iter() + .zip(&blur.data) + .map(|(l, b)| { + let det = l - b; + if mask { crate::finish::sharpen_mask_weight(masking, det) } else { det } + }) + .collect(); + Some(Plane { width: log_l.width, height: log_l.height, data }) +} + +/// [`Overlay::SharpenDetail`]: encoded grey per EV of detail (±0.4 EV spans black to white). +const DETAIL_GAIN: f32 = 320.0; + +/// Draw overlay `o` over `img` (rendered with `plan`). `plane` is what the overlay shows, at the +/// image's size: the evaluated alpha of the mask an [`Overlay::Mask`] shows (see +/// [`Overlay::mask`]) or a sharpening view's [`sharpen_plane`]. +pub fn apply(img: &mut Rgba8, o: Overlay, plan: &Plan<'_>, plane: Option<&Plane>) { match o { // drawn by the renderer from the HDR rendition ([`hdr_range`]) Overlay::None | Overlay::HdrRange => {} @@ -180,13 +272,60 @@ pub fn apply(img: &mut Rgba8, o: Overlay, plan: &Plan<'_>, mask: Option<&Plane>) } Overlay::Spots(t) => spots(img, t, plan.px_per_long), Overlay::Mask { view, color, opacity, .. } => { - if let Some(a) = mask.filter(|a| (a.width, a.height) == (img.width, img.height)) { + if let Some(a) = plane.filter(|a| (a.width, a.height) == (img.width, img.height)) { mask_view(img, a, view, color, opacity); } } + Overlay::ClipShadows => clipping(img, false), + Overlay::ClipHighlights => clipping(img, true), + Overlay::Grayscale => grayscale(img), + Overlay::SharpenDetail | Overlay::SharpenMask => { + if let Some(p) = plane.filter(|a| (a.width, a.height) == (img.width, img.height)) { + let detail = o == Overlay::SharpenDetail; + let w = img.width; + for_rows(&mut img.data, w, |y, row| { + for (x, px) in row.iter_mut().enumerate() { + let v = p.data[y * w + x]; + let g = if detail { 128.0 + v * DETAIL_GAIN } else { 255.0 * v }; + let g = g.round().clamp(0.0, 255.0) as u8; + *px = [g, g, g, 255]; + } + }); + } + } } } +/// [`Overlay::ClipHighlights`] (`high`) or [`Overlay::ClipShadows`]: each channel shows only +/// whether it is clipped, on black (highlights) or white (shadows). +pub fn clipping(img: &mut Rgba8, high: bool) { + let w = img.width; + for_rows(&mut img.data, w, |_, row| { + for px in row.iter_mut() { + for c in &mut px[..3] { + *c = if high { + if *c == 255 { 255 } else { 0 } + } else if *c == 0 { + 0 + } else { + 255 + }; + } + px[3] = 255; + } + }); +} + +fn grayscale(img: &mut Rgba8) { + let w = img.width; + for_rows(&mut img.data, w, |_, row| { + for px in row.iter_mut() { + let g = (0.2126 * px[0] as f32 + 0.7152 * px[1] as f32 + 0.0722 * px[2] as f32).round().clamp(0.0, 255.0) as u8; + *px = [g, g, g, 255]; + } + }); +} + /// Draw mask alpha `a` over `img` as `view`. pub fn mask_view(img: &mut Rgba8, a: &Plane, view: MaskView, color: [u8; 3], opacity: u8) { let op = opacity.min(100) as f32 / 100.0; @@ -380,4 +519,96 @@ mod tests { assert_eq!(MaskView::WhiteOnBlack.next(), MaskView::Color); assert_eq!(MaskView::parse("colorOnBw"), Some(MaskView::ColorOnBw)); } + + #[test] + fn clipping_views_show_the_clipped_channels() { + let mut img = Rgba8 { + width: 5, + height: 1, + data: vec![[0, 0, 0, 255], [255, 255, 255, 255], [255, 40, 0, 255], [0, 128, 200, 255], [120, 130, 140, 255]], + }; + let mut hi = img.clone(); + clipping(&mut hi, true); + // black background, white where every channel clips, the clipped channels' colour otherwise + assert_eq!(hi.data, vec![[0, 0, 0, 255], [255; 4], [255, 0, 0, 255], [0, 0, 0, 255], [0, 0, 0, 255]]); + clipping(&mut img, false); + // white background, black where every channel clips, the unclipped channels' colour otherwise + assert_eq!(img.data, vec![[0, 0, 0, 255], [255; 4], [255, 255, 0, 255], [0, 255, 255, 255], [255; 4]]); + } + + #[test] + fn clipping_previews_follow_the_rendered_clipping() { + // left: deep shadow, middle: grey, right: bright highlight + let src = Rgb32f::from_fn(60, 20, |x, _| match x / 20 { + 0 => [0.0; 3], + 1 => [0.18; 3], + _ => [3.0; 3], + }); + let info = SourceInfo::default(); + let mut s = DevelopSettings::default(); + s.light.blacks = -100.0; + s.light.whites = 100.0; + let req = |o| RenderRequest { overlay: o, ..RenderRequest::fit(60, 20) }; + let plain = render(&src, &info, &s, &req(Overlay::None)); + let hi = render(&src, &info, &s, &req(Overlay::ClipHighlights)); + let lo = render(&src, &info, &s, &req(Overlay::ClipShadows)); + let at = |i: &Rgba8, x: usize| i.data[10 * 60 + x]; + assert_eq!(at(&plain.image, 55), [255; 4], "the highlight clips"); + assert_eq!(at(&plain.image, 5), [0, 0, 0, 255], "the shadow clips"); + assert_eq!((at(&hi.image, 55), at(&hi.image, 30), at(&hi.image, 5)), ([255; 4], [0, 0, 0, 255], [0, 0, 0, 255])); + assert_eq!((at(&lo.image, 5), at(&lo.image, 30), at(&lo.image, 55)), ([0, 0, 0, 255], [255; 4], [255; 4])); + // a preview only: the histogram is the photo's + assert_eq!(hi.histogram, plain.histogram); + assert_eq!(render(&src, &info, &s, &req(Overlay::None)).image, plain.image); + } + + #[test] + fn sharpening_previews_show_the_mask_and_detail() { + // a smooth ramp (left) and a hard edge (at x = 60) + let src = Rgb32f::from_fn(80, 40, |x, _| if x < 60 { [0.05 + 0.002 * x as f32; 3] } else { [0.6; 3] }); + let info = SourceInfo::default(); + let mut s = DevelopSettings::default(); + s.detail.sharpen_amount = 60.0; + let shot = |s: &DevelopSettings, o| render(&src, &info, s, &RenderRequest { overlay: o, ..RenderRequest::fit(80, 40) }).image; + let (flat, edge) = (20 * 80 + 20, 20 * 80 + 60); + // Masking 0 sharpens everything: an all-white mask + let m = shot(&s, Overlay::SharpenMask); + assert!(m.data.iter().all(|p| *p == [255; 4])); + // Masking 100 keeps the edge, protects the ramp + s.detail.sharpen_masking = 100.0; + let m = shot(&s, Overlay::SharpenMask); + assert_eq!((m.data[edge], m.data[flat]), ([255; 4], [0, 0, 0, 255])); + // the same without sharpening on (the render has no texture plane then) + s.detail.sharpen_amount = 0.0; + assert_eq!(shot(&s, Overlay::SharpenMask), m); + // detail: mid grey on the ramp, light / dark either side of the edge + let d = shot(&s, Overlay::SharpenDetail); + assert!(d.data[flat][0].abs_diff(128) <= 3, "{:?}", d.data[flat]); + assert!(d.data[edge][0] > 200 && d.data[edge - 1][0] < 60, "{:?} {:?}", d.data[edge], d.data[edge - 1]); + // greyscale + let src = Rgb32f::from_fn(8, 8, |_, _| [0.4, 0.1, 0.05]); + let g = render(&src, &info, &s, &RenderRequest { overlay: Overlay::Grayscale, ..RenderRequest::fit(8, 8) }).image; + assert!(g.data.iter().all(|p| p[0] == p[1] && p[1] == p[2] && p[0] > 0)); + } + + #[test] + fn alt_previews_map_to_controls_and_round_trip() { + assert_eq!(Overlay::for_control("light.blacks"), Some(Overlay::ClipShadows)); + assert_eq!(Overlay::for_control("light.shadows"), Some(Overlay::ClipShadows)); + assert_eq!(Overlay::for_control("light.whites"), Some(Overlay::ClipHighlights)); + assert_eq!(Overlay::for_control("light.exposure"), Some(Overlay::ClipHighlights)); + assert_eq!(Overlay::for_control("light.highlights"), Some(Overlay::ClipHighlights)); + assert_eq!(Overlay::for_control("detail.sharpenAmount"), Some(Overlay::Grayscale)); + assert_eq!(Overlay::for_control("detail.sharpenDetail"), Some(Overlay::SharpenDetail)); + assert_eq!(Overlay::for_control("detail.sharpenMasking"), Some(Overlay::SharpenMask)); + assert_eq!(Overlay::for_control("light.contrast"), None); + let all = [Overlay::ClipShadows, Overlay::ClipHighlights, Overlay::Grayscale, Overlay::SharpenDetail, Overlay::SharpenMask]; + for o in all { + let (k, v) = o.to_parts(); + assert_eq!(Overlay::from_parts(k, v), o); + assert_eq!(all.iter().filter(|p| p.key() == o.key()).count(), 1); + assert_ne!(o.key(), Overlay::HdrRange.key()); + } + assert!(Overlay::ClipShadows.per_pixel() && !Overlay::SharpenDetail.per_pixel()); + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 9a387c77c..5dc965129 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1018,6 +1018,8 @@ impl LightcraftApp { } } self.ui.was_focused = focused; + // Alt (Option on macOS) while a slider is dragged shows its temporary preview + self.ui.alt_held = ctx.input(|i| i.modifiers.alt); // auto import: list the watched folder every few seconds (on a worker thread: it may be on // a network share), then import what's new like any import (also on a worker thread) #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 234476167..4eb03c8c6 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -253,7 +253,7 @@ fn animated_rect(ctx: &egui::Context, anim: &mut bool, target: Rect) -> Rect { /// Mask overlays are evaluated from the same mask alpha as the full render; the other diagnostic /// overlays either need whole-frame context or deliberately replace the loupe. pub(crate) fn overlay_supports_window(overlay: lightcraft_pipeline::Overlay) -> bool { - matches!(overlay, lightcraft_pipeline::Overlay::None | lightcraft_pipeline::Overlay::Mask { .. }) + overlay.per_pixel() } /// What a window render of the loupe needs to know about the view it belongs to. @@ -444,7 +444,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let look = d.hash64(); let overlay = view_overlay(app, &d); // Diagnostic views that need whole-frame context, soft proofing, or reads too far for one - // window still render the whole frame at the size it is drawn. A mask overlay is window-safe. + // window still render the whole frame at the size it is drawn. Per-pixel ones (a mask, + // clipping) are window-safe. let sizes = crate::region::ViewSizes { drawn_long, canvas_long: canvas.width().max(canvas.height()) * ppp, @@ -1118,13 +1119,20 @@ fn targeted_drag(app: &mut LightcraftApp, resp: &egui::Response, map: &CanvasMap } } -/// The diagnostic overlay the loupe shows (the selected mask, Point Color's visualized range, -/// Visualize Spots). +/// The diagnostic overlay the loupe shows (an Alt-drag preview, the selected mask, Point Color's +/// visualized range, Visualize Spots). pub(crate) fn view_overlay(app: &LightcraftApp, d: &DevelopSettings) -> lightcraft_pipeline::Overlay { use lightcraft_pipeline::{MaskView, Overlay}; if app.ui.fullscreen { return Overlay::None; } + // Lightroom's temporary previews: clipping, greyscale, sharpening detail / mask + if app.ui.right == RightPanel::Edit + && app.ui.alt_held + && let Some(o) = app.ui.dragging_control.as_deref().and_then(Overlay::for_control) + { + return o; + } if app.ui.right == RightPanel::Masking && app.ui.mask_overlay && let Some(m) = app.session.active_mask.and_then(|id| d.masks.iter().find(|m| m.id == id)) diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index b76066d55..839bfa7a4 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -603,6 +603,10 @@ pub struct UiState { /// The develop control whose slider is being dragged (geometry sliders show a grid). #[serde(skip)] pub dragging_control: Option, + /// Alt (Option on macOS) is held this frame: a dragged Edit slider shows its temporary + /// preview ([`lightcraft_pipeline::Overlay::for_control`]). + #[serde(skip)] + pub alt_held: bool, pub search: String, /// Focus the search field on the next frame (Edit → Find…). #[serde(skip)] @@ -1074,6 +1078,7 @@ impl Default for UiState { unnamed_focus: false, hidden_locations: Vec::new(), dragging_control: None, + alt_held: false, external_edits: Vec::new(), was_focused: true, auto_import_at: 0.0, diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index 25edf3bfa..a7accb362 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -2141,3 +2141,40 @@ fn right_click_add_to_album_offers_a_new_album_when_there_is_none() { assert!(trip.photos.contains(&photo), "the right-clicked photo is in it: {:?}", trip.photos); assert!(h.app.ui.dialog.is_none(), "{:?}", h.app.ui.dialog); } + +#[test] +fn alt_while_dragging_an_edit_slider_shows_its_preview() { + use crate::state::RightPanel; + use lightcraft_pipeline::Overlay; + let mut h = demo([1400.0, 900.0], json!({"view": "detail", "right": "edit"})); + let id = h.app.session.active().expect("active photo"); + let d = (*h.app.session.develop_of(id).unwrap_or_default()).clone(); + let view = |h: &Headless| crate::panels::detail::view_overlay(&h.app, &d); + assert_eq!(h.app.ui.right, RightPanel::Edit); + h.app.ui.dragging_control = Some("light.blacks".into()); + assert_eq!(view(&h), Overlay::None, "dragging without Alt shows the photo"); + h.app.ui.alt_held = true; + assert_eq!(view(&h), Overlay::ClipShadows); + for (c, o) in [ + ("light.whites", Overlay::ClipHighlights), + ("light.exposure", Overlay::ClipHighlights), + ("detail.sharpenAmount", Overlay::Grayscale), + ("detail.sharpenRadius", Overlay::SharpenDetail), + ("detail.sharpenMasking", Overlay::SharpenMask), + ("light.contrast", Overlay::None), + ] { + h.app.ui.dragging_control = Some(c.into()); + assert_eq!(view(&h), o, "{c}"); + } + // Alt alone (no drag) or released mid-drag: back to the photo + h.app.ui.dragging_control = None; + assert_eq!(view(&h), Overlay::None); + h.app.ui.dragging_control = Some("light.whites".into()); + h.app.ui.alt_held = false; + assert_eq!(view(&h), Overlay::None); + // the Masking panel's local Blacks slider is not a global clipping preview + h.app.ui.alt_held = true; + h.app.ui.right = RightPanel::Masking; + h.app.ui.dragging_control = Some("light.blacks".into()); + assert_eq!(view(&h), Overlay::None); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 77ae26e4c..75d2aed25 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 (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-11 · **Change:** minor (Alt/Option-drag previews, LR-EDIT-DETAIL-SHARPEN to 🟡; 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 @@ -31,7 +31,7 @@ cited here still exists. Formerly docs/parity.md. | C. Views & navigation (VIEW) | 15 | 1 | 1 | 0 | 8/9 (89%) | 4/4 (100%) | | D. Search & filter (FILT) | 11 | 1 | 1 | 0 | 4/4 (100%) | 4/4 (100%) | | E. Metadata (META) | 5 | 1 | 0 | 0 | 2/2 (100%) | 2/2 (100%) | -| F. Edit panel — global adjustments (EDIT) | 43 | 2 | 3 | 1 | 28/28 (100%) | 13/14 (93%) | +| F. Edit panel — global adjustments (EDIT) | 42 | 3 | 3 | 1 | 27/28 (96%) | 13/14 (93%) | | G. Profiles (PROF) | 7 | 1 | 3 | 0 | 3/4 (75%) | 3/3 (100%) | | H. Crop & rotate (CROP) | 9 | 0 | 0 | 1 | 6/6 (100%) | 3/3 (100%) | | I. Remove / healing (REM) | 8 | 1 | 2 | 2 | 4/4 (100%) | 2/3 (67%) | @@ -53,9 +53,9 @@ cited here still exists. Formerly docs/parity.md. | Y. Menus | 84 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | | Z. Keyboard shortcuts (desktop) | 76 | 3 | 3 | 1 | 49/52 (94%) | 23/24 (96%) | | Lightroom Classic extras | 34 | 16 | 35 | 8 | — | 24/26 (92%) | -| **Total** | 407 | 50 | 73 | 35 | 192/200 (96%) | 145/157 (92%) | +| **Total** | 406 | 51 | 73 | 35 | 191/200 (96%) | 145/157 (92%) | -Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.5%** of 530 in-scope rows — P0 98.0% of 200 · P1 95.2% of 157 · P2 49.7% of 172. +Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.4%** of 530 in-scope rows — P0 97.8% of 200 · P1 95.2% of 157 · P2 49.7% of 172. ## Top gaps and shortcut notes @@ -177,12 +177,12 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-AUTO | Auto settings | P0 | ✅ | `cmd:develop.auto`, `crates/pipeline/src/auto.rs` | a Lightroom-like recipe calibrated black-box on Lightroom's Auto results for 24 Sony raws: Exposure from the scene's scene-linear log-average luminance (Lightroom's observed slope, -0.41 EV per EV), Highlights -66 and Shadows +52 (Lightroom's average Auto values), Contrast +68 and Vibrance +30 fitted on our scales. Held out one photo at a time: on the 16 photos whose start Sony's DRO doesn't brighten, a median dE76 of 6.4 from Lightroom's Auto results against 8.3 unedited; on all 24, 7.7 against 7.9 (issues #318, #209). Calibrated on Sony files only; Whites and Blacks stay 0 | | LR-EDIT-BW | Black & white | P0 | ✅ | `cmd:develop.treatment` | | | LR-EDIT-HDR-MODE | HDR editing | P2 | ✅ | `cmd:develop.hdr`, `ctl:hdr.maxEv`, `crates/ui-egui/src/panels/edit.rs` (`hdr_row`) | HDR button next to B&W in the Edit panel header, as Lightroom (Visualize HDR and the headroom limit in the Light panel while it's on); see Q. HDR | -| LR-EDIT-LIGHT-EXPOSURE | Exposure | P0 | ✅ | `ctl:light.exposure` | | +| LR-EDIT-LIGHT-EXPOSURE | Exposure | P0 | ✅ | `ctl:light.exposure` | Alt/Option-drag shows highlight clipping (black, clipped channels in colour) | | 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) 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-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. Alt/Option-drag shows highlight clipping | +| 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). Alt/Option-drag shows shadow clipping (white, clipped channels in colour) | +| LR-EDIT-LIGHT-WHITES | Whites | P0 | ✅ | `ctl:light.whites`, `crates/pipeline/src/visualize.rs` | Alt/Option-drag shows highlight clipping (black, clipped channels in colour) | +| LR-EDIT-LIGHT-BLACKS | Blacks | P0 | ✅ | `ctl:light.blacks`, `crates/pipeline/src/visualize.rs` | Alt/Option-drag shows shadow clipping (white, clipped channels in colour) | | LR-EDIT-LIGHT-CURVE-PARAM | Parametric curve | P0 | ✅ | `ctl:curve.highlights`, `ctl:curve.lights`, `ctl:curve.darks`, `ctl:curve.shadows`, `ctl:curve.split*` | | | LR-EDIT-LIGHT-CURVE-POINT | Point curve | P0 | ✅ | `cmd:develop.curve`, `cmd:curve.reset`, `cmd:curve.presets`, `cmd:curve.applyPreset`, `cmd:curve.savePreset`, `cmd:curve.deletePreset`, `cmd:curve.importPresets`, `cmd:curve.exportPresets`, `cmd:file.importCurvePresets`, `cmd:file.exportCurvePresets`, `crates/engine/src/cmd/curves.rs`, `crates/ui-egui/src/panels/edit.rs` | click to add a point; drag a point in both axes (between its neighbours, input / output readout, one undo step per drag); drag empty space to add and drag; double-click removes; reset: double-click a channel selector (that channel), the Reset button under the graph (every curve incl. parametric) or right-click the graph (channel / all); Point Curve preset dropdown: own Linear / Medium Contrast / Strong Contrast, user presets (save the current point curves, delete by right-click; saved with the library), import / export as `.lccurve` JSON | | LR-EDIT-LIGHT-CURVE-RGB | Per-channel curves | P0 | ✅ | `cmd:develop.curve` (`channel`) | | @@ -203,7 +203,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-EFFECTS-DEHAZE | Dehaze | P0 | ✅ | `ctl:effects.dehaze` | | | 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-SHARPEN | Sharpening | P0 | 🟡 | `ctl:detail.sharpenAmount`, `ctl:detail.sharpenRadius`, `ctl:detail.sharpenDetail`, `ctl:detail.sharpenMasking`, `crates/pipeline/src/visualize.rs` | Amount and Masking work; Radius and Detail are stored (XMP round trip) but do not change the render yet (the band is a fixed 0.18 % of the long edge). Alt/Option-drag previews: Amount greyscale, Radius / Detail the detail band sharpening amplifies, Masking the mask (white = sharpened) | | 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-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) | @@ -787,6 +787,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-11 | minor | Alt/Option-drag previews on Exposure, Highlights, Shadows, Whites, Blacks and Sharpening (issue #937); LR-EDIT-DETAIL-SHARPEN ✅ → 🟡: Radius and Detail don't change the render | | 2026-10-11 | minor | KEY-SLIDER-TYPE: honor immediate Return/Escape and keep typed keys in the value editor; add regression references | | 2026-10-11 | minor | LR-PROF-CAMERACOLOR: raws with a black-and-white camera JPEG take its tone curve (issue #577) | | 2026-10-11 | minor | LR-VIEW-ZOOM and LR-VIEW-COMPARE: Compare and Reference are sharp at 1:1 (zoom windows per pane, two originals kept decoded while they are open); statuses unchanged | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 44aa1c703..06a728db4 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (zoom row: Compare and Reference sharp at 1:1) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (Alt/Option-drag slider previews) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 How LightCraft's interaction compares with Lightroom's: views, panels, tools and handles, modifier keys, shortcuts, zoom, context menus and feel. Rows for individual menu items and shortcuts live in @@ -67,6 +67,14 @@ under the UI command that wraps it (a no-op error); the UI command now wins. **Library culling (KEYC-RATING):** `0–5`, `6–9` and `P/X/U` reach egui on macOS even when displayed in the native menu (adapted from PR #261; issue #283). `Shift+0–9` applies and advances once; `Shift+P` picks and advances in Photo Grid / Square Grid and retains Presets elsewhere. See [library shortcuts](library-shortcuts.md) for the verified Classic mapping and regression coverage. Remaining Classic gaps: rating `[` / `]`, flag cycling and filter-bar keys. +**Alt/Option-drag previews (issue #937):** hold Alt (Option on macOS) while dragging an Edit slider, before or during +the drag. Exposure, Highlights and Whites show highlight clipping (black, clipped channels in their colour, white where +all three clip); Shadows and Blacks show shadow clipping (white, black where all three clip). Sharpening Amount shows +greyscale, Radius and Detail the detail band sharpening amplifies, Masking the mask (white = sharpened). Releasing the +key or the slider brings the photo back; the edit and its history are unchanged. Not yet: Noise Reduction, mask range +sliders and the Masking panel's local sliders. On Linux desktops that move windows with Alt+drag (KDE Plasma's +default, also some GNOME / Xfce setups) the window manager takes the drag first: set its window-move modifier to Meta. + **Still missing / broken:** - No command yet: F1 help (verify the rest of the old list: full screen, settings, stacks, visualize spots and merges have commands now). @@ -78,5 +86,6 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Date | Change | Summary | |---|---|---| +| 2026-10-11 | minor | Alt/Option-drag slider previews (clipping, sharpening) and the Linux window-manager note | | 2026-10-11 | minor | Zoom row: Compare and Reference get zoom windows; what remains of LR-VIEW-ZOOM is overlays and soft proofing | | 2026-10-10 | major | Created: measured command and shortcut counts, assessment by area, estimates; moved the shortcut conflicts section here from the checklist | diff --git a/docs/whats-new.md b/docs/whats-new.md index fb8b285a3..5107eae27 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -11,6 +11,17 @@ - Lossy JPEG XL files in Display P3 or Rec. 2020 now open in their real colours; saturated colours used to come in noticeably duller. +### Alt/Option-drag previews +- Hold Alt (Option on macOS) while dragging Exposure, Highlights or Whites to see which areas clip to white: the + photo turns black, fully clipped areas show white and areas clipped in one or two channels show those channels' + colour. Shadows and Blacks do the same for clipping to black, on white (issue #937). +- In Detail ▸ Sharpening the same key shows Amount in greyscale, Radius and Detail as the fine detail being + sharpened, and Masking as the mask: white is sharpened, black is protected. Radius and Detail don't change the + render yet, so their preview stays the same while you drag them. +- The preview lasts while the key and the slider are held, and never changes the edit, its history or exports. +- Linux: KDE Plasma (and some other desktops) move windows with Alt+drag. If Alt+drag moves the window instead, + set the window manager's move modifier to Meta. + ### Open Library knows a Lightroom catalog - Choosing a Lightroom Classic catalog folder in File → Open Library… (or one of its `Previews.lrdata` / `.lrcat-data` folders, or the `.lrcat`) no longer makes an empty LightCraft library inside it (issue #891). The From 86f4bde867d1ffa44519b23d39733eac015e6e52 Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Mon, 12 Oct 2026 13:53:24 +0200 Subject: [PATCH 02/29] LR-VIEW-CLIPPING: Alt-drag previews keep the histogram on profiled displays; NR previews (#937) Folds the parts of #995 that #999 lacked into #999: - engine/media.rs: with a display profile, a diagnostic view keeps the photo's histogram (taken before the overlay is drawn) and stays out of the view cache; the clipping previews test sRGB values and are converted for the display afterwards. Before, holding Alt on a P3 display swapped the histogram for the clipping view's (mean luma 0.59 -> 0.999). - Noise Reduction Luminance / Detail / Contrast show greyscale while Alt-dragged (Overlay::for_control). - Tests: diagnostic_views_keep_the_photos_histogram (P3 display; fails without the media.rs change), an end-to-end headless drag that presses and releases Alt mid-drag and checks the loupe pixels (tests_alt_preview.rs), and every Alt-mapped id is a real control. - Docs: NR row, What's New and ui-parity note; parity summary refreshed. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/display.rs | 19 +++++++ crates/engine/src/media.rs | 15 +++++- crates/pipeline/src/visualize.rs | 31 ++++++++++- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/tests_alt_preview.rs | 71 +++++++++++++++++++++++++ docs/parity-checklist.md | 6 +-- docs/ui-parity.md | 7 +-- docs/whats-new.md | 3 +- 8 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 crates/ui-egui/src/tests_alt_preview.rs diff --git a/crates/engine/src/display.rs b/crates/engine/src/display.rs index f47e099b6..34840e46d 100644 --- a/crates/engine/src/display.rs +++ b/crates/engine/src/display.rs @@ -152,6 +152,25 @@ mod tests { } } + /// Alt-drag previews on a profiled display (issue #937): the histogram stays the photo's (not + /// the preview's), and the clipping view is tested and drawn in sRGB, so it stays black and white. + #[test] + fn diagnostic_views_keep_the_photos_histogram() { + use lightcraft_pipeline::Overlay; + let mut session = crate::Session::with_demo(); + let id = session.visible_cloned()[0]; + let d = std::sync::Arc::new(p3()); + let mut run = |o| session.loupe_job(id, 320, 240, true).unwrap().with_display(Some(d.clone())).with_overlay(o).run().rendered.unwrap(); + let plain = run(Overlay::None); + let clip = run(Overlay::ClipShadows); + let (a, b) = (plain.histogram.mean_luma(), clip.histogram.mean_luma()); + assert!((a - b).abs() < 0.02, "the photo's histogram: {a} vs {b}"); + let px = &clip.image.data; + // (the profile may round white to 254) + let binary = px.iter().filter(|p| p[..3].iter().all(|v| *v <= 2 || *v >= 253)).count(); + assert!(binary as f64 > 0.99 * px.len() as f64, "{binary} of {}", px.len()); + } + #[test] fn load_reports_and_rejects() { let dir = std::env::temp_dir().join(format!("lightcraft-display-{}", std::process::id())); diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 6f6f0359c..cd26f222d 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -838,11 +838,24 @@ impl RenderJob { // Thumbnails (many small jobs side by side) stay on the CPU; views and exports use // the GPU when there is one. let gpu = self.cache.is_none(); - let (mut rendered, gpu_fallback) = develop_reporting(src, &info, &self.settings, &self.request, self.stages.as_deref(), gpu); + // a clipping preview tests sRGB values, like the histogram: rendered in sRGB, then + // converted for the display below + let clip_view = self.request.overlay.is_clipping(); + let request = if clip_view { RenderRequest { display: None, ..self.request } } else { self.request }; + let (mut rendered, gpu_fallback) = develop_reporting(src, &info, &self.settings, &request, self.stages.as_deref(), gpu); // for a display profile: the image goes to the display's values, the histogram // and the cached view preview stay sRGB let mut srgb = None; let display = match (&self.display, self.request.display) { + // a diagnostic view: the histogram stays the photo's (taken before the overlay + // was drawn), and nothing goes to the view cache + (Some(d), Some(_)) if self.request.overlay != lightcraft_pipeline::Overlay::None => { + let converted = if clip_view { d.profile.from_srgb(&mut rendered.image) } else { d.profile.correct(&mut rendered.image) }; + if let Err(e) = converted { + log::warn!("display profile: {e}"); + } + Some(d.id()) + } (Some(d), Some(_)) => { let keep = self.view_cache.is_some() && self.request.quality == Quality::Full; let (s, h) = crate::display::finish_render(&mut rendered.image, d, keep); diff --git a/crates/pipeline/src/visualize.rs b/crates/pipeline/src/visualize.rs index baa992fd1..50b959332 100644 --- a/crates/pipeline/src/visualize.rs +++ b/crates/pipeline/src/visualize.rs @@ -185,13 +185,19 @@ impl Overlay { match id { "light.exposure" | "light.highlights" | "light.whites" => Some(Overlay::ClipHighlights), "light.shadows" | "light.blacks" => Some(Overlay::ClipShadows), - "detail.sharpenAmount" => Some(Overlay::Grayscale), + "detail.sharpenAmount" | "detail.nrLuminance" | "detail.nrDetail" | "detail.nrContrast" => Some(Overlay::Grayscale), "detail.sharpenRadius" | "detail.sharpenDetail" => Some(Overlay::SharpenDetail), "detail.sharpenMasking" => Some(Overlay::SharpenMask), _ => None, } } + /// The clipping previews, which test the sRGB values the histogram counts (a render for a + /// display profile draws them in sRGB, then converts). + pub fn is_clipping(self) -> bool { + matches!(self, Overlay::ClipShadows | Overlay::ClipHighlights) + } + /// Whether the overlay draws from the render's detail planes ([`sharpen_plane`]). pub fn reads_detail(self) -> bool { matches!(self, Overlay::SharpenDetail | Overlay::SharpenMask) @@ -601,7 +607,30 @@ mod tests { assert_eq!(Overlay::for_control("detail.sharpenAmount"), Some(Overlay::Grayscale)); assert_eq!(Overlay::for_control("detail.sharpenDetail"), Some(Overlay::SharpenDetail)); assert_eq!(Overlay::for_control("detail.sharpenMasking"), Some(Overlay::SharpenMask)); + for c in ["detail.nrLuminance", "detail.nrDetail", "detail.nrContrast"] { + assert_eq!(Overlay::for_control(c), Some(Overlay::Grayscale), "{c}"); + } + assert_eq!(Overlay::for_control("detail.nrColor"), None); assert_eq!(Overlay::for_control("light.contrast"), None); + // every mapped id is a real control (a rename would silently drop its preview) + for c in [ + "light.exposure", + "light.highlights", + "light.whites", + "light.shadows", + "light.blacks", + "detail.sharpenAmount", + "detail.sharpenRadius", + "detail.sharpenDetail", + "detail.sharpenMasking", + "detail.nrLuminance", + "detail.nrDetail", + "detail.nrContrast", + ] { + assert!(lightcraft_develop::controls::find(c).is_some(), "{c}"); + assert!(Overlay::for_control(c).is_some(), "{c}"); + } + assert!(Overlay::ClipShadows.is_clipping() && Overlay::ClipHighlights.is_clipping() && !Overlay::Grayscale.is_clipping()); let all = [Overlay::ClipShadows, Overlay::ClipHighlights, Overlay::Grayscale, Overlay::SharpenDetail, Overlay::SharpenMask]; for o in all { let (k, v) = o.to_parts(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 5dc965129..be0b6653e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -44,6 +44,8 @@ mod tests_activity; #[cfg(test)] mod tests_album_picker; #[cfg(test)] +mod tests_alt_preview; +#[cfg(test)] mod tests_appearance; #[cfg(test)] mod tests_before_after; diff --git a/crates/ui-egui/src/tests_alt_preview.rs b/crates/ui-egui/src/tests_alt_preview.rs new file mode 100644 index 000000000..7a212869f --- /dev/null +++ b/crates/ui-egui/src/tests_alt_preview.rs @@ -0,0 +1,71 @@ +//! Headless test of the Alt-drag previews (issue #937) through real input: a slider drag, Alt +//! (Option) pressed and released during it, and the loupe's pixels at each step. + +use std::time::Duration; + +use egui::{Event, Modifiers, PointerButton}; +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn detail() -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1400.0, 1400.0], 1.0); + let r = h.request("ui.set", json!({"view": "detail", "right": "edit"}), T); + assert_eq!(r["ok"], true, "{r}"); + if !h.app.ui.section_open("light") { + h.app.ui.toggle_section("light"); + } + h.app.renderer.keep_pixels = true; + h.settle(SETTLE); + h +} + +/// Share of the loupe render's pixels whose channels are all 0 or 255 (a clipping view). +fn binary_share(h: &Headless) -> f64 { + let t = h.app.renderer.textures.get(&crate::render::Slot::Main).expect("loupe render"); + let px = &t.pixels.as_ref().expect("pixels kept").pixels; + let binary = px.iter().filter(|c| [c.r(), c.g(), c.b()].iter().all(|v| *v == 0 || *v == 255)).count(); + binary as f64 / px.len().max(1) as f64 +} + +#[test] +fn alt_during_a_blacks_drag_shows_shadow_clipping_until_released() { + let mut h = detail(); + assert!(binary_share(&h) < 0.5, "the photo itself is no clipping view"); + let track = h.app.widgets.iter().find(|(w, _)| w == "slider:light.blacks").map(|(_, r)| *r).expect("Blacks slider on screen"); + let from = track.center(); + let before = h.app.session.develop_of(h.app.session.active().expect("photo")).expect("develop").light.blacks; + // a plain drag to the left, button held: the photo as usual + h.app.synthetic.push(Event::PointerMoved(from)); + h.app.synthetic.push(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + for i in 1..=8 { + h.app.synthetic.push(Event::PointerMoved(from - egui::vec2(6.0 * i as f32, 0.0))); + } + h.settle(SETTLE); + assert_eq!(h.app.ui.dragging_control.as_deref(), Some("light.blacks")); + assert!(binary_share(&h) < 0.5, "no preview without Alt"); + // Alt pressed mid-drag: the shadow clipping view + h.app.synthetic.push(Event::ModifiersChanged(Modifiers::ALT)); + h.settle(SETTLE); + assert!(binary_share(&h) > 0.99, "the clipping view: {}", binary_share(&h)); + // Alt released: the photo again, the drag still on + h.app.synthetic.push(Event::ModifiersChanged(Modifiers::NONE)); + h.settle(SETTLE); + assert!(binary_share(&h) < 0.5, "the photo is back"); + h.app.synthetic.push(Event::PointerButton { + pos: from - egui::vec2(48.0, 0.0), + button: PointerButton::Primary, + pressed: false, + modifiers: Modifiers::NONE, + }); + h.settle(SETTLE); + assert_eq!(h.app.ui.dragging_control, None); + let after = h.app.session.develop_of(h.app.session.active().expect("photo")).expect("develop").light.blacks; + assert!(after < before, "the drag itself still moved Blacks: {before} → {after}"); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 75d2aed25..cfd9248df 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -53,9 +53,9 @@ cited here still exists. Formerly docs/parity.md. | Y. Menus | 84 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | | Z. Keyboard shortcuts (desktop) | 76 | 3 | 3 | 1 | 49/52 (94%) | 23/24 (96%) | | Lightroom Classic extras | 34 | 16 | 35 | 8 | — | 24/26 (92%) | -| **Total** | 406 | 51 | 73 | 35 | 191/200 (96%) | 145/157 (92%) | +| **Total** | 407 | 52 | 72 | 35 | 192/201 (96%) | 145/157 (92%) | -Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.4%** of 530 in-scope rows — P0 97.8% of 200 · P1 95.2% of 157 · P2 49.7% of 172. +Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **81.5%** of 531 in-scope rows — P0 97.8% of 201 · P1 95.2% of 157 · P2 50.0% of 172. ## Top gaps and shortcut notes @@ -204,7 +204,7 @@ 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`, `crates/pipeline/src/visualize.rs` | Amount and Masking work; Radius and Detail are stored (XMP round trip) but do not change the render yet (the band is a fixed 0.18 % of the long edge). Alt/Option-drag previews: Amount greyscale, Radius / Detail the detail band sharpening amplifies, Masking the mask (white = sharpened) | -| LR-EDIT-DETAIL-NR | Luminance noise reduction | P0 | ✅ | `ctl:detail.nrLuminance`, `ctl:detail.nrDetail`, `ctl:detail.nrContrast` | | +| LR-EDIT-DETAIL-NR | Luminance noise reduction | P0 | ✅ | `ctl:detail.nrLuminance`, `ctl:detail.nrDetail`, `ctl:detail.nrContrast` | Alt/Option-drag on Luminance, Detail and Contrast shows the photo in greyscale | | LR-EDIT-DETAIL-CNR | Colour noise reduction | P0 | ✅ | `ctl:detail.nrColor`, `ctl:detail.nrColorDetail`, `ctl:detail.nrColorSmoothness` | | | 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 | ⬜ | | | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 06a728db4..6782cc6b3 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -70,9 +70,10 @@ under the UI command that wraps it (a no-op error); the UI command now wins. **Alt/Option-drag previews (issue #937):** hold Alt (Option on macOS) while dragging an Edit slider, before or during the drag. Exposure, Highlights and Whites show highlight clipping (black, clipped channels in their colour, white where all three clip); Shadows and Blacks show shadow clipping (white, black where all three clip). Sharpening Amount shows -greyscale, Radius and Detail the detail band sharpening amplifies, Masking the mask (white = sharpened). Releasing the -key or the slider brings the photo back; the edit and its history are unchanged. Not yet: Noise Reduction, mask range -sliders and the Masking panel's local sliders. On Linux desktops that move windows with Alt+drag (KDE Plasma's +greyscale, Radius and Detail the detail band sharpening amplifies, Masking the mask (white = sharpened); luminance Noise +Reduction (Luminance, Detail, Contrast) shows greyscale. Releasing the key or the slider brings the photo back; the +edit, its history and the histogram are unchanged (on a profiled display too). Not yet: mask range sliders and the +Masking panel's local sliders. On Linux desktops that move windows with Alt+drag (KDE Plasma's default, also some GNOME / Xfce setups) the window manager takes the drag first: set its window-move modifier to Meta. **Still missing / broken:** diff --git a/docs/whats-new.md b/docs/whats-new.md index 5107eae27..8d2329dbc 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -17,7 +17,8 @@ colour. Shadows and Blacks do the same for clipping to black, on white (issue #937). - In Detail ▸ Sharpening the same key shows Amount in greyscale, Radius and Detail as the fine detail being sharpened, and Masking as the mask: white is sharpened, black is protected. Radius and Detail don't change the - render yet, so their preview stays the same while you drag them. + render yet, so their preview stays the same while you drag them. Noise Reduction's Luminance, Detail and Contrast + show the photo in greyscale. - The preview lasts while the key and the slider are held, and never changes the edit, its history or exports. - Linux: KDE Plasma (and some other desktops) move windows with Alt+drag. If Alt+drag moves the window instead, set the window manager's move modifier to Meta. From adcba3a865284801570ecf0454c01fcdb84dc262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Lu=C3=ADs=20da=20Silva?= Date: Fri, 23 Oct 2026 02:17:38 -0300 Subject: [PATCH 03/29] Never crash: tiles far larger than the image are refused before decoding (#2554) check_image bounded the image but not its tiles, whose nominal size comes from the file: a 16x16 DNG with one 16384x16384 lossless-JPEG tile (coded 16 rows tall) passed the header-only import, then the full decode resized that tile to 512 MiB (N x 2 GiB with more tiles). check_image now bounds the sum of the chunks' nominal samples to four times the image's (a tile grid no larger than the image covers less than twice each dimension), with a floor of one 2048x2048 tile for tiny images. That covers DNG and both ARW tiled readers, in the header probe and the full decode. The #1512 test keeps covering the uncompressed little-data check with a 2048x2048 tile; a 16384x16384 one is now refused earlier. --- crates/raw/src/tiffraw.rs | 48 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/crates/raw/src/tiffraw.rs b/crates/raw/src/tiffraw.rs index a781151b4..203d5c773 100644 --- a/crates/raw/src/tiffraw.rs +++ b/crates/raw/src/tiffraw.rs @@ -54,6 +54,14 @@ pub fn check_image(data: &[u8], info: &ImageInfo) -> Result { if chunks.len() < expected { return Err(RawError::Corrupt(format!("{} image data chunks, expected {expected}", chunks.len()))); } + // Tile sizes don't depend on the image size, and every tile is decoded at its nominal size and held until the copy: + // a grid of tiles no larger than the image covers less than twice its width and height, so more than four times + // its samples (or one tile of more than TILE_SAMPLES_FLOOR on a tiny image) is no picture (issue #2554). + let ccpp = if info.planar == 2 { 1 } else { cpp as u64 }; + let nominal = chunks.iter().fold(0u64, |sum, c| sum.saturating_add(u64::from(c.width).saturating_mul(u64::from(c.height)).saturating_mul(ccpp))); + if nominal > (total as u64).saturating_mul(4).max(TILE_SAMPLES_FLOOR) { + return Err(RawError::Limit("image data chunks far larger than the image")); + } // Plausibility bound before allocating: no supported coding stores more than ~2000 samples per byte // (Deflate of constant data is the extreme), so tiny files cannot trigger huge allocations. let available: u64 = chunks.iter().map(|c| chunk_bytes(data, c).map_or(0, |s| s.len() as u64)).sum(); @@ -95,6 +103,9 @@ pub(crate) fn read_image_in(mode: crate::Mode, data: &[u8], info: &ImageInfo, or } } +/// The nominal samples of all chunks [`check_image`] allows whatever the image size: one 2048 × 2048 tile. +const TILE_SAMPLES_FLOOR: u64 = 1 << 22; + /// Decode all chunks of `info` into one buffer of `width × height × cpp` samples. pub fn read_image(data: &[u8], info: &ImageInfo, order: ByteOrder, packing: Packing) -> Result { let (w, h) = (info.width as usize, info.height as usize); @@ -417,8 +428,9 @@ mod lossy_tests { assert!(read_image(&file, &past_end, ByteOrder::Little, Packing::Msb).is_err()); } - /// A small image whose uncompressed tile claims to be enormous (16384 x 16384 nominal, a few bytes of data) is - /// refused before that tile is allocated (issue #1512); an edge tile stored at its full nominal size decodes. + /// A small image whose uncompressed tile claims to be enormous (2048 x 2048 nominal, a few bytes of data) is + /// refused before that tile is allocated (issue #1512); an edge tile stored at its full nominal size decodes. (A + /// tile larger still is refused by `check_image` before any decode: `tiles_far_larger_than_the_image_are_refused`.) #[test] fn a_huge_nominal_uncompressed_tile_with_little_data_is_refused() { let info = |tile: u32, bytes: u64| ImageInfo { @@ -439,7 +451,37 @@ mod lossy_tests { let small = vec![1u8; 2 * 16 * 16]; assert!(read_image(&small, &info(16, small.len() as u64), ByteOrder::Little, Packing::Msb).is_ok()); let data = vec![1u8; 4096]; - assert!(matches!(read_image(&data, &info(16384, data.len() as u64), ByteOrder::Little, Packing::Msb), Err(RawError::Corrupt(_)))); + assert!(matches!(read_image(&data, &info(2048, data.len() as u64), ByteOrder::Little, Packing::Msb), Err(RawError::Corrupt(_)))); + } + + /// A 16 × 16 image whose lossless-JPEG tile claims to be 16384 × 16384 (coded only as tall as the image, so the + /// file is small) is refused before the tile is allocated, by the header check too (issue #2554); a 256 × 256 + /// tile on the same image, as a writer with a fixed tile size stores it, still decodes. + #[test] + fn tiles_far_larger_than_the_image_are_refused() { + let read = |tile: u32, mode: crate::Mode| { + let t = ljpeg::encode(&vec![300u16; tile as usize * 16], tile as usize, 16, 1, 12, 1, 0); + let info = ImageInfo { + width: 16, + height: 16, + bits_per_sample: vec![12], + samples_per_pixel: 1, + compression: comp::JPEG, + photometric: 32803, + planar: 1, + predictor: 1, + sample_format: 1, + new_subfile_type: 0, + layout: Layout::Tiles { tile_width: tile, tile_height: tile }, + offsets: vec![0], + byte_counts: vec![t.len() as u64], + }; + read_image_in(mode, &t, &info, ByteOrder::Little, Packing::Msb) + }; + assert_eq!(read(256, crate::Mode::Full), Ok(RawData::U16(vec![300; 16 * 16]))); + for mode in [crate::Mode::Header, crate::Mode::Full] { + assert!(matches!(read(16384, mode), Err(RawError::Limit(_)))); + } } /// A lossless-JPEG tile may be coded only as tall as the part inside the image, never shorter. From 556c95c7309f86338d46353118d64929f34b4c23 Mon Sep 17 00:00:00 2001 From: strebdom Date: Fri, 23 Oct 2026 15:09:58 +0200 Subject: [PATCH 04/29] LR-IMP-FORMATS: AHD blends its candidates where noise alone would pick one, no maze in noisy raws (#2094) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AHD chose the horizontal or the vertical candidate outright by the larger 3×3 homogeneity sum. In a flat, noisy area noise alone decides which is larger, so the picture became short horizontal and vertical strokes (a maze, orange and blue under strong white balance) that noise reduction then made stand out. It now blends the two linearly while their sums are within 12 of each other (of 36), and takes one alone beyond that. On an ISO 4000 ILCE-7CM2 ARW under tungsten light the maze measure of a flat region drops ~30 %; PSNR on the synthetic smooth and edge scenes is unchanged or better (73.0 → 73.7 dB, 37.7 → 37.9 dB). The colour part of the pattern mostly comes from demosaicing before white balance (#1328, PR #1463), which this does not change. New test flat_noise_does_not_turn_into_a_maze fails on the outright pick. RENDER_CACHE_VERSION 70. --- crates/engine/src/media.rs | 2 +- crates/raw/src/demosaic/ahd.rs | 17 ++++++------ crates/raw/src/demosaic/mod.rs | 47 ++++++++++++++++++++++++++++++++++ docs/raw-parity.md | 5 ++-- docs/whats-new.md | 9 +++++++ 5 files changed, 69 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 292a9fb1e..e76ef8169 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 = 69; +pub const RENDER_CACHE_VERSION: u64 = 70; /// 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/raw/src/demosaic/ahd.rs b/crates/raw/src/demosaic/ahd.rs index d9fc27d76..622a19ce9 100644 --- a/crates/raw/src/demosaic/ahd.rs +++ b/crates/raw/src/demosaic/ahd.rs @@ -6,7 +6,8 @@ //! 2. Complete each candidate with red/blue by colour-difference (C − G) interpolation. //! 3. Convert both candidates to CIELab and measure, per pixel, how many of its 4 neighbours lie within the //! adaptive luminance / chrominance tolerances (ε_L, ε_C from the paper) — the homogeneity. -//! 4. Sum homogeneity over a 3×3 window and pick the more homogeneous candidate (average on ties). +//! 4. Sum homogeneity over a 3×3 window and pick the more homogeneous candidate. The paper picks one outright; +//! we blend the two by how far apart their sums are (see [`BLEND`]), so a clear edge still takes one candidate. //! //! Work is done in overlapping tiles (bounded memory, rayon-parallel), one band of tiles at a time written //! straight into the output. Each step runs only where the tile's output depends on it: the choice reads the @@ -19,6 +20,11 @@ use rayon::prelude::*; const TILE: usize = 128; const MARGIN: usize = 6; +/// Difference of the two candidates' 3×3 homogeneity sums (each 0–36) from which the more homogeneous one is taken +/// alone; closer sums blend the two linearly, equal sums half and half. In a flat, noisy area noise alone decides +/// which sum is larger, and picking outright turned the noise into short horizontal and vertical strokes (a maze, +/// coloured where white balance is strong). 12 keeps the PSNR of the outright pick on the edge tests. +const BLEND: f32 = 12.0; pub(crate) fn ahd(m: &Mosaic) -> Rgb32f { let (w, h) = (m.w, m.h); @@ -152,13 +158,8 @@ fn tile(m: &Mosaic, x0: usize, y0: usize, tw: usize, th: usize) -> Vec<[f32; 3]> sh += hh[j] as u32; sv += hv[j] as u32; } - out.push(if sh > sv { - rh[i] - } else if sv > sh { - rv[i] - } else { - [(rh[i][0] + rv[i][0]) * 0.5, (rh[i][1] + rv[i][1]) * 0.5, (rh[i][2] + rv[i][2]) * 0.5] - }); + let wh = (0.5 + (sh as f32 - sv as f32) / (2.0 * BLEND)).clamp(0.0, 1.0); + out.push([0, 1, 2].map(|c| rh[i][c] * wh + rv[i][c] * (1.0 - wh))); } out } diff --git a/crates/raw/src/demosaic/mod.rs b/crates/raw/src/demosaic/mod.rs index b8a3c599a..b99f7b015 100644 --- a/crates/raw/src/demosaic/mod.rs +++ b/crates/raw/src/demosaic/mod.rs @@ -226,6 +226,53 @@ mod tests { assert!(xp > 20.0 && xp >= xb - 0.5, "xtrans directional {xp:.1} bilinear {xb:.1}"); } + /// Mean anisotropy of a demosaiced picture's fine luminance detail: per pixel, how much more its 3 × 3 high-pass + /// correlates with its horizontal neighbours than with its vertical ones (or the reverse). Noise has none; the + /// short horizontal and vertical strokes of a maze pattern have a lot. + fn anisotropy(img: &Rgb32f) -> f64 { + let (w, h) = (img.width, img.height); + let lum: Vec = img.data.iter().map(|p| 0.25 * p[0] + 0.5 * p[1] + 0.25 * p[2]).collect(); + let mean3 = |x: usize, y: usize| (0..9).map(|k| lum[(y + k / 3 - 1) * w + x + k % 3 - 1]).sum::() / 9.0; + let hp: Vec = + (0..w * h).map(|i| if i % w == 0 || i % w == w - 1 || i < w || i >= w * (h - 1) { 0.0 } else { lum[i] - mean3(i % w, i / w) }).collect(); + let (mut sum, mut n) = (0f64, 0f64); + for y in 2..h - 2 { + for x in 2..w - 2 { + let i = y * w + x; + sum += f64::from((hp[i] * (hp[i - 1] + hp[i + 1] - hp[i - w] - hp[i + w])).abs()); + n += 1.0; + } + } + sum / n + } + + #[test] + fn flat_noise_does_not_turn_into_a_maze() { + // a flat grey patch with sensor-like noise: AHD picked horizontal or vertical candidates per pixel by a + // homogeneity count that noise alone decides, so the picture became short horizontal and vertical strokes + let (w, h) = (192, 160); + let mut seed = 0x2545_f491_4f6c_dd1du64; + let mut noise = || { + // sum of four uniforms ≈ Gaussian, σ ≈ 0.02 + (0..4) + .map(|_| { + seed ^= seed << 13; + seed ^= seed >> 7; + seed ^= seed << 17; + (seed >> 40) as f32 / (1u64 << 24) as f32 - 0.5 + }) + .sum::() + * 0.035 + }; + let cfa = Cfa::bayer("RGGB").unwrap(); + let data = (0..w * h).map(|_| 0.2 + noise()).collect(); + let n = Normalized { width: w, height: h, cpp: 1, data, cfa: Some(cfa) }; + let (ahd, bilinear) = (anisotropy(&demosaic(&n, Method::Ahd)), anisotropy(&demosaic(&n, Method::Bilinear))); + eprintln!("anisotropy: ahd {ahd:.3e}, bilinear {bilinear:.3e}"); + // the hard choice gave about 11 times bilinear's, the blend about 7.5 times + assert!(ahd < 9.0 * bilinear, "ahd {ahd:.3e} bilinear {bilinear:.3e}"); + } + #[test] fn tiny_and_degenerate_inputs() { let cfa = Cfa::bayer("RGGB").unwrap(); diff --git a/docs/raw-parity.md b/docs/raw-parity.md index 2923ea880..3151255d4 100644 --- a/docs/raw-parity.md +++ b/docs/raw-parity.md @@ -1,6 +1,6 @@ # RAW parity: cameras, formats, demosaic, colour and lens profiles -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-18 · **Change:** minor (Minolta MRW decoded, 6 bodies verified; lossy JPEG XL DNG tiles with a non-linear declared transfer curve decode; Canon ColorData Kelvin; X-Trans CMOS 4 bodies take the X-T3's spectral matrices) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 (Camera Raw 18.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-23 · **Change:** minor (AHD no longer turns flat noise into a maze pattern) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 (Camera Raw 18.7) Raw processing is the deepest single dimension of Lightroom parity: a photographer whose camera does not decode, or decodes with the wrong colour, cannot switch. This document measures it model by @@ -194,7 +194,7 @@ sample files. A pointer for contributors, not legal advice. | Stage | Target | LightCraft | Status | |---|---|---|---| -| Bayer demosaic | Adobe's own, plus AI "Raw Details" | AHD (default), PPG, bilinear (`crates/raw/src/demosaic`) | 🟡 quality unmeasured against Lightroom; no Raw Details | +| Bayer demosaic | Adobe's own, plus AI "Raw Details" | AHD (default; blends its horizontal and vertical candidates where neither is clearly more homogeneous, so flat noise doesn't become a maze), PPG, bilinear (`crates/raw/src/demosaic`) | 🟡 quality unmeasured against Lightroom; demosaics before white balance (#1328, PR #1463); no Raw Details | | X-Trans demosaic | ✅ | in-house edge-weighted directional | 🟡 unmeasured | | Foveon | ✅ (older bodies) | — | ⬜ | | Monochrome / LinearRaw | ✅ | handled, no demosaic | ✅ | @@ -328,3 +328,4 @@ Beyond decoding: | 2026-10-14 | minor | Integration batch 36: CC0 per-model verification for 117 Nikon/Fujifilm, 97 Canon CR2/Pentax/Ricoh, 21 Sony/Leica and 23 Olympus/Samsung bodies, 22 more CR3 bodies and 30 more models; Epson ERF and Mamiya MEF decode; measured Nikon image areas (#1053, #1770): 515 verified, 675 unverified, 218 preview, 38 unsupported (re-measured with `coverage.py`) | | 2026-10-15 | minor | By-maker and by-use tables re-measured with `coverage.py` after batches 36–39 (545 verified, 651 unverified, 212 preview, 38 unsupported; ~94% of photographers' cameras fully work, ~85% on a verified body); rows the per-model PRs had updated are current again | | 2026-10-18 | minor | Minolta MRW decoded (`vendor/mrw.rs`): DiMAGE 7, 7Hi, A1, A2, A200 and Dynax / Maxxum 7D verified; 551 verified, 656 unverified, 201 preview (re-measured with `coverage.py`) | +| 2026-10-23 | minor | AHD blends its horizontal and vertical candidates where their homogeneity is close, so flat noise no longer becomes a maze pattern; Bayer demosaic row notes the pre-white-balance demosaic (#1328); scores unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..18470577c 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,14 @@ # What's new in LightCraft +## 2026-10-23 + +### No maze pattern in noisy raws +- Flat areas of high-ISO raws, such as a dim wall or a blurred background, could show short horizontal and vertical + strokes instead of grain, often orange and blue under warm light. Noise reduction made them stand out more, because + it smooths the grain around them. The demosaic chose between a horizontal and a vertical guess at each pixel, and + in a flat area the noise alone decided which. Where neither guess is clearly better it now blends the two, so the + noise looks like grain. Edges are as sharp as before. Thumbnails and previews of raws are redrawn. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges From 89424084b222e7f9260f7a348314d0428d0fedb4 Mon Sep 17 00:00:00 2001 From: strebdom Date: Fri, 23 Oct 2026 15:25:55 +0200 Subject: [PATCH 05/29] LR-IMP-FORMATS: AHD's blend leaves out a candidate that isn't finite (#2698 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The outright pick never let the losing candidate's NaN (beside a float raw's NaN sample) into the output; the blend did, as 0 · NaN. A candidate taken alone is now copied, and one that isn't finite is left out while the other is. A NaN sample reaches as many pixels as before the blend (9 in the new test; the blend made it 33). --- crates/raw/src/demosaic/ahd.rs | 11 ++++++++++- crates/raw/src/demosaic/mod.rs | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/raw/src/demosaic/ahd.rs b/crates/raw/src/demosaic/ahd.rs index 622a19ce9..c4180de4d 100644 --- a/crates/raw/src/demosaic/ahd.rs +++ b/crates/raw/src/demosaic/ahd.rs @@ -159,7 +159,16 @@ fn tile(m: &Mosaic, x0: usize, y0: usize, tw: usize, th: usize) -> Vec<[f32; 3]> sv += hv[j] as u32; } let wh = (0.5 + (sh as f32 - sv as f32) / (2.0 * BLEND)).clamp(0.0, 1.0); - out.push([0, 1, 2].map(|c| rh[i][c] * wh + rv[i][c] * (1.0 - wh))); + // a candidate taken alone is copied, and one that isn't finite (beside a float raw's NaN sample) is left out, + // so the other's value doesn't become 0 · NaN + let finite = |p: &[f32; 3]| p.iter().all(|v| v.is_finite()); + out.push(if wh >= 1.0 || !finite(&rv[i]) { + rh[i] + } else if wh <= 0.0 || !finite(&rh[i]) { + rv[i] + } else { + [0, 1, 2].map(|c| rh[i][c] * wh + rv[i][c] * (1.0 - wh)) + }); } out } diff --git a/crates/raw/src/demosaic/mod.rs b/crates/raw/src/demosaic/mod.rs index b99f7b015..07d45b6de 100644 --- a/crates/raw/src/demosaic/mod.rs +++ b/crates/raw/src/demosaic/mod.rs @@ -290,6 +290,20 @@ mod tests { assert_eq!(demosaic(&n, Method::Ahd).data, vec![[0.1, 0.2, 0.3]]); } + #[test] + fn a_nan_sample_spreads_no_further_than_the_chosen_candidate_takes_it() { + // vertical stripes: the vertical candidate wins outright beside them, and the horizontal one, which reads two + // samples to each side, is the only one that sees a NaN two columns away + let img = Rgb32f::from_fn(32, 24, |x, _| if (x / 2) % 2 == 0 { [0.2, 0.3, 0.25] } else { [0.7, 0.8, 0.6] }); + let cfa = Cfa::bayer("RGGB").unwrap(); + let mut n = mosaic_from_rgb(&img, &cfa); + n.data[12 * 32 + 16] = f32::NAN; + let out = demosaic(&n, Method::Ahd); + let bad = out.data.iter().filter(|p| p.iter().any(|v| !v.is_finite())).count(); + // the pixels that read the NaN sample themselves, as many as the outright pick gave; the blend made it 33 + assert!(bad <= 9, "{bad} pixels not finite"); + } + #[test] fn non_finite_samples_do_not_panic() { // regression: NaN samples (a float DNG) gave AHD and PPG NaN bounds for `f32::clamp`, which panics From 2e5c1dd41d7a91602b765370d387d50629dcefdd Mon Sep 17 00:00:00 2001 From: darthnut <120891176+darthnut@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:31:29 -0700 Subject: [PATCH 06/29] Never crash: tone curves take NaN input and lookup tables shorter than two entries (#2579) MonotoneCurve::eval let NaN past both bound checks into the binary search, which then indexed one past the end (or, for a NaN with the sign bit set, underflowed to -1). to_lut(1) divided 0 by 0, and Lut1::eval underflowed on `n - 2` for a table under two entries, which its public `v` allows. NaN now comes back as NaN, as Lut1::eval already did; to_lut, Lut1::identity and Lut1::from_fn make at least two entries; and a shorter table built by hand reads as a constant (one entry) or the identity (none). No caller reaches these today. Item 3 of #2579. Co-Authored-By: Claude Opus 5.5 --- crates/color/src/spline.rs | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/color/src/spline.rs b/crates/color/src/spline.rs index c8860d950..5f9eee304 100644 --- a/crates/color/src/spline.rs +++ b/crates/color/src/spline.rs @@ -56,6 +56,10 @@ impl MonotoneCurve { } pub fn eval(&self, x: f64) -> f64 { + // NaN passes both bound checks below and would send the search past either end + if x.is_nan() { + return x; + } let n = self.xs.len(); if x <= self.xs[0] { return self.ys[0] + self.ms[0] * (x - self.xs[0]).min(0.0) * 0.0; @@ -76,8 +80,9 @@ impl MonotoneCurve { + (t3 - t2) * h * self.ms[i + 1] } - /// Sample into a LUT of `n` entries over `0..1`. + /// Sample into a LUT of `n` entries (at least 2) over `0..1`. pub fn to_lut(&self, n: usize) -> Lut1 { + let n = n.max(2); Lut1 { v: (0..n).map(|i| self.eval(i as f64 / (n - 1) as f64) as f32).collect() } } @@ -93,15 +98,22 @@ pub struct Lut1 { } impl Lut1 { + /// At least 2 entries, like [`Lut1::from_fn`]. pub fn identity(n: usize) -> Lut1 { - Lut1 { v: (0..n).map(|i| i as f32 / (n - 1) as f32).collect() } + Lut1::from_fn(n, |x| x) } + /// `f` sampled at `n` evenly spaced points over `0..1` (at least 2). pub fn from_fn(n: usize, f: impl Fn(f32) -> f32) -> Lut1 { + let n = n.max(2); Lut1 { v: (0..n).map(|i| f(i as f32 / (n - 1) as f32)).collect() } } + /// `v` is public, so a table shorter than 2 entries is read as constant (1) or the identity (0). #[inline] pub fn eval(&self, x: f32) -> f32 { let n = self.v.len(); + if n < 2 { + return self.v.first().copied().unwrap_or(x); + } let f = x.clamp(0.0, 1.0) * (n - 1) as f32; let i = (f as usize).min(n - 2); let t = f - i as f32; @@ -163,6 +175,20 @@ mod tests { assert!((c.eval(0.3) - 0.3).abs() < 2e-3); } + #[test] + fn nan_input_and_short_luts_do_not_panic() { + let c = MonotoneCurve::new(&[(0.0, 0.0), (0.5, 0.6), (1.0, 1.0)]); + assert!(c.eval(f64::NAN).is_nan()); + assert!(c.eval(-f64::NAN).is_nan()); + for n in [0, 1] { + assert_eq!(c.to_lut(n).v.len(), 2, "{n}"); + assert!(Lut1::identity(n).is_identity(), "{n}"); + assert_eq!(Lut1::from_fn(n, |x| x).v.len(), 2, "{n}"); + } + assert_eq!(Lut1 { v: vec![] }.eval(0.3), 0.3); + assert_eq!(Lut1 { v: vec![0.7] }.eval(0.3), 0.7); + } + proptest::proptest! { #[test] fn monotone_for_increasing_data(ys in proptest::collection::vec(0.0f64..1.0, 3..8)) { From 8b3ce31b665c16983129649ebcab298e8b893ba7 Mon Sep 17 00:00:00 2001 From: darthnut <120891176+darthnut@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:31:29 -0700 Subject: [PATCH 07/29] Never crash: a UTC offset of i16::MIN formats and validates without overflow (#2579) DateTime deserializes with public fields, so its offset can arrive without passing valid(). `i16::MIN.abs()` overflows: a panic in debug builds and a negative minute count in release, which to_iso() then printed. valid() and to_iso() use unsigned_abs(). Item 4 of #2579. Co-Authored-By: Claude Opus 5.5 --- crates/meta/src/datetime.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/meta/src/datetime.rs b/crates/meta/src/datetime.rs index 4b7d09634..e33600cda 100644 --- a/crates/meta/src/datetime.rs +++ b/crates/meta/src/datetime.rs @@ -31,7 +31,7 @@ impl DateTime { // chrono rejects impossible days (leap years and all); the time is checked // by hand so a leap second (`second == 60`) stays acceptable. let day_ok = self.year > 0 && chrono::NaiveDate::from_ymd_opt(i32::from(self.year), u32::from(self.month), u32::from(self.day)).is_some(); - let ok = day_ok && self.hour < 24 && self.minute < 60 && self.second < 61 && self.offset_minutes.is_none_or(|o| o.abs() <= 18 * 60); + let ok = day_ok && self.hour < 24 && self.minute < 60 && self.second < 61 && self.offset_minutes.is_none_or(|o| o.unsigned_abs() <= 18 * 60); ok.then_some(self) } @@ -135,7 +135,8 @@ impl DateTime { } if let Some(o) = self.offset_minutes { let sign = if o < 0 { '-' } else { '+' }; - s += &format!("{sign}{:02}:{:02}", o.abs() / 60, o.abs() % 60); + let o = o.unsigned_abs(); + s += &format!("{sign}{:02}:{:02}", o / 60, o % 60); } s } @@ -189,6 +190,14 @@ mod tests { assert_eq!(DateTime::parse_offset("+99:00"), None); } + #[test] + fn extreme_offsets_do_not_overflow() { + // `DateTime` deserializes with public fields, so an offset can arrive without passing `valid()` + let d = DateTime { offset_minutes: Some(i16::MIN), ..DateTime::parse_iso("2024-06-01T13:45:59").unwrap() }; + assert_eq!(d.to_iso(), "2024-06-01T13:45:59-546:08"); + assert_eq!(d.valid(), None); + } + #[test] fn unicode_offsets_are_rejected_without_panicking() { for offset in ["+1€", "-1€", "+€1", "-€1"] { From 2f622737c4dc23a4155f62df9a3569cac7ff5de6 Mon Sep 17 00:00:00 2001 From: darthnut <120891176+darthnut@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:46:34 -0700 Subject: [PATCH 08/29] Never crash: an empty lookup table clamps its input to 0..1 like any other (#2579) Review follow-up: Lut1::eval returned the input unclamped for an empty hand-built table, before the 0..1 clamp the type documents. The clamp now comes first for every table. Co-Authored-By: Claude Opus 5.5 --- crates/color/src/spline.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/color/src/spline.rs b/crates/color/src/spline.rs index 5f9eee304..c2c75f305 100644 --- a/crates/color/src/spline.rs +++ b/crates/color/src/spline.rs @@ -110,11 +110,12 @@ impl Lut1 { /// `v` is public, so a table shorter than 2 entries is read as constant (1) or the identity (0). #[inline] pub fn eval(&self, x: f32) -> f32 { + let x = x.clamp(0.0, 1.0); let n = self.v.len(); if n < 2 { return self.v.first().copied().unwrap_or(x); } - let f = x.clamp(0.0, 1.0) * (n - 1) as f32; + let f = x * (n - 1) as f32; let i = (f as usize).min(n - 2); let t = f - i as f32; self.v[i] + (self.v[i + 1] - self.v[i]) * t @@ -186,6 +187,8 @@ mod tests { assert_eq!(Lut1::from_fn(n, |x| x).v.len(), 2, "{n}"); } assert_eq!(Lut1 { v: vec![] }.eval(0.3), 0.3); + assert_eq!(Lut1 { v: vec![] }.eval(-1.0), 0.0, "inputs outside 0..1 clamp, as for any table"); + assert_eq!(Lut1 { v: vec![] }.eval(2.0), 1.0); assert_eq!(Lut1 { v: vec![0.7] }.eval(0.3), 0.7); } From 3862e5e91017b804e1cd9b8b062775eb1ed88052 Mon Sep 17 00:00:00 2001 From: Bruno-Alumn <171815746+Bruno-Alumn@users.noreply.github.com> Date: Fri, 23 Oct 2026 22:22:29 +0200 Subject: [PATCH 09/29] LR-VIEW-HISTOGRAM: the histogram stays in view while the Edit panel's sliders scroll (#2679) The histogram was drawn inside the right panel's scroll area, at the top of the Edit panel's contents, so it scrolled out of sight as soon as the sliders further down were reached. The right panel now draws it above the scroll area when the Edit panel is shown, with its clipping buttons and EXIF strip, and only the soft proofing strip and the sliders scroll under it. --- crates/ui-egui/src/panels/edit.rs | 11 +++++------ crates/ui-egui/src/panels/right.rs | 5 +++++ crates/ui-egui/src/tests_panels.rs | 24 ++++++++++++++++++++++++ docs/parity-checklist.md | 5 +++-- docs/whats-new.md | 6 ++++++ 5 files changed, 43 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index 08f70bd02..62826a0fb 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -96,9 +96,6 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } let preview_only = app.session.catalog.photo(id).and_then(|p| p.preview_only.clone()); - if app.ui.histogram { - histogram(app, ui, id); - } if app.ui.soft_proof { soft_proofing(app, ui, id); } @@ -606,7 +603,9 @@ fn steady_hdr_histogram(ctx: &egui::Context, id: PhotoId, hdr_edit: bool, hist: } } -fn histogram(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { +/// The histogram, its clipping buttons and the EXIF strip, drawn above the Edit panel's scrolling sliders +/// ([`super::right::show`]). +pub fn histogram(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { let t = Tokens::get(ui.ctx()); let w = ui.available_width(); let hdr_edit = app.session.develop_of(id).is_some_and(|d| d.hdr.enabled); @@ -721,8 +720,8 @@ fn histogram(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } } let mut clicked = resp.clicked(); - // the panel's floating scroll bar widens over the right edge when hovered: both buttons keep - // clear of it, the left one too so they stay symmetric (#1640) + // both buttons keep clear of the panel's right edge, where its floating scroll bar widens when + // hovered beside the sliders, the left one too so they stay symmetric (#1640) let scroll = ui.spacing().scroll; let inset = 20.0 + scroll.bar_width + scroll.bar_outer_margin; for (name, x, label, clipped) in [ diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 76ecc746a..6f2ae0121 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -40,6 +40,11 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } return; }; + // the Edit panel's histogram stays above its sliders as they scroll (#2679) + if app.ui.right == RightPanel::Edit && app.ui.histogram { + ui.spacing_mut().item_spacing.y = 0.0; + super::edit::histogram(app, ui, id); + } let mut area = egui::ScrollArea::vertical().id_salt("right-scroll").auto_shrink([false, false]); if let Some(offset) = held { area = area.vertical_scroll_offset(offset); diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index 96b0a1a9d..857855b2e 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -2569,6 +2569,30 @@ fn histogram_corners_toggle_clipping_warnings() { assert!(!h.app.ui.show_clipping); } +/// Issue #2679: the histogram stays at the top of the Edit panel while its sliders scroll under it. +#[test] +fn the_histogram_stays_in_view_while_the_sliders_scroll() { + let mut h = demo( + [1400.0, 700.0], + json!({"view": "detail", "right": "edit", "histogram": true, "openSections": ["light", "color", "effects", "detail", "optics"]}), + ); + let (hist, exposure) = (widget(&h, "histogram"), widget(&h, "slider:light.exposure")); + let panel = widget(&h, "panel:right_panel"); + let r = h.request("ui.move", json!({"x": panel.center().x, "y": panel.bottom() - 40.0}), T); + assert_eq!(r["ok"], true, "{r}"); + for _ in 0..4 { + let r = h.request("ui.scroll", json!({"dy": -300.0}), T); + assert_eq!(r["ok"], true, "{r}"); + } + h.step(); + let moved = h.app.widgets.iter().find(|(w, _)| w == "slider:light.exposure").map(|(_, r)| *r); + assert!(moved.is_none_or(|r| r.top() < exposure.top() - 100.0), "the sliders scrolled: {exposure:?} → {moved:?}"); + assert_eq!(widget(&h, "histogram"), hist, "the histogram stayed where it was"); + for id in ["histogram:shadows", "histogram:highlights"] { + assert!(hist.contains_rect(widget(&h, id)), "{id} is still on the histogram"); + } +} + #[test] fn the_histogram_clipping_buttons_stay_clear_of_the_hovered_scroll_bar() { // every section open: the Edit panel scrolls, so its scroll bar runs past the histogram diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 21140ae26..a149c42f5 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-21 · **Change:** minor (LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-23 · **Change:** minor (LR-VIEW-HISTOGRAM: the histogram stays in view while the Edit panel's sliders scroll, #2679; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **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 @@ -139,7 +139,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-VIEW-SLIDESHOW | Slideshow | P2 | ✅ | `cmd:view.slideshow` | View ▸ Slideshow (⌥⌘↩): the photos in view full screen from the selected one, played with the library's playback settings (see LRC-SS-PLAYBACK: fades, random order, repeat, previews prepared ahead; `interval` / any setting overrides one run); Space pauses, ←/→ step (a cut), Esc ends; playback controls on pointer movement | | LR-VIEW-SECONDWINDOW | Second display window [Classic] | P2 | ✅ | `cmd:view.secondWindow`, `crates/ui-egui/src/panels/second.rs` | Window ▸ Second Window (⌘F11): the active photo fitted in its own native window with its own render (a floating panel where there are no native windows); loupe view only (no grid / compare / survey there) | | LR-VIEW-CLIPPING | Clipping indicators | P0 | ✅ | `cmd:view.clipping` | | -| LR-VIEW-HISTOGRAM | Histogram | P0 | ✅ | `cmd:view.histogram`, `crates/ui-egui/src/panels/edit.rs`, `crates/pipeline/src/lib.rs` (`hdr_histogram`) | HDR edits: SDR on the left half, the HDR range (0 to 5 stops above SDR white, a fixed scale) on the right with the headroom limit marked; colour bar (white, or Visualize HDR range's colours when that is on) and SDR / HDR labels; no drag-to-adjust on the histogram; it follows slider drags live (a small render beside the drafts, `Slot::HdrHistogram`) and never drops to the SDR layout mid-drag | +| LR-VIEW-HISTOGRAM | Histogram | P0 | ✅ | `cmd:view.histogram`, `crates/ui-egui/src/panels/edit.rs`, `crates/ui-egui/src/panels/right.rs`, `crates/pipeline/src/lib.rs` (`hdr_histogram`) | stays at the top of the Edit panel while its sliders scroll under it (issue #2679, `the_histogram_stays_in_view_while_the_sliders_scroll`); HDR edits: SDR on the left half, the HDR range (0 to 5 stops above SDR white, a fixed scale) on the right with the headroom limit marked; colour bar (white, or Visualize HDR range's colours when that is on) and SDR / HDR labels; no drag-to-adjust on the histogram; it follows slider drags live (a small render beside the drafts, `Slot::HdrHistogram`) and never drops to the SDR layout mid-drag | | LR-VIEW-HDR-DISPLAY | HDR display output | P2 | ⬜ | |, `apps/lightcraft/src/hdr_display.rs` (`choose`, `preferred_hdr_format`) | SDR/HDR decision (platform, surface format, OS headroom, `LIGHTCRAFT_HDR_DISPLAY=0`) is unit-tested and logged at start; always SDR today because eframe configures an 8-bit surface; no OS headroom query yet; | ## D. Search & filter (FILT) @@ -798,6 +798,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-23 | patch | LR-VIEW-HISTOGRAM: the histogram stays at the top of the Edit panel while its sliders scroll (#2679); status unchanged | | 2026-10-21 | patch | LR-IMP-ADD-DIALOG: reconcile #2362 with current main; deleted-album guard and hierarchical picker retained; counts unchanged | | 2026-10-20 | minor | LR-IMP-ADD-DIALOG: #2362 gives the import review the shared hierarchy/search album picker while retaining None/New album and existing destination IDs; status and feature counts unchanged | | 2026-10-21 | patch | KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the slider's row; leaving it (even between two frames) or Tab focus alone does not arm them (issue #1355); status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..0848c987f 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,11 @@ # What's new in LightCraft +## 2026-10-23 + +### The histogram stays in view +- The histogram stays at the top of the Edit panel while its sliders scroll under it (issue #2679). It used to scroll + away with them, so it was out of sight while using the sliders further down the panel. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges From 408b3ec844a3f69818f9117e78869504b380923a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Lu=C3=ADs=20da=20Silva?= Date: Fri, 23 Oct 2026 01:24:32 -0300 Subject: [PATCH 10/29] Never crash: ARW and SRW strips shorter than declared are refused before allocating (#2561) The ARW one-byte and packed 12-bit strips and the SRW word and packed 12-bit strips sized the frame from the declared StripByteCounts, while chunk_bytes clamps the strip to the file. A file of a few hundred bytes that declares a large frame made the full decode allocate it (up to 2 GiB) and unpack zeros. Both now check the bytes present against the declared strip first and return Corrupt, as the ORF block path and RAF already did. --- crates/raw/src/vendor/arw.rs | 30 ++++++++++++++++++++++++++++++ crates/raw/src/vendor/mod.rs | 18 ++++++++++++++++++ crates/raw/src/vendor/srw.rs | 15 +++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/crates/raw/src/vendor/arw.rs b/crates/raw/src/vendor/arw.rs index 3938fa50d..c40475b1b 100644 --- a/crates/raw/src/vendor/arw.rs +++ b/crates/raw/src/vendor/arw.rs @@ -642,6 +642,9 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { } 32767 if one_byte_per_sample => { let src = chunk_bytes(bytes, &chunks[0]).ok_or_else(|| RawError::Corrupt("raw strip outside file".into()))?; + if src.len() as u64 != strip_len { + return Err(RawError::Corrupt("raw strip shorter than its rows".into())); + } let curve = code_curve(&raw.u64s(TONE_CURVE).unwrap_or_else(|| vec![8000, 10400, 12900, 14100])); let mut data = vec![0u16; samples as usize]; data.par_chunks_mut(w).enumerate().for_each(|(y, out)| { @@ -659,6 +662,9 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { } 32767 if packed12 => { let src = chunk_bytes(bytes, &chunks[0]).ok_or_else(|| RawError::Corrupt("raw strip outside file".into()))?; + if src.len() as u64 != strip_len { + return Err(RawError::Corrupt("raw strip shorter than its rows".into())); + } let row_bytes = w / 2 * 3; let mut data = vec![0u16; samples as usize]; data.par_chunks_mut(w).enumerate().for_each(|(y, out)| { @@ -1561,4 +1567,28 @@ mod tests { assert!(is_quad_tiled(&file, &info)); assert_eq!(read_quad_tiles(&file, &info).unwrap(), mosaic); } + + /// #2561: a small file whose one-byte or packed 12-bit strip declares a large frame is refused before the full + /// decode allocates the frame. + #[test] + fn a_strip_cut_short_is_refused_before_allocating() { + use lightcraft_tiff::{IfdBuilder, ImageData, TiffWriter, Value}; + let (w, h) = (3000u32, 2000u32); + for declared in [w * h, w * h / 2 * 3] { + let mut raw = IfdBuilder::new(); + raw.set(t::MAKE, Value::Ascii("SONY".into())); + raw.set(t::MODEL, Value::Ascii("DSLR-A100".into())); + raw.set(t::IMAGE_WIDTH, Value::Long(vec![w])); + raw.set(t::IMAGE_LENGTH, Value::Long(vec![h])); + raw.set(t::BITS_PER_SAMPLE, Value::Short(vec![12])); + raw.set(t::SAMPLES_PER_PIXEL, Value::Short(vec![1])); + raw.set(t::PHOTOMETRIC, Value::Short(vec![photometric::CFA])); + raw.set(t::COMPRESSION, Value::Short(vec![32767])); + raw.set(TONE_CURVE, Value::Short(vec![8000, 10400, 12900, 14100])); + raw.set_image(ImageData::Strips { rows_per_strip: h, strips: vec![vec![0u8; 12]] }); + let mut f = TiffWriter::default().write(&[raw]).unwrap(); + crate::vendor::declare_strip_len(&mut f, 12, declared); + assert!(matches!(decode(&f, Mode::Full), Err(RawError::Corrupt(_))), "{declared} bytes declared"); + } + } } diff --git a/crates/raw/src/vendor/mod.rs b/crates/raw/src/vendor/mod.rs index b0f65f49e..ab3c8b7e9 100644 --- a/crates/raw/src/vendor/mod.rs +++ b/crates/raw/src/vendor/mod.rs @@ -225,6 +225,24 @@ fn solve4(mut a: [[f64; 4]; 4], mut b: [f64; 4]) -> Option<[f64; 4]> { x.iter().all(|v| v.is_finite()).then_some(x) } +/// Test helper: make the single-strip `StripByteCounts` entry (LONG, count 1) that holds `len` declare `declared` +/// bytes instead, as a file cut short or a hostile header does. +#[cfg(test)] +pub(crate) fn declare_strip_len(file: &mut [u8], len: u32, declared: u32) { + let big = file.starts_with(b"MM"); + let entry = |v: u32| -> Vec { + if big { + [&279u16.to_be_bytes()[..], &4u16.to_be_bytes(), &1u32.to_be_bytes(), &v.to_be_bytes()].concat() + } else { + [&279u16.to_le_bytes()[..], &4u16.to_le_bytes(), &1u32.to_le_bytes(), &v.to_le_bytes()].concat() + } + }; + let (old, new) = (entry(len), entry(declared)); + let at: Vec = file.windows(old.len()).enumerate().filter(|(_, w)| *w == old.as_slice()).map(|(i, _)| i).collect(); + assert_eq!(at.len(), 1, "one StripByteCounts entry of {len} bytes"); + file[at[0]..at[0] + new.len()].copy_from_slice(&new); +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/raw/src/vendor/srw.rs b/crates/raw/src/vendor/srw.rs index b22faeec0..17a2a1d70 100644 --- a/crates/raw/src/vendor/srw.rs +++ b/crates/raw/src/vendor/srw.rs @@ -107,6 +107,9 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { }; let data = if mode == Mode::Full { let src = chunk_bytes(bytes, &chunks[0]).ok_or_else(|| RawError::Corrupt("SRW strip outside file".into()))?; + if src.len() as u64 != strip { + return Err(RawError::Corrupt("SRW strip shorter than its rows".into())); + } let mut d = vec![0u16; n]; match coding { Coding::Words => read_u16s(src, ByteOrder::Little, &mut d), @@ -540,4 +543,16 @@ mod tests { let _ = probe_info(&bytes[..cut]); } } + + /// #2561: a small file whose strip declares a large frame is refused before the full decode allocates the frame. + #[test] + fn a_strip_cut_short_is_refused_before_allocating() { + let (w, h) = (3000u32, 2000u32); + for (compression, declared) in [(32769, w * h * 2), (32770, w * h / 2 * 3)] { + let mut f = srw(Spec { w, h, compression, strip: vec![0u8; 12], pattern: Some([0, 1, 1, 2]), note: None, picture: None }); + crate::vendor::declare_strip_len(&mut f, 12, declared); + assert_eq!(probe(&f), Some(crate::RawFormat::Srw)); + assert!(matches!(decode(&f), Err(RawError::Corrupt(_))), "compression {compression}"); + } + } } From 3f873bbad060f772c24ef9c357a24b7a493ebca0 Mon Sep 17 00:00:00 2001 From: Dominik Strebel Date: Sat, 24 Oct 2026 12:43:59 +0200 Subject: [PATCH 11/29] docs(whats-new): only LightCraft's own raw renders are redrawn, not the camera's embedded preview --- docs/whats-new.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/whats-new.md b/docs/whats-new.md index b4ec250bd..cf145c1f7 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -7,7 +7,8 @@ strokes instead of grain, often orange and blue under warm light. Noise reduction made them stand out more, because it smooths the grain around them. The demosaic chose between a horizontal and a vertical guess at each pixel, and in a flat area the noise alone decided which. Where neither guess is clearly better it now blends the two, so the - noise looks like grain. Edges are as sharp as before. Thumbnails and previews of raws are redrawn. + noise looks like grain. Edges are as sharp as before. Thumbnails and previews that LightCraft rendered from a raw + are redrawn; the camera's own embedded preview, shown for an unedited raw until its render is ready, is unchanged. ### Tab leaves the name field in New Album, New Smart Album and the name prompts - In New Album, New Smart Album, Save Metadata Preset and the other dialogs with one name field, the field has the From d0670de8cec51b1fa07837e1d5010c6c1aafc2bb Mon Sep 17 00:00:00 2001 From: Julian Zahnow <54999052+Enthendor@users.noreply.github.com> Date: Sat, 24 Oct 2026 15:50:00 +0200 Subject: [PATCH 12/29] fix(ui): wrap the Edit header buttons instead of letting Reset cover HDR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reset was laid out right-to-left in whatever width Auto, B&W and HDR left, so a long label drew over HDR: German Zurücksetzen in the default panel, and 16 of the 18 languages at some panel width. A button that doesn't fit now starts the next row; Reset stays right-aligned. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/edit.rs | 154 +++++++++++++++++++++++++----- crates/ui-egui/src/widgets.rs | 14 ++- 2 files changed, 141 insertions(+), 27 deletions(-) diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index 1a770ebd8..dd8fa7db5 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -107,32 +107,19 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { let (hr, _) = ui.allocate_exact_size(vec2(ui.available_width(), 40.0), Sense::hover()); ui.painter().text(pos2(hr.left() + 24.0, hr.bottom() - 10.0), Align2::LEFT_CENTER, crate::i18n::tr("Edit"), t.semibold(15.0), t.text); egui::Frame::NONE.inner_margin(egui::Margin { left: 24, right: 22, top: 8, bottom: 14 }).show(ui, |ui| { - ui.horizontal(|ui| { - ui.spacing_mut().item_spacing.x = 8.0; - if text_button(ui, "auto", crate::i18n::tr("Auto"), false).clicked() { - let _ = app.run("develop.auto", json!({})); - app.toast(ui.ctx(), crate::i18n::tr("Auto settings applied")); - } - let bw = crate::is_bw(&d); - if text_button(ui, "bw", crate::i18n::tr("B&W"), bw).clicked() { - let _ = app.run("develop.treatment", json!({})); - } - // HDR editing, next to B&W as Lightroom has it - if text_button(ui, "hdr", crate::i18n::tr("HDR"), d.hdr.enabled) - .on_hover_text(crate::i18n::tr("Edit in HDR: highlights above SDR white, for HDR exports and displays")) - .clicked() - { - let _ = app.run("develop.hdr", json!({"enabled": !d.hdr.enabled})); - } - ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - if text_button(ui, "reset", crate::i18n::tr("Reset"), false) - .on_hover_text(reset_tip(&app.ui.settings.keymap, ui.ctx().os() == egui::os::OperatingSystem::Mac)) - .clicked() - { - reset_clicked = true; - } - }); - }); + let tip = reset_tip(&app.ui.settings.keymap, ui.ctx().os() == egui::os::OperatingSystem::Mac); + let clicks = header_buttons(ui, crate::is_bw(&d), d.hdr.enabled, &tip); + if clicks.auto { + let _ = app.run("develop.auto", json!({})); + app.toast(ui.ctx(), crate::i18n::tr("Auto settings applied")); + } + if clicks.bw { + let _ = app.run("develop.treatment", json!({})); + } + if clicks.hdr { + let _ = app.run("develop.hdr", json!({"enabled": !d.hdr.enabled})); + } + reset_clicked = clicks.reset; }); if let Some(why) = &preview_only { let reload = egui::Frame::NONE @@ -1932,6 +1919,75 @@ pub(crate) fn reload_preview_only(app: &mut LightcraftApp, ctx: &egui::Context, } /// The Reset button's tooltip with the reset shortcut as this platform writes it (issue #1751). +/// Which of the Edit header's buttons were clicked. +#[derive(Default)] +struct HeaderClicks { + auto: bool, + bw: bool, + hdr: bool, + reset: bool, +} + +/// The Edit header's buttons: Auto, B&W and HDR on the left, Reset on the right. A button that doesn't fit on the +/// row starts the next one (long translations in a narrow panel, such as German *Zurücksetzen* beside HDR), so none +/// covers another; Reset stays right-aligned on whichever row it lands. +fn header_buttons(ui: &mut egui::Ui, bw: bool, hdr: bool, reset_tip: &str) -> HeaderClicks { + const GAP: f32 = 8.0; + let mut clicks = HeaderClicks::default(); + let avail = ui.available_width(); + let left = [("auto", "Auto", false), ("bw", "B&W", bw), ("hdr", "HDR", hdr)]; + // rows of `left` indices, filled greedily; `used` is the width taken on the current (last) row + let mut rows: Vec> = Vec::new(); + let mut row: Vec = Vec::new(); + let mut used = 0.0; + for (i, (_, label, _)) in left.iter().enumerate() { + let w = crate::widgets::text_button_size(ui, label); + if row.is_empty() { + used = w; + } else if used + GAP + w > avail { + rows.push(std::mem::take(&mut row)); + used = w; + } else { + used += GAP + w; + } + row.push(i); + } + rows.push(row); + let reset_on_last = used + GAP + crate::widgets::text_button_size(ui, "Reset") <= avail; + let reset = |ui: &mut egui::Ui, clicks: &mut HeaderClicks| { + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + clicks.reset = text_button(ui, "reset", crate::i18n::tr("Reset"), false).on_hover_text(reset_tip).clicked(); + }); + }; + let last = rows.len() - 1; + for (r, row) in rows.iter().enumerate() { + ui.horizontal(|ui| { + ui.spacing_mut().item_spacing.x = GAP; + for &i in row { + let (id, label, active) = left[i]; + let mut resp = text_button(ui, id, crate::i18n::tr(label), active); + if id == "hdr" { + // HDR editing, next to B&W as Lightroom has it + resp = resp.on_hover_text(crate::i18n::tr("Edit in HDR: highlights above SDR white, for HDR exports and displays")); + } + let clicked = resp.clicked(); + match id { + "auto" => clicks.auto = clicked, + "bw" => clicks.bw = clicked, + _ => clicks.hdr = clicked, + } + } + if r == last && reset_on_last { + reset(ui, &mut clicks); + } + }); + } + if !reset_on_last { + ui.horizontal(|ui| reset(ui, &mut clicks)); + } + clicks +} + fn reset_tip(keymap: &crate::shortcuts::Keymap, mac: bool) -> String { let base = crate::i18n::tr("Reset all edits"); match crate::shortcuts::shortcut_of(keymap, "develop.reset") { @@ -1969,6 +2025,52 @@ mod curve_histogram_tests { mod tests { use super::*; + /// The Edit header's buttons never cover each other or run past the panel, in any language: a button that doesn't + /// fit starts the next row, and Reset stays right-aligned. Before, Reset covered HDR in 11 of the 18 languages at + /// the default 222 pt (German *Zurücksetzen* among them) and in all but the two Chinese ones at some width. 200 pt + /// is about the narrowest panel (`RIGHT_WIDTH.min` less the margins). + #[test] + fn edit_header_buttons_do_not_overlap_in_any_language() { + use crate::i18n::Locale; + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + crate::theme::apply(&ctx); + let mut wrapped = Vec::new(); + for &locale in Locale::ALL { + crate::i18n::set_language(locale); + for width in [200.0, 222.0, 260.0, 400.0] { + let mut rects = Vec::new(); + for frame in 0..2 { + let raw = crate::headless::HeadlessView::raw_input(vec2(width, 200.0), 1.0, frame as f64 / 60.0, vec![]); + let mut out = ctx.run_ui(raw, |ui| { + egui::CentralPanel::default().frame(egui::Frame::NONE).show(ui, |ui| { + header_buttons(ui, false, false, ""); + }); + }); + out.textures_delta.clear(); + rects = crate::widgets::take_registry(&ctx); + } + let get = |id: &str| rects.iter().find(|(i, _)| i == id).map(|(_, r)| *r).unwrap(); + let buttons = ["button:auto", "button:bw", "button:hdr", "button:reset"].map(get); + for (i, a) in buttons.iter().enumerate() { + assert!(a.right() <= width + 0.5, "{locale:?} {width}: button {i} past the edge: {a:?}"); + for b in &buttons[i + 1..] { + assert!(!a.intersects(b.shrink(0.5)), "{locale:?} {width}: {a:?} overlaps {b:?}"); + } + } + let [_, _, hdr, reset] = buttons; + assert!((reset.right() - width).abs() < 0.5, "{locale:?} {width}: Reset stays right-aligned: {reset:?}"); + if reset.top() > hdr.top() { + wrapped.push((locale, width)); + } + } + } + crate::i18n::set_language(Locale::En); + assert!(wrapped.contains(&(Locale::De, 222.0)), "German wraps in the default panel: {wrapped:?}"); + // English keeps its single row from the default width up + assert!(!wrapped.iter().any(|&(l, w)| l == Locale::En && w >= 222.0), "{wrapped:?}"); + } + #[test] fn an_hdr_edit_keeps_its_hdr_histogram_through_8_bit_renders() { let ctx = egui::Context::default(); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 01d5b65f8..1f1987d3b 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -694,7 +694,7 @@ pub fn text_button(ui: &mut Ui, id: &str, label: &str, active: bool) -> Response let t = Tokens::get(ui.ctx()); let font = t.semibold(11.5); let galley = ui.painter().layout_no_wrap(label.to_string(), font, t.text); - let size = vec2((galley.size().x + 18.0).max(37.0), 24.0); + let size = vec2(text_button_width(galley.size().x), 24.0); let (r, resp) = ui.allocate_exact_size(size, Sense::click()); resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::Button, true, active, label)); register(ui.ctx(), format!("button:{id}"), r); @@ -711,6 +711,18 @@ pub fn text_button(ui: &mut Ui, id: &str, label: &str, active: bool) -> Response resp } +/// The width of a [`text_button`] whose label is `text_width` wide. +fn text_button_width(text_width: f32) -> f32 { + (text_width + 18.0).max(37.0) +} + +/// The width [`text_button`] takes for `label` (translated as it would be), to plan a row before drawing it. +pub fn text_button_size(ui: &Ui, label: &str) -> f32 { + let t = Tokens::get(ui.ctx()); + let galley = ui.painter().layout_no_wrap(crate::i18n::tr(label).to_string(), t.semibold(11.5), t.text); + text_button_width(galley.size().x) +} + /// A segmented control: `labels` share the available width equally, `per_row` per row. Each segment is /// addressable as `button:{id}-{key}` (keys parallel to labels). Returns the clicked index. pub fn segmented(ui: &mut Ui, id: &str, items: &[(&str, &str)], active: Option, per_row: usize) -> Option { From f621d24753b71048835c7a18c4d67f9bfb6c564c Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Mon, 19 Oct 2026 23:23:42 +1100 Subject: [PATCH 13/29] LR-EDIT-EFFECTS-GRAIN: skip grain on a degenerate frame instead of producing NaN pixels A zero-width crop mapped output pixels to NaN/inf grain coordinates, so grain turned the whole render into NaN (CPU path and GPU params). Grain is now skipped when the frame has no finite mapping, grain_noise returns 0 for non-finite coordinates, and a non-finite grain term is never added. Sources: own design. (cherry picked from commit 4023853f332fc39586190429d630cff8dfaae1ba) Co-Authored-By: Claude Opus 5.5 --- crates/pipeline/src/finish.rs | 27 ++++++++++++++++++++++++--- crates/pipeline/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/crates/pipeline/src/finish.rs b/crates/pipeline/src/finish.rs index c1e6a9bc2..b7ef8644a 100644 --- a/crates/pipeline/src/finish.rs +++ b/crates/pipeline/src/finish.rs @@ -78,6 +78,10 @@ pub const GRAIN_HASH: [u32; 4] = [0x8da6_b343, 0xd816_3841, 0xcb1a_b31f, 0x5bd1_ #[inline] fn grain_noise(x: f32, y: f32, seed: u32) -> f32 { + // A degenerate frame (zero-width crop) maps pixels to NaN/inf coordinates: no grain there. + if !x.is_finite() || !y.is_finite() { + return 0.0; + } let (x0, y0) = (x.floor(), y.floor()); let (fx, fy) = (x - x0, y - y0); let h = |i: i32, j: i32| { @@ -207,7 +211,15 @@ impl FinishParams { }; let ev = s.light.exposure as f32; let gain = 2f32.powf(ev); - let grain = (s.grain.amount > 0.0 && effects).then(|| { + // A degenerate frame (zero-width crop, zero scale) has no finite grain mapping: skip grain (CPU and GPU). + let out_to_norm = frame.out_to_norm(w, h); + let finite_frame = px_per_long.is_finite() + && px_per_long > 0.0 + && frame.ow.is_finite() + && frame.oh.is_finite() + && frame.ow.max(frame.oh) > 0.0 + && out_to_norm.0.iter().all(|v| v.is_finite()); + let grain = (s.grain.amount > 0.0 && effects && finite_frame).then(|| { let cell = (0.0006 + (s.grain.size / 100.0) as f32 * 0.0024) * px_per_long as f32; ((s.grain.amount / 100.0) as f32 * 0.13, cell.max(0.6), (s.grain.roughness / 100.0) as f32, s.grain.seed) }); @@ -239,7 +251,7 @@ impl FinishParams { gain, ev, grain, - out_to_norm: frame.out_to_norm(w, h), + out_to_norm, ow: frame.ow, oh: frame.oh, w, @@ -642,7 +654,9 @@ pub(crate) fn finish_with( lum = lum.clamp(0.0, 1.0); } let k = amt * g * (0.35 + 2.6 * lum * (1.0 - lum)); - e = e.map(|v| v + k); + if k.is_finite() { + e = e.map(|v| v + k); + } } let e = match &p_lut { Some((l, k)) if hdr => { @@ -767,6 +781,13 @@ fn enc(v: f32) -> u8 { mod tests { use super::*; + #[test] + fn grain_noise_is_finite_for_degenerate_coordinates() { + for (x, y) in [(f32::NAN, 1.0), (1.0, f32::INFINITY), (f32::NEG_INFINITY, f32::NAN)] { + assert_eq!(grain_noise(x, y, 7), 0.0); + } + } + #[test] fn refine_saturation_restores_the_pre_curve_saturation() { let before = [0.5, 0.3, 0.2]; diff --git a/crates/pipeline/src/tests.rs b/crates/pipeline/src/tests.rs index 23eca0ba7..5c5f1b95c 100644 --- a/crates/pipeline/src/tests.rs +++ b/crates/pipeline/src/tests.rs @@ -840,3 +840,19 @@ fn sliver_crops_render_quickly_with_wide_stages_on() { assert!(r.image.width >= 1 && r.image.height >= 1, "{rect:?}"); } } + +/// A crop of no width used to give grain NaN coordinates (and NaN pixels); the render stays finite. +#[test] +fn zero_width_crop_with_grain_renders_finite_pixels() { + let mut s = DevelopSettings::default(); + s.grain.amount = 60.0; + s.crop.geometry.rect = lightcraft_geom::Rect::new(0.5, 0.1, 0.5, 0.9); + let mut req = RenderRequest::fit(240, 240); + req.depth = crate::OutputDepth::F32Linear; + let r = render(&scene(), &SourceInfo::default(), &s, &req); + if let Some(d) = r.deep + && let crate::DeepSamples::F32(v) = d.samples + { + assert!(v.iter().all(|x| x.is_finite()), "non-finite sample in a zero-width crop with grain"); + } +} From 1fd7332de46290213543370a617d8d227f02644e Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:53:52 +0000 Subject: [PATCH 14/29] Reject signed list indexes in indexed develop control ids Found while testing crates/develop/src/controls.rs:413 (split_indexed) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/develop/src/controls.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/develop/src/controls.rs b/crates/develop/src/controls.rs index 715e8ac33..4d937ab21 100644 --- a/crates/develop/src/controls.rs +++ b/crates/develop/src/controls.rs @@ -410,6 +410,9 @@ pub static INDEXED: &[ControlSpec] = &[ fn split_indexed(id: &str) -> Option<(&str, usize, &str)> { let mut it = id.splitn(3, '.'); let (fam, i, field) = (it.next()?, it.next()?, it.next()?); + if i.is_empty() || !i.bytes().all(|b| b.is_ascii_digit()) { + return None; + } Some((fam, i.parse().ok()?, field)) } @@ -538,6 +541,18 @@ mod tests { } } + #[test] + fn signed_indexes_are_unknown_ids() { + let mut s = DevelopSettings::default(); + s.point_colors.push(Default::default()); + let before = s.point_colors[0].hue_shift; + assert!(!set(&mut s, "pointColor.+0.hueShift", 10.0), "signed index"); + assert_eq!(s.point_colors[0].hue_shift, before); + assert_eq!(get(&s, "pointColor.+0.hueShift"), None); + assert!(find("pointColor.+0.hueShift").is_none()); + assert!(find("redEye.+0.darken").is_none()); + } + #[test] fn formatting() { let e = find("light.exposure").unwrap(); From 0474ea9e1bb2eb809a02f6a3173301dbc5f709fe Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:26:19 +0000 Subject: [PATCH 15/29] Drop the sign from slider text that rounds to zero Found while testing crates/develop/src/controls.rs:98 (ControlSpec::format) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/develop/src/controls.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/develop/src/controls.rs b/crates/develop/src/controls.rs index 715e8ac33..f58cb17bb 100644 --- a/crates/develop/src/controls.rs +++ b/crates/develop/src/controls.rs @@ -97,6 +97,10 @@ impl ControlSpec { } pub fn format(&self, v: f64) -> String { let s = format!("{:.*}", self.decimals as usize, v); + if v.is_finite() && !s.bytes().any(|b| matches!(b, b'1'..=b'9')) { + // Rounds to zero: no sign. + return s.trim_start_matches('-').to_string(); + } if v > 0.0 && self.min < 0.0 { format!("+{s}") } else { s } } /// Where `v` sits on the slider track, from 0 (left end) to 1 (right end). Linear, except for the Kelvin @@ -493,6 +497,17 @@ pub fn in_section(section: Section) -> impl Iterator Date: Thu, 22 Oct 2026 15:05:37 +0300 Subject: [PATCH 16/29] Reset the profile section to the default profile A photo that was given a creative or film profile cannot get back to its neutral look through the Profile section's reset, which is what the report describes as edits that refuse to go back to zero. Make reset_section restore the whole profile (id and amount) when the Profile section is reset, so the section really returns to its default state. Test: resetting_the_profile_section_restores_the_default_profile failed before the change and passes after. --- crates/develop/src/lib.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/develop/src/lib.rs b/crates/develop/src/lib.rs index 4631e5e49..f27aa9c23 100644 --- a/crates/develop/src/lib.rs +++ b/crates/develop/src/lib.rs @@ -145,7 +145,8 @@ impl DevelopSettings { } } - /// Reset every control in `section` to its default. + /// Reset every control in `section` to its default, and the settings of the section that are not + /// controls (the creative profile, the curve, the white balance mode). pub fn reset_section(&mut self, section: Section) { for c in controls::in_section(section) { controls::set(self, c.id, c.default); @@ -157,6 +158,8 @@ impl DevelopSettings { } Section::Color => self.wb.mode = WbMode::AsShot, Section::Detail => self.enhance.denoise_on = None, + // the creative profile is not a control: the chosen look goes back to the default one + Section::Profile => self.profile.id = Profile::default().id, _ => {} } } @@ -316,6 +319,17 @@ mod tests { assert!(s.is_unedited()); } + #[test] + fn resetting_the_profile_section_restores_the_default_profile() { + // the chosen creative profile is not a control, so resetting the section must restore it too + let mut s = DevelopSettings::default(); + s.profile.id = "lc.muted.matte-soft".into(); + s.profile.amount = 40.0; + s.reset_section(Section::Profile); + assert_eq!(s.profile.id, "lc.color", "the creative profile survives a Profile reset"); + assert_eq!(s.profile.amount, 100.0); + } + #[test] fn section_toggles() { let mut s = DevelopSettings::default(); From c73bf309870bd00e67671768bbca5d43d63b976b Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:57:00 +0300 Subject: [PATCH 17/29] Address review findings on the new test and fix --- crates/develop/src/lib.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/develop/src/lib.rs b/crates/develop/src/lib.rs index f27aa9c23..acdf4836c 100644 --- a/crates/develop/src/lib.rs +++ b/crates/develop/src/lib.rs @@ -158,8 +158,13 @@ impl DevelopSettings { } Section::Color => self.wb.mode = WbMode::AsShot, Section::Detail => self.enhance.denoise_on = None, - // the creative profile is not a control: the chosen look goes back to the default one - Section::Profile => self.profile.id = Profile::default().id, + // the creative profile is not a control: the chosen look goes back to the default one, + // and so does the camera's own local tone mapping + Section::Profile => { + let d = Profile::default(); + self.profile.id = d.id; + self.profile.camera_local_tone = d.camera_local_tone; + } _ => {} } } @@ -330,6 +335,15 @@ mod tests { assert_eq!(s.profile.amount, 100.0); } + #[test] + fn resetting_the_profile_section_clears_the_camera_local_tone() { + // the camera's local tone mapping is not a control either: a Profile reset switches it off + let mut s = DevelopSettings::default(); + s.profile.camera_local_tone = true; + s.reset_section(Section::Profile); + assert!(!s.profile.camera_local_tone); + } + #[test] fn section_toggles() { let mut s = DevelopSettings::default(); From 34a36d87987f3a132f4553ed21904210eef17670 Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:29:36 +0300 Subject: [PATCH 18/29] Correct command documentation requested in review --- crates/engine/src/cmd/develop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs index fbc3d4041..f5e362a0e 100644 --- a/crates/engine/src/cmd/develop.rs +++ b/crates/engine/src/cmd/develop.rs @@ -333,7 +333,7 @@ pub fn specs() -> Vec { "Reset Section", [], None, - "{section: light|curve|color|mixer|grading|effects|vignette|grain|detail|optics|geometry}", + "{section: profile|light|curve|color|mixer|grading|effects|vignette|grain|detail|optics|geometry}", has_active, |s, p| { let sec = section_param(p, "develop.resetSection")?; From b0ef53e986f91ca250840baddfb62063c6f957ee Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Wed, 28 Oct 2026 06:49:31 +0200 Subject: [PATCH 19/29] Tighten the change after a final review --- crates/develop/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/develop/src/lib.rs b/crates/develop/src/lib.rs index acdf4836c..72fd5133c 100644 --- a/crates/develop/src/lib.rs +++ b/crates/develop/src/lib.rs @@ -161,9 +161,7 @@ impl DevelopSettings { // the creative profile is not a control: the chosen look goes back to the default one, // and so does the camera's own local tone mapping Section::Profile => { - let d = Profile::default(); - self.profile.id = d.id; - self.profile.camera_local_tone = d.camera_local_tone; + self.profile = Profile::default(); } _ => {} } @@ -331,7 +329,7 @@ mod tests { s.profile.id = "lc.muted.matte-soft".into(); s.profile.amount = 40.0; s.reset_section(Section::Profile); - assert_eq!(s.profile.id, "lc.color", "the creative profile survives a Profile reset"); + assert_eq!(s.profile.id, "lc.color", "the creative profile is reset to the default one"); assert_eq!(s.profile.amount, 100.0); } From 63b6de336fe29614eeaab867c355d13f96846bef Mon Sep 17 00:00:00 2001 From: radialmonster Date: Sun, 25 Oct 2026 20:22:54 -0400 Subject: [PATCH 20/29] raw: CR3 preview box scan checks 32-bit lengths for overflow (#2569) On wasm32 a PRVW/THMB box or JPEG length near u32::MAX wrapped j + len and start + size, so both bounds checks passed and the slice panicked (abort in the web build). Use checked_add and bytes.get, and add a regression test. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014ti8KemZbjE2ZgHgV1q1BM --- crates/raw/src/preview.rs | 6 ++++-- crates/raw/src/preview/tests.rs | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/raw/src/preview.rs b/crates/raw/src/preview.rs index 2047174da..2403e5035 100644 --- a/crates/raw/src/preview.rs +++ b/crates/raw/src/preview.rs @@ -438,10 +438,12 @@ fn cr3_preview_boxes(bytes: &[u8]) -> Option<&[u8]> { let be32 = |at: usize| bytes.get(at..at + 4).map(|s| u32::from_be_bytes([s[0], s[1], s[2], s[3]]) as usize); let (Some(size), Some(len)) = (be32(start), be32(start + 20)) else { continue }; let j = start + 24; - if len < 4 || j + len > start + size.max(24) || j + len > bytes.len() { + // 32-bit box lengths can wrap `usize` on wasm32: check the sums, then slice with `get` + let (Some(end), Some(lim)) = (j.checked_add(len), start.checked_add(size.max(24))) else { continue }; + if len < 4 || end > lim { continue; } - let jpeg = &bytes[j..j + len]; + let Some(jpeg) = bytes.get(j..end) else { continue }; // One that doesn't fit the budget left is skipped; a smaller one later may still fit. if walked + jpeg.len() > CR3_MAX_WALK { continue; diff --git a/crates/raw/src/preview/tests.rs b/crates/raw/src/preview/tests.rs index 2ee098414..f526e868b 100644 --- a/crates/raw/src/preview/tests.rs +++ b/crates/raw/src/preview/tests.rs @@ -199,6 +199,20 @@ fn picks_largest_dct_jpeg() { } } +/// #2569: `PRVW` box and JPEG lengths near `u32::MAX` (which wrap `usize` on wasm32) find no preview, never panic. +#[test] +fn cr3_preview_box_huge_lengths_are_rejected() { + let j = fake_jpeg(300); + for (size, len) in [(u32::MAX, u32::MAX), (u32::MAX, u32::MAX - 23), (24 + j.len() as u32, u32::MAX - 3)] { + let mut cr3 = b"ftypcrx crx isom".to_vec(); + cr3.extend_from_slice(&size.to_be_bytes()); + cr3.extend_from_slice(b"PRVWT8"); + cr3.extend_from_slice(&len.to_be_bytes()); + cr3.extend_from_slice(&j); + assert_eq!(cr3_preview_boxes(&cr3), None, "size {size:#x}, len {len:#x}"); + } +} + /// A JPEG like [`fake_jpeg`] whose frame header says `w` x `h` with `nc` components. fn sized_jpeg(w: u16, h: u16, nc: u8, n: usize) -> Vec { let mut j = vec![0xff, 0xd8, 0xff, 0xc0, 0x00, 8 + 3 * nc, 8]; From 42e54b5bb08317295d361b7fadc17fe5b240642d Mon Sep 17 00:00:00 2001 From: Christian Bram Date: Thu, 29 Oct 2026 14:55:24 +0100 Subject: [PATCH 21/29] LR-IMP-FORMATS: decode compressed RAF 13-17 % faster with a buffered bit reader The Fujifilm entropy reader read each unary prefix one bit at a time and refilled its accumulator a byte at a time. It now keeps up to 64 bits buffered and counts a prefix's zeros with leading_zeros, failing exactly where reading bit by bit would (escape limit, end of stream). A property test checks it against reading bit by bit. Sensor arrays are unchanged bit for bit: the 17 reference arrays of corpus_fujifilm_compressed_samples, plus decode checksums of 20 X-Trans CMOS 4 files. Decode on an Apple M2: X-S10 lossless 265 -> 226 ms, X-H2 448 -> 378 ms, GFX 100 762 -> 644 ms (docs/raf-compression.md). Co-Authored-By: Claude Opus 5.5 --- .../raw/proptest-regressions/vendor/rafc.txt | 7 ++ crates/raw/src/vendor/rafc.rs | 88 +++++++++++++++++-- docs/raf-compression.md | 4 +- 3 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 crates/raw/proptest-regressions/vendor/rafc.txt diff --git a/crates/raw/proptest-regressions/vendor/rafc.txt b/crates/raw/proptest-regressions/vendor/rafc.txt new file mode 100644 index 000000000..7633abe0f --- /dev/null +++ b/crates/raw/proptest-regressions/vendor/rafc.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 8cb6b5999a8279e655a148a18b220d75b02c519f644583f4593166286297a3f4 # shrinks to data = [0, 0, 0, 8], reads = [(false, 13, 0), (true, 0, 15)] diff --git a/crates/raw/src/vendor/rafc.rs b/crates/raw/src/vendor/rafc.rs index f3b925b51..fd08f9869 100644 --- a/crates/raw/src/vendor/rafc.rs +++ b/crates/raw/src/vendor/rafc.rs @@ -114,26 +114,57 @@ impl<'a> Bits<'a> { Self { src, pos: 0, acc: 0, available: 0 } } + /// Tops the accumulator up to at least 57 bits, or to every remaining byte near the end of the stream. #[inline] - fn get(&mut self, n: u32) -> Result { - while self.available < n { - let b = self.src.get(self.pos).copied().ok_or_else(|| corrupt("truncated entropy stream"))?; + fn refill(&mut self) { + while self.available <= 56 { + let Some(&b) = self.src.get(self.pos) else { return }; self.pos += 1; self.acc = (self.acc << 8) | u64::from(b); self.available += 8; } + } + + #[inline] + fn get(&mut self, n: u32) -> Result { + if self.available < n { + self.refill(); + if self.available < n { + return Err(corrupt("truncated entropy stream")); + } + } self.available -= n; Ok(((self.acc >> self.available) & ((1u64 << n) - 1)) as u32) } - fn residual(&mut self, k: u32, bits: u32, range: u32, limit: u32) -> Result { + /// Reads the unary prefix: the zeros before the next one bit, which is consumed. Errors exactly where reading + /// bit by bit would: more than `limit` zeros, or a stream that ends first. + #[inline] + fn zeros(&mut self, limit: u32) -> Result { let mut zeros = 0; - while self.get(1)? == 0 { - zeros += 1; - if zeros > limit { + loop { + if self.available == 0 { + self.refill(); + if self.available == 0 { + return Err(corrupt("truncated entropy stream")); + } + } + // the unread bits, left-aligned; the zeros shifted in below them are cut off by `min` + let run = (self.acc << (64 - self.available)).leading_zeros().min(self.available); + if zeros + run > limit { return Err(corrupt("invalid residual escape")); } + zeros += run; + if run < self.available { + self.available -= run + 1; + return Ok(zeros); + } + self.available = 0; } + } + + fn residual(&mut self, k: u32, bits: u32, range: u32, limit: u32) -> Result { + let zeros = self.zeros(limit)?; let code = if zeros == limit { self.get(bits)? + 1 } else { (zeros << k) | self.get(k)? }; if code > range { return Err(corrupt(&format!( @@ -501,5 +532,48 @@ mod tests { src.extend_from_slice(&data); let _ = decode(&src, 780, 6, bits, xtrans, Mode::Full); } + + /// The word-at-a-time reader returns what reading bit by bit returns, and fails where it fails: long zero + /// runs, the escape limit and the end of the stream included. + #[test] + fn bit_reader_matches_reading_bit_by_bit( + data in prop::collection::vec(prop_oneof![3 => Just(0u8), 1 => any::()], 0..64), + reads in prop::collection::vec((any::(), 0u32..=24, 0u32..=63), 1..200), + ) { + let bit = |i: usize| data.get(i / 8).map(|b| (b >> (7 - i % 8)) & 1 == 1); + let mut at = 0usize; + let mut reader = Bits::new(&data); + for (prefix, n, limit) in reads { + let expected = if prefix { + let mut zeros = 0; + loop { + let Some(b) = bit(at) else { break Err("truncated") }; + at += 1; + if b { + break Ok(zeros); + } + zeros += 1; + if zeros > limit { + break Err("escape"); + } + } + } else if at + n as usize > data.len() * 8 { + Err("truncated") + } else { + let value = (0..n as usize).fold(0, |v, i| (v << 1) | u32::from(bit(at + i) == Some(true))); + at += n as usize; + Ok(value) + }; + let actual = if prefix { reader.zeros(limit) } else { reader.get(n) }; + match (expected, actual) { + (Ok(want), Ok(got)) => prop_assert_eq!(want, got), + (Err(want), Err(got)) => { + prop_assert!(got.to_string().contains(want), "{got}"); + break; + } + (want, got) => prop_assert!(false, "expected {want:?}, got {got:?}"), + } + } + } } } diff --git a/docs/raf-compression.md b/docs/raf-compression.md index 0141d893e..29056502e 100644 --- a/docs/raf-compression.md +++ b/docs/raf-compression.md @@ -17,7 +17,7 @@ Fujifilm now uses a guarded colour/tone estimate from each file’s own embedded - Signed prediction errors use adaptive Rice coding: a unary prefix followed by a context-dependent number of bits, with a bounded escape for large errors. Three pair phases have separate even/odd statistics. The 81 signed gradient contexts fold into 41 states. - Lossy quantizers change the error range and reconstruction step. Low-gradient sites retain finer precision (0, 1 or 2), with five folded contexts per precision and statistics that survive main-quantizer changes. The main context statistics reset when its quantizer changes. Their initial sum is `max(2, (range + 32) / 64)`; using 16 as the minimum breaks more heavily quantized files. -Stripes decode in parallel using the existing Rayon dependency. Offsets, sizes, dimensions and entropy reads are checked; truncated data, impossible codes and inconsistent headers return errors. Unknown container compression modes, disagreements between container and stream modes, and compressed X-Trans layouts other than the supported CFA return errors. Header-only probing validates the tables and stripe ranges without decompressing the image. +Stripes decode in parallel using the existing Rayon dependency. The entropy reader keeps up to 64 bits buffered and counts a unary prefix's zeros in one step instead of bit by bit; a property test checks it against reading bit by bit, errors included. Offsets, sizes, dimensions and entropy reads are checked; truncated data, impossible codes and inconsistent headers return errors. Unknown container compression modes, disagreements between container and stream modes, and compressed X-Trans layouts other than the supported CFA return errors. Header-only probing validates the tables and stripe ranges without decompressing the image. ## Verification @@ -36,7 +36,7 @@ The 17 CC0 files live in the gitignored corpus. Their published SHA-256 identiti Unconditional procedural tests cover Bayer/X-Trans, 12/14/16 bits, partial final stripes, quantizer changes, residual signs and escapes, malformed headers and truncated/random entropy streams. Regressions also check overflowing uncompressed row strides, unsupported modes and CFA layouts, and container/stream mode mismatches. -Local release decode measurements: about 0.16–0.24 seconds for 24–40 MP X-Trans files, 0.29 seconds for 50 MP Bayer, and 0.5–0.6 seconds for 100 MP Bayer. These measure sensor decoding, not the full develop/export pipeline. +Local release decode measurements: about 0.16–0.24 seconds for 24–40 MP X-Trans files, 0.29 seconds for 50 MP Bayer, and 0.5–0.6 seconds for 100 MP Bayer. These measure sensor decoding, not the full develop/export pipeline. The buffered prefix count took 13–17 % off the decode time on an Apple M2 (8 cores, minimum of 5 runs, old and new build alternated): X-T2 178 → 152 ms, X-S10 lossless 265 → 226 ms, X-E5 442 → 385 ms, X-H2 448 → 378 ms, GFX 50S 522 → 435 ms, GFX 100 762 → 644 ms, with sensor arrays unchanged bit for bit. App verification imported all four supplied compressed files, confirmed `previewOnly: null`, applied +2 EV exposure edits and inspected headless UI screenshots. CLI JPEG exports at a 1600-pixel long edge succeeded for all four files (0.58–0.76 seconds each, including process startup). From bd00b6a818ccbd48cfd3a972baa20aaf3cba2f74 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 29 Oct 2026 13:18:39 -0400 Subject: [PATCH 22/29] LR-VIEW-HISTOGRAM: the clipping buttons' tile and chevron come from the theme and stand 3:1 in every theme --- crates/ui-egui/src/panels/edit/histogram.rs | 49 ++++++++++++++------- docs/parity-checklist.md | 5 ++- docs/whats-new.md | 7 +++ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/crates/ui-egui/src/panels/edit/histogram.rs b/crates/ui-egui/src/panels/edit/histogram.rs index 085fbda5d..798ba2078 100644 --- a/crates/ui-egui/src/panels/edit/histogram.rs +++ b/crates/ui-egui/src/panels/edit/histogram.rs @@ -169,26 +169,12 @@ pub(super) fn histogram(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) let active = app.ui.show_clipping; // Keep the hit target comfortable, with a smaller box and clear space to the plot edges. let box_rect = rect.shrink2(vec2(2.0, 2.0)); - p.rect_filled( - box_rect, - 3.0, - if active { - t.pressed - } else if response.hovered() { - t.hover - } else { - Color32::from_gray(60) - }, - ); + let (tile, mark) = clip_button_colors(&t, active, response.hovered(), clipped == Some(true)); + p.rect_filled(box_rect, 3.0, tile); if active { p.rect_stroke(box_rect, 3.0, Stroke::new(1.0, t.text), egui::StrokeKind::Inside); } - paint( - p, - Rect::from_center_size(rect.center(), vec2(16.0, 16.0)), - Icon::ChevronDown, - if clipped == Some(true) || active { t.text } else { t.text_dim }, - ); + paint(p, Rect::from_center_size(rect.center(), vec2(16.0, 16.0)), Icon::ChevronDown, mark); let status = match clipped { Some(true) => "Clipping detected in this preview.", Some(false) => "No clipping detected in this preview.", @@ -317,10 +303,39 @@ pub(super) fn soft_proofing(app: &mut LightcraftApp, ui: &mut egui::Ui, id: Phot divider(ui); } +/// The tile and chevron colours of a clipping button, from the theme's own raised fills: the tile +/// sits one step above the plot's backdrop (`button`) and the chevron (bright when clipping shows, dim otherwise) stands 3:1 on it in every +/// state and theme. +fn clip_button_colors(t: &Tokens, active: bool, hovered: bool, clipped: bool) -> (Color32, Color32) { + let tile = if active || hovered { t.pressed } else { t.hover }; + (tile, if clipped || active { t.text } else { t.text_dim }) +} + #[cfg(test)] mod tests { use super::*; + /// The two clipping chevrons were a fixed dark-grey tile (`from_gray(60)`) with a dimmed mark: on + /// a light theme they were near-black tiles on a light panel, and the mark barely read. + #[test] + fn the_clipping_chevron_stands_3_to_1_against_its_tile_in_every_theme() { + use crate::theme::ThemeKind; + for kind in ThemeKind::ALL { + let t = Tokens::for_kind(kind); + for (active, hovered, clipped) in + [(false, false, false), (false, false, true), (false, true, false), (true, false, false), (true, true, true)] + { + let (tile, mark) = clip_button_colors(&t, active, hovered, clipped); + let c = crate::widgets::button::contrast(mark, tile); + assert!(c >= 3.0, "{kind:?} active={active} hovered={hovered} clipped={clipped}: chevron {c:.2}:1 on its tile"); + } + // the idle tile is the theme's own raised fill, not a fixed gray, and a quiet one next to the plot's backdrop + let (idle, _) = clip_button_colors(&t, false, false, false); + assert_ne!(idle, t.button, "{kind:?}: the tile vanishes into the histogram's backdrop"); + assert!(crate::widgets::button::contrast(idle, t.button) < 3.0, "{kind:?}: the tile is a heavy block, not a quiet raised fill"); + } + } + #[test] fn an_hdr_edit_keeps_its_hdr_histogram_through_8_bit_renders() { let ctx = egui::Context::default(); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 90cfc248f..a742678af 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-25 · **Change:** minor (LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-29 · **Change:** minor (LR-VIEW-HISTOGRAM: the shadow and highlight clipping buttons take their tile and chevron from the theme, so the chevron reads in every theme; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **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 @@ -139,7 +139,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-VIEW-SLIDESHOW | Slideshow | P2 | ✅ | `cmd:view.slideshow` | View ▸ Slideshow (⌥⌘↩): the photos in view full screen from the selected one, played with the library's playback settings (see LRC-SS-PLAYBACK: fades, random order, repeat, previews prepared ahead; `interval` / any setting overrides one run); Space pauses, ←/→ step (a cut), Esc ends; playback controls on pointer movement | | LR-VIEW-SECONDWINDOW | Second display window [Classic] | P2 | ✅ | `cmd:view.secondWindow`, `crates/ui-egui/src/panels/second.rs` | Window ▸ Second Window (⌘F11): the active photo fitted in its own native window with its own render (a floating panel where there are no native windows); loupe view only (no grid / compare / survey there) | | LR-VIEW-CLIPPING | Clipping indicators | P0 | ✅ | `cmd:view.clipping` | | -| LR-VIEW-HISTOGRAM | Histogram | P0 | ✅ | `cmd:view.histogram`, `crates/ui-egui/src/panels/edit.rs`, `crates/pipeline/src/lib.rs` (`hdr_histogram`) | HDR edits: SDR on the left half, the HDR range (0 to 5 stops above SDR white, a fixed scale) on the right with the headroom limit marked; colour bar (white, or Visualize HDR range's colours when that is on) and SDR / HDR labels; the plot's five tonal zones also drag to adjust their Basic control (LRC-DEV-HISTOGRAM-DRAG); it follows slider drags live (a small render beside the drafts, `Slot::HdrHistogram`) and never drops to the SDR layout mid-drag | +| LR-VIEW-HISTOGRAM | Histogram | P0 | ✅ | `cmd:view.histogram`, `crates/ui-egui/src/panels/edit.rs`, `crates/pipeline/src/lib.rs` (`hdr_histogram`) | HDR edits: SDR on the left half, the HDR range (0 to 5 stops above SDR white, a fixed scale) on the right with the headroom limit marked; colour bar (white, or Visualize HDR range's colours when that is on) and SDR / HDR labels; the plot's five tonal zones also drag to adjust their Basic control (LRC-DEV-HISTOGRAM-DRAG); it follows slider drags live (a small render beside the drafts, `Slot::HdrHistogram`) and never drops to the SDR layout mid-drag; the shadow and highlight clipping buttons at its top corners are the theme's raised fill with a chevron that stands 3:1 on it in every theme and state | | LR-VIEW-HDR-DISPLAY | HDR display output | P2 | ⬜ | |, `apps/lightcraft/src/hdr_display.rs` (`choose`, `preferred_hdr_format`) | SDR/HDR decision (platform, surface format, OS headroom, `LIGHTCRAFT_HDR_DISPLAY=0`) is unit-tested and logged at start; always SDR today because eframe configures an 8-bit surface; no OS headroom query yet; | ## D. Search & filter (FILT) @@ -799,6 +799,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-29 | patch | LR-VIEW-HISTOGRAM: the clipping buttons' tile and chevron come from the theme's tokens instead of a fixed dark gray, so the chevron stands 3:1 on its tile in all four themes; status counts unchanged | | 2026-10-25 | patch | LR-BEHAV-TITLEBAR: the top bar's title-bar gestures come from craft-window's `title_bar`: a double-click follows the system setting, and a press on a button, the search field or a panel floating over the bar never moves the window; status counts unchanged | | 2026-10-24 | minor | KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 (`keyword.nextSet`, `keyword.previousSet`) go to the next / previous keyword set; status counts unchanged (KEYC-METADATA stays 🟡) | | 2026-10-24 | minor | KEYC-METADATA: ⌘K / Ctrl+K (`panel.addKeywords`, Add Keywords) shows the Keywords panel, opens Keywording and puts the cursor in the Add keyword box; status counts unchanged (KEYC-METADATA stays 🟡) | diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..5145a6442 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-29 + +### The histogram's clipping buttons read in light themes +- The two buttons at the histogram's top corners (shadow and highlight clipping) are no longer near-black tiles with a + grey chevron. They take the theme's own raised fill, and the chevron stands out from it in Charcoal, Midnight, Silver + and Paper alike. + ## 2026-10-24 ### Relative-date filters and smart albums follow the clock From 29f9c190468b4df0edb2bbdbc8db4f99a4111d2d Mon Sep 17 00:00:00 2001 From: radialmonster Date: Thu, 29 Oct 2026 21:48:04 -0400 Subject: [PATCH 23/29] test(raw): spell control bytes in CR3 preview literals as escapes Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014ti8KemZbjE2ZgHgV1q1BM --- crates/raw/src/preview/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/raw/src/preview/tests.rs b/crates/raw/src/preview/tests.rs index f526e868b..56a726f78 100644 --- a/crates/raw/src/preview/tests.rs +++ b/crates/raw/src/preview/tests.rs @@ -204,9 +204,9 @@ fn picks_largest_dct_jpeg() { fn cr3_preview_box_huge_lengths_are_rejected() { let j = fake_jpeg(300); for (size, len) in [(u32::MAX, u32::MAX), (u32::MAX, u32::MAX - 23), (24 + j.len() as u32, u32::MAX - 3)] { - let mut cr3 = b"ftypcrx crx isom".to_vec(); + let mut cr3 = b"\x00\x00\x00\x18ftypcrx \x00\x00\x00\x01crx isom".to_vec(); cr3.extend_from_slice(&size.to_be_bytes()); - cr3.extend_from_slice(b"PRVWT8"); + cr3.extend_from_slice(b"PRVW\x00\x00\x00\x00\x00\x01\x06T\x048\x00\x01"); cr3.extend_from_slice(&len.to_be_bytes()); cr3.extend_from_slice(&j); assert_eq!(cr3_preview_boxes(&cr3), None, "size {size:#x}, len {len:#x}"); From 1c906ea3db250369860ba1357ed1dc50935bbf27 Mon Sep 17 00:00:00 2001 From: Bruno-Alumn <171815746+Bruno-Alumn@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:37:16 +0100 Subject: [PATCH 24/29] Never crash: strip byte counts near u64::MAX don't overflow their sum in ORF, SRW, ARW and PEF The TIFF reader accepts LONG8 (type 16) StripByteCounts in classic TIFF, so a file can declare strips of u64::MAX and 1 bytes. The ORF, SRW, ARW and PEF decoders added the strip lengths with Iterator::sum::(), which panics on overflow in debug builds. They now add them with a saturating fold, as raw/src/lib.rs already does, so such a file is refused or read as far as its data go. The tests rewrite a synthetic file of each decoder into two strips with LONG8 counts [u64::MAX, 1] (vendor::tests::huge_strip_counts); PEF is checked for the Huffman-coded and the uncompressed path. --- crates/raw/src/vendor.rs | 34 ++++++++++++++++++++++++++++++++++ crates/raw/src/vendor/arw.rs | 11 ++++++++++- crates/raw/src/vendor/orf.rs | 11 ++++++++++- crates/raw/src/vendor/pef.rs | 16 ++++++++++++++-- crates/raw/src/vendor/srw.rs | 19 ++++++++++++++++++- docs/whats-new.md | 7 +++++++ 6 files changed, 93 insertions(+), 5 deletions(-) diff --git a/crates/raw/src/vendor.rs b/crates/raw/src/vendor.rs index b0f65f49e..07e8e42fc 100644 --- a/crates/raw/src/vendor.rs +++ b/crates/raw/src/vendor.rs @@ -228,6 +228,40 @@ fn solve4(mut a: [[f64; 4]; 4], mut b: [f64; 4]) -> Option<[f64; 4]> { #[cfg(test)] mod tests { use super::*; + use lightcraft_tiff::tags as t; + + /// `file`, whose image IFD holds one strip, as two strips at the same offset whose StripByteCounts are LONG8 (which + /// classic TIFF readers accept) `[u64::MAX, 1]`: their sum is past `u64::MAX`. + pub(super) fn huge_strip_counts(mut file: Vec) -> Vec { + let tiff = Tiff::parse(&file).unwrap(); + let o = tiff.order; + let ifd = tiff.all_ifds().into_iter().find(|i| i.contains(t::STRIP_OFFSETS)).unwrap(); + let (base, rows, offset) = (ifd.offset as usize, ifd.u32(t::IMAGE_LENGTH).unwrap(), ifd.u32(t::STRIP_OFFSETS).unwrap()); + assert!(rows >= 2); + let at = file.len() as u32; + // appended: the two strip offsets (LONG), then the two byte counts (LONG8) + let mut tail = Vec::new(); + o.put_u32(&mut tail, offset); + o.put_u32(&mut tail, offset); + o.put_u64(&mut tail, u64::MAX); + o.put_u64(&mut tail, 1); + for i in 0..usize::from(o.read_u16(&file, base).unwrap()) { + let e = base + 2 + 12 * i; + let (ty, count, value) = match o.read_u16(&file, e).unwrap() { + t::STRIP_OFFSETS => (4u16, 2u32, at), + t::ROWS_PER_STRIP => (4, 1, rows.div_ceil(2)), + t::STRIP_BYTE_COUNTS => (16, 2, at + 8), + _ => continue, + }; + let mut entry = Vec::new(); + o.put_u16(&mut entry, ty); + o.put_u32(&mut entry, count); + o.put_u32(&mut entry, value); + file[e + 2..e + 12].copy_from_slice(&entry); + } + file.extend_from_slice(&tail); + file + } #[test] fn black_columns_by_parity() { diff --git a/crates/raw/src/vendor/arw.rs b/crates/raw/src/vendor/arw.rs index 3938fa50d..76f13fa2d 100644 --- a/crates/raw/src/vendor/arw.rs +++ b/crates/raw/src/vendor/arw.rs @@ -628,7 +628,7 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { let bits = info.bits() as u32; let linear_rgb = info.compression == 7 && info.photometric == photometric::YCBCR; let chunks = info.chunks(bytes.len() as u64); - let strip_len: u64 = chunks.iter().map(|c| c.len).sum(); + let strip_len = chunks.iter().fold(0u64, |a, c| a.saturating_add(c.len)); // The row-per-w-bytes (ARW2) layout stores exactly one byte per pixel in a single strip; all 108 such files in the test // archive measure exactly width x height. A200/A230/A350 files carry a strip 4-23 % larger than that (and A290/A390 one // smaller), which is a different packing, so they are refused rather than read with the wrong layout. @@ -900,6 +900,15 @@ mod tests { TiffWriter::default().write(&[raw]).unwrap() } + /// Strip byte counts that add up past `u64::MAX` (LONG8 `[u64::MAX, 1]`) are an error or a picture, not an overflow. + #[test] + fn strip_byte_counts_past_u64_max_do_not_overflow() { + let file = crate::vendor::tests::huge_strip_counts(arw_with_strip(32 * 4)); + for mode in [Mode::Header, Mode::Full] { + let _ = super::decode(&file, mode); + } + } + /// The ILCE-7RM6's distortion table reaches `OpcodeList3`, in the headers-only probe and in the full decode, as /// one plane, also with a CA table (lateral CA is off by default); another body's tables don't (see /// [`super::super::arw_lens_rm6`]). diff --git a/crates/raw/src/vendor/orf.rs b/crates/raw/src/vendor/orf.rs index 92caeec26..172f83013 100644 --- a/crates/raw/src/vendor/orf.rs +++ b/crates/raw/src/vendor/orf.rs @@ -206,7 +206,7 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { let (w, h) = (info.width as usize, info.height as usize); let n = w.checked_mul(h).filter(|n| *n > 0 && *n <= crate::MAX_SAMPLES).ok_or(RawError::Limit("image too large"))?; let chunks = info.chunks(bytes.len() as u64); - let total: u64 = chunks.iter().map(|c| c.len).sum(); + let total = chunks.iter().fold(0u64, |a, c| a.saturating_add(c.len)); let stated = cfa_from_exif(&tiff); let mut two_field = false; let (mut data, bits) = if info.compression != 1 { @@ -394,6 +394,15 @@ mod tests { b } + /// Strip byte counts that add up past `u64::MAX` (LONG8 `[u64::MAX, 1]`) are an error or a picture, not an overflow. + #[test] + fn strip_byte_counts_past_u64_max_do_not_overflow() { + let file = crate::vendor::tests::huge_strip_counts(orf(4, 2, 16, vec![0; 16])); + for mode in [Mode::Header, Mode::Full] { + let _ = super::decode(&file, mode); + } + } + /// An old-style little-endian Olympus maker note (`OLYMP\0`, version 1, then the IFD) of SHORT (type 3) or /// UNDEFINED (type 7) entries. Values longer than four bytes follow the IFD; their offsets count from the TIFF /// header, so `at` is where the note sits in the file. diff --git a/crates/raw/src/vendor/pef.rs b/crates/raw/src/vendor/pef.rs index 9b7d92bc7..45efeec68 100644 --- a/crates/raw/src/vendor/pef.rs +++ b/crates/raw/src/vendor/pef.rs @@ -267,14 +267,14 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { let chunks = info.chunks(bytes.len() as u64); let first = chunks.first().ok_or_else(|| RawError::Corrupt("PEF without image data".into()))?; let mut c = *first; - c.len = chunks.iter().map(|c| c.len).sum::().max(first.len); + c.len = chunks.iter().fold(0u64, |a, c| a.saturating_add(c.len)).max(first.len); let src = chunk_bytes(bytes, &c) .or_else(|| bytes.get(first.offset as usize..)) .ok_or_else(|| RawError::Corrupt("PEF data outside file".into()))?; if read == Mode::Full { RawData::U16(decode_huffman(src, &huff, w, hgt, bits)?) } else { RawData::U16(Vec::new()) } } 1 => { - let strip: u64 = info.chunks(bytes.len() as u64).iter().map(|c| c.len).sum(); + let strip = info.chunks(bytes.len() as u64).iter().fold(0u64, |a, c| a.saturating_add(c.len)); let packing = if strip >= (w * hgt * 2) as u64 { Packing::Word16 } else { Packing::Msb }; read_image_in(read, bytes, &info, tiff.order, packing)? } @@ -707,4 +707,16 @@ mod tests { let file = pef_with_black_point(w, h, 64, |x, y| 64 + ((x * 7 + y * 13) % 5) as u16); assert_eq!(decode(&file, Mode::Full).unwrap().active_area, Rect::new(0, 0, w, h)); } + + /// Strip byte counts that add up past `u64::MAX` (LONG8 `[u64::MAX, 1]`) are an error or a picture, not an overflow, + /// for the Huffman-coded and the uncompressed samples. + #[test] + fn strip_byte_counts_past_u64_max_do_not_overflow() { + for compression in [65535u16, 1] { + let file = crate::vendor::tests::huge_strip_counts(pef_with_pattern(compression, [0, 1, 1, 2])); + for mode in [Mode::Header, Mode::Full] { + let _ = decode(&file, mode); + } + } + } } diff --git a/crates/raw/src/vendor/srw.rs b/crates/raw/src/vendor/srw.rs index b22faeec0..0931c208d 100644 --- a/crates/raw/src/vendor/srw.rs +++ b/crates/raw/src/vendor/srw.rs @@ -97,7 +97,7 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { let (w, h) = (info.width as usize, info.height as usize); let n = w.checked_mul(h).filter(|n| *n > 0 && *n <= crate::MAX_SAMPLES).ok_or(RawError::Limit("image too large"))?; let chunks = info.chunks(bytes.len() as u64); - let strip: u64 = chunks.iter().map(|c| c.len).sum(); + let strip = chunks.iter().fold(0u64, |a, c| a.saturating_add(c.len)); let Some(coding) = coding(info.compression, w, h, strip).filter(|_| chunks.len() == 1) else { return Err(RawError::Unsupported(format!( "Samsung compressed raw (compression {}, {:.2} stored bits per sample)", @@ -355,6 +355,23 @@ mod tests { TiffWriter::new(lightcraft_tiff::ByteOrder::Big, false).write(&[ifd0]).unwrap() } + /// Strip byte counts that add up past `u64::MAX` (LONG8 `[u64::MAX, 1]`) are an error or a picture, not an overflow. + #[test] + fn strip_byte_counts_past_u64_max_do_not_overflow() { + let file = crate::vendor::tests::huge_strip_counts(srw(Spec { + w: 4, + h: 2, + compression: 32769, + strip: vec![0; 16], + pattern: None, + note: None, + picture: None, + })); + for mode in [Mode::Header, Mode::Full] { + let _ = super::decode(&file, mode); + } + } + /// The camera-stated description every test file carries: JPEG framing, white balance, levels. fn full_note(area: [u32; 4], white: i64, black: i64) -> Vec { let mut levels = vec![0i64; 4]; diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..b9076b3c1 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-30 + +### Reliability +- A damaged or crafted Olympus ORF, Samsung SRW, Sony ARW or Pentax PEF whose strip sizes add up past the largest + 64-bit number no longer crashes LightCraft (debug builds) when it is read: the file is refused, or read as far as + its data go. + ## 2026-10-24 ### Relative-date filters and smart albums follow the clock From be6e462db430c67f10053535fa07fb51724e8bc4 Mon Sep 17 00:00:00 2001 From: Bruno-Alumn <171815746+Bruno-Alumn@users.noreply.github.com> Date: Fri, 30 Oct 2026 16:09:18 +0100 Subject: [PATCH 25/29] Cap red eye corrections at 64 per photo (#2556) Every output pixel goes through every eye, and nothing limited the list: a sidecar, preset or agent with 100,000 eyes made one preview take hours. The render resolves the first MAX_RED_EYES and redeye.add refuses more, as Point Color does with MAX_POINT_COLORS. --- crates/develop/src/settings.rs | 5 +++++ crates/engine/src/cmd/masks.rs | 5 ++++- crates/engine/src/tests/color.rs | 6 ++++++ crates/pipeline/src/redeye.rs | 11 +++++++++++ docs/whats-new.md | 6 ++++++ 5 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/develop/src/settings.rs b/crates/develop/src/settings.rs index a40f094c2..0064a4f81 100644 --- a/crates/develop/src/settings.rs +++ b/crates/develop/src/settings.rs @@ -134,6 +134,7 @@ pub struct DevelopSettings { pub orientation: Orientation, pub masks: Vec, pub spots: Vec, + /// Red eye corrections; the first [`MAX_RED_EYES`] render. pub red_eye: Vec, pub lens_blur: LensBlur, pub enhance: Enhance, @@ -1069,6 +1070,10 @@ impl Default for Spot { } } +/// Most red eye / pet eye corrections a photo renders. Every output pixel goes through every eye, so render cost is +/// pixels x eyes; a sidecar, preset or agent with thousands could stall a render for hours (issue #2556). +pub const MAX_RED_EYES: usize = 64; + /// A red eye / pet eye correction: the user's ellipse (centre normalized, radii as fractions of /// the long edge); the pupil inside it is found automatically. #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] diff --git a/crates/engine/src/cmd/masks.rs b/crates/engine/src/cmd/masks.rs index 2fd44d566..6ec6d39f5 100644 --- a/crates/engine/src/cmd/masks.rs +++ b/crates/engine/src/cmd/masks.rs @@ -1,6 +1,6 @@ //! Masking and Remove (spot) commands on the active photo. -use lightcraft_develop::{BrushStroke, LocalAdjustments, Mask, MaskComponent, MaskOp, MaskShape, RedEye, Spot, SpotMode}; +use lightcraft_develop::{BrushStroke, LocalAdjustments, MAX_RED_EYES, Mask, MaskComponent, MaskOp, MaskShape, RedEye, Spot, SpotMode}; use lightcraft_geom::Point; use serde_json::{Value, json}; @@ -1025,6 +1025,9 @@ pub fn specs() -> Vec { catchlight: None, }; let id = s.active().ok_or_else(|| bad(c, "no active photo"))?; + if s.develop_of(id).is_some_and(|d| d.red_eye.len() >= MAX_RED_EYES) { + return Err(bad(c, format!("at most {MAX_RED_EYES} red eye corrections"))); + } let mut dd = (*s.develop_of(id).unwrap_or_default()).clone(); dd.red_eye.push(eye); let n = dd.red_eye.len(); diff --git a/crates/engine/src/tests/color.rs b/crates/engine/src/tests/color.rs index e526d4c47..af59f02be 100644 --- a/crates/engine/src/tests/color.rs +++ b/crates/engine/src/tests/color.rs @@ -75,6 +75,12 @@ fn red_eye_commands_and_controls() { assert_eq!(active_dev(&s).red_eye.len(), 1); assert!(s.execute("redeye.delete", &json!({"index": 5})).is_err()); assert!(s.execute("redeye.add", &json!({"rx": 0.03})).is_err()); + // at most MAX_RED_EYES: every pixel goes through every eye (issue #2556) + while active_dev(&s).red_eye.len() < lightcraft_develop::MAX_RED_EYES { + s.execute("redeye.add", &json!({"center": [0.5, 0.5], "rx": 0.01, "ry": 0.01})).unwrap(); + } + assert!(s.execute("redeye.add", &json!({"center": [0.5, 0.5], "rx": 0.01, "ry": 0.01})).is_err()); + assert_eq!(active_dev(&s).red_eye.len(), lightcraft_develop::MAX_RED_EYES); let d = active_dev(&s); assert_eq!(lightcraft_develop::DevelopSettings::from_json(&d.to_json()).unwrap(), d); } diff --git a/crates/pipeline/src/redeye.rs b/crates/pipeline/src/redeye.rs index 3bad1d634..bb2944388 100644 --- a/crates/pipeline/src/redeye.rs +++ b/crates/pipeline/src/redeye.rs @@ -142,6 +142,7 @@ pub fn resolve(src: &Rgb32f, eyes: &[RedEye], orientation: Orientation, frame: & } let to_out = frame.norm_to_out(w, h); eyes.iter() + .take(lightcraft_develop::MAX_RED_EYES) .map(|e| { let p = detect(src, orientation, frame.ow, frame.oh, e); let r = (p.radius * (0.5 + e.pupil_size.clamp(0.0, 100.0) / 100.0)).min(e.rx.max(e.ry) * 1.1); @@ -246,6 +247,16 @@ mod tests { assert!(!detect(&plain, Orientation::Normal, 200.0, 100.0, &eye).found); } + #[test] + fn at_most_max_red_eyes_render() { + // a sidecar, preset or agent can hold any number; each one costs a pass over every pixel (issue #2556) + let src = eye_image(40, 20, 18.0, 10.0, 2.0, [0.5, 0.04, 0.04]); + let s = lightcraft_develop::DevelopSettings { red_eye: vec![RedEye::default(); 10_000], ..Default::default() }; + let frame = Frame::new(40, 20, &s, true); + let eyes = resolve(&src, &s.red_eye, Orientation::Normal, &frame, 40, 20, 40.0); + assert_eq!(eyes.len(), lightcraft_develop::MAX_RED_EYES); + } + #[test] fn renders_fix_the_red_pupil_at_any_size() { use crate::{RenderRequest, SourceInfo, render}; diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..d9d3cbba5 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,11 @@ # What's new in LightCraft +## 2026-10-30 + +### At most 64 red eye corrections per photo +- Every pixel of a render goes through every red eye correction, so a sidecar, preset or agent with thousands of + them could stall a preview or export for hours. A photo now renders its first 64, and Red Eye refuses a 65th. + ## 2026-10-24 ### Relative-date filters and smart albums follow the clock From 7597810299f2da19d9e242998b2c28b64d2e62d2 Mon Sep 17 00:00:00 2001 From: Bruno-Alumn <171815746+Bruno-Alumn@users.noreply.github.com> Date: Fri, 30 Oct 2026 16:25:20 +0100 Subject: [PATCH 26/29] SRW: a huge encryption key doesn't overflow keyed() The maker note's key (0xa020) can be LONG8, so a value past i64::MAX made `-1 * key` overflow: a panic in debug and test builds. Use a wrapping multiply; only the low 32 bits are kept, so real files decode the same. --- crates/raw/src/vendor/srw.rs | 42 ++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/crates/raw/src/vendor/srw.rs b/crates/raw/src/vendor/srw.rs index b22faeec0..41727715d 100644 --- a/crates/raw/src/vendor/srw.rs +++ b/crates/raw/src/vendor/srw.rs @@ -62,7 +62,8 @@ fn keyed(mn: &Ifd, tag: u16, sign: i64) -> Option> { let key = mn.u64s(ENCRYPTION_KEY).filter(|k| k.len() == 11)?; let vals = mn.f64s(tag)?; // 32-bit entries: the stored value wraps modulo 2^32 (a negative number stored as a LONG) - Some(vals.iter().enumerate().map(|(i, &v)| i64::from((v as i64).wrapping_add(sign * key[i % 11] as i64) as i32)).collect()) + // (a key value past i64::MAX must not overflow the multiply: only the low 32 bits are kept) + Some(vals.iter().enumerate().map(|(i, &v)| i64::from((v as i64).wrapping_add((key[i % 11] as i64).wrapping_mul(sign)) as i32)).collect()) } /// The raw image's IFD: the one with a private compression value. @@ -283,19 +284,25 @@ mod tests { /// A maker note in the Samsung layout: a big-endian IFD of LONG vectors, offsets relative to the note's start. fn note(entries: &[(u16, Vec)]) -> Vec { + let typed: Vec<_> = entries.iter().map(|(tag, v)| (*tag, 4, v.len() as u32, v.iter().flat_map(|x| x.to_be_bytes()).collect())).collect(); + note_typed(&typed) + } + + /// [`note`] with any entry types: `(tag, type, count, big-endian value bytes)`. + fn note_typed(entries: &[(u16, u16, u32, Vec)]) -> Vec { let mut head = Vec::new(); let mut tail = Vec::new(); head.extend_from_slice(&(entries.len() as u16).to_be_bytes()); let values_at = 2 + 12 * entries.len() + 4; - for (tag, v) in entries { + for (tag, kind, count, v) in entries { head.extend_from_slice(&tag.to_be_bytes()); - head.extend_from_slice(&4u16.to_be_bytes()); - head.extend_from_slice(&(v.len() as u32).to_be_bytes()); - if v.len() == 1 { - head.extend_from_slice(&v[0].to_be_bytes()); + head.extend_from_slice(&kind.to_be_bytes()); + head.extend_from_slice(&count.to_be_bytes()); + if v.len() <= 4 { + head.extend(v.iter().copied().chain(std::iter::repeat(0)).take(4)); } else { head.extend_from_slice(&((values_at + tail.len()) as u32).to_be_bytes()); - tail.extend(v.iter().flat_map(|x| x.to_be_bytes())); + tail.extend_from_slice(v); } } head.extend_from_slice(&[0; 4]); @@ -511,6 +518,27 @@ mod tests { } /// A framing rectangle outside the array, or reversed, is ignored; truncated files fail without panicking. + /// An encryption key value past i64::MAX (LONG8) overflowed `-1 * key` in debug builds. + #[test] + fn a_huge_encryption_key_does_not_panic() { + let key: Vec = std::iter::repeat_n((1u64 << 63).to_be_bytes(), 11).flatten().collect(); + let note = note_typed(&[(ENCRYPTION_KEY, 16, 11, key), (WB_UNCORRECTED, 4, 4, [0u8; 16].to_vec()), (WB_BLACK, 4, 4, [0u8; 16].to_vec())]); + let bytes = srw(Spec { + w: 64, + h: 32, + compression: 32770, + strip: pack12(&ramp(64, 32, 4096)), + pattern: Some([2, 1, 1, 0]), + note: Some(note), + picture: None, + }); + let mn = maker_note(&bytes, &Tiff::parse(&bytes).unwrap()).unwrap(); + assert_eq!(mn.u64s(ENCRYPTION_KEY).map(|k| k[0]), Some(1 << 63), "the key reaches keyed()"); + assert_eq!(keyed(&mn, WB_BLACK, -1), Some(vec![0; 4]), "(2^63 mod 2^32 is 0)"); + assert!(decode(&bytes).is_ok()); + assert!(probe_info(&bytes).is_ok()); + } + #[test] fn hostile_framing_and_truncation_do_not_panic() { let v = ramp(64, 32, 4096); From abdc26c854fecf3e14869923bb81902930f420ad Mon Sep 17 00:00:00 2001 From: x-labb <154824493+x-labb@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:00:10 +0100 Subject: [PATCH 27/29] Issue #2819: the histogram stays SDR when HDR is switched off The small HDR render of the last HDR drag is kept in Renderer. With HDR switched off, dragging a slider still read it as the live histogram, so the panel flipped back to the HDR layout. Read it only while an HDR edit is being dragged. The LR-VIEW-HISTOGRAM row says so. --- crates/ui-egui/src/panels/edit/histogram.rs | 24 ++++++++++++++++++++- docs/parity-checklist.md | 3 ++- docs/whats-new.md | 7 ++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/edit/histogram.rs b/crates/ui-egui/src/panels/edit/histogram.rs index 085fbda5d..af7418381 100644 --- a/crates/ui-egui/src/panels/edit/histogram.rs +++ b/crates/ui-egui/src/panels/edit/histogram.rs @@ -33,6 +33,13 @@ fn steady_hdr_histogram(ctx: &egui::Context, id: PhotoId, hdr_edit: bool, hist: } } +/// The histogram of the last drag's small HDR render, while an HDR edit of `id` is being dragged. It stays in +/// `Renderer` after the drag, so an edit with HDR switched off must not read it: its drags would show the old HDR +/// histogram (`dragging`: a drag of an HDR edit is on). +fn live_hdr_histogram(live: Option<&(PhotoId, u64, Histogram)>, id: PhotoId, dragging: bool) -> Option { + live.filter(|l| dragging && l.0 == id).map(|l| l.2.clone()) +} + pub(super) fn histogram(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { let t = Tokens::get(ui.ctx()); let w = ui.available_width(); @@ -48,7 +55,7 @@ pub(super) fn histogram(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) app.renderer.request(Slot::HdrHistogram, job, 95); } } - let live = app.renderer.live_hdr_histogram.as_ref().filter(|l| interacting && l.0 == id).map(|l| l.2.clone()); + let live = live_hdr_histogram(app.renderer.live_hdr_histogram.as_ref(), id, interacting && hdr_edit); // else the loupe render of this photo, else its thumbnail (grid view before the loupe has // rendered it) let hist = live.or_else(|| preview_histogram(app, id)); @@ -342,6 +349,21 @@ mod tests { assert_eq!(steady_hdr_histogram(&ctx, PhotoId(8), true, Some(sdr.clone())), Some(sdr)); } + /// Issue #2819: the last HDR drag's histogram is kept in `Renderer`; with HDR switched off, a drag must not read it. + #[test] + fn a_drag_with_hdr_off_does_not_read_the_last_hdr_drags_histogram() { + let id = PhotoId(7); + let kept = (id, 3, Histogram::of_hdr(&[[2.0, 2.0, 2.0]], 5.0)); + assert!(kept.2.hdr.is_some()); + // a drag of an HDR edit: the small HDR render's histogram + assert_eq!(live_hdr_histogram(Some(&kept), id, true), Some(kept.2.clone())); + // HDR switched off, a drag of the SDR edit: the panel's own (SDR) histogram + assert_eq!(live_hdr_histogram(Some(&kept), id, false), None); + // another photo, or nothing rendered yet + assert_eq!(live_hdr_histogram(Some(&kept), PhotoId(8), true), None); + assert_eq!(live_hdr_histogram(None, id, true), None); + } + /// Issue #1751: Windows and Linux read Ctrl where macOS reads Cmd, and a rebound shortcut shows. #[test] fn reset_tooltip_names_the_shortcut_of_this_platform() { diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 08dab300b..b23c37d17 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -139,7 +139,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-VIEW-SLIDESHOW | Slideshow | P2 | ✅ | `cmd:view.slideshow` | View ▸ Slideshow (⌥⌘↩): the photos in view full screen from the selected one, played with the library's playback settings (see LRC-SS-PLAYBACK: fades, random order, repeat, previews prepared ahead; `interval` / any setting overrides one run); Space pauses, ←/→ step (a cut), Esc ends; playback controls on pointer movement | | LR-VIEW-SECONDWINDOW | Second display window [Classic] | P2 | ✅ | `cmd:view.secondWindow`, `crates/ui-egui/src/panels/second.rs` | Window ▸ Second Window (⌘F11): the active photo fitted in its own native window with its own render (a floating panel where there are no native windows); loupe view only (no grid / compare / survey there) | | LR-VIEW-CLIPPING | Clipping indicators | P0 | ✅ | `cmd:view.clipping` | | -| LR-VIEW-HISTOGRAM | Histogram | P0 | ✅ | `cmd:view.histogram`, `crates/ui-egui/src/panels/edit.rs`, `crates/pipeline/src/lib.rs` (`hdr_histogram`) | HDR edits: SDR on the left half, the HDR range (0 to 5 stops above SDR white, a fixed scale) on the right with the headroom limit marked; colour bar (white, or Visualize HDR range's colours when that is on) and SDR / HDR labels; the plot's five tonal zones also drag to adjust their Basic control (LRC-DEV-HISTOGRAM-DRAG); it follows slider drags live (a small render beside the drafts, `Slot::HdrHistogram`) and never drops to the SDR layout mid-drag | +| LR-VIEW-HISTOGRAM | Histogram | P0 | ✅ | `cmd:view.histogram`, `crates/ui-egui/src/panels/edit.rs`, `crates/pipeline/src/lib.rs` (`hdr_histogram`) | HDR edits: SDR on the left half, the HDR range (0 to 5 stops above SDR white, a fixed scale) on the right with the headroom limit marked; colour bar (white, or Visualize HDR range's colours when that is on) and SDR / HDR labels; the plot's five tonal zones also drag to adjust their Basic control (LRC-DEV-HISTOGRAM-DRAG); it follows slider drags of an HDR edit live (a small render beside the drafts, `Slot::HdrHistogram`) and never drops to the SDR layout mid-drag; an edit with HDR switched off keeps its SDR histogram while it is dragged (#2819) | | LR-VIEW-HDR-DISPLAY | HDR display output | P2 | ⬜ | |, `apps/lightcraft/src/hdr_display.rs` (`choose`, `preferred_hdr_format`) | SDR/HDR decision (platform, surface format, OS headroom, `LIGHTCRAFT_HDR_DISPLAY=0`) is unit-tested and logged at start; always SDR today because eframe configures an 8-bit surface; no OS headroom query yet; | ## D. Search & filter (FILT) @@ -799,6 +799,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-27 | patch | LR-VIEW-HISTOGRAM: an edit with HDR switched off no longer shows the last HDR drag's histogram while it is dragged (#2819); status unchanged | | 2026-10-25 | patch | LR-BEHAV-TITLEBAR: the top bar's title-bar gestures come from craft-window's `title_bar`: a double-click follows the system setting, and a press on a button, the search field or a panel floating over the bar never moves the window; status counts unchanged | | 2026-10-24 | minor | KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 (`keyword.nextSet`, `keyword.previousSet`) go to the next / previous keyword set; status counts unchanged (KEYC-METADATA stays 🟡) | | 2026-10-24 | minor | KEYC-METADATA: ⌘K / Ctrl+K (`panel.addKeywords`, Add Keywords) shows the Keywords panel, opens Keywording and puts the cursor in the Add keyword box; status counts unchanged (KEYC-METADATA stays 🟡) | diff --git a/docs/whats-new.md b/docs/whats-new.md index a0213da82..ada328897 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-30 + +### The histogram stays SDR when HDR is switched off +- After an HDR edit was dragged and HDR was then switched off, dragging a slider showed the old HDR histogram (with its + SDR / HDR colour bar) again until the drag ended. The histogram now follows the edit: an SDR edit shows the SDR + histogram while you drag, too. + ## 2026-10-27 ### The Edit menu groups Select Only Active Photo and Paste Settings from Previous with their kin From a261544b7707fb4344c8fab83943983c3354a453 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 31 Oct 2026 02:10:32 +0000 Subject: [PATCH 28/29] Integration: #3400 move the framing test's doc line back above its test --- crates/raw/src/vendor/srw.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/raw/src/vendor/srw.rs b/crates/raw/src/vendor/srw.rs index 772c39fb3..dbdb21b52 100644 --- a/crates/raw/src/vendor/srw.rs +++ b/crates/raw/src/vendor/srw.rs @@ -537,7 +537,6 @@ mod tests { assert!(matches!(decode(&bytes), Err(RawError::Unsupported(w)) if w.contains("CFAPattern"))); } - /// A framing rectangle outside the array, or reversed, is ignored; truncated files fail without panicking. /// An encryption key value past i64::MAX (LONG8) overflowed `-1 * key` in debug builds. #[test] fn a_huge_encryption_key_does_not_panic() { @@ -559,6 +558,7 @@ mod tests { assert!(probe_info(&bytes).is_ok()); } + /// A framing rectangle outside the array, or reversed, is ignored; truncated files fail without panicking. #[test] fn hostile_framing_and_truncation_do_not_panic() { let v = ramp(64, 32, 4096); From 0715fbd0b9b0c5e21bbd92e129477307f4efac9e Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 31 Oct 2026 02:10:40 +0000 Subject: [PATCH 29/29] Integration: RENDER_CACHE_VERSION 72 for the demosaic, grain and clipping-preview changes --- crates/engine/src/media/source.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/media/source.rs b/crates/engine/src/media/source.rs index 8490d2ee2..bf2e81804 100644 --- a/crates/engine/src/media/source.rs +++ b/crates/engine/src/media/source.rs @@ -1,7 +1,7 @@ use super::{Arc, Deserialize, DevelopSettings, MediaKind, Photo, Rgb32f, Rgba8, Serialize, SourceInfo}; /// Bump when the pipeline's output changes, to invalidate cached thumbnails. -pub const RENDER_CACHE_VERSION: u64 = 71; +pub const RENDER_CACHE_VERSION: u64 = 72; /// 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];