#1927: a slider drag and unrelated section eyes don't misplace crops

Two more cases from the review of #1990: the end of a slider drag replayed
the crop Constrain Crop had fitted during the drag, so Auto Sync still
copied the active photo's fitted crop to the others; the end of an
interaction now starts again from the original crop when the drag didn't
edit the crop. And any section eye counted as a warp change, so turning
Light off could refit a hand-drawn crop: only the Geometry and Optics
eyes do now.

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-18 16:48:52 +00:00
parent 23f00822eb
commit debfa644ec
2 changed files with 64 additions and 5 deletions
+17 -5
View File
@@ -184,6 +184,10 @@ pub struct Interaction {
pub label: String,
pub photo: PhotoId,
pub original: Arc<DevelopSettings>,
/// 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<()> {
+47
View File
@@ -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");
}