diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs index 1ae72e48e..6d795aacb 100644 --- a/crates/engine/src/cmd/develop.rs +++ b/crates/engine/src/cmd/develop.rs @@ -497,13 +497,15 @@ pub fn specs() -> Vec { }), cmd!("develop.curve", "Set Point Curve", [], None, "{channel: master|red|green|blue, points: [[x,y],...] in 0..1}", has_active, |s, p| { let ch = str_param(p, "channel").unwrap_or("master").to_string(); - let pts: Vec = p + let mut pts: Vec = p .get("points") .and_then(Value::as_array) .ok_or_else(|| bad("develop.curve", "missing points"))? .iter() .filter_map(|q| Some(Point::new(q.get(0)?.as_f64()?.clamp(0.0, 1.0), q.get(1)?.as_f64()?.clamp(0.0, 1.0)))) .collect(); + // stored in the order the curve runs, as the editor draws and drags them + pts.sort_by(|a, b| a.x.total_cmp(&b.x)); edit(s, "develop.curve", "Tone Curve", |d| { match ch.as_str() { "master" | "rgb" => d.curve.master = pts, diff --git a/crates/engine/src/crs.rs b/crates/engine/src/crs.rs index b7488a023..9de8c3d64 100644 --- a/crates/engine/src/crs.rs +++ b/crates/engine/src/crs.rs @@ -173,7 +173,8 @@ fn curve(props: &Props, k: &str) -> Option { let mut pts = Vec::new(); for it in items { let (x, y) = it.split_once(',')?; - let (x, y) = (x.trim().parse::().ok()?, y.trim().parse::().ok()?); + // (`parse` reads "NaN" and "inf" too: a curve with such a point is not a curve) + let (x, y) = (x.trim().parse::().ok().filter(|v| v.is_finite())?, y.trim().parse::().ok().filter(|v| v.is_finite())?); pts.push(json!({"x": (x / 255.0).clamp(0.0, 1.0), "y": (y / 255.0).clamp(0.0, 1.0)})); } if pts.len() < 2 { @@ -617,6 +618,19 @@ mod tests { assert_eq!((c.rect.x0, c.rect.y0, c.rect.x1, c.rect.y1, c.angle), (0.05, 0.1, 0.8, 0.9, 1.5)); } + /// A sidecar curve with a `NaN` or `inf` point (Rust's `parse` reads both) is not a curve: + /// it is left out, rather than stored with a point the curve editor can't place. + #[test] + fn curves_with_non_finite_points_are_left_out() { + for bad in ["NaN, 0", "64, inf", "-inf, 3"] { + let x = SIDECAR.replace("64, 52", &format!("{bad}")); + let partial = to_partial(&props(&x), Target::RawAbsolute); + let s = apply_partial(&DevelopSettings::for_raw(5000.0, 0.0), &partial, 1.0); + assert!(s.curve.master.is_empty(), "{bad}: {:?}", s.curve.master); + assert!(s.curve.master.iter().all(|p| p.x.is_finite() && p.y.is_finite())); + } + } + #[test] fn only_present_fields_are_in_the_partial() { let x = r#" &d.curve.blue, _ => &d.curve.master, }; - if v.is_empty() { vec![Point::new(0.0, 0.0), Point::new(1.0, 1.0)] } else { v.clone() } + // in the order the curve is drawn (points from a sidecar, a preset or an agent can come + // unsorted or not finite): the drag below keeps each point between its neighbours + let mut v: Vec = v.iter().copied().filter(|q| q.x.is_finite() && q.y.is_finite()).collect(); + v.sort_by(|a, b| a.x.total_cmp(&b.x)); + if v.len() < 2 { vec![Point::new(0.0, 0.0), Point::new(1.0, 1.0)] } else { v } }; let pts = pts_of(d); let curve = lightcraft_color::spline::MonotoneCurve::new(&pts.iter().map(|q| (q.x, q.y)).collect::>()); @@ -1041,8 +1045,12 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev x = x.min(hi); } else if i + 1 == v.len() { x = x.max(lo); - } else { + } else if lo <= hi { x = x.clamp(lo, hi); + } else { + // neighbours closer than the 0.02 spacing (two clicks side by side, or points + // from a file) leave no room: the point stays midway (`clamp` would panic: min above max) + x = (v[i - 1].x + v[i + 1].x) / 2.0; } v[i] = Point::new(x, y); new_pts = Some(v); diff --git a/crates/ui-egui/src/tests_curve.rs b/crates/ui-egui/src/tests_curve.rs index fc46d6991..4d3b45702 100644 --- a/crates/ui-egui/src/tests_curve.rs +++ b/crates/ui-egui/src/tests_curve.rs @@ -186,3 +186,38 @@ fn targeted_adjustment_announces_itself_and_esc_turns_it_off() { assert!(h.app.ui.tool.is_empty(), "Esc turns the tool off: {:?}", h.app.ui.tool); assert!(matches!(h.app.ui.view, crate::state::ViewMode::Detail), "the first Esc only leaves the tool"); } + +/// Dragging a point whose neighbours are closer than the editor's 0.02 spacing (two clicks side +/// by side, or a curve from a sidecar or an agent) clamped it between bounds that had crossed, +/// which panics: the app went down mid-drag. The point now stays between its neighbours. +#[test] +fn dragging_a_point_between_close_neighbours_does_not_panic() { + let mut h = curve_open("master"); + let close = [[0.0, 0.0], [0.5, 0.5], [0.508, 0.6], [0.515, 0.3], [1.0, 1.0]]; + exec(&mut h, "develop.curve", json!({"channel": "master", "points": close})); + h.step(); + let side = curve_side(&mut h); + let r = h.request("ui.dragWidget", json!({"id": "curve", "fx": 0.508, "fy": 0.4, "dx": 0.05 * side, "dy": -0.1 * side, "steps": 8}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + let c = pts(&develop(&h).curve.master); + assert_eq!(c.len(), 5, "{c:?}"); + assert!(c.windows(2).all(|w| w[0].0 <= w[1].0), "the points stay in order: {c:?}"); + assert!(c[2].0 > 0.5 && c[2].0 < 0.515, "the dragged point stays between its neighbours: {c:?}"); +} + +/// Points an agent sends out of order are stored in the order the curve runs, so the editor's +/// neighbours are the curve's. +#[test] +fn curve_points_from_an_agent_are_stored_in_order() { + let mut h = curve_open("green"); + exec(&mut h, "develop.curve", json!({"channel": "green", "points": [[0.0, 0.0], [0.7, 0.8], [0.3, 0.2], [1.0, 1.0]]})); + h.step(); + assert_eq!(pts(&develop(&h).curve.green), vec![(0.0, 0.0), (0.3, 0.2), (0.7, 0.8), (1.0, 1.0)]); + let side = curve_side(&mut h); + let r = h.request("ui.dragWidget", json!({"id": "curve", "fx": 0.3, "fy": 0.8, "dx": 0.6 * side, "dy": 0.0, "steps": 8}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + let c = pts(&develop(&h).curve.green); + assert!(c[1].0 < c[2].0, "dragged up to its neighbour, not past it: {c:?}"); +} diff --git a/docs/whats-new.md b/docs/whats-new.md index b5c2ac68c..9fe5d397d 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,11 @@ ## 2026-10-13 +### Reliability +- Dragging a Tone Curve point that sits very close to its neighbours (two clicks side by side, or a curve from a + sidecar, a preset or an agent) no longer crashes LightCraft; the point stays between them. Curve points sent to + `develop.curve` are kept in order, and a sidecar curve with a "NaN" or "inf" point is ignored. + ### Slider drags on large raws - Dragging a slider no longer re-renders the edited photo's filmstrip thumbnail on every frame; it follows once you let go. On a 64 MP raw the first of those renders decoded the whole file again, which held up the loupe's