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"); +}