From ef6e62449f3fc7c191e2a6646d04988f02dfb0a4 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 05:38:10 +0200 Subject: [PATCH] LR-EDIT-COLOR-GRADING: a wheel drag holds the pointer and puts it back where it was pressed (#1899) --- crates/ui-egui/src/headless.rs | 6 +- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/panels/edit.rs | 105 +++++++++++++++++++++++- crates/ui-egui/src/tests_grading.rs | 122 ++++++++++++++++++++++++++++ docs/parity-checklist.md | 2 +- 5 files changed, 231 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 6178990b0..48ca62a2d 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -241,7 +241,8 @@ pub struct Headless { quit: bool, /// The simulated window is zoomed (`Maximized(true)` seen, reported back to the app like a real host). pub window_maximized: bool, - /// Window-management commands the app sent (`StartDrag`, `Maximized`, …), oldest first. + /// Window-management and pointer commands the app sent (`StartDrag`, `Maximized`, + /// `CursorPosition`, …), oldest first. pub window_commands: Vec, /// The pointer cursor the last frame asked for (what a real host would show). pub last_cursor: egui::CursorIcon, @@ -342,7 +343,8 @@ impl Headless { self.window_maximized = on; self.window_commands.push(c); } - ViewportCommand::StartDrag => self.window_commands.push(c), + // recorded, not carried out: the simulated pointer moves only when a test moves it + ViewportCommand::StartDrag | ViewportCommand::CursorPosition(_) | ViewportCommand::CursorGrab(_) => self.window_commands.push(c), _ => {} } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index bb290bd86..17df83b94 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1383,6 +1383,7 @@ impl LightcraftApp { /// Frame timings once layout is done (`t0`: when layout started). fn end_frame(&mut self, ctx: &egui::Context, t0: f64) { + panels::edit::wheel_pointer_frame_end(ctx); ctx.output(|o| { for c in &o.commands { if let egui::OutputCommand::CopyText(text) = c { diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index ded1c0bd0..13c449170 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -1535,7 +1535,8 @@ fn grading_wheel(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings // puck as it is at that moment. let grab_id = egui::Id::new(("wheelGrab", key)); let mods = ui.input(|i| WheelMods::from_modifiers(i.modifiers)); - let grab = match resp.interact_pointer_pos() { + let pointer = wheel_pointer(ui, &resp); + let grab = match pointer { Some(q) if resp.dragged() || resp.clicked() => { let held = ui.data(|m| m.get_temp::(grab_id)).filter(|g| g.mods == mods && !resp.drag_started()); Some(held.unwrap_or_else(|| { @@ -1574,7 +1575,7 @@ fn grading_wheel(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings let _ = app.run("develop.beginInteraction", json!({"label": crate::i18n::tr_format!("{label} Grading", label = label)})); } if (resp.dragged() || resp.clicked()) - && let Some(q) = resp.interact_pointer_pos() + && let Some(q) = pointer { let (hue, sat) = grab.map_or_else(|| wheel_pick(q - c, rad, None), |g| g.pick(q - c, rad)); let _ = app.run("develop.set", json!({"values": {format!("grading.{key}.hue"): hue.round(), format!("grading.{key}.sat"): sat.round()}})); @@ -1582,6 +1583,13 @@ fn grading_wheel(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings if resp.drag_stopped() || resp.clicked() { let _ = app.run("develop.endInteraction", json!({})); } + // released, or Esc (egui ends a drag on it) + if resp.drag_stopped() + && let Some(held) = ui.ctx().data_mut(|m| m.remove_temp::(egui::Id::new(WHEEL_CAPTURE))) + && !held.dropped + { + give_pointer_back(ui.ctx(), held.origin); + } if resp.double_clicked() { let _ = app.run("develop.set", json!({"values": {format!("grading.{key}.hue"): 0, format!("grading.{key}.sat"): 0}})); } @@ -1685,6 +1693,99 @@ impl WheelGrab { } } +/// A wheel drag holds the pointer, as in Lightroom (issue #1899): it is hidden for the drag, the +/// puck follows the mouse's own travel, and on release the pointer is back where the button went +/// down, so a long (Alt) drag doesn't leave it far from the wheels. On macOS the pointer is locked +/// in place for the drag; elsewhere it moves unseen and is put back. The browser can't move it +/// (eframe's web host ignores the command), so there it only shows again where the drag ended. +#[derive(Clone, Copy, Debug, Default, PartialEq)] +struct WheelCapture { + /// Where the button went down: the pointer goes back there. + origin: Pos2, + /// Where the pointer would be had it been free: where the drag began plus the travel since. + pos: Pos2, + /// Let go early (the window lost focus): the rest of this drag leaves the wheel alone. + dropped: bool, + /// The pass that last drew the dragged wheel. + pass: u64, +} + +/// One wheel is dragged at a time, so one capture for them all. +const WHEEL_CAPTURE: &str = "wheelCapture"; + +/// The mouse's travel this frame: the pointer's, or, while the pointer stands still (locked, or +/// against a screen edge), the raw motion the host reports. +fn wheel_travel(delta: egui::Vec2, motion: Option) -> egui::Vec2 { + if delta != egui::Vec2::ZERO { + return delta; + } + motion.filter(|m| m.x.is_finite() && m.y.is_finite()).unwrap_or(egui::Vec2::ZERO) +} + +/// Where the wheel `resp` reads the pointer this frame: where it clicked, or for a drag the held +/// pointer ([`WheelCapture`]), `None` once a lost focus let go of it (Esc ends the drag itself: +/// egui stops a drag on Esc). +fn wheel_pointer(ui: &egui::Ui, resp: &egui::Response) -> Option { + let q = resp.interact_pointer_pos()?; + if !resp.dragged() { + return Some(q); + } + let ctx = ui.ctx(); + let id = egui::Id::new(WHEEL_CAPTURE); + let held = ctx.data(|m| m.get_temp::(id)).filter(|_| !resp.drag_started()); + let mut cap = held.unwrap_or_else(|| { + if cfg!(target_os = "macos") { + ctx.send_viewport_cmd(egui::ViewportCommand::CursorGrab(egui::CursorGrab::Locked)); + } + WheelCapture { origin: ui.input(|i| i.pointer.press_origin()).unwrap_or(q), pos: q, ..Default::default() } + }); + cap.pass = ctx.cumulative_pass_nr(); + if !cap.dropped { + let (travel, unfocused) = ui.input(|i| { + // the event, not `focused`: a click that activates the window may land before its focus + let unfocused = i.events.iter().any(|e| matches!(e, egui::Event::WindowFocused(false))); + (wheel_travel(i.pointer.delta(), i.pointer.motion()), unfocused) + }); + if !resp.drag_started() { + cap.pos += travel; + } + if unfocused { + give_pointer_back(ctx, cap.origin); + cap.dropped = true; + } else { + ctx.set_cursor_icon(egui::CursorIcon::None); + } + } + ctx.data_mut(|m| m.insert_temp(id, cap)); + (!cap.dropped).then_some(cap.pos) +} + +/// Unlock and show the pointer a wheel drag held, back at `origin`. +fn give_pointer_back(ctx: &egui::Context, origin: Pos2) { + if cfg!(target_os = "macos") { + ctx.send_viewport_cmd(egui::ViewportCommand::CursorGrab(egui::CursorGrab::None)); + } + if cfg!(not(target_arch = "wasm32")) { + ctx.send_viewport_cmd(egui::ViewportCommand::CursorPosition(origin)); + } +} + +/// Once a frame, after the panels: a held pointer stays hidden over whatever set its own cursor, +/// and is given back if no wheel drew this pass (the flyout closed or the view changed under the +/// drag), so it is never left locked or hidden. +pub(crate) fn wheel_pointer_frame_end(ctx: &egui::Context) { + let id = egui::Id::new(WHEEL_CAPTURE); + let Some(cap) = ctx.data(|m| m.get_temp::(id)) else { return }; + if cap.pass != ctx.cumulative_pass_nr() { + ctx.data_mut(|m| m.remove::(id)); + if !cap.dropped { + give_pointer_back(ctx, cap.origin); + } + } else if !cap.dropped { + ctx.set_cursor_icon(egui::CursorIcon::None); + } +} + /// Hue (degrees) and saturation (0..100) for a pointer at offset `v` from a wheel's centre. /// `lock`: hold the hue (the saturation is `v` projected onto that hue's spoke) or hold the /// saturation (the hue is the angle of `v`). A non-finite `v` gives a finite answer (the locked diff --git a/crates/ui-egui/src/tests_grading.rs b/crates/ui-egui/src/tests_grading.rs index f6903550c..cd1f190cf 100644 --- a/crates/ui-egui/src/tests_grading.rs +++ b/crates/ui-egui/src/tests_grading.rs @@ -375,3 +375,125 @@ fn wheel_rim_is_fully_saturated_and_the_centre_neutral() { let (hi, lo) = (p.r().max(p.g()).max(p.b()), p.r().min(p.g()).min(p.b())); assert!(hi - lo <= 40 && lo >= 100, "centre is not a neutral grey: {p:?}"); } + +// ------------------------------------------------------------------ the held pointer (issue #1899) + +fn pointer_event(h: &mut Headless, e: egui::Event) { + h.events.push(e); + h.step(); +} + +fn button(h: &mut Headless, at: egui::Pos2, pressed: bool) { + let e = egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }; + pointer_event(h, e); +} + +/// Press the midtones wheel at `at` and move the pointer 10 to the right, far enough to make it +/// a drag; where the pointer is then. +fn start_wheel_drag(h: &mut Headless, at: egui::Pos2) -> egui::Pos2 { + pointer_event(h, egui::Event::PointerMoved(at)); + button(h, at, true); + let to = at + egui::vec2(10.0, 0.0); + pointer_event(h, egui::Event::PointerMoved(to)); + h.step(); + to +} + +fn cursor_moves(h: &Headless) -> Vec { + h.window_commands + .iter() + .filter_map(|c| match c { + egui::ViewportCommand::CursorPosition(p) => Some(*p), + _ => None, + }) + .collect() +} + +#[test] +fn a_held_wheel_follows_the_mouses_motion_not_where_the_pointer_sits() { + use crate::panels::edit::wheel_pick; + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + set_wheel(&mut h, "midtones", 0.0, 0.0); + let c = widget(&h, "wheel:midtones").unwrap().center(); + let held = start_wheel_drag(&mut h, c + egui::vec2(rad * 0.4, 0.0)); + // the pointer stays where it is (macOS locks it there) while the mouse goes on moving up + for _ in 0..5 { + pointer_event(&mut h, egui::Event::MouseMoved(egui::vec2(0.0, -rad * 0.1))); + } + let (hue, sat) = wheel_value(&h, "midtones"); + let (want_hue, want_sat) = wheel_pick(held + egui::vec2(0.0, -rad * 0.5) - c, rad, None); + assert!((hue - want_hue.round() as f64).abs() <= 1.0 && (sat - want_sat.round() as f64).abs() <= 1.0, "{hue} {sat} vs {want_hue} {want_sat}"); + assert!(hue > 30.0, "the puck went up with the mouse: {hue}"); + button(&mut h, held, false); +} + +#[test] +fn a_wheel_drag_hides_the_pointer_and_releasing_puts_it_back_where_it_was_pressed() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + set_wheel(&mut h, "midtones", 0.0, 0.0); + let press = widget(&h, "wheel:midtones").unwrap().center() + egui::vec2(rad * 0.3, rad * 0.2); + let at = start_wheel_drag(&mut h, press); + let far = at + egui::vec2(-rad * 0.6, -rad * 0.9); + pointer_event(&mut h, egui::Event::PointerMoved(far)); + assert_eq!(h.last_cursor, egui::CursorIcon::None, "hidden while the wheel is dragged"); + assert!(cursor_moves(&h).is_empty(), "nothing moves the pointer before the release"); + button(&mut h, far, false); + h.step(); + assert_eq!(cursor_moves(&h), [press], "back where the button went down"); + assert_ne!(h.last_cursor, egui::CursorIcon::None, "shown again"); + #[cfg(target_os = "macos")] + assert_eq!( + h.window_commands.iter().filter(|c| matches!(c, egui::ViewportCommand::CursorGrab(_))).collect::>(), + [&egui::ViewportCommand::CursorGrab(egui::CursorGrab::Locked), &egui::ViewportCommand::CursorGrab(egui::CursorGrab::None)], + "locked for the drag, unlocked at the end" + ); + // a click is not a drag: the pointer stays visible where it is + h.window_commands.clear(); + let r = h.request("ui.clickWidget", json!({"id": "wheel:midtones"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.window_commands.is_empty(), "{:?}", h.window_commands); +} + +#[test] +fn escape_or_losing_focus_gives_the_pointer_back_mid_drag() { + let esc = egui::Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::NONE }; + for let_go in [esc, egui::Event::WindowFocused(false)] { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + set_wheel(&mut h, "midtones", 0.0, 0.0); + let press = widget(&h, "wheel:midtones").unwrap().center() + egui::vec2(rad * 0.3, 0.0); + let at = start_wheel_drag(&mut h, press); + assert_eq!(h.last_cursor, egui::CursorIcon::None, "{let_go:?}: hidden while dragged"); + pointer_event(&mut h, let_go.clone()); + h.step(); + assert_ne!(h.last_cursor, egui::CursorIcon::None, "{let_go:?}: shown again"); + assert_eq!(cursor_moves(&h), [press], "{let_go:?}: back where it was pressed"); + // the rest of the drag no longer moves the puck, and the pointer stays visible + let before = wheel_value(&h, "midtones"); + pointer_event(&mut h, egui::Event::PointerMoved(at + egui::vec2(-rad * 0.5, -rad * 0.5))); + assert_eq!(wheel_value(&h, "midtones"), before, "{let_go:?}"); + assert_ne!(h.last_cursor, egui::CursorIcon::None, "{let_go:?}"); + button(&mut h, at, false); + h.step(); + assert_eq!(cursor_moves(&h), [press], "{let_go:?}: given back once"); + } +} + +#[test] +fn a_wheel_that_goes_away_mid_drag_gives_the_pointer_back() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + let press = widget(&h, "wheel:midtones").unwrap().center() + egui::vec2(rad * 0.3, 0.0); + start_wheel_drag(&mut h, press); + assert_eq!(h.last_cursor, egui::CursorIcon::None); + // the flyout closes under the drag: no wheel is drawn to let go of the pointer itself + h.app.ui.toggle_flyout("grading"); + h.step(); + h.step(); + assert!(widget(&h, "wheel:midtones").is_none()); + assert_ne!(h.last_cursor, egui::CursorIcon::None); + assert_eq!(cursor_moves(&h), [press]); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 95dcd2aef..7c2d50e8c 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -200,7 +200,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-COLOR-MIXER-HSL | 8-band colour mixer | P0 | ✅ | `ctl:mixer.*` | no targeted (drag-on-image) mode | | LR-EDIT-COLOR-MIXER-BW | B&W mix | P1 | ✅ | `ctl:bw.*`, `cmd:develop.autoBwMix` | eight bands; Auto pushes each hue band's colourful pixels away from the mean lightness (own rule) | | LR-EDIT-COLOR-POINTCOLOR | Point colour | P1 | ✅ | `cmd:pointColor.pick`, `cmd:pointColor.delete` | | -| LR-EDIT-COLOR-GRADING | Colour grading wheels | P0 | ✅ | `ctl:grading.*`, `crates/ui-egui/src/panels/edit.rs` (`grading`) | 3-Way / Shadows / Midtones / Highlights tabs; a range tab shows one large wheel with Hue, Saturation and Luminance sliders; Blending and Balance in every tab; 3-Way shows the hovered wheel's hue and saturation; every wheel marks its neutral centre; wheel modifiers as in Lightroom: Shift-drag keeps the puck's hue and changes saturation only (the spoke is the guide; a puck at the centre has no hue, so it takes the angle where the pointer pressed), Cmd/Ctrl-drag keeps the saturation and changes hue only (the circle is the guide), Alt/Option-drag fine-adjusts at 0.1x and combines with either lock, and a hover tooltip lists them (issues #559, #1160; tests `shift_dragging_across_the_tangent_keeps_the_hue_and_moves_saturation`, `shift_drag_keeps_the_pucks_hue_wherever_the_pointer_grabs`, `cmd_drag_keeps_the_saturation_and_changes_only_the_hue`, `alt_drag_fine_adjusts_to_a_tenth_of_the_pointers_travel`, `alt_works_with_shift_and_cmd_constraints` in `crates/ui-egui/src/tests_grading.rs`); the large wheel adds 25/50/75 % saturation rings (issue #559); each wheel's eye hides its effect for viewing only (`RenderJob::with_hidden_grading`, not saved, export unaffected) (issue #737). No Global wheel tab yet | +| LR-EDIT-COLOR-GRADING | Colour grading wheels | P0 | ✅ | `ctl:grading.*`, `crates/ui-egui/src/panels/edit.rs` (`grading`) | 3-Way / Shadows / Midtones / Highlights tabs; a range tab shows one large wheel with Hue, Saturation and Luminance sliders; Blending and Balance in every tab; 3-Way shows the hovered wheel's hue and saturation; every wheel marks its neutral centre; wheel modifiers as in Lightroom: Shift-drag keeps the puck's hue and changes saturation only (the spoke is the guide; a puck at the centre has no hue, so it takes the angle where the pointer pressed), Cmd/Ctrl-drag keeps the saturation and changes hue only (the circle is the guide), Alt/Option-drag fine-adjusts at 0.1x and combines with either lock, and a hover tooltip lists them (issues #559, #1160; tests `shift_dragging_across_the_tangent_keeps_the_hue_and_moves_saturation`, `shift_drag_keeps_the_pucks_hue_wherever_the_pointer_grabs`, `cmd_drag_keeps_the_saturation_and_changes_only_the_hue`, `alt_drag_fine_adjusts_to_a_tenth_of_the_pointers_travel`, `alt_works_with_shift_and_cmd_constraints` in `crates/ui-egui/src/tests_grading.rs`); a wheel drag holds the pointer as in Lightroom: hidden while dragging (the puck follows the mouse's travel; on macOS the pointer is locked in place), back where the button went down on release, and given back at once on Esc, a lost window focus or a wheel that goes away mid-drag; the browser can't move the pointer, so there it only shows again where the drag ended (issue #1899; tests `a_held_wheel_follows_the_mouses_motion_not_where_the_pointer_sits`, `a_wheel_drag_hides_the_pointer_and_releasing_puts_it_back_where_it_was_pressed`, `escape_or_losing_focus_gives_the_pointer_back_mid_drag`, `a_wheel_that_goes_away_mid_drag_gives_the_pointer_back`); the large wheel adds 25/50/75 % saturation rings (issue #559); each wheel's eye hides its effect for viewing only (`RenderJob::with_hidden_grading`, not saved, export unaffected) (issue #737). No Global wheel tab yet | | LR-EDIT-EFFECTS-TEXTURE | Texture | P0 | ✅ | `ctl:effects.texture` | | | LR-EDIT-EFFECTS-CLARITY | Clarity | P0 | ✅ | `ctl:effects.clarity` | | | LR-EDIT-EFFECTS-DEHAZE | Dehaze | P0 | ✅ | `ctl:effects.dehaze` | |