diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e60cf5cf8..da435b7d7 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -184,6 +184,10 @@ pub struct Interaction { pub label: String, pub photo: PhotoId, pub original: Arc, + /// Whether an edit during the interaction changed the crop itself. If none did, the crop the + /// photo shows was fitted by Constrain Crop, and the final commit starts again from the original + /// crop, so Auto Sync carries the edit and not the active photo's fitted crop. + pub crop_edited: bool, } /// The most photos an edit may touch for the view to follow it photo by photo; a larger edit is worked out whole. @@ -877,9 +881,11 @@ impl Session { // 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 + let crop_edited = self.develop_of(id).is_some_and(|d| d.crop.geometry != requested.crop.geometry); + if let Some(i) = &mut self.interaction && i.photo == id { + i.crop_edited |= crop_edited; 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))?]; @@ -903,7 +909,7 @@ impl Session { let warp_changed = before.geometry != d.geometry || before.optics != d.optics || before.orientation != d.orientation - || before.disabled_sections != d.disabled_sections; + || ["geometry", "optics"].iter().any(|s| before.section_enabled(s) != d.section_enabled(s)); let crop_edited = before.crop.geometry != d.crop.geometry; if !(turned_on || (warp_changed && !crop_edited)) { return d; @@ -971,7 +977,7 @@ impl Session { } let id = self.active().ok_or_else(|| EngineError::Other("no active photo".into()))?; let original = self.develop_of(id).unwrap_or_default(); - self.interaction = Some(Interaction { label: label.into(), photo: id, original }); + self.interaction = Some(Interaction { label: label.into(), photo: id, original, crop_edited: false }); Ok(()) } @@ -982,9 +988,15 @@ impl Session { if *cur == *i.original { return Ok(()); } - // Restore the original silently, then commit the final value as one step. + // Restore the original silently, then commit the final value as one step. A crop nobody + // edited goes back to the original's: Constrain Crop fits it again for the final warp, and + // Auto Sync carries the edit, not the active photo's fitted crop (review of #1990). self.apply_silent(Op::SetDevelop { id: i.photo, settings: i.original.clone(), label: i.label.clone(), edited: None })?; - self.set_develop(i.photo, (*cur).clone(), &i.label) + let mut last = (*cur).clone(); + if !i.crop_edited { + last.crop.geometry = i.original.crop.geometry; + } + self.set_develop(i.photo, last, &i.label) } pub fn cancel_interaction(&mut self) -> Result<()> { diff --git a/crates/engine/src/tests_constrain_crop.rs b/crates/engine/src/tests_constrain_crop.rs index 51dcee1c0..9f01ed05d 100644 --- a/crates/engine/src/tests_constrain_crop.rs +++ b/crates/engine/src/tests_constrain_crop.rs @@ -98,3 +98,50 @@ fn auto_sync_fits_each_photos_own_crop() { s.selection = crate::Selection { ids: vec![b], active: Some(b) }; assert!(fits(&s), "and fits B's warp"); } + +/// Review of #1990 (CodeRabbit): the same with a slider drag (an interaction), the path the +/// window uses: the drag fits A's crop live, and its end must not carry that crop over to B. +#[test] +fn auto_sync_after_a_slider_drag_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]); + 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; + s.selection = crate::Selection { ids: vec![a, b], active: Some(a) }; + s.execute("develop.autoSync", &json!({"on": true})).unwrap(); + s.execute("develop.beginInteraction", &json!({"label": "Vertical"})).unwrap(); + for v in [-20.0, -40.0, -60.0] { + vertical(&mut s, v); + } + s.execute("develop.endInteraction", &json!({})).unwrap(); + let (da, db) = (s.develop_of(a).unwrap(), s.develop_of(b).unwrap()); + assert_eq!((da.geometry.vertical, db.geometry.vertical), (-60.0, -60.0)); + 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_before:?} -> {b_after:?}"); + for photo in [a, b] { + s.selection = crate::Selection { ids: vec![photo], active: Some(photo) }; + assert!(fits(&s), "each crop fits its photo's warp"); + } +} + +/// Review of #1990 (CodeRabbit): only the Geometry and Optics eyes change the warp; turning +/// another section's eye off leaves a crop drawn by hand as it is. +#[test] +fn an_unrelated_section_eye_leaves_a_hand_drawn_crop() { + 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(); + s.execute("develop.sectionEnabled", &json!({"section": "light", "enabled": false})).unwrap(); + assert_eq!(crop(&s).rect, Rect::new(0.0, 0.0, 1.0, 1.0), "the Light eye is no warp change"); + s.execute("develop.sectionEnabled", &json!({"section": "geometry", "enabled": false})).unwrap(); + s.execute("develop.sectionEnabled", &json!({"section": "geometry", "enabled": true})).unwrap(); + assert!(fits(&s), "the Geometry eye is one: the crop fits again"); +}