crop.set refuses a crop smaller than the crop tool allows, instead of exporting 1×1 (part of #1952)

crates/engine/src/cmd/develop.rs

@@ -2,7 +2,7 @@
 
 use lightcraft_catalog::{Op, PhotoId, Version};
 use lightcraft_develop::{DevelopSettings, Preset, ProcessVersion, Section, SettingsGroup, Treatment, Upright, WbMode, controls};
-use lightcraft_geom::{CropGeometry, CropHandle, Point, Rect, crop_fit_angle, drag_crop};
+use lightcraft_geom::{CropGeometry, CropHandle, MIN_CROP_FRACTION, Point, Rect, crop_fit_angle, drag_crop};
 use serde_json::{Value, json};
 
 use super::{CommandSpec, always, bad, bool_or, cmd, f64_or, f64_req, has_active, has_clipboard, has_selection, index_req, ok, str_param};
@@ -530,7 +530,16 @@ pub fn specs() -> Vec<CommandSpec> {
                     if v.len() != 4 {
                         return Err(bad("crop.set", "rect needs 4 numbers"));
                     }
-                    d.crop.geometry.rect = Rect::new(v[0].min(v[2]), v[1].min(v[3]), v[0].max(v[2]), v[1].max(v[3]));
+                    let r = Rect::new(v[0].min(v[2]), v[1].min(v[3]), v[0].max(v[2]), v[1].max(v[3]));
+                    // the smallest crop a drag of the crop tool can make: below it an export comes out 1×1 (#1952)
+                    let min_side = MIN_CROP_FRACTION * w.min(h) * (1.0 - 1e-9);
+                    if r.width() * w < min_side || r.height() * h < min_side {
+                        return Err(bad(
+                            "crop.set",
+                            format!("rect is smaller than the smallest crop ({} % of the shorter side)", MIN_CROP_FRACTION * 100.0),
+                        ));
+                    }
+                    d.crop.geometry.rect = r;
                 }
                 let angle = p.get("angle").and_then(Value::as_f64).map(|a| a.clamp(-45.0, 45.0));
                 d.crop.geometry = match (p.get("rect"), angle) {

crates/engine/src/tests.rs

@@ -519,7 +519,7 @@ fn paste_selected_settings_pastes_only_chosen_copied_groups() {
     assert_eq!(s.develop_of(ids[1]).unwrap().color.vibrance, 30.0);
 }
 
-/// A crop of no size (or a hair wide), as `crop.set` or a sidecar can set it, made the render's
+/// A crop of no size (or a hair wide), as a sidecar can set it, made the render's
 /// samplers read at infinite coordinates and its blurs ask for sigmas of millions of pixels: with a
 /// healing spot and masks on, render workers panicked (an i32 square in the blur, a bilinear
 /// neighbour past isize::MAX). Such a photo renders, quickly.
@@ -533,7 +533,7 @@ fn degenerate_crops_render_with_spots_and_masks() {
         s.execute("mask.add", &json!({"kind": "linear"})).unwrap();
         s.execute("develop.set", &json!({"control": "effects.clarity", "value": 80})).unwrap();
         s.execute("develop.set", &json!({"control": "effects.dehaze", "value": 50})).unwrap();
-        s.execute("crop.set", &json!({"rect": rect, "angle": 30})).unwrap();
+        set_crop_unchecked(&mut s, rect, 30.0);
         let t = std::time::Instant::now();
         let r = s.render_now(id, 700, 450).unwrap();
         assert!(r.image.width >= 1 && r.image.height >= 1, "{rect:?}");
@@ -541,13 +541,23 @@ fn degenerate_crops_render_with_spots_and_masks() {
     }
 }
 
+/// Store a crop as a sidecar can, without `crop.set`'s minimum size, kept inside the turned photo.
+fn set_crop_unchecked(s: &mut Session, [x0, y0, x1, y1]: [f64; 4], angle: f64) {
+    let id = s.active().unwrap();
+    let p = s.catalog.photo(id).unwrap();
+    let (w, h) = (p.width as f64, p.height as f64);
+    let mut d = active_dev(s);
+    d.crop.geometry = lightcraft_geom::CropGeometry { rect: lightcraft_geom::Rect::new(x0, y0, x1, y1), angle }.constrained(w, h);
+    s.set_develop(id, d, "Crop").unwrap();
+}
+
 /// `crop.rotateAspect` on a crop of no size took its aspect from 0/0 = NaN and stored a NaN crop
 /// rectangle; it now falls back to the image's own aspect, a finite crop inside the image.
 #[test]
 fn rotate_aspect_of_degenerate_crop_stays_finite() {
     for rect in [[0.5, 0.5, 0.5, 0.5], [0.2, 0.2, 0.2, 0.8], [0.2, 0.2, 0.8, 0.2]] {
         let mut s = demo();
-        s.execute("crop.set", &json!({"rect": rect})).unwrap();
+        set_crop_unchecked(&mut s, rect, 0.0);
         s.execute("crop.rotateAspect", &json!({})).unwrap();
         let r = active_dev(&s).crop.geometry.rect;
         assert!([r.x0, r.y0, r.x1, r.y1].iter().all(|v| v.is_finite()), "{rect:?}: {r:?}");
@@ -567,6 +577,22 @@ fn crop_set_huge_rect_stays_finite() {
     }
 }
 
+/// `crop.set` took a crop of no size (or a sliver), and the next export wrote a 1×1 image (#1952).
+/// It now refuses a crop smaller than a drag of the crop tool can make, and keeps the one it had.
+#[test]
+fn crop_set_refuses_a_crop_smaller_than_the_tool_allows() {
+    let mut s = demo();
+    s.execute("crop.set", &json!({"rect": [0.1, 0.1, 0.9, 0.9]})).unwrap();
+    let before = active_dev(&s).crop.geometry;
+    for rect in [[0.5, 0.5, 0.5, 0.5], [0.2, 0.2, 0.2, 0.8], [0.2, 0.4, 0.8, 0.4], [0.0, 0.0, 1e-9, 1.0]] {
+        let e = s.execute("crop.set", &json!({"rect": rect})).unwrap_err();
+        assert!(e.to_string().contains("smaller"), "{rect:?}: {e}");
+        assert_eq!(active_dev(&s).crop.geometry, before, "{rect:?}");
+    }
+    s.execute("crop.set", &json!({"rect": [0.45, 0.45, 0.55, 0.55]})).unwrap();
+    assert_eq!(active_dev(&s).crop.geometry.rect, lightcraft_geom::Rect::new(0.45, 0.45, 0.55, 0.55));
+}
+
 /// `mask.move` with a delta near `i64::MAX` overflowed `index + delta` (a panic wherever overflow
 /// checks are on); extreme deltas and indices now move the mask to the bottom or the top.
 #[test]

crates/ui-egui/src/tests_zoom.rs

@@ -109,7 +109,7 @@ fn zoomed_before_after_views_stay_in_their_panes() {
     }
 }
 
-/// A crop of no size (or a hair wide), from an agent or a sidecar, gave the loupe a NaN aspect:
+/// A crop of no size (or a hair wide), from a sidecar, gave the loupe a NaN aspect:
 /// the photo was laid out at a NaN size, which egui's debug checks stop on (a panic in a debug
 /// build) and nothing could be drawn or clicked. It now lays out at a finite size, fitted and at
 /// 100 %.
@@ -117,8 +117,8 @@ fn zoomed_before_after_views_stay_in_their_panes() {
 fn a_crop_of_no_size_lays_out_at_a_finite_size() {
     let mut h = detail();
     for rect in [[0.5, 0.5, 0.5, 0.5], [0.0, 0.0, 1e-9, 1.0], [0.0, 0.3, 1.0, 0.3]] {
-        let r = h.request("engine.execute", json!({"command": "crop.set", "params": {"rect": rect}}), T);
-        assert_eq!(r["ok"], true, "{r}");
+        // as a sidecar can set it: `crop.set` refuses a crop this small
+        edit(&mut h, |d| d.crop.geometry.rect = lightcraft_geom::Rect::new(rect[0], rect[1], rect[2], rect[3]));
         for zoom in ["view.zoomFit", "view.zoom100"] {
             let r = h.request("engine.execute", json!({"command": zoom}), T);
             assert_eq!(r["ok"], true, "{zoom}: {r}");