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] 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);