Merge PR #1550

# Conflicts:
#	docs/whats-new.md

crates/engine/src/cmd/develop.rs

@@ -500,13 +500,15 @@ pub fn specs() -> Vec<CommandSpec> {
         }),
         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<Point> = p
+            let mut pts: Vec<Point> = 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,

crates/engine/src/crs.rs

@@ -173,7 +173,8 @@ fn curve(props: &Props, k: &str) -> Option<Value> {
     let mut pts = Vec::new();
     for it in items {
         let (x, y) = it.split_once(',')?;
-        let (x, y) = (x.trim().parse::<f64>().ok()?, y.trim().parse::<f64>().ok()?);
+        // (`parse` reads "NaN" and "inf" too: a curve with such a point is not a curve)
+        let (x, y) = (x.trim().parse::<f64>().ok().filter(|v| v.is_finite())?, y.trim().parse::<f64>().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("<rdf:li>64, 52</rdf:li>", &format!("<rdf:li>{bad}</rdf:li>"));
+            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#"<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"><rdf:Description

crates/ui-egui/src/panels/edit.rs

@@ -950,7 +950,11 @@ fn curve_editor(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, d: &Dev
             "blue" => &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<Point> = 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::<Vec<_>>());
@@ -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);

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:?}");
+}

docs/whats-new.md

@@ -76,6 +76,11 @@
 - Exporting a photo more than 65,535 times as long as it is wide (a one-pixel strip), or opening the Export dialog's
   watermark preview for it, no longer crashes LightCraft; the export is capped at the encoders' 65,535 pixels.
 
+### 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