diff --git a/crates/pipeline/src/curve.rs b/crates/pipeline/src/curve.rs new file mode 100644 index 000000000..b2c2b0b14 --- /dev/null +++ b/crates/pipeline/src/curve.rs @@ -0,0 +1,238 @@ +//! 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 = 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)]; + 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); + 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()); + } + 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 = 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); + 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 = 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); + 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..=2) 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 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() }; + 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 b61b6a62c..c1e6a9bc2 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 57cec33ec..8f2702760 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 b37c52604..2063580ae 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -6,6 +6,7 @@ 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_geom::Point; +use lightcraft_pipeline::{PARAM_AMOUNTS, ParametricCurve}; use lightcraft_raster::Histogram; use serde_json::{Value, json}; @@ -937,6 +938,115 @@ fn draw_curve_histogram(p: &egui::Painter, r: Rect, h: &lightcraft_raster::Histo p.add(mesh); } +/// Segments the graph's curve line is drawn with. +pub(crate) const CURVE_SAMPLES: i32 = 96; + +/// 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 +} + +/// 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) { + 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()); + 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"); + // (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 + // 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(q) = press + && near_curve(q) <= PARAM_GRAB + { + let x = from_screen_x(q); + let _ = app.run("develop.beginInteraction", json!({"label": "Parametric Curve"})); + 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, initial_amount, start_y)) = dragged + && resp.dragged() + { + // 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 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 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})); + } + let dropped = resp.drag_stopped(); + if commit_interaction(app, ui, dragged.is_some(), dropped) { + dragged = None; + 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 { + 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 Some(&id) = PARAM_AMOUNTS.get(region) + && 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); + } +} + fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &DevelopSettings) { let t = Tokens::get(ui.ctx()); // channel selector @@ -1013,29 +1123,17 @@ 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)))); + param_drag(app, ui, r, &resp, d); curve_footer(app, ui, d); - for c in ["curve.highlights", "curve.lights", "curve.darks", "curve.shadows"] { + for &c in PARAM_AMOUNTS.iter().rev() { control(app, ui, d, c, true); } control(app, ui, d, "curve.refineSaturation", true); @@ -1167,9 +1265,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 4d3b45702..a658ee9a8 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -6,6 +6,7 @@ use serde_json::json; use crate::headless::Headless; use crate::{LightcraftApp, Services}; +use lightcraft_pipeline::ParametricCurve; const T: Duration = Duration::from_secs(20); @@ -221,3 +222,370 @@ 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"); +} + +/// 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]] +} + +/// 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_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(); + // 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 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 really is redrawn below where the line was + let graph = curve_graph_rect(&mut h); + let painted = painted_param_curve(&h, graph)(0.625); + 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() { + let mut h = curve_open("parametric"); + let r = curve_graph_rect(&mut h); + let c = develop(&h).curve; + // 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 line up: {}", after.darks); + assert_eq!((after.shadows, after.lights, after.highlights), (0.0, 0.0, 0.0), "the other regions stay put: {after:?}"); + assert!(ParametricCurve::new(&after).at(0.375) > ParametricCurve::new(&c).at(0.375), "the curve is drawn bent up at the Darks region"); +} + +/// 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 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 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_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 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}), + 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"); +} + +/// 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 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 [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_eq!(h.last_cursor, egui::CursorIcon::Grab, "the line offers a grab: {:?}", h.last_cursor); + // 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 +/// 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_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 { + 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 + (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}"); + } + } + } + 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"); +} + +/// 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 the_four_regions_tile_the_graph() { + let model = ParametricCurve::new(&lightcraft_develop::ToneCurve::default()); + 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; + } +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 322d26530..3e453a15b 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 de37d2635..c6c259200 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -14,6 +14,11 @@ sideways moves that control — the zone you pressed stays locked while the pointer crosses the others, Shift drags a tenth as far, and the whole drag is one undo step. Double-clicking a zone puts its control back to its default. +### 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