From 54c6c43b07eb6944fcd19d2741a2ad4b30652d11 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 23:30:07 +0000 Subject: [PATCH] #1927: Constrain Crop tightens the stored crop Constrain Crop only applied at render time, and the Crop tool shows the uncropped view with the stored crop, so in the Crop panel (where the checkbox is) ticking it or moving a Geometry slider changed nothing. Session::set_develop now shrinks the stored crop about its centre until it fits the warped photo when Constrain Crop is turned on, or when an edit changes the warp (Geometry, Upright, lens corrections, section eyes) while it is on. A crop edit itself is kept as drawn. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK --- crates/engine/src/lib.rs | 35 ++++++++++- crates/engine/src/tests_constrain_crop.rs | 72 +++++++++++++++++++++++ docs/parity-checklist.md | 2 +- docs/whats-new.md | 9 +++ 4 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 crates/engine/src/tests_constrain_crop.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8a282ce3e..f472d7b60 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -874,7 +874,7 @@ impl Session { /// undo step; otherwise it is committed with a history entry. pub fn set_develop(&mut self, id: PhotoId, settings: DevelopSettings, label: &str) -> Result<()> { let now = (self.clock)(); - let settings = Arc::new(settings); + let settings = Arc::new(self.fit_constrained_crop(id, settings)); if let Some(i) = &self.interaction && i.photo == id { @@ -886,6 +886,37 @@ impl Session { self.commit(label, op) } + /// "Constrain Crop" keeps the stored crop inside the warped photo, as Lightroom does (issue + /// #1927): turning it on, or an edit that changes the warp (Geometry, Upright, lens corrections, + /// their sections' eyes) while it is on, shrinks the crop about its centre until no blank corner + /// is left, so the Crop tool shows what the render keeps. An edit of the crop itself is left as + /// drawn (the render still constrains it). + fn fit_constrained_crop(&self, id: PhotoId, mut d: DevelopSettings) -> DevelopSettings { + if !d.geometry.constrain_crop { + return d; + } + let Some(photo) = self.catalog.photo(id) else { return d }; + let before = &photo.develop; + let turned_on = !before.geometry.constrain_crop; + let warp_changed = before.geometry != d.geometry + || before.optics != d.optics + || before.orientation != d.orientation + || before.disabled_sections != d.disabled_sections; + let crop_edited = before.crop.geometry != d.crop.geometry; + if !(turned_on || (warp_changed && !crop_edited)) { + return d; + } + let frame = lightcraft_pipeline::geometry::Frame::with_lens( + photo.width.max(1) as usize, + photo.height.max(1) as usize, + &d, + true, + photo.embedded_lens.as_ref(), + ); + d.crop.geometry.rect = frame.crop.rect; + d + } + /// With Auto Sync on, the ops that carry an edit of the active photo `id` (to `new`) over to the /// other selected photos: only the settings that changed; never spot removal or red eye (they /// belong to one photo's pixels), nor the rendering process (Sync doesn't carry it either), nor @@ -1242,6 +1273,8 @@ mod tests_albums_local; #[cfg(test)] mod tests_color; #[cfg(test)] +mod tests_constrain_crop; +#[cfg(test)] mod tests_crop_angle; #[cfg(test)] mod tests_denoise; diff --git a/crates/engine/src/tests_constrain_crop.rs b/crates/engine/src/tests_constrain_crop.rs new file mode 100644 index 000000000..37c6d734d --- /dev/null +++ b/crates/engine/src/tests_constrain_crop.rs @@ -0,0 +1,72 @@ +//! Constrain Crop changes the stored crop, as in Lightroom (issue #1927): with it on, the crop the +//! Crop tool shows (the stored one) never takes in the blank corners a Geometry warp leaves, so +//! ticking it, or moving a Geometry slider while it is ticked, visibly tightens the crop. + +use lightcraft_geom::{CropGeometry, Rect}; +use lightcraft_pipeline::geometry::Frame; +use serde_json::json; + +use crate::Session; + +fn crop(s: &Session) -> CropGeometry { + s.develop_of(s.active().unwrap()).unwrap().crop.geometry +} + +/// Does the stored crop already fit the warped photo (the render's constraint would leave it alone)? +fn fits(s: &Session) -> bool { + let p = s.catalog.photo(s.active().unwrap()).unwrap(); + let mut d = (*p.develop).clone(); + d.geometry.constrain_crop = true; + let f = Frame::with_lens(p.width as usize, p.height as usize, &d, true, p.embedded_lens.as_ref()); + let (a, b) = (f.crop.rect, p.develop.crop.geometry.rect); + [a.x0 - b.x0, a.y0 - b.y0, a.x1 - b.x1, a.y1 - b.y1].iter().all(|d| d.abs() < 1e-9) +} + +fn smaller(a: Rect, b: Rect) -> bool { + a.width() < b.width() - 1e-3 && a.height() < b.height() - 1e-3 +} + +fn constrain(s: &mut Session, on: bool) { + s.execute("develop.merge", &json!({"settings": {"geometry": {"constrain_crop": on}}, "label": "Constrain Crop"})).unwrap(); +} + +fn vertical(s: &mut Session, v: f64) { + s.execute("develop.set", &json!({"control": "geometry.vertical", "value": v})).unwrap(); +} + +#[test] +fn a_geometry_slider_tightens_the_crop_while_constrained() { + let mut s = Session::with_demo(); + let full = crop(&s).rect; + constrain(&mut s, true); + assert_eq!(crop(&s).rect, full, "no warp yet: nothing to cut off"); + vertical(&mut s, -60.0); + assert!(smaller(crop(&s).rect, full), "the stored crop shrank: {:?}", crop(&s).rect); + assert!(fits(&s), "and holds no blank corner"); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(crop(&s).rect, full, "undo puts the crop back with the slider"); +} + +#[test] +fn ticking_constrain_after_the_warp_tightens_the_crop() { + let mut s = Session::with_demo(); + let full = crop(&s).rect; + vertical(&mut s, -60.0); + assert_eq!(crop(&s).rect, full, "unconstrained: the crop keeps the blank corners"); + assert!(!fits(&s)); + constrain(&mut s, true); + assert!(smaller(crop(&s).rect, full) && fits(&s), "{:?}", crop(&s).rect); + // unticking keeps the tighter crop, as in Lightroom + let tight = crop(&s).rect; + constrain(&mut s, false); + assert_eq!(crop(&s).rect, tight); +} + +#[test] +fn a_crop_drawn_by_hand_is_kept_as_drawn() { + let mut s = Session::with_demo(); + constrain(&mut s, true); + vertical(&mut s, -60.0); + s.execute("crop.set", &json!({"rect": [0.0, 0.0, 1.0, 1.0]})).unwrap(); + assert_eq!(crop(&s).rect, Rect::new(0.0, 0.0, 1.0, 1.0), "the crop edit itself is not refitted"); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 95dcd2aef..3504eb77d 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -220,7 +220,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-OPTICS-MANUAL | Manual distortion / vignetting | P1 | ✅ | `ctl:optics.distortion`, `ctl:optics.vignetting`, `ctl:optics.vignettingMidpoint` | | | LR-EDIT-GEOM-UPRIGHT | Upright | P1 | ✅ | `cmd:geometry.upright`, `cmd:geometry.guides` | | | LR-EDIT-GEOM-MANUAL | Manual transform | P1 | ✅ | `ctl:geometry.*` | | -| LR-EDIT-GEOM-CONSTRAIN | Constrain crop | P1 | ✅ | `crates/ui-egui/src/panels/right.rs` (checkbox → `cmd:develop.merge`) | | +| LR-EDIT-GEOM-CONSTRAIN | Constrain crop | P1 | ✅ | `crates/ui-egui/src/panels/right.rs` (checkbox → `cmd:develop.merge`), `crates/engine/src/lib.rs` (`fit_constrained_crop`) | Ticking it, or changing Geometry, Upright or lens corrections while it is ticked, shrinks the stored crop about its centre until no blank corner is left, so the Crop tool shows it (it used to apply at render time only, outside the Crop tool: issue #1927; tests in `crates/engine/src/tests_constrain_crop.rs`). A crop drawn by hand is kept as drawn; unticking keeps the tighter crop | | LR-EDIT-GEOM-GRID | Grid while transforming | P2 | ✅ | `crates/ui-egui/src/panels/detail.rs` | a fine grid over the photo while a geometry slider is dragged | | LR-EDIT-LENSBLUR | Lens blur | P2 | ⬜ | | settings field reserved, not rendered | | LR-EDIT-CALIB | Calibration [Classic] | P1 | ✅ | `ctl:calibration.*` | shadows tint, red/green/blue primary hue and saturation; read/written in XMP | diff --git a/docs/whats-new.md b/docs/whats-new.md index 97e1367a8..0eabf447b 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,14 @@ # What's new in LightCraft +## 2026-10-15 + +### Constrain Crop tightens the crop you see +- Geometry ▸ Constrain Crop now changes the crop itself, as in Lightroom (issue #1927). Ticking it, or moving a + Geometry slider, Upright or a lens correction while it is ticked, shrinks the crop about its centre until the blank + corners left by the correction are gone, and the Crop tool shows the tighter crop. Before, it only applied when the + photo was shown outside the Crop tool, so in the Crop panel, where the checkbox is, nothing seemed to change. A crop + you draw by hand is kept as drawn, and unticking keeps the tighter crop. + ## 2026-10-14 ### Lightroom radial masks on the right side