diff --git a/crates/engine/src/tests_crop_angle.rs b/crates/engine/src/tests_crop_angle.rs index 47759d60..90a12b2d 100644 --- a/crates/engine/src/tests_crop_angle.rs +++ b/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:?}"); +} diff --git a/crates/geom/src/crop.rs b/crates/geom/src/crop.rs index 810e39cd..857ea19b 100644 --- a/crates/geom/src/crop.rs +++ b/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 = ∞)