#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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK
This commit is contained in:
Claude
2026-10-16 00:01:00 +00:00
parent 0da998f24d
commit 54c6c43b07
4 changed files with 116 additions and 2 deletions
+34 -1
View File
@@ -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;
+72
View File
@@ -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");
}
+1 -1
View File
@@ -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 |
+9
View File
@@ -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