From 8e61ad6b13d94c743ecd35649c2150e596c97bee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Sun, 11 Oct 2026 15:56:17 +0800 Subject: [PATCH 01/14] Add Ctrl+O / Cmd+O to show every mask overlay at once Previously the masking overlay showed only the selected mask's region (toggled by O). This adds a new Overlay::AllMasks variant that paints the evaluated alpha of every visible mask that has components in one tint, wired to a new "Show All Mask Overlays" command (Ctrl+O on Windows/Linux, Cmd+O on macOS) reachable from the View menu, the masking panel header (stack icon), and the keyboard shortcut. Both the CPU and GPU render paths evaluate all visible masks and hand their alpha planes to visualize::apply, which layers each mask's region so multiple masks are visible at once. The all-masks overlay takes precedence over the single-mask overlay so the shortcut is not shadowed by the always-on per-mask view. --- crates/gpu/src/render.rs | 12 +++++- crates/pipeline/src/lib.rs | 21 ++++++++++- crates/pipeline/src/visualize.rs | 56 +++++++++++++++++++++++++++- crates/ui-egui/src/menubar.rs | 2 + crates/ui-egui/src/menus.rs | 6 +++ crates/ui-egui/src/panels/detail.rs | 12 ++++++ crates/ui-egui/src/panels/masking.rs | 4 ++ crates/ui-egui/src/state.rs | 4 ++ crates/ui-egui/src/tests_masking.rs | 9 +++++ 9 files changed, 122 insertions(+), 4 deletions(-) diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 45a5a5f6d..1ace48078 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -688,6 +688,16 @@ pub fn render( _ => Err(m), } }); + // every visible mask's alpha, for `Overlay::AllMasks` (one tint over all masks at once) + let all_overlay_alphas: Option> = if matches!(req.overlay, lightcraft_pipeline::Overlay::AllMasks { .. }) { + let list: Vec<_> = s.masks.iter().filter(|m| m.visible && !m.components.is_empty()).collect(); + masks.as_ref().map(|all| { + list.iter().enumerate().map(|(mi, _)| { + let b = cx.slice(all, mi * n, n); + cx.read_plane(&b, w, h) + }).collect() + }) + } else { None }; let data: Vec<[u8; 4]> = cx.read(&out, n); let image = Rgba8 { width: w, height: h, data }; lap("finish + readback", &mut t, &mut cx); @@ -723,7 +733,7 @@ pub fn render( lightcraft_pipeline::masks::evaluate_one(m, &plan.frame, w, h, &img, &l, s.light.exposure as f32, plan.mattes.as_deref()) } }); - lightcraft_pipeline::visualize::apply(&mut image, req.overlay, &plan, overlay_mask.as_ref()); + lightcraft_pipeline::visualize::apply(&mut image, req.overlay, &plan, overlay_mask.as_ref(), all_overlay_alphas.as_deref()); // spots grew the window the render worked on: cut it back to the request if let Some(k) = plan.keep { image = image.crop(k.x, k.y, k.w, k.h); diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index f7c33ee9e..82d05800b 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -551,7 +551,8 @@ fn render_impl(src: Src<'_>, info: &SourceInfo, s: &DevelopSettings, req: &Rende lap("histogram", &mut t); let mut image = image; let mask = overlay_alpha(req.overlay, &plan, &prep); - visualize::apply(&mut image, req.overlay, &plan, mask.as_ref()); + let masks = overlay_alphas_all(req.overlay, &plan, &prep); + visualize::apply(&mut image, req.overlay, &plan, mask.as_ref(), masks.as_deref()); let mut image = if plan.keep.is_some() { cut(&image) } else { image }; if req.overlay == Overlay::HdrRange && edit.hdr.enabled { // the HDR rendition of the same request says how far above SDR white each pixel goes @@ -603,6 +604,24 @@ fn overlay_alpha(o: Overlay, plan: &Plan<'_>, prep: &Prepared) -> Option Some(masks::evaluate_one(m, &plan.frame, plan.w, plan.h, &prep.img, &prep.log_l, ev, plan.mattes.as_deref())) } +/// The alpha planes an `Overlay::AllMasks` shows: one per visible mask that has components, +/// reusing the render's own evaluation where available (a fresh evaluation otherwise). +fn overlay_alphas_all(o: Overlay, plan: &Plan<'_>, prep: &Prepared) -> Option> { + if !matches!(o, Overlay::AllMasks { .. }) { + return None; + } + let ev = plan.settings.light.exposure as f32; + let mut out = Vec::new(); + for m in plan.settings.masks.iter().filter(|m| m.visible && !m.components.is_empty()) { + if let Some(e) = prep.masks.iter().find(|e| e.id == m.id) { + out.push(e.alpha.clone()); + } else { + out.push(masks::evaluate_one(m, &plan.frame, plan.w, plan.h, &prep.img, &prep.log_l, ev, plan.mattes.as_deref())); + } + } + Some(out) +} + /// Convenience: render a before/after pair side by side is up to the UI; this renders "before" /// (default look, keeping the crop so framing matches, under the photo's own rendering process). pub fn before_settings(s: &DevelopSettings) -> DevelopSettings { diff --git a/crates/pipeline/src/visualize.rs b/crates/pipeline/src/visualize.rs index fc3d8390f..dd87ce332 100644 --- a/crates/pipeline/src/visualize.rs +++ b/crates/pipeline/src/visualize.rs @@ -33,6 +33,9 @@ pub enum Overlay { /// The evaluated alpha of mask `id` (a [`lightcraft_develop::Mask`] id), drawn as `view` in /// `color` at `opacity` (0..100; the colour views only). Mask { id: u16, view: MaskView, color: [u8; 3], opacity: u8 }, + /// The evaluated alpha of every visible mask that has components, drawn as one `view` in + /// `color` at `opacity` (0..100; the colour views only) — every mask's region at once. + AllMasks { view: MaskView, color: [u8; 3], opacity: u8 }, /// 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, @@ -120,6 +123,7 @@ impl Overlay { Overlay::PointColorRange(i) => (1, i as f64), Overlay::Spots(t) => (2, t as f64), Overlay::Mask { id, view, color, opacity } => (3, pack_mask(id, view, color, opacity) as f64), + Overlay::AllMasks { view, color, opacity } => (5, pack_mask(0, view, color, opacity) as f64), Overlay::HdrRange => (4, 0.0), } } @@ -131,6 +135,14 @@ 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 if v.is_finite() && v >= 0.0 => { + let rgb = (v as u64 >> 20) & 0xff_ffff; + Overlay::AllMasks { + view: MaskView::ALL.get(((v as u64 >> 16) & 0xf) as usize).copied().unwrap_or_default(), + color: [(rgb >> 16) as u8, (rgb >> 8) as u8, rgb as u8], + opacity: ((v as u64 >> 44) & 0x7f).min(100) as u8, + } + } _ => Overlay::None, } } @@ -142,6 +154,7 @@ impl Overlay { Overlay::PointColorRange(i) => 0x1000 + i as u64, Overlay::Spots(t) => 0x2000 + t as u64, Overlay::Mask { id, view, color, opacity } => 3 << 60 | pack_mask(id, view, color, opacity), + Overlay::AllMasks { view, color, opacity } => 5 << 60 | pack_mask(0, view, color, opacity), Overlay::HdrRange => 0x4000, } } @@ -168,8 +181,9 @@ 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>) { +/// an [`Overlay::Mask`] shows, at the image's size (see [`Overlay::mask`]); `masks` is the set of +/// alphas an [`Overlay::AllMasks`] shows, one per visible mask with components. +pub fn apply(img: &mut Rgba8, o: Overlay, plan: &Plan<'_>, mask: Option<&Plane>, masks: Option<&[Plane]>) { match o { // drawn by the renderer from the HDR rendition ([`hdr_range`]) Overlay::None | Overlay::HdrRange => {} @@ -184,6 +198,15 @@ pub fn apply(img: &mut Rgba8, o: Overlay, plan: &Plan<'_>, mask: Option<&Plane>) mask_view(img, a, view, color, opacity); } } + Overlay::AllMasks { view, color, opacity } => { + if let Some(planes) = masks { + for a in planes { + if (a.width, a.height) == (img.width, img.height) { + mask_view(img, a, view, color, opacity); + } + } + } + } } } @@ -380,4 +403,33 @@ mod tests { assert_eq!(MaskView::WhiteOnBlack.next(), MaskView::Color); assert_eq!(MaskView::parse("colorOnBw"), Some(MaskView::ColorOnBw)); } + + #[test] + fn all_masks_overlay_shows_every_visible_mask_at_once() { + use lightcraft_develop::{Mask, MaskComponent, MaskOp, MaskShape}; + use lightcraft_geom::Point; + let src = Rgb32f::from_fn(80, 40, |_, _| [0.1, 0.1, 0.1]); + let info = SourceInfo::default(); + let left = MaskShape::Radial { center: Point::new(0.25, 0.5), rx: 0.15, ry: 0.15, angle: 0.0, feather: 10.0, invert: false }; + let right = MaskShape::Radial { center: Point::new(0.75, 0.5), rx: 0.15, ry: 0.15, angle: 0.0, feather: 10.0, invert: false }; + let mut s = DevelopSettings::default(); + s.masks.push(Mask { id: 1, visible: true, components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape: left }], ..Default::default() }); + s.masks.push(Mask { id: 2, visible: true, components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape: right }], ..Default::default() }); + let plain = render(&src, &info, &s, &RenderRequest::fit(80, 40)).image; + let o = Overlay::AllMasks { view: MaskView::WhiteOnBlack, color: [255, 0, 0], opacity: 100 }; + let img = render(&src, &info, &s, &RenderRequest { overlay: o, ..RenderRequest::fit(80, 40) }).image; + let (in_l, in_r, mid) = (20 * 80 + 12, 20 * 80 + 60, 20 * 80 + 40); + assert_eq!(img.data[in_l], [255; 4], "left mask region painted"); + assert_eq!(img.data[in_r], [255; 4], "right mask region painted"); + assert_eq!(img.data[mid], [0, 0, 0, 255], "between the masks: black"); + // a hidden mask is skipped + s.masks[1].visible = false; + let img2 = render(&src, &info, &s, &RenderRequest { overlay: o, ..RenderRequest::fit(80, 40) }).image; + assert_eq!(img2.data[in_r], plain.data[in_r], "hidden right mask not painted"); + assert_eq!(img2.data[in_l], [255; 4], "visible left mask still painted"); + // parts round trip and a stable, distinct cache key + let (k, v) = o.to_parts(); + assert_eq!(Overlay::from_parts(k, v), o); + assert_ne!(o.key(), Overlay::Mask { id: 1, view: MaskView::WhiteOnBlack, color: [255, 0, 0], opacity: 100 }.key()); + } } diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index b919aa2e6..89f59d1ea 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -149,6 +149,7 @@ const LAYOUT: &[(&str, &[&str])] = &[ "view.visualizeHdr", "view.softProof", "view.maskOverlay", + "view.maskOverlayAll", "view.maskOverlayMode", "view.maskOverlayColor", "view.maskPins", @@ -312,6 +313,7 @@ pub fn checked(app: &LightcraftApp, id: &str) -> Option { "view.visualizeHdr" => Some(u.hdr_visualize), "view.softProof" => Some(u.soft_proof), "view.maskOverlay" => Some(u.mask_overlay), + "view.maskOverlayAll" => Some(u.mask_overlay_all), "view.maskPins" => Some(u.mask_pins), "view.showOriginal" => Some(u.before_after == crate::state::BeforeAfter::Original), "view.beforeAfter" => Some(u.before_after == crate::state::BeforeAfter::SideBySide), diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 81a46eb98..5ae64e1a5 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -91,6 +91,8 @@ pub const UI_COMMANDS: &[UiCommand] = &[ ("view.softProof", "Soft Proofing", Some("S"), "View"), ("view.histogram", "Histogram", Some("Cmd+Shift+H"), "View"), ("view.maskOverlay", "Show Mask Overlay", Some("O"), "View"), + // Ctrl+O / Cmd+O: every mask's region at once, not just the selected one + ("view.maskOverlayAll", "Show All Mask Overlays", Some("Cmd+O"), "View"), // Shift+O in the Masking panel (elsewhere it cycles the crop overlay) ("view.maskOverlayMode", "Cycle Mask Overlay Mode", None, "View"), ("view.maskOverlayColor", "Cycle Mask Overlay Color", None, "View"), @@ -634,6 +636,10 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { + app.ui.mask_overlay_all = !app.ui.mask_overlay_all; + Ok(json!({"maskOverlayAll": app.ui.mask_overlay_all})) + } "view.maskOverlayMode" => { use lightcraft_engine::pipeline::MaskView; let cur = MaskView::parse(&app.ui.mask_overlay_mode).unwrap_or_default(); diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index c3678ae20..24976d94e 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1065,6 +1065,18 @@ pub(crate) fn view_overlay(app: &LightcraftApp, d: &DevelopSettings) -> lightcra if app.ui.fullscreen { return Overlay::None; } + // Show every visible mask's region at once (Ctrl+O / Cmd+O), ahead of the selected-mask overlay + // so the shortcut is not shadowed by the always-on single-mask overlay. + if app.ui.right == RightPanel::Masking + && app.ui.mask_overlay_all + && d.masks.iter().any(|m| m.visible && !m.components.is_empty()) + { + return Overlay::AllMasks { + view: MaskView::parse(&app.ui.mask_overlay_mode).unwrap_or_default(), + color: app.ui.mask_overlay_color, + opacity: app.ui.mask_overlay_opacity.clamp(0.0, 100.0).round() as u8, + }; + } 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/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 640bc4f11..47b96ee6d 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -197,6 +197,10 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { if icon_button(ui, "maskOverlay", Icon::Eye, vec2(24.0, 24.0), on, true, "Show overlay (O)").clicked() { app.ui.mask_overlay = !on; } + let all = app.ui.mask_overlay_all; + if icon_button(ui, "maskOverlayAll", Icon::Stack, vec2(24.0, 24.0), all, true, "Show all overlays (Ctrl+O)").clicked() { + app.ui.mask_overlay_all = !all; + } }); }); if d.masks.is_empty() { diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 6a50e2baf..b52f7ec64 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -332,6 +332,9 @@ pub struct UiState { pub mask_overlay_color: [u8; 3], pub mask_overlay_opacity: f32, pub mask_pins: bool, + /// Masking: show every visible mask's region as one overlay at once (Ctrl+O / Cmd+O), + /// instead of just the selected mask's ([`mask_overlay`]). + pub mask_overlay_all: bool, /// The overlay (on, mode) to go back to when Show Luminance Map is turned off. #[serde(skip)] pub luminance_map_restore: Option<(bool, String)>, @@ -813,6 +816,7 @@ impl Default for UiState { mask_overlay_color: [230, 30, 40], mask_overlay_opacity: 50.0, mask_pins: true, + mask_overlay_all: false, crop_overlay: CropOverlay::Thirds, crop_overlay_orient: 0, show_filenames: true, diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index b95fa1ab2..db009e816 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -168,6 +168,15 @@ fn mask_adjustments_hide_overlay_without_disabling_the_mask() { assert_eq!(develop(&h).masks[0], mask, "toggling the overlay leaves the mask and its effect unchanged"); assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::Mask { .. })); + // Ctrl+O shows every visible mask's region at once (the new command), ahead of the selected + // mask overlay so it is not shadowed by the always-on single-mask overlay. + exec(&mut h, "view.maskOverlayAll", json!({})); + assert!(h.app.ui.mask_overlay_all, "Ctrl+O turns on the all-masks overlay"); + assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::AllMasks { .. })); + exec(&mut h, "view.maskOverlayAll", json!({})); + assert!(!h.app.ui.mask_overlay_all, "Ctrl+O toggles the all-masks overlay off"); + assert!(!matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::AllMasks { .. })); + // Failed edits and global adjustments must not hide the mask overlay. assert!(h.app.run("mask.adjust", json!({"id": 999, "values": {"exposure": 1.0}})).is_err()); assert!(h.app.ui.mask_overlay); From a61a52ed8b37880244d1e62a3b3d2fd4c3b71144 Mon Sep 17 00:00:00 2001 From: THEtheChad <880190+THEtheChad@users.noreply.github.com> Date: Sun, 11 Oct 2026 08:08:14 +0000 Subject: [PATCH 02/14] perf(gpu): LR-BEHAV-GPU: horizontal box blur reads adjacent pixels The horizontal box pass gave each thread 16+ consecutive pixels to slide a running sum over, so a workgroup's threads read pixels 16 apart. On a Radeon 780M (DX12) that pass took 9-15x as long as the vertical one, and it sits under every GPU Gaussian and guided filter (NR, texture, dehaze, clarity/base). Each thread now sums one pixel's 2r+1 window: adjacent threads read adjacent pixels. Same 64x4 workgroups, parameters and bindings; box_v unchanged. Measured on the Radeon 780M (6000x4000): guided filter at sigma 1.75 828 -> 146 ms, colour-NR Gaussian 495 -> 104 ms. Full-size heavy-edit export of the CC0 Sony ARW: GPU 3.6-4.0 s -> 1.9-2.0 s (CPU 1.9-2.1 s); GPU-vs-CPU output distance unchanged. Adds gaussian_matches_cpu (GPU blur vs lightcraft_raster::blur::gaussian, 1-3 channels, partial workgroups, windows wider than the image) and per-pass rows to bench_kernels. Co-Authored-By: Claude Opus 5.5 --- crates/gpu/src/render.rs | 58 ++++++++++++++++++++++++++++++----- crates/gpu/src/wgsl/blur.wgsl | 22 ++++++------- docs/gpu-pipeline.md | 13 ++++++-- docs/whats-new.md | 5 +++ 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 44458350f..acafbdf5d 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -229,10 +229,19 @@ pub(crate) fn rgb_words(img: &Rgb32f) -> &[f32] { // --------------------------------------------------------------------------------------------- // Filters (twins of `lightcraft_raster::blur` / `resample` and `lightcraft_pipeline::local`) -/// Pixels each blur thread slides its running sum over (at least; longer for large radii, so the +/// Rows each `box_v` thread slides its running sum over (at least; longer for large radii, so the /// priming sum stays a small share of the work). const CHUNK: usize = 16; +/// One box pass of radius `r` (clamped edges) over a `w × h` image of `nc` interleaved channels, +/// `src → dst`: horizontal or vertical. +fn box_pass(cx: &mut Cx<'_>, horizontal: bool, src: &Buf, dst: &Buf, (w, h): (usize, usize), nc: usize, r: usize) { + let chunk = (2 * r + 1).clamp(CHUNK, 128); + let p = [w as u32, h as u32, nc as u32, r as u32, chunk as u32]; + let (k, groups) = if horizontal { ("box_h", groups2(w, h, [64, 4])) } else { ("box_v", groups2(w, h.div_ceil(chunk), [64, 4])) }; + cx.run(k, &p, &[Some(src), Some(dst)], groups); +} + /// Gaussian blur of a `w × h` image of `nc` interleaved channels: three box passes each way /// (`lightcraft_raster::blur::gaussian`). pub(crate) fn gaussian(cx: &mut Cx<'_>, src: &Buf, w: usize, h: usize, nc: usize, sigma: f32) -> Buf { @@ -240,18 +249,15 @@ pub(crate) fn gaussian(cx: &mut Cx<'_>, src: &Buf, w: usize, h: usize, nc: usize return cx.copy(src); } let radii = lightcraft_raster::blur::box_radii(sigma); - let passes: Vec<(&str, usize)> = radii.iter().map(|r| ("box_h", *r)).chain(radii.iter().map(|r| ("box_v", *r))).filter(|p| p.1 > 0).collect(); + let passes: Vec<(bool, usize)> = radii.iter().map(|r| (true, *r)).chain(radii.iter().map(|r| (false, *r))).filter(|p| p.1 > 0).collect(); if passes.is_empty() { return cx.copy(src); } // ping-pong between two buffers let mut bufs = [cx.gpu.buffer(w * h * nc), cx.gpu.buffer(w * h * nc)]; - for (i, (k, r)) in passes.iter().enumerate() { - let chunk = (2 * r + 1).clamp(CHUNK, 128); - let groups = if *k == "box_h" { groups2(w.div_ceil(chunk), h, [64, 4]) } else { groups2(w, h.div_ceil(chunk), [64, 4]) }; - let p = [w as u32, h as u32, nc as u32, *r as u32, chunk as u32]; + for (i, (horizontal, r)) in passes.iter().enumerate() { let (out, prev) = (&bufs[i % 2], &bufs[(i + 1) % 2]); - cx.run(k, &p, &[Some(if i == 0 { src } else { prev }), Some(out)], groups); + box_pass(cx, *horizontal, if i == 0 { src } else { prev }, out, (w, h), nc, *r); } let last = (passes.len() - 1) % 2; let [a, b] = std::mem::replace(&mut bufs, [cx.gpu.buffer(0), cx.gpu.buffer(0)]); @@ -1132,6 +1138,33 @@ mod tests { assert!(covered > 0 && covered < w * h, "{covered} px covered"); } + /// The blur against `lightcraft_raster::blur::gaussian`: widths that do and don't fill the last + /// workgroup, windows wider than the image, and 1, 2 (as two planes) and 3 channels. + #[test] + fn gaussian_matches_cpu() { + use lightcraft_raster::blur; + let Some(gpu) = crate::device() else { return }; + let _scope = crate::ctx::RenderScope::new(gpu); + let v = |x: usize, y: usize, c: usize| ((x * 7 + y * 13 + c * 5) % 17) as f32 / 16.0 + if x % 61 < 3 { 2.0 } else { 0.0 }; + for (w, h) in [(1000, 5), (257, 3), (5, 4), (1, 9)] { + for sigma in [0.8f32, 2.0, 4.5, 30.0, 250.0, 1000.0] { + let planes: Vec = (0..3).map(|c| Plane::from_fn(w, h, |x, y| v(x, y, c))).collect(); + let want: Vec = planes.iter().map(|p| blur::gaussian(p, sigma)).collect(); + for nc in [1, 2, 3] { + let words: Vec = (0..w * h).flat_map(|i| (0..nc).map(move |c| (i, c))).map(|(i, c)| planes[c].data[i]).collect(); + let mut cx = Cx::new(gpu); + let src = gpu.upload(&words); + let out = gaussian(&mut cx, &src, w, h, nc, sigma); + let got: Vec = cx.read(&out, w * h * nc); + for (i, g) in got.iter().enumerate() { + let want = want[i % nc].data[i / nc]; + assert!((g - want).abs() < 1e-5 * (1.0 + sigma / 10.0), "{w}×{h} nc={nc} sigma={sigma}: word {i} {g} vs {want}"); + } + } + } + } + } + /// Kernel timings on a 24 MP plane: `cargo test --release -p lightcraft-gpu -- --ignored --nocapture`. #[test] #[ignore] @@ -1139,6 +1172,7 @@ mod tests { let Some(gpu) = crate::device() else { return }; let _scope = crate::ctx::RenderScope::new(gpu); let (w, h) = (6000, 4000); + eprintln!("{} ({:?})", gpu.info.name, gpu.info.backend); let src = gpu.upload(&vec![0.5f32; w * h * 3]); let time = |name: &str, f: &mut dyn FnMut(&mut Cx<'_>)| { let mut best = f64::MAX; @@ -1152,9 +1186,19 @@ mod tests { } eprintln!("{name:<40} {best:.1} ms"); }; + for nc in [1, 2, 3] { + let out = gpu.buffer(w * h * nc); + // noise reduction's radii, Texture's and Dehaze's at 6000 px + for r in [2, 11, 120] { + for (k, horizontal) in [("box_h", true), ("box_v", false)] { + time(&format!("{k} nc={nc} r={r}"), &mut |cx| box_pass(cx, horizontal, &src, &out, (w, h), nc, r)); + } + } + } for nc in [1, 2, 3] { time(&format!("gaussian nc={nc} sigma=2"), &mut |cx| drop(gaussian(cx, &src, w, h, nc, 2.0))); } + time("gaussian nc=3 sigma=4.5 (colour NR)", &mut |cx| drop(gaussian(cx, &src, w, h, 3, 4.5))); time("map log_lum", &mut |cx| { let o = cx.gpu.buffer(w * h); map(cx, "log_lum_k", w * h, &[], [Some(&src), None, None], &o); diff --git a/crates/gpu/src/wgsl/blur.wgsl b/crates/gpu/src/wgsl/blur.wgsl index b0e28370d..375fb6d55 100644 --- a/crates/gpu/src/wgsl/blur.wgsl +++ b/crates/gpu/src/wgsl/blur.wgsl @@ -1,6 +1,6 @@ // Box passes (clamped edges) over an image of NC interleaved channels; three of each approximate a -// Gaussian (`lightcraft_raster::blur::gaussian`). Each thread slides a running sum over CH pixels. -// P: w, h, nc, r, ch. Bindings: src, dst. +// Gaussian (`lightcraft_raster::blur::gaussian`). +// P: w, h, nc, r, ch (box_v only). Bindings: src, dst. // Pixel `i` (all nc ≤ 3 channels at once: one contiguous read per pixel). fn ld(i: u32, nc: u32) -> vec3 { @@ -25,31 +25,27 @@ fn st(i: u32, nc: u32, v: vec3) { } } -// Horizontal: one thread per CH consecutive pixels of a row. +// Horizontal: one thread per pixel, summing its 2r + 1 window; adjacent threads read adjacent +// pixels, which the cache serves. (Threads that each slid a running sum over 16 pixels read pixels +// 16 apart across a workgroup: on a Radeon 780M that pass took 9–15× as long as `box_v`.) @compute @workgroup_size(64, 4) fn box_h(@builtin(global_invocation_id) g: vec3) { let w = pu(0u); let h = pu(1u); let nc = pu(2u); let r = i32(pu(3u)); - let ch = pu(4u); + let x = g.x; let y = g.y; - let x0 = g.x * ch; - if (y >= h || x0 >= w) { + if (x >= w || y >= h) { return; } let last = i32(w) - 1; - let inv = 1.0 / f32(2 * r + 1); let row = y * w; - let x1 = min(x0 + ch, w); var acc = vec3(0.0); for (var k = -r; k <= r; k++) { - acc += ld(row + u32(clamp(i32(x0) + k, 0, last)), nc); - } - for (var x = x0; x < x1; x++) { - st(row + x, nc, acc * inv); - acc = acc + ld(row + u32(min(i32(x) + r + 1, last)), nc) - ld(row + u32(max(i32(x) - r, 0)), nc); + acc += ld(row + u32(clamp(i32(x) + k, 0, last)), nc); } + st(row + x, nc, acc * (1.0 / f32(2 * r + 1))); } // Vertical: one thread per CH consecutive rows of a column (the CPU re-primes every BAND rows the diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md index 883c3a0bf..edfdf55d6 100644 --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -139,7 +139,8 @@ walks the drive. - Kernels (`crates/gpu/src/wgsl/`): `geom` (orientation pixel map, bilinear through the crop/straighten/flip affine or the full lens + perspective warp), `resize` (separable resample with the CPU's taps; Mitchell prefilter, box/bilinear for the fast guided filter), `blur` (box - passes with running sums over pixel chunks; three each way = the CPU's Gaussian), `map` + passes, three each way = the CPU's Gaussian: horizontal ones sum each pixel's window, so adjacent + threads read adjacent pixels; vertical ones slide running sums down chunks of a column), `map` (log luminance, dark channel, guided-filter steps, white balance, luminance / colour NR, airlight sampling), `mask` (linear / radial / luminance / colour range / brush shapes, combine, finalize), `finish` (the whole per-pixel stage) — each mirrors a named CPU function. @@ -233,7 +234,7 @@ noise-reduction size come from the whole frame, and a window that spots or Auto cut back after the readback. The tests skip (pass with a note) when no adapter exists. Remaining differences come from f32 vs f64 coordinate math, fast-math transcendental functions on -Metal and running-sum order in the box filters — all far below one 8-bit step. +Metal and summation order in the box filters — all far below one 8-bit step. ## Measurements `render_bench` on `corpus/raw/arw-sony-a7m3-compressed.arw` (24 MP; loupe = 2560 px preview → @@ -251,6 +252,14 @@ so CPU numbers are pessimistic; the GPU numbers are less affected): | per-pixel stage 1920×1280, + vibrance/saturation | 53–65 ms | 5 ms | | export render 6000×4000 | 1037–1414 ms | 291–330 ms | +On Windows (Radeon 780M iGPU, DX12), a full-size JPEG export of the same raw with heavy edits (NR 30 +/ 25, clarity, texture, dehaze, sharpening, highlights / shadows) took 3.6–4.0 s on the GPU against +1.9–2.0 s on the CPU: the horizontal box pass had each thread slide a running sum over 16 pixels, so +the threads of a workgroup read pixels 16 apart, and it took 9–15× as long as the vertical pass. +Summing each pixel's window instead (adjacent threads, adjacent pixels): 1.9–2.0 s, with the GPU's +NR stage 1.5 → 0.4 s and spatial planes 0.89 → 0.29 s (`LIGHTCRAFT_PROFILE`). The ignored test +`bench_kernels` (`crates/gpu/src/render.rs`) times the passes on a 24 MP plane. + GPU timings include the readback of the 8-bit result and the histogram. Device creation + kernel compilation: ~0.4 s once per process — the desktop app starts it on a background thread at launch (`lightcraft_gpu::warm_up`), so the first loupe render doesn't wait for it; other processes create diff --git a/docs/whats-new.md b/docs/whats-new.md index fb8b285a3..6f93109dd 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -41,6 +41,11 @@ the other editor (Edit in Other Editor, now ⌥⌘E), and where PhotoCraft is if you use a portable copy. - Export can write PSD too (flat, 8- or 16-bit). +### Faster GPU exports +- Exports with noise reduction, Texture, Clarity, Dehaze or Highlights and Shadows can be much faster on the GPU, + depending on the graphics card. On a Windows laptop with an AMD Radeon 780M, a full-size export of a 24 MP raw + with those edits went from about 3.7 s to 1.9 s. Photos look the same as before. + ### Reliability - Upgrading from a Windows installer older than 0.6 (such as 0.5.0) keeps LightCraft in the folder it was installed in, instead of moving it back to Program Files (issue #399). If that folder has been deleted since, the upgrade From b7222213c9df1bafce1df58ac34847201da95002 Mon Sep 17 00:00:00 2001 From: OneSlips Date: Sun, 11 Oct 2026 13:08:55 +1000 Subject: [PATCH 03/14] PERF-PRESETS: thousands of presets no longer stall every command and frame A Lightroom collection imported whole (11 113 presets, a 42 MB presets.json) made every interaction take seconds: - Session::persist ran after every command and serialized the whole preset list to compare it with the last write: 609 ms per slider tick (release, the real collection). Session::presets is now a PresetList whose generation changes on mutable access; persist serializes only when it (or the profile lists) changed, and skips the write when the bytes hash the same. develop.set: 609 ms -> 2.3 ms; preset.apply: 598 ms -> 3.1 ms. - The Presets panel deep-cloned every preset into a BTreeMap, built a row, widget id and (with thumbnails) a variant render job for each of them every frame. Grouping is cached per generation and only rows in view are laid out: 5041 -> ~20 rows a frame, 2.3 ms with 5000 presets. - The preset pickers (Import dialog, Settings > Import) build only the rows in view. - add_presets and opening a library compared every preset with every other (quadratic); both use hash indexes now. Opening no longer serializes the list once just to remember it. Tests: develop commands don't re-serialize unchanged presets (and a change is still saved), generation semantics, a 20 000-preset import, and the panel building only visible rows (fails on the old code: 5041 rows). An ignored bench (presets_scale_bench, LIGHTCRAFT_BENCH_PRESETS) times a real presets.json. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/lib.rs | 7 +- crates/engine/src/library.rs | 79 ++++++++++++-- crates/engine/src/presets.rs | 84 ++++++++++++++- crates/engine/src/tests_presets_scale.rs | 126 ++++++++++++++++++++++ crates/ui-egui/src/headless.rs | 45 ++++++++ crates/ui-egui/src/import.rs | 13 +-- crates/ui-egui/src/panels/presets.rs | 132 ++++++++++++++++++++--- crates/ui-egui/src/panels/settings.rs | 17 +-- docs/whats-new.md | 8 ++ 9 files changed, 460 insertions(+), 51 deletions(-) create mode 100644 crates/engine/src/tests_presets_scale.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3bc6c431e..6644f0f99 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -221,7 +221,8 @@ pub struct Session { pub previous_active: Option, /// Groups last used for Copy (Lightroom remembers them). pub copy_groups: Vec, - pub presets: Vec, + /// Built-in then user presets; its generation tells savers and caches when it changed. + pub presets: presets::PresetList, /// Favourite profile ids (persisted with the library, like preset favourites). pub profile_favorites: Vec, /// Recently applied profile ids, newest first (at most [`presets::RECENT_PROFILES`]). @@ -354,7 +355,7 @@ impl Session { library_folder: None, previous_active: None, copy_groups: lightcraft_develop::SettingsGroup::default_copy(), - presets: presets::builtin(), + presets: presets::PresetList::new(presets::builtin()), profile_favorites: Vec::new(), profile_recent: Vec::new(), journal: Vec::new(), @@ -1019,6 +1020,8 @@ mod tests_persist; #[cfg(test)] mod tests_prefs; #[cfg(test)] +mod tests_presets_scale; +#[cfg(test)] mod tests_process; #[cfg(test)] mod tests_segment; diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index 2bb5511c2..0305623f2 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -88,7 +88,8 @@ pub struct Library { retry_at: Option, /// What forgetting untouched Local records did when the library opened. pub forgot_local: Option, - presets_written: String, + /// What presets.json holds (see [`PresetsSaved`]). + presets_saved: PresetsSaved, view_written: Vec, /// Settings files that were unreadable or damaged when the library opened (`library.info` → /// `settingsWarnings`; shown by the UI once, see [`Session::take_library_warnings`]). @@ -123,6 +124,53 @@ struct PresetsFile { profile_recent: Vec, } +/// The state presets.json was last written (or read) from. Saving checks the preset list's +/// generation and the small profile lists before serializing anything: with a 10 000-preset +/// collection presets.json is ~40 MB, and serializing it after every command made each slider +/// tick take seconds. +#[derive(Default)] +struct PresetsSaved { + generation: u64, + profile_favorites: Vec, + profile_recent: Vec, + /// Hash of the bytes written: a list borrowed mutably but left as it was isn't rewritten. + hash: u64, +} + +impl PresetsSaved { + fn state_of(s: &Session) -> PresetsSaved { + PresetsSaved { + generation: s.presets.generation(), + profile_favorites: s.profile_favorites.clone(), + profile_recent: s.profile_recent.clone(), + hash: 0, + } + } + + /// `s` changed its presets or profile lists since this state. + fn differs(&self, s: &Session) -> bool { + self.generation != s.presets.generation() || self.profile_favorites != s.profile_favorites || self.profile_recent != s.profile_recent + } +} + +fn bytes_hash(b: &[u8]) -> u64 { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + b.hash(&mut h); + h.finish() +} + +#[cfg(test)] +thread_local! { + static PRESETS_SERIALIZED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// How many times this thread serialized the preset list (tests). +#[cfg(test)] +pub(crate) fn presets_serialized() -> usize { + PRESETS_SERIALIZED.with(|c| c.get()) +} + #[derive(Default, Serialize, Deserialize)] #[serde(default)] struct ViewFile { @@ -187,6 +235,8 @@ struct PrefsFile { } fn presets_json(s: &Session) -> String { + #[cfg(test)] + PRESETS_SERIALIZED.with(|c| c.set(c.get() + 1)); let presets = &s.presets; let f = PresetsFile { user: presets.iter().filter(|p| !p.builtin).cloned().collect(), @@ -414,8 +464,9 @@ impl Session { for p in &mut self.presets { p.favorite = p.builtin && f.favorites.contains(&p.id) || (!p.builtin && p.favorite); } + let mut ids: std::collections::HashSet = self.presets.iter().map(|p| p.id.clone()).collect(); for u in f.user { - if !self.presets.iter().any(|p| p.id == u.id) { + if ids.insert(u.id.clone()) { self.presets.push(u); } } @@ -447,7 +498,8 @@ impl Session { { self.selection = Selection::single(*first); } - let presets_written = presets_json(self); + // presets.json is as read: saved again only after the list changes (hash 0 = unknown bytes) + let presets_saved = PresetsSaved::state_of(self); // photo ids are per library: drop decoded sources of the previous one self.media.clear_sources(); if on_disk { @@ -464,7 +516,7 @@ impl Session { unsaved_error: None, retry_at: None, forgot_local: None, - presets_written, + presets_saved, view_written, settings_warnings: unlocked_warning(lock.as_ref()).into_iter().chain(settings.warnings).collect(), warnings_reported: 0, @@ -526,14 +578,21 @@ impl Session { log::error!("library: compaction: {e}"); lib.last_error = Some(format!("compaction: {e}")); } + let due = self.library.as_ref().is_some_and(|l| l.presets_saved.differs(self) && !l.blocked.contains(&"presets.json")); + if !due { + return Ok(()); + } let presets = presets_json(self); + let mut state = PresetsSaved::state_of(self); + state.hash = bytes_hash(presets.as_bytes()); let Some(lib) = self.library.as_mut() else { return Ok(()) }; - if presets != lib.presets_written && !lib.blocked.contains(&"presets.json") { - if let Err(e) = lib.files.write_atomic("presets.json", presets.as_bytes()) { - log::error!("library: presets: {e}"); - } else { - lib.presets_written = presets; - } + if state.hash == lib.presets_saved.hash { + lib.presets_saved = state; + } else if let Err(e) = lib.files.write_atomic("presets.json", presets.as_bytes()) { + // the state stays stale: the next save tries again + log::error!("library: presets: {e}"); + } else { + lib.presets_saved = state; } Ok(()) } diff --git a/crates/engine/src/presets.rs b/crates/engine/src/presets.rs index 8c36aa742..c3675e227 100644 --- a/crates/engine/src/presets.rs +++ b/crates/engine/src/presets.rs @@ -23,6 +23,73 @@ pub struct PresetFile { pub presets: Vec, } +/// The session's presets (built-in, then the user's) with a generation that changes on every +/// mutable access, so saving and the UI's caches skip their O(presets) work while the list is +/// untouched: a Lightroom collection imported whole is 10 000+ presets and ~40 MB of JSON. +/// +/// Reads go through `Deref` (`iter`, `len`, `find`…) and keep the generation; `push`, `iter_mut`, +/// `remove`… go through `DerefMut` and bump it, whether or not anything is then changed. +/// Generations are unique across all lists in the process. +#[derive(Clone, Debug)] +pub struct PresetList { + items: Vec, + generation: u64, +} + +static PRESET_GENERATION: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); + +fn next_generation() -> u64 { + PRESET_GENERATION.fetch_add(1, std::sync::atomic::Ordering::Relaxed) +} + +impl PresetList { + pub fn new(items: Vec) -> PresetList { + PresetList { items, generation: next_generation() } + } + + /// Changes whenever the list may have changed. + pub fn generation(&self) -> u64 { + self.generation + } +} + +impl Default for PresetList { + fn default() -> PresetList { + PresetList::new(Vec::new()) + } +} + +impl std::ops::Deref for PresetList { + type Target = Vec; + fn deref(&self) -> &Vec { + &self.items + } +} + +impl std::ops::DerefMut for PresetList { + fn deref_mut(&mut self) -> &mut Vec { + self.generation = next_generation(); + &mut self.items + } +} + +impl<'a> IntoIterator for &'a PresetList { + type Item = &'a Preset; + type IntoIter = std::slice::Iter<'a, Preset>; + fn into_iter(self) -> Self::IntoIter { + self.items.iter() + } +} + +impl<'a> IntoIterator for &'a mut PresetList { + type Item = &'a mut Preset; + type IntoIter = std::slice::IterMut<'a, Preset>; + fn into_iter(self) -> Self::IntoIter { + self.generation = next_generation(); + self.items.iter_mut() + } +} + /// Lower-case ASCII slug for ids (`"Warm & Soft"` → `warm-soft`). pub fn slug(s: &str) -> String { let mut out = String::new(); @@ -107,22 +174,33 @@ impl Session { /// Add imported presets. Identical presets (same name, group and settings) already present are /// skipped; id clashes get a fresh id. Returns the ids added. pub fn add_presets(&mut self, presets: Vec) -> Vec { + use std::collections::{HashMap, HashSet}; + // indexed: a collection of 10 000+ presets was quadratic (minutes) to import + let mut ids: HashSet = self.presets.iter().map(|q| q.id.clone()).collect(); + let mut by_name: HashMap<(String, String), Vec> = HashMap::new(); + for (i, q) in self.presets.iter().enumerate() { + by_name.entry((q.name.clone(), q.group.clone())).or_default().push(i); + } let mut added = Vec::new(); for mut p in presets { - if self.presets.iter().any(|q| q.name == p.name && q.group == p.group && q.settings == p.settings) { + let key = (p.name.clone(), p.group.clone()); + let same = by_name.get(&key).is_some_and(|ix| ix.iter().any(|&i| self.presets.get(i).is_some_and(|q| q.settings == p.settings))); + if same { continue; } - if p.id.trim().is_empty() || p.id.starts_with("lc.") || self.presets.iter().any(|q| q.id == p.id) { + if p.id.trim().is_empty() || p.id.starts_with("lc.") || ids.contains(&p.id) { let base = format!("user.{}", slug(&format!("{}-{}", p.group, p.name))); let mut id = base.clone(); let mut n = 2; - while self.presets.iter().any(|q| q.id == id) { + while ids.contains(&id) { id = format!("{base}-{n}"); n += 1; } p.id = id; } p.builtin = false; + ids.insert(p.id.clone()); + by_name.entry(key).or_default().push(self.presets.len()); added.push(p.id.clone()); self.presets.push(p); } diff --git a/crates/engine/src/tests_presets_scale.rs b/crates/engine/src/tests_presets_scale.rs new file mode 100644 index 000000000..866b7deb3 --- /dev/null +++ b/crates/engine/src/tests_presets_scale.rs @@ -0,0 +1,126 @@ +//! A library with thousands of user presets (a Lightroom preset collection imported whole) stays +//! responsive: commands that don't touch presets never re-serialize them, and opening or importing +//! doesn't compare every preset with every other. + +use std::path::PathBuf; +use std::time::Instant; + +use lightcraft_develop::Preset; +use serde_json::json; + +use crate::Session; + +fn temp_dir(tag: &str) -> PathBuf { + let d = std::env::temp_dir().join(format!("lc-presets-scale-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d +} + +fn many(n: usize) -> Vec { + (0..n) + .map(|i| Preset { + id: format!("user.coll-{}.look-{i}", i / 100), + name: format!("Look {i}"), + group: format!("Collection {}", i / 100), + settings: json!({"light": {"exposure": (i % 40) as f64 / 10.0 - 2.0, "contrast": (i % 50) as f64}, "color": {"vibrance": (i % 30) as f64}}), + favorite: false, + builtin: false, + }) + .collect() +} + +#[test] +fn develop_commands_do_not_reserialize_unchanged_presets() { + let lib = temp_dir("no-reserialize"); + let mut s = Session::new(); + s.open_library(&lib, true).unwrap(); + let added = s.add_presets(many(2000)); + assert_eq!(added.len(), 2000); + s.persist().unwrap(); + assert!(std::fs::metadata(lib.join("presets.json")).unwrap().len() > 100_000, "presets saved"); + let before = crate::library::presets_serialized(); + for i in 0..20 { + s.execute("develop.set", &json!({"control": "light.exposure", "value": i as f64 / 10.0})).unwrap(); + } + assert_eq!(crate::library::presets_serialized(), before, "slider commands left the preset list alone"); + // a preset change is still saved + let pid = added[7].clone(); + s.execute("preset.favorite", &json!({"id": pid})).unwrap(); + assert!(crate::library::presets_serialized() > before); + s.close_library().unwrap(); + drop(s); + let mut t = Session::new(); + t.open_library(&lib, false).unwrap(); + assert!(t.presets.iter().any(|p| p.id == pid && p.favorite), "favourite persisted"); + assert_eq!(t.presets.iter().filter(|p| !p.builtin).count(), 2000); + let _ = std::fs::remove_dir_all(&lib); +} + +#[test] +fn preset_list_generation_follows_mutable_access_only() { + let mut s = Session::new(); + let g0 = s.presets.generation(); + let _ = s.presets.iter().filter(|p| p.builtin).count(); + let _ = s.presets.len(); + assert_eq!(s.presets.generation(), g0, "reads keep the generation"); + s.presets.push(many(1).remove(0)); + let g1 = s.presets.generation(); + assert_ne!(g1, g0); + for p in &mut s.presets { + p.favorite = false; + } + assert_ne!(s.presets.generation(), g1); + // generations are unique across sessions (UI caches key on them alone) + let other = Session::new(); + assert_ne!(other.presets.generation(), s.presets.generation()); +} + +#[test] +fn importing_thousands_of_presets_is_not_quadratic() { + let mut s = Session::new(); + let t = Instant::now(); + let added = s.add_presets(many(20_000)); + assert_eq!(added.len(), 20_000); + // the same presets again are all skipped as duplicates + assert!(s.add_presets(many(20_000)).is_empty()); + // a clashing id with different settings gets a fresh, unique one + let mut clash = many(1).remove(0); + clash.settings = json!({"light": {"exposure": 3.0}}); + let id = s.add_presets(vec![clash]).remove(0); + assert_ne!(id, "user.coll-0.look-0"); + assert_eq!(s.presets.iter().filter(|p| p.id == id).count(), 1); + // quadratic was ~200 M comparisons here (tens of seconds in debug builds) + assert!(t.elapsed().as_secs() < 20, "took {:?}", t.elapsed()); +} + +/// Timing with a real preset collection: `LIGHTCRAFT_BENCH_PRESETS=path/to/presets.json cargo test +/// -p lightcraft-engine --release presets_scale_bench -- --ignored --nocapture`. The file is copied +/// into a temporary library; the original is only read. +#[test] +#[ignore] +fn presets_scale_bench() { + let Ok(src) = std::env::var("LIGHTCRAFT_BENCH_PRESETS") else { + eprintln!("set LIGHTCRAFT_BENCH_PRESETS"); + return; + }; + let lib = temp_dir("bench"); + std::fs::copy(&src, lib.join("presets.json")).unwrap(); + let mut s = Session::new(); + let t = Instant::now(); + s.open_library(&lib, true).unwrap(); + eprintln!("open library: {:?} ({} presets)", t.elapsed(), s.presets.len()); + let t = Instant::now(); + for i in 0..20 { + s.execute("develop.set", &json!({"control": "light.exposure", "value": i as f64 / 10.0})).unwrap(); + } + eprintln!("develop.set: {:?} per command", t.elapsed() / 20); + let pid = s.presets.iter().rev().find(|p| !p.builtin).map(|p| p.id.clone()).unwrap(); + let t = Instant::now(); + s.execute("preset.apply", &json!({"id": pid, "amount": 100})).unwrap(); + eprintln!("preset.apply: {:?}", t.elapsed()); + let t = Instant::now(); + s.execute("preset.favorite", &json!({"id": pid})).unwrap(); + eprintln!("preset.favorite (saves presets.json): {:?}", t.elapsed()); + let _ = std::fs::remove_dir_all(&lib); +} diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 8cb91b9b3..26b333073 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1510,6 +1510,51 @@ mod tests { h.settle(SETTLE); } + /// A collection of thousands of presets (a Lightroom library imported whole) lists only the + /// rows in view: a few dozen widgets and thumbnails a frame, not one per preset (every frame + /// built all of them and froze the app for seconds); scrolling brings later rows in. + #[test] + fn presets_panel_with_thousands_of_presets_builds_only_visible_rows() { + let mut h = demo([1400.0, 900.0]); + let t = Duration::from_secs(10); + let many = (0..5000) + .map(|i| lightcraft_develop::Preset { + id: format!("user.big.{i}"), + name: format!("Look {i}"), + group: format!("Big {:02}", i / 500), + settings: json!({"light": {"exposure": (i % 20) as f64 / 10.0}}), + favorite: false, + builtin: false, + }) + .collect(); + assert_eq!(h.app.session.add_presets(many).len(), 5000); + h.request("ui.set", json!({"view": "detail"}), t); + h.request("engine.execute", json!({"command": "panel.presets"}), t); + h.app.ui.preset_thumbs = true; + h.settle(SETTLE); + h.step(); + let rows = |h: &Headless| h.app.widgets.iter().filter(|(w, _)| w.starts_with("preset:")).map(|(w, _)| w.clone()).collect::>(); + let first = rows(&h); + assert!((5..60).contains(&first.len()), "{} rows built", first.len()); + assert!(h.app.renderer.variant_textures() < 60, "{} thumbnails", h.app.renderer.variant_textures()); + assert!(h.app.widgets.iter().any(|(w, _)| w == "presetGroup:Big 09"), "every group header is listed"); + let started = std::time::Instant::now(); + h.step(); + eprintln!("presets panel frame with 5000 presets: {:?}", started.elapsed()); + // scroll far down over the list: later presets come into view + let r = h.request("ui.hoverWidget", json!({"id": first[first.len() / 2]}), t); + assert_eq!(r["ok"], true, "{r}"); + for _ in 0..20 { + h.request("ui.scroll", json!({"dy": -2000}), t); + h.step(); + } + h.step(); + let later = rows(&h); + assert!((5..60).contains(&later.len()), "{} rows built", later.len()); + let deep = later.iter().filter_map(|w| w.strip_prefix("preset:user.big.")).filter_map(|n| n.parse::().ok()).max(); + assert!(deep.is_some_and(|n| n > 500), "scrolled rows: {later:?}"); + } + /// The filter bar drives `library.filter` and saves the view as a smart album. #[test] fn filter_bar_filters_and_saves_a_smart_album() { diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index a18357eca..620c441ad 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -1212,16 +1212,9 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { if ui.selectable_label(d.preset.is_empty(), crate::i18n::tr("None")).clicked() { d.preset.clear(); } - for p in &app.session.presets { - if ui - .selectable_label( - d.preset == p.id, - format!("{} — {}", crate::i18n::builtin_label(&p.group, p.builtin), crate::i18n::builtin_label(&p.name, p.builtin)), - ) - .clicked() - { - d.preset = p.id.clone(); - } + let selected = (!d.preset.is_empty()).then_some(d.preset.as_str()); + if let Some(pid) = crate::panels::presets::picker_rows(ui, "import-preset", &app.session.presets, selected) { + d.preset = pid; } }); }); diff --git a/crates/ui-egui/src/panels/presets.rs b/crates/ui-egui/src/panels/presets.rs index 0968ab67d..fb253aa15 100644 --- a/crates/ui-egui/src/panels/presets.rs +++ b/crates/ui-egui/src/panels/presets.rs @@ -4,9 +4,11 @@ //! files. use std::collections::BTreeMap; +use std::sync::Arc; use egui::{Align2, Color32, Rect, Sense, pos2, vec2}; use lightcraft_develop::{ControlSpec, Preset, Section, Track}; +use lightcraft_engine::presets::PresetList; use serde_json::json; use crate::icons::{Icon, paint}; @@ -73,16 +75,14 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let current = active.and_then(|id| app.session.develop_of(id)).map(|d| (*d).clone()); let thumbs = app.ui.preset_thumbs && active.is_some(); let row_h = if thumbs { 54.0 } else { 26.0 }; + let groups = grouped(ui.ctx(), &app.session.presets); let scroll = egui::ScrollArea::vertical().id_salt("presets-scroll").auto_shrink([false, false]).show(ui, |ui| { ui.spacing_mut().item_spacing.y = 0.0; - let mut groups: BTreeMap> = BTreeMap::new(); - for p in &app.session.presets { - groups.entry(p.group.clone()).or_default().push(p.clone()); - } - for (g, items) in groups { - // A group holding stock presets is a stock group (shown in the UI language), even - // when user presets share its name; a group of user presets keeps its name. - let builtin = items.iter().any(|p| p.builtin); + // Only the rows in view are laid out, drawn and given thumbnails: a Lightroom collection + // imported whole is 10 000+ presets, and building every row each frame took seconds. + let visible = ui.clip_rect().expand(row_h); + for grp in groups.iter() { + let (g, builtin) = (grp.name.clone(), grp.builtin); let open_id = egui::Id::new(("preset-group", &g)); let open: bool = ui.data(|d| d.get_temp(open_id)).unwrap_or(true); let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 32.0), Sense::click()); @@ -111,7 +111,12 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { if !open { continue; } - for pr in items { + let (first, end) = rows_in_view(ui.cursor().top(), row_h, grp.items.len(), visible); + if first > 0 { + ui.allocate_space(vec2(ui.available_width(), first as f32 * row_h)); + } + for &i in grp.items.get(first..end).unwrap_or_default() { + let Some(pr) = app.session.presets.get(i).cloned() else { continue }; let (pid, name, fav) = (pr.id.clone(), crate::i18n::builtin_label(&pr.name, pr.builtin).to_string(), pr.favorite); let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), row_h), Sense::click()); register(ui.ctx(), format!("preset:{pid}"), r); @@ -175,10 +180,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { crate::panels::dialogs::prompt(app, "Rename Preset", "Preset name", &name, "preset.rename", json!({"id": pid}), "name"); } ui.menu_button(crate::i18n::tr("Move to Group"), |ui| { - let mut groups: Vec = app.session.presets.iter().filter(|p| !p.builtin).map(|p| p.group.clone()).collect(); - groups.sort(); - groups.dedup(); - for g in groups.iter().filter(|g| **g != group) { + for g in groups.iter().filter(|x| x.user && x.name != group).map(|x| &x.name) { if ui.button(g).clicked() { let _ = app.run("preset.move", json!({"id": pid, "group": g})); } @@ -201,6 +203,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } }); } + if end < grp.items.len() { + ui.allocate_space(vec2(ui.available_width(), (grp.items.len() - end) as f32 * row_h)); + } } ui.add_space(30.0); }); @@ -221,6 +226,107 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { }); } +/// A preset group as the panel lists it: indices into the session's presets. +struct Group { + name: String, + /// Holds a stock preset: a stock group (shown in the UI language), even when user presets + /// share its name; a group of user presets keeps its name. + builtin: bool, + /// Holds a user preset (a Move to Group target). + user: bool, + items: Vec, +} + +/// The presets by group (groups sorted by name, presets in list order), rebuilt only when the +/// list changed (its generation). +fn grouped(ctx: &egui::Context, presets: &PresetList) -> Arc> { + let id = egui::Id::new("presets-grouped"); + if let Some((generation, groups)) = ctx.data(|d| d.get_temp::<(u64, Arc>)>(id)) + && generation == presets.generation() + { + return groups; + } + let mut by_name: BTreeMap<&str, Group> = BTreeMap::new(); + for (i, p) in presets.iter().enumerate() { + let g = by_name.entry(&p.group).or_insert_with(|| Group { name: p.group.clone(), builtin: false, user: false, items: Vec::new() }); + g.builtin |= p.builtin; + g.user |= !p.builtin; + g.items.push(i); + } + let groups = Arc::new(by_name.into_values().collect::>()); + ctx.data_mut(|d| d.insert_temp(id, (presets.generation(), groups.clone()))); + groups +} + +/// The presets as the rows of a picker (a combo box's list), in list order with a header row +/// wherever the group changes. Only the rows in view are built (10 000+ presets made an open +/// picker take seconds a frame). Returns the id of the preset clicked. +pub fn picker_rows(ui: &mut egui::Ui, salt: &str, presets: &PresetList, selected: Option<&str>) -> Option { + let id = egui::Id::new("preset-picker-rows"); + let rows = match ui.ctx().data(|d| d.get_temp::<(u64, Arc>)>(id)) { + Some((generation, rows)) if generation == presets.generation() => rows, + _ => { + let mut rows = Vec::with_capacity(presets.len() + 16); + let mut group: Option<&str> = None; + for (i, p) in presets.iter().enumerate() { + if group != Some(p.group.as_str()) { + group = Some(&p.group); + rows.push(PickRow::Header(crate::i18n::builtin_label(&p.group, p.builtin).to_string())); + } + rows.push(PickRow::Preset(i)); + } + let rows = Arc::new(rows); + ui.ctx().data_mut(|d| d.insert_temp(id, (presets.generation(), rows.clone()))); + rows + } + }; + let row_h = ui.spacing().interact_size.y; + let mut out = None; + egui::ScrollArea::vertical().id_salt(("preset-picker", salt)).max_height(300.0).auto_shrink([false, true]).show_rows( + ui, + row_h, + rows.len(), + |ui, range| { + for row in rows.get(range).unwrap_or_default() { + let size = vec2(ui.available_width(), row_h); + match row { + PickRow::Header(g) => { + ui.add_sized(size, egui::Label::new(egui::RichText::new(g).size(10.5).weak())); + } + PickRow::Preset(i) => { + let Some(p) = presets.get(*i) else { continue }; + let label = crate::i18n::builtin_label(&p.name, p.builtin); + if ui.add_sized(size, egui::Button::selectable(selected == Some(p.id.as_str()), label)).clicked() { + out = Some(p.id.clone()); + } + } + } + } + }, + ); + out +} + +#[derive(Clone)] +enum PickRow { + /// A group's name (in the UI language for stock groups). + Header(String), + /// An index into the session's presets. + Preset(usize), +} + +/// The rows `first..end` of a block of `n` rows `row_h` high starting at `top` that meet +/// `visible`. +fn rows_in_view(top: f32, row_h: f32, n: usize, visible: Rect) -> (usize, usize) { + if row_h <= 0.0 || !top.is_finite() { + return (0, n); + } + // `as usize` saturates (negative and NaN give 0) + let first = (((visible.top() - top) / row_h).floor() as usize).min(n); + let end = (((visible.bottom() - top) / row_h).ceil() as usize).clamp(first, n); + (first, end) +} + /// The part of a `size` image that covers `r` (centre crop), as texture coordinates. pub fn cover_uv(r: Rect, size: [usize; 2]) -> Rect { let a = size[0] as f32 / size[1].max(1) as f32; diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 3c7261e20..7970afaa8 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -203,15 +203,8 @@ fn preset_combo(app: &LightcraftApp, ui: &mut egui::Ui, id: &str, current: Optio if ui.selectable_label(current.is_none(), crate::i18n::tr(none_label)).clicked() { out = Some(None); } - let mut group = ""; - for p in &app.session.presets { - if p.group != group { - group = &p.group; - ui.label(RichText::new(crate::i18n::builtin_label(group, p.builtin)).size(10.5).weak()); - } - if ui.selectable_label(current == Some(p.id.as_str()), crate::i18n::builtin_label(&p.name, p.builtin)).clicked() { - out = Some(Some(p.id.clone())); - } + if let Some(pid) = crate::panels::presets::picker_rows(ui, id, &app.session.presets, current) { + out = Some(Some(pid)); } }); register(ui.ctx(), format!("combo:{id}"), r.response.rect); @@ -283,10 +276,8 @@ fn import_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { if ui.selectable_label(current.is_none(), crate::i18n::tr("LightCraft Default")).clicked() { pick = Some(json!({"camera": cam, "preset": null})); } - for p in &app.session.presets { - if ui.selectable_label(current == Some(p.id.as_str()), crate::i18n::builtin_label(&p.name, p.builtin)).clicked() { - pick = Some(json!({"camera": cam, "preset": p.id})); - } + if let Some(pid) = crate::panels::presets::picker_rows(ui, &id, &app.session.presets, current) { + pick = Some(json!({"camera": cam, "preset": pid})); } }); register(ui.ctx(), format!("combo:{id}"), r.response.rect); diff --git a/docs/whats-new.md b/docs/whats-new.md index fb8b285a3..df4896242 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -46,6 +46,14 @@ in, instead of moving it back to Program Files (issue #399). If that folder has been deleted since, the upgrade installs to Program Files. +### Large preset collections +- A library holding thousands of presets (a Lightroom collection imported whole: 11 000 presets, a 42 MB + presets.json) stays responsive. Every command used to re-serialize the whole list to see whether it had changed + (~0.6 s per slider tick); now the list is saved only after it changes, and a slider tick takes ~2 ms. +- The Presets panel builds only the rows in view (and their thumbnails) instead of every preset on every frame; the + preset pickers in Import and Settings ▸ Import scroll the same way. +- Importing a large collection and opening its library no longer compare every preset with every other. + ## 2026-10-10 ### Dialogs fit the window From fdccb413cd8887fce4e848ea396c50bdbcab11ed Mon Sep 17 00:00:00 2001 From: Nigel Ihza Firdausy <106657905+Ni-zav@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:15:34 +0700 Subject: [PATCH 04/14] fix(import): avoid scanning symlink aliases into the library --- crates/engine/src/walk.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/walk.rs b/crates/engine/src/walk.rs index 479c877cf..4c35b66bb 100644 --- a/crates/engine/src/walk.rs +++ b/crates/engine/src/walk.rs @@ -57,6 +57,8 @@ pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl if let Ok(c) = std::fs::canonicalize(root) { linked.insert(c); } + // Include aliases to the library in the exclusion, not just its lexical path. + let skip_canonical = skip.and_then(|p| std::fs::canonicalize(p).ok()); // (path, depth of the folder it is in, is a folder); popped in path order let mut stack: Vec<(PathBuf, usize, bool)> = vec![(root.to_path_buf(), 0, true)]; while let Some((path, depth, is_dir)) = stack.pop() { @@ -66,7 +68,9 @@ pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl } continue; } - if skip.is_some_and(|s| path == s) { + if skip.is_some_and(|s| path == s) + || skip_canonical.as_ref().is_some_and(|s| std::fs::canonicalize(&path).is_ok_and(|p| p.starts_with(s))) + { continue; } if depth >= limits.max_depth { @@ -165,6 +169,31 @@ mod tests { let _ = std::fs::remove_dir_all(&d); } + /// Folder imports must not revisit the user's own library through symlinks + /// or through an alias to the library's parent. + #[cfg(unix)] + #[test] + fn skipped_library_stays_skipped_through_symlinks_and_ancestors() { + let d = tmp("skip-alias"); + let root = d.join("incoming"); + let library = d.join("library"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::create_dir_all(library.join("Originals")).unwrap(); + std::fs::write(root.join("ordinary.jpg"), b"photo").unwrap(); + std::fs::write(library.join("Originals").join("library-only.jpg"), b"original").unwrap(); + std::os::unix::fs::symlink(&library, root.join("library-link")).unwrap(); + std::os::unix::fs::symlink(&d, root.join("parent-link")).unwrap(); + + let photos = files_in(&root, Some(&library), Limits::default(), |p| p.extension().is_some_and(|e| e == "jpg")); + assert!(photos.files.contains(&root.join("ordinary.jpg"))); + assert!(!photos.files.iter().any(|p| p.ends_with("library-only.jpg")), "{photos:?}"); + + // Scanning a subfolder already inside the excluded library is also forbidden. + let inside = files_in(&library.join("Originals"), Some(&library), Limits::default(), |_| true); + assert!(inside.files.is_empty(), "{inside:?}"); + let _ = std::fs::remove_dir_all(&d); + } + /// A link back up the tree (a loop) is followed at most once: the walk ends. #[cfg(unix)] #[test] From 465601b76c50e3775f69c1ca7c62b013a96536e4 Mon Sep 17 00:00:00 2001 From: s17sh70x2jfl031ag <287216888+s17sh70x2jfl031ag@users.noreply.github.com> Date: Sun, 11 Oct 2026 12:46:22 -0700 Subject: [PATCH 05/14] HEIF: move the heic-rs pin from 0.1.1 to 0.1.4 heic-rs 0.1.2 to 0.1.4 fix six panics on malformed files, found by fuzzing: four in release builds (a uuid box too small for its extended type, a block starting right of the picture, a conformance window larger than the picture, and a reference row below the picture) and two with overflow checks on (a PCM bit depth above the sample bit depth, a slice QP out of range). The API, MSRV and output are unchanged. Mirrors the same change in PhotoCraft's photocraft-heif. --- Cargo.lock | 4 ++-- crates/heif/Cargo.toml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f2609a5b..6fff653bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2548,9 +2548,9 @@ checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" [[package]] name = "heic-rs" -version = "0.1.1" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "adf9dde634bb6d5b718bacd01bef80192f2334306b2f785519a0136cca7b9618" +checksum = "8883c3bfa6b4ff19a40e52a6dc051c5612cbe21bf408fc6d4875e8dfd777cb66" dependencies = [ "rayon", ] diff --git a/crates/heif/Cargo.toml b/crates/heif/Cargo.toml index b3bc69d62..95641fb2f 100644 --- a/crates/heif/Cargo.toml +++ b/crates/heif/Cargo.toml @@ -11,11 +11,11 @@ license.workspace = true # Mirrors PhotoCraft's photocraft-heif (same team, same decision): heic-rs was chosen over the # other permissive candidates (heif-oxide's HEVC decoder, rust_h265, follows FFmpeg's (LGPL) # code structure; gamut-heic has no HEVC decoder of its own). -heic-rs = { version = "=0.1.1", default-features = false, features = ["std"] } +heic-rs = { version = "=0.1.4", default-features = false, features = ["std"] } # The tiles of a HEIC photo decode in parallel on native; wasm stays single-threaded. [target.'cfg(not(target_arch = "wasm32"))'.dependencies] -heic-rs = { version = "=0.1.1", default-features = false, features = ["std", "parallel"] } +heic-rs = { version = "=0.1.4", default-features = false, features = ["std", "parallel"] } rayon = { workspace = true } [features] From fa7ee6ee9d7af875b0aff73c38827390d24af2f4 Mon Sep 17 00:00:00 2001 From: Nigel Ihza Firdausy <106657905+Ni-zav@users.noreply.github.com> Date: Mon, 12 Oct 2026 09:18:16 +0700 Subject: [PATCH 06/14] UI: explain Nikon HE NEF codec limitation and conversion options (#1172) --- crates/ui-egui/src/widgets.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 0872b2ec4..bbdadfab5 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -45,6 +45,11 @@ pub fn preview_only_variant(reason: &str) -> &str { /// What a preview-only raw means for the user (see `Photo::preview_only`). pub fn preview_only_explanation(reason: &str) -> String { + if reason.starts_with("Nikon High Efficiency NEF (") { + return crate::i18n::tr( + "Nikon High Efficiency NEF uses licensed TicoRAW compression, which LightCraft cannot decode. Only the camera's embedded JPEG preview is available. For editable raw data, shoot Lossless compressed NEF or convert existing HE files to DNG using a licensed converter.", + ); + } crate::i18n::tr_format!( "LightCraft can't decode this raw variant yet ({}). You're editing the camera's embedded JPEG preview, which already includes the camera's picture style (e.g. Monochrome) and white balance.", preview_only_variant(reason) @@ -594,6 +599,17 @@ pub fn dropdown(ui: &mut Ui, id: &str, text: &str, font: egui::FontId, color: Co mod tests { use super::*; + #[test] + fn high_efficiency_nef_explains_the_codec_limit_and_workarounds() { + let reason = "Nikon High Efficiency NEF (HE / HE★, a JPEG XS-based codec with no public specification; shoot RAW Lossless compressed to edit the raw data)"; + let explanation = preview_only_explanation(reason); + assert!(explanation.contains("TicoRAW"), "{explanation}"); + assert!(explanation.contains("Lossless compressed NEF"), "{explanation}"); + assert!(explanation.contains("DNG"), "{explanation}"); + assert!(!explanation.contains("yet"), "{explanation}"); + assert!(preview_only_explanation("unsupported CR3").contains("unsupported CR3")); + } + /// Issue #647: hovering a star previews the rating a click would set. #[test] fn hovered_star_previews_its_rating() { From a73382a5e219b49673997752fb02aee5b763c58a Mon Sep 17 00:00:00 2001 From: Nigel Ihza Firdausy <106657905+Ni-zav@users.noreply.github.com> Date: Mon, 12 Oct 2026 09:18:38 +0700 Subject: [PATCH 07/14] UI: return translated Nikon codec notice as owned text --- crates/ui-egui/src/widgets.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index bbdadfab5..0a4c6ca7a 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -48,7 +48,8 @@ pub fn preview_only_explanation(reason: &str) -> String { if reason.starts_with("Nikon High Efficiency NEF (") { return crate::i18n::tr( "Nikon High Efficiency NEF uses licensed TicoRAW compression, which LightCraft cannot decode. Only the camera's embedded JPEG preview is available. For editable raw data, shoot Lossless compressed NEF or convert existing HE files to DNG using a licensed converter.", - ); + ) + .to_string(); } crate::i18n::tr_format!( "LightCraft can't decode this raw variant yet ({}). You're editing the camera's embedded JPEG preview, which already includes the camera's picture style (e.g. Monochrome) and white balance.", From 90a4088f1051c118a226a8b11927cd694ec5b28f Mon Sep 17 00:00:00 2001 From: Nigel Ihza Firdausy <106657905+Ni-zav@users.noreply.github.com> Date: Mon, 12 Oct 2026 09:27:54 +0700 Subject: [PATCH 08/14] fix(ui): make Nikon HE NEF codec licensing limitation explicit (#1172) --- crates/ui-egui/src/widgets.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 0a4c6ca7a..411a1e374 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -47,7 +47,7 @@ pub fn preview_only_variant(reason: &str) -> &str { pub fn preview_only_explanation(reason: &str) -> String { if reason.starts_with("Nikon High Efficiency NEF (") { return crate::i18n::tr( - "Nikon High Efficiency NEF uses licensed TicoRAW compression, which LightCraft cannot decode. Only the camera's embedded JPEG preview is available. For editable raw data, shoot Lossless compressed NEF or convert existing HE files to DNG using a licensed converter.", + "Nikon High Efficiency NEF (HE / HE★) uses intoPIX TicoRAW compression. LightCraft does not currently have a license for this codec, so only the camera's embedded JPEG preview is available, not the raw sensor data. To edit raw data, shoot Lossless compressed NEF or convert existing HE files to DNG using a licensed converter.", ) .to_string(); } @@ -605,6 +605,8 @@ mod tests { let reason = "Nikon High Efficiency NEF (HE / HE★, a JPEG XS-based codec with no public specification; shoot RAW Lossless compressed to edit the raw data)"; let explanation = preview_only_explanation(reason); assert!(explanation.contains("TicoRAW"), "{explanation}"); + assert!(explanation.contains("does not currently have a license"), "{explanation}"); + assert!(!explanation.contains("cannot decode"), "{explanation}"); assert!(explanation.contains("Lossless compressed NEF"), "{explanation}"); assert!(explanation.contains("DNG"), "{explanation}"); assert!(!explanation.contains("yet"), "{explanation}"); From 6fd740ac2db0ce253d34510e73d29d4bb3e0d622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Mon, 12 Oct 2026 11:44:03 +0800 Subject: [PATCH 09/14] Address maintainer review: free shortcut, derived tooltip, fmt, locales - view.maskOverlayAll: Cmd+O clashed with library.open; rebind to Cmd+Shift+O - Masking panel tooltip now derives the key label from the binding (platform aware: Cmd vs Ctrl) instead of hardcoding Ctrl+O; base string localized - gpu/render.rs: rewrite the AllMasks alpha gather as a for-loop so it passes cargo fmt --check (max_width 150) - docs/parity-checklist.md: add LR-MASK-OVERLAYALL row - locales: add 'Show All Mask Overlays' and 'Show all overlays' to de, es, fr, ja, pt-br, ru, uk, zh-hans, zh-hant --- crates/gpu/src/render.rs | 14 +++++++++----- crates/ui-egui/locales/de.json | 2 ++ crates/ui-egui/locales/es.json | 2 ++ crates/ui-egui/locales/fr.json | 2 ++ crates/ui-egui/locales/ja.json | 2 ++ crates/ui-egui/locales/pt-br.json | 2 ++ crates/ui-egui/locales/ru.json | 2 ++ crates/ui-egui/locales/uk.json | 2 ++ crates/ui-egui/locales/zh-hans.json | 2 ++ crates/ui-egui/locales/zh-hant.json | 2 ++ crates/ui-egui/src/menus.rs | 4 ++-- crates/ui-egui/src/panels/detail.rs | 2 +- crates/ui-egui/src/panels/masking.rs | 8 +++++++- crates/ui-egui/src/tests_masking.rs | 6 +++--- docs/parity-checklist.md | 1 + 15 files changed, 41 insertions(+), 12 deletions(-) diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 1ace48078..d0bd3aa45 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -690,14 +690,18 @@ pub fn render( }); // every visible mask's alpha, for `Overlay::AllMasks` (one tint over all masks at once) let all_overlay_alphas: Option> = if matches!(req.overlay, lightcraft_pipeline::Overlay::AllMasks { .. }) { - let list: Vec<_> = s.masks.iter().filter(|m| m.visible && !m.components.is_empty()).collect(); masks.as_ref().map(|all| { - list.iter().enumerate().map(|(mi, _)| { + let list: Vec<_> = s.masks.iter().filter(|m| m.visible && !m.components.is_empty()).collect(); + let mut alphas = Vec::with_capacity(list.len()); + for (mi, _) in list.iter().enumerate() { let b = cx.slice(all, mi * n, n); - cx.read_plane(&b, w, h) - }).collect() + alphas.push(cx.read_plane(&b, w, h)); + } + alphas }) - } else { None }; + } else { + None + }; let data: Vec<[u8; 4]> = cx.read(&out, n); let image = Rgba8 { width: w, height: h, data }; lap("finish + readback", &mut t, &mut cx); diff --git a/crates/ui-egui/locales/de.json b/crates/ui-egui/locales/de.json index 6520fb3e3..ca1013abe 100644 --- a/crates/ui-egui/locales/de.json +++ b/crates/ui-egui/locales/de.json @@ -1376,6 +1376,8 @@ "Show Luminance Map": "Luminanzkarte anzeigen", "Show Mask": "Maske anzeigen", "Show Mask Overlay": "Maskenüberlagerung anzeigen", + "Show All Mask Overlays": "Alle Maskenüberlagerungen anzeigen", + "Show all overlays": "Alle Überlagerungen anzeigen", "Show Mask Pins": "Maskenpins anzeigen", "Show Original": "Original anzeigen", "Show Photo Counts": "Fotoanzahl anzeigen", diff --git a/crates/ui-egui/locales/es.json b/crates/ui-egui/locales/es.json index fc53b6392..e379f2803 100644 --- a/crates/ui-egui/locales/es.json +++ b/crates/ui-egui/locales/es.json @@ -1323,6 +1323,8 @@ "Show Luminance Map": "Mostrar mapa de luminancia", "Show Mask": "Mostrar máscara", "Show Mask Overlay": "Mostrar superposición de máscara", + "Show All Mask Overlays": "Mostrar todas las superposiciones de máscara", + "Show all overlays": "Mostrar todas las superposiciones", "Show Mask Pins": "Mostrar chinchetas de máscara", "Show Original": "Mostrar original", "Show Photo Counts": "Mostrar recuento de fotos", diff --git a/crates/ui-egui/locales/fr.json b/crates/ui-egui/locales/fr.json index 53509a503..ce7a94733 100644 --- a/crates/ui-egui/locales/fr.json +++ b/crates/ui-egui/locales/fr.json @@ -1313,6 +1313,8 @@ "Show Luminance Map": "Afficher la carte de luminance", "Show Mask": "Afficher le masque", "Show Mask Overlay": "Afficher l'incrustation du masque", + "Show All Mask Overlays": "Afficher toutes les superpositions de masque", + "Show all overlays": "Afficher toutes les superpositions", "Show Mask Pins": "Afficher les épingles de masque", "Show Original": "Afficher l'original", "Show Photo Counts": "Afficher le nombre de photos", diff --git a/crates/ui-egui/locales/ja.json b/crates/ui-egui/locales/ja.json index 2babd30b6..b4243e701 100644 --- a/crates/ui-egui/locales/ja.json +++ b/crates/ui-egui/locales/ja.json @@ -1259,6 +1259,8 @@ "Show Luminance Map": "輝度マップを表示", "Show Mask": "マスクを表示", "Show Mask Overlay": "マスクの重ね表示", + "Show All Mask Overlays": "すべてのマスクオーバーレイを表示", + "Show all overlays": "すべてのオーバーレイを表示", "Show Mask Pins": "マスクのピンを表示", "Show Original": "元の写真を表示", "Show Photo Counts": "写真の枚数を表示", diff --git a/crates/ui-egui/locales/pt-br.json b/crates/ui-egui/locales/pt-br.json index dfe472818..63515b41f 100644 --- a/crates/ui-egui/locales/pt-br.json +++ b/crates/ui-egui/locales/pt-br.json @@ -1334,6 +1334,8 @@ "Show Luminance Map": "Mostrar mapa de luminância", "Show Mask": "Mostrar máscara", "Show Mask Overlay": "Mostrar sobreposição da máscara", + "Show All Mask Overlays": "Mostrar todas as sobreposições de máscara", + "Show all overlays": "Mostrar todas as sobreposições", "Show Mask Pins": "Mostrar pinos da máscara", "Show Original": "Mostrar original", "Show Photo Counts": "Mostrar contagem de fotos", diff --git a/crates/ui-egui/locales/ru.json b/crates/ui-egui/locales/ru.json index fafb65a14..fc0c7edea 100644 --- a/crates/ui-egui/locales/ru.json +++ b/crates/ui-egui/locales/ru.json @@ -1102,6 +1102,8 @@ "Show Luminance Map": "Показать карту яркости", "Show Mask": "Показать маску", "Show Mask Overlay": "Показать наложение маски", + "Show All Mask Overlays": "Показать все наложения масок", + "Show all overlays": "Показать все наложения", "Show Mask Pins": "Показать булавки масок", "Show Original": "Показать оригинал", "Show Photo Counts": "Показывать количество фото", diff --git a/crates/ui-egui/locales/uk.json b/crates/ui-egui/locales/uk.json index 7ebad5b10..56954b6b8 100644 --- a/crates/ui-egui/locales/uk.json +++ b/crates/ui-egui/locales/uk.json @@ -1326,6 +1326,8 @@ "Show Hidden Local Locations": "Показати приховані локальні розташування", "Show Luminance Map": "Показати карту яскравості", "Show Mask Overlay": "Показати накладання маски", + "Show All Mask Overlays": "Показати всі накладення масок", + "Show all overlays": "Показати всі накладення", "Show Mask Pins": "Показати маркери масок", "Show Original": "Показати оригінал", "Show Photo Counts": "Показати кількість фото", diff --git a/crates/ui-egui/locales/zh-hans.json b/crates/ui-egui/locales/zh-hans.json index 8a0e58b8e..795dc70ae 100644 --- a/crates/ui-egui/locales/zh-hans.json +++ b/crates/ui-egui/locales/zh-hans.json @@ -1321,6 +1321,8 @@ "Show Luminance Map": "显示明亮度图", "Show Mask": "显示蒙版", "Show Mask Overlay": "显示蒙版叠加", + "Show All Mask Overlays": "显示所有蒙版叠加", + "Show all overlays": "显示所有叠加", "Show Mask Pins": "显示蒙版图钉", "Show Original": "显示原图", "Show Photo Counts": "显示照片数量", diff --git a/crates/ui-egui/locales/zh-hant.json b/crates/ui-egui/locales/zh-hant.json index f1cbec3a0..c4a8ab1fd 100644 --- a/crates/ui-egui/locales/zh-hant.json +++ b/crates/ui-egui/locales/zh-hant.json @@ -1321,6 +1321,8 @@ "Show Luminance Map": "顯示明度圖", "Show Mask": "顯示遮色片", "Show Mask Overlay": "顯示遮色片疊加", + "Show All Mask Overlays": "顯示所有蒙版疊加", + "Show all overlays": "顯示所有疊加", "Show Mask Pins": "顯示遮色片控制點", "Show Original": "顯示原始照片", "Show Photo Counts": "顯示照片數量", diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 5ae64e1a5..5f31025cb 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -91,8 +91,8 @@ pub const UI_COMMANDS: &[UiCommand] = &[ ("view.softProof", "Soft Proofing", Some("S"), "View"), ("view.histogram", "Histogram", Some("Cmd+Shift+H"), "View"), ("view.maskOverlay", "Show Mask Overlay", Some("O"), "View"), - // Ctrl+O / Cmd+O: every mask's region at once, not just the selected one - ("view.maskOverlayAll", "Show All Mask Overlays", Some("Cmd+O"), "View"), + // Ctrl+Shift+O / Cmd+Shift+O: every mask's region at once, not just the selected one + ("view.maskOverlayAll", "Show All Mask Overlays", Some("Cmd+Shift+O"), "View"), // Shift+O in the Masking panel (elsewhere it cycles the crop overlay) ("view.maskOverlayMode", "Cycle Mask Overlay Mode", None, "View"), ("view.maskOverlayColor", "Cycle Mask Overlay Color", None, "View"), diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 24976d94e..f766fe663 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1065,7 +1065,7 @@ pub(crate) fn view_overlay(app: &LightcraftApp, d: &DevelopSettings) -> lightcra if app.ui.fullscreen { return Overlay::None; } - // Show every visible mask's region at once (Ctrl+O / Cmd+O), ahead of the selected-mask overlay + // Show every visible mask's region at once (Ctrl+Shift+O / Cmd+Shift+O), ahead of the selected-mask overlay // so the shortcut is not shadowed by the always-on single-mask overlay. if app.ui.right == RightPanel::Masking && app.ui.mask_overlay_all diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 47b96ee6d..4698a1655 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -198,7 +198,13 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { app.ui.mask_overlay = !on; } let all = app.ui.mask_overlay_all; - if icon_button(ui, "maskOverlayAll", Icon::Stack, vec2(24.0, 24.0), all, true, "Show all overlays (Ctrl+O)").clicked() { + let all_mac = ui.ctx().os() == egui::os::OperatingSystem::Mac; + let all_base = crate::i18n::tr("Show all overlays"); + let all_tip = match crate::shortcuts::shortcut_of(&app.ui.settings.keymap, "view.maskOverlayAll") { + Some(sc) => format!("{all_base} ({})", crate::menubar::shortcut_text(sc, all_mac)), + None => all_base.to_string(), + }; + if icon_button(ui, "maskOverlayAll", Icon::Stack, vec2(24.0, 24.0), all, true, &all_tip).clicked() { app.ui.mask_overlay_all = !all; } }); diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index db009e816..3ad52b8e5 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -168,13 +168,13 @@ fn mask_adjustments_hide_overlay_without_disabling_the_mask() { assert_eq!(develop(&h).masks[0], mask, "toggling the overlay leaves the mask and its effect unchanged"); assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::Mask { .. })); - // Ctrl+O shows every visible mask's region at once (the new command), ahead of the selected + // Ctrl+Shift+O / Cmd+Shift+O shows every visible mask's region at once (the new command), ahead of the selected // mask overlay so it is not shadowed by the always-on single-mask overlay. exec(&mut h, "view.maskOverlayAll", json!({})); - assert!(h.app.ui.mask_overlay_all, "Ctrl+O turns on the all-masks overlay"); + assert!(h.app.ui.mask_overlay_all, "the all-masks overlay command turns it on"); assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::AllMasks { .. })); exec(&mut h, "view.maskOverlayAll", json!({})); - assert!(!h.app.ui.mask_overlay_all, "Ctrl+O toggles the all-masks overlay off"); + assert!(!h.app.ui.mask_overlay_all, "the all-masks overlay command toggles it off"); assert!(!matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::AllMasks { .. })); // Failed edits and global adjustments must not hide the mask overlay. diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 099411089..ec6991c4b 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -302,6 +302,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-MASK-FEATHER-EDGE | Refine mask edges | P2 | ✅ | `cmd:mask.refine`, `crates/pipeline/src/masks.rs` (`evaluate_one`) | Refine Edges slider per mask: guided filter on the photo's luminance snaps soft mask edges to the photo's edges (rendered on the CPU) | | LR-MASK-SLIDERS | Local adjustment sliders | P0 | ✅ | `cmd:mask.adjust`, `crates/pipeline/src/finish.rs` | every slider renders (CPU + GPU), incl. Noise, Moiré, Defringe (negative Defringe has no effect); no local curve or effect presets | | LR-MASK-OVERLAY | Mask overlay | P0 | ✅ | `cmd:view.maskOverlay`, `cmd:view.maskOverlayMode`, `cmd:view.maskOverlayColor`, `crates/pipeline/src/visualize.rs` | rendered alpha of the selected mask (CPU + GPU): colour, colour on B&W, image on black/white, white on black; ⇧O cycles while masking; successful local adjustments (including Amount) hide the overlay until manually shown again with O / the eye; shape/edge edits retain it; no auto-show on hover | +| LR-MASK-OVERLAYALL | Show all mask overlays | P1 | ✅ | `cmd:view.maskOverlayAll`, `crates/ui-egui/src/panels/masking.rs`, `crates/pipeline/src/visualize.rs` | one diagnostic tint over every visible mask at once (CPU + GPU): the masks still stack their adjustments as before — this only paints each mask's region together, so you can see all masked areas at a glance (⌘⇧O / Ctrl+Shift+O) | | LR-MASK-PINS | Pins | P1 | ✅ | `cmd:view.maskPins`, `crates/ui-egui/src/panels/detail.rs` | a pin per radial/linear/brush component: click selects its mask, drag moves it; no pins for range/AI components, no Auto mode | | LR-MASK-UPDATE | Recompute AI masks | P2 | ⬜ | | | | LR-MASK-SYNC | Copy masks to other photos | P1 | ✅ | `cmd:develop.copy` (`groups`) | | From 9fc2e69328cdbeafa32f3eb366fbc312088e97d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Mon, 12 Oct 2026 13:18:21 +0800 Subject: [PATCH 10/14] gpu: read all mask overlay alphas in a single GPU->CPU readback For Overlay::AllMasks, the masks buffer already holds each visible mask's alpha back-to-back in the same order the single-mask overlay uses (cx.slice(all, mi*n, n)). Read the whole buffer once with cx.read instead of one synchronous read_plane per mask, then split it into per-mask Planes on the CPU. Same bytes, but the pipeline stalls once instead of N times. --- crates/gpu/src/render.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 12c7ec75e..828d2ba5a 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -712,14 +712,21 @@ pub fn render( _ => Err(m), } }); - // every visible mask's alpha, for `Overlay::AllMasks` (one tint over all masks at once) + // every visible mask's alpha, for `Overlay::AllMasks` (one tint over all masks at once). + // The masks buffer holds each visible mask's alpha back-to-back in the same `list` order the + // single-mask overlay uses, so read the whole buffer back in ONE GPU->CPU transfer and split it + // on the CPU, instead of one synchronous readback per mask (which stalls the pipeline N times). let all_overlay_alphas: Option> = if matches!(req.overlay, lightcraft_pipeline::Overlay::AllMasks { .. }) { masks.as_ref().map(|all| { let list: Vec<_> = s.masks.iter().filter(|m| m.visible && !m.components.is_empty()).collect(); + if list.is_empty() { + return Vec::new(); + } + let raw: Vec = cx.read(all, n * list.len()); let mut alphas = Vec::with_capacity(list.len()); - for (mi, _) in list.iter().enumerate() { - let b = cx.slice(all, mi * n, n); - alphas.push(cx.read_plane(&b, w, h)); + for mi in 0..list.len() { + let s = mi * n; + alphas.push(Plane { width: w, height: h, data: raw[s..s + n].to_vec() }); } alphas }) From 92a2e721e24aa560517e8f90acb6c8fdc564481b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Mon, 12 Oct 2026 13:41:14 +0800 Subject: [PATCH 11/14] ui: fix stale Ctrl+O / Cmd+O in mask_overlay_all doc comment The shortcut moved to Ctrl+Shift+O / Cmd+Shift+O; this doc comment still advertised the old binding. --- crates/ui-egui/src/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index b46542c1f..897ef8052 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -542,7 +542,7 @@ pub struct UiState { pub mask_overlay_color: [u8; 3], pub mask_overlay_opacity: f32, pub mask_pins: bool, - /// Masking: show every visible mask's region as one overlay at once (Ctrl+O / Cmd+O), + /// Masking: show every visible mask's region as one overlay at once (Ctrl+Shift+O / Cmd+Shift+O), /// instead of just the selected mask's ([`mask_overlay`]). pub mask_overlay_all: bool, /// The overlay (on, mode) to go back to when Show Luminance Map is turned off. From 40b7b9b385259cc7cec5f45af8d84c27ac9002e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Mon, 12 Oct 2026 14:04:19 +0800 Subject: [PATCH 12/14] fmt: satisfy rustfmt on AllMasks test and detail panel --- crates/pipeline/src/visualize.rs | 14 ++++++++++++-- crates/ui-egui/src/panels/detail.rs | 5 +---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/crates/pipeline/src/visualize.rs b/crates/pipeline/src/visualize.rs index dd87ce332..15c164bb1 100644 --- a/crates/pipeline/src/visualize.rs +++ b/crates/pipeline/src/visualize.rs @@ -413,8 +413,18 @@ mod tests { let left = MaskShape::Radial { center: Point::new(0.25, 0.5), rx: 0.15, ry: 0.15, angle: 0.0, feather: 10.0, invert: false }; let right = MaskShape::Radial { center: Point::new(0.75, 0.5), rx: 0.15, ry: 0.15, angle: 0.0, feather: 10.0, invert: false }; let mut s = DevelopSettings::default(); - s.masks.push(Mask { id: 1, visible: true, components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape: left }], ..Default::default() }); - s.masks.push(Mask { id: 2, visible: true, components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape: right }], ..Default::default() }); + s.masks.push(Mask { + id: 1, + visible: true, + components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape: left }], + ..Default::default() + }); + s.masks.push(Mask { + id: 2, + visible: true, + components: vec![MaskComponent { name: None, op: MaskOp::Add, invert: false, shape: right }], + ..Default::default() + }); let plain = render(&src, &info, &s, &RenderRequest::fit(80, 40)).image; let o = Overlay::AllMasks { view: MaskView::WhiteOnBlack, color: [255, 0, 0], opacity: 100 }; let img = render(&src, &info, &s, &RenderRequest { overlay: o, ..RenderRequest::fit(80, 40) }).image; diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 4c27ed8a0..de77369eb 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1127,10 +1127,7 @@ pub(crate) fn view_overlay(app: &LightcraftApp, d: &DevelopSettings) -> lightcra } // Show every visible mask's region at once (Ctrl+Shift+O / Cmd+Shift+O), ahead of the selected-mask overlay // so the shortcut is not shadowed by the always-on single-mask overlay. - if app.ui.right == RightPanel::Masking - && app.ui.mask_overlay_all - && d.masks.iter().any(|m| m.visible && !m.components.is_empty()) - { + if app.ui.right == RightPanel::Masking && app.ui.mask_overlay_all && d.masks.iter().any(|m| m.visible && !m.components.is_empty()) { return Overlay::AllMasks { view: MaskView::parse(&app.ui.mask_overlay_mode).unwrap_or_default(), color: app.ui.mask_overlay_color, From a10bc04b8bc72e3cd0c0a19389ba414a17e6958f Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Sun, 11 Oct 2026 02:27:54 +0300 Subject: [PATCH 13/14] smart previews: store proxies at JPEG quality 97 and version the encoding (#709) The proxy's largest visible error against the original was the JPEG encoder's quantised 8x8 DCT DC - each block's own mean, luma and chroma - which shows as block steps across the frame and a small constant colour shift, and grows with saturation. Quality 92 -> 97 shrinks those quantisers to about a third (block -mean dE76 0.138 vs 0.202 on the fidelity test's smooth scene), at about twice the file size; what remains is the 8-bit sRGB quantisation both encoders share, which only a lossless proxy format can close. The header gains an `enc` field (encode version 2) and the folder's look marker becomes " ": a proxy or marker from before the encode version is rebuilt/rescanned once, so existing libraries pick the new quality up without any command. Old files still decode bit-identically (the scale mapping is untouched, decode ignores `enc`), and a new proxy read by an older build is left alone by its staleness check. The fidelity test pins the gain with a frozen copy of the old encoder and a block-mean dE76 metric (per-pixel dE is dominated by the shared 8-bit floor); the 0.8 bound is the geometric midpoint between the no-op ratio 1.000 and the 0.632 floor cap the 8-bit quantisation sets on this scene. --- crates/engine/src/smart.rs | 256 ++++++++++++++++++++++++++++-- crates/engine/src/tests_import.rs | 10 +- 2 files changed, 249 insertions(+), 17 deletions(-) diff --git a/crates/engine/src/smart.rs b/crates/engine/src/smart.rs index 3628f188c..46b18e0c9 100644 --- a/crates/engine/src/smart.rs +++ b/crates/engine/src/smart.rs @@ -55,6 +55,28 @@ const HEADER_MAX: u64 = 4096; /// wrote the proxy's pixels (colour matrix and profile are baked into them) and stored tone curve. const LOOK_FIELD: &str = "look_version"; +/// The JPEG quality a smart preview's pixels are stored at. The original 92 left the 8×8 DCT's +/// quantised DC — each block's own mean, luma and chroma — visible as block steps across the frame +/// and as a small constant colour shift; that is the largest part of what a proxy gets wrong +/// against the original, and the only part a parameter can reach. 97 shrinks those quantisers to +/// roughly 1/2.6 of their steps, which is what removes the steps, at about twice the file size +/// (~1 MB per preview at 2560 px). What is left is the 8-bit sRGB quantisation itself, which only +/// a lossless proxy format can close. (ICED: the measured change is printed by the fidelity test +/// below; drop this to 95 for ~1.4× the size and roughly half the gain.) +const JPEG_QUALITY: u8 = 97; + +/// The header field that records which version of this module's own encoding wrote a proxy's pixels +/// (the source mapping above and [`JPEG_QUALITY`]). Not to be confused with [`LOOK_FIELD`], which +/// tracks the camera-look fit that is baked into those pixels. +const ENC_FIELD: &str = "enc"; + +/// The value of [`ENC_FIELD`] this build writes. Bump it whenever [`encode`]'s mapping or JPEG +/// settings change, so proxies an older build wrote are rebuilt: a missing field reads as 0, which +/// makes every proxy from before this version stale. Version 2 is the first to record the field at +/// all (a version 1 proxy is stored at JPEG quality 92, which this build raises to [`JPEG_QUALITY`]). +/// Crate-visible so the look-marker test pins the marker format to it instead of to a literal. +pub(crate) const ENCODE_VERSION: u64 = 2; + /// Encode a source image (and the decoder's camera tone curve, if any) as a smart preview. pub fn encode(img: &Rgb32f, tone: Option<&CameraTone>) -> Result, String> { // scale so all but the brightest 0.05 % fit into 0..1 @@ -78,13 +100,19 @@ pub fn encode(img: &Rgb32f, tone: Option<&CameraTone>) -> Result, String let rgba = Rgba8 { width: img.width, height: img.height, data }; let jpg = lightcraft_codecs::encode_jpeg( &lightcraft_codecs::EncodeImage::rgba8(&rgba), - 92, + JPEG_QUALITY, lightcraft_codecs::ChromaSubsampling::S444, &Default::default(), ) .map_err(|e| e.to_string())?; let mut out = MAGIC.to_vec(); - let mut head = serde_json::json!({"w": img.width, "h": img.height, "scale": scale, LOOK_FIELD: crate::camera_preview::LOOK_VERSION}); + let mut head = serde_json::json!({ + "w": img.width, + "h": img.height, + "scale": scale, + LOOK_FIELD: crate::camera_preview::LOOK_VERSION, + ENC_FIELD: ENCODE_VERSION, + }); if let Some(t) = tone { head["tone"] = serde_json::to_value(t).map_err(|e| e.to_string())?; } @@ -265,10 +293,17 @@ fn header_look_version(head: &serde_json::Value) -> u64 { head.get(LOOK_FIELD).and_then(serde_json::Value::as_u64).unwrap_or(0) } -/// Whether the complete proxy at `path` was written by an older camera-look fit than this build's -/// (so its stored curve, and the colour baked into its pixels, are out of date). A proxy from a -/// newer build is left alone (rebuilding it here would only undo that build's work), as is one -/// that can't be read: those are [`is_valid`]'s business. +/// The encode version a header records: a missing, negative, fractional or otherwise malformed +/// value counts as 0 (older than any real version), like [`header_look_version`]. +fn header_encode_version(head: &serde_json::Value) -> u64 { + head.get(ENC_FIELD).and_then(serde_json::Value::as_u64).unwrap_or(0) +} + +/// Whether the complete proxy at `path` is out of date for this build: written by an older +/// camera-look fit than this build's (so its stored curve, and the colour baked into its pixels, are +/// out of date), or by an older encoding of its own pixels ([`ENCODE_VERSION`], i.e. a lower JPEG +/// quality). A proxy from a newer build is left alone (rebuilding it here would only undo that +/// build's work), as is one that can't be read: those are [`is_valid`]'s business. pub fn is_stale(path: &Path) -> bool { use std::io::Read; if !is_valid(path) { @@ -283,15 +318,18 @@ pub fn is_stale(path: &Path) -> bool { let Some(nl) = rest.iter().position(|b| *b == b'\n') else { return false }; rest.get(..nl) .and_then(|h| serde_json::from_slice::(h).ok()) - .is_some_and(|h| header_look_version(&h) < u64::from(crate::camera_preview::LOOK_VERSION)) + .is_some_and(|h| header_look_version(&h) < u64::from(crate::camera_preview::LOOK_VERSION) || header_encode_version(&h) < ENCODE_VERSION) } -/// The marker file in a smart previews folder: the `LOOK_VERSION` its proxies were last checked -/// at ([`look_checked`]). +/// The marker file in a smart previews folder: the camera-look fit version and encode version its +/// proxies were last checked at ([`look_checked`]). const LOOK_MARKER: &str = "look-version"; -/// Whether the smart previews folder `dir` has been checked for stale proxies at this build's -/// camera-look fit version (or a newer one). A missing or unreadable marker counts as not. +/// Whether the smart previews folder `dir` has been checked for out-of-date proxies at this build's +/// camera-look fit version and encode version (or newer ones). A missing or unreadable marker counts +/// as not, and so does one written before the encode version was recorded — it only says the look +/// was checked, so this build scans the folder once more and rebuilds the proxies [`is_stale`] +/// finds. pub fn look_checked(dir: &Path) -> bool { use std::io::Read; let Ok(f) = std::fs::File::open(dir.join(LOOK_MARKER)) else { return false }; @@ -299,14 +337,20 @@ pub fn look_checked(dir: &Path) -> bool { if f.take(64).read_to_string(&mut text).is_err() { return false; } - text.trim().parse::().is_ok_and(|v| v >= u64::from(crate::camera_preview::LOOK_VERSION)) + let mut versions = text.split_whitespace(); + let (Some(look), Some(enc)) = (versions.next(), versions.next()) else { return false }; + match (look.parse::(), enc.parse::()) { + (Ok(look), Ok(enc)) => look >= u64::from(crate::camera_preview::LOOK_VERSION) && enc >= ENCODE_VERSION, + _ => false, + } } -/// Record that the proxies in `dir` were checked at this build's camera-look fit version. +/// Record that the proxies in `dir` were checked at this build's camera-look fit version and encode +/// version. pub fn mark_look_checked(dir: &Path) -> Result<(), String> { let path = dir.join(LOOK_MARKER); - lightcraft_catalog::safe_file::write_atomic(&path, format!("{}\n", crate::camera_preview::LOOK_VERSION).as_bytes()) - .map_err(|e| format!("{}: {e}", path.display())) + let text = format!("{} {}\n", crate::camera_preview::LOOK_VERSION, ENCODE_VERSION); + lightcraft_catalog::safe_file::write_atomic(&path, text.as_bytes()).map_err(|e| format!("{}: {e}", path.display())) } /// Load the proxy at `path`. @@ -451,4 +495,186 @@ mod tests { assert!(mean < 0.02, "mean encoded error {mean}"); assert!(decode(b"nope").is_err()); } + + /// A smooth, deterministic stand-in for a photo: a bright, near-neutral "sky" crossing over into + /// a saturated colour field, every value inside 0..1 so the proxy's 0.05 % highlight clip never + /// enters what is measured. Built from a rendered demo scene so the dimensions and the flat + /// row-major layout are the real ones. + fn smooth_scene(w: usize, h: usize) -> Rgb32f { + let mut img = lightcraft_scenes::demo_library()[0].render(w, h); + let (w, h) = (img.width.max(1), img.height.max(1)); + for (i, px) in img.data.iter_mut().enumerate() { + let u = (i % w) as f32 / (w - 1).max(1) as f32; + let v = (i / w) as f32 / (h - 1).max(1) as f32; + let t = (v * v * (3.0 - 2.0 * v)).clamp(0.0, 1.0); // smoothstep: a fade, not a step + let sky = [0.10 + 0.70 * v, 0.14 + 0.72 * v, 0.30 + 0.62 * v]; + let field = [0.86 - 0.26 * u, 0.18 + 0.62 * u, 0.12 + 0.30 * (1.0 - u)]; + *px = std::array::from_fn(|k| (sky[k] * (1.0 - t) + field[k] * t).clamp(0.0, 1.0)); + } + img + } + + /// CIE L*a*b* (D65) of a scene-linear colour: the perceptual yardstick a proxy is held to. The + /// input is clamped into 0..1, the only range a proxy ever stores. + fn lab(c: [f32; 3]) -> [f64; 3] { + let (r, g, b) = (c[0].clamp(0.0, 1.0) as f64, c[1].clamp(0.0, 1.0) as f64, c[2].clamp(0.0, 1.0) as f64); + let (x, y, z) = ( + (0.4124 * r + 0.3576 * g + 0.1805 * b) / 0.95047, + 0.2126 * r + 0.7152 * g + 0.0722 * b, + (0.0193 * r + 0.1192 * g + 0.9505 * b) / 1.08883, + ); + let f = |t: f64| if t > 216.0 / 24389.0 { t.cbrt() } else { (841.0 / 108.0) * t + 4.0 / 29.0 }; + let (fx, fy, fz) = (f(x), f(y), f(z)); + [116.0 * fy - 16.0, 500.0 * (fx - fy), 200.0 * (fy - fz)] + } + + /// The mean CIE76 ΔE between two same-sized scene-linear images: monotone in CIEDE2000 for a + /// low-contrast, near-neutral difference like a proxy's, and enough to rank two encoders. + fn mean_delta_e(a: &Rgb32f, b: &Rgb32f) -> f64 { + let n = a.data.len().min(b.data.len()); + if n == 0 { + return 0.0; + } + (0..n) + .map(|i| { + let (p, q) = (lab(a.data[i]), lab(b.data[i])); + ((p[0] - q[0]).powi(2) + (p[1] - q[1]).powi(2) + (p[2] - q[2]).powi(2)).sqrt() + }) + .sum::() + / n as f64 + } + + /// A deliberately frozen copy of the pixel mapping and container this module used before the + /// quality raise: the 99.95 % highlight scale, the sRGB 8-bit mapping, JPEG quality 92, and the + /// version-1 header (width, height, scale and camera-look fit — no encode version). A copy on + /// purpose: it must not follow `encode` as that changes, so the fidelity test measures what the + /// quality raise bought instead of trusting a written-down number. + fn encode_at_q92(img: &Rgb32f) -> Vec { + let mut lum: Vec = img.data.iter().map(|c| c[0].max(c[1]).max(c[2])).filter(|v| v.is_finite()).collect(); + let scale = if lum.is_empty() { + 1.0 + } else { + let k = ((lum.len() as f64) * 0.9995) as usize; + let k = k.min(lum.len() - 1); + let (_, v, _) = lum.select_nth_unstable_by(k, f32::total_cmp); + v.max(1.0) + }; + let data: Vec<[u8; 4]> = img + .data + .iter() + .map(|c| { + let e = |v: f32| (linear_to_srgb((v / scale).clamp(0.0, 1.0)) * 255.0).round() as u8; + [e(c[0]), e(c[1]), e(c[2]), 255] + }) + .collect(); + let rgba = Rgba8 { width: img.width, height: img.height, data }; + let jpg = lightcraft_codecs::encode_jpeg( + &lightcraft_codecs::EncodeImage::rgba8(&rgba), + 92, + lightcraft_codecs::ChromaSubsampling::S444, + &Default::default(), + ) + .unwrap(); + // the version-1 container the decoder needs: magic, header, JPEG — and no `enc` field + let mut out = MAGIC.to_vec(); + let head = serde_json::json!({"w": img.width, "h": img.height, "scale": scale, LOOK_FIELD: crate::camera_preview::LOOK_VERSION}); + out.extend_from_slice(head.to_string().as_bytes()); + out.push(b'\n'); + out.extend_from_slice(&jpg); + out + } + + /// The mean CIE76 ΔE between two same-sized scene-linear images, each reduced first to one + /// sample per 8×8 block — the block's mean colour. JPEG stores a block's DC coefficient, its + /// mean, and rebuilds the whole block from it, so a proxy differs from the original by a + /// constant per block: the "8×8 block pattern" a preview shows. Averaging a block's 64 pixels + /// keeps that constant while shrinking the 8-bit sRGB rounding the two encoders share + /// eightfold, which is what a per-pixel ΔE drowns in. + fn mean_block_delta_e(a: &Rgb32f, b: &Rgb32f) -> f64 { + let w = a.width.min(b.width).max(1); + let h = a.height.min(b.height).max(1); + let (mut sum, mut n) = (0.0f64, 0usize); + for by in (0..h).step_by(8) { + for bx in (0..w).step_by(8) { + let (mut sa, mut sb, mut count) = ([0.0f64; 3], [0.0f64; 3], 0u32); + for y in by..(by + 8).min(h) { + for x in bx..(bx + 8).min(w) { + if let (Some(ca), Some(cb)) = (a.data.get(y * a.width + x), b.data.get(y * b.width + x)) { + for k in 0..3 { + sa[k] += f64::from(ca[k]); + sb[k] += f64::from(cb[k]); + } + count += 1; + } + } + } + let d = f64::from(count.max(1)); + let (pa, pb) = ( + lab([(sa[0] / d) as f32, (sa[1] / d) as f32, (sa[2] / d) as f32]), + lab([(sb[0] / d) as f32, (sb[1] / d) as f32, (sb[2] / d) as f32]), + ); + sum += ((pa[0] - pb[0]).powi(2) + (pa[1] - pb[1]).powi(2) + (pa[2] - pb[2]).powi(2)).sqrt(); + n += 1; + } + } + sum / n.max(1) as f64 + } + + /// for [`a_smooth_proxy_keeps_the_scene_closer_than_the_old_quality`] to pass. The shared 8-bit + /// sRGB quantization floor both encoders share caps the achievable ratio at 0.632 on this scene, + /// a no-op encoder scores 1.000, and this is their geometric midpoint: it fails the moment the + /// quality raise is reverted and never trips on the honest result (measured 0.683). + const PROXY_BLOCK_GAIN: f64 = 0.8; + + #[test] + fn a_smooth_proxy_keeps_the_scene_closer_than_the_old_quality() { + let img = smooth_scene(256, 160); + let ours = decode(&encode(&img, None).unwrap()).unwrap().0; + let before = decode(&encode_at_q92(&img)).unwrap().0; + let block = (mean_block_delta_e(&img, &ours), mean_block_delta_e(&img, &before)); + let pixel = (mean_delta_e(&img, &ours), mean_delta_e(&img, &before)); + println!( + "smooth-scene proxy round-trip: block-mean ΔE76 {:.3} at q{JPEG_QUALITY} vs {:.3} at q92 \ + (per-pixel {:.3} vs {:.3})", + block.0, block.1, pixel.0, pixel.1 + ); + assert!(block.1 > 0.0, "the old encoder must measure something to compare against"); + assert!( + block.0 <= PROXY_BLOCK_GAIN * block.1, + "q{JPEG_QUALITY} leaves a block-mean ΔE of {:.3}, not the ≤{:.3} the quality raise should buy over q92's {:.3} \ + (the 8-bit sRGB floor caps the achievable ratio at ~0.63 on this scene)", + block.0, + PROXY_BLOCK_GAIN * block.1, + block.1 + ); + } + + #[test] + fn a_proxy_from_an_older_encode_is_stale() { + let dir = temp("encode-version"); + let img = lightcraft_scenes::demo_library()[0].render(64, 40); + let bytes = encode(&img, None).unwrap(); + let path = dir.join("p.lcsp"); + std::fs::write(&path, &bytes).unwrap(); + assert!(!is_stale(&path), "this build's own proxy is up to date"); + // one written before the encode version was recorded is not + let nl = MAGIC.len() + bytes[MAGIC.len()..].iter().position(|b| *b == b'\n').unwrap(); + let head: serde_json::Value = serde_json::from_slice(&bytes[MAGIC.len()..nl]).unwrap(); + assert_eq!(head[ENC_FIELD].as_u64(), Some(ENCODE_VERSION)); + let mut older = head.as_object().cloned().unwrap_or_default(); + older.remove(ENC_FIELD); + let mut old = MAGIC.to_vec(); + old.extend_from_slice(serde_json::Value::Object(older).to_string().as_bytes()); + old.extend_from_slice(&bytes[nl..]); + std::fs::write(&path, &old).unwrap(); + assert!(is_valid(&path), "losing the field keeps the file readable"); + assert!(is_stale(&path), "an older encoding is rebuilt"); + // the marker records both versions, and one from before the encode version forces a scan + assert!(!look_checked(&dir)); + mark_look_checked(&dir).unwrap(); + assert!(look_checked(&dir)); + std::fs::write(dir.join(LOOK_MARKER), "99\n").unwrap(); + assert!(!look_checked(&dir), "a look-only marker is not enough to skip the rebuild"); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 185993494..5ea368d61 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -1404,14 +1404,20 @@ fn smart_preview_look_marker_is_read_safely() { let dir = temp_dir("smartlook-marker"); assert!(!crate::smart::look_checked(&dir), "no marker"); let cur = crate::camera_preview::LOOK_VERSION; + let enc = crate::smart::ENCODE_VERSION; for (text, checked) in [ (String::new(), false), ("banana".into(), false), ("-1".into(), false), (format!("{}", cur.saturating_sub(1)), false), ("9".repeat(200), false), - (format!("{cur}\n"), true), - (format!(" {}\n", cur + 1), true), + // Written before the encode version was recorded: only says the look was checked, so the + // folder is scanned once more and the old-encoding proxies are rebuilt. + (format!("{cur}\n"), false), + (format!("{cur} {}\n", enc.saturating_sub(1)), false), + (format!("{cur} banana\n"), false), + (format!("{cur} {enc}\n"), true), + (format!(" {} {}\n", cur + 1, enc + 1), true), ] { std::fs::write(dir.join("look-version"), &text).unwrap(); assert_eq!(crate::smart::look_checked(&dir), checked, "{text:?}"); From ae9b09d3c2ee116d8c31542530e9fa740bc685a8 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 13 Oct 2026 02:07:07 -0400 Subject: [PATCH 14/14] Batch 32: rustfmt after merges Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/walk.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/engine/src/walk.rs b/crates/engine/src/walk.rs index 4c35b66bb..1c1b53b4a 100644 --- a/crates/engine/src/walk.rs +++ b/crates/engine/src/walk.rs @@ -68,9 +68,7 @@ pub fn files_in(root: &Path, skip: Option<&Path>, limits: Limits, mut keep: impl } continue; } - if skip.is_some_and(|s| path == s) - || skip_canonical.as_ref().is_some_and(|s| std::fs::canonicalize(&path).is_ok_and(|p| p.starts_with(s))) - { + if skip.is_some_and(|s| path == s) || skip_canonical.as_ref().is_some_and(|s| std::fs::canonicalize(&path).is_ok_and(|p| p.starts_with(s))) { continue; } if depth >= limits.max_depth {