From 2d8fc776b2496f98a50b44b64f8d0a71c6acebaf Mon Sep 17 00:00:00 2001 From: Atmik Shetty Date: Mon, 12 Oct 2026 09:24:49 +0530 Subject: [PATCH] LR-EDIT-COLOR-GRADING: Cmd/Ctrl-drag keeps the saturation, Alt/Option fine-adjusts the colour wheels (#1160) The wheels had Shift (keep the hue, change saturation). Lightroom's other two modifiers are now there too: - Cmd/Ctrl-drag keeps the puck's saturation and changes only the hue, so it moves round its circle; the circle is drawn as the guide, as Shift draws the spoke. Shift wins when both are down (keeping both would freeze the puck). - Alt/Option-drag fine-adjusts: the puck moves a tenth as far as the pointer from where it was grabbed (the slider's fine-adjust factor), counted from the press so no travel is lost to egui's drag threshold. It combines with Shift and Cmd/Ctrl. - Pressing or releasing a modifier mid-drag re-grabs the puck as it is at that moment (WheelGrab); wheel_pick takes a WheelLock (Hue / Sat) and returns finite values for any input. - A puck at the centre has no hue, so a Shift-drag from there now takes the angle where the pointer pressed, not where egui first reported a drag. - The wheel has a hover tooltip naming the modifiers. Shift-drag already kept the puck's own hue whenever its saturation was 1 or more; shift_drag_keeps_the_pucks_hue_wherever_the_pointer_grabs pins that. Tests (tests_grading.rs) drive the wheel through ui.dragWidget with each modifier and check the develop values and one undo step per drag, plus unit tests of wheel_pick (lock, clamp, centre, NaN/inf) and WheelGrab. --- crates/ui-egui/src/panels/edit.rs | 137 +++++++++++++--- crates/ui-egui/src/tests_grading.rs | 245 +++++++++++++++++++++++++++- docs/parity-checklist.md | 4 +- 3 files changed, 359 insertions(+), 27 deletions(-) diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index ed2ee4635..d730f9a5d 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -1289,6 +1289,9 @@ fn grading_wheel(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings let zone = r.expand(10.0); let hovering = ui.rect_contains_pointer(zone); let resp = ui.interact(r, egui::Id::new(("wheel", key)), Sense::click_and_drag()); + let resp = resp.on_hover_text(crate::i18n::tr( + "Drag to set hue and saturation. Hold Shift to keep the hue, Cmd/Ctrl to keep the saturation, Alt/Option to fine-adjust. Double-click to reset.", + )); register(ui.ctx(), format!("wheel:{key}"), r); paint_wheel(ui.painter(), c, rad); if hidden { @@ -1305,25 +1308,40 @@ fn grading_wheel(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings } let a = (wh.hue as f32).to_radians(); let dot = c + vec2(a.cos(), -a.sin()) * rad * (wh.sat as f32 / 100.0); - // Shift-drag: saturation only, the hue locked to the spoke the point is on (as in Lightroom). - // Pressing or releasing Shift mid-drag turns the lock on or off there. - let lock_id = egui::Id::new(("wheelHueLock", key)); - let shift = ui.input(|i| i.modifiers.shift); - let lock = match resp.interact_pointer_pos() { - Some(q) if resp.dragged() && shift => { - // a point at the centre has no hue yet: lock to where the pointer is - let start = if wh.sat >= 1.0 { wh.hue as f32 } else { wheel_pick(q - c, rad, None).0 }; - Some(ui.data_mut(|m| *m.get_temp_mut_or_insert_with(lock_id, || start))) + // Modifiers (as in Lightroom): Shift keeps the hue, Cmd/Ctrl keeps the saturation, Alt moves + // the puck a tenth as far as the pointer. Pressing or releasing one mid-drag re-grabs the + // 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() { + 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(|| { + // a fine drag counts the pointer's travel from the press, not from the frame + // egui first called it a drag + let origin = if resp.drag_started() { ui.input(|i| i.pointer.press_origin()).unwrap_or(q) } else { q }; + let g = WheelGrab::new(mods, wh.hue as f32, wh.sat as f32, origin - c, rad); + ui.data_mut(|m| m.insert_temp(grab_id, g)); + g + })) } _ => { - ui.data_mut(|m| m.remove::(lock_id)); + ui.data_mut(|m| m.remove::(grab_id)); None } }; - // while locked: the whole spoke, a guide for the saturation-only move - if let Some(h) = lock { - let a = h.to_radians(); - ui.painter().line_segment([c, c + vec2(a.cos(), -a.sin()) * rad], Stroke::new(1.5, Color32::from_white_alpha(170))); + // while a constraint is held: the guide for the one move left (the whole spoke for the hue, + // the circle for the saturation) + let guide = Stroke::new(1.5, Color32::from_white_alpha(170)); + match grab.and_then(|g| g.lock()) { + Some(WheelLock::Hue(h)) => { + let a = h.to_radians(); + ui.painter().line_segment([c, c + vec2(a.cos(), -a.sin()) * rad], guide); + } + Some(WheelLock::Sat(s)) => { + ui.painter().circle_stroke(c, rad * s / 100.0, guide); + } + None => {} } // the neutral centre (issue #559), under the point ui.painter().circle_filled(c, 2.0, Color32::WHITE); @@ -1336,7 +1354,7 @@ fn grading_wheel(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings if (resp.dragged() || resp.clicked()) && let Some(q) = resp.interact_pointer_pos() { - let (hue, sat) = wheel_pick(q - c, rad, lock); + 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()}})); } if resp.drag_stopped() || resp.clicked() { @@ -1369,17 +1387,98 @@ fn grading_wheel(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &DevelopSettings } } +/// A wheel's modifier state. Shift keeps the hue, Cmd/Ctrl the saturation (Shift wins when both +/// are down: keeping both would freeze the puck), Alt fine-adjusts. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct WheelMods { + pub keep_hue: bool, + pub keep_sat: bool, + pub fine: bool, +} + +impl WheelMods { + pub(crate) fn from_modifiers(m: egui::Modifiers) -> Self { + Self { keep_hue: m.shift, keep_sat: m.command && !m.shift, fine: m.alt } + } +} + +/// What a constraint holds still while the pointer moves. +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) enum WheelLock { + /// Keep this hue (degrees): only the saturation changes, along the spoke at that hue. + Hue(f32), + /// Keep this saturation (0..100): only the hue changes, round the circle at that radius. + Sat(f32), +} + +/// Pointer travel to puck travel with Alt held, as for a slider's fine adjust. +const WHEEL_FINE: f32 = 0.1; + +/// The puck as it was when the pointer grabbed it (or a modifier last changed): the hue and +/// saturation a constraint keeps, and the origin a fine adjust measures the pointer's travel from. +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct WheelGrab { + pub mods: WheelMods, + /// The puck's hue (degrees) and saturation (0..100) at the grab. + hue: f32, + sat: f32, + /// The pointer at the grab, as an offset from the wheel's centre. + origin: egui::Vec2, +} + +impl WheelGrab { + /// Grab a puck at `hue` / `sat` with the pointer `origin` from the centre of a wheel of + /// radius `rad`. The kept hue is the puck's own, never the angle the pointer happens to be + /// at. A puck at the centre (saturation under 1) has no hue yet, so there it is the angle of + /// the pointer at the grab; the same rule covers a non-finite hue. + pub(crate) fn new(mods: WheelMods, hue: f32, sat: f32, origin: egui::Vec2, rad: f32) -> Self { + let sat = if sat.is_finite() { sat.clamp(0.0, 100.0) } else { 0.0 }; + let hue = if hue.is_finite() && sat >= 1.0 { hue.rem_euclid(360.0) } else { wheel_pick(origin, rad, None).0 }; + Self { mods, hue, sat, origin } + } + + /// The constraint this grab holds, if any. + pub(crate) fn lock(&self) -> Option { + if self.mods.keep_hue { + Some(WheelLock::Hue(self.hue)) + } else if self.mods.keep_sat { + Some(WheelLock::Sat(self.sat)) + } else { + None + } + } + + /// The new hue and saturation for the pointer at offset `v` from the centre. + pub(crate) fn pick(&self, v: egui::Vec2, rad: f32) -> (f32, f32) { + let v = if self.mods.fine { + // the puck where it was, moved a tenth of the pointer's travel since the grab + let a = self.hue.to_radians(); + let puck = vec2(a.cos(), -a.sin()) * rad.max(1.0) * (self.sat / 100.0); + puck + (v - self.origin) * WHEEL_FINE + } else { + v + }; + wheel_pick(v, rad, self.lock()) + } +} + /// Hue (degrees) and saturation (0..100) for a pointer at offset `v` from a wheel's centre. -/// `lock`: keep this hue and take the saturation from `v` projected onto its spoke (Shift-drag). -pub(crate) fn wheel_pick(v: egui::Vec2, rad: f32, lock: Option) -> (f32, f32) { +/// `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 +/// value, else 0). +pub(crate) fn wheel_pick(v: egui::Vec2, rad: f32, lock: Option) -> (f32, f32) { let rad = rad.max(1.0); + let v = if v.x.is_finite() && v.y.is_finite() { v } else { egui::Vec2::ZERO }; + let angle = || ((-v.y).atan2(v.x).to_degrees() + 360.0) % 360.0; match lock { - Some(h) => { + Some(WheelLock::Hue(h)) => { + let h = if h.is_finite() { h } else { 0.0 }; let a = h.to_radians(); let along = v.x * a.cos() - v.y * a.sin(); (h, (along / rad * 100.0).clamp(0.0, 100.0)) } - None => (((-v.y).atan2(v.x).to_degrees() + 360.0) % 360.0, (v.length() / rad * 100.0).min(100.0)), + Some(WheelLock::Sat(s)) => (angle(), if s.is_finite() { s.clamp(0.0, 100.0) } else { 0.0 }), + None => (angle(), (v.length() / rad * 100.0).min(100.0)), } } diff --git a/crates/ui-egui/src/tests_grading.rs b/crates/ui-egui/src/tests_grading.rs index 8b6ec7c7b..6abce5460 100644 --- a/crates/ui-egui/src/tests_grading.rs +++ b/crates/ui-egui/src/tests_grading.rs @@ -104,18 +104,251 @@ fn hovering_a_three_way_wheel_shows_its_hue_and_saturation() { } #[test] -fn shift_drag_keeps_the_hue_and_changes_only_saturation() { - use crate::panels::edit::wheel_pick; +fn wheel_pick_reads_the_angle_and_distance_and_honours_a_lock() { + use crate::panels::edit::{WheelLock, wheel_pick}; let close = |a: f32, b: f32| (a - b).abs() < 0.01; // free: hue from the angle (screen y points down), saturation from the distance let (h, s) = wheel_pick(egui::vec2(0.0, -20.0), 40.0, None); assert!(close(h, 90.0) && close(s, 50.0), "{h} {s}"); - // locked at 0°: a rough drag up and right keeps the hue; only the part along the spoke counts - let (h, s) = wheel_pick(egui::vec2(30.0, -12.0), 40.0, Some(0.0)); + // hue locked at 0°: a rough drag up and right keeps the hue; only the part along the spoke counts + let (h, s) = wheel_pick(egui::vec2(30.0, -12.0), 40.0, Some(WheelLock::Hue(0.0))); assert!(close(h, 0.0) && close(s, 75.0), "{h} {s}"); // past the centre or the rim: clamped to 0..100 - assert_eq!(wheel_pick(egui::vec2(-10.0, 0.0), 40.0, Some(0.0)).1, 0.0); - assert_eq!(wheel_pick(egui::vec2(90.0, 0.0), 40.0, Some(0.0)).1, 100.0); + assert_eq!(wheel_pick(egui::vec2(-10.0, 0.0), 40.0, Some(WheelLock::Hue(0.0))).1, 0.0); + assert_eq!(wheel_pick(egui::vec2(90.0, 0.0), 40.0, Some(WheelLock::Hue(0.0))).1, 100.0); + assert_eq!(wheel_pick(egui::vec2(90.0, 0.0), 40.0, None).1, 100.0); + // saturation locked: the hue follows the pointer's angle whatever its distance + let (h, s) = wheel_pick(egui::vec2(0.0, -39.0), 40.0, Some(WheelLock::Sat(25.0))); + assert!(close(h, 90.0) && close(s, 25.0), "{h} {s}"); + let (h, s) = wheel_pick(egui::vec2(-3.0, 0.0), 40.0, Some(WheelLock::Sat(25.0))); + assert!(close(h, 180.0) && close(s, 25.0), "{h} {s}"); // a degenerate radius doesn't divide by zero assert!(wheel_pick(egui::vec2(1.0, 1.0), 0.0, None).1.is_finite()); } + +#[test] +fn wheel_pick_never_returns_nan_or_inf() { + use crate::panels::edit::{WheelLock, wheel_pick}; + let bad = [f32::NAN, f32::INFINITY, f32::NEG_INFINITY]; + let locks = [None, Some(WheelLock::Hue(f32::NAN)), Some(WheelLock::Hue(400.0)), Some(WheelLock::Sat(f32::NAN)), Some(WheelLock::Sat(250.0))]; + for lock in locks { + for x in bad { + for rad in [0.0, -5.0, f32::NAN, 40.0] { + for v in [egui::vec2(x, 3.0), egui::vec2(3.0, x), egui::vec2(x, x), egui::vec2(0.0, 0.0)] { + let (h, s) = wheel_pick(v, rad, lock); + assert!(h.is_finite() && s.is_finite() && (0.0..=100.0).contains(&s), "{v:?} {rad} {lock:?} -> {h} {s}"); + } + } + } + } + // at the exact centre a saturation lock still answers + assert_eq!(wheel_pick(egui::Vec2::ZERO, 40.0, Some(WheelLock::Sat(30.0))).1, 30.0); +} + +#[test] +fn a_grab_keeps_the_pucks_own_hue_not_the_pointers_angle() { + use crate::panels::edit::{WheelGrab, WheelLock, WheelMods}; + let shift = WheelMods { keep_hue: true, ..Default::default() }; + // the puck is at 100° / 60 %, the pointer grabs it at 10°: the locked hue is 100° + let g = WheelGrab::new(shift, 100.0, 60.0, egui::vec2(30.0, -5.0), 40.0); + assert_eq!(g.lock(), Some(WheelLock::Hue(100.0))); + // a puck at the centre has no hue yet: the pointer's angle at the grab stands in for it + let g = WheelGrab::new(shift, 100.0, 0.0, egui::vec2(0.0, -20.0), 40.0); + match g.lock() { + Some(WheelLock::Hue(h)) => assert!((h - 90.0).abs() < 0.01, "{h}"), + other => panic!("{other:?}"), + } + // Cmd/Ctrl keeps the puck's saturation; Shift wins when both are down + let m = crate::panels::edit::WheelMods::from_modifiers(egui::Modifiers { shift: true, command: true, ..Default::default() }); + assert!(m.keep_hue && !m.keep_sat); + let m = crate::panels::edit::WheelMods::from_modifiers(egui::Modifiers { command: true, ..Default::default() }); + assert_eq!(WheelGrab::new(m, 100.0, 60.0, egui::Vec2::ZERO, 40.0).lock(), Some(WheelLock::Sat(60.0))); + // nothing held: free + assert_eq!(WheelGrab::new(WheelMods::default(), 100.0, 60.0, egui::Vec2::ZERO, 40.0).lock(), None); +} + +#[test] +fn a_fine_grab_moves_the_puck_a_tenth_as_far_as_the_pointer() { + use crate::panels::edit::{WheelGrab, WheelMods}; + let fine = WheelMods { fine: true, ..Default::default() }; + // the puck at 0° / 50 % on a wheel of radius 40 sits 20 right of the centre; the pointer + // grabs it there and moves 10 up: the puck moves 1 up (hue ≈ 2.9°), not 10 + let g = WheelGrab::new(fine, 0.0, 50.0, egui::vec2(20.0, 0.0), 40.0); + let (h, s) = g.pick(egui::vec2(20.0, -10.0), 40.0); + assert!((h - (1.0f32 / 20.0).atan().to_degrees()).abs() < 0.01 && (s - 50.0 * (1.0 + 0.0025f32).sqrt()).abs() < 0.1, "{h} {s}"); + // with no travel the puck stays put, wherever the pointer grabbed it + let g = WheelGrab::new(fine, 120.0, 30.0, egui::vec2(-5.0, 5.0), 40.0); + let (h, s) = g.pick(egui::vec2(-5.0, 5.0), 40.0); + assert!((h - 120.0).abs() < 0.01 && (s - 30.0).abs() < 0.01, "{h} {s}"); +} + +/// A wheel's size on screen: its radius. +fn wheel_radius(h: &Headless, key: &str) -> f32 { + widget(h, &format!("wheel:{key}")).unwrap().width() / 2.0 +} + +/// Where a puck at `hue` / `sat` sits, from the centre of a wheel of radius `rad`. +fn puck(hue: f64, sat: f64, rad: f32) -> egui::Vec2 { + let a = (hue as f32).to_radians(); + egui::vec2(a.cos(), -a.sin()) * rad * (sat as f32 / 100.0) +} + +fn wheel_value(h: &Headless, key: &str) -> (f64, f64) { + let id = h.app.session.active().unwrap(); + let g = h.app.session.catalog.photo(id).unwrap().develop.grading; + let w = match key { + "shadows" => g.shadows, + "midtones" => g.midtones, + _ => g.highlights, + }; + (w.hue, w.sat) +} + +fn set_wheel(h: &mut Headless, key: &str, hue: f64, sat: f64) { + h.app.session.execute("develop.set", &json!({"values": {format!("grading.{key}.hue"): hue, format!("grading.{key}.sat"): sat}})).unwrap(); + h.step(); + h.step(); +} + +/// Drag the midtones wheel from `from` (an offset from its centre) by `by`, holding `mods` +/// (`shift`, `cmd`, `alt`); the puck's new hue and saturation, and how many undo steps it took. +fn drag_wheel(h: &mut Headless, from: egui::Vec2, by: egui::Vec2, mods: &[&str]) -> ((f64, f64), usize) { + let w = widget(h, "wheel:midtones").unwrap().width(); + let mut req = json!({"id": "wheel:midtones", "fx": 0.5 + from.x / w, "fy": 0.5 + from.y / w, "dx": by.x, "dy": by.y, "steps": 12}); + for m in mods { + req[*m] = json!(true); + } + let undo = h.app.session.undo.len(); + let r = h.request("ui.dragWidget", req, T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); + (wheel_value(h, "midtones"), h.app.session.undo.len() - undo) +} + +/// The distance a puck moved, in points. +fn travelled(rad: f32, a: (f64, f64), b: (f64, f64)) -> f32 { + (puck(b.0, b.1, rad) - puck(a.0, a.1, rad)).length() +} + +#[test] +fn shift_dragging_across_the_tangent_keeps_the_hue_and_moves_saturation() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + set_wheel(&mut h, "midtones", 30.0, 50.0); + let a = 30f32.to_radians(); + let radial = egui::vec2(a.cos(), -a.sin()); + let tangent = egui::vec2(-a.sin(), -a.cos()); + // grab the puck and go 10 points outward and 15 round the wheel (rad 40: +25 % saturation) + let by = (radial * 10.0 + tangent * 15.0) * (rad / 40.0); + let ((hue, sat), undo) = drag_wheel(&mut h, puck(30.0, 50.0, rad), by, &["shift"]); + assert!((hue - 30.0).abs() <= 1.0, "hue stays: {hue}"); + assert!((sat - 75.0).abs() <= 1.5, "saturation follows the radial part: {sat}"); + assert_eq!(undo, 1, "one undo step per drag"); + // the same drag unconstrained does swing the hue round, so the lock is what held it + set_wheel(&mut h, "midtones", 30.0, 50.0); + let ((hue, _), undo) = drag_wheel(&mut h, puck(30.0, 50.0, rad), by, &[]); + assert!(hue > 50.0, "unlocked, the hue follows the pointer: {hue}"); + assert_eq!(undo, 1); +} + +#[test] +fn shift_drag_keeps_the_pucks_hue_wherever_the_pointer_grabs() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + // the puck is at 30°, the pointer grabs the wheel far off that angle (at 120°, and at 250°): + // the hue stays the puck's own, it doesn't take the angle under the pointer + for grab_angle in [120.0f64, 250.0] { + set_wheel(&mut h, "midtones", 30.0, 50.0); + let from = puck(grab_angle, 60.0, rad); + let ((hue, sat), undo) = drag_wheel(&mut h, from, egui::vec2(8.0, -6.0) * (rad / 40.0), &["shift"]); + assert!((hue - 30.0).abs() <= 1.0, "grabbed at {grab_angle}°, the hue is {hue}"); + let a = 30f32.to_radians(); + let end = from + egui::vec2(8.0, -6.0) * (rad / 40.0); + let along = (end.x * a.cos() - end.y * a.sin()) / rad * 100.0; + assert!((sat as f32 - along.clamp(0.0, 100.0)).abs() <= 1.5, "saturation is the pointer's part along the spoke: {sat} vs {along}"); + assert_eq!(undo, 1); + } +} + +#[test] +fn shift_drag_from_the_neutral_centre_locks_the_hue_where_the_pointer_grabbed() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + set_wheel(&mut h, "midtones", 0.0, 0.0); + // no hue yet: the angle of the grab (90°, straight up) stands in for it + let ((hue, sat), undo) = drag_wheel(&mut h, puck(90.0, 50.0, rad), egui::vec2(6.0, -10.0) * (rad / 40.0), &["shift"]); + assert!((hue - 90.0).abs() <= 1.0, "{hue}"); + assert!((sat - 75.0).abs() <= 1.5, "{sat}"); + assert_eq!(undo, 1); +} + +#[test] +fn cmd_drag_keeps_the_saturation_and_changes_only_the_hue() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + set_wheel(&mut h, "midtones", 30.0, 50.0); + // drag the puck out to 200° at 90 % of the radius: the hue follows, the saturation stays + let from = puck(30.0, 50.0, rad); + let to = puck(200.0, 90.0, rad); + let ((hue, sat), undo) = drag_wheel(&mut h, from, to - from, &["cmd"]); + assert!((hue - 200.0).abs() <= 1.0, "{hue}"); + assert_eq!(sat, 50.0, "the saturation is held"); + assert_eq!(undo, 1, "one undo step per drag"); + // and the same drag without Cmd takes the pointer's saturation too + set_wheel(&mut h, "midtones", 30.0, 50.0); + let ((_, sat), _) = drag_wheel(&mut h, from, to - from, &[]); + assert!((sat - 90.0).abs() <= 1.5, "{sat}"); +} + +#[test] +fn alt_drag_fine_adjusts_to_a_tenth_of_the_pointers_travel() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + let start = (30.0, 50.0); + let from = puck(start.0, start.1, rad); + let by = egui::vec2(20.0, 0.0) * (rad / 40.0); + set_wheel(&mut h, "midtones", start.0, start.1); + let (free, undo_free) = drag_wheel(&mut h, from, by, &[]); + set_wheel(&mut h, "midtones", start.0, start.1); + let (fine, undo_fine) = drag_wheel(&mut h, from, by, &["alt"]); + let (free_move, fine_move) = (travelled(rad, start, free), travelled(rad, start, fine)); + let expect = by.length() * 0.1; + assert!(free_move > by.length() * 0.7, "unmodified: the puck follows the pointer ({free_move})"); + assert!((fine_move - expect).abs() <= 1.5 * (rad / 40.0), "Alt: a tenth of {} is {expect}, moved {fine_move}", by.length()); + assert!(fine_move < free_move / 5.0); + assert_eq!((undo_free, undo_fine), (1, 1)); +} + +#[test] +fn alt_works_with_shift_and_cmd_constraints() { + let mut h = demo(); + let rad = wheel_radius(&h, "midtones"); + let from = puck(30.0, 50.0, rad); + let a = 30f32.to_radians(); + // 20 points outward along the spoke, then 20 round the wheel + let radial = egui::vec2(a.cos(), -a.sin()) * 20.0 * (rad / 40.0); + let tangent = egui::vec2(-a.sin(), -a.cos()) * 20.0 * (rad / 40.0); + set_wheel(&mut h, "midtones", 30.0, 50.0); + let ((hue, sat), undo) = drag_wheel(&mut h, from, radial, &["shift", "alt"]); + assert!((hue - 30.0).abs() <= 1.0, "{hue}"); + assert!((sat - 55.0).abs() <= 1.5, "20 points is 50 %, a tenth of it is 5: {sat}"); + assert_eq!(undo, 1); + set_wheel(&mut h, "midtones", 30.0, 50.0); + let ((hue, sat), undo) = drag_wheel(&mut h, from, tangent, &["cmd", "alt"]); + assert_eq!(sat, 50.0); + // 2 points round a circle 20 points from the centre: 0.1 rad ≈ 5.7° + assert!((hue - 35.7).abs() <= 1.5, "{hue}"); + assert_eq!(undo, 1); +} + +#[test] +fn double_click_still_resets_the_wheel_with_a_modifier_held() { + let mut h = demo(); + set_wheel(&mut h, "midtones", 30.0, 50.0); + let r = h.request("ui.clickWidget", json!({"id": "wheel:midtones", "count": 2, "shift": true}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); + assert_eq!(wheel_value(&h, "midtones"), (0.0, 0.0)); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 77ae26e4c..2838a28b1 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 (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-12 · **Change:** minor (LR-EDIT-COLOR-GRADING: Cmd/Ctrl-drag keeps saturation, Alt/Option fine-adjusts (#1160); 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 @@ -197,7 +197,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; Shift-drag locks the hue and changes saturation only, showing the spoke as a guide; 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`); 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` | |