Merge PR #1394

crates/engine/src/tests_crop_angle.rs

@@ -81,11 +81,11 @@ fn quick_develop_reset_and_batch_paths_fit_too() {
     let got = try_cmd(&mut s, "develop.set", json!({"control": "crop.angle", "value": 12.0, "ids": [id.0]}));
     assert!(close(got, want), "develop.set ids {got:?} vs {want:?}");
 
-    // back to 0°: the crop that fitted at 12° fits at 0° too and is kept
+    // back to 0°: the crop shrunk to fit at 12° grows back to the full photo (issue #1388)
     s.execute("develop.set", &json!({"control": "crop.angle", "value": 12.0})).unwrap();
     s.execute("develop.resetControl", &json!({"control": "crop.angle"})).unwrap();
     let g = geometry(&s);
-    assert_eq!((g.rect, g.angle), (want.rect, 0.0));
+    assert_eq!((g.rect, g.angle), (Rect::UNIT, 0.0));
     assert!(g.is_within_image(w, h));
 }
 
@@ -106,3 +106,21 @@ fn a_straighten_slider_drag_fits_live_and_is_one_undo_step() {
     s.execute("edit.undo", &json!({})).unwrap();
     assert!(close(geometry(&s), start));
 }
+
+#[test]
+fn a_crop_shrunk_by_straightening_grows_back_towards_0() {
+    // issue #1388: slide the Straighten slider out and back, in one drag or in several
+    let mut s = Session::with_demo();
+    let (w, h) = dims(&s);
+    s.execute("develop.beginInteraction", &json!({"label": "Straighten"})).unwrap();
+    for angle in [5.0, 12.0, 20.0, 8.0, -20.0, -3.0, 0.0] {
+        s.execute("crop.straighten", &json!({"angle": angle})).unwrap();
+        let g = geometry(&s);
+        assert!(close(g, lightcraft_geom::crop_fit_angle(w, h, angle, None)), "the largest crop at {angle}°, got {g:?}");
+    }
+    s.execute("develop.endInteraction", &json!({})).unwrap();
+    s.execute("crop.straighten", &json!({"angle": 20.0})).unwrap();
+    s.execute("crop.straighten", &json!({"angle": 0.0})).unwrap();
+    let g = geometry(&s);
+    assert!(close(g, CropGeometry::default()), "two separate drags: back to the full photo at 0°, got {g:?}");
+}

crates/geom/src/crop.rs

@@ -84,15 +84,21 @@ impl CropGeometry {
     /// the rotated image, else the largest crop of its aspect that fits, centred (Lightroom's
     /// "Constrain to image"). Every way of setting the angle goes through here (the rotate handle,
     /// the Straighten slider and Angle field, `crop.straighten`, `develop.set` on `crop.angle`), so
-    /// they all give the same crop (issue #742).
+    /// they all give the same crop (issue #742). A crop that is the largest of its aspect at its
+    /// own angle (the full photo, or one an earlier turn shrank to fit) stays the largest at the new
+    /// angle, so it grows back as the angle returns towards 0° (issue #1388).
     pub fn with_angle(&self, angle_deg: f64, w: f64, h: f64) -> CropGeometry {
-        let cand = CropGeometry { rect: self.rect, angle: angle_deg };
-        if cand.is_within_image(w, h) {
-            return cand;
-        }
         let r = self.rect_px(w, h);
         // a degenerate rect has no aspect to keep: fall back to the image's own
         let aspect = Some(r.width() / r.height()).filter(|a| a.is_finite() && *a > 0.0);
+        let largest = crop_fit_angle(w, h, self.angle, aspect).rect;
+        let is_largest = [(self.rect.x0, largest.x0), (self.rect.y0, largest.y0), (self.rect.x1, largest.x1), (self.rect.y1, largest.y1)]
+            .iter()
+            .all(|(a, b)| (a - b).abs() < 1e-9);
+        let cand = CropGeometry { rect: self.rect, angle: angle_deg };
+        if !is_largest && cand.is_within_image(w, h) {
+            return cand;
+        }
         crop_fit_angle(w, h, angle_deg, aspect)
     }
 }
@@ -172,6 +178,23 @@ mod tests {
         }
     }
 
+    #[test]
+    fn with_angle_grows_a_fitted_crop_back_as_the_angle_returns() {
+        // issue #1388: the crop a turn shrank to fit is the largest of its aspect; turning back
+        // gives the largest at the new angle, not the shrunk one again
+        for aspect in [None, Some(1.0), Some(16.0 / 9.0)] {
+            let mut c = crop_fit_angle(600.0, 400.0, 0.0, aspect);
+            for ang in [10.0, 25.0, -25.0, 7.0, 0.0] {
+                c = c.with_angle(ang, 600.0, 400.0);
+                let want = crop_fit_angle(600.0, 400.0, ang, aspect);
+                assert!(
+                    (c.rect.width() - want.rect.width()).abs() < 1e-9 && (c.rect.height() - want.rect.height()).abs() < 1e-9,
+                    "{aspect:?} at {ang}°: {c:?} vs {want:?}"
+                );
+            }
+        }
+    }
+
     #[test]
     fn with_angle_survives_a_degenerate_crop() {
         // zero height: no aspect to keep (width / height = ∞)