From e1acdb876d47d231e2a61186a810901640cc4735 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 02:36:01 -0400 Subject: [PATCH] LR-EDIT-CURVE: drag point-curve points in both axes The editor stored the dragged point index as Option but read it back as usize, so the index was lost after the first frame: pressing on a point did nothing and only click-to-add (overwrite) worked. Points now drag in X and Y (X clamped between the neighbours), picked at the press origin, with a grab cursor, a highlighted point and an input/output readout; one undo step per drag via develop.beginInteraction / endInteraction. Fixes #12 Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/edit.rs | 37 ++++++++++--- crates/ui-egui/src/tests_curve.rs | 86 +++++++++++++++++++++++++++++++ docs/parity.md | 2 +- 4 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 crates/ui-egui/src/tests_curve.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6ec391b44..642d89ffb 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -22,6 +22,8 @@ pub mod state; pub mod theme; pub mod widgets; +#[cfg(test)] +mod tests_curve; #[cfg(test)] mod tests_masking; #[cfg(test)] diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index dd1872bd7..a22effc37 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -677,13 +677,37 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev }; let line: Vec = (0..=96).map(|i| to_screen(i as f64 / 96.0, curve.eval(i as f64 / 96.0).clamp(0.0, 1.0))).collect(); p.add(egui::Shape::line(line, Stroke::new(2.0, color))); - for q in &pts { - p.circle_filled(to_screen(q.x, q.y), 4.0, color); - } - // interaction: drag the nearest point, click empty space to add, double-click to delete + // interaction: drag a point (both axes, between its neighbours), drag empty space to add and + // drag a new point, click empty space to add, double-click a point to delete it let drag_id = egui::Id::new(("curve-drag", &ch)); - let mut dragging: Option = ui.data(|dd| dd.get_temp(drag_id)); + // the point being dragged (stored as `Option` so a stale value can be cleared) + let mut dragging: Option = ui.data(|dd| dd.get_temp::>(drag_id)).flatten(); let nearest = |q: Pos2| pts.iter().enumerate().map(|(i, p)| (i, to_screen(p.x, p.y).distance(q))).min_by(|a, b| a.1.total_cmp(&b.1)); + let hovered = resp.hover_pos().and_then(nearest).filter(|(_, dist)| *dist < 10.0).map(|(i, _)| i); + if dragging.is_some() { + ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + } else if hovered.is_some() { + ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); + } + for (i, q) in pts.iter().enumerate() { + let c = to_screen(q.x, q.y); + if dragging == Some(i) || (dragging.is_none() && hovered == Some(i)) { + p.circle_filled(c, 5.5, color); + p.circle_stroke(c, 5.5, Stroke::new(1.0, t.canvas)); + } else { + p.circle_filled(c, 4.0, color); + } + } + // input / output readout of the point being dragged (0–255 like the histogram) + if let Some(q) = dragging.and_then(|i| pts.get(i)) { + p.text( + r.left_top() + vec2(6.0, 4.0), + egui::Align2::LEFT_TOP, + format!("{} / {}", (q.x * 255.0).round(), (q.y * 255.0).round()), + t.font(11.0), + t.text_dim, + ); + } let mut new_pts = None; if resp.double_clicked() && let Some(q) = resp.interact_pointer_pos() @@ -696,8 +720,9 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev v.remove(i); new_pts = Some(v); } else if resp.drag_started() - && let Some(q) = resp.interact_pointer_pos() + && let Some(q) = ui.input(|i| i.pointer.press_origin()).or(resp.interact_pointer_pos()) { + // pick the point under the press, not where the pointer is once the drag threshold passed let _ = app.run("develop.beginInteraction", json!({"label": "Tone Curve"})); match nearest(q) { Some((i, dist)) if dist < 10.0 => dragging = Some(i), diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs new file mode 100644 index 000000000..ebe3f9ca8 --- /dev/null +++ b/crates/ui-egui/src/tests_curve.rs @@ -0,0 +1,86 @@ +//! Headless tests of the point-curve editor: dragging points, resetting, presets. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +/// Detail view with the Edit panel's Curve flyout open on `channel`. +fn curve_open(channel: &str) -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1200.0, 1400.0], 1.0); + let r = h.request("ui.set", json!({"view": "detail"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.app.ui.right = crate::state::RightPanel::Edit; + if !h.app.ui.section_open("light") { + h.app.ui.toggle_section("light"); + } + if !h.app.ui.flyout_open("curve") { + h.app.ui.toggle_flyout("curve"); + } + h.app.ui.curve_channel = channel.into(); + h.step(); + h.step(); + h +} + +fn exec(h: &mut Headless, command: &str, params: serde_json::Value) -> serde_json::Value { + let r = h.request("engine.execute", json!({"command": command, "params": params}), T); + assert_eq!(r["ok"], true, "{command}: {r}"); + r["result"].clone() +} + +fn develop(h: &Headless) -> lightcraft_develop::DevelopSettings { + let id = h.app.session.active().expect("active photo"); + (*h.app.session.develop_of(id).unwrap_or_default()).clone() +} + +fn pts(v: &[lightcraft_geom::Point]) -> Vec<(f64, f64)> { + v.iter().map(|p| (p.x, p.y)).collect() +} + +/// Side of the (square) curve graph in screen points. +fn curve_side(h: &mut Headless) -> f64 { + let r = h.request("ui.widgets", json!({"filter": "curve"}), T); + let w = r["result"].as_array().unwrap().iter().find(|w| w["id"] == "curve").cloned().unwrap_or_else(|| panic!("curve widget on screen: {r}")); + w["rect"][2].as_f64().unwrap() +} + +#[test] +fn dragging_a_point_moves_it_in_both_axes_with_one_undo_step() { + let mut h = curve_open("master"); + exec(&mut h, "develop.curve", json!({"channel": "master", "points": [[0.0, 0.0], [0.5, 0.5], [1.0, 1.0]]})); + h.step(); + let side = curve_side(&mut h); + let undo_before = h.app.session.undo.len(); + // grab the middle point and drag it up-left: x 0.5 → 0.4, y 0.5 → 0.7 + let r = h.request("ui.dragWidget", json!({"id": "curve", "fx": 0.5, "fy": 0.5, "dx": -0.1 * side, "dy": -0.2 * side, "steps": 12}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + let c = pts(&develop(&h).curve.master); + assert_eq!(c.len(), 3, "dragging moves the point, it doesn't add one: {c:?}"); + assert!((c[1].0 - 0.4).abs() < 0.03, "x follows the pointer: {c:?}"); + assert!((c[1].1 - 0.7).abs() < 0.03, "y follows the pointer: {c:?}"); + assert_eq!(h.app.session.undo.len(), undo_before + 1, "one undo step per drag"); +} + +#[test] +fn dragging_a_point_past_its_neighbour_is_clamped() { + let mut h = curve_open("red"); + exec(&mut h, "develop.curve", json!({"channel": "red", "points": [[0.0, 0.0], [0.3, 0.3], [0.6, 0.6], [1.0, 1.0]]})); + h.step(); + let side = curve_side(&mut h); + // drag the point at 0.3 far to the right (past 0.6) + let r = h.request("ui.dragWidget", json!({"id": "curve", "fx": 0.3, "fy": 0.7, "dx": 0.5 * side, "dy": 0.0, "steps": 12}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + let c = pts(&develop(&h).curve.red); + assert_eq!(c.len(), 4, "{c:?}"); + assert!(c[1].0 < c[2].0 && c[1].0 > 0.55, "clamped just left of its neighbour: {c:?}"); + assert_eq!(c[2], (0.6, 0.6), "the neighbour stays: {c:?}"); +} diff --git a/docs/parity.md b/docs/parity.md index 2b28322cb..23434a54e 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -211,7 +211,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-EDIT-LIGHT-WHITES | Whites | P0 | ✅ | `ctl:light.whites` | | | LR-EDIT-LIGHT-BLACKS | Blacks | P0 | ✅ | `ctl:light.blacks` | | | LR-EDIT-LIGHT-CURVE-PARAM | Parametric curve | P0 | ✅ | `ctl:curve.highlights`, `ctl:curve.lights`, `ctl:curve.darks`, `ctl:curve.shadows`, `ctl:curve.split*` | | -| LR-EDIT-LIGHT-CURVE-POINT | Point curve | P0 | ✅ | `cmd:develop.curve`, `crates/ui-egui/src/panels/edit.rs` | no curve presets (linear / medium / strong) | +| LR-EDIT-LIGHT-CURVE-POINT | Point curve | P0 | ✅ | `cmd:develop.curve`, `crates/ui-egui/src/panels/edit.rs` | click to add a point; drag a point in both axes (between its neighbours, input / output readout, one undo step per drag); drag empty space to add and drag; double-click removes; no curve presets (linear / medium / strong) | | LR-EDIT-LIGHT-CURVE-RGB | Per-channel curves | P0 | ✅ | `cmd:develop.curve` (`channel`) | | | LR-EDIT-LIGHT-CURVE-REFINESAT | Curve saturation compensation | P1 | ✅ | `ctl:curve.refineSaturation` | | | LR-EDIT-LIGHT-CURVE-TAT | Drag-on-image curve adjust | P1 | ✅ | `cmd:develop.targeted` (`target: curve`) | |