From f86459db58d0cd04d458009c0d4f3e3a67e118d5 Mon Sep 17 00:00:00 2001 From: nevicia2010-hub <259836477+nevicia2010-hub@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:29:16 +0200 Subject: [PATCH] LR-MASK: fix range handle drags and color sampling settings --- crates/pipeline/src/lib.rs | 8 +++- crates/pipeline/src/tests.rs | 55 ++++++++++++++++++++++++++++ crates/ui-egui/src/panels/masking.rs | 13 ++++++- crates/ui-egui/src/tests_masking.rs | 25 +++++++++++++ docs/parity-checklist.md | 7 ++-- 5 files changed, 101 insertions(+), 7 deletions(-) diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index d94d3a2fc..aa8e5d910 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -618,7 +618,11 @@ fn render_impl(src: Src<'_>, info: &SourceInfo, s: &DevelopSettings, req: &Rende /// exposed scene colours, as [`MaskShape::ColorRange`](lightcraft_develop::MaskShape) compares /// them), averaged over 3 × 3 pixels of a render fitting `req`. `None` outside the image. pub fn color_range_sample(src: &Rgb32f, info: &SourceInfo, s: &DevelopSettings, req: &RenderRequest, p: lightcraft_geom::Point) -> Option<[f64; 3]> { - let plan = plan(src, info, s, req); + // Pick from the same settings as the render: disabled panels contribute nothing, and an + // SDR preview of an HDR edit includes the SDR rendition's exposure offset. + let effective = s.effective(); + let rendition = settings_for(&effective, req); + let plan = plan(src, info, &rendition, req); let mut img = plan.frame.sample(src, plan.w, plan.h); lin_cpu(&mut img, info, &plan); let o = plan.frame.norm_to_out(plan.w, plan.h).apply(p); @@ -626,7 +630,7 @@ pub fn color_range_sample(src: &Rgb32f, info: &SourceInfo, s: &DevelopSettings, if cx < 0 || cy < 0 || cx >= plan.w as i64 || cy >= plan.h as i64 { return None; } - let gain = (s.light.exposure as f32).exp2(); + let gain = (plan.settings.light.exposure as f32).exp2(); let mut acc = [0f64; 3]; let mut n = 0.0; for y in (cy - 1).max(0)..=(cy + 1).min(plan.h as i64 - 1) { diff --git a/crates/pipeline/src/tests.rs b/crates/pipeline/src/tests.rs index 2398feece..d407939d4 100644 --- a/crates/pipeline/src/tests.rs +++ b/crates/pipeline/src/tests.rs @@ -272,6 +272,61 @@ fn exposure_after_spatial_filters_equals_exposing_the_source() { assert!(d <= 2, "max difference {d}"); } +#[test] +fn color_range_sampling_respects_disabled_sections() { + use crate::{MaskView, Overlay, color_range_sample}; + let src = Rgb32f::from_fn(32, 24, |_, _| [0.15, 0.25, 0.08]); + let info = SourceInfo::default(); + let req = RenderRequest::fit(32, 24); + let point = lightcraft_geom::Point::new(0.5, 0.5); + let plain = color_range_sample(&src, &info, &DevelopSettings::default(), &req, point).unwrap(); + for section in ["light", "color"] { + let mut s = DevelopSettings::default(); + if section == "light" { + s.light.exposure = 3.0; + } else { + s.wb.mode = lightcraft_develop::WbMode::Custom; + s.wb.temp = 12000.0; + s.wb.tint = 100.0; + } + assert_ne!(color_range_sample(&src, &info, &s, &req, point).unwrap(), plain, "enabled {section}"); + s.set_section_enabled(section, false); + let sample = color_range_sample(&src, &info, &s, &req, point).unwrap(); + assert_eq!(sample, plain, "disabled {section} must not change the picked colour"); + s.masks.push(lightcraft_develop::Mask { + id: 1, + components: vec![lightcraft_develop::MaskComponent { + name: None, + op: lightcraft_develop::MaskOp::Add, + invert: false, + shape: lightcraft_develop::MaskShape::ColorRange { samples: vec![sample], refine: 0.0 }, + }], + ..Default::default() + }); + let overlay = RenderRequest { overlay: Overlay::Mask { id: 1, view: MaskView::WhiteOnBlack, color: [255, 0, 0], opacity: 100 }, ..req }; + let img = render(&src, &info, &s, &overlay).image; + assert_eq!(img.data[12 * 32 + 16], [255; 4], "the sampled colour is selected with {section} off"); + } +} + +#[test] +fn color_range_sampling_uses_the_requested_hdr_rendition() { + use crate::{OutputDepth, color_range_sample}; + let src = Rgb32f::from_fn(32, 24, |_, _| [0.15, 0.25, 0.08]); + let info = SourceInfo::default(); + let point = lightcraft_geom::Point::new(0.5, 0.5); + let mut s = DevelopSettings::default(); + s.hdr.enabled = true; + s.hdr.sdr_brightness = 100.0; + let req = RenderRequest::fit(32, 24); + let expected = color_range_sample(&src, &info, &s.sdr_rendition(), &req, point).unwrap(); + assert_eq!(color_range_sample(&src, &info, &s, &req, point).unwrap(), expected, "SDR preview brightness affects the picked colour"); + let hdr = RenderRequest { depth: OutputDepth::F32Hdr, ..req }; + let plain = color_range_sample(&src, &info, &DevelopSettings::default(), &hdr, point).unwrap(); + assert_eq!(color_range_sample(&src, &info, &s, &hdr, point).unwrap(), plain, "HDR does not use the SDR offset"); + assert_ne!(plain, expected); +} + /// A section whose eye is off renders as if it were at its defaults (issue #316): Light, Color /// and Detail, which the pipeline doesn't switch off where it applies them. #[test] diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 534a207ca..78ac49cd5 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -537,18 +537,27 @@ fn range_controls(app: &mut LightcraftApp, ui: &mut egui::Ui, comp: usize, shape p.circle_filled(pos2(x(v), rect.center().y), 5.0, egui::Color32::WHITE); p.circle_stroke(pos2(x(v), rect.center().y), 5.0, Stroke::new(1.0, egui::Color32::from_gray(40))); } - // drag the nearer handle; one undo step per drag + // Choose at the press position, then keep that handle for the entire drag. Testing + // proximity on every frame can move the opposite endpoint after a fast/crossing drag. + let handle_id = resp.id.with("luminance-handle"); + let nearer_low = |px: f32| (px - x(lo)).abs() <= (px - x(hi)).abs(); if resp.drag_started() { + if let Some(origin) = ui.input(|i| i.pointer.press_origin()) { + ui.data_mut(|d| d.insert_temp(handle_id, nearer_low(origin.x))); + } let _ = app.run("develop.beginInteraction", json!({"label": "Luminance Range"})); } if (resp.dragged() || resp.clicked()) && let Some(pos) = resp.interact_pointer_pos() { let v = (((pos.x - rect.left()) / w) as f64).clamp(0.0, 1.0); - let (nlo, nhi) = if (v - lo).abs() <= (v - hi).abs() { (v.min(hi - 0.01), hi) } else { (lo, v.max(lo + 0.01)) }; + let low = + if resp.dragged() { ui.data(|d| d.get_temp::(handle_id)).unwrap_or_else(|| nearer_low(pos.x)) } else { nearer_low(pos.x) }; + let (nlo, nhi) = if low { (v.min((hi - 0.01).max(0.0)), hi) } else { (lo, v.max((lo + 0.01).min(1.0))) }; update(app, MaskShape::LuminanceRange { lo: nlo, hi: nhi, lo_feather: *lo_feather, hi_feather: *hi_feather }); } if resp.drag_stopped() { + ui.data_mut(|d| d.remove::(handle_id)); let _ = app.run("develop.endInteraction", json!({})); } // Smoothness: both falloffs at once diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index bf808b532..9c279c73d 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -832,6 +832,31 @@ fn g_toggles_grids_and_shift_g_starts_guided_upright() { h.settle(SETTLE); } +#[test] +fn luminance_range_drag_keeps_the_grabbed_handle() { + let mut h = detail("panel.masking"); + exec(&mut h, "mask.add", json!({"kind": "luminanceRange", "lo": 0.2, "hi": 0.8})); + h.settle(SETTLE); + let original = develop(&h); + for (start, dx, steps) in [(0.2, 180.0, 10), (0.8, -180.0, 10), (0.2, 120.0, 1), (0.8, -120.0, 1)] { + let undo0 = h.app.session.undo.len(); + let r = h.request("ui.dragWidget", json!({"id": "lumRange:0", "fx": start, "fy": 0.5, "dx": dx, "steps": steps}), T); + assert_eq!(r["ok"], true, "{r}"); + let d = develop(&h); + let MaskShape::LuminanceRange { lo, hi, .. } = &d.masks[0].components[0].shape else { panic!() }; + if start < 0.5 { + assert_eq!(*hi, 0.8, "dragging low must leave high fixed, including past high"); + assert!(*lo > 0.6 && *lo < *hi, "{lo}..{hi}"); + } else { + assert_eq!(*lo, 0.2, "dragging high must leave low fixed, including past low"); + assert!(*hi < 0.4 && *hi > *lo, "{lo}..{hi}"); + } + assert_eq!(h.app.session.undo.len(), undo0 + 1); + exec(&mut h, "edit.undo", json!({})); + assert_eq!(develop(&h), original); + } +} + #[test] fn luminance_range_controls_and_map() { let mut h = detail("panel.masking"); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 76d91ba3e..01f9b58dd 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (mask range regressions: luminance drag handle identity and colour sampling with disabled sections / HDR SDR rendition; LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -295,8 +295,8 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-MASK-BRUSH | Brush mask | P0 | 🟡 | `cmd:tool.brush`, `cmd:mask.brushStroke` (`autoMask`), `crates/pipeline/src/masks.rs`, `crates/ui-egui/src/brush_live.rs` | size/feather/flow/density/erase; Create New Mask starts a separate brush in paint mode; the stroke shows while the button is held with the committed stroke's dab spacing, feather and flow, so fast strokes have no gaps (#517); coverage builds up by passes, not hold time (holding the brush still adds nothing, live or committed, #518); Auto Mask: dabs weighted by similarity to the colour under the dab centre, refined by a guided filter on luminance (CPU + GPU); no A/B brushes, no pressure | | LR-MASK-LINEAR | Linear gradient | P0 | ✅ | `cmd:tool.linear`, `cmd:mask.update` | | | LR-MASK-RADIAL | Radial gradient | P0 | ✅ | `cmd:tool.radial`, `cmd:mask.update`, `crates/ui-egui/src/panels/detail.rs` | drag the selected ellipse's body or centre pin to move; edge grips resize, outer grip rotates; rotated and compound radial components, hidden pins, outside hit rejection and one-step undo tested | -| LR-MASK-COLORRANGE | Colour range | P1 | ✅ | `cmd:mask.add` (`colorRange`), `cmd:mask.sampleColor`, `cmd:mask.update`, `crates/pipeline/src/lib.rs` (`color_range_sample`) | Color tile → click the photo to sample (⇧-click adds, up to 5; samples taken in the space the mask compares in), Pick button, Refine slider | -| LR-MASK-LUMRANGE | Luminance range | P1 | ✅ | `cmd:mask.add` (`luminanceRange`), `cmd:mask.update`, `crates/ui-egui/src/panels/masking.rs` (`range_controls`) | range bar with two handles (one undo step per drag), Smoothness, Show Luminance Map (B&W photo with the range tinted) | +| LR-MASK-COLORRANGE | Colour range | P1 | ✅ | `cmd:mask.add` (`colorRange`), `cmd:mask.sampleColor`, `cmd:mask.update`, `crates/pipeline/src/lib.rs` (`color_range_sample`) | Color tile → click the photo to sample (⇧-click adds, up to 5; samples taken in the space the mask compares in, respecting section eyes and the requested HDR/SDR rendition), Pick button, Refine slider | +| LR-MASK-LUMRANGE | Luminance range | P1 | ✅ | `cmd:mask.add` (`luminanceRange`), `cmd:mask.update`, `crates/ui-egui/src/panels/masking.rs` (`range_controls`) | range bar with two handles (the pressed endpoint stays selected through fast/crossing drags; one undo step per drag), Smoothness, Show Luminance Map (B&W photo with the range tinted) | | LR-MASK-DEPTHRANGE | Depth range | P2 | ⬜ | | shape exists, needs depth data | | LR-MASK-COMBINE | Add / subtract / intersect | P0 | ✅ | `cmd:mask.addComponent`, `cmd:tool.brush` | Add/Subtract > Brush creates a component with the chosen operation; Erase removes paint within that brush component | | LR-MASK-INVERT | Invert | P0 | ✅ | `cmd:mask.invert` | | @@ -789,6 +789,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-13 | patch | LR-MASK-LUMRANGE / LR-MASK-COLORRANGE: range drags keep the grabbed endpoint; colour samples use the render's enabled sections and HDR/SDR settings; regression coverage, status unchanged | | 2026-10-11 | minor | LR-MERGE-HDR: HDR and panorama merges of raws without colour matrices (CR2, NEF, ARW…) write the colour they open with as DNG tags (issue #965); status unchanged | | 2026-10-12 | minor | LR-META-XMP: Read Metadata from File no longer auto-writes the sidecar it has just read, which added `lc:settings` and hid another app's later `crs:` edits (#934); status unchanged (✅) | | 2026-10-12 | patch | LR-IMP-CAMERA-COVERAGE: CR2 white level from the active area, not the saturated masked rows above it (issue #1272); status unchanged (🟡) |