From 23f00822eb87b047ff9d2adc2dfb1339d945d158 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 12:37:56 +0000 Subject: [PATCH] #1927: Auto Sync fits each photo's own crop under Constrain Crop The crop fit ran before the Auto Sync delta, so a Geometry edit carried the active photo's fitted crop rect into the delta and onto every other selected photo, replacing (and could enlarge) their own crops. The delta is now taken from the edit as asked, and each target fits its own crop. Review on #1990. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK --- crates/engine/src/lib.rs | 17 ++++++++------ crates/engine/src/tests_constrain_crop.rs | 28 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f472d7b60..e60cf5cf8 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -874,14 +874,16 @@ 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(self.fit_constrained_crop(id, settings)); + // Auto Sync carries the edit as asked; each photo then fits its own crop (review of #1990) + let requested = settings; + let settings = Arc::new(self.fit_constrained_crop(id, requested.clone())); if let Some(i) = &self.interaction && i.photo == id { return self.apply_silent(Op::SetDevelop { id, settings, label: label.into(), edited: Some(now) }); } let mut ops = vec![self.develop_op(id, (*settings).clone(), label).ok_or(lightcraft_catalog::CatalogError::NoPhoto(id))?]; - ops.extend(self.auto_sync_ops(id, &settings, label)); + ops.extend(self.auto_sync_ops(id, &requested, label)); let op = if ops.len() == 1 { ops.remove(0) } else { Op::Batch { ops } }; self.commit(label, op) } @@ -917,10 +919,11 @@ impl Session { 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 - /// history / snapshot restores. + /// With Auto Sync on, the ops that carry an edit of the active photo `id` (to `new`, as asked, + /// before its crop is fitted) 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 history / snapshot restores. Constrain Crop then fits each + /// photo's own crop: the active photo's fitted crop is never copied over another's. fn auto_sync_ops(&self, id: PhotoId, new: &DevelopSettings, label: &str) -> Vec { if !self.auto_sync || self.active() != Some(id) @@ -946,7 +949,7 @@ impl Session { .filter(|x| **x != id) .filter_map(|x| self.develop_of(*x).map(|d| (*x, d))) .filter_map(|(x, d)| { - let synced = lightcraft_develop::apply_partial(&d, &delta, 1.0); + let synced = self.fit_constrained_crop(x, lightcraft_develop::apply_partial(&d, &delta, 1.0)); (synced != *d).then(|| self.develop_op(x, synced, label)).flatten() }) .collect() diff --git a/crates/engine/src/tests_constrain_crop.rs b/crates/engine/src/tests_constrain_crop.rs index 37c6d734d..51dcee1c0 100644 --- a/crates/engine/src/tests_constrain_crop.rs +++ b/crates/engine/src/tests_constrain_crop.rs @@ -70,3 +70,31 @@ fn a_crop_drawn_by_hand_is_kept_as_drawn() { 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"); } + +/// Review of #1990: with Auto Sync, a Geometry edit on the active photo fits each selected photo's +/// own crop; the active photo's fitted crop is never copied over another photo's (smaller) crop. +#[test] +fn auto_sync_fits_each_photos_own_crop() { + let mut s = Session::with_demo(); + let ids: Vec<_> = s.catalog.photos().take(2).map(|p| p.id).collect(); + let (a, b) = (ids[0], ids[1]); + // B: a small crop off-centre; both constrained + s.selection = crate::Selection { ids: vec![b], active: Some(b) }; + s.execute("crop.set", &json!({"rect": [0.05, 0.1, 0.45, 0.5]})).unwrap(); + constrain(&mut s, true); + s.selection = crate::Selection { ids: vec![a], active: Some(a) }; + constrain(&mut s, true); + let b_before = s.develop_of(b).unwrap().crop.geometry.rect; + // A active, both selected, Auto Sync on: a Geometry edit on A + s.selection = crate::Selection { ids: vec![a, b], active: Some(a) }; + s.execute("develop.autoSync", &json!({"on": true})).unwrap(); + vertical(&mut s, -60.0); + let (da, db) = (s.develop_of(a).unwrap(), s.develop_of(b).unwrap()); + assert_eq!(db.geometry.vertical, -60.0, "the edit reached B"); + let b_after = db.crop.geometry.rect; + assert_ne!(b_after, da.crop.geometry.rect, "B keeps a crop of its own"); + let inside = |r: Rect, o: Rect| r.x0 >= o.x0 - 1e-9 && r.y0 >= o.y0 - 1e-9 && r.x1 <= o.x1 + 1e-9 && r.y1 <= o.y1 + 1e-9; + assert!(inside(b_after, b_before), "B's crop only shrinks within itself: {b_before:?} -> {b_after:?}"); + s.selection = crate::Selection { ids: vec![b], active: Some(b) }; + assert!(fits(&s), "and fits B's warp"); +}