Merge PR #1804

crates/engine/src/cmd/develop.rs

@@ -588,14 +588,14 @@ pub fn specs() -> Vec<CommandSpec> {
             "Straighten",
             [],
             None,
-            "{angle: degrees -45..45} — keeps the largest crop of the current aspect",
+            "{angle: degrees -45..45} — keeps the crop if it still fits, else shrinks it about its centre (the largest crop of its aspect stays the largest)",
             has_active,
             |s, p| {
                 let id = active(s, "crop.straighten")?;
                 let (w, h) = crop_dims(s, id);
                 let angle = f64_req(p, "angle", "crop.straighten")?.clamp(-45.0, 45.0);
                 edit(s, "crop.straighten", "Straighten", |d| {
-                    // keep the current crop if it still fits, else the largest of its aspect that does
+                    // keep the current crop if it still fits, else shrink it about its centre until it does
                     d.crop.geometry = d.crop.geometry.with_angle(angle, w, h);
                     Ok(())
                 })

crates/engine/src/tests_crop_angle.rs

@@ -107,6 +107,33 @@ fn a_straighten_slider_drag_fits_live_and_is_one_undo_step() {
     assert!(close(geometry(&s), start));
 }
 
+#[test]
+fn turning_a_crop_dragged_in_from_a_corner_keeps_that_crop() {
+    // issue #1794: lock the aspect, drag a corner in, then rotate: the crop went back to the photo's
+    // size, because the dragged crop still touches two edges and stops fitting at once
+    let mut s = Session::with_demo();
+    let (w, h) = dims(&s);
+    s.execute("crop.aspect", &json!({"aspect": "original"})).unwrap();
+    s.execute("crop.drag", &json!({"handle": "bottomRight", "from": [1.0, 1.0], "to": [0.6, 0.6]})).unwrap();
+    let made = geometry(&s).rect_px(w, h);
+    assert!(made.width() < 0.7 * w, "the drag cropped: {made:?}");
+    s.execute("develop.beginInteraction", &json!({"label": "Crop"})).unwrap();
+    for angle in [0.25, 1.0, 2.5, 5.0, -3.0] {
+        s.execute("crop.straighten", &json!({"angle": angle})).unwrap();
+        let g = geometry(&s);
+        let r = g.rect_px(w, h);
+        assert!(g.is_within_image(w, h), "{angle}°: inside the photo");
+        assert!(r.width() <= made.width() + 1e-6, "{angle}°: no larger than the crop made, got {r:?} from {made:?}");
+        assert!((r.aspect() - made.aspect()).abs() < 1e-6, "{angle}°: the locked aspect holds");
+    }
+    s.execute("develop.endInteraction", &json!({})).unwrap();
+    // the first small turn only trims it
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("crop.straighten", &json!({"angle": 0.25})).unwrap();
+    let r = geometry(&s).rect_px(w, h);
+    assert!(r.width() > 0.97 * made.width(), "0.25° keeps nearly all of it: {r:?} from {made:?}");
+}
+
 #[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

crates/geom/src/crop.rs

@@ -7,7 +7,7 @@
 
 use serde::{Deserialize, Serialize};
 
-use crate::{Affine, Point, Rect, Vec2};
+use crate::{Affine, MIN_CROP_FRACTION, Point, Rect, Vec2};
 
 #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)]
 pub struct CropGeometry {
@@ -81,12 +81,14 @@ impl CropGeometry {
     }
 
     /// This crop turned to `angle_deg` on a `w × h` image: kept as it is when it still fits inside
-    /// 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). 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).
+    /// the rotated image, else shrunk about its own centre, keeping its aspect, until it fits
+    /// (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). 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).
+    /// Any other crop never grows: a crop dragged in from a corner still touches two edges of the
+    /// photo, and turning it used to swap it for the largest crop of its aspect (issue #1794).
     pub fn with_angle(&self, angle_deg: f64, w: f64, h: f64) -> CropGeometry {
         let r = self.rect_px(w, h);
         // a degenerate rect has no aspect to keep: fall back to the image's own
@@ -99,7 +101,45 @@ impl CropGeometry {
         if !is_largest && cand.is_within_image(w, h) {
             return cand;
         }
-        crop_fit_angle(w, h, angle_deg, aspect)
+        let fitted = crop_fit_angle(w, h, angle_deg, aspect);
+        if is_largest || aspect.is_none() {
+            return fitted;
+        }
+        // shrunk in place, as long as that leaves a crop the drag model would allow
+        let min_side = MIN_CROP_FRACTION * w.min(h);
+        if let Some(s) = cand.fit_scale_about_centre(w, h).filter(|s| r.width().min(r.height()) * s >= min_side) {
+            return CropGeometry { rect: Rect::from_center(r.center(), r.width() * s, r.height() * s).scale(1.0 / w, 1.0 / h), angle: angle_deg };
+        }
+        // its centre is off the turned photo (a sliver in a corner): centred, at its own size at most
+        let f = fitted.rect_px(w, h);
+        let Some(s) = Some(r.width() / f.width()).filter(|s| s.is_finite() && *s < 1.0) else {
+            return fitted;
+        };
+        CropGeometry { rect: Rect::from_center(f.center(), f.width() * s, f.height() * s).scale(1.0 / w, 1.0 / h), angle: angle_deg }
+    }
+
+    /// The largest scale, at most 1, at which this crop shrunk about its own centre lies inside the
+    /// rotated image; `None` when its centre is outside it.
+    fn fit_scale_about_centre(&self, w: f64, h: f64) -> Option<f64> {
+        let r = self.rect_px(w, h);
+        let back = Affine::rotate_about(-self.angle.to_radians(), Point::new(w / 2.0, h / 2.0));
+        let c = back.apply(r.center());
+        if !(0.0..=w).contains(&c.x) || !(0.0..=h).contains(&c.y) {
+            return None;
+        }
+        // each corner, in the image frame, is c + s·d: keep both coordinates in 0..w and 0..h
+        let mut s: f64 = 1.0;
+        for corner in r.corners() {
+            let d = back.apply(corner) - c;
+            for (q, dq, max) in [(c.x, d.x, w), (c.y, d.y, h)] {
+                if dq > 0.0 {
+                    s = s.min((max - q) / dq);
+                } else if dq < 0.0 {
+                    s = s.min(q / -dq);
+                }
+            }
+        }
+        Some(s).filter(|s| s.is_finite() && *s > 0.0)
     }
 }
 
@@ -195,6 +235,45 @@ mod tests {
         }
     }
 
+    #[test]
+    fn with_angle_shrinks_a_crop_against_the_edge_about_its_own_centre() {
+        // issue #1794: a crop dragged in from one corner keeps the opposite corner on the photo's,
+        // so it stops fitting at the first fraction of a degree; it used to become the largest crop
+        // of its aspect, nearly the whole photo
+        let (w, h) = (600.0, 400.0);
+        let c = CropGeometry { rect: Rect::new(0.0, 0.0, 0.6, 0.6), angle: 0.0 };
+        let a = c.rect_px(w, h);
+        for ang in [-10.0, -1.0, 0.5, 3.0, 20.0] {
+            let t = c.with_angle(ang, w, h);
+            let b = t.rect_px(w, h);
+            assert!(t.is_within_image(w, h), "{ang}°");
+            assert_eq!(t.angle, ang);
+            assert!(b.width() <= a.width() + 1e-9, "{ang}°: never larger than the crop made, {} vs {}", b.width(), a.width());
+            assert!((a.aspect() - b.aspect()).abs() < 1e-9, "{ang}°: aspect kept");
+            assert!(b.center().dist(a.center()) < 1e-9, "{ang}°: shrunk about its own centre");
+            // as large as fits there: 1 % bigger about the same centre does not
+            let big = CropGeometry { rect: Rect::from_center(t.rect.center(), t.rect.width() * 1.01, t.rect.height() * 1.01), angle: ang };
+            assert!(!big.is_within_image(w, h), "{ang}°: the largest about its centre");
+        }
+        // a small turn is a small change, not a jump
+        let t = c.with_angle(0.5, w, h).rect_px(w, h);
+        assert!(t.width() > 0.97 * a.width(), "0.5° keeps nearly all of the crop: {} of {}", t.width(), a.width());
+    }
+
+    #[test]
+    fn with_angle_never_grows_a_crop_whose_centre_leaves_the_photo() {
+        // a sliver in the corner: at 45° its centre is outside the turned photo, so it can't shrink
+        // in place; it moves to the centre at its own size, not at the largest of its aspect
+        let (w, h) = (600.0, 400.0);
+        let c = CropGeometry { rect: Rect::new(0.0, 0.0, 0.05, 0.05), angle: 0.0 };
+        let a = c.rect_px(w, h);
+        let t = c.with_angle(45.0, w, h);
+        let b = t.rect_px(w, h);
+        assert!(t.is_within_image(w, h));
+        assert!(b.width() <= a.width() + 1e-9 && (a.aspect() - b.aspect()).abs() < 1e-9, "{b:?} from {a:?}");
+        assert!(b.center().dist(Point::new(w / 2.0, h / 2.0)) < 1e-9, "centred: {b:?}");
+    }
+
     #[test]
     fn with_angle_survives_a_degenerate_crop() {
         // zero height: no aspect to keep (width / height = ∞)

docs/parity-checklist.md

@@ -252,7 +252,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
 |---|---|---|---|---|---|
 | LR-CROP-RECT | Crop rectangle | P0 | ✅ | `cmd:crop.set`, `cmd:crop.drag`, `crates/geom/src/crop_drag.rs`, `crates/ui-egui/src/panels/detail.rs` | double-click inside the box applies the crop (like Return) |
 | LR-CROP-ASPECT | Aspect ratios | P0 | ✅ | `cmd:crop.aspect`, `cmd:crop.rotateAspect`, `crates/ui-egui/src/panels/right.rs` | menu with presets + custom W × H, visible Lock toggle (also A), button names the active ratio; every handle honours the lock, holding Shift while dragging a handle keeps the frame's proportions for that drag (issue #614), and the drag stops at the image edge instead of shifting the frame (issue #295); "Original" is the photo's own shape (no separate As Shot); the last chosen aspect is saved as a crop tool default, resolving Original per photo and preserving existing crops; opening Crop or browsing never edits a photo: the default applies only when a crop gesture starts, within its single undo step |
-| LR-CROP-STRAIGHTEN | Straighten tool | P0 | ✅ | `cmd:crop.straighten`, `cmd:crop.autoStraighten`, `crates/ui-egui/src/panels/detail.rs` (`rotate_cursor`, `angle_readout`), `crates/ui-egui/src/panels/right.rs` (Angle field), `crates/ui-egui/src/tests_crop_rotate.rs` | Straighten Tool button: drag along a horizon/vertical; double-click or Auto levels automatically. Dragging outside the crop box rotates it: there the pointer is a curved double arrow that bulges away from the side of the box it is on, diagonally off a corner and following a rotated box (`rotate_cursor_turn`, tested by `rotate_cursor_faces_away_from_the_crop_box`) (with ⌘ held, the straighten crosshair), and while rotating the angle shows next to it, kept on the canvas (issue #534); an exact angle goes in the Angle field under Straighten (`cropAngleField`: applies on Return or leaving it, Esc keeps the old angle, takes a decimal comma; a drag on it is one undo step) or by clicking the Straighten value (its hover says so). Every way of setting the angle (rotate handle, Straighten slider, Angle field, `crop.straighten`, `crop.set {angle}`, and the generic `develop.set` / `develop.adjust` / `develop.quickAdjust` / `develop.resetControl` on `crop.angle`, as MCP, the control channel and `lightcraft-cli render --set` use them) keeps the crop if it still fits, else shrinks it to the largest crop of its aspect inside the photo (`CropGeometry::with_angle`, `crates/engine/src/tests_crop_angle.rs`; before issue #742 the generic path left the box, and the export, past the photo with smeared edge pixels in the corners). The dimming outside the box is the photo minus the (rotated) box, drawn as one mesh (`outside_crop_mesh`), so rotating never dims wedges inside the box or doubles them outside (issue #731) |
+| LR-CROP-STRAIGHTEN | Straighten tool | P0 | ✅ | `cmd:crop.straighten`, `cmd:crop.autoStraighten`, `crates/ui-egui/src/panels/detail.rs` (`rotate_cursor`, `angle_readout`), `crates/ui-egui/src/panels/right.rs` (Angle field), `crates/ui-egui/src/tests_crop_rotate.rs` | Straighten Tool button: drag along a horizon/vertical; double-click or Auto levels automatically. Dragging outside the crop box rotates it: there the pointer is a curved double arrow that bulges away from the side of the box it is on, diagonally off a corner and following a rotated box (`rotate_cursor_turn`, tested by `rotate_cursor_faces_away_from_the_crop_box`) (with ⌘ held, the straighten crosshair), and while rotating the angle shows next to it, kept on the canvas (issue #534); an exact angle goes in the Angle field under Straighten (`cropAngleField`: applies on Return or leaving it, Esc keeps the old angle, takes a decimal comma; a drag on it is one undo step) or by clicking the Straighten value (its hover says so). Every way of setting the angle (rotate handle, Straighten slider, Angle field, `crop.straighten`, `crop.set {angle}`, and the generic `develop.set` / `develop.adjust` / `develop.quickAdjust` / `develop.resetControl` on `crop.angle`, as MCP, the control channel and `lightcraft-cli render --set` use them) keeps the crop if it still fits, else shrinks it about its own centre, keeping its aspect, until it fits; the full photo (or a crop an earlier turn shrank to the largest of its aspect) stays the largest, so it grows back towards 0° (issue #1388), and no other crop ever grows (issue #1794: a crop dragged in from a corner turned into nearly the whole photo) (`CropGeometry::with_angle`, `crates/engine/src/tests_crop_angle.rs`; before issue #742 the generic path left the box, and the export, past the photo with smeared edge pixels in the corners). The dimming outside the box is the photo minus the (rotated) box, drawn as one mesh (`outside_crop_mesh`), so rotating never dims wedges inside the box or doubles them outside (issue #731) |
 | LR-CROP-AUTO | Auto straighten | P1 | ✅ | `cmd:crop.autoStraighten` | crop-angle leveling from detected horizon/plumb lines (consensus required) |
 | LR-CROP-ANGLE | Angle slider | P0 | ✅ | `ctl:crop.angle`, `crates/ui-egui/src/panels/right.rs` | the Straighten slider; an exact angle can also be typed in the Angle field under it (see LR-CROP-STRAIGHTEN); setting `crop.angle` through `develop.set` fits the crop inside the photo as Straighten does (issue #742) |
 | LR-CROP-ROTATE90 | Rotate 90° | P0 | ✅ | `cmd:photo.rotateLeft`, `cmd:photo.rotateRight` | |