Merge pull request #353 from storytold/claude/fix-316-section-eye

Fix #316: the section eye switches Light, Color and Detail off, and can be clicked

crates/develop/src/lib.rs

@@ -68,6 +68,32 @@ impl DevelopSettings {
         !self.disabled_sections.iter().any(|s| s == section)
     }
 
+    /// The settings as rendered: an Edit-panel section whose eye is off contributes nothing (issue
+    /// #316). Light (with its tone curve), Color (white balance, presence, the mixers, colour grading
+    /// and point colour) and Detail are reset to their defaults here; Effects, Optics, Geometry and
+    /// Calibration the pipeline switches off where it applies them. Borrowed when every section is on.
+    pub fn effective(&self) -> std::borrow::Cow<'_, DevelopSettings> {
+        let off = |section: &str| !self.section_enabled(section);
+        if !(off("light") || off("color") || off("detail")) {
+            return std::borrow::Cow::Borrowed(self);
+        }
+        let mut d = self.clone();
+        if off("light") {
+            d.reset_section(Section::Light);
+            d.reset_section(Section::Curve);
+        }
+        if off("color") {
+            for section in [Section::Color, Section::Mixer, Section::BwMix, Section::Grading, Section::PointColor] {
+                d.reset_section(section);
+            }
+            d.point_colors.clear();
+        }
+        if off("detail") {
+            d.reset_section(Section::Detail);
+        }
+        std::borrow::Cow::Owned(d)
+    }
+
     pub fn set_section_enabled(&mut self, section: &str, on: bool) {
         self.disabled_sections.retain(|s| s != section);
         if !on {

crates/gpu/src/lib.rs

@@ -294,6 +294,9 @@ pub fn stage_bytes(stages: &StageCache) -> usize {
 pub fn render(src: &Arc<Rgb32f>, info: &SourceInfo, s: &DevelopSettings, req: &RenderRequest, stages: Option<&StageCache>) -> Option<Rendered> {
     #[cfg(not(target_arch = "wasm32"))]
     {
+        // sections switched off with their eye render as if at their defaults (issue #316)
+        let effective = s.effective();
+        let s: &DevelopSettings = &effective;
         // the kernel writes 8-bit output: high-bit-depth exports (and soft proofs) render on the CPU
         if !enabled() || req.depth != lightcraft_pipeline::OutputDepth::U8 || req.proof.is_some() {
             return None;

crates/pipeline/src/lib.rs

@@ -365,6 +365,9 @@ enum Src<'a> {
 }
 
 fn render_impl(src: Src<'_>, info: &SourceInfo, s: &DevelopSettings, req: &RenderRequest, cache: Option<&StageCache>) -> Rendered {
+    // sections switched off with their eye render as if at their defaults (issue #316)
+    let effective = s.effective();
+    let s: &DevelopSettings = &effective;
     // `Instant::now()` panics on wasm32-unknown-unknown: only read the clock when profiling.
     let lap = |what: &str, t: &mut Option<std::time::Instant>| {
         if let Some(t) = t {

crates/pipeline/src/tests.rs

@@ -237,6 +237,35 @@ fn exposure_after_spatial_filters_equals_exposing_the_source() {
     assert!(d <= 2, "max difference {d}");
 }
 
+/// 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]
+fn sections_switched_off_render_as_defaults() {
+    let src = Rgb32f::from_fn(32, 24, |x, y| [0.05 + x as f32 / 40.0, 0.1 + y as f32 / 30.0, 0.3]);
+    let info = SourceInfo::default();
+    let req = RenderRequest::fit(32, 24);
+    let plain = render(&src, &info, &DevelopSettings::default(), &req).image;
+    let mut s = DevelopSettings::default();
+    s.light.exposure = 1.5;
+    s.light.contrast = 40.0;
+    s.color.saturation = -60.0;
+    s.color.vibrance = 30.0;
+    s.detail.sharpen_amount = 120.0;
+    let edited = render(&src, &info, &s, &req).image;
+    assert_ne!(edited.data, plain.data);
+    for section in ["light", "color", "detail"] {
+        s.set_section_enabled(section, false);
+    }
+    assert_eq!(render(&src, &info, &s, &req).image.data, plain.data, "every edited section off = the unedited photo");
+    // one section back on brings back only its own adjustments
+    s.set_section_enabled("light", true);
+    let mut light_only = DevelopSettings::default();
+    light_only.light.exposure = 1.5;
+    light_only.light.contrast = 40.0;
+    assert_eq!(render(&src, &info, &s, &req).image.data, render(&src, &info, &light_only, &req).image.data);
+    assert!(matches!(DevelopSettings::default().effective(), std::borrow::Cow::Borrowed(_)), "nothing is copied when every section is on");
+}
+
 #[test]
 fn calibration_shifts_colours_but_keeps_greys() {
     let info = SourceInfo { raw: true, ..Default::default() };

crates/ui-egui/src/headless.rs

@@ -1084,6 +1084,24 @@ mod tests {
         assert_eq!(h.app.session.undo.len(), undo0 + 1);
     }
 
+    /// The eye on a section header switches the section off and on again, one undo step each
+    /// (issue #316).
+    #[test]
+    fn section_eye_switches_a_section_off() {
+        let mut h = demo([1300.0, 900.0]);
+        let t = Duration::from_secs(10);
+        h.request("ui.set", json!({"view": "detail", "right": "edit"}), t);
+        h.settle(SETTLE);
+        let light_on = |h: &Headless| h.app.session.develop_of(h.app.session.active().unwrap()).unwrap().section_enabled("light");
+        for expected in [false, true] {
+            // the eye shows while the pointer is on the header
+            assert_eq!(h.request("ui.hoverWidget", json!({"id": "section:light"}), t)["ok"], true);
+            assert_eq!(h.request("ui.clickWidget", json!({"id": "sectionEye:light"}), t)["ok"], true);
+            assert_eq!(light_on(&h), expected);
+        }
+        assert!(h.app.ui.section_open("light"), "the click didn't fold the section");
+    }
+
     /// Return commits a tool panel back to Edit; elsewhere it does nothing.
     #[test]
     fn return_commits_the_crop_tool() {

crates/ui-egui/src/widgets.rs

@@ -329,15 +329,18 @@ pub fn section_header(ui: &mut Ui, id: &str, title: &str, open: bool, enabled: O
     resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::CollapsingHeader, true, open, title));
     register(ui.ctx(), format!("section:{id}"), r);
     let p = ui.painter();
-    if resp.hovered() {
+    if ui.rect_contains_pointer(r) {
         p.rect_filled(r, 0.0, t.chrome.gamma_multiply(1.06));
     }
     let chev = Rect::from_center_size(pos2(r.left() + 30.0, r.center().y), vec2(14.0, 14.0));
     paint(p, chev, if open { Icon::ChevronDown } else { Icon::ChevronRight }, t.text_label);
     p.text(pos2(r.left() + 44.0, r.center().y), Align2::LEFT_CENTER, title, t.semibold(14.0), t.text);
     let mut toggled = None;
+    // shown while the pointer is anywhere on the header: `resp.hovered()` turns false as soon as the
+    // pointer is over the eye itself (the eye is on top), which hid the eye before it could be
+    // clicked (issue #316)
     if let Some(on) = enabled
-        && (resp.hovered() || !on)
+        && (ui.rect_contains_pointer(r) || !on)
     {
         let eye = Rect::from_center_size(pos2(r.right() - 34.0, r.center().y), vec2(18.0, 18.0));
         let er = ui.interact(eye, ui.id().with(("eye", id)), Sense::click());

docs/parity.md

@@ -275,7 +275,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
 | LR-EDIT-GEOM-GRID | Grid while transforming | P2 | ✅ | `crates/ui-egui/src/panels/detail.rs` | a fine grid over the photo while a geometry slider is dragged |
 | LR-EDIT-LENSBLUR | Lens blur | P2 | ⬜ | | settings field reserved, not rendered |
 | LR-EDIT-CALIB | Calibration [Classic] | P1 | ✅ | `ctl:calibration.*` | shadows tint, red/green/blue primary hue and saturation; read/written in XMP |
-| LR-EDIT-SECTION-TOGGLE | Section on/off | P1 | ✅ | `cmd:develop.sectionEnabled` | |
+| LR-EDIT-SECTION-TOGGLE | Section on/off | P1 | ✅ | `cmd:develop.sectionEnabled`, `crates/develop/src/lib.rs` (`effective`) | the eye on a section header hides its adjustments: Light (with the curve), Color (white balance, presence, mixers, grading, point colour), Effects, Detail, Optics, Geometry, Calibration; masks stay on (issue #316 fixed Light, Color and Detail, which had no effect) |
 | LR-EDIT-RESET | Reset all / section / slider | P0 | ✅ | `cmd:develop.reset`, `cmd:develop.resetSection`, `cmd:develop.resetControl`, `crates/ui-egui/src/widgets.rs` (double-click) | no "reset to open" |
 | LR-EDIT-SHOWORIG | Show original | P0 | ✅ | `cmd:view.showOriginal` | |
 

docs/whats-new.md

@@ -54,6 +54,10 @@
 - Type an exact value into any slider (issue #322): click the number next to its name, type (`1.5`, `-20`, `5600`)
   and press Return; Esc keeps the old value.
 
+### Editing
+- The eye on the Light, Color and Detail section headers now hides their adjustments, as it already did for Effects,
+  Optics, Geometry and Calibration (issue #316).
+
 ### Reliability
 - On macOS, single-key shortcuts that appear in the menu bar now work: E, C, H, M, ⇧P, I, K, D, ratings 0–5,
   labels 6–9, P / U and the rest did nothing, because macOS only passes ⌘ / ⌃ combinations and function keys