From 286a5ead2c46e21dccfad5f4fa82f5b5da73a3af Mon Sep 17 00:00:00 2001 From: deunderscorepaul <131645063+deunderscorepaul@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:44:56 +0200 Subject: [PATCH 1/6] Tone Curve: the parametric graph draws the renderer's own curve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The panel drew the Effective curve with its own copy of the region formula, which omitted the running maximum `lightcraft_pipeline::finish::curve_luts` uses to keep the curve monotone. With neighbouring regions pushed opposite ways the drawn line folded back into a dip the renderer never produced, so the graph and the four sliders under it disagreed. `ParametricCurve` is now the one curve both use: the region bumps summed and then forced monotone, sampled at the renderer's resolution. The graph asks it every frame and holds no state of its own, so a slider moved by hand, Reset and Undo all redraw the line from the settings in the frame they change — the two can no longer be left showing different things. Reset and the undo step were already atomic (the whole Tone Curve is one `develop` state, and a drag is one engine interaction); tests now pin that the painted line, not just the settings, follows Reset and Undo. Tests: the folded shape proves the bare sum is not monotone while the model is; the painted path is compared point-by-point against the model; Reset clears the four sliders and the graph together; one undo takes back a whole drag. Co-Authored-By: deepseek-flash --- crates/ui-egui/src/panels/edit.rs | 88 +++++++++++---- crates/ui-egui/src/tests_curve.rs | 174 ++++++++++++++++++++++++++++++ 2 files changed, 241 insertions(+), 21 deletions(-) diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index 08f70bd02..f423d69e7 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -4,7 +4,7 @@ use egui::epaint::{Mesh, Vertex}; use egui::{Align2, Color32, Pos2, Rect, RichText, Sense, Stroke, pos2, vec2}; use lightcraft_catalog::PhotoId; -use lightcraft_develop::{ControlSpec, DevelopSettings, Section, Track, WbMode, controls}; +use lightcraft_develop::{ControlSpec, DevelopSettings, Section, ToneCurve, Track, WbMode, controls}; use lightcraft_geom::Point; use lightcraft_raster::Histogram; use serde_json::{Value, json}; @@ -929,6 +929,65 @@ fn draw_curve_histogram(p: &egui::Painter, r: Rect, h: &lightcraft_raster::Histo p.add(mesh); } +/// The four shape controls of the parametric curve, in graph order: region 0 is the darkest. +pub(crate) const PARAM_AMOUNTS: [&str; 4] = ["curve.shadows", "curve.darks", "curve.lights", "curve.highlights"]; +/// Resolution the parametric curve is sampled at, matching `lightcraft_pipeline::finish`'s table. +const PARAM_LUT: usize = 1024; +/// Segments the graph's curve line is drawn with. +pub(crate) const CURVE_SAMPLES: i32 = 96; + +/// The parametric (region) curve of a [`ToneCurve`]: the four region amounts, each a cosine bump +/// over its window between two splits, summed and then forced monotone. +/// +/// This is the renderer's curve, not an approximation of it — `lightcraft_pipeline::finish::curve_luts` +/// builds the same shape (its own table, whose `x` is the encoded level the curve sees). The graph +/// draws this curve, so the line and the sliders can never disagree: both come from the settings, +/// recomputed every frame, with no state of their own. +/// +/// The running maximum is what makes the curve monotone. Two neighbouring regions pushed opposite +/// ways can otherwise fold it back on itself, and a fold is exactly what a drawn line used to show +/// while the renderer straightened it out. +pub(crate) struct ParametricCurve { + lut: [f32; PARAM_LUT], +} + +impl ParametricCurve { + pub(crate) fn new(c: &ToneCurve) -> ParametricCurve { + let s = [(c.split_shadows / 100.0) as f32, (c.split_mid / 100.0) as f32, (c.split_highlights / 100.0) as f32]; + let amounts = [c.shadows, c.darks, c.lights, c.highlights]; + let regions = [(0.0, s[0], amounts[0]), (s[0], s[1], amounts[1]), (s[1], s[2], amounts[2]), (s[2], 1.0, amounts[3])]; + let mut lut = [0.0_f32; PARAM_LUT]; + let mut prev = 0.0_f32; + for i in 0..PARAM_LUT { + let x = i as f32 / (PARAM_LUT - 1) as f32; + let mut d = 0.0_f32; + for (a, b, amt) in regions { + if amt == 0.0 { + continue; + } + let (ctr, half) = ((a + b) / 2.0, (b - a) * 0.75 + 0.05); + let t = ((x - ctr) / half).clamp(-1.0, 1.0); + d += (amt / 100.0) as f32 * 0.22 * (0.5 + 0.5 * (t * std::f32::consts::PI).cos()); + } + let y = (x + d * 4.0 * x * (1.0 - x)).clamp(0.0, 1.0); + // the running maximum: the renderer's own monotone pass + prev = prev.max(y); + lut[i] = prev; + } + ParametricCurve { lut } + } + + /// The curve at `x` (0 = black, 1 = white), read off the table (so it never overshoots the ends). + pub(crate) fn at(&self, x: f64) -> f64 { + let x = x.clamp(0.0, 1.0) as f32; + let at = x * (PARAM_LUT - 1) as f32; + let i = at.floor().clamp(0.0, (PARAM_LUT - 1) as f32) as usize; + let next = self.lut.get(i + 1).copied().unwrap_or(self.lut[i]); + let f = at - i as f32; + (self.lut[i] + (next - self.lut[i]) * f) as f64 + } +} + fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &DevelopSettings) { let t = Tokens::get(ui.ctx()); // channel selector @@ -1005,29 +1064,16 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev let to_screen = |x: f64, y: f64| pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()); let from_screen = |q: Pos2| (((q.x - r.left()) / r.width()).clamp(0.0, 1.0) as f64, ((r.bottom() - q.y) / r.height()).clamp(0.0, 1.0) as f64); if ch == "parametric" { - // draw the effective parametric curve by sampling the pipeline's region curve approximation + // The Effective curve: the model the renderer builds from the four region sliders, sampled + // here and rebuilt every frame. The graph holds no state of its own, so a slider moved by + // hand, a Reset or an Undo redraws it from the settings in the same frame it changes. let c = &d.curve; - let regions = [ - (0.0, c.split_shadows / 100.0, c.shadows), - (c.split_shadows / 100.0, c.split_mid / 100.0, c.darks), - (c.split_mid / 100.0, c.split_highlights / 100.0, c.lights), - (c.split_highlights / 100.0, 1.0, c.highlights), - ]; - let pts: Vec = (0..=64) - .map(|i| { - let x = i as f64 / 64.0; - let mut dd = 0.0; - for (a, b, amt) in regions { - let (ctr, half) = ((a + b) / 2.0, (b - a) * 0.75 + 0.05); - let tt = ((x - ctr) / half).clamp(-1.0, 1.0); - dd += amt / 100.0 * 0.22 * (0.5 + 0.5 * (tt * std::f64::consts::PI).cos()); - } - to_screen(x, (x + dd * 4.0 * x * (1.0 - x)).clamp(0.0, 1.0)) - }) - .collect(); + let model = ParametricCurve::new(c); + let pts: Vec = + (0..=CURVE_SAMPLES).map(|i| to_screen(i as f64 / CURVE_SAMPLES as f64, model.at(i as f64 / CURVE_SAMPLES as f64))).collect(); p.add(egui::Shape::line(pts, Stroke::new(2.0, Color32::from_gray(220)))); curve_footer(app, ui, d); - for c in ["curve.highlights", "curve.lights", "curve.darks", "curve.shadows"] { + for c in PARAM_AMOUNTS { control(app, ui, d, c, true); } control(app, ui, d, "curve.refineSaturation", true); diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs index 4d3b45702..7131c1ade 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -5,6 +5,7 @@ use std::time::Duration; use serde_json::json; use crate::headless::Headless; +use crate::panels::edit::ParametricCurve; use crate::{LightcraftApp, Services}; const T: Duration = Duration::from_secs(20); @@ -221,3 +222,176 @@ fn curve_points_from_an_agent_are_stored_in_order() { let c = pts(&develop(&h).curve.green); assert!(c[1].0 < c[2].0, "dragged up to its neighbour, not past it: {c:?}"); } + +/// The settings the parametric graph is drawn from: two neighbouring regions pushed opposite ways, +/// where the raw sum of their bumps folds back on itself before the renderer's running maximum +/// straightens it out. +fn folded_regions() -> lightcraft_develop::ToneCurve { + lightcraft_develop::ToneCurve { shadows: 90.0, darks: -80.0, lights: -70.0, highlights: 60.0, ..Default::default() } +} + +/// The bare sum of the four region bumps is not monotone, so the drawn curve needs the renderer's +/// running maximum: this is the shape that made the graph disagree with the sliders. +#[test] +fn the_parametric_model_is_monotone_where_the_bare_sum_of_bumps_is_not() { + let c = folded_regions(); + let raw = |x: f64| { + let s = [0.0, c.split_shadows / 100.0, c.split_mid / 100.0, c.split_highlights / 100.0, 1.0]; + let amounts = [c.shadows, c.darks, c.lights, c.highlights]; + let mut d = 0.0; + for i in 0..4 { + let (a, b) = (s[i], s[i + 1]); + let (ctr, half) = ((a + b) / 2.0, (b - a) * 0.75 + 0.05); + let t = ((x - ctr) / half).clamp(-1.0, 1.0); + d += amounts[i] / 100.0 * 0.22 * (0.5 + 0.5 * (t * std::f64::consts::PI).cos()); + } + x + d * 4.0 * x * (1.0 - x) + }; + assert!((0..=64).any(|i| raw(i as f64 / 64.0) < raw((i + 1) as f64 / 64.0)), "the shape folds back, or it proves nothing"); + let model = ParametricCurve::new(&c); + let mut prev = f64::MIN; + for i in 0..=256 { + let y = model.at(i as f64 / 256.0); + assert!(y >= prev - 1e-12, "the model never folds back, at {i}: {y} < {prev}"); + prev = y; + } +} + +/// What the graph *paints* is the model the renderer computes from the sliders. They used to be two +/// copies of the formula and the panel's copy was missing the renderer's monotone pass, so the drawn +/// line could fold back while the renderer straightened it: the graph and the sliders below it +/// disagreed. This reads the painted line itself, not the function behind it. +#[test] +fn the_parametric_graph_paints_the_model_the_renderer_uses() { + let mut h = curve_open("parametric"); + let c = folded_regions(); + for (control, value) in [("curve.shadows", c.shadows), ("curve.darks", c.darks), ("curve.lights", c.lights), ("curve.highlights", c.highlights)] { + exec(&mut h, "develop.set", json!({"control": control, "value": value})); + } + h.step(); + h.step(); + let settings = develop(&h).curve; + assert_eq!(settings, c, "the sliders hold the folded shape"); + let graph = curve_graph_rect(&mut h); + let painted = painted_param_curve(&h, graph); + let model = ParametricCurve::new(&c); + let mut checked = 0; + for i in 0..=24 { + let x = i as f64 / 24.0; + let want = model.at(x); + let got = painted(x); + assert!((got - want).abs() < 0.02, "the painted curve is the renderer's at {x}: painted {got}, model {want}"); + checked += 1; + } + assert_eq!(checked, 25); + // and the shape really does fold back before the monotone pass, so this proves something + assert!(model.at(0.6) > model.at(0.3), "the region bumps still bend the curve"); +} + +/// The graph rect as `[left, top, width, height]` plus a reader for the painted curve's height at a +/// graph fraction (0 = the bottom of the graph, 1 = the top). +fn curve_graph_rect(h: &mut Headless) -> [f32; 4] { + 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}")); + let v = w["rect"].as_array().unwrap(); + std::array::from_fn(|i| v[i].as_f64().unwrap() as f32) +} + +/// A reader for the height of the painted parametric curve at a graph fraction. The curve is the +/// wide grey path the graph draws; the histogram is a mesh and the grid lines are single segments. +fn painted_param_curve(h: &Headless, r: [f32; 4]) -> impl Fn(f64) -> f64 { + let points = h + .view + .painted_shapes() + .into_iter() + .find_map(|s| match s { + egui::Shape::Path(p) if p.points.len() > 32 && (p.stroke.width - 2.0).abs() < 0.1 => Some(p.points), + _ => None, + }) + .unwrap_or_else(|| panic!("the parametric curve line is painted")); + move |x: f64| { + let want_x = r[0] + x as f32 * r[2]; + let q = points + .iter() + .min_by(|a, b| (a.x - want_x).abs().total_cmp(&(b.x - want_x).abs())) + .copied() + .unwrap_or_else(|| panic!("the curve has points")); + ((r[1] + r[3] - q.y) / r[3]).clamp(0.0, 1.0) as f64 + } +} + +/// The tone the parametric graph paints at `x`, read by value from the painted line. Stepped first +/// so the panel has drawn the state the command left behind. +fn painted_tone(h: &mut Headless, x: f64) -> f64 { + h.step(); + h.step(); + let r = curve_graph_rect(h); + painted_param_curve(h, r)(x) +} + +/// Where the midtones sit on a linear curve: a point well inside the Lights region, so lifting +/// Lights bends the line there. +const MID: f64 = 0.625; + +/// Reset restores the four sliders *and* puts the graph back to the straight diagonal, as one state. +#[test] +fn reset_clears_the_sliders_and_the_graph_together() { + let mut h = curve_open("parametric"); + let straight = painted_tone(&mut h, MID); + assert!((straight - MID).abs() < 0.02, "a fresh parametric curve is the diagonal: {straight}"); + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 60})); + let bent = painted_tone(&mut h, MID); + assert!(bent > MID + 0.02, "the graph follows the slider up: {bent}"); + let undo_before = h.app.session.undo.len(); + click_widget(&mut h, "button:curveReset", 1); + let c = develop(&h).curve; + assert_eq!(c, lightcraft_develop::ToneCurve::default(), "Reset puts every slider and split back: {c:?}"); + let flat = painted_tone(&mut h, MID); + assert!((flat - MID).abs() < 0.02, "and the graph is the diagonal again: {flat}"); + assert_eq!(h.app.session.undo.len(), undo_before + 1, "Reset is one undo step"); +} + +/// Undo after a Reset brings back both the slider and the graph: the state the graph draws is the +/// state Undo restores, so the two can never be left showing different things. +#[test] +fn undo_restores_the_slider_and_the_graph_together() { + let mut h = curve_open("parametric"); + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 60})); + let bent = painted_tone(&mut h, MID); + assert!(bent > MID + 0.02, "the graph is bent: {bent}"); + click_widget(&mut h, "button:curveReset", 1); + assert_eq!(develop(&h).curve, lightcraft_develop::ToneCurve::default(), "reset first"); + exec(&mut h, "edit.undo", json!({})); + let c = develop(&h).curve; + assert_eq!(c.lights, 60.0, "Undo brings the slider back: {c:?}"); + let again = painted_tone(&mut h, MID); + assert!(again > MID + 0.02, "and the graph is bent with it, not left flat: {again}"); + // redo puts the diagonal back, and the graph follows that too + exec(&mut h, "edit.redo", json!({})); + assert_eq!(develop(&h).curve, lightcraft_develop::ToneCurve::default(), "redo restores the reset"); + let flat = painted_tone(&mut h, MID); + assert!((flat - MID).abs() < 0.02, "the graph is flat with the sliders: {flat}"); +} + +/// One graph drag that bends several regions at once is one undo step: Ctrl+Z takes back the whole +/// gesture, not the last slider it happened to touch. +#[test] +fn one_graph_drag_that_moves_several_regions_is_one_undo_step() { + let mut h = curve_open("parametric"); + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 40})); + exec(&mut h, "develop.set", json!({"control": "curve.darks", "value": -40})); + let before = develop(&h).curve; + let undo_before = h.app.session.undo.len(); + // drag the Lights slider's own track, the way a graph handle writes it: begin, live updates, end + exec(&mut h, "develop.beginInteraction", json!({"label": "Parametric Curve"})); + for v in [45, 55, 70, 85] { + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": v})); + } + exec(&mut h, "develop.endInteraction", json!({})); + let after = develop(&h).curve; + assert_eq!(after.lights, 85.0, "the drag ended at its last value: {after:?}"); + assert_ne!(after, before, "the drag changed the state"); + assert_eq!(h.app.session.undo.len(), undo_before + 1, "the whole drag is one step, not one per frame"); + exec(&mut h, "edit.undo", json!({})); + assert_eq!(develop(&h).curve, before, "undo takes back the whole gesture at once"); +} From 3c3df53ca12c11f2c0355ce40409dc3d13bfb965 Mon Sep 17 00:00:00 2001 From: deunderscorepaul <131645063+deunderscorepaul@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:44:57 +0200 Subject: [PATCH 2/6] Tone Curve: drag the parametric graph, the sliders follow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parametric graph is now a control surface. Each of the four regions has a handle on its own bump: drag it up or down and that region's slider moves with it. Each of the three splits has a notch on the graph's bottom edge: drag it left or right and the region boundary moves. Every write goes through `develop.set` on the frame it happens, so the sliders under the graph, the curve and the handles are all the same state and are seen to agree mid-drag. A drag runs inside one `develop` interaction, so a whole gesture is one undo step and Ctrl+Z takes it back at once rather than unwinding it frame by frame. Details that make it usable: - The handle is the one under the *press*, not under the pointer once the drag threshold has passed — that is most of a handle's width away, and hit-testing the moved pointer grabbed nothing at all on a short drag. - The drag follows the pointer's motion from the handle's own centre, so a grab never makes the control jump by the distance from the pointer to that centre. - Splits are looked for before region handles: a notch sits on the bottom edge where the curve itself passes on the dark end of a shallow line, so on equal terms a region handle would always win and a notch could never be grabbed. - A split is kept a step clear of its neighbours, and a region handle saturates at ±100 rather than jumping when the pointer goes past what the region can reach. ⌥ is a tenth of the motion, as the point curve's drag is. Tests cover a region drag down and up, a split drag (including its bounds), a drag that starts off every handle moving nothing, the readout naming the control under the pointer, the level/amount inverse, and one undo step per gesture. Co-Authored-By: deepseek-flash --- crates/ui-egui/src/panels/edit.rs | 248 +++++++++++++++++++++++++++++- crates/ui-egui/src/tests_curve.rs | 173 +++++++++++++++++++++ 2 files changed, 417 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index f423d69e7..7dda3ab54 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -931,8 +931,12 @@ fn draw_curve_histogram(p: &egui::Painter, r: Rect, h: &lightcraft_raster::Histo /// The four shape controls of the parametric curve, in graph order: region 0 is the darkest. pub(crate) const PARAM_AMOUNTS: [&str; 4] = ["curve.shadows", "curve.darks", "curve.lights", "curve.highlights"]; +/// The three region boundaries between those four regions, in graph order. +pub(crate) const PARAM_SPLITS: [&str; 3] = ["curve.splitShadows", "curve.splitMid", "curve.splitHighlights"]; /// Resolution the parametric curve is sampled at, matching `lightcraft_pipeline::finish`'s table. const PARAM_LUT: usize = 1024; +/// Radius (screen points) within which the pointer counts as being on a curve handle. +pub(crate) const CURVE_HIT: f32 = 8.0; /// Segments the graph's curve line is drawn with. pub(crate) const CURVE_SAMPLES: i32 = 96; @@ -948,13 +952,19 @@ pub(crate) const CURVE_SAMPLES: i32 = 96; /// ways can otherwise fold it back on itself, and a fold is exactly what a drawn line used to show /// while the renderer straightened it out. pub(crate) struct ParametricCurve { + /// The curve, sampled and already forced monotone. lut: [f32; PARAM_LUT], + /// The three region boundaries as fractions of the graph. + pub(crate) splits: [f64; 3], + /// The four region amounts, in graph order (0 = the darkest). + pub(crate) amounts: [f64; 4], } impl ParametricCurve { pub(crate) fn new(c: &ToneCurve) -> ParametricCurve { - let s = [(c.split_shadows / 100.0) as f32, (c.split_mid / 100.0) as f32, (c.split_highlights / 100.0) as f32]; + let splits = [c.split_shadows / 100.0, c.split_mid / 100.0, c.split_highlights / 100.0]; let amounts = [c.shadows, c.darks, c.lights, c.highlights]; + let s = [splits[0] as f32, splits[1] as f32, splits[2] as f32]; let regions = [(0.0, s[0], amounts[0]), (s[0], s[1], amounts[1]), (s[1], s[2], amounts[2]), (s[2], 1.0, amounts[3])]; let mut lut = [0.0_f32; PARAM_LUT]; let mut prev = 0.0_f32; @@ -974,7 +984,7 @@ impl ParametricCurve { prev = prev.max(y); lut[i] = prev; } - ParametricCurve { lut } + ParametricCurve { lut, splits, amounts } } /// The curve at `x` (0 = black, 1 = white), read off the table (so it never overshoots the ends). @@ -986,6 +996,233 @@ impl ParametricCurve { let f = at - i as f32; (self.lut[i] + (next - self.lut[i]) * f) as f64 } + + /// Where region `region`'s handle sits on the input axis: the middle of its own window, so a + /// handle never has to cross a split to be grabbed. + pub(crate) fn region_x(&self, region: usize) -> f64 { + let (a, b) = self.region_window(region); + (a + b) / 2.0 + } + + /// The window (two splits, as fractions of the graph) region `region` owns. + fn region_window(&self, region: usize) -> (f64, f64) { + let s = [self.splits[0], self.splits[1], self.splits[2]]; + match region { + 0 => (0.0, s[0]), + 1 => (s[0], s[1]), + 2 => (s[1], s[2]), + _ => (s[2], 1.0), + } + } + + /// The half-width of region `region`'s own bump, as fraction of the graph. + fn region_half(&self, region: usize) -> f64 { + let (a, b) = self.region_window(region); + (b - a) * 0.75 + 0.05 + } + + /// The cosine window region `region` applies at `x` (1 at its centre, 0 at the window edges). + fn region_window_at(&self, region: usize, x: f64) -> f64 { + let ctr = self.region_x(region); + let t = ((x - ctr) / self.region_half(region)).clamp(-1.0, 1.0); + 0.5 + 0.5 * (t * std::f64::consts::PI).cos() + } + + /// The level region `region`'s bump puts the curve at when its slider holds `amount` — where the + /// region's handle is drawn. This is the region's *own* bump (the shape Lightroom draws a handle + /// on), not the monotone curve through it: the bump is linear in the amount and one-to-one, so a + /// drag inverts it exactly and the handle stays under the pointer. + pub(crate) fn handle_level(&self, region: usize, amount: f64) -> f64 { + let x = self.region_x(region); + let bump = amount / 100.0 * 0.22 * self.region_window_at(region, x) * 4.0 * x * (1.0 - x); + (x + bump).clamp(0.0, 1.0) + } + + /// The amount that puts region `region`'s handle at `level` — the inverse of [`Self::handle_level`]. + /// A drag that cannot reach the level asked for saturates at ±100 rather than jumping. + pub(crate) fn amount_at_level(&self, region: usize, level: f64) -> f64 { + let x = self.region_x(region); + let per_100 = 0.22 * self.region_window_at(region, x) * 4.0 * x * (1.0 - x); + if per_100.abs() <= f64::EPSILON { + return 0.0; + } + (((level.clamp(0.0, 1.0) - x) / per_100) * 100.0).clamp(-100.0, 100.0).round() + } + + /// Where split `split` (0..3) sits on the input axis. + pub(crate) fn split_x(&self, split: usize) -> f64 { + self.splits.get(split).copied().unwrap_or(0.5) + } + + /// The graph position split `split` may be dragged to: inside its own window, a step away from + /// the splits either side of it so the four regions keep a width. + pub(crate) fn split_x_limited(&self, split: usize, x: f64) -> f64 { + let lo = if split == 0 { 0.0 } else { self.split_x(split - 1) + SPLIT_MIN_GAP }; + let hi = if split + 1 >= PARAM_SPLITS.len() { 1.0 } else { self.split_x(split + 1) - SPLIT_MIN_GAP }; + if lo > hi { (lo + hi) / 2.0 } else { x.clamp(lo, hi) } + } + + /// The value split `split`'s slider takes when its handle is dragged to graph position `x`: the + /// position read out in that slider's own range, the way its track reads. + pub(crate) fn split_value(&self, split: usize, x: f64) -> f64 { + let x = self.split_x_limited(split, x); + let id = PARAM_SPLITS.get(split).copied().unwrap_or(PARAM_SPLITS[0]); + controls::find(id).map_or(x * 100.0, |s| (s.min + x * (s.max - s.min)).round().clamp(s.min, s.max)) + } + + /// Screen position of a handle's own centre, the level a drag starts from. + fn handle_screen_start(&self, h: ParamHandle, r: Rect) -> Pos2 { + let (x, y) = match h { + ParamHandle::Region(region) => (self.region_x(region), self.handle_level(region, self.amounts[region])), + ParamHandle::Split(split) => (self.split_x(split), 0.0), + }; + pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()) + } +} + +/// Close a graph drag's `develop` interaction, as one undo step. +/// +/// It is closed when the drag ends *or* the pointer comes up without egui reporting `drag_stopped` +/// for this widget (released outside the window, over a popup, or on a frame the widget does not +/// run). A dangling interaction would make every later edit a silent preview — Undo would then +/// appear to do nothing at all. Returns whether the drag is over, so the caller clears its state. +fn commit_interaction(app: &mut LightcraftApp, ui: &egui::Ui, dragging: bool, stopped: bool) -> bool { + let released = ui.input(|i| i.pointer.any_released()) || !ui.input(|i| i.pointer.primary_down()); + if !dragging || !(stopped || released) { + return false; + } + let _ = app.run("develop.endInteraction", json!({})); + true +} + +/// One graph drag of the parametric curve: begin the interaction, write the handle's slider on every +/// frame of the drag, end it on release. Every write goes through `develop.set`, so the slider under +/// the graph shows the value the handle holds — and the whole drag is one engine interaction, which +/// is one undo step (Ctrl+Z takes the gesture back, not the last frame of it). +fn param_handles(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui::Response, d: &DevelopSettings) { + let t = Tokens::get(ui.ctx()); + let model = ParametricCurve::new(&d.curve); + let to_screen = |x: f64, y: f64| pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()); + let drag_id = egui::Id::new("curve-param-handle-drag"); + let mut dragged: Option = ui.data(|dd| dd.get_temp::>(drag_id)).flatten(); + let hovered = if dragged.is_none() { resp.hover_pos().and_then(|q| param_handle_at(&model, r, q)) } else { None }; + let mut set: Option<(&'static str, f64)> = None; + // The handle is the one under the *press*, not under where the pointer is once the drag + // threshold has passed: that can be most of a handle's own width away, so hit-testing the + // current position would grab the wrong control (or none at all on a short, quick drag). + let press = ui.input(|i| i.pointer.press_origin()); + let grab = press.or(resp.interact_pointer_pos()); + if resp.drag_started() + && let Some(h) = grab.and_then(|q| param_handle_at(&model, r, q)) + { + let _ = app.run("develop.beginInteraction", json!({"label": "Parametric Curve"})); + dragged = Some(h); + } + if let Some(h) = dragged + && resp.dragged() + { + // The drag follows how far the pointer has moved, not where it is: the cursor it tracks is + // seeded at the handle's own centre (so grabbing a handle never makes its control jump by + // the distance between the pointer and that centre) and then advances by the pointer's + // motion. ⌥ = a tenth of the motion, as the point curve's drag does. + let cursor_id = egui::Id::new("curve-param-cursor"); + let alt = ui.input(|i| i.modifiers.alt); + let dd = ui.input(|i| i.pointer.delta()); + let step = if alt { vec2(dd.x * 0.1, dd.y * 0.1) } else { vec2(dd.x, dd.y) }; + let start = model.handle_screen_start(h, r); + // seeded at the handle's centre (so the grab itself moves nothing) and advanced by the + // pointer's motion from there + let advanced = ui.data(|d| d.get_temp::(cursor_id)).unwrap_or(start) + step; + ui.data_mut(|d| d.insert_temp(cursor_id, advanced)); + let level = ((r.bottom() - advanced.y) / r.height()).clamp(0.0, 1.0) as f64; + set = Some(match h { + ParamHandle::Region(region) => (h.control(), model.amount_at_level(region, level)), + ParamHandle::Split(split) => { + let x = ((advanced.x - r.left()) / r.width()).clamp(0.0, 1.0) as f64; + (h.control(), model.split_value(split, x)) + } + }); + } + if let Some((id, v)) = set { + let _ = app.run("develop.set", json!({"control": id, "value": v})); + } + if commit_interaction(app, ui, dragged.is_some(), resp.drag_stopped()) { + dragged = None; + // the next drag seeds its cursor from its own handle, not from where this one finished + ui.data_mut(|d| d.remove::(egui::Id::new("curve-param-cursor"))); + } + ui.data_mut(|dd| dd.insert_temp(drag_id, dragged)); + if dragged.is_some() { + ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); + } else if hovered.is_some() { + ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); + } + // the handles: a region's on its own bump, a split's a notch on the bottom edge + let p = ui.painter(); + for region in 0..PARAM_AMOUNTS.len() { + let at = to_screen(model.region_x(region), model.handle_level(region, model.amounts[region])); + let on = dragged == Some(ParamHandle::Region(region)) || hovered == Some(ParamHandle::Region(region)); + p.circle_filled(at, if on { 5.5 } else { 4.0 }, Color32::from_gray(230)); + p.circle_stroke(at, if on { 5.5 } else { 4.0 }, Stroke::new(1.0, t.canvas)); + } + for split in 0..PARAM_SPLITS.len() { + let at = to_screen(model.split_x(split), 0.0); + let on = dragged == Some(ParamHandle::Split(split)) || hovered == Some(ParamHandle::Split(split)); + let (w, h) = if on { (2.5, 9.0) } else { (1.5, 6.0) }; + p.rect_filled(Rect::from_min_max(pos2(at.x - w / 2.0, at.y - h), pos2(at.x + w / 2.0, at.y + h)), 1.0, Color32::from_gray(190)); + } + // what the handle in hand (or under the pointer) holds, as its slider below reads it + if let Some(h) = dragged.or(hovered) { + let label = controls::get(d, h.control()).map(|v| { + let name = controls::find(h.control()).map_or("", |s| s.label); + if v > 0.0 { format!("{name} {v:+.0}") } else { format!("{name} {v:.0}") } + }); + if let Some(label) = label { + p.text(r.left_top() + vec2(6.0, 4.0), Align2::LEFT_TOP, label, t.font(11.0), t.text_dim); + } + } +} + +/// Keeps a split clear of its neighbours on the graph. +const SPLIT_MIN_GAP: f64 = 0.01; + +/// A region handle of the parametric graph: which of the four regions, or which of the three splits +/// between them. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(crate) enum ParamHandle { + /// Region `0..4` (0 = the darkest), grabbed by its slider's amount. + Region(usize), + /// Split `0..3`, grabbed on the graph's bottom edge. + Split(usize), +} + +impl ParamHandle { + /// The control this handle writes. + pub(crate) fn control(self) -> &'static str { + match self { + ParamHandle::Region(r) => PARAM_AMOUNTS.get(r).copied().unwrap_or(PARAM_AMOUNTS[0]), + ParamHandle::Split(s) => PARAM_SPLITS.get(s).copied().unwrap_or(PARAM_SPLITS[0]), + } + } +} + +/// The handle under `q`, if any is within [`CURVE_HIT`]. +/// +/// A split's notch is looked for first. It lives on the graph's bottom edge, where the curve itself +/// passes on the dark end of a shallow line, so on equal terms a region handle would always win and +/// the notch could never be grabbed. +fn param_handle_at(model: &ParametricCurve, r: Rect, q: Pos2) -> Option { + let to_screen = |x: f64, y: f64| pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()); + let near = |at: Pos2| at.distance(q) <= CURVE_HIT; + for split in 0..PARAM_SPLITS.len() { + if near(to_screen(model.split_x(split), 0.0)) { + return Some(ParamHandle::Split(split)); + } + } + let closest = (0..PARAM_AMOUNTS.len()) + .map(|region| (region, to_screen(model.region_x(region), model.handle_level(region, model.amounts[region])).distance(q))) + .min_by(|a, b| a.1.total_cmp(&b.1))?; + (closest.1 <= CURVE_HIT).then_some(ParamHandle::Region(closest.0)) } fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &DevelopSettings) { @@ -1072,6 +1309,7 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev let pts: Vec = (0..=CURVE_SAMPLES).map(|i| to_screen(i as f64 / CURVE_SAMPLES as f64, model.at(i as f64 / CURVE_SAMPLES as f64))).collect(); p.add(egui::Shape::line(pts, Stroke::new(2.0, Color32::from_gray(220)))); + param_handles(app, ui, r, &resp, d); curve_footer(app, ui, d); for c in PARAM_AMOUNTS { control(app, ui, d, c, true); @@ -1205,9 +1443,11 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev let arr: Vec<[f64; 2]> = v.iter().map(|p| [p.x, p.y]).collect(); let _ = app.run("develop.curve", json!({"channel": ch, "points": arr})); } - if resp.drag_stopped() { + // the drag is over when egui says so, or when the pointer comes up without it saying so (a + // release outside the window would otherwise leave the interaction open, and every later edit a + // silent preview that Undo cannot take back) + if commit_interaction(app, ui, dragging.is_some(), resp.drag_stopped()) { dragging = None; - let _ = app.run("develop.endInteraction", json!({})); } ui.data_mut(|dd| dd.insert_temp(drag_id, dragging)); curve_footer(app, ui, d); diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs index 7131c1ade..e918d8d60 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -395,3 +395,176 @@ fn one_graph_drag_that_moves_several_regions_is_one_undo_step() { exec(&mut h, "edit.undo", json!({})); assert_eq!(develop(&h).curve, before, "undo takes back the whole gesture at once"); } + +/// The screen position of a region's handle: its region's window centre, at the level the region's +/// own bump puts the curve, so a drag starts on the dot the graph draws. +fn region_handle(r: [f32; 4], c: &lightcraft_develop::ToneCurve, region: usize) -> [f32; 2] { + let model = ParametricCurve::new(c); + let x = model.region_x(region); + let y = model.handle_level(region, model.amounts[region]); + [r[0] + x as f32 * r[2], r[1] + (1.0 - y as f32) * r[3]] +} + +/// The screen position of a split's notch, on the graph's bottom edge. +fn split_handle(r: [f32; 4], c: &lightcraft_develop::ToneCurve, split: usize) -> [f32; 2] { + let model = ParametricCurve::new(c); + [r[0] + model.split_x(split) as f32 * r[2], r[1] + r[3]] +} + +/// The point to press to grab a split's notch: its x, a few points up from the graph's bottom edge +/// (the notch is drawn on the edge, and a press exactly on the widget's border falls outside egui's +/// hit rect). Well inside [`CURVE_HIT`] of the notch. +fn split_grab(r: [f32; 4], c: &lightcraft_develop::ToneCurve, split: usize) -> [f32; 2] { + let [x, y] = split_handle(r, c, split); + [x, y - 3.0] +} + +/// Dragging a region's handle on the graph bends that region and moves its slider with it, live: +/// the value under the pointer while the drag is in flight, and the same value when it lands. +#[test] +fn dragging_a_region_handle_moves_its_slider() { + let mut h = curve_open("parametric"); + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 60})); + h.step(); + let r = curve_graph_rect(&mut h); + let c = develop(&h).curve; + let undo_before = h.app.session.undo.len(); + let [x, y] = region_handle(r, &c, 2); + let down = 0.08 * r[3]; + let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x, "toY": y + down, "steps": 8}), T); + assert_eq!(d["ok"], true, "{d}"); + h.step(); + let after = develop(&h).curve; + assert!(after.lights < 60.0, "the Lights slider follows the handle down: {}", after.lights); + assert!(after.lights > 0.0, "and lands on a value, not at the bottom: {}", after.lights); + assert_eq!((after.shadows, after.darks, after.highlights), (0.0, 0.0, 0.0), "the other regions stay put: {after:?}"); + assert_eq!(after.split_shadows, c.split_shadows, "and the splits are untouched"); + assert_eq!(h.app.session.undo.len(), undo_before + 1, "one undo step for the whole drag"); + // the graph is redrawn bent, with the handle where the drag left it + let graph = curve_graph_rect(&mut h); + let model = ParametricCurve::new(&after); + let painted = painted_param_curve(&h, graph)(model.region_x(2)); + let want = model.handle_level(2, after.lights); + assert!((painted - want).abs() < 0.03, "the handle sits on the redrawn curve: painted {painted}, handle {want}"); + // and Undo takes the whole gesture back to where it started + exec(&mut h, "edit.undo", json!({})); + assert_eq!(develop(&h).curve, c, "undo restores the value the drag started from"); +} + +/// Dragging up bends the region the other way, so the gesture is not just "downwards works". +#[test] +fn dragging_a_region_handle_up_raises_its_slider() { + let mut h = curve_open("parametric"); + let r = curve_graph_rect(&mut h); + let c = develop(&h).curve; + let [x, y] = region_handle(r, &c, 1); + let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x, "toY": y - 0.08 * r[3], "steps": 8}), T); + assert_eq!(d["ok"], true, "{d}"); + h.step(); + let after = develop(&h).curve; + assert!(after.darks > 0.0, "the Darks slider follows the handle up: {}", after.darks); + assert_eq!((after.shadows, after.lights, after.highlights), (0.0, 0.0, 0.0), "the other regions stay put: {after:?}"); + // the drawn curve really did move up where that region lives + assert!(ParametricCurve::new(&after).at(0.375) > ParametricCurve::new(&c).at(0.375), "the curve bends up at the Darks region"); +} + +/// Dragging a split's notch moves the region boundary — the same split slider the pipeline reads — +/// leaving the four region amounts alone, in one undo step. +#[test] +fn dragging_a_split_moves_the_region_boundary() { + let mut h = curve_open("parametric"); + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 40})); + h.step(); + let r = curve_graph_rect(&mut h); + let c = develop(&h).curve; + let undo_before = h.app.session.undo.len(); + let [x, y] = split_grab(r, &c, 1); + let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x + 0.1 * r[2], "toY": y, "steps": 8}), T); + assert_eq!(d["ok"], true, "{d}"); + h.step(); + let after = develop(&h).curve; + assert!(after.split_mid > c.split_mid, "the Midtones split follows the notch right: {} → {}", c.split_mid, after.split_mid); + assert_eq!((after.shadows, after.darks, after.lights, after.highlights), (0.0, 0.0, 40.0, 0.0), "the region amounts stay put"); + assert_eq!(h.app.session.undo.len(), undo_before + 1, "one undo step for the drag"); + // dragged far right it stops inside the split's own range, without crossing the next split + let [x, y] = split_grab(r, &after, 1); + let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x + 0.9 * r[2], "toY": y, "steps": 8}), T); + assert_eq!(d["ok"], true, "{d}"); + h.step(); + let far = develop(&h).curve; + assert!((20.0..=80.0).contains(&far.split_mid), "the split stays in its slider's range: {}", far.split_mid); + let model = ParametricCurve::new(&far); + assert!(model.split_x(1) < model.split_x(2), "and stays left of the split after it: {far:?}"); +} + +/// A drag that starts away from every handle is not a graph gesture at all: nothing moves and no +/// undo step is made (the graph is not a "click anywhere to bend" surface). +#[test] +fn a_parametric_drag_off_a_handle_changes_nothing() { + let mut h = curve_open("parametric"); + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 40})); + h.step(); + let before = develop(&h).curve; + let undo = h.app.session.undo.len(); + let r = curve_graph_rect(&mut h); + // a corner of the graph, far from the curve and from both handle rows + let d = h.request( + "ui.drag", + json!({"x": r[0] + 0.06 * r[2], "y": r[1] + 0.06 * r[3], "toX": r[0] + 0.2 * r[2], "toY": r[1] + 0.3 * r[3], "steps": 6}), + T, + ); + assert_eq!(d["ok"], true, "{d}"); + h.step(); + assert_eq!(develop(&h).curve, before, "no control moved"); + assert_eq!(h.app.session.undo.len(), undo, "and no undo step was made"); +} + +/// The graph shows the value under the pointer while the drag is in flight, so the slider and the +/// handle are seen to agree mid-gesture, not only once it lands. +#[test] +fn the_value_under_the_pointer_reads_out_during_the_drag() { + let mut h = curve_open("parametric"); + exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 60})); + h.step(); + let r = curve_graph_rect(&mut h); + let c = develop(&h).curve; + let [x, y] = region_handle(r, &c, 2); + // press on the handle and hover the same place: the readout names the control it will write + let q = h.request("ui.move", json!({"x": x, "y": y}), T); + assert_eq!(q["ok"], true, "{q}"); + h.step(); + assert!(h.painted_text().iter().any(|t| t.contains("Lights")), "the handle names the slider it writes: {:?}", h.painted_text()); +} + +/// A region's handle level and its slider amount are inverses, so a drag puts the handle exactly +/// where the pointer is; a level the region cannot reach saturates rather than jumping. +#[test] +fn a_region_level_and_its_amount_are_inverses() { + let c = lightcraft_develop::ToneCurve { shadows: 20.0, lights: 40.0, ..Default::default() }; + let model = ParametricCurve::new(&c); + for region in 0..4 { + for amount in [-100.0, -40.0, 0.0, 30.0, 100.0] { + let level = model.handle_level(region, amount); + let back = model.amount_at_level(region, level); + assert!((back - amount).abs() <= 1.0, "region {region}: {amount} → level {level} → {back}"); + } + } + assert_eq!(model.amount_at_level(3, 1.0), 100.0, "a level above the region's reach saturates high"); + assert_eq!(model.amount_at_level(3, 0.0), -100.0, "and one below saturates low"); + // a split reads out in its own slider's range (10..=70 here), not the graph's 0..=100 … + assert_eq!(model.split_value(0, 0.0), 10.0, "the Shadows split bottoms out at its slider's low end"); + // … and its neighbours are what actually bound it, closer than that range's own ends + assert!(model.split_value(0, 1.0) <= 70.0 && model.split_value(0, 1.0) > 10.0, "kept inside its range"); + assert!(model.split_x(1) > model.split_x(0) && model.split_x(2) > model.split_x(1), "and the splits stay in order"); + assert!(model.split_value(2, 1.0) <= 90.0, "the Highlights split stays in its own range too"); +} + +/// A dragged split never crosses its neighbours, however far the pointer goes. +#[test] +fn a_dragged_split_stays_between_its_neighbours() { + let model = ParametricCurve::new(&lightcraft_develop::ToneCurve::default()); + assert!((model.split_x_limited(1, 0.0) - 0.26).abs() < 1e-9, "Midtones stops just right of the split before it"); + assert!((model.split_x_limited(1, 1.0) - 0.74).abs() < 1e-9, "and just left of the split after it"); + assert!(model.split_x_limited(0, -1.0).abs() < 1e-9, "the first split can reach black"); + assert!((model.split_x_limited(2, 2.0) - 1.0).abs() < 1e-9, "the last can reach white"); +} From 7b6d88ce14bd09cfe9128fd254623f7d304d73d9 Mon Sep 17 00:00:00 2001 From: deunderscorepaul <131645063+deunderscorepaul@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:44:57 +0200 Subject: [PATCH 3/6] Tone Curve: the parametric graph is a clean curve you drag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The graph now shows the Effective curve and nothing else, as the parametric tab should: the four region handles and three split notches are gone. You grab the line itself, anywhere, and drag it up or down. Which slider moves is decided by where on the line you grabbed — the region whose window that input level falls in — and that slider follows on the same frame, so the line, the handle-free graph and the sliders are one state. The mapping is the renderer's own: a point's curve level against the region's slider is a straight line, so the drag inverts it exactly and the line tracks the hand instead of drifting away from it. The four regions tile the graph, so any grab moves exactly one slider. ⌥ is still a tenth of the motion, ⌥ or not the grab itself moves nothing, and a press away from the line is not a gesture (no value change, no undo step). The whole gesture is still one `develop` interaction, so it is one undo step. Note: `ui.data` stores and loads one type, so the drag state is kept as `Option<(region, x)>` and loaded with `.flatten()` — storing one shape and loading another silently returns `None` every frame, which is what made the first attempt drop the drag after a single frame. Tests rewritten for the line: a drag down and up, which region each quarter of the graph belongs to, the level/amount inverse at off-centre points, the region tiling, hovering naming the region, and one undo step per gesture. Co-Authored-By: deepseek-flash --- crates/ui-egui/src/panels/edit.rs | 238 +++++++++++------------------- crates/ui-egui/src/tests_curve.rs | 185 +++++++++++------------ 2 files changed, 179 insertions(+), 244 deletions(-) diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index 7dda3ab54..b465c544e 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -931,12 +931,8 @@ fn draw_curve_histogram(p: &egui::Painter, r: Rect, h: &lightcraft_raster::Histo /// The four shape controls of the parametric curve, in graph order: region 0 is the darkest. pub(crate) const PARAM_AMOUNTS: [&str; 4] = ["curve.shadows", "curve.darks", "curve.lights", "curve.highlights"]; -/// The three region boundaries between those four regions, in graph order. -pub(crate) const PARAM_SPLITS: [&str; 3] = ["curve.splitShadows", "curve.splitMid", "curve.splitHighlights"]; /// Resolution the parametric curve is sampled at, matching `lightcraft_pipeline::finish`'s table. const PARAM_LUT: usize = 1024; -/// Radius (screen points) within which the pointer counts as being on a curve handle. -pub(crate) const CURVE_HIT: f32 = 8.0; /// Segments the graph's curve line is drawn with. pub(crate) const CURVE_SAMPLES: i32 = 96; @@ -1028,56 +1024,52 @@ impl ParametricCurve { 0.5 + 0.5 * (t * std::f64::consts::PI).cos() } - /// The level region `region`'s bump puts the curve at when its slider holds `amount` — where the - /// region's handle is drawn. This is the region's *own* bump (the shape Lightroom draws a handle - /// on), not the monotone curve through it: the bump is linear in the amount and one-to-one, so a - /// drag inverts it exactly and the handle stays under the pointer. - pub(crate) fn handle_level(&self, region: usize, amount: f64) -> f64 { - let x = self.region_x(region); - let bump = amount / 100.0 * 0.22 * self.region_window_at(region, x) * 4.0 * x * (1.0 - x); - (x + bump).clamp(0.0, 1.0) + /// The region that owns graph position `x`: the one whose own window contains it. The four + /// windows tile 0..1, so this is always defined and a drag anywhere on the curve moves exactly + /// one slider — the region under the pointer. + pub(crate) fn region_at(&self, x: f64) -> usize { + let x = x.clamp(0.0, 1.0); + // the bands are black..split0, split0..split1, split1..split2, split2..white + let (s0, s1, s2) = (self.split_x(0), self.split_x(1), self.split_x(2)); + if x <= s0 { + 0 + } else if x <= s1 { + 1 + } else if x <= s2 { + 2 + } else { + 3 + } } - /// The amount that puts region `region`'s handle at `level` — the inverse of [`Self::handle_level`]. - /// A drag that cannot reach the level asked for saturates at ±100 rather than jumping. - pub(crate) fn amount_at_level(&self, region: usize, level: f64) -> f64 { - let x = self.region_x(region); - let per_100 = 0.22 * self.region_window_at(region, x) * 4.0 * x * (1.0 - x); - if per_100.abs() <= f64::EPSILON { + /// How much output level region `region`'s bump gains at `x` per unit of its slider: the bump is + /// linear in the amount, so this is a constant for a given `x`. A drag converts the pointer's + /// vertical motion into a slider change through it. + pub(crate) fn level_per_unit(&self, region: usize, x: f64) -> f64 { + let x = x.clamp(0.0, 1.0); + 0.22 / 100.0 * self.region_window_at(region, x) * 4.0 * x * (1.0 - x) + } + + /// The curve level region `region` sits at when its slider holds `amount`, at graph position `x` + /// — where the curve is at the point a drag grabbed, which is what that drag starts from. + pub(crate) fn handle_level_at(&self, region: usize, amount: f64, x: f64) -> f64 { + (x.clamp(0.0, 1.0) + amount * self.level_per_unit(region, x)).clamp(0.0, 1.0) + } + + /// The amount that puts region `region`'s curve at `level` at `x` — the inverse of + /// [`Self::handle_level_at`]. A level the region cannot reach saturates rather than jumping. + pub(crate) fn amount_at_level(&self, region: usize, level: f64, x: f64) -> f64 { + let per_unit = self.level_per_unit(region, x); + if per_unit.abs() <= f64::EPSILON { return 0.0; } - (((level.clamp(0.0, 1.0) - x) / per_100) * 100.0).clamp(-100.0, 100.0).round() + ((level.clamp(0.0, 1.0) - x.clamp(0.0, 1.0)) / per_unit).clamp(-100.0, 100.0).round() } - /// Where split `split` (0..3) sits on the input axis. + /// The graph position split `split` (0..3) sits at, 0..1. pub(crate) fn split_x(&self, split: usize) -> f64 { self.splits.get(split).copied().unwrap_or(0.5) } - - /// The graph position split `split` may be dragged to: inside its own window, a step away from - /// the splits either side of it so the four regions keep a width. - pub(crate) fn split_x_limited(&self, split: usize, x: f64) -> f64 { - let lo = if split == 0 { 0.0 } else { self.split_x(split - 1) + SPLIT_MIN_GAP }; - let hi = if split + 1 >= PARAM_SPLITS.len() { 1.0 } else { self.split_x(split + 1) - SPLIT_MIN_GAP }; - if lo > hi { (lo + hi) / 2.0 } else { x.clamp(lo, hi) } - } - - /// The value split `split`'s slider takes when its handle is dragged to graph position `x`: the - /// position read out in that slider's own range, the way its track reads. - pub(crate) fn split_value(&self, split: usize, x: f64) -> f64 { - let x = self.split_x_limited(split, x); - let id = PARAM_SPLITS.get(split).copied().unwrap_or(PARAM_SPLITS[0]); - controls::find(id).map_or(x * 100.0, |s| (s.min + x * (s.max - s.min)).round().clamp(s.min, s.max)) - } - - /// Screen position of a handle's own centre, the level a drag starts from. - fn handle_screen_start(&self, h: ParamHandle, r: Rect) -> Pos2 { - let (x, y) = match h { - ParamHandle::Region(region) => (self.region_x(region), self.handle_level(region, self.amounts[region])), - ParamHandle::Split(split) => (self.split_x(split), 0.0), - }; - pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()) - } } /// Close a graph drag's `develop` interaction, as one undo step. @@ -1095,136 +1087,86 @@ fn commit_interaction(app: &mut LightcraftApp, ui: &egui::Ui, dragging: bool, st true } -/// One graph drag of the parametric curve: begin the interaction, write the handle's slider on every -/// frame of the drag, end it on release. Every write goes through `develop.set`, so the slider under -/// the graph shows the value the handle holds — and the whole drag is one engine interaction, which -/// is one undo step (Ctrl+Z takes the gesture back, not the last frame of it). -fn param_handles(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui::Response, d: &DevelopSettings) { +/// How close (screen points) to the curve a press counts as grabbing it. +const PARAM_GRAB: f32 = 14.0; + +/// The parametric curve's own drag: the graph shows the curve and nothing else, and dragging the +/// line bends the region the pointer is over, with that region's slider following on the same frame. +/// +/// The whole gesture runs inside one `develop` interaction, so it is one undo step (Ctrl+Z takes the +/// gesture back, not the last frame of it), and every write goes through `develop.set`, so the +/// slider, the curve and what is drawn are the same state rather than three things kept in step. +fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui::Response, d: &DevelopSettings) { let t = Tokens::get(ui.ctx()); let model = ParametricCurve::new(&d.curve); let to_screen = |x: f64, y: f64| pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()); - let drag_id = egui::Id::new("curve-param-handle-drag"); - let mut dragged: Option = ui.data(|dd| dd.get_temp::>(drag_id)).flatten(); - let hovered = if dragged.is_none() { resp.hover_pos().and_then(|q| param_handle_at(&model, r, q)) } else { None }; + let from_screen_x = |q: Pos2| ((q.x - r.left()) / r.width()).clamp(0.0, 1.0) as f64; + // the graph x of a point, and how far it is from the drawn curve (in screen points) + let near_curve = |q: Pos2| { + let x = from_screen_x(q); + to_screen(x, model.at(x)).distance(q) + }; + let drag_id = egui::Id::new("curve-param-drag"); + let cursor_id = egui::Id::new("curve-param-cursor"); + // the region being dragged, and the graph x the drag is anchored at (stored as an `Option` so a + // stale value can be cleared, as the point-curve editor's drag state is) + let mut dragged: Option<(usize, f64)> = ui.data(|dd| dd.get_temp::>(drag_id)).flatten(); + let hovered = dragged.is_none() && resp.hover_pos().is_some_and(|q| near_curve(q) <= PARAM_GRAB); let mut set: Option<(&'static str, f64)> = None; - // The handle is the one under the *press*, not under where the pointer is once the drag - // threshold has passed: that can be most of a handle's own width away, so hit-testing the - // current position would grab the wrong control (or none at all on a short, quick drag). - let press = ui.input(|i| i.pointer.press_origin()); - let grab = press.or(resp.interact_pointer_pos()); + // the press, not where the pointer is once the drag threshold has passed: a press slightly off + // the line is still a grab, and the pointer has usually left that spot by the time it starts + let press = ui.input(|i| i.pointer.press_origin()).or(resp.interact_pointer_pos()); if resp.drag_started() - && let Some(h) = grab.and_then(|q| param_handle_at(&model, r, q)) + && let Some(q) = press + && near_curve(q) <= PARAM_GRAB { + let x = from_screen_x(q); let _ = app.run("develop.beginInteraction", json!({"label": "Parametric Curve"})); - dragged = Some(h); + dragged = Some((model.region_at(x), x)); + ui.data_mut(|dd| dd.insert_temp(cursor_id, to_screen(x, model.at(x)))); } - if let Some(h) = dragged + if let Some((region, x)) = dragged && resp.dragged() { - // The drag follows how far the pointer has moved, not where it is: the cursor it tracks is - // seeded at the handle's own centre (so grabbing a handle never makes its control jump by - // the distance between the pointer and that centre) and then advances by the pointer's - // motion. ⌥ = a tenth of the motion, as the point curve's drag does. - let cursor_id = egui::Id::new("curve-param-cursor"); + // Follow the pointer's motion from where the curve was grabbed, so the press itself moves + // nothing and the line tracks the hand. ⌥ = a tenth of the motion, as the point curve does. let alt = ui.input(|i| i.modifiers.alt); let dd = ui.input(|i| i.pointer.delta()); let step = if alt { vec2(dd.x * 0.1, dd.y * 0.1) } else { vec2(dd.x, dd.y) }; - let start = model.handle_screen_start(h, r); - // seeded at the handle's centre (so the grab itself moves nothing) and advanced by the - // pointer's motion from there - let advanced = ui.data(|d| d.get_temp::(cursor_id)).unwrap_or(start) + step; - ui.data_mut(|d| d.insert_temp(cursor_id, advanced)); - let level = ((r.bottom() - advanced.y) / r.height()).clamp(0.0, 1.0) as f64; - set = Some(match h { - ParamHandle::Region(region) => (h.control(), model.amount_at_level(region, level)), - ParamHandle::Split(split) => { - let x = ((advanced.x - r.left()) / r.width()).clamp(0.0, 1.0) as f64; - (h.control(), model.split_value(split, x)) - } - }); + let start = to_screen(x, model.handle_level_at(region, model.amounts[region], x)); + let cursor = ui.data(|d| d.get_temp::(cursor_id)).unwrap_or(start) + step; + ui.data_mut(|d| d.insert_temp(cursor_id, cursor)); + let level = ((r.bottom() - cursor.y) / r.height()).clamp(0.0, 1.0) as f64; + set = Some((PARAM_AMOUNTS[region], model.amount_at_level(region, level, x))); } if let Some((id, v)) = set { let _ = app.run("develop.set", json!({"control": id, "value": v})); } - if commit_interaction(app, ui, dragged.is_some(), resp.drag_stopped()) { + let dropped = resp.drag_stopped(); + if commit_interaction(app, ui, dragged.is_some(), dropped) { dragged = None; - // the next drag seeds its cursor from its own handle, not from where this one finished - ui.data_mut(|d| d.remove::(egui::Id::new("curve-param-cursor"))); + ui.data_mut(|d| { + d.remove::(cursor_id); + d.remove::>(drag_id); + }); } ui.data_mut(|dd| dd.insert_temp(drag_id, dragged)); if dragged.is_some() { ui.ctx().set_cursor_icon(egui::CursorIcon::Grabbing); - } else if hovered.is_some() { + } else if hovered { ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); } - // the handles: a region's on its own bump, a split's a notch on the bottom edge - let p = ui.painter(); - for region in 0..PARAM_AMOUNTS.len() { - let at = to_screen(model.region_x(region), model.handle_level(region, model.amounts[region])); - let on = dragged == Some(ParamHandle::Region(region)) || hovered == Some(ParamHandle::Region(region)); - p.circle_filled(at, if on { 5.5 } else { 4.0 }, Color32::from_gray(230)); - p.circle_stroke(at, if on { 5.5 } else { 4.0 }, Stroke::new(1.0, t.canvas)); - } - for split in 0..PARAM_SPLITS.len() { - let at = to_screen(model.split_x(split), 0.0); - let on = dragged == Some(ParamHandle::Split(split)) || hovered == Some(ParamHandle::Split(split)); - let (w, h) = if on { (2.5, 9.0) } else { (1.5, 6.0) }; - p.rect_filled(Rect::from_min_max(pos2(at.x - w / 2.0, at.y - h), pos2(at.x + w / 2.0, at.y + h)), 1.0, Color32::from_gray(190)); - } - // what the handle in hand (or under the pointer) holds, as its slider below reads it - if let Some(h) = dragged.or(hovered) { - let label = controls::get(d, h.control()).map(|v| { - let name = controls::find(h.control()).map_or("", |s| s.label); - if v > 0.0 { format!("{name} {v:+.0}") } else { format!("{name} {v:.0}") } - }); - if let Some(label) = label { - p.text(r.left_top() + vec2(6.0, 4.0), Align2::LEFT_TOP, label, t.font(11.0), t.text_dim); + // the region under the pointer (or the one in hand), named as its slider below reads it + let shown = dragged.map(|(region, _)| region).or_else(|| hovered.then(|| resp.hover_pos()).flatten().map(|q| model.region_at(from_screen_x(q)))); + if let Some(region) = shown { + let id = PARAM_AMOUNTS[region]; + if let Some(v) = controls::get(d, id) { + let name = controls::find(id).map_or("", |s| s.label); + ui.painter().text(r.left_top() + vec2(6.0, 4.0), Align2::LEFT_TOP, format!("{name} {v:.0}"), t.font(11.0), t.text_dim); } } } -/// Keeps a split clear of its neighbours on the graph. -const SPLIT_MIN_GAP: f64 = 0.01; - -/// A region handle of the parametric graph: which of the four regions, or which of the three splits -/// between them. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub(crate) enum ParamHandle { - /// Region `0..4` (0 = the darkest), grabbed by its slider's amount. - Region(usize), - /// Split `0..3`, grabbed on the graph's bottom edge. - Split(usize), -} - -impl ParamHandle { - /// The control this handle writes. - pub(crate) fn control(self) -> &'static str { - match self { - ParamHandle::Region(r) => PARAM_AMOUNTS.get(r).copied().unwrap_or(PARAM_AMOUNTS[0]), - ParamHandle::Split(s) => PARAM_SPLITS.get(s).copied().unwrap_or(PARAM_SPLITS[0]), - } - } -} - -/// The handle under `q`, if any is within [`CURVE_HIT`]. -/// -/// A split's notch is looked for first. It lives on the graph's bottom edge, where the curve itself -/// passes on the dark end of a shallow line, so on equal terms a region handle would always win and -/// the notch could never be grabbed. -fn param_handle_at(model: &ParametricCurve, r: Rect, q: Pos2) -> Option { - let to_screen = |x: f64, y: f64| pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()); - let near = |at: Pos2| at.distance(q) <= CURVE_HIT; - for split in 0..PARAM_SPLITS.len() { - if near(to_screen(model.split_x(split), 0.0)) { - return Some(ParamHandle::Split(split)); - } - } - let closest = (0..PARAM_AMOUNTS.len()) - .map(|region| (region, to_screen(model.region_x(region), model.handle_level(region, model.amounts[region])).distance(q))) - .min_by(|a, b| a.1.total_cmp(&b.1))?; - (closest.1 <= CURVE_HIT).then_some(ParamHandle::Region(closest.0)) -} - fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &DevelopSettings) { let t = Tokens::get(ui.ctx()); // channel selector @@ -1309,7 +1251,7 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev let pts: Vec = (0..=CURVE_SAMPLES).map(|i| to_screen(i as f64 / CURVE_SAMPLES as f64, model.at(i as f64 / CURVE_SAMPLES as f64))).collect(); p.add(egui::Shape::line(pts, Stroke::new(2.0, Color32::from_gray(220)))); - param_handles(app, ui, r, &resp, d); + param_drag(app, ui, r, &resp, d); curve_footer(app, ui, d); for c in PARAM_AMOUNTS { control(app, ui, d, c, true); diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs index e918d8d60..7dd86e512 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -396,56 +396,37 @@ fn one_graph_drag_that_moves_several_regions_is_one_undo_step() { assert_eq!(develop(&h).curve, before, "undo takes back the whole gesture at once"); } -/// The screen position of a region's handle: its region's window centre, at the level the region's -/// own bump puts the curve, so a drag starts on the dot the graph draws. -fn region_handle(r: [f32; 4], c: &lightcraft_develop::ToneCurve, region: usize) -> [f32; 2] { - let model = ParametricCurve::new(c); - let x = model.region_x(region); - let y = model.handle_level(region, model.amounts[region]); - [r[0] + x as f32 * r[2], r[1] + (1.0 - y as f32) * r[3]] +/// The screen point where a drag should grab the curve: graph fraction `fx`, on the line itself. +fn on_curve(r: [f32; 4], c: &lightcraft_develop::ToneCurve, fx: f32) -> [f32; 2] { + let y = ParametricCurve::new(c).at(fx as f64); + [r[0] + fx * r[2], r[1] + (1.0 - y as f32) * r[3]] } -/// The screen position of a split's notch, on the graph's bottom edge. -fn split_handle(r: [f32; 4], c: &lightcraft_develop::ToneCurve, split: usize) -> [f32; 2] { - let model = ParametricCurve::new(c); - [r[0] + model.split_x(split) as f32 * r[2], r[1] + r[3]] -} - -/// The point to press to grab a split's notch: its x, a few points up from the graph's bottom edge -/// (the notch is drawn on the edge, and a press exactly on the widget's border falls outside egui's -/// hit rect). Well inside [`CURVE_HIT`] of the notch. -fn split_grab(r: [f32; 4], c: &lightcraft_develop::ToneCurve, split: usize) -> [f32; 2] { - let [x, y] = split_handle(r, c, split); - [x, y - 3.0] -} - -/// Dragging a region's handle on the graph bends that region and moves its slider with it, live: -/// the value under the pointer while the drag is in flight, and the same value when it lands. +/// Dragging the curve itself bends the region under the pointer and moves that region's slider with +/// it, live. The graph draws no handles, so the only thing to grab is the line. #[test] -fn dragging_a_region_handle_moves_its_slider() { +fn dragging_the_curve_moves_the_region_slider() { let mut h = curve_open("parametric"); exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 60})); h.step(); let r = curve_graph_rect(&mut h); let c = develop(&h).curve; let undo_before = h.app.session.undo.len(); - let [x, y] = region_handle(r, &c, 2); - let down = 0.08 * r[3]; - let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x, "toY": y + down, "steps": 8}), T); + // grab the line inside the Lights window (x = 0.625) and pull it down + let [x, y] = on_curve(r, &c, 0.625); + let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x, "toY": y + 0.08 * r[3], "steps": 8}), T); assert_eq!(d["ok"], true, "{d}"); h.step(); let after = develop(&h).curve; - assert!(after.lights < 60.0, "the Lights slider follows the handle down: {}", after.lights); + assert!(after.lights < 60.0, "the Lights slider follows the line down: {}", after.lights); assert!(after.lights > 0.0, "and lands on a value, not at the bottom: {}", after.lights); assert_eq!((after.shadows, after.darks, after.highlights), (0.0, 0.0, 0.0), "the other regions stay put: {after:?}"); assert_eq!(after.split_shadows, c.split_shadows, "and the splits are untouched"); assert_eq!(h.app.session.undo.len(), undo_before + 1, "one undo step for the whole drag"); - // the graph is redrawn bent, with the handle where the drag left it + // the graph really is redrawn below where the line was let graph = curve_graph_rect(&mut h); - let model = ParametricCurve::new(&after); - let painted = painted_param_curve(&h, graph)(model.region_x(2)); - let want = model.handle_level(2, after.lights); - assert!((painted - want).abs() < 0.03, "the handle sits on the redrawn curve: painted {painted}, handle {want}"); + let painted = painted_param_curve(&h, graph)(0.625); + assert!(painted < c.lights / 100.0 + 0.625, "the painted line followed the drag down: {painted}"); // and Undo takes the whole gesture back to where it started exec(&mut h, "edit.undo", json!({})); assert_eq!(develop(&h).curve, c, "undo restores the value the drag started from"); @@ -453,61 +434,60 @@ fn dragging_a_region_handle_moves_its_slider() { /// Dragging up bends the region the other way, so the gesture is not just "downwards works". #[test] -fn dragging_a_region_handle_up_raises_its_slider() { +fn dragging_the_curve_up_raises_the_region_slider() { let mut h = curve_open("parametric"); let r = curve_graph_rect(&mut h); let c = develop(&h).curve; - let [x, y] = region_handle(r, &c, 1); + // x = 0.375 is inside the Darks window + let [x, y] = on_curve(r, &c, 0.375); let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x, "toY": y - 0.08 * r[3], "steps": 8}), T); assert_eq!(d["ok"], true, "{d}"); h.step(); let after = develop(&h).curve; - assert!(after.darks > 0.0, "the Darks slider follows the handle up: {}", after.darks); + assert!(after.darks > 0.0, "the Darks slider follows the line up: {}", after.darks); assert_eq!((after.shadows, after.lights, after.highlights), (0.0, 0.0, 0.0), "the other regions stay put: {after:?}"); - // the drawn curve really did move up where that region lives - assert!(ParametricCurve::new(&after).at(0.375) > ParametricCurve::new(&c).at(0.375), "the curve bends up at the Darks region"); + assert!(ParametricCurve::new(&after).at(0.375) > ParametricCurve::new(&c).at(0.375), "the curve is drawn bent up at the Darks region"); } -/// Dragging a split's notch moves the region boundary — the same split slider the pipeline reads — -/// leaving the four region amounts alone, in one undo step. +/// Which region a drag moves is decided by where on the line it is grabbed: the same drag gesture +/// four times over grabs four different sliders. #[test] -fn dragging_a_split_moves_the_region_boundary() { - let mut h = curve_open("parametric"); - exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 40})); - h.step(); - let r = curve_graph_rect(&mut h); - let c = develop(&h).curve; - let undo_before = h.app.session.undo.len(); - let [x, y] = split_grab(r, &c, 1); - let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x + 0.1 * r[2], "toY": y, "steps": 8}), T); - assert_eq!(d["ok"], true, "{d}"); - h.step(); - let after = develop(&h).curve; - assert!(after.split_mid > c.split_mid, "the Midtones split follows the notch right: {} → {}", c.split_mid, after.split_mid); - assert_eq!((after.shadows, after.darks, after.lights, after.highlights), (0.0, 0.0, 40.0, 0.0), "the region amounts stay put"); - assert_eq!(h.app.session.undo.len(), undo_before + 1, "one undo step for the drag"); - // dragged far right it stops inside the split's own range, without crossing the next split - let [x, y] = split_grab(r, &after, 1); - let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x + 0.9 * r[2], "toY": y, "steps": 8}), T); - assert_eq!(d["ok"], true, "{d}"); - h.step(); - let far = develop(&h).curve; - assert!((20.0..=80.0).contains(&far.split_mid), "the split stays in its slider's range: {}", far.split_mid); - let model = ParametricCurve::new(&far); - assert!(model.split_x(1) < model.split_x(2), "and stays left of the split after it: {far:?}"); +fn the_region_dragged_is_the_one_under_the_pointer() { + for (fx, control, region) in + [(0.125_f32, "curve.shadows", 0usize), (0.375, "curve.darks", 1), (0.625, "curve.lights", 2), (0.875, "curve.highlights", 3)] + { + let mut h = curve_open("parametric"); + let r = curve_graph_rect(&mut h); + let c = develop(&h).curve; + assert_eq!(ParametricCurve::new(&c).region_at(fx as f64), region, "x = {fx} belongs to {control}"); + let [x, y] = on_curve(r, &c, fx); + let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x, "toY": y - 0.06 * r[3], "steps": 6}), T); + assert_eq!(d["ok"], true, "{d}"); + h.step(); + let after = develop(&h).curve; + let moved = after.shadows + after.darks + after.lights + after.highlights; + let want = match region { + 0 => after.shadows, + 1 => after.darks, + 2 => after.lights, + _ => after.highlights, + }; + assert!(want > 0.0, "grabbing at {fx} raises {control}: {after:?}"); + assert!((moved - want).abs() < 1e-9, "and nothing else moved: {after:?}"); + } } -/// A drag that starts away from every handle is not a graph gesture at all: nothing moves and no -/// undo step is made (the graph is not a "click anywhere to bend" surface). +/// A drag that starts away from the line is not a graph gesture at all: nothing moves and no undo +/// step is made (the graph is not a "click anywhere to bend" surface). #[test] -fn a_parametric_drag_off_a_handle_changes_nothing() { +fn a_parametric_drag_off_the_line_changes_nothing() { let mut h = curve_open("parametric"); exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 40})); h.step(); let before = develop(&h).curve; let undo = h.app.session.undo.len(); let r = curve_graph_rect(&mut h); - // a corner of the graph, far from the curve and from both handle rows + // a corner of the graph, far above the line let d = h.request( "ui.drag", json!({"x": r[0] + 0.06 * r[2], "y": r[1] + 0.06 * r[3], "toX": r[0] + 0.2 * r[2], "toY": r[1] + 0.3 * r[3], "steps": 6}), @@ -519,52 +499,65 @@ fn a_parametric_drag_off_a_handle_changes_nothing() { assert_eq!(h.app.session.undo.len(), undo, "and no undo step was made"); } -/// The graph shows the value under the pointer while the drag is in flight, so the slider and the -/// handle are seen to agree mid-gesture, not only once it lands. +/// Hovering the line names the region a press there would move (and offers the grab cursor), so the +/// graph says which slider it is about to change before the drag starts. #[test] -fn the_value_under_the_pointer_reads_out_during_the_drag() { +fn hovering_the_line_names_the_region_it_would_move() { let mut h = curve_open("parametric"); exec(&mut h, "develop.set", json!({"control": "curve.lights", "value": 60})); h.step(); let r = curve_graph_rect(&mut h); - let c = develop(&h).curve; - let [x, y] = region_handle(r, &c, 2); - // press on the handle and hover the same place: the readout names the control it will write + let [x, y] = on_curve(r, &develop(&h).curve, 0.625); let q = h.request("ui.move", json!({"x": x, "y": y}), T); assert_eq!(q["ok"], true, "{q}"); h.step(); - assert!(h.painted_text().iter().any(|t| t.contains("Lights")), "the handle names the slider it writes: {:?}", h.painted_text()); + assert_eq!(h.last_cursor, egui::CursorIcon::Grab, "the line offers a grab: {:?}", h.last_cursor); + assert!(h.painted_text().iter().any(|t| t.contains("Lights")), "and names the slider it would move: {:?}", h.painted_text()); + // well above the line there is no grab and no name + let q = h.request("ui.move", json!({"x": x, "y": r[1] + 0.06 * r[3]}), T); + assert_eq!(q["ok"], true, "{q}"); + h.step(); + assert_eq!(h.last_cursor, egui::CursorIcon::Default, "away from the line there is nothing to grab: {:?}", h.last_cursor); } -/// A region's handle level and its slider amount are inverses, so a drag puts the handle exactly -/// where the pointer is; a level the region cannot reach saturates rather than jumping. +/// A point's level on the region's curve and its slider amount are inverses, so a drag puts the line +/// exactly where the hand is, at whatever point on the graph it was grabbed. A level the region +/// cannot reach saturates rather than jumping. #[test] -fn a_region_level_and_its_amount_are_inverses() { +fn a_curve_level_and_its_amount_are_inverses() { let c = lightcraft_develop::ToneCurve { shadows: 20.0, lights: 40.0, ..Default::default() }; let model = ParametricCurve::new(&c); for region in 0..4 { - for amount in [-100.0, -40.0, 0.0, 30.0, 100.0] { - let level = model.handle_level(region, amount); - let back = model.amount_at_level(region, level); - assert!((back - amount).abs() <= 1.0, "region {region}: {amount} → level {level} → {back}"); + let centre = model.region_x(region); + // the region's window centre, and a point off-centre inside the same window + for x in [centre, centre + (model.split_x(region) - centre) * 0.5] { + for amount in [-100.0, -40.0, 0.0, 30.0, 100.0] { + let level = model.handle_level_at(region, amount, x); + let back = model.amount_at_level(region, level, x); + assert!((back - amount).abs() <= 1.0, "region {region} at {x}: {amount} → {level} → {back}"); + } } } - assert_eq!(model.amount_at_level(3, 1.0), 100.0, "a level above the region's reach saturates high"); - assert_eq!(model.amount_at_level(3, 0.0), -100.0, "and one below saturates low"); - // a split reads out in its own slider's range (10..=70 here), not the graph's 0..=100 … - assert_eq!(model.split_value(0, 0.0), 10.0, "the Shadows split bottoms out at its slider's low end"); - // … and its neighbours are what actually bound it, closer than that range's own ends - assert!(model.split_value(0, 1.0) <= 70.0 && model.split_value(0, 1.0) > 10.0, "kept inside its range"); - assert!(model.split_x(1) > model.split_x(0) && model.split_x(2) > model.split_x(1), "and the splits stay in order"); - assert!(model.split_value(2, 1.0) <= 90.0, "the Highlights split stays in its own range too"); + let centre = model.region_x(3); + assert_eq!(model.amount_at_level(3, 1.0, centre), 100.0, "a level above the region's reach saturates high"); + assert_eq!(model.amount_at_level(3, 0.0, centre), -100.0, "and one below saturates low"); } -/// A dragged split never crosses its neighbours, however far the pointer goes. +/// The four regions tile the graph: every position belongs to exactly one of them, in order, so a +/// drag anywhere on the line moves exactly one slider and the one the pointer is over. #[test] -fn a_dragged_split_stays_between_its_neighbours() { +fn the_four_regions_tile_the_graph() { let model = ParametricCurve::new(&lightcraft_develop::ToneCurve::default()); - assert!((model.split_x_limited(1, 0.0) - 0.26).abs() < 1e-9, "Midtones stops just right of the split before it"); - assert!((model.split_x_limited(1, 1.0) - 0.74).abs() < 1e-9, "and just left of the split after it"); - assert!(model.split_x_limited(0, -1.0).abs() < 1e-9, "the first split can reach black"); - assert!((model.split_x_limited(2, 2.0) - 1.0).abs() < 1e-9, "the last can reach white"); + assert_eq!(model.region_at(0.0), 0, "black is the darkest region"); + assert_eq!(model.region_at(1.0), 3, "white is the lightest"); + assert_eq!(model.region_at(0.2), 0, "below the first split"); + assert_eq!(model.region_at(0.3), 1, "between the first two splits"); + assert_eq!(model.region_at(0.6), 2); + assert_eq!(model.region_at(0.8), 3); + let mut prev = model.region_at(0.0); + for i in 0..=100 { + let r = model.region_at(i as f64 / 100.0); + assert!(r >= prev, "the regions never go backwards: {r} after {prev}"); + prev = r; + } } From 5eebb9c2f9eda14d4202e313fedba4b2d0480157 Mon Sep 17 00:00:00 2001 From: Paul <131645063+deunderscorepaul@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:48:43 +0200 Subject: [PATCH 4/6] Tone Curve: expose ParametricCurve from pipeline and preserve other regions on drag - Expose ParametricCurve from lightcraft-pipeline and use it in both curve_luts and the UI curve editor, eliminating duplicate implementations and ensuring render and graph never drift. - Invert the parametric model while holding the other three regions fixed, converting vertical pointer motion relative to the grabbed region's initial amount so a horizontal drag with zero vertical travel leaves all sliders untouched even with heavy overlapping neighbouring bumps (fixes AlexChambers report: Darks=60, Lights=0 grabbed at x=0.51). - Replace all raw table indexing with .get() per never-crash rules. - Add unit and headless integration tests verifying that horizontal drag with other regions non-zero changes nothing. Co-Authored-By: deepseek-flash --- crates/pipeline/src/curve.rs | 213 ++++++++++++++++++++++++++++++ crates/pipeline/src/finish.rs | 29 ++-- crates/pipeline/src/lib.rs | 2 + crates/ui-egui/src/panels/edit.rs | 187 ++++---------------------- crates/ui-egui/src/tests_curve.rs | 33 ++++- 5 files changed, 281 insertions(+), 183 deletions(-) create mode 100644 crates/pipeline/src/curve.rs diff --git a/crates/pipeline/src/curve.rs b/crates/pipeline/src/curve.rs new file mode 100644 index 000000000..d336274e0 --- /dev/null +++ b/crates/pipeline/src/curve.rs @@ -0,0 +1,213 @@ +//! The parametric tone curve model shared by the pipeline renderer and the UI curve editor. +//! +//! Lightroom-style tone curve parametric model: four regions (shadows, darks, lights, highlights) +//! separated by three split points. Each region contributes a cosine-windowed bump, summed and +//! forced monotone. + +use lightcraft_color::spline::Lut1; +use lightcraft_develop::ToneCurve; + +/// The four shape controls of the parametric curve, in graph order: region 0 is the darkest. +pub const PARAM_AMOUNTS: [&str; 4] = ["curve.shadows", "curve.darks", "curve.lights", "curve.highlights"]; + +/// Resolution the parametric curve is sampled at, matching the renderer's LUT. +pub const PARAM_LUT: usize = 1024; + +/// The parametric (region) curve of a [`ToneCurve`]: the four region amounts, each a cosine bump +/// over its window between two splits, summed and then forced monotone. +/// +/// This is the renderer's curve, not an approximation of it — both `curve_luts` in the pipeline +/// and the curve editor in the UI use this struct so they can never disagree. +#[derive(Clone, Debug)] +pub struct ParametricCurve { + /// The curve, sampled and already forced monotone. + lut: [f32; PARAM_LUT], + /// The three region boundaries as fractions of the graph (0..1). + pub splits: [f64; 3], + /// The four region amounts, in graph order (0 = shadows, 3 = highlights). + pub amounts: [f64; 4], +} + +impl ParametricCurve { + pub fn new(c: &ToneCurve) -> ParametricCurve { + let (s0, s1, s2) = ((c.split_shadows / 100.0) as f32, (c.split_mid / 100.0) as f32, (c.split_highlights / 100.0) as f32); + let (a0, a1, a2, a3) = (c.shadows, c.darks, c.lights, c.highlights); + let splits = [s0 as f64, s1 as f64, s2 as f64]; + let amounts = [a0, a1, a2, a3]; + let regions = [(0.0, s0, a0), (s0, s1, a1), (s1, s2, a2), (s2, 1.0, a3)]; + let mut lut = [0.0_f32; PARAM_LUT]; + let mut prev = 0.0_f32; + for (i, entry) in lut.iter_mut().enumerate() { + let x = i as f32 / (PARAM_LUT - 1) as f32; + let mut d = 0.0_f32; + for (a, b, amt) in regions { + if amt == 0.0 { + continue; + } + let (ctr, half) = ((a + b) / 2.0, (b - a) * 0.75 + 0.05); + let t = ((x - ctr) / half).clamp(-1.0, 1.0); + d += (amt / 100.0) as f32 * 0.22 * (0.5 + 0.5 * (t * std::f32::consts::PI).cos()); + } + let y = (x + d * 4.0 * x * (1.0 - x)).clamp(0.0, 1.0); + prev = prev.max(y); + *entry = prev; + } + ParametricCurve { lut, splits, amounts } + } + + /// Convert the evaluated parametric curve into a 1024-entry [`Lut1`]. + pub fn to_lut(&self) -> Lut1 { + Lut1 { v: self.lut.to_vec() } + } + + /// The curve at `x` (0 = black, 1 = white), read off the table (so it never overshoots the ends). + pub fn at(&self, x: f64) -> f64 { + let x = x.clamp(0.0, 1.0) as f32; + let at = x * (PARAM_LUT - 1) as f32; + let i = at.floor().clamp(0.0, (PARAM_LUT - 1) as f32) as usize; + let cur = self.lut.get(i).copied().unwrap_or(0.0); + let next = self.lut.get(i + 1).copied().unwrap_or(cur); + let f = at - i as f32; + (cur + (next - cur) * f) as f64 + } + + /// Where region `region`'s handle sits on the input axis: the middle of its own window, so a + /// handle never has to cross a split to be grabbed. + pub fn region_x(&self, region: usize) -> f64 { + let (a, b) = self.region_window(region); + (a + b) / 2.0 + } + + /// The window (two splits, as fractions of the graph) region `region` owns. + pub fn region_window(&self, region: usize) -> (f64, f64) { + let s0 = self.split_x(0); + let s1 = self.split_x(1); + let s2 = self.split_x(2); + match region { + 0 => (0.0, s0), + 1 => (s0, s1), + 2 => (s1, s2), + _ => (s2, 1.0), + } + } + + /// The half-width of region `region`'s own bump, as fraction of the graph. + pub fn region_half(&self, region: usize) -> f64 { + let (a, b) = self.region_window(region); + (b - a) * 0.75 + 0.05 + } + + /// The cosine window region `region` applies at `x` (1 at its centre, 0 at the window edges). + pub fn region_window_at(&self, region: usize, x: f64) -> f64 { + let ctr = self.region_x(region); + let half = self.region_half(region); + if half <= f64::EPSILON { + return 0.0; + } + let t = ((x - ctr) / half).clamp(-1.0, 1.0); + 0.5 + 0.5 * (t * std::f64::consts::PI).cos() + } + + /// The region that owns graph position `x`: the one whose own window contains it. The four + /// windows tile 0..1, so this is always defined and a drag anywhere on the curve moves exactly + /// one slider — the region under the pointer. + pub fn region_at(&self, x: f64) -> usize { + let x = x.clamp(0.0, 1.0); + let s0 = self.split_x(0); + let s1 = self.split_x(1); + let s2 = self.split_x(2); + if x <= s0 { + 0 + } else if x <= s1 { + 1 + } else if x <= s2 { + 2 + } else { + 3 + } + } + + /// How much output level region `region`'s bump gains at `x` per unit of its slider: the bump is + /// linear in the amount, so this is a constant for a given `x`. A drag converts the pointer's + /// vertical motion into a slider change through it. + pub fn level_per_unit(&self, region: usize, x: f64) -> f64 { + let x = x.clamp(0.0, 1.0); + 0.22 / 100.0 * self.region_window_at(region, x) * 4.0 * x * (1.0 - x) + } + + /// The baseline level at `x` from the identity diagonal plus the contributions of all regions + /// *except* `region`. Used when inverting the model for `region` while holding the other three + /// regions fixed. + pub fn base_level_without(&self, region: usize, x: f64) -> f64 { + let x = x.clamp(0.0, 1.0); + let mut d = 0.0; + for i in 0..4 { + if i == region { + continue; + } + let amt = self.amounts.get(i).copied().unwrap_or(0.0); + if amt == 0.0 { + continue; + } + d += (amt / 100.0) * 0.22 * self.region_window_at(i, x); + } + x + d * 4.0 * x * (1.0 - x) + } + + /// The curve level region `region` sits at when its slider holds `amount`, at graph position `x`, + /// holding the other regions fixed at their current settings. + pub fn handle_level_at(&self, region: usize, amount: f64, x: f64) -> f64 { + (self.base_level_without(region, x) + amount * self.level_per_unit(region, x)).clamp(0.0, 1.0) + } + + /// The amount that puts region `region`'s curve at `level` at `x`, with the other three + /// regions held fixed — the inverse of [`Self::handle_level_at`]. A level the region cannot + /// reach saturates rather than jumping. + pub fn amount_at_level(&self, region: usize, level: f64, x: f64) -> f64 { + let per_unit = self.level_per_unit(region, x); + if per_unit.abs() <= f64::EPSILON { + return self.amounts.get(region).copied().unwrap_or(0.0); + } + let base = self.base_level_without(region, x); + ((level.clamp(0.0, 1.0) - base) / per_unit).clamp(-100.0, 100.0).round() + } + + /// The graph position split `split` (0..3) sits at, 0..1. + pub fn split_x(&self, split: usize) -> f64 { + self.splits.get(split).copied().unwrap_or(0.5) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn horizontal_inversion_at_x_0_51_with_darks_60_preserves_zero_lights() { + let c = ToneCurve { darks: 60.0, lights: 0.0, ..Default::default() }; + let model = ParametricCurve::new(&c); + assert_eq!(model.region_at(0.51), 2, "x=0.51 is inside the Lights region"); + let level = model.at(0.51); + assert!((level - 0.5619).abs() < 0.001, "extracted model at 0.51 is ~0.5619: {level}"); + // With other regions held fixed, inverting at level ~0.5619 must return Lights = 0, NOT 45! + let lights = model.amount_at_level(2, level, 0.51); + assert_eq!(lights, 0.0, "zero vertical delta preserves Lights=0: {lights}"); + } + + #[test] + fn a_curve_level_and_its_amount_are_inverses_with_other_regions_present() { + let c = ToneCurve { shadows: 20.0, darks: -30.0, lights: 40.0, highlights: -10.0, ..Default::default() }; + let model = ParametricCurve::new(&c); + for region in 0..4 { + let centre = model.region_x(region); + let (_, upper) = model.region_window(region); + for x in [centre, centre + (upper - centre) * 0.5] { + for amount in [-100.0, -40.0, 0.0, 30.0, 100.0] { + let level = model.handle_level_at(region, amount, x); + let back = model.amount_at_level(region, level, x); + assert!((back - amount).abs() <= 1.0, "region {region} at {x}: {amount} -> {level} -> {back}"); + } + } + } + } +} diff --git a/crates/pipeline/src/finish.rs b/crates/pipeline/src/finish.rs index eb65d51a2..ea1736359 100644 --- a/crates/pipeline/src/finish.rs +++ b/crates/pipeline/src/finish.rs @@ -29,31 +29,20 @@ fn curve_luts(c: &ToneCurve) -> Option<[Lut1; 3]> { return None; } const N: usize = 1024; - let (s1, s2, s3) = ((c.split_shadows / 100.0) as f32, (c.split_mid / 100.0) as f32, (c.split_highlights / 100.0) as f32); - let regions = [(0.0, s1, c.shadows), (s1, s2, c.darks), (s2, s3, c.lights), (s3, 1.0, c.highlights)]; - let mut base = Lut1::from_fn(N, |x| { - let mut d = 0.0; - for (a, b, amt) in regions { - if amt == 0.0 { - continue; - } - let (ctr, half) = ((a + b) / 2.0, (b - a) * 0.75 + 0.05); - let t = ((x - ctr) / half).clamp(-1.0, 1.0); - let win = 0.5 + 0.5 * (t * std::f32::consts::PI).cos(); - d += (amt / 100.0) as f32 * 0.22 * win; - } - (x + d * 4.0 * x * (1.0 - x)).clamp(0.0, 1.0) - }); - // keep monotone - for i in 1..N { - base.v[i] = base.v[i].max(base.v[i - 1]); - } + let mut base = crate::curve::ParametricCurve::new(c).to_lut(); let to_pts = |p: &[Point]| p.iter().map(|q| (q.x, q.y)).collect::>(); if master { base = MonotoneCurve::new(&to_pts(&c.master)).to_lut(N).compose(&base); } let per = [&c.red, &c.green, &c.blue]; - Some(std::array::from_fn(|i| if chans[i] { MonotoneCurve::new(&to_pts(per[i])).to_lut(N).compose(&base) } else { base.clone() })) + Some(std::array::from_fn(|i| { + if chans.get(i).copied().unwrap_or(false) { + let pts = per.get(i).map(|&p| to_pts(p)).unwrap_or_default(); + MonotoneCurve::new(&pts).to_lut(N).compose(&base) + } else { + base.clone() + } + })) } /// Vignette (post-crop) parameters. diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index 34df1f8a9..f6b0d995b 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -27,6 +27,7 @@ pub mod auto; pub mod blemish; pub mod colorops; pub mod cull; +pub mod curve; pub mod dust; pub mod finish; pub mod geometry; @@ -44,6 +45,7 @@ pub mod transform; pub mod upright; pub mod visualize; +pub use curve::{PARAM_AMOUNTS, PARAM_LUT, ParametricCurve}; pub use output::{DeepImage, DeepSamples, DisplaySpace, OutputDepth, OutputSpace, OutputTrc, Proof, ProofLut, proof_profile, register_proof_profile}; pub use visualize::{MaskView, Overlay}; diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index b465c544e..a1b6d362f 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -4,8 +4,9 @@ use egui::epaint::{Mesh, Vertex}; use egui::{Align2, Color32, Pos2, Rect, RichText, Sense, Stroke, pos2, vec2}; use lightcraft_catalog::PhotoId; -use lightcraft_develop::{ControlSpec, DevelopSettings, Section, ToneCurve, Track, WbMode, controls}; +use lightcraft_develop::{ControlSpec, DevelopSettings, Section, Track, WbMode, controls}; use lightcraft_geom::Point; +use lightcraft_pipeline::{PARAM_AMOUNTS, ParametricCurve}; use lightcraft_raster::Histogram; use serde_json::{Value, json}; @@ -929,149 +930,9 @@ fn draw_curve_histogram(p: &egui::Painter, r: Rect, h: &lightcraft_raster::Histo p.add(mesh); } -/// The four shape controls of the parametric curve, in graph order: region 0 is the darkest. -pub(crate) const PARAM_AMOUNTS: [&str; 4] = ["curve.shadows", "curve.darks", "curve.lights", "curve.highlights"]; -/// Resolution the parametric curve is sampled at, matching `lightcraft_pipeline::finish`'s table. -const PARAM_LUT: usize = 1024; /// Segments the graph's curve line is drawn with. pub(crate) const CURVE_SAMPLES: i32 = 96; -/// The parametric (region) curve of a [`ToneCurve`]: the four region amounts, each a cosine bump -/// over its window between two splits, summed and then forced monotone. -/// -/// This is the renderer's curve, not an approximation of it — `lightcraft_pipeline::finish::curve_luts` -/// builds the same shape (its own table, whose `x` is the encoded level the curve sees). The graph -/// draws this curve, so the line and the sliders can never disagree: both come from the settings, -/// recomputed every frame, with no state of their own. -/// -/// The running maximum is what makes the curve monotone. Two neighbouring regions pushed opposite -/// ways can otherwise fold it back on itself, and a fold is exactly what a drawn line used to show -/// while the renderer straightened it out. -pub(crate) struct ParametricCurve { - /// The curve, sampled and already forced monotone. - lut: [f32; PARAM_LUT], - /// The three region boundaries as fractions of the graph. - pub(crate) splits: [f64; 3], - /// The four region amounts, in graph order (0 = the darkest). - pub(crate) amounts: [f64; 4], -} - -impl ParametricCurve { - pub(crate) fn new(c: &ToneCurve) -> ParametricCurve { - let splits = [c.split_shadows / 100.0, c.split_mid / 100.0, c.split_highlights / 100.0]; - let amounts = [c.shadows, c.darks, c.lights, c.highlights]; - let s = [splits[0] as f32, splits[1] as f32, splits[2] as f32]; - let regions = [(0.0, s[0], amounts[0]), (s[0], s[1], amounts[1]), (s[1], s[2], amounts[2]), (s[2], 1.0, amounts[3])]; - let mut lut = [0.0_f32; PARAM_LUT]; - let mut prev = 0.0_f32; - for i in 0..PARAM_LUT { - let x = i as f32 / (PARAM_LUT - 1) as f32; - let mut d = 0.0_f32; - for (a, b, amt) in regions { - if amt == 0.0 { - continue; - } - let (ctr, half) = ((a + b) / 2.0, (b - a) * 0.75 + 0.05); - let t = ((x - ctr) / half).clamp(-1.0, 1.0); - d += (amt / 100.0) as f32 * 0.22 * (0.5 + 0.5 * (t * std::f32::consts::PI).cos()); - } - let y = (x + d * 4.0 * x * (1.0 - x)).clamp(0.0, 1.0); - // the running maximum: the renderer's own monotone pass - prev = prev.max(y); - lut[i] = prev; - } - ParametricCurve { lut, splits, amounts } - } - - /// The curve at `x` (0 = black, 1 = white), read off the table (so it never overshoots the ends). - pub(crate) fn at(&self, x: f64) -> f64 { - let x = x.clamp(0.0, 1.0) as f32; - let at = x * (PARAM_LUT - 1) as f32; - let i = at.floor().clamp(0.0, (PARAM_LUT - 1) as f32) as usize; - let next = self.lut.get(i + 1).copied().unwrap_or(self.lut[i]); - let f = at - i as f32; - (self.lut[i] + (next - self.lut[i]) * f) as f64 - } - - /// Where region `region`'s handle sits on the input axis: the middle of its own window, so a - /// handle never has to cross a split to be grabbed. - pub(crate) fn region_x(&self, region: usize) -> f64 { - let (a, b) = self.region_window(region); - (a + b) / 2.0 - } - - /// The window (two splits, as fractions of the graph) region `region` owns. - fn region_window(&self, region: usize) -> (f64, f64) { - let s = [self.splits[0], self.splits[1], self.splits[2]]; - match region { - 0 => (0.0, s[0]), - 1 => (s[0], s[1]), - 2 => (s[1], s[2]), - _ => (s[2], 1.0), - } - } - - /// The half-width of region `region`'s own bump, as fraction of the graph. - fn region_half(&self, region: usize) -> f64 { - let (a, b) = self.region_window(region); - (b - a) * 0.75 + 0.05 - } - - /// The cosine window region `region` applies at `x` (1 at its centre, 0 at the window edges). - fn region_window_at(&self, region: usize, x: f64) -> f64 { - let ctr = self.region_x(region); - let t = ((x - ctr) / self.region_half(region)).clamp(-1.0, 1.0); - 0.5 + 0.5 * (t * std::f64::consts::PI).cos() - } - - /// The region that owns graph position `x`: the one whose own window contains it. The four - /// windows tile 0..1, so this is always defined and a drag anywhere on the curve moves exactly - /// one slider — the region under the pointer. - pub(crate) fn region_at(&self, x: f64) -> usize { - let x = x.clamp(0.0, 1.0); - // the bands are black..split0, split0..split1, split1..split2, split2..white - let (s0, s1, s2) = (self.split_x(0), self.split_x(1), self.split_x(2)); - if x <= s0 { - 0 - } else if x <= s1 { - 1 - } else if x <= s2 { - 2 - } else { - 3 - } - } - - /// How much output level region `region`'s bump gains at `x` per unit of its slider: the bump is - /// linear in the amount, so this is a constant for a given `x`. A drag converts the pointer's - /// vertical motion into a slider change through it. - pub(crate) fn level_per_unit(&self, region: usize, x: f64) -> f64 { - let x = x.clamp(0.0, 1.0); - 0.22 / 100.0 * self.region_window_at(region, x) * 4.0 * x * (1.0 - x) - } - - /// The curve level region `region` sits at when its slider holds `amount`, at graph position `x` - /// — where the curve is at the point a drag grabbed, which is what that drag starts from. - pub(crate) fn handle_level_at(&self, region: usize, amount: f64, x: f64) -> f64 { - (x.clamp(0.0, 1.0) + amount * self.level_per_unit(region, x)).clamp(0.0, 1.0) - } - - /// The amount that puts region `region`'s curve at `level` at `x` — the inverse of - /// [`Self::handle_level_at`]. A level the region cannot reach saturates rather than jumping. - pub(crate) fn amount_at_level(&self, region: usize, level: f64, x: f64) -> f64 { - let per_unit = self.level_per_unit(region, x); - if per_unit.abs() <= f64::EPSILON { - return 0.0; - } - ((level.clamp(0.0, 1.0) - x.clamp(0.0, 1.0)) / per_unit).clamp(-100.0, 100.0).round() - } - - /// The graph position split `split` (0..3) sits at, 0..1. - pub(crate) fn split_x(&self, split: usize) -> f64 { - self.splits.get(split).copied().unwrap_or(0.5) - } -} - /// Close a graph drag's `develop` interaction, as one undo step. /// /// It is closed when the drag ends *or* the pointer comes up without egui reporting `drag_stopped` @@ -1108,9 +969,9 @@ fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui:: }; let drag_id = egui::Id::new("curve-param-drag"); let cursor_id = egui::Id::new("curve-param-cursor"); - // the region being dragged, and the graph x the drag is anchored at (stored as an `Option` so a - // stale value can be cleared, as the point-curve editor's drag state is) - let mut dragged: Option<(usize, f64)> = ui.data(|dd| dd.get_temp::>(drag_id)).flatten(); + // (region, x, initial_amount, start_y): the region being dragged, the graph x the drag is anchored + // at, the region's starting amount, and the starting screen Y position. + let mut dragged: Option<(usize, f64, f64, f32)> = ui.data(|dd| dd.get_temp::>(drag_id)).flatten(); let hovered = dragged.is_none() && resp.hover_pos().is_some_and(|q| near_curve(q) <= PARAM_GRAB); let mut set: Option<(&'static str, f64)> = None; // the press, not where the pointer is once the drag threshold has passed: a press slightly off @@ -1122,10 +983,13 @@ fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui:: { let x = from_screen_x(q); let _ = app.run("develop.beginInteraction", json!({"label": "Parametric Curve"})); - dragged = Some((model.region_at(x), x)); - ui.data_mut(|dd| dd.insert_temp(cursor_id, to_screen(x, model.at(x)))); + let region = model.region_at(x); + let initial_amount = model.amounts.get(region).copied().unwrap_or(0.0); + let start_pos = to_screen(x, model.at(x)); + dragged = Some((region, x, initial_amount, start_pos.y)); + ui.data_mut(|dd| dd.insert_temp(cursor_id, start_pos)); } - if let Some((region, x)) = dragged + if let Some((region, x, initial_amount, start_y)) = dragged && resp.dragged() { // Follow the pointer's motion from where the curve was grabbed, so the press itself moves @@ -1133,11 +997,16 @@ fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui:: let alt = ui.input(|i| i.modifiers.alt); let dd = ui.input(|i| i.pointer.delta()); let step = if alt { vec2(dd.x * 0.1, dd.y * 0.1) } else { vec2(dd.x, dd.y) }; - let start = to_screen(x, model.handle_level_at(region, model.amounts[region], x)); - let cursor = ui.data(|d| d.get_temp::(cursor_id)).unwrap_or(start) + step; + let cursor = ui.data(|d| d.get_temp::(cursor_id)).unwrap_or(pos2(0.0, start_y)) + step; ui.data_mut(|d| d.insert_temp(cursor_id, cursor)); - let level = ((r.bottom() - cursor.y) / r.height()).clamp(0.0, 1.0) as f64; - set = Some((PARAM_AMOUNTS[region], model.amount_at_level(region, level, x))); + let delta_y = start_y - cursor.y; + let delta_level = (delta_y / r.height()) as f64; + let per_unit = model.level_per_unit(region, x); + let new_amount = + if per_unit.abs() > f64::EPSILON { (initial_amount + delta_level / per_unit).clamp(-100.0, 100.0).round() } else { initial_amount }; + if let Some(&id) = PARAM_AMOUNTS.get(region) { + set = Some((id, new_amount)); + } } if let Some((id, v)) = set { let _ = app.run("develop.set", json!({"control": id, "value": v})); @@ -1147,7 +1016,7 @@ fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui:: dragged = None; ui.data_mut(|d| { d.remove::(cursor_id); - d.remove::>(drag_id); + d.remove::>(drag_id); }); } ui.data_mut(|dd| dd.insert_temp(drag_id, dragged)); @@ -1157,13 +1026,13 @@ fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui:: ui.ctx().set_cursor_icon(egui::CursorIcon::Grab); } // the region under the pointer (or the one in hand), named as its slider below reads it - let shown = dragged.map(|(region, _)| region).or_else(|| hovered.then(|| resp.hover_pos()).flatten().map(|q| model.region_at(from_screen_x(q)))); - if let Some(region) = shown { - let id = PARAM_AMOUNTS[region]; - if let Some(v) = controls::get(d, id) { - let name = controls::find(id).map_or("", |s| s.label); - ui.painter().text(r.left_top() + vec2(6.0, 4.0), Align2::LEFT_TOP, format!("{name} {v:.0}"), t.font(11.0), t.text_dim); - } + let shown = dragged.map(|(region, ..)| region).or_else(|| hovered.then(|| resp.hover_pos()).flatten().map(|q| model.region_at(from_screen_x(q)))); + if let Some(region) = shown + && let Some(&id) = PARAM_AMOUNTS.get(region) + && let Some(v) = controls::get(d, id) + { + let name = controls::find(id).map_or("", |s| s.label); + ui.painter().text(r.left_top() + vec2(6.0, 4.0), Align2::LEFT_TOP, format!("{name} {v:.0}"), t.font(11.0), t.text_dim); } } diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs index 7dd86e512..e9bbc9c92 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -5,8 +5,8 @@ use std::time::Duration; use serde_json::json; use crate::headless::Headless; -use crate::panels::edit::ParametricCurve; use crate::{LightcraftApp, Services}; +use lightcraft_pipeline::ParametricCurve; const T: Duration = Duration::from_secs(20); @@ -247,7 +247,7 @@ fn the_parametric_model_is_monotone_where_the_bare_sum_of_bumps_is_not() { } x + d * 4.0 * x * (1.0 - x) }; - assert!((0..=64).any(|i| raw(i as f64 / 64.0) < raw((i + 1) as f64 / 64.0)), "the shape folds back, or it proves nothing"); + assert!((0..=64).any(|i| raw(i as f64 / 64.0) > raw((i + 1) as f64 / 64.0)), "the shape folds back, or it proves nothing"); let model = ParametricCurve::new(&c); let mut prev = f64::MIN; for i in 0..=256 { @@ -426,12 +426,36 @@ fn dragging_the_curve_moves_the_region_slider() { // the graph really is redrawn below where the line was let graph = curve_graph_rect(&mut h); let painted = painted_param_curve(&h, graph)(0.625); - assert!(painted < c.lights / 100.0 + 0.625, "the painted line followed the drag down: {painted}"); + let before_level = ParametricCurve::new(&c).at(0.625); + assert!(painted < before_level - 0.02, "the painted line followed the drag down: {painted} vs before {before_level}"); // and Undo takes the whole gesture back to where it started exec(&mut h, "edit.undo", json!({})); assert_eq!(develop(&h).curve, c, "undo restores the value the drag started from"); } +/// A horizontal drag on the curve (zero vertical motion) changes no region amount, even when +/// neighbouring regions contribute heavily to the curve at the grabbed position (AlexChambers report: +/// Darks=60, Lights=0 grabbed at x=0.51 used to jump Lights to 45 with zero vertical motion). +#[test] +fn a_horizontal_drag_with_zero_vertical_motion_changes_nothing() { + let mut h = curve_open("parametric"); + exec(&mut h, "develop.set", json!({"control": "curve.darks", "value": 60})); + h.step(); + let before = develop(&h).curve; + assert_eq!(before.darks, 60.0); + assert_eq!(before.lights, 0.0); + let r = curve_graph_rect(&mut h); + let [x, y] = on_curve(r, &before, 0.51); + // Drag purely horizontally from x=0.51: dx = 20 px, dy = 0 + let d = h.request("ui.drag", json!({"x": x, "y": y, "toX": x + 20.0, "toY": y, "steps": 6}), T); + assert_eq!(d["ok"], true, "{d}"); + h.step(); + let after = develop(&h).curve; + assert_eq!(after.lights, 0.0, "horizontal motion preserves Lights=0: {}", after.lights); + assert_eq!(after.darks, 60.0, "Darks is preserved: {}", after.darks); + assert_eq!(after, before, "horizontal drag changes nothing: {after:?}"); +} + /// Dragging up bends the region the other way, so the gesture is not just "downwards works". #[test] fn dragging_the_curve_up_raises_the_region_slider() { @@ -529,8 +553,9 @@ fn a_curve_level_and_its_amount_are_inverses() { let model = ParametricCurve::new(&c); for region in 0..4 { let centre = model.region_x(region); + let (_, upper) = model.region_window(region); // the region's window centre, and a point off-centre inside the same window - for x in [centre, centre + (model.split_x(region) - centre) * 0.5] { + for x in [centre, centre + (upper - centre) * 0.5] { for amount in [-100.0, -40.0, 0.0, 30.0, 100.0] { let level = model.handle_level_at(region, amount, x); let back = model.amount_at_level(region, level, x); From 9c7b4f221927e4ae1822ed1e6e6e1dd4f9429b81 Mon Sep 17 00:00:00 2001 From: Paul <131645063+deunderscorepaul@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:32:38 +0200 Subject: [PATCH 5/6] Tone Curve: restore slider order to Lightroom layout and guard parametric curve against hostile inputs - Reverse slider loop order in UI to match Lightroom layout (Highlights, Lights, Darks, Shadows). - Sanitize splits and amounts against NaN, inf, and inverted split bounds in ParametricCurve::new, at, and region_at. - Guard param_drag against zero/negative widget dimensions. - Update live region readout to display the current frame's dragged value immediately. - Add regression test for non-finite and inverted split values. Co-Authored-By: deepseek-flash --- crates/pipeline/src/curve.rs | 35 ++++++++++++++++++++++++++----- crates/ui-egui/src/panels/edit.rs | 7 +++++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/crates/pipeline/src/curve.rs b/crates/pipeline/src/curve.rs index d336274e0..b2c2b0b14 100644 --- a/crates/pipeline/src/curve.rs +++ b/crates/pipeline/src/curve.rs @@ -30,8 +30,11 @@ pub struct ParametricCurve { impl ParametricCurve { pub fn new(c: &ToneCurve) -> ParametricCurve { - let (s0, s1, s2) = ((c.split_shadows / 100.0) as f32, (c.split_mid / 100.0) as f32, (c.split_highlights / 100.0) as f32); - let (a0, a1, a2, a3) = (c.shadows, c.darks, c.lights, c.highlights); + let s0 = if c.split_shadows.is_nan() { 0.25 } else { (c.split_shadows / 100.0).clamp(0.0, 1.0) as f32 }; + let s1 = if c.split_mid.is_nan() { 0.50_f32.max(s0) } else { (c.split_mid / 100.0).clamp(s0 as f64, 1.0) as f32 }; + let s2 = if c.split_highlights.is_nan() { 0.75_f32.max(s1) } else { (c.split_highlights / 100.0).clamp(s1 as f64, 1.0) as f32 }; + let clean_amt = |a: f64| if a.is_nan() { 0.0 } else { a.clamp(-100.0, 100.0) }; + let (a0, a1, a2, a3) = (clean_amt(c.shadows), clean_amt(c.darks), clean_amt(c.lights), clean_amt(c.highlights)); let splits = [s0 as f64, s1 as f64, s2 as f64]; let amounts = [a0, a1, a2, a3]; let regions = [(0.0, s0, a0), (s0, s1, a1), (s1, s2, a2), (s2, 1.0, a3)]; @@ -45,6 +48,9 @@ impl ParametricCurve { continue; } let (ctr, half) = ((a + b) / 2.0, (b - a) * 0.75 + 0.05); + if half <= f32::EPSILON { + continue; + } let t = ((x - ctr) / half).clamp(-1.0, 1.0); d += (amt / 100.0) as f32 * 0.22 * (0.5 + 0.5 * (t * std::f32::consts::PI).cos()); } @@ -62,7 +68,7 @@ impl ParametricCurve { /// The curve at `x` (0 = black, 1 = white), read off the table (so it never overshoots the ends). pub fn at(&self, x: f64) -> f64 { - let x = x.clamp(0.0, 1.0) as f32; + let x = if x.is_nan() { 0.0 } else { x.clamp(0.0, 1.0) as f32 }; let at = x * (PARAM_LUT - 1) as f32; let i = at.floor().clamp(0.0, (PARAM_LUT - 1) as f32) as usize; let cur = self.lut.get(i).copied().unwrap_or(0.0); @@ -112,7 +118,7 @@ impl ParametricCurve { /// windows tile 0..1, so this is always defined and a drag anywhere on the curve moves exactly /// one slider — the region under the pointer. pub fn region_at(&self, x: f64) -> usize { - let x = x.clamp(0.0, 1.0); + let x = if x.is_nan() { 0.0 } else { x.clamp(0.0, 1.0) }; let s0 = self.split_x(0); let s1 = self.split_x(1); let s2 = self.split_x(2); @@ -172,7 +178,7 @@ impl ParametricCurve { ((level.clamp(0.0, 1.0) - base) / per_unit).clamp(-100.0, 100.0).round() } - /// The graph position split `split` (0..3) sits at, 0..1. + /// The graph position split `split` (0..=2) sits at, 0..1. pub fn split_x(&self, split: usize) -> f64 { self.splits.get(split).copied().unwrap_or(0.5) } @@ -182,6 +188,25 @@ impl ParametricCurve { mod tests { use super::*; + #[test] + fn non_finite_or_inverted_splits_do_not_panic_or_poison_curve() { + let c = ToneCurve { + split_shadows: 80.0, + split_mid: 20.0, + split_highlights: f64::NAN, + shadows: f64::NAN, + lights: f64::INFINITY, + ..Default::default() + }; + let model = ParametricCurve::new(&c); + assert!(model.at(0.5).is_finite()); + assert!(model.at(f64::NAN).is_finite()); + assert!(model.region_at(f64::NAN) < 4); + assert!((model.splits[0] - 0.8).abs() < 1e-6); + assert!((model.splits[1] - 0.8).abs() < 1e-6); + assert!((model.splits[2] - 0.8).abs() < 1e-6); + } + #[test] fn horizontal_inversion_at_x_0_51_with_darks_60_preserves_zero_lights() { let c = ToneCurve { darks: 60.0, lights: 0.0, ..Default::default() }; diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index a1b6d362f..13471bd36 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -958,6 +958,9 @@ const PARAM_GRAB: f32 = 14.0; /// gesture back, not the last frame of it), and every write goes through `develop.set`, so the /// slider, the curve and what is drawn are the same state rather than three things kept in step. fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui::Response, d: &DevelopSettings) { + if r.width() <= 1.0 || r.height() <= 1.0 { + return; + } let t = Tokens::get(ui.ctx()); let model = ParametricCurve::new(&d.curve); let to_screen = |x: f64, y: f64| pos2(r.left() + x as f32 * r.width(), r.bottom() - y as f32 * r.height()); @@ -1029,7 +1032,7 @@ fn param_drag(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect, resp: &egui:: let shown = dragged.map(|(region, ..)| region).or_else(|| hovered.then(|| resp.hover_pos()).flatten().map(|q| model.region_at(from_screen_x(q)))); if let Some(region) = shown && let Some(&id) = PARAM_AMOUNTS.get(region) - && let Some(v) = controls::get(d, id) + && let Some(v) = set.and_then(|(s_id, v)| (s_id == id).then_some(v)).or_else(|| controls::get(d, id)) { let name = controls::find(id).map_or("", |s| s.label); ui.painter().text(r.left_top() + vec2(6.0, 4.0), Align2::LEFT_TOP, format!("{name} {v:.0}"), t.font(11.0), t.text_dim); @@ -1122,7 +1125,7 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev p.add(egui::Shape::line(pts, Stroke::new(2.0, Color32::from_gray(220)))); param_drag(app, ui, r, &resp, d); curve_footer(app, ui, d); - for c in PARAM_AMOUNTS { + for &c in PARAM_AMOUNTS.iter().rev() { control(app, ui, d, c, true); } control(app, ui, d, "curve.refineSaturation", true); From c19e1bd095962081a9a0e04204a7574b36204298 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 24 Oct 2026 10:19:51 -0400 Subject: [PATCH 6/6] Tone Curve: What's New and parity row for the draggable parametric graph; the hover test compares with a frame away from the line Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests_curve.rs | 5 ++++- docs/parity-checklist.md | 2 +- docs/whats-new.md | 7 +++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs index e9bbc9c92..a658ee9a8 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -536,12 +536,15 @@ fn hovering_the_line_names_the_region_it_would_move() { assert_eq!(q["ok"], true, "{q}"); h.step(); assert_eq!(h.last_cursor, egui::CursorIcon::Grab, "the line offers a grab: {:?}", h.last_cursor); - assert!(h.painted_text().iter().any(|t| t.contains("Lights")), "and names the slider it would move: {:?}", h.painted_text()); + // the slider rows also paint "Lights", so compare with a frame away from the line + let lights = |h: &Headless| h.painted_text().iter().filter(|t| t.contains("Lights")).count(); + let hovered = lights(&h); // well above the line there is no grab and no name let q = h.request("ui.move", json!({"x": x, "y": r[1] + 0.06 * r[3]}), T); assert_eq!(q["ok"], true, "{q}"); h.step(); assert_eq!(h.last_cursor, egui::CursorIcon::Default, "away from the line there is nothing to grab: {:?}", h.last_cursor); + assert!(hovered > lights(&h), "hovering names the slider it would move: {hovered} vs {} away", lights(&h)); } /// A point's level on the region's curve and its slider amount are inverses, so a drag puts the line diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 9790523e1..62c2d5b9a 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -184,7 +184,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-LIGHT-SHADOWS | Shadows | P0 | ✅ | `ctl:light.shadows` | From process V2, the same base as Highlights: dark areas keep their local contrast when lifted (#632) | | 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-PARAM | Parametric curve | P0 | ✅ | `ctl:curve.highlights`, `ctl:curve.lights`, `ctl:curve.darks`, `ctl:curve.shadows`, `ctl:curve.split*`, `crates/pipeline/src/curve.rs` | the graph draws the renderer's own curve model; dragging the line moves the region under the pointer (one undo step) | | LR-EDIT-LIGHT-CURVE-POINT | Point curve | P0 | ✅ | `cmd:develop.curve`, `cmd:curve.reset`, `cmd:curve.presets`, `cmd:curve.applyPreset`, `cmd:curve.savePreset`, `cmd:curve.deletePreset`, `cmd:curve.importPresets`, `cmd:curve.exportPresets`, `cmd:file.importCurvePresets`, `cmd:file.exportCurvePresets`, `crates/engine/src/cmd/curves.rs`, `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 moves it a tenth as far for fine adjustment, issue #1363); drag empty space to add and drag; double-click removes; reset: double-click a channel selector (that channel), the Reset button under the graph (every curve incl. parametric) or right-click the graph (channel / all); Point Curve preset dropdown: own Linear / Medium Contrast / Strong Contrast, user presets (save the current point curves, delete by right-click; saved with the library), import / export as `.lccurve` JSON | | 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` | | diff --git a/docs/whats-new.md b/docs/whats-new.md index c09dddc09..156378876 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-24 + +### Drag the parametric Tone Curve +- In the Tone Curve's parametric mode the graph is now drawn from the renderer's own curve, so it always matches the + photo, and the line can be dragged: pressing it picks the region under the pointer (Highlights, Lights, Darks or + Shadows) and moving up or down changes that slider, as one undo step. Hovering the line names the region first. + ## 2026-10-23 ### Tab leaves the name field in New Album, New Smart Album and the name prompts