Merge PR #957

crates/ui-egui/src/panels/detail.rs

@@ -1195,16 +1195,7 @@ fn general_interaction(
         return;
     }
     if app.ui.tool == "colorRange" {
-        // click: sample the colour for the selected mask's colour range; ⇧-click adds (up to 5)
-        if resp.clicked()
-            && let Some(q) = resp.interact_pointer_pos()
-        {
-            let n = map.norm(q);
-            let add = ui.input(|i| i.modifiers.shift);
-            if let Err(e) = app.run("mask.sampleColor", json!({"x": n.x, "y": n.y, "add": add})) {
-                app.toast(ui.ctx(), e);
-            }
-        }
+        color_range_pick(app, ui, resp, map);
         return;
     }
     if app.ui.tool == "pointColor" {
@@ -1631,6 +1622,21 @@ fn radial_body_contains(shape: &MaskShape, at: Point, map: &CanvasMap) -> bool {
 /// The Masking tool on the photo: outlines and handles of the selected mask, a pin per mask
 /// component (click selects its mask, drag moves the component), and brush painting. The mask
 /// itself shows as a rendered overlay ([`view_overlay`]).
+/// Colour Range eyedropper: a click samples the colour under it for the selected mask's
+/// colour range, at normalized frame coordinates (crop, rotation and zoom aware);
+/// ⇧-click adds a sample (up to 5).
+fn color_range_pick(app: &mut LightcraftApp, ui: &egui::Ui, resp: &egui::Response, map: &CanvasMap) {
+    if resp.clicked()
+        && let Some(q) = resp.interact_pointer_pos()
+    {
+        let n = map.norm(q);
+        let add = ui.input(|i| i.modifiers.shift);
+        if let Err(e) = app.run("mask.sampleColor", json!({"x": n.x, "y": n.y, "add": add})) {
+            app.toast(ui.ctx(), e);
+        }
+    }
+}
+
 fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, d: &DevelopSettings) {
     let tint = {
         let [r, g, b] = app.ui.mask_overlay_color;
@@ -1723,6 +1729,15 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons
             }
         }
     }
+    // Colour Range "Pick": the Masking panel owns the loupe's clicks (general_interaction
+    // never runs while it is open), so the eyedropper is handled here too.
+    if app.ui.tool == "colorRange" {
+        if resp.hovered() {
+            ui.ctx().set_cursor_icon(egui::CursorIcon::Crosshair);
+        }
+        color_range_pick(app, ui, resp, map);
+        return;
+    }
     // Object tool: a click includes what's under it, ⌥-click leaves it out
     if app.ui.tool == "object" {
         if resp.hovered() {

crates/ui-egui/src/tests_masking.rs

@@ -1152,3 +1152,63 @@ fn ai_masks_without_the_model_offer_the_download() {
     assert!(develop(&h).masks.is_empty());
     assert!(t.elapsed() < SETTLE, "{:?}", t.elapsed());
 }
+
+fn color_samples(h: &Headless) -> serde_json::Value {
+    let d = develop(h);
+    let m = d.masks.last().expect("a mask");
+    let Some(MaskShape::ColorRange { samples, .. }) = m.components.iter().map(|c| &c.shape).find(|s| matches!(s, MaskShape::ColorRange { .. }))
+    else {
+        panic!("colour range")
+    };
+    serde_json::to_value(samples).expect("samples")
+}
+
+/// Same samples, up to sub-pixel rounding and the preview level sampled.
+#[track_caller]
+fn assert_close(a: &serde_json::Value, b: &serde_json::Value) {
+    let flat = |v: &serde_json::Value| -> Vec<f64> {
+        v.as_array().unwrap().iter().flat_map(|s| s.as_array().unwrap().iter().map(|x| x.as_f64().unwrap())).collect()
+    };
+    let (a, b) = (flat(a), flat(b));
+    assert_eq!(a.len(), b.len());
+    assert!(a.iter().zip(&b).all(|(x, y)| (x - y).abs() < 0.05), "{a:?} vs {b:?}");
+}
+
+/// Regression: the Colour Range "Pick" button armed the eyedropper, but with the Masking panel
+/// open the loupe's clicks went to the mask overlay, so no sample was ever taken.
+#[test]
+fn color_range_pick_button_samples_the_photo() {
+    let mut h = detail("panel.masking");
+    exec(&mut h, "mask.add", json!({"kind": "colorRange"}));
+    h.app.ui.tool.clear();
+    h.settle(SETTLE);
+    click(&mut h, "button:colorPick0");
+    assert_eq!(h.app.ui.tool, "colorRange", "Pick arms the eyedropper");
+    brush_at(&mut h, 0.25, 0.5);
+    let ui_sample = color_samples(&h);
+    assert_eq!(ui_sample.as_array().map(Vec::len), Some(1), "a click samples a colour");
+    // the sample is the colour at that normalized point
+    exec(&mut h, "mask.sampleColor", json!({"x": 0.25, "y": 0.5}));
+    assert_close(&color_samples(&h), &ui_sample);
+    // ⇧-click adds
+    let r = h.request("ui.pointer", json!({"shift": true, "events": [{"kind": "down", "x": 0.7, "y": 0.2}, {"kind": "up", "x": 0.7, "y": 0.2}]}), T);
+    assert_eq!(r["ok"], true, "{r}");
+    assert_eq!(color_samples(&h).as_array().map(Vec::len), Some(2), "⇧-click adds a sample");
+
+    // cropped and zoomed: the click samples the photo point under it
+    exec(&mut h, "crop.set", json!({"rect": [0.2, 0.1, 0.9, 0.8]}));
+    h.app.ui.zoom = crate::state::Zoom::Percent(200.0);
+    h.app.ui.pan = (0.5, 0.5);
+    h.settle(SETTLE);
+    brush_at(&mut h, 0.5, 0.5);
+    let ui_sample = color_samples(&h);
+    assert_eq!(ui_sample.as_array().map(Vec::len), Some(1));
+    // (the centre of the crop box, in normalized source coordinates)
+    let c = develop(&h).crop.geometry.rect;
+    exec(&mut h, "mask.sampleColor", json!({"x": (c.x0 + c.x1) / 2.0, "y": (c.y0 + c.y1) / 2.0}));
+    assert_close(&color_samples(&h), &ui_sample); // a cropped, zoomed pick maps to the same frame point
+    // Pick again ends picking
+    click(&mut h, "button:colorPick0");
+    assert!(h.app.ui.tool.is_empty());
+    h.settle(SETTLE);
+}