From a7382aacfb6b4fb675b0458717bc4409b78b632f Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Sun, 11 Oct 2026 22:20:03 +1100 Subject: [PATCH] Masking: Color Range "Pick" samples the photo With the Masking panel open the loupe routes clicks to mask_overlay, so the colorRange eyedropper armed by the Pick button (handled only in general_interaction) never ran and no sample was taken. mask_overlay now handles the eyedropper (crosshair cursor, click samples, shift-click adds) through a shared color_range_pick helper. Fixes #954 Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/ui-egui/src/panels/detail.rs | 35 ++++++++++++----- crates/ui-egui/src/tests_masking.rs | 60 +++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index c3678ae2..d997559a 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1148,16 +1148,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" { @@ -1546,6 +1537,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; @@ -1638,6 +1644,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() { diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index b95fa1ab..bda58d65 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -1126,3 +1126,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 { + 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); +}