diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs index 7bb168169..5b0a0688c 100644 --- a/crates/engine/src/cmd/develop.rs +++ b/crates/engine/src/cmd/develop.rs @@ -242,20 +242,29 @@ pub fn specs() -> Vec { Ok(()) }) }), - cmd!("develop.merge", "Apply Settings JSON", [], None, "{settings: partial DevelopSettings JSON, label?}", has_active, |s, p| { - let partial = p.get("settings").cloned().ok_or_else(|| bad("develop.merge", "missing settings"))?; - let label = str_param(p, "label").unwrap_or("Edit").to_string(); - edit(s, "develop.merge", &label, |d| { - let before = d.clone(); - *d = d.merged(&partial).map_err(|e| bad("develop.merge", e.to_string()))?; - // a turn turns the Upright correction the photo keeps, as Rotate does; a new mode - // drops it; a correction stated in the JSON is taken as given - if partial.pointer("/geometry/upright_transform").is_none() { - crate::upright_transfer::settle_edit(&before, d); - } - Ok(()) - }) - }), + cmd!( + "develop.merge", + "Apply Settings JSON", + [], + None, + "{settings: partial DevelopSettings JSON, label?} (a change of `orientation` turns the photo's Upright correction with it unless `geometry.upright_transform` states a different one; a change of `geometry.upright` drops it to be solved again)", + has_active, + |s, p| { + let partial = p.get("settings").cloned().ok_or_else(|| bad("develop.merge", "missing settings"))?; + let label = str_param(p, "label").unwrap_or("Edit").to_string(); + edit(s, "develop.merge", &label, |d| { + let before = d.clone(); + *d = d.merged(&partial).map_err(|e| bad("develop.merge", e.to_string()))?; + // a turn turns the Upright correction the photo keeps, as Rotate does; a new mode + // drops it; a correction stated in the JSON is taken as given, unless it is the + // photo's own unchanged (settings sent back whole, as develop.get returned them) + if !crate::upright_transfer::states_new_correction(&partial, &before) { + crate::upright_transfer::settle_edit(&before, d); + } + Ok(()) + }) + } + ), cmd!("develop.reset", "Reset Edits", ["Photo"], Some("Cmd+Shift+R"), "{ids?}", has_selection, |s, p| { let ids = s.targets(p); let ops = ids @@ -794,7 +803,7 @@ pub fn specs() -> Vec { } let d = s.develop_of(id).unwrap_or_default(); s.clipboard = Some(lightcraft_develop::extract_groups(&d, &s.copy_groups)); - s.clipboard_source = s.catalog.photo(id).map(|p| crate::upright_transfer::source_of(p, &d)); + s.clipboard_source = s.catalog.photo(id).map(|p| crate::upright_transfer::source_of(p, &d, &s.library_identity)); Ok(json!({"groups": s.copy_groups})) } ), @@ -812,7 +821,7 @@ pub fn specs() -> Vec { let groups = groups_param(p).unwrap_or_else(|| s.copy_groups.clone()); let partial = lightcraft_develop::extract_groups(&s.develop_of(prev).unwrap_or_default(), &groups); let ids: Vec = s.targets(p).into_iter().filter(|id| *id != prev).collect(); - let source = s.catalog.photo(prev).map(|ph| crate::upright_transfer::source_of(ph, &ph.develop)); + let source = s.catalog.photo(prev).map(|ph| crate::upright_transfer::source_of(ph, &ph.develop, &s.library_identity)); let ops = ids .iter() .filter_map(|id| { @@ -841,7 +850,8 @@ pub fn specs() -> Vec { clip = lightcraft_develop::extract_groups(&full, &only); } let ids = s.targets(p); - let source = s.clipboard_source.clone(); + // copied in another library: its photo ids say nothing about this one's + let source = s.clipboard_source.clone().filter(|from| from.taken_in(&s.library_identity)); let ops = ids .iter() .filter_map(|id| { @@ -866,7 +876,7 @@ pub fn specs() -> Vec { let groups = groups_param(p).unwrap_or_else(SettingsGroup::default_copy); let d = s.develop_of(id).unwrap_or_default(); let partial = lightcraft_develop::extract_groups(&d, &groups); - let source = s.catalog.photo(id).map(|ph| crate::upright_transfer::source_of(ph, &d)); + let source = s.catalog.photo(id).map(|ph| crate::upright_transfer::source_of(ph, &d, &s.library_identity)); let ops = s .selection .ids @@ -992,10 +1002,7 @@ pub fn specs() -> Vec { .filter_map(|(id, d)| { // a preset carries no Upright correction: the photo keeps its own, turned with // it, or solves its own for another mode - let mut after = preset.apply(&d, amount); - if let Some(ph) = s.catalog.photo(id) { - crate::upright_transfer::settle(ph, &d, &mut after, &preset.settings, None); - } + let after = s.preset_look(id, &preset, amount).unwrap_or_else(|| preset.apply(&d, amount)); s.develop_op(id, after, &format!("Preset: {}", preset.name)) }) .collect::>(); diff --git a/crates/engine/src/crs.rs b/crates/engine/src/crs.rs index d98dc7acd..cec3920fa 100644 --- a/crates/engine/src/crs.rs +++ b/crates/engine/src/crs.rs @@ -442,7 +442,13 @@ pub fn to_partial(props: &Props, target: Target) -> Value { ); } } - Some(false) => put(o, "crop.geometry", json!({"rect": {"x0": 0.0, "y0": 0.0, "x1": 1.0, "y1": 1.0}, "angle": 0.0})), + Some(false) => { + // "no crop": the edges Lightroom keeps writing beside it are read, not lost adjustments + for k in ["Left", "Top", "Right", "Bottom", "Angle"] { + note(&format!("crs:Crop{k}")); + } + put(o, "crop.geometry", json!({"rect": {"x0": 0.0, "y0": 0.0, "x1": 1.0, "y1": 1.0}, "angle": 0.0})) + } None => {} } out diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index a74380ac5..a22b44f5b 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -357,9 +357,7 @@ pub fn apply_import_defaults(s: &Session, p: &mut Photo) { match preset { Some(pr) => { // a preset carries no Upright correction (an old one may still hold one: not this photo's) - let mut look = pr.apply(&base, 1.0); - crate::upright_transfer::settle(p, &base, &mut look, &pr.settings, None); - let look = std::sync::Arc::new(look); + let look = std::sync::Arc::new(crate::upright_transfer::apply_preset(p, &base, pr, 1.0)); p.develop = look.clone(); p.import_look = (*look != base).then_some(look); } @@ -1339,9 +1337,7 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare } if let Some(preset) = &opts.preset { // after the sidecar: the photo keeps the Upright correction it was given - let mut d = preset.apply(&p.develop, 1.0); - crate::upright_transfer::settle(&p, &p.develop, &mut d, &preset.settings, None); - let d = std::sync::Arc::new(d); + let d = std::sync::Arc::new(crate::upright_transfer::apply_preset(&p, &p.develop, preset, 1.0)); let label = format!("Preset: {}", preset.name); p.develop = d.clone(); p.edited = Some(now.clone()); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7d51206ce..e63778835 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -790,7 +790,7 @@ impl Session { } } // an Upright correction moves only between entries of the same file - let source = self.catalog.photo(id).map(|p| crate::upright_transfer::source_of(p, new)); + let source = self.catalog.photo(id).map(|p| crate::upright_transfer::source_of(p, new, &self.library_identity)); self.selection .ids .iter() @@ -806,6 +806,13 @@ impl Session { .collect() } + /// What `preset` at `amount` makes of photo `id`'s settings, as Apply Preset stores it (its + /// Upright correction settled: [`crate::upright_transfer::apply_preset`]); for previews too. + pub fn preset_look(&self, id: PhotoId, preset: &lightcraft_develop::Preset, amount: f64) -> Option { + let p = self.catalog.photo(id)?; + Some(crate::upright_transfer::apply_preset(p, &p.develop, preset, amount)) + } + /// The op that sets a photo's develop settings and appends a History entry (for batches). pub fn develop_op(&self, id: PhotoId, settings: DevelopSettings, label: &str) -> Option { self.catalog.photo(id)?; diff --git a/crates/engine/src/sidecar.rs b/crates/engine/src/sidecar.rs index 80fa995e7..5c15e3fce 100644 --- a/crates/engine/src/sidecar.rs +++ b/crates/engine/src/sidecar.rs @@ -136,11 +136,23 @@ impl StoredFrame { } } -/// The stored image of the file at `path` as it is now, read from the file (never recorded: a -/// file can change): `None` when it can't be read. -pub fn probe_frame(path: &str) -> Option { - let bytes = std::fs::read(path).ok()?; - crate::files::probe_bytes(path, &bytes).ok()?.stored_frame() +/// Whether partial settings `v` hold a crop that depends on the frame it is read in: anything but +/// "no crop" (the whole image, not straightened), which is the same in every orientation. +fn crop_needs_frame(v: &Value) -> bool { + let Some(g) = v.pointer("/crop/geometry") else { return false }; + let at = |k: &str, want: f64| g.pointer(k).and_then(Value::as_f64).is_some_and(|x| x == want); + !(at("/rect/x0", 0.0) + && at("/rect/y0", 0.0) + && at("/rect/x1", 1.0) + && at("/rect/y1", 1.0) + && g.get("angle").and_then(Value::as_f64).is_none_or(|a| a == 0.0)) +} + +/// The stored image of the file at `path` as it is now, through the session's file probe `probe` +/// (the one import uses, inside its memory budget; never recorded: a file can change): `None` when +/// it can't be read. +pub fn probe_frame(probe: Option<&crate::media::FileProbe>, path: &str) -> Option { + probe?(path).ok()?.stored_frame() } #[derive(Clone, Debug, PartialEq)] @@ -341,22 +353,24 @@ impl SidecarData { self.placed_upright.is_some() } - /// Whether the packet's crop or saved Upright needs the photo's stored frame. + /// Whether the packet's crop or saved Upright needs the photo's stored frame. "No crop" (the + /// whole image, not straightened) is the same in every frame. pub fn needs_frame(&self) -> bool { - self.upright.is_some() || matches!(&self.develop, Some(DevelopPatch::Partial(v)) if v.pointer("/crop/geometry").is_some()) + self.upright.is_some() || matches!(&self.develop, Some(DevelopPatch::Partial(v)) if crop_needs_frame(v)) } /// For a photo already in the library whose file's orientation can't be learnt: leave its crop /// as it is rather than take Lightroom's crop edges in an unknown frame, and say so. Returns - /// whether a crop was held. - pub fn hold_unframed_crop(&mut self) -> bool { + /// the crop held (as the packet wrote it). + pub fn hold_unframed_crop(&mut self) -> Option { if self.frame.is_some() { - return false; + return None; } - let Some(DevelopPatch::Partial(Value::Object(partial))) = self.develop.as_mut() else { return false }; - if partial.remove("crop").is_none() { - return false; + let Some(DevelopPatch::Partial(partial)) = self.develop.as_mut() else { return None }; + if !crop_needs_frame(partial) { + return None; } + let held = partial.as_object_mut()?.remove("crop")?; self.warnings.push(crate::import::ImportWarning { path: String::new(), code: crate::import::ImportWarningCode::UnmappedXmp, @@ -364,7 +378,7 @@ impl SidecarData { mode: None, fields: vec!["Crop (the file's orientation couldn't be read)".into()], }); - true + Some(held) } /// Read the label through the catalog's label names: other editors write the label's name diff --git a/crates/engine/src/sync.rs b/crates/engine/src/sync.rs index ec432d4ee..cab2c4325 100644 --- a/crates/engine/src/sync.rs +++ b/crates/engine/src/sync.rs @@ -78,6 +78,9 @@ pub struct SyncInput { photos: Vec<(Arc, SidecarNaming)>, /// For the colour label names a sidecar may use. labels: Catalog, + /// The session's file probe (import's, inside its memory budget): how a sidecar's crop and + /// saved Upright learn the file's orientation. + probe: Option, } /// What changed in a folder since it came into the library. @@ -181,7 +184,7 @@ impl SyncInput { let namings = s.sidecar_namings(&ids); let photos = ids.iter().zip(namings).filter_map(|(id, naming)| s.catalog.photo(*id).map(|p| (Arc::clone(p), naming))).collect(); let (scan, _) = ScanInput::new(s, &[path.to_string()]); - Ok(SyncInput { folder: path.to_string(), scan, photos, labels: s.catalog.clone() }) + Ok(SyncInput { folder: path.to_string(), scan, photos, labels: s.catalog.clone(), probe: s.media.file_probe.clone() }) } } @@ -192,19 +195,21 @@ impl SyncInput { /// library's photos, which run on several threads too. Stops early when `progress.files.cancel` /// is set. pub fn scan_with(input: SyncInput, progress: &SyncProgress) -> FolderChanges { - let SyncInput { folder, scan, photos, labels } = input; + let SyncInput { folder, scan, photos, labels, probe } = input; let seen: Vec> = photos.iter().map(|(p, _)| Arc::clone(p)).collect(); if !cfg!(target_arch = "wasm32") && !Path::new(&folder).is_dir() { return FolderChanges { path: folder, seen, offline: true, ..Default::default() }; } // (no threads on the web: one after the other there) #[cfg(target_arch = "wasm32")] - let (out, checks) = (crate::import::scan_with(scan, std::slice::from_ref(&folder), &progress.files), check_photos(&photos, &labels, progress)); + let (out, checks) = + (crate::import::scan_with(scan, std::slice::from_ref(&folder), &progress.files), check_photos(&photos, &labels, probe.as_ref(), progress)); #[cfg(not(target_arch = "wasm32"))] let (out, checks) = std::thread::scope(|sc| { let photos = &photos; let labels = &labels; - let checks = sc.spawn(move || check_photos(photos, labels, progress)); + let probe = probe.as_ref(); + let checks = sc.spawn(move || check_photos(photos, labels, probe, progress)); let out = crate::import::scan_with(scan, std::slice::from_ref(&folder), &progress.files); // (a check that panicked counts as nothing found; the scan still reports the rest) (out, checks.join().unwrap_or_default()) @@ -329,11 +334,11 @@ enum Check { News, } -fn check_photo(p: &Photo, naming: SidecarNaming, labels: &Catalog) -> Check { +fn check_photo(p: &Photo, naming: SidecarNaming, labels: &Catalog, probe: Option<&crate::media::FileProbe>) -> Check { let Some(file) = crate::cmd::missing::checked_path(p) else { return Check::Nothing }; if !Path::new(file).exists() { Check::Missing - } else if p.copy_of.is_none() && sidecar_has_news(labels, p, naming) { + } else if p.copy_of.is_none() && sidecar_has_news(labels, p, naming, probe) { Check::News } else { Check::Nothing @@ -342,12 +347,17 @@ fn check_photo(p: &Photo, naming: SidecarNaming, labels: &Catalog) -> Check { /// [`check_photo`] for each photo, in order, on several threads (each file is a wait on the /// disk, not work for the CPU). Photos not reached before a cancel count as nothing found. -fn check_photos(photos: &[(Arc, SidecarNaming)], labels: &Catalog, progress: &SyncProgress) -> Vec { +fn check_photos( + photos: &[(Arc, SidecarNaming)], + labels: &Catalog, + probe: Option<&crate::media::FileProbe>, + progress: &SyncProgress, +) -> Vec { use std::sync::atomic::Ordering::Relaxed; progress.to_check.store(photos.len(), Relaxed); let cancel = &progress.files.cancel; let check = |p: &Photo, naming: SidecarNaming| { - let c = check_photo(p, naming, labels); + let c = check_photo(p, naming, labels, probe); progress.checked.fetch_add(1, Relaxed); c }; @@ -415,7 +425,7 @@ impl Session { /// Whether `p`'s XMP sidecar file was saved after the library last had its say about the photo /// (imported, or edited here) and says something the library doesn't. -fn sidecar_has_news(labels: &Catalog, p: &Photo, naming: SidecarNaming) -> bool { +fn sidecar_has_news(labels: &Catalog, p: &Photo, naming: SidecarNaming, probe: Option<&crate::media::FileProbe>) -> bool { let Source::File { path } = &p.source else { return false }; let Some(file) = crate::sidecar::find_sidecar(path, naming) else { return false }; let Ok(meta) = std::fs::metadata(&file) else { return false }; @@ -430,9 +440,11 @@ fn sidecar_has_news(labels: &Catalog, p: &Photo, naming: SidecarNaming) -> bool } let Ok(text) = std::fs::read_to_string(&file) else { return false }; let Ok(mut sc) = crate::sidecar::parse_sidecar(&text, crate::crs::Target::for_photo(p)) else { return false }; - // compared in the photo's frame, as it would be read; a crop that can't be placed is still news, - // so the synchronize that reads it says why it was left - if framed(&mut sc, path) { + // compared in the photo's frame, as it would be read; a crop that can't be placed (no frame) and + // differs from the photo's is still news, so the synchronize that reads it says why it was left + if let Some(held) = framed(&mut sc, path, probe) + && lightcraft_develop::apply_partial(&p.develop, &serde_json::json!({"crop": held}), 1.0).crop != p.develop.crop + { return true; } let sc = sc.resolve_label(labels); @@ -470,9 +482,10 @@ pub struct SyncWork { import: Option<(crate::import::ImportJob, Vec)>, moved: Vec, missing: Vec, - /// (photo, its file, how its sidecar is named) /// (photo, its file, sidecar naming, white balance target) sidecars: Vec<(PhotoId, String, SidecarNaming, crate::crs::Target)>, + /// The session's file probe, for the files' orientations ([`SyncInput`]). + probe: Option, } /// The catalog half of a synchronize (see [`SyncJob`]). @@ -537,6 +550,7 @@ impl SyncJob { moved: if choice.relink_moved { moved } else { Vec::new() }, missing: if choice.remove_missing { missing } else { Vec::new() }, sidecars, + probe: s.media.file_probe.clone(), }; let commit = SyncCommit { opts, @@ -558,7 +572,7 @@ impl SyncWork { pub fn run(self, progress: &SyncProgress, mut send: impl FnMut(SyncStep) -> bool) { use std::sync::atomic::Ordering::Relaxed; let cancel = &progress.files.cancel; - let SyncWork { import, moved, missing, sidecars } = self; + let SyncWork { import, moved, missing, sidecars, probe } = self; let pieces = import.as_ref().map_or(0, |(_, f)| f.len()) + moved.len() + missing.len() + sidecars.len(); progress.to_check.store(pieces, Relaxed); progress.checked.store(0, Relaxed); @@ -602,7 +616,7 @@ impl SyncWork { if cancel.load(Relaxed) { break; } - read.push((id, path.clone(), read_sidecar(&path, naming, target))); + read.push((id, path.clone(), read_sidecar(&path, naming, target, probe.as_ref()))); step(1); } if !read.is_empty() { @@ -613,7 +627,12 @@ impl SyncWork { /// Read and parse photo file `path`'s XMP sidecar file, its crop and saved Upright in the frame the /// file has now, as import reads them. -fn read_sidecar(path: &str, naming: SidecarNaming, target: crate::crs::Target) -> std::result::Result { +fn read_sidecar( + path: &str, + naming: SidecarNaming, + target: crate::crs::Target, + probe: Option<&crate::media::FileProbe>, +) -> std::result::Result { let file = crate::sidecar::find_sidecar(path, naming).ok_or_else(|| "no XMP sidecar".to_string())?; let len = std::fs::metadata(&file).map_err(|e| format!("{}: {e}", file.display()))?.len(); if len > MAX_SIDECAR_BYTES { @@ -621,18 +640,18 @@ fn read_sidecar(path: &str, naming: SidecarNaming, target: crate::crs::Target) - } let text = std::fs::read_to_string(&file).map_err(|e| format!("{}: {e}", file.display()))?; let mut sc = crate::sidecar::parse_sidecar(&text, target).map_err(|e| format!("{}: {e}", file.display()))?; - framed(&mut sc, path); + framed(&mut sc, path, probe); Ok(sc) } -/// Give a parsed sidecar of the photo whose file is `path` the frame the file has now (read only -/// when the packet's crop or saved Upright needs it). Without one, the photo's crop is left as it -/// is and the report says so; returns whether a crop was held that way. -fn framed(sc: &mut crate::sidecar::SidecarData, path: &str) -> bool { +/// Give a parsed sidecar of the photo whose file is `path` the frame the file has now, through the +/// session's file probe (only when the packet's crop or saved Upright needs it). Without one, the +/// photo's crop is left as it is and the report says so; returns the crop held that way. +fn framed(sc: &mut crate::sidecar::SidecarData, path: &str, probe: Option<&crate::media::FileProbe>) -> Option { if !sc.needs_frame() { - return false; + return None; } - sc.set_frame(crate::sidecar::probe_frame(path)); + sc.set_frame(crate::sidecar::probe_frame(probe, path)); sc.hold_unframed_crop() } diff --git a/crates/engine/src/tests_upright.rs b/crates/engine/src/tests_upright.rs index 7307b9efe..94c8f4960 100644 --- a/crates/engine/src/tests_upright.rs +++ b/crates/engine/src/tests_upright.rs @@ -1016,9 +1016,10 @@ fn sweep_transfer(s: &mut Session, command: &str, groups: &[&str]) { } /// Paste, Sync and Paste from Previous, every source and target EXIF orientation and user turn and -/// the Geometry, Crop and both groups: between different files (9,216 cases) each target keeps its -/// own correction, in the frame it ends up in; between a photo and its virtual copy (1,152 cases) -/// the correction moves into the copy's frame. Undo restores every paste exactly. +/// the Geometry, Crop and both groups: between different files (9,216 cases, each with its own +/// scene) each target keeps its own correction, in the frame it ends up in; within one file +/// (original to copy, copy to original, copy to copy: 3,456 cases) the correction moves into the +/// target's frame. Undo restores every paste exactly. #[test] fn transfers_in_every_frame_and_group() { let (mine, theirs) = (matrix(SLOTS[1]), matrix(SLOTS[2])); @@ -1051,29 +1052,32 @@ fn transfers_in_every_frame_and_group() { } } } - // the same file: a virtual copy of photo 1 - for ua in TURNS { - for ub in TURNS { - let a = sweep_photo(1, "/sweep/a.jpg", fa, ua, mine); - let mut b = sweep_photo(2, "/sweep/a.jpg", fa, ub, mine); - b.copy_of = Some(lightcraft_catalog::PhotoId(1)); - let mut s = Session::new(); - s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(a) }).unwrap(); - s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(b) }).unwrap(); - sweep_transfer(&mut s, command, groups); - let got = dev(&s, lightcraft_catalog::PhotoId(2)); - let user = if groups.contains(&"crop") { ua } else { ub }; - assert!( - same(got.geometry.upright_transform, oracle(fa, user, mine)), - "same file {command} {groups:?} {fa:?} {ua:?} -> {ub:?}" - ); - cases += 1; + // the same file: original to copy, copy to original, copy to copy + for (copy_a, copy_b) in [(None, Some(1)), (Some(2), None), (Some(9), Some(9))] { + for ua in TURNS { + for ub in TURNS { + let mut a = sweep_photo(1, "/sweep/a.jpg", fa, ua, mine); + let mut b = sweep_photo(2, "/sweep/a.jpg", fa, ub, mine); + a.copy_of = copy_a.map(lightcraft_catalog::PhotoId); + b.copy_of = copy_b.map(lightcraft_catalog::PhotoId); + let mut s = Session::new(); + s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(a) }).unwrap(); + s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(b) }).unwrap(); + sweep_transfer(&mut s, command, groups); + let got = dev(&s, lightcraft_catalog::PhotoId(2)); + let user = if groups.contains(&"crop") { ua } else { ub }; + assert!( + same(got.geometry.upright_transform, oracle(fa, user, mine)), + "same file {copy_a:?}/{copy_b:?} {command} {groups:?} {fa:?} {ua:?} -> {ub:?}" + ); + cases += 1; + } } } } } } - assert_eq!(cases, 9216 + 1152); + assert_eq!(cases, 9216 + 3 * 1152); } /// Auto Sync of an edited correction (develop.merge on the active photo) for every source and @@ -1112,3 +1116,171 @@ fn auto_sync_in_every_frame() { } assert_eq!(cases, 1024); } + +// ------------------------------------------------------------------ round four + +/// Settings JSON sent back whole (`develop.get`, the rotation changed, `develop.merge`) states the +/// photo's unchanged correction: it is turned with the photo all the same (it used to stay in the +/// old frame, 15.5 px off), on the photo and, through Auto Sync, on its virtual copy. +#[test] +fn whole_settings_json_round_trip_turns_the_correction() { + let src = temp_dir("upright-round-trip"); + let mut s = Session::new().with_fs(); + let id = import_jpeg(&mut s, &src, "a", 6, Some(&lightroom(1))); + s.execute("library.select", &json!({"ids": [id.0]})).unwrap(); + let copy = lightcraft_catalog::PhotoId(s.execute("photo.virtualCopy", &json!({})).unwrap()["ids"][0].as_u64().unwrap()); + let original = placed(1, Orientation::Rotate90, 60.0, 40.0); + s.selection = crate::Selection { ids: vec![id, copy], active: Some(id) }; + s.execute("photo.rotateRight", &json!({})).unwrap(); + s.selection = crate::Selection { ids: vec![id, copy], active: Some(id) }; + s.execute("develop.autoSync", &json!({"on": true})).unwrap(); + let mut whole = s.execute("develop.get", &json!({"id": id.0})).unwrap(); + whole["orientation"] = json!("Normal"); + s.execute("develop.merge", &json!({"settings": whole})).unwrap(); + for x in [id, copy] { + let d = dev(&s, x); + assert_eq!(d.orientation, Orientation::Normal); + assert!(same(d.geometry.upright_transform, original), "{x:?}: {:?}", d.geometry); + } + let _ = std::fs::remove_dir_all(&src); +} + +/// Copy, open another library, Paste: photo ids start again in each library, so a photo there with +/// the copied photo's id is another file and keeps its own correction (it used to get the copied +/// one). +#[test] +fn paste_after_opening_another_library_keeps_each_photos_own() { + let (lib_a, lib_b, src) = (temp_dir("upright-lib-a"), temp_dir("upright-lib-b"), temp_dir("upright-lib-src")); + let mut s = Session::new().with_fs(); + s.open_library(&lib_a, false).unwrap(); + let a = import_jpeg(&mut s, &src, "a", 1, Some(&lightroom(1))); + s.execute("library.select", &json!({"ids": [a.0]})).unwrap(); + s.execute("develop.copy", &json!({"groups": ["geometry"]})).unwrap(); + s.open_library(&lib_b, false).unwrap(); + let b = import_jpeg(&mut s, &src, "b", 1, Some(&lightroom(1).replace(SLOTS[1], SLOTS[2]))); + assert_eq!(a, b, "the regression needs the same id in both libraries"); + s.execute("library.select", &json!({"ids": [b.0]})).unwrap(); + s.execute("develop.paste", &json!({})).unwrap(); + let own = upright_in_frame(&SavedUpright { mode: Upright::Auto, matrix: matrix(SLOTS[2]) }, Orientation::Normal, 60.0, 40.0).unwrap(); + assert!(same(dev(&s, b).geometry.upright_transform, own), "{:?}", dev(&s, b).geometry); + for d in [lib_a, lib_b, src] { + let _ = std::fs::remove_dir_all(&d); + } +} + +/// "No crop" needs no frame: on a photo whose file can't be read, a sidecar saying HasCrop="False" +/// removes the crop without a warning (it used to be held, warned about, and listed again on every +/// Synchronize Folder). +#[test] +fn a_sidecar_without_a_crop_needs_no_frame() { + let src = temp_dir("upright-no-crop"); + let mut s = Session::new().with_fs(); + let crop = crop_packet(r#"crs:CropLeft="0.206655" crs:CropTop="0" crs:CropRight="1" crs:CropBottom="1" crs:CropAngle="0""#); + let id = import_jpeg(&mut s, &src, "gone", 8, Some(&crop)); + assert!(!dev(&s, id).crop.geometry.is_identity()); + std::fs::write(src.join("gone.jpg"), b"not a photo any more").unwrap(); + let sidecar = src.join("gone.xmp"); + std::fs::write(&sidecar, crop.replace(r#"crs:HasCrop="True""#, r#"crs:HasCrop="False""#)).unwrap(); + let dir = src.to_string_lossy().to_string(); + let sync = |s: &mut Session| s.execute("folder.synchronize", &json!({"path": dir, "readMetadata": true, "importNew": false})).unwrap(); + bump(&sidecar, 5); + let r = sync(&mut s); + assert!(dev(&s, id).crop.geometry.is_identity(), "{r}"); + assert!(r.get("warnings").is_none(), "{r}"); + bump(&sidecar, 10); + let r = sync(&mut s); + assert_eq!(r["read"], 0, "{r}"); + let _ = std::fs::remove_dir_all(&src); +} + +/// Synchronize Folder learns a file's orientation through the session's file probe (the one import +/// uses, inside its memory budget), not by reading the file on its own. +#[test] +fn synchronize_folder_probes_files_through_the_session() { + use std::sync::atomic::{AtomicUsize, Ordering}; + let src = temp_dir("upright-sync-probe"); + let mut s = Session::new().with_fs(); + let (_, fs_probe) = crate::files::fs_hooks(); + let probes = std::sync::Arc::new(AtomicUsize::new(0)); + let counted = probes.clone(); + s.media.file_probe = Some(std::sync::Arc::new(move |p: &str| { + counted.fetch_add(1, Ordering::Relaxed); + fs_probe(p) + })); + let crop = r#"crs:HasCrop="True" crs:CropLeft="0.206655" crs:CropTop="0" crs:CropRight="1" crs:CropBottom="1" crs:CropAngle="0""#; + let packet = |exposure: &str| lightroom(1).replace(r#"crs:Exposure2012="+0.30""#, &format!(r#"crs:Exposure2012="{exposure}" {crop}"#)); + let id = import_jpeg(&mut s, &src, "portrait", 8, Some(&packet("+0.30"))); + let before = dev(&s, id); + probes.store(0, Ordering::Relaxed); + let sidecar = src.join("portrait.xmp"); + std::fs::write(&sidecar, packet("+0.70")).unwrap(); + bump(&sidecar, 5); + let r = s.execute("folder.synchronize", &json!({"path": src.to_string_lossy(), "readMetadata": true, "importNew": false})).unwrap(); + assert_eq!(dev(&s, id).light.exposure, 0.7, "{r}"); + assert_eq!((dev(&s, id).crop, dev(&s, id).geometry), (before.crop, before.geometry)); + assert!(probes.load(Ordering::Relaxed) > 0, "the file's orientation was read around the session's probe"); + let _ = std::fs::remove_dir_all(&src); +} + +/// A preset saved before presets stopped holding a correction still has one: it is ignored at any +/// amount and as an import preset; a photo keeps its own, or gets the mode and solves its own. +#[test] +fn older_presets_holding_a_correction_are_ignored() { + let src = temp_dir("upright-old-preset"); + let mut s = Session::new().with_fs(); + let theirs = upright_in_frame(&SavedUpright { mode: Upright::Auto, matrix: matrix(SLOTS[2]) }, Orientation::Normal, 60.0, 40.0).unwrap(); + s.presets.push(lightcraft_develop::Preset { + id: "user.old-upright".into(), + name: "Old Upright".into(), + group: "User Presets".into(), + settings: json!({"geometry": {"upright": "auto", "upright_transform": theirs}}), + favorite: false, + builtin: false, + }); + let own = import_jpeg(&mut s, &src, "own", 1, Some(&lightroom(1))); + let off = import_jpeg(&mut s, &src, "off", 1, None); + for amount in [50.0, 100.0, 150.0] { + s.execute("library.select", &json!({"ids": [own.0, off.0]})).unwrap(); + s.execute("preset.apply", &json!({"id": "user.old-upright", "amount": amount})).unwrap(); + assert!(same(dev(&s, own).geometry.upright_transform, placed(1, Orientation::Normal, 60.0, 40.0)), "{amount}"); + assert_eq!((dev(&s, off).geometry.upright, dev(&s, off).geometry.upright_transform), (Upright::Auto, None), "{amount}"); + s.execute("edit.undo", &json!({})).unwrap(); + } + // as an import preset, after the sidecar + let photo = src.join("late.jpg"); + write_jpeg_seeded(&photo, 60, 40, 1, 211); + std::fs::write(src.join("late.xmp"), lightroom(1)).unwrap(); + s.execute("library.import", &json!({"paths": [photo.to_string_lossy()], "preset": "user.old-upright"})).unwrap(); + let late = s.catalog.photos().find(|p| p.file_name == "late.jpg").unwrap().id; + assert!(same(dev(&s, late).geometry.upright_transform, placed(1, Orientation::Normal, 60.0, 40.0))); + let _ = std::fs::remove_dir_all(&src); +} + +/// What the Presets panel previews (`Session::preset_look`) is what Apply stores: a preset that +/// changes the mode leaves no correction, one that turns the photo turns it (the panel used to +/// preview `Preset::apply` alone: up to 15.4 px off). +#[test] +fn preset_previews_match_what_apply_stores() { + let src = temp_dir("upright-preset-preview"); + let mut s = Session::new().with_fs(); + let id = import_jpeg(&mut s, &src, "a", 1, Some(&lightroom(1))); + let preset = |pid: &str, settings: Value| lightcraft_develop::Preset { + id: pid.into(), + name: pid.into(), + group: "User Presets".into(), + settings, + favorite: false, + builtin: false, + }; + for p in [preset("vertical", json!({"geometry": {"upright": "vertical"}})), preset("turned", json!({"orientation": "Rotate90"}))] { + s.presets.push(p.clone()); + let preview = s.preset_look(id, &p, 1.0).unwrap(); + s.execute("library.select", &json!({"ids": [id.0]})).unwrap(); + s.execute("preset.apply", &json!({"id": p.id})).unwrap(); + assert_eq!(dev(&s, id), preview, "{}", p.id); + s.execute("edit.undo", &json!({})).unwrap(); + } + let turned = s.preset_look(id, s.presets.iter().find(|p| p.id == "turned").unwrap(), 1.0).unwrap(); + assert!(same(turned.geometry.upright_transform, placed(1, Orientation::Rotate90, 60.0, 40.0))); + let _ = std::fs::remove_dir_all(&src); +} diff --git a/crates/engine/src/upright_transfer.rs b/crates/engine/src/upright_transfer.rs index cfb445cf8..268c24a22 100644 --- a/crates/engine/src/upright_transfer.rs +++ b/crates/engine/src/upright_transfer.rs @@ -18,21 +18,33 @@ use lightcraft_geom::Orientation; use serde_json::Value; /// The photo settings were taken from, as far as its Upright correction is concerned. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug)] pub struct UprightSource { source: Source, master: PhotoId, /// Its Rotate Left / Right when the settings were taken. user: Orientation, + /// The library it was in (photo ids start again in each library). + library: std::sync::Weak<()>, } -/// Photo `p` with settings `s` as the source of a transfer. -pub fn source_of(p: &Photo, s: &DevelopSettings) -> UprightSource { - UprightSource { source: p.source.clone(), master: p.copy_of.unwrap_or(p.id), user: s.orientation } +impl UprightSource { + /// Whether it was taken in the library `identity` is the identity of + /// (`Session::library_identity`). Photo ids belong to one library, so a source from another + /// one can't be matched to a photo by id. + pub fn taken_in(&self, identity: &std::sync::Arc<()>) -> bool { + std::sync::Weak::ptr_eq(&self.library, &std::sync::Arc::downgrade(identity)) + } } -/// Whether photo `p` shows the same file as `from`: the photo itself, its original or another of -/// its virtual copies. +/// Photo `p` with settings `s`, in the library whose identity is `library`, as the source of a +/// transfer. +pub fn source_of(p: &Photo, s: &DevelopSettings, library: &std::sync::Arc<()>) -> UprightSource { + UprightSource { source: p.source.clone(), master: p.copy_of.unwrap_or(p.id), user: s.orientation, library: std::sync::Arc::downgrade(library) } +} + +/// Whether photo `p` (in the library `from` was taken in) shows the same file as `from`: the photo +/// itself, its original or another of its virtual copies, or another entry of the file. fn same_file(from: &UprightSource, p: &Photo) -> bool { from.master == p.copy_of.unwrap_or(p.id) || from.source == p.source } @@ -43,8 +55,8 @@ fn turn(from: Orientation, to: Orientation) -> Orientation { } /// Settings `after` of photo `p` (which had `before`) once `patch` was applied, with its Upright -/// correction settled as the module describes. `source`: the photo the patch was taken from -/// (`None`: none, as for a preset). +/// correction settled as the module describes. `source`: the photo the patch was taken from, in +/// this photo's library (`None`: none, as for a preset). pub fn settle(p: &Photo, before: &DevelopSettings, after: &mut DevelopSettings, patch: &Value, source: Option<&UprightSource>) { if patch.pointer("/geometry/upright_transform").is_none() { return settle_edit(before, after); @@ -73,6 +85,15 @@ pub fn settle_edit(before: &DevelopSettings, after: &mut DevelopSettings) { } } +/// Whether settings JSON `patch` states a correction other than the one `before` holds: settings +/// sent back whole (as `develop.get` returned them) state the photo's own, unchanged, and that one +/// is turned with the photo like any kept correction. +pub fn states_new_correction(patch: &Value, before: &DevelopSettings) -> bool { + patch + .pointer("/geometry/upright_transform") + .is_some_and(|v| serde_json::from_value::>(v.clone()).ok() != Some(before.geometry.upright_transform)) +} + /// `patch` applied to settings `before` of photo `p`, taken from `source` ([`settle`]). pub fn transferred(p: &Photo, before: &DevelopSettings, patch: &Value, source: Option<&UprightSource>) -> DevelopSettings { let mut after = lightcraft_develop::apply_partial(before, patch, 1.0); @@ -80,6 +101,14 @@ pub fn transferred(p: &Photo, before: &DevelopSettings, patch: &Value, source: O after } +/// `preset` at `amount` applied to settings `before` of photo `p`, as Apply Preset stores it: a +/// preset carries no Upright correction ([`settle`] with no source photo). Previews use it too. +pub fn apply_preset(p: &Photo, before: &DevelopSettings, preset: &lightcraft_develop::Preset, amount: f64) -> DevelopSettings { + let mut after = preset.apply(before, amount); + settle(p, before, &mut after, &preset.settings, None); + after +} + /// Preset settings without a saved Upright correction (the mode and the sliders stay). pub fn without_correction(mut settings: Value) -> Value { if let Some(g) = settings.get_mut("geometry").and_then(Value::as_object_mut) { diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 93b0567d8..0a9ae3877 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -188,7 +188,7 @@ pub fn helper_tools(has_ui: bool) -> Vec { tool( "set_develop", "Set develop settings", - "Edit a photo (one undo step each for values and settings). `values` sets sliders by control id (see list_controls), e.g. {\"light.exposure\": 0.7, \"light.contrast\": 20}. `settings` deep-merges a partial develop-settings JSON (as returned by get_develop).", + "Edit a photo (one undo step each for values and settings). `values` sets sliders by control id (see list_controls), e.g. {\"light.exposure\": 0.7, \"light.contrast\": 20}. `settings` deep-merges a partial develop-settings JSON (as returned by get_develop). A change of `orientation` turns the photo's Upright correction with it, also when the settings are sent back whole; only a different `geometry.upright_transform` replaces it.", json!({ "id": photo_id, "values": {"type": "object", "additionalProperties": {"type": "number"}, "description": "controlId → value"}, diff --git a/crates/mcp/tests/upright_round_trip.rs b/crates/mcp/tests/upright_round_trip.rs new file mode 100644 index 000000000..1d642092b --- /dev/null +++ b/crates/mcp/tests/upright_round_trip.rs @@ -0,0 +1,54 @@ +//! MCP `set_develop` with the settings `get_develop` returned, the rotation changed: the photo's +//! Upright correction is turned with it, as Rotate does (issue #205). The settings state the +//! unchanged correction; it used to be kept in the old frame. + +use lightcraft_engine::Session; +use lightcraft_mcp::{Headless, call_tool}; +use serde_json::{Value, json}; + +const AUTO: &str = "1.010477639,-0.008540717,-0.006124940,0.014938050,1.011645918,-0.015359712,-0.007190889,-0.003396748,1.000000000"; + +/// A 60 × 40 JPEG stored on its side (EXIF 6) with a Lightroom sidecar holding a saved Auto matrix. +fn fixture() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("lc-mcp-upright-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let (w, h) = (60usize, 40usize); + let img = lightcraft_raster::Rgba8 { width: w, height: h, data: (0..w * h).map(|i| [(i % w * 4) as u8, (i / w * 6) as u8, 91, 255]).collect() }; + let exif = lightcraft_codecs::exif::minimal_exif(6); + let meta = lightcraft_codecs::EncodeMeta { exif: Some(&exif), ..Default::default() }; + let bytes = + lightcraft_codecs::encode_jpeg(&lightcraft_codecs::EncodeImage::rgba8(&img), 90, lightcraft_codecs::ChromaSubsampling::S444, &meta).unwrap(); + std::fs::write(dir.join("a.jpg"), bytes).unwrap(); + let xmp = format!( + r#""# + ); + std::fs::write(dir.join("a.xmp"), xmp).unwrap(); + dir.join("a.jpg") +} + +fn correction(b: &Headless) -> Value { + let p = b.session.catalog.photos().next().unwrap(); + serde_json::to_value(p.develop.geometry.upright_transform).unwrap() +} + +#[test] +fn set_develop_with_get_develop_settings_turns_the_correction() { + let path = fixture(); + let mut s = Session::new().with_fs(); + s.execute("library.import", &json!({"paths": [path]})).unwrap(); + let id = s.catalog.photos().next().unwrap().id.0; + s.execute("library.select", &json!({"ids": [id]})).unwrap(); + let mut b = Headless::new(s); + let original = correction(&b); + assert!(!original.is_null()); + assert!(!call_tool(&mut b, "command_run", &json!({"id": "photo.rotateRight", "params": {}})).is_error); + assert_ne!(correction(&b), original); + let got = call_tool(&mut b, "get_develop", &json!({"id": id})); + let mut whole = got.structured.clone().unwrap(); + whole["orientation"] = json!("Normal"); + let r = call_tool(&mut b, "set_develop", &json!({"id": id, "settings": whole})); + assert!(!r.is_error, "{:?}", r.structured); + assert_eq!(correction(&b), original); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); +} diff --git a/crates/ui-egui/src/panels/presets.rs b/crates/ui-egui/src/panels/presets.rs index 0968ab67d..8eb32097e 100644 --- a/crates/ui-egui/src/panels/presets.rs +++ b/crates/ui-egui/src/panels/presets.rs @@ -121,7 +121,12 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } else if resp.hovered() { ui.painter().rect_filled(r, 0.0, t.hover.gamma_multiply(0.7)); } - let look = if thumbs || resp.hovered() { current.as_ref().map(|d| pr.apply(d, 1.0)) } else { None }; + // as Apply stores it (an Upright correction settled with the photo) + let look = if thumbs || resp.hovered() { + active.and_then(|id| app.session.preset_look(id, &pr, 1.0)).or_else(|| current.as_ref().map(|d| pr.apply(d, 1.0))) + } else { + None + }; let mut text_x = r.left() + 40.0; if thumbs && let (Some(id), Some(s)) = (active, look.as_ref()) { let tr = Rect::from_min_size(pos2(r.left() + 36.0, r.top() + 5.0), vec2(66.0, row_h - 10.0)); @@ -213,7 +218,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { ui.data(|d| d.get_temp::<(lightcraft_catalog::PhotoId, Preset, String)>(last_hover_id)) && saved_id == id { - app.hover_preview = Some(HoverPreview { label: saved_label, settings: saved_preset.apply(curr, 1.0) }); + let settings = app.session.preset_look(id, &saved_preset, 1.0).unwrap_or_else(|| saved_preset.apply(curr, 1.0)); + app.hover_preview = Some(HoverPreview { label: saved_label, settings }); } } else { ui.data_mut(|d| d.remove::<(lightcraft_catalog::PhotoId, Preset, String)>(last_hover_id)); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 194be3f6a..961f716fa 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -215,7 +215,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). ) | LR-EDIT-OPTICS-DEFRINGE | Defringe | P1 | ✅ | `ctl:optics.defringe*` | no fringe eyedropper | | 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` | LightCraft's own solver; Lightroom's saved Upright from XMP is kept as it is (LR-META-XMP); Rotate Left / Right turns a stored correction with the photo; a correction is the photo's own: Paste / Sync / Auto Sync turn it into virtual copies of the same file and carry only the mode and sliders to other files, presets keep the mode only | +| LR-EDIT-GEOM-UPRIGHT | Upright | P1 | ✅ | `cmd:geometry.upright`, `cmd:geometry.guides` | LightCraft's own solver; Lightroom's saved Upright from XMP is kept as it is (LR-META-XMP); Rotate Left / Right turns a stored correction with the photo; a correction is the photo's own: Paste / Sync / Auto Sync turn it into virtual copies of the same file and carry the rest of the Geometry group, not the correction, to other files; presets never hold one | | 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-GRID | Grid while transforming | P2 | ✅ | `crates/ui-egui/src/panels/detail.rs` | a fine grid over the photo while a geometry slider is dragged | diff --git a/docs/whats-new.md b/docs/whats-new.md index f93b6b0b7..53679eeb6 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -13,12 +13,13 @@ says so. - Rotate Left and Rotate Right turn an Upright correction with the photo. It used to stay as it was, so the photo came out skewed after a turn. Settings JSON that changes the rotation (Apply Settings JSON, MCP `set_develop`, the - CLI) turns it the same way. + CLI) turns it the same way, also when the settings are sent back whole. - An Upright correction stays with its photo: it is the analysis of that one image. Paste, Sync and Auto Sync give a - virtual copy the correction turned its way. Other photos get the Upright mode and the Transform sliders and keep - their own correction, or work out their own when the mode changes, as Lightroom does when it syncs the Upright - mode. Presets keep the mode, never a correction. LightCraft used to copy its own correction to other photos as it - was, which applied one scene's correction to another. + virtual copy the correction turned its way. Other photos get the rest of the Geometry settings (the Upright mode, + the Transform sliders, guides) and keep their own correction, or work out their own when the mode changes; Lightroom + community experts report that Lightroom's Sync with only Upright Mode checked works out each photo too. Presets keep + the mode, never a correction. LightCraft used to copy its own correction to other photos as it was, which applied + one scene's correction to another. - A crop read from an XMP sidecar now lands where Lightroom put it on photos shown on their side (portrait raws), and straightened crops keep Lightroom's frame. A portrait crop used to cut the wrong side of the photo. Synchronize Folder reads sidecars the same way, and its report lists what a sidecar couldn't carry over. diff --git a/docs/xmp-interop.md b/docs/xmp-interop.md index bca8748df..edad42633 100644 --- a/docs/xmp-interop.md +++ b/docs/xmp-interop.md @@ -183,20 +183,26 @@ correction for that scene, in the frame that photo is shown in. So when settings - **Paste, Paste from Previous, Sync, Auto Sync** with the Geometry group: - Between a photo and its virtual copies (the same file), the correction moves, turned to each one's Rotate Left / Right. - - Between different files, only the Upright mode and the Transform sliders move. A target whose mode stays the - same keeps its own correction; one whose mode changes solves its own. This is what Lightroom does when it syncs - the Upright mode. Lightroom's separate option to copy the computed Upright transforms (identical geometry - across brackets) isn't offered. + - Between different files, everything in the Geometry group moves except the correction itself: the Upright + mode, the Transform sliders, Guided Upright's guides and Constrain Crop. A target whose mode stays the same + keeps its own correction; one whose mode changes solves its own. Lightroom community experts report the same + for Lightroom's Synchronize and Copy Settings with only Upright Mode checked, which recompute each photo; + Adobe's documentation doesn't say, and reports about Auto Sync conflict. Lightroom recomputes even a target + whose mode stays the same: LightCraft keeps that target's stored correction, which goes stale if the same Sync + changes its lens corrections (choose the Upright mode again then). Lightroom's separate option to copy the + computed Upright transforms (identical geometry across brackets) isn't offered. - LightCraft used to copy its own solved correction verbatim to other files, applying one scene's correction to another. That is no longer done. - **A crop paste** carries the Rotate Left / Right with it, and the target's own correction turns with it. -- **Presets** keep the Upright mode and the Transform sliders, never a correction. A preset made before this, - which still holds one, has it ignored in the same way. +- **Presets** keep the Upright mode, the Transform sliders and the rest of the Geometry group, never a correction. A + preset made before this, which still holds one, has it ignored in the same way, on Apply, as an import preset and + in the Presets panel's preview. - **Rotate Left / Right** turns the correction with the photo. - **Settings JSON** (`develop.merge`, MCP `set_develop`, the CLI): - - A change of rotation alone turns the correction the photo keeps, as Rotate does. + - A change of rotation turns the correction the photo keeps, as Rotate does, also when the settings are sent + back whole (as `develop.get` returned them, the photo's own correction in them unchanged). - A change of Upright mode drops the correction, to be solved again. - - A correction stated in the JSON is taken as given. + - A different correction stated in the JSON is taken as given. Not measured yet: the Transform sliders (Vertical, Horizontal, Rotate, Aspect, Scale, X / Y Offset) together with a saved Upright (LightCraft applies its own slider mapping on top, as with its own Upright), Level, Vertical and Full