LR-META-XMP: an Upright correction is the photo's own; sidecars are read in the file's current frame (#205)
Third review round. A saved Upright matrix (Lightroom's, or LightCraft's own) is the analysis of one image, so it no longer travels to other files: - Paste, Paste from Previous, Sync and Auto Sync move it only between a photo and its virtual copies, turned by the difference in their Rotate Left / Right. Between different files only the Upright mode and the Transform sliders move: the target keeps its own correction while its mode stays, else solves its own, as Lightroom does when it syncs the Upright mode. Main copied LightCraft's own correction verbatim to other files, one scene's correction applied to another. - Presets keep the mode and sliders, never a correction (preset.create and preset.update strip it; an older preset's, on apply or on import, is ignored the same way). - develop.merge (MCP set_develop, the CLI): a change of rotation alone turns the kept correction, as Rotate does, before the commit and Auto Sync's delta; a change of mode drops it; a correction stated in the JSON is taken as given. - Nothing about the file's frame is recorded in the catalog any more (Photo::file_orientation is gone, so the catalog format is unchanged): Read Metadata from File and Synchronize Folder read the file's orientation from the file each time, so a file whose EXIF changed is read as it is now. - A crop-only sidecar change that can't be placed is still listed by the scan, so Synchronize Folder reports why the crop was kept. The new regressions fail on the previous commit and pass here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1c37bda98f
commit
6471d93b10
14 files changed
+519
-181
No files matched your search
@@ -10,7 +10,6 @@ workspace = true
|
||||
|
||||
[dependencies]
|
||||
lightcraft-develop = { workspace = true }
|
||||
lightcraft-geom = { workspace = true }
|
||||
lightcraft-meta = { workspace = true }
|
||||
lightcraft-raw = { workspace = true }
|
||||
serde = { workspace = true }
|
||||
@@ -21,4 +20,5 @@ log = { workspace = true }
|
||||
web-time = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
lightcraft-geom = { workspace = true }
|
||||
proptest = { workspace = true }
|
||||
@@ -292,13 +292,6 @@ pub struct Photo {
|
||||
/// whether the user changed anything since (see [`crate::local`]).
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub local_baseline: Option<u64>,
|
||||
/// How the file's stored image is turned to be shown (its EXIF orientation, as the decoder
|
||||
/// applies it; `width × height` is the turned size), recorded on import. Lightroom's crop
|
||||
/// edges and saved Upright refer to the stored image, and Upright corrections move between
|
||||
/// photos through it. `None` for photos imported before it was recorded: they are taken as
|
||||
/// stored the way they are shown.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub file_orientation: Option<lightcraft_geom::Orientation>,
|
||||
}
|
||||
|
||||
/// What assisted culling measured on a photo.
|
||||
@@ -366,7 +359,6 @@ impl Photo {
|
||||
analysis: None,
|
||||
preview_only: None,
|
||||
local_baseline: None,
|
||||
file_orientation: None,
|
||||
}
|
||||
}
|
||||
/// A raw file developed from its sensor data: not a rendered image, and not a raw shown from
|
||||
|
||||
@@ -246,7 +246,13 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
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(())
|
||||
})
|
||||
}),
|
||||
@@ -788,7 +794,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
}
|
||||
let d = s.develop_of(id).unwrap_or_default();
|
||||
s.clipboard = Some(lightcraft_develop::extract_groups(&d, &s.copy_groups));
|
||||
s.clipboard_frame = s.catalog.photo(id).and_then(|p| crate::upright_frames::frame_of(p, &d));
|
||||
s.clipboard_source = s.catalog.photo(id).map(|p| crate::upright_transfer::source_of(p, &d));
|
||||
Ok(json!({"groups": s.copy_groups}))
|
||||
}
|
||||
),
|
||||
@@ -806,11 +812,11 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
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<PhotoId> = s.targets(p).into_iter().filter(|id| *id != prev).collect();
|
||||
let source = s.catalog.photo(prev).and_then(|ph| crate::upright_frames::frame_of(ph, &ph.develop));
|
||||
let source = s.catalog.photo(prev).map(|ph| crate::upright_transfer::source_of(ph, &ph.develop));
|
||||
let ops = ids
|
||||
.iter()
|
||||
.filter_map(|id| {
|
||||
s.catalog.photo(*id).map(|ph| (*id, crate::upright_frames::transferred(ph, &ph.develop, &partial, source.as_ref())))
|
||||
s.catalog.photo(*id).map(|ph| (*id, crate::upright_transfer::transferred(ph, &ph.develop, &partial, source.as_ref())))
|
||||
})
|
||||
.filter_map(|(id, d)| s.develop_op(id, d, "Paste from Previous"))
|
||||
.collect::<Vec<_>>();
|
||||
@@ -835,11 +841,11 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
clip = lightcraft_develop::extract_groups(&full, &only);
|
||||
}
|
||||
let ids = s.targets(p);
|
||||
let source = s.clipboard_frame;
|
||||
let source = s.clipboard_source.clone();
|
||||
let ops = ids
|
||||
.iter()
|
||||
.filter_map(|id| {
|
||||
s.catalog.photo(*id).map(|ph| (*id, crate::upright_frames::transferred(ph, &ph.develop, &clip, source.as_ref())))
|
||||
s.catalog.photo(*id).map(|ph| (*id, crate::upright_transfer::transferred(ph, &ph.develop, &clip, source.as_ref())))
|
||||
})
|
||||
.filter_map(|(id, d)| s.develop_op(id, d, "Paste Settings"))
|
||||
.collect::<Vec<_>>();
|
||||
@@ -860,14 +866,16 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
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).and_then(|ph| crate::upright_frames::frame_of(ph, &d));
|
||||
let source = s.catalog.photo(id).map(|ph| crate::upright_transfer::source_of(ph, &d));
|
||||
let ops = s
|
||||
.selection
|
||||
.ids
|
||||
.clone()
|
||||
.into_iter()
|
||||
.filter(|x| *x != id)
|
||||
.filter_map(|x| s.catalog.photo(x).map(|ph| (x, crate::upright_frames::transferred(ph, &ph.develop, &partial, source.as_ref()))))
|
||||
.filter_map(|x| {
|
||||
s.catalog.photo(x).map(|ph| (x, crate::upright_transfer::transferred(ph, &ph.develop, &partial, source.as_ref())))
|
||||
})
|
||||
.filter_map(|(x, dd)| s.develop_op(x, dd, "Sync Settings"))
|
||||
.collect::<Vec<_>>();
|
||||
let n = ops.len();
|
||||
@@ -982,10 +990,11 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
.iter()
|
||||
.filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
|
||||
.filter_map(|(id, d)| {
|
||||
// a preset that turns the photo turns the Upright correction it keeps
|
||||
// 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_frames::settle(ph, &d, &mut after, &preset.settings, None);
|
||||
crate::upright_transfer::settle(ph, &d, &mut after, &preset.settings, None);
|
||||
}
|
||||
s.develop_op(id, after, &format!("Preset: {}", preset.name))
|
||||
})
|
||||
@@ -1000,7 +1009,10 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
let groups = groups_param(p).unwrap_or_else(SettingsGroup::default_copy);
|
||||
let d = s.develop_of(id).unwrap_or_default();
|
||||
let pid = format!("user.{}.{}", s.presets.len(), name.to_lowercase().replace(' ', "-"));
|
||||
s.presets.push(Preset::from_settings(&pid, name, group, &d, &groups));
|
||||
let mut preset = Preset::from_settings(&pid, name, group, &d, &groups);
|
||||
// an Upright correction is this photo's own analysis: a preset keeps the mode
|
||||
preset.settings = crate::upright_transfer::without_correction(preset.settings);
|
||||
s.presets.push(preset);
|
||||
Ok(json!({"id": pid}))
|
||||
}),
|
||||
cmd!("preset.delete", "Delete Preset", [], None, "{id}", always, |s, p| {
|
||||
@@ -1043,14 +1055,14 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
let current = s.develop_of(id).unwrap_or_default().to_json();
|
||||
let groups = groups_param(p);
|
||||
let pr = user_preset(s, p, c)?;
|
||||
pr.settings = match groups {
|
||||
pr.settings = crate::upright_transfer::without_correction(match groups {
|
||||
Some(g) => lightcraft_develop::extract_groups(&serde_json::from_value(current).unwrap_or_default(), &g),
|
||||
// the same keys as before, with the current values
|
||||
None => {
|
||||
let keys: Vec<String> = pr.settings.as_object().map(|o| o.keys().cloned().collect()).unwrap_or_default();
|
||||
Value::Object(keys.into_iter().filter_map(|k| current.get(&k).cloned().map(|v| (k, v))).collect())
|
||||
}
|
||||
};
|
||||
});
|
||||
ok()
|
||||
}
|
||||
),
|
||||
|
||||
@@ -676,13 +676,6 @@ pub fn upright_in_frame(saved: &SavedUpright, orientation: Orientation, stored_w
|
||||
normalized(a.inverse()?.mul(&Homography(saved.matrix)).mul(&a))
|
||||
}
|
||||
|
||||
/// The inverse of [`upright_in_frame`]: a correction in LightCraft's frame, as a matrix on the
|
||||
/// stored image (Lightroom's convention), so it can be put in another photo's frame.
|
||||
pub fn upright_on_stored(h: &Homography, orientation: Orientation, stored_w: f64, stored_h: f64) -> Option<[f64; 9]> {
|
||||
let a = centred_to_stored(orientation, stored_w, stored_h)?;
|
||||
normalized(a.mul(h).mul(&a.inverse()?)).map(|n| n.0)
|
||||
}
|
||||
|
||||
/// Centred coordinates of the oriented image → normalized coordinates of the stored image.
|
||||
fn centred_to_stored(orientation: Orientation, stored_w: f64, stored_h: f64) -> Option<Homography> {
|
||||
if !(stored_w.is_finite() && stored_h.is_finite() && stored_w > 0.0 && stored_h > 0.0) {
|
||||
|
||||
@@ -356,7 +356,10 @@ pub fn apply_import_defaults(s: &Session, p: &mut Photo) {
|
||||
let preset = s.import_defaults.preset_for(raw, &p.meta.camera).and_then(|id| s.presets.iter().find(|x| x.id == id));
|
||||
match preset {
|
||||
Some(pr) => {
|
||||
let look = std::sync::Arc::new(pr.apply(&base, 1.0));
|
||||
// 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);
|
||||
p.develop = look.clone();
|
||||
p.import_look = (*look != base).then_some(look);
|
||||
}
|
||||
@@ -1316,7 +1319,6 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare
|
||||
p.content_hash = info.content_hash;
|
||||
p.embedded_lens = info.embedded_lens;
|
||||
p.preview_only = info.preview_only.clone();
|
||||
p.file_orientation = info.orientation;
|
||||
apply_import_defaults(s, &mut p);
|
||||
if let Some(sc) = &sidecar {
|
||||
crate::sidecar::merge_into(&mut p, sc, &now);
|
||||
@@ -1336,7 +1338,10 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare
|
||||
}
|
||||
}
|
||||
if let Some(preset) = &opts.preset {
|
||||
let d = std::sync::Arc::new(preset.apply(&p.develop, 1.0));
|
||||
// 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 label = format!("Preset: {}", preset.name);
|
||||
p.develop = d.clone();
|
||||
p.edited = Some(now.clone());
|
||||
|
||||
@@ -62,7 +62,7 @@ pub mod segment;
|
||||
pub mod sidecar;
|
||||
pub mod smart;
|
||||
pub mod sync;
|
||||
pub mod upright_frames;
|
||||
pub mod upright_transfer;
|
||||
mod view;
|
||||
pub mod walk;
|
||||
|
||||
@@ -211,9 +211,9 @@ pub struct Session {
|
||||
pub(crate) faces: faces_index::FacesState,
|
||||
/// Copied develop settings (partial JSON) for Paste.
|
||||
pub clipboard: Option<Value>,
|
||||
/// The frame of the photo the clipboard was copied from (its Upright correction is put into
|
||||
/// each target's frame on Paste: [`crate::upright_frames`]).
|
||||
pub clipboard_frame: Option<crate::upright_frames::PhotoFrame>,
|
||||
/// The photo the clipboard was copied from (an Upright correction moves only between entries of
|
||||
/// the same file: [`crate::upright_transfer`]).
|
||||
pub clipboard_source: Option<crate::upright_transfer::UprightSource>,
|
||||
/// The folder on disk the [`LibrarySource::Folder`] view browses.
|
||||
pub browse: Option<Browse>,
|
||||
/// The folder the [`LibrarySource::LibraryFolder`] view shows: the library's photos imported
|
||||
@@ -353,7 +353,7 @@ impl Session {
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
faces: Default::default(),
|
||||
clipboard: None,
|
||||
clipboard_frame: None,
|
||||
clipboard_source: None,
|
||||
meta_clipboard: None,
|
||||
browse: None,
|
||||
library_folder: None,
|
||||
@@ -789,8 +789,8 @@ impl Session {
|
||||
return Vec::new();
|
||||
}
|
||||
}
|
||||
// an Upright correction goes into each photo's own frame
|
||||
let source = self.catalog.photo(id).and_then(|p| crate::upright_frames::frame_of(p, new));
|
||||
// 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));
|
||||
self.selection
|
||||
.ids
|
||||
.iter()
|
||||
@@ -798,7 +798,7 @@ impl Session {
|
||||
.filter_map(|x| self.develop_of(*x).map(|d| (*x, d)))
|
||||
.filter_map(|(x, d)| {
|
||||
let synced = match self.catalog.photo(x) {
|
||||
Some(p) => crate::upright_frames::transferred(p, &d, &delta, source.as_ref()),
|
||||
Some(p) => crate::upright_transfer::transferred(p, &d, &delta, source.as_ref()),
|
||||
None => lightcraft_develop::apply_partial(&d, &delta, 1.0),
|
||||
};
|
||||
(synced != *d).then(|| self.develop_op(x, synced, label)).flatten()
|
||||
|
||||
@@ -136,13 +136,8 @@ impl StoredFrame {
|
||||
}
|
||||
}
|
||||
|
||||
/// Photo `p`'s stored image, from its catalog record (import records the file's orientation).
|
||||
pub fn recorded_frame(p: &Photo) -> Option<StoredFrame> {
|
||||
StoredFrame::of_shown(p.file_orientation?, f64::from(p.width), f64::from(p.height))
|
||||
}
|
||||
|
||||
/// The stored image of the file at `path`, read from the file (photos imported before the
|
||||
/// orientation was recorded): `None` when it can't be read.
|
||||
/// 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<StoredFrame> {
|
||||
let bytes = std::fs::read(path).ok()?;
|
||||
crate::files::probe_bytes(path, &bytes).ok()?.stored_frame()
|
||||
@@ -352,21 +347,24 @@ impl SidecarData {
|
||||
}
|
||||
|
||||
/// 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.
|
||||
pub fn hold_unframed_crop(&mut self) {
|
||||
/// 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 {
|
||||
if self.frame.is_some() {
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
let Some(DevelopPatch::Partial(Value::Object(partial))) = self.develop.as_mut() else { return };
|
||||
if partial.remove("crop").is_some() {
|
||||
self.warnings.push(crate::import::ImportWarning {
|
||||
path: String::new(),
|
||||
code: crate::import::ImportWarningCode::UnmappedXmp,
|
||||
profile: None,
|
||||
mode: None,
|
||||
fields: vec!["Crop (the file's orientation couldn't be read)".into()],
|
||||
});
|
||||
let Some(DevelopPatch::Partial(Value::Object(partial))) = self.develop.as_mut() else { return false };
|
||||
if partial.remove("crop").is_none() {
|
||||
return false;
|
||||
}
|
||||
self.warnings.push(crate::import::ImportWarning {
|
||||
path: String::new(),
|
||||
code: crate::import::ImportWarningCode::UnmappedXmp,
|
||||
profile: None,
|
||||
mode: None,
|
||||
fields: vec!["Crop (the file's orientation couldn't be read)".into()],
|
||||
});
|
||||
true
|
||||
}
|
||||
|
||||
/// Read the label through the catalog's label names: other editors write the label's name
|
||||
@@ -661,11 +659,13 @@ impl Session {
|
||||
let Some((packet, from)) = read_packet(orig, p.kind, self.sidecar_naming(id)) else { return Ok(None) };
|
||||
let mut sc = parse_sidecar(&packet, crate::crs::Target::for_photo(p)).map_err(|e| EngineError::Other(format!("{}: {e}", from.display())))?;
|
||||
if sc.needs_frame() {
|
||||
// Lightroom's crop and saved Upright are on the stored image: the catalog or the file says
|
||||
// how it is oriented
|
||||
let frame = recorded_frame(p).or_else(|| {
|
||||
crate::import::probe_paths(self, &[orig.to_string()]).into_iter().next().and_then(std::result::Result::ok)?.stored_frame()
|
||||
});
|
||||
// Lightroom's crop and saved Upright are on the stored image: the file, as it is now,
|
||||
// says how it is oriented
|
||||
let frame = crate::import::probe_paths(self, &[orig.to_string()])
|
||||
.into_iter()
|
||||
.next()
|
||||
.and_then(std::result::Result::ok)
|
||||
.and_then(|i| i.stored_frame());
|
||||
sc.set_frame(frame);
|
||||
sc.hold_unframed_crop();
|
||||
}
|
||||
|
||||
+22
-23
@@ -430,8 +430,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
|
||||
framed(&mut sc, crate::sidecar::recorded_frame(p), path);
|
||||
// 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) {
|
||||
return true;
|
||||
}
|
||||
let sc = sc.resolve_label(labels);
|
||||
let mut q = p.clone();
|
||||
crate::sidecar::merge_into(&mut q, &sc, "");
|
||||
@@ -468,8 +471,8 @@ pub struct SyncWork {
|
||||
moved: Vec<FolderMoved>,
|
||||
missing: Vec<FolderPhoto>,
|
||||
/// (photo, its file, how its sidecar is named)
|
||||
/// (photo, its file, sidecar naming, white balance target, its stored frame when recorded)
|
||||
sidecars: Vec<(PhotoId, String, SidecarNaming, crate::crs::Target, Option<crate::sidecar::StoredFrame>)>,
|
||||
/// (photo, its file, sidecar naming, white balance target)
|
||||
sidecars: Vec<(PhotoId, String, SidecarNaming, crate::crs::Target)>,
|
||||
}
|
||||
|
||||
/// The catalog half of a synchronize (see [`SyncJob`]).
|
||||
@@ -523,7 +526,7 @@ impl SyncJob {
|
||||
.zip(namings)
|
||||
.filter_map(|(m, naming)| {
|
||||
let p = s.catalog.photo(PhotoId(m.id))?;
|
||||
Some((p.id, m.path, naming, crate::crs::Target::for_photo(p), crate::sidecar::recorded_frame(p)))
|
||||
Some((p.id, m.path, naming, crate::crs::Target::for_photo(p)))
|
||||
})
|
||||
.collect()
|
||||
} else {
|
||||
@@ -595,11 +598,11 @@ impl SyncWork {
|
||||
return;
|
||||
}
|
||||
let mut read = Vec::with_capacity(sidecars.len());
|
||||
for (id, path, naming, target, frame) in sidecars {
|
||||
for (id, path, naming, target) in sidecars {
|
||||
if cancel.load(Relaxed) {
|
||||
break;
|
||||
}
|
||||
read.push((id, path.clone(), read_sidecar(&path, naming, target, frame)));
|
||||
read.push((id, path.clone(), read_sidecar(&path, naming, target)));
|
||||
step(1);
|
||||
}
|
||||
if !read.is_empty() {
|
||||
@@ -608,14 +611,9 @@ impl SyncWork {
|
||||
}
|
||||
}
|
||||
|
||||
/// Read and parse photo file `path`'s XMP sidecar file, its crop and saved Upright in the photo's
|
||||
/// stored frame (`frame`, else read from the file), as import reads them.
|
||||
fn read_sidecar(
|
||||
path: &str,
|
||||
naming: SidecarNaming,
|
||||
target: crate::crs::Target,
|
||||
frame: Option<crate::sidecar::StoredFrame>,
|
||||
) -> std::result::Result<crate::sidecar::SidecarData, String> {
|
||||
/// 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<crate::sidecar::SidecarData, String> {
|
||||
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 {
|
||||
@@ -623,18 +621,19 @@ fn read_sidecar(
|
||||
}
|
||||
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, frame, path);
|
||||
framed(&mut sc, path);
|
||||
Ok(sc)
|
||||
}
|
||||
|
||||
/// Give a parsed sidecar of the photo whose file is `path` its stored frame: `frame` when the
|
||||
/// catalog recorded it, else read from the file (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.
|
||||
fn framed(sc: &mut crate::sidecar::SidecarData, frame: Option<crate::sidecar::StoredFrame>, path: &str) {
|
||||
if sc.needs_frame() {
|
||||
sc.set_frame(frame.or_else(|| crate::sidecar::probe_frame(path)));
|
||||
sc.hold_unframed_crop();
|
||||
/// 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 {
|
||||
if !sc.needs_frame() {
|
||||
return false;
|
||||
}
|
||||
sc.set_frame(crate::sidecar::probe_frame(path));
|
||||
sc.hold_unframed_crop()
|
||||
}
|
||||
|
||||
impl SyncCommit {
|
||||
|
||||
@@ -391,7 +391,12 @@ fn write_turned_jpeg(path: &Path) {
|
||||
|
||||
/// A `w × h` JPEG (stored size) with EXIF orientation `exif`.
|
||||
fn write_jpeg(path: &Path, w: usize, h: usize, exif: u16) {
|
||||
let data: Vec<[u8; 4]> = (0..w * h).map(|i| [(i % w * 6) as u8, (i / w * 9) as u8, 30, 255]).collect();
|
||||
write_jpeg_seeded(path, w, h, exif, 30);
|
||||
}
|
||||
|
||||
/// [`write_jpeg`] with its own pixels (`seed`), so two of them aren't duplicates.
|
||||
fn write_jpeg_seeded(path: &Path, w: usize, h: usize, exif: u16, seed: u8) {
|
||||
let data: Vec<[u8; 4]> = (0..w * h).map(|i| [(i % w * 6) as u8, (i / w * 9) as u8, seed, 255]).collect();
|
||||
let img = lightcraft_raster::Rgba8 { width: w, height: h, data };
|
||||
let exif = lightcraft_codecs::exif::minimal_exif(exif);
|
||||
let meta = lightcraft_codecs::EncodeMeta { exif: Some(&exif), ..Default::default() };
|
||||
@@ -681,7 +686,8 @@ fn same(a: Option<Homography>, b: Homography) -> bool {
|
||||
/// A 60 × 40 JPEG `name` with EXIF `exif` (and, with `sidecar`, that XMP next to it), imported.
|
||||
fn import_jpeg(s: &mut Session, dir: &Path, name: &str, exif: u16, sidecar: Option<&str>) -> lightcraft_catalog::PhotoId {
|
||||
let photo = dir.join(format!("{name}.jpg"));
|
||||
write_jpeg(&photo, 60, 40, exif);
|
||||
// each name its own pixels: identical files would be skipped as duplicates
|
||||
write_jpeg_seeded(&photo, 60, 40, exif, name.bytes().fold(17u8, |a, b| a.wrapping_mul(31).wrapping_add(b)));
|
||||
if let Some(x) = sidecar {
|
||||
std::fs::write(dir.join(format!("{name}.xmp")), x).unwrap();
|
||||
}
|
||||
@@ -718,25 +724,38 @@ fn transferred_upright_lands_in_the_target_photo_frame() {
|
||||
}
|
||||
}
|
||||
|
||||
/// Between files with different orientations the correction goes through the stored image, as
|
||||
/// Lightroom keeps it: into an EXIF-8 photo, and into an EXIF-6 photo the user turned right.
|
||||
/// A saved Upright matrix is one photo's own analysis: between different files only the mode and
|
||||
/// the Transform sliders move (as Lightroom does when it syncs the Upright mode). A target with the
|
||||
/// same mode keeps its own correction; one whose mode changes solves its own. (Converting the
|
||||
/// source's matrix into another file's frame was wrong whenever a frame wasn't recorded.)
|
||||
#[test]
|
||||
fn transferred_upright_follows_each_files_orientation() {
|
||||
let src = temp_dir("upright-transfer-exif");
|
||||
let mut s = Session::new().with_fs();
|
||||
let a = import_jpeg(&mut s, &src, "a", 1, Some(&lightroom(1)));
|
||||
let b = import_jpeg(&mut s, &src, "b", 8, None);
|
||||
let c = import_jpeg(&mut s, &src, "c", 6, None);
|
||||
s.execute("library.select", &json!({"ids": [c.0]})).unwrap();
|
||||
s.execute("photo.rotateRight", &json!({})).unwrap();
|
||||
s.execute("library.select", &json!({"ids": [a.0]})).unwrap();
|
||||
s.execute("develop.copy", &json!({"groups": ["geometry"]})).unwrap();
|
||||
s.execute("library.select", &json!({"ids": [b.0, c.0]})).unwrap();
|
||||
s.execute("develop.paste", &json!({})).unwrap();
|
||||
assert!(same(dev(&s, b).geometry.upright_transform, placed(1, Orientation::Rotate270, 60.0, 40.0)), "EXIF 8: {:?}", dev(&s, b).geometry);
|
||||
let c_frame = Orientation::Rotate90.then(Orientation::Rotate90);
|
||||
assert!(same(dev(&s, c).geometry.upright_transform, placed(1, c_frame, 60.0, 40.0)), "EXIF 6 turned right: {:?}", dev(&s, c).geometry);
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
fn transfers_between_files_carry_the_upright_mode_not_the_matrix() {
|
||||
for command in ["develop.paste", "develop.sync", "develop.pastePrevious"] {
|
||||
let src = temp_dir(&format!("upright-between-files-{}", command.replace('.', "-")));
|
||||
let mut s = Session::new().with_fs();
|
||||
let a = import_jpeg(&mut s, &src, "a", 8, Some(&lightroom(1)));
|
||||
// b has its own saved Auto matrix (another one), c has Upright Off
|
||||
let b = import_jpeg(&mut s, &src, "b", 8, Some(&lightroom(1).replace(SLOTS[1], SLOTS[2])));
|
||||
let c = import_jpeg(&mut s, &src, "c", 6, None);
|
||||
let own_b = upright_in_frame(&SavedUpright { mode: Upright::Auto, matrix: matrix(SLOTS[2]) }, Orientation::Rotate270, 60.0, 40.0).unwrap();
|
||||
assert!(same(dev(&s, b).geometry.upright_transform, own_b));
|
||||
s.execute("library.select", &json!({"ids": [a.0]})).unwrap();
|
||||
s.execute("develop.copy", &json!({"groups": ["geometry"]})).unwrap();
|
||||
for target in [b, c] {
|
||||
if command == "develop.sync" {
|
||||
s.selection = crate::Selection { ids: vec![a, target], active: Some(a) };
|
||||
} else {
|
||||
s.execute("library.select", &json!({"ids": [a.0]})).unwrap();
|
||||
s.execute("library.select", &json!({"ids": [target.0]})).unwrap();
|
||||
}
|
||||
s.execute(command, &json!({"groups": ["geometry"]})).unwrap();
|
||||
}
|
||||
let gb = dev(&s, b).geometry;
|
||||
assert!(gb.upright == Upright::Auto && same(gb.upright_transform, own_b), "{command}: b keeps its own: {gb:?}");
|
||||
let gc = dev(&s, c).geometry;
|
||||
assert_eq!((gc.upright, gc.upright_transform), (Upright::Auto, None), "{command}: c solves its own");
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
}
|
||||
}
|
||||
|
||||
/// A crop paste carries the source's rotation: the target's own Upright correction turns with it.
|
||||
@@ -830,3 +849,266 @@ fn synchronize_folder_reads_sidecars_in_the_photo_frame() {
|
||||
assert!(warned, "{r}");
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------ round three: presets, merge, stale frames
|
||||
|
||||
/// The catalog keeps its format: nothing about the file's frame is written into it (a recorded
|
||||
/// frame would be dropped by an older build that reads format 7, and go stale when the file
|
||||
/// changes).
|
||||
#[test]
|
||||
fn catalogs_keep_their_format() {
|
||||
let src = temp_dir("upright-catalog-format");
|
||||
let mut s = Session::new().with_fs();
|
||||
import_jpeg(&mut s, &src, "a", 8, Some(&lightroom(1)));
|
||||
assert!(!s.catalog.to_snapshot().contains("file_orientation"));
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
}
|
||||
|
||||
/// Sidecars are read in the frame the file has now: a file whose EXIF orientation changed (8 to 6)
|
||||
/// gets its crop and saved Upright in the new frame on Read Metadata from File and Synchronize
|
||||
/// Folder (a frame recorded at import used to stay, 19 px off, the crop on the wrong end).
|
||||
#[test]
|
||||
fn sidecars_are_read_in_the_files_current_frame() {
|
||||
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}"#));
|
||||
for via_sync in [false, true] {
|
||||
let src = temp_dir(&format!("upright-current-frame-{via_sync}"));
|
||||
let mut s = Session::new().with_fs();
|
||||
let id = import_jpeg(&mut s, &src, "turned", 8, Some(&packet("+0.30")));
|
||||
write_jpeg_seeded(&src.join("turned.jpg"), 60, 40, 6, b"turned".iter().fold(17u8, |a, b| a.wrapping_mul(31).wrapping_add(*b)));
|
||||
let sidecar = src.join("turned.xmp");
|
||||
std::fs::write(&sidecar, packet("+0.60")).unwrap();
|
||||
if via_sync {
|
||||
bump(&sidecar, 5);
|
||||
let r = s.execute("folder.synchronize", &json!({"path": src.to_string_lossy(), "readMetadata": true, "importNew": false})).unwrap();
|
||||
assert_eq!(r["read"], 1, "{r}");
|
||||
} else {
|
||||
s.execute("photo.readMetadataFromFile", &json!({"ids": [id.0]})).unwrap();
|
||||
}
|
||||
let d = dev(&s, id);
|
||||
assert_eq!(d.light.exposure, 0.6);
|
||||
let r = [d.crop.geometry.rect.x0, d.crop.geometry.rect.y0, d.crop.geometry.rect.x1, d.crop.geometry.rect.y1];
|
||||
assert_eq!(r.map(|v| (v * 1e6).round() / 1e6), [0.0, 0.206655, 1.0, 1.0], "sync: {via_sync}");
|
||||
assert!(same(d.geometry.upright_transform, placed(1, Orientation::Rotate90, 60.0, 40.0)), "sync: {via_sync}: {:?}", d.geometry);
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
}
|
||||
}
|
||||
|
||||
/// A preset keeps the Upright mode and the Transform sliders, never a saved correction: made from a
|
||||
/// photo turned right and applied after it is turned back, the photo keeps its own correction (the
|
||||
/// preset used to restore the quarter-turned one); a photo with another mode solves its own.
|
||||
#[test]
|
||||
fn presets_carry_the_upright_mode_not_a_correction() {
|
||||
let src = temp_dir("upright-presets");
|
||||
let mut s = Session::new().with_fs();
|
||||
let id = import_jpeg(&mut s, &src, "a", 1, Some(&lightroom(1)));
|
||||
let other = import_jpeg(&mut s, &src, "b", 1, None);
|
||||
s.execute("library.select", &json!({"ids": [id.0]})).unwrap();
|
||||
s.execute("photo.rotateRight", &json!({})).unwrap();
|
||||
let pid = s.execute("preset.create", &json!({"name": "Upright", "groups": ["geometry"]})).unwrap()["id"].as_str().unwrap().to_string();
|
||||
let preset = s.presets.iter().find(|p| p.id == pid).unwrap().clone();
|
||||
assert!(preset.settings.pointer("/geometry/upright_transform").is_none_or(Value::is_null), "{}", preset.settings);
|
||||
s.execute("photo.rotateLeft", &json!({})).unwrap();
|
||||
s.execute("preset.apply", &json!({"id": pid})).unwrap();
|
||||
assert!(same(dev(&s, id).geometry.upright_transform, placed(1, Orientation::Normal, 60.0, 40.0)), "{:?}", dev(&s, id).geometry);
|
||||
s.execute("library.select", &json!({"ids": [other.0]})).unwrap();
|
||||
s.execute("preset.apply", &json!({"id": pid})).unwrap();
|
||||
assert_eq!((dev(&s, other).geometry.upright, dev(&s, other).geometry.upright_transform), (Upright::Auto, None));
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
}
|
||||
|
||||
/// Settings JSON (develop.merge: MCP `set_develop`, the CLI) that only turns the photo turns the
|
||||
/// correction it keeps, as Rotate does; one that changes the mode drops it to be solved again; an
|
||||
/// explicit correction is taken as given. Undo restores each step.
|
||||
#[test]
|
||||
fn merging_settings_json_keeps_upright_in_the_photo_frame() {
|
||||
let src = temp_dir("upright-merge");
|
||||
let mut s = Session::new().with_fs();
|
||||
let id = import_jpeg(&mut s, &src, "a", 1, Some(&lightroom(1)));
|
||||
s.execute("library.select", &json!({"ids": [id.0]})).unwrap();
|
||||
let original = placed(1, Orientation::Normal, 60.0, 40.0);
|
||||
s.execute("photo.rotateRight", &json!({})).unwrap();
|
||||
s.execute("develop.merge", &json!({"settings": {"orientation": "Normal"}})).unwrap();
|
||||
assert!(same(dev(&s, id).geometry.upright_transform, original), "{:?}", dev(&s, id).geometry);
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert!(same(dev(&s, id).geometry.upright_transform, placed(1, Orientation::Rotate90, 60.0, 40.0)));
|
||||
s.execute("edit.redo", &json!({})).unwrap();
|
||||
let given = placed(2, Orientation::Rotate90, 60.0, 40.0);
|
||||
s.execute("develop.merge", &json!({"settings": {"orientation": "Rotate90", "geometry": {"upright_transform": given}}})).unwrap();
|
||||
assert!(same(dev(&s, id).geometry.upright_transform, given));
|
||||
s.execute("develop.merge", &json!({"settings": {"geometry": {"upright": "vertical"}}})).unwrap();
|
||||
assert_eq!((dev(&s, id).geometry.upright, dev(&s, id).geometry.upright_transform), (Upright::Vertical, None));
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
}
|
||||
|
||||
/// A sidecar whose only change is a crop the photo's file can't be read to place is still reported
|
||||
/// by Synchronize Folder (holding the crop used to hide the change, and with it the warning).
|
||||
#[test]
|
||||
fn a_crop_only_change_that_cant_be_placed_is_reported() {
|
||||
let src = temp_dir("upright-held-crop");
|
||||
let mut s = Session::new().with_fs();
|
||||
let crop = |left: &str| crop_packet(&format!(r#"crs:CropLeft="{left}" crs:CropTop="0" crs:CropRight="1" crs:CropBottom="1" crs:CropAngle="0""#));
|
||||
let id = import_jpeg(&mut s, &src, "gone", 8, Some(&crop("0.206655")));
|
||||
let before = dev(&s, id);
|
||||
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("0.3")).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).crop, before.crop, "{r}");
|
||||
let held = r["warnings"].as_array().into_iter().flatten().any(|w| w["code"] == "unmappedXmp" && w["fields"].to_string().contains("Crop"));
|
||||
assert!(held, "{r}");
|
||||
let _ = std::fs::remove_dir_all(&src);
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------ sweeps over every frame
|
||||
|
||||
/// The correction `m` (a matrix on the stored 60 × 40 image) in the frame of a photo whose file is
|
||||
/// turned by `file` and then by the user's `user`, built from coordinate maps alone.
|
||||
fn oracle(file: Orientation, user: Orientation, m: [f64; 9]) -> Homography {
|
||||
let (sw, sh) = if file.swaps_axes() { (40.0, 60.0) } else { (60.0, 40.0) };
|
||||
let (ow, oh) = if user.swaps_axes() { (sh, sw) } else { (sw, sh) };
|
||||
let l = f64::max(ow, oh) / 2.0;
|
||||
let at = |x: f64, y: f64| {
|
||||
let (u, v) = file.map(x * 60.0, y * 40.0, 60.0, 40.0);
|
||||
let (u, v) = user.map(u, v, sw, sh);
|
||||
Point::new((u - ow / 2.0) / l, (v - oh / 2.0) / l)
|
||||
};
|
||||
let (o, x, y) = (at(0.0, 0.0), at(1.0, 0.0), at(0.0, 1.0));
|
||||
let c = Homography([x.x - o.x, y.x - o.x, o.x, x.y - o.y, y.y - o.y, o.y, 0.0, 0.0, 1.0]);
|
||||
let h = c.mul(&Homography(m)).mul(&c.inverse().unwrap());
|
||||
Homography(h.0.map(|v| v / h.0[8]))
|
||||
}
|
||||
|
||||
const TURNS: [Orientation; 4] = [Orientation::Normal, Orientation::Rotate90, Orientation::Rotate180, Orientation::Rotate270];
|
||||
|
||||
/// A photo of a 60 × 40 file (`path`) with EXIF `file`, shown turned `user`, with saved Auto
|
||||
/// correction `m` in its frame.
|
||||
fn sweep_photo(id: u64, path: &str, file: Orientation, user: Orientation, m: [f64; 9]) -> lightcraft_catalog::Photo {
|
||||
let (w, h) = if file.swaps_axes() { (40, 60) } else { (60, 40) };
|
||||
let mut p = lightcraft_catalog::Photo::new(
|
||||
lightcraft_catalog::PhotoId(id),
|
||||
lightcraft_catalog::Source::File { path: path.into() },
|
||||
"a.jpg",
|
||||
"JPEG",
|
||||
w,
|
||||
h,
|
||||
"2026-01-01T00:00:00",
|
||||
);
|
||||
let mut d = DevelopSettings { orientation: user, ..Default::default() };
|
||||
d.geometry.upright = Upright::Auto;
|
||||
d.geometry.upright_transform = Some(oracle(file, user, m));
|
||||
p.develop = std::sync::Arc::new(d);
|
||||
p
|
||||
}
|
||||
|
||||
fn sweep_transfer(s: &mut Session, command: &str, groups: &[&str]) {
|
||||
let (a, b) = (lightcraft_catalog::PhotoId(1), lightcraft_catalog::PhotoId(2));
|
||||
s.selection = crate::Selection::single(a);
|
||||
s.execute("develop.copy", &json!({"groups": groups})).unwrap();
|
||||
if command == "develop.sync" {
|
||||
s.selection = crate::Selection { ids: vec![a, b], active: Some(a) };
|
||||
} else {
|
||||
s.previous_active = Some(a);
|
||||
s.selection = crate::Selection::single(b);
|
||||
}
|
||||
s.execute(command, &json!({"groups": groups})).unwrap();
|
||||
}
|
||||
|
||||
/// 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.
|
||||
#[test]
|
||||
fn transfers_in_every_frame_and_group() {
|
||||
let (mine, theirs) = (matrix(SLOTS[1]), matrix(SLOTS[2]));
|
||||
let mut cases = 0;
|
||||
for command in ["develop.paste", "develop.sync", "develop.pastePrevious"] {
|
||||
for groups in [&["geometry"][..], &["crop"][..], &["geometry", "crop"][..]] {
|
||||
for fa in ORIENTATIONS {
|
||||
for fb in ORIENTATIONS {
|
||||
for ua in TURNS {
|
||||
for ub in TURNS {
|
||||
let a = sweep_photo(1, "/sweep/a.jpg", fa, ua, mine);
|
||||
let b = sweep_photo(2, "/sweep/b.jpg", fb, ub, theirs);
|
||||
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();
|
||||
let before = dev(&s, lightcraft_catalog::PhotoId(2));
|
||||
sweep_transfer(&mut s, command, groups);
|
||||
let got = dev(&s, lightcraft_catalog::PhotoId(2));
|
||||
let user = if groups.contains(&"crop") { ua } else { ub };
|
||||
assert_eq!(got.orientation, user);
|
||||
assert!(
|
||||
same(got.geometry.upright_transform, oracle(fb, user, theirs)),
|
||||
"{command} {groups:?} {fa:?}/{ua:?} -> {fb:?}/{ub:?}"
|
||||
);
|
||||
if command == "develop.paste" {
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
assert_eq!(dev(&s, lightcraft_catalog::PhotoId(2)), before);
|
||||
}
|
||||
cases += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
assert_eq!(cases, 9216 + 1152);
|
||||
}
|
||||
|
||||
/// Auto Sync of an edited correction (develop.merge on the active photo) for every source and
|
||||
/// target EXIF orientation and user turn (1,024 cases): a virtual copy of the same file gets it in
|
||||
/// its own frame, another file keeps its own.
|
||||
#[test]
|
||||
fn auto_sync_in_every_frame() {
|
||||
let (mine, theirs, edited) = (matrix(SLOTS[1]), matrix(SLOTS[2]), matrix(SLOTS[4]));
|
||||
let mut cases = 0;
|
||||
for fa in ORIENTATIONS {
|
||||
for fb in ORIENTATIONS {
|
||||
for ua in TURNS {
|
||||
for ub in TURNS {
|
||||
let same_file = fa == fb && cases % 2 == 0;
|
||||
let a = sweep_photo(1, "/sweep/a.jpg", fa, ua, mine);
|
||||
let mut b = if same_file { sweep_photo(2, "/sweep/a.jpg", fb, ub, mine) } else { sweep_photo(2, "/sweep/b.jpg", fb, ub, theirs) };
|
||||
if same_file {
|
||||
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();
|
||||
s.selection = crate::Selection {
|
||||
ids: vec![lightcraft_catalog::PhotoId(1), lightcraft_catalog::PhotoId(2)],
|
||||
active: Some(lightcraft_catalog::PhotoId(1)),
|
||||
};
|
||||
s.auto_sync = true;
|
||||
s.execute("develop.merge", &json!({"settings": {"geometry": {"upright_transform": oracle(fa, ua, edited)}}})).unwrap();
|
||||
let got = dev(&s, lightcraft_catalog::PhotoId(2)).geometry.upright_transform;
|
||||
let want = if same_file { oracle(fb, ub, edited) } else { oracle(fb, ub, theirs) };
|
||||
assert!(same(got, want), "same file {same_file}: {fa:?}/{ua:?} -> {fb:?}/{ub:?}");
|
||||
cases += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
assert_eq!(cases, 1024);
|
||||
}
|
||||
@@ -1,61 +0,0 @@
|
||||
//! Upright corrections belong to the frame of the photo they are stored with
|
||||
//! (`Geometry::upright_transform`: centred coordinates of the photo as shown, after its file's
|
||||
//! orientation and its own Rotate Left / Right). Settings moved between photos (Paste, Paste from
|
||||
//! Previous, Sync, Auto Sync) re-express a correction in the target's frame through the stored
|
||||
//! image, which is how Lightroom keeps it, and a settings patch that turns a photo turns the
|
||||
//! correction it keeps (issue #205).
|
||||
|
||||
use lightcraft_catalog::Photo;
|
||||
use lightcraft_develop::DevelopSettings;
|
||||
use lightcraft_geom::{Homography, Orientation};
|
||||
use serde_json::Value;
|
||||
|
||||
use crate::sidecar::StoredFrame;
|
||||
|
||||
/// A photo's frame for its Upright correction: its file's stored image, then its own rotation.
|
||||
#[derive(Clone, Copy, Debug, PartialEq)]
|
||||
pub struct PhotoFrame {
|
||||
pub stored: StoredFrame,
|
||||
pub user: Orientation,
|
||||
}
|
||||
|
||||
/// The frame of photo `p` with settings `s`. A photo imported before the file's orientation was
|
||||
/// recorded is taken as stored the way it is shown.
|
||||
pub fn frame_of(p: &Photo, s: &DevelopSettings) -> Option<PhotoFrame> {
|
||||
let stored = crate::sidecar::recorded_frame(p).or_else(|| StoredFrame::of_shown(Orientation::Normal, f64::from(p.width), f64::from(p.height)))?;
|
||||
Some(PhotoFrame { stored, user: s.orientation })
|
||||
}
|
||||
|
||||
/// Correction `h`, made in frame `from`, as the same correction of the stored image in frame `to`.
|
||||
pub fn reexpress(h: &Homography, from: &PhotoFrame, to: &PhotoFrame) -> Option<Homography> {
|
||||
let on_stored = crate::crs::upright_on_stored(h, from.stored.orientation.then(from.user), from.stored.width, from.stored.height)?;
|
||||
let saved = crate::crs::SavedUpright { mode: lightcraft_develop::Upright::Auto, matrix: on_stored };
|
||||
crate::crs::upright_in_frame(&saved, to.stored.orientation.then(to.user), to.stored.width, to.stored.height)
|
||||
}
|
||||
|
||||
/// Settings `after` of photo `p` (which had `before`) once `patch` is applied: a correction the
|
||||
/// patch brought from photo frame `source` is put into `p`'s frame; a correction `p` keeps is
|
||||
/// turned when the patch turned the photo. A patch from nowhere in particular (a preset) leaves a
|
||||
/// correction it brings as it is.
|
||||
pub fn settle(p: &Photo, before: &DevelopSettings, after: &mut DevelopSettings, patch: &Value, source: Option<&PhotoFrame>) {
|
||||
let Some(target) = frame_of(p, after) else { return };
|
||||
let (from, h) = if patch.pointer("/geometry/upright_transform").is_some() {
|
||||
(source.copied(), after.geometry.upright_transform)
|
||||
} else if after.orientation != before.orientation {
|
||||
(frame_of(p, before), after.geometry.upright_transform)
|
||||
} else {
|
||||
return;
|
||||
};
|
||||
if let (Some(from), Some(h)) = (from, h) {
|
||||
// a frame it can't be put in (no size) leaves it to be solved again
|
||||
after.geometry.upright_transform = reexpress(&h, &from, &target);
|
||||
}
|
||||
}
|
||||
|
||||
/// `patch` applied to settings `before` of photo `p` for a transfer from photo frame `source`,
|
||||
/// with the Upright correction in `p`'s frame ([`settle`]).
|
||||
pub fn transferred(p: &Photo, before: &DevelopSettings, patch: &Value, source: Option<&PhotoFrame>) -> DevelopSettings {
|
||||
let mut after = lightcraft_develop::apply_partial(before, patch, 1.0);
|
||||
settle(p, before, &mut after, patch, source);
|
||||
after
|
||||
}
|
||||
@@ -0,0 +1,89 @@
|
||||
//! An Upright correction (`Geometry::upright_transform`) is one photo's own analysis result: the
|
||||
//! correction found for that image (by LightCraft, or Lightroom's saved one for it), stored so
|
||||
//! previews and exports agree, in the frame the photo is shown in. When settings move or change
|
||||
//! (issue #205):
|
||||
//!
|
||||
//! - between entries of the same file (a photo and its virtual copies) the correction moves too,
|
||||
//! turned by the difference in their Rotate Left / Right;
|
||||
//! - between different files only the Upright mode and the Transform sliders move: the target
|
||||
//! keeps its own correction while its mode stays the same, else solves its own, as Lightroom
|
||||
//! does when it syncs the Upright mode (a correction found for one scene is wrong for another);
|
||||
//! - presets never carry a correction (they are for other photos);
|
||||
//! - a change that turns a photo turns the correction it keeps, and a change of mode drops it to
|
||||
//! be solved again.
|
||||
|
||||
use lightcraft_catalog::{Photo, PhotoId, Source};
|
||||
use lightcraft_develop::DevelopSettings;
|
||||
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)]
|
||||
pub struct UprightSource {
|
||||
source: Source,
|
||||
master: PhotoId,
|
||||
/// Its Rotate Left / Right when the settings were taken.
|
||||
user: Orientation,
|
||||
}
|
||||
|
||||
/// 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 }
|
||||
}
|
||||
|
||||
/// Whether photo `p` shows the same file as `from`: the photo itself, its original or another of
|
||||
/// its virtual copies.
|
||||
fn same_file(from: &UprightSource, p: &Photo) -> bool {
|
||||
from.master == p.copy_of.unwrap_or(p.id) || from.source == p.source
|
||||
}
|
||||
|
||||
/// The turn that takes a photo shown with `from` to `to` (both after the file's own orientation).
|
||||
fn turn(from: Orientation, to: Orientation) -> Orientation {
|
||||
from.inverse().then(to)
|
||||
}
|
||||
|
||||
/// 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).
|
||||
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);
|
||||
}
|
||||
let g = &mut after.geometry;
|
||||
g.upright_transform = match source.filter(|from| same_file(from, p)) {
|
||||
// the same file's correction, in this entry's rotation
|
||||
Some(from) => g.upright_transform.map(|h| lightcraft_pipeline::upright::turned(&h, turn(from.user, after.orientation))),
|
||||
// another file's (or a preset's): not this photo's; it keeps its own while the mode stays
|
||||
None if g.upright == before.geometry.upright => {
|
||||
before.geometry.upright_transform.map(|h| lightcraft_pipeline::upright::turned(&h, turn(before.orientation, after.orientation)))
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
}
|
||||
|
||||
/// A change of a photo's own settings (`before` to `after`) that doesn't state a correction: one
|
||||
/// that turns the photo turns the correction it keeps; one that changes the Upright mode drops it
|
||||
/// to be solved again.
|
||||
pub fn settle_edit(before: &DevelopSettings, after: &mut DevelopSettings) {
|
||||
let g = &mut after.geometry;
|
||||
if g.upright != before.geometry.upright {
|
||||
g.upright_transform = None;
|
||||
} else if after.orientation != before.orientation {
|
||||
g.upright_transform = g.upright_transform.map(|h| lightcraft_pipeline::upright::turned(&h, turn(before.orientation, after.orientation)));
|
||||
}
|
||||
}
|
||||
|
||||
/// `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);
|
||||
settle(p, before, &mut after, patch, source);
|
||||
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) {
|
||||
g.remove("upright_transform");
|
||||
}
|
||||
settings
|
||||
}
|
||||
@@ -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, and Paste / Sync / Auto Sync put it into each photo's frame |
|
||||
| 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-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 |
|
||||
@@ -787,7 +787,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-12 | minor | LR-META-XMP: Lightroom's saved Upright correction for the selected mode is kept from XMP instead of being solved again (issue #205; catalogs fall back and report it); `PerspectiveUpright` 2, 3, 4 read as Full, Level, Vertical; sidecar crops follow the file's orientation (Synchronize Folder too); LR-EDIT-GEOM-UPRIGHT: Rotate turns a stored correction, Paste / Sync carry it into each photo's frame; statuses unchanged |
|
||||
| 2026-10-12 | minor | LR-META-XMP: Lightroom's saved Upright correction for the selected mode is kept from XMP instead of being solved again (issue #205; catalogs fall back and report it); `PerspectiveUpright` 2, 3, 4 read as Full, Level, Vertical; sidecar crops follow the file's orientation (Synchronize Folder too); LR-EDIT-GEOM-UPRIGHT: Rotate turns a stored correction; Paste / Sync / presets carry the Upright mode to other files and the correction only to virtual copies; statuses unchanged |
|
||||
| 2026-10-11 | minor | KEY-SLIDER-TYPE: honor immediate Return/Escape and keep typed keys in the value editor; add regression references |
|
||||
| 2026-10-11 | minor | LR-PROF-CAMERACOLOR: raws with a black-and-white camera JPEG take its tone curve (issue #577) |
|
||||
| 2026-10-11 | minor | LR-VIEW-ZOOM and LR-VIEW-COMPARE: Compare and Reference are sharp at 1:1 (zoom windows per pane, two originals kept decoded while they are open); statuses unchanged |
|
||||
|
||||
+7
-2
@@ -12,8 +12,13 @@
|
||||
and with Synchronize Folder. Lightroom Classic catalogs still have it worked out again for now, and the import
|
||||
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. Paste, Paste from Previous, Sync and Auto Sync now put a copied Upright correction
|
||||
the right way round for each photo, for example on a turned virtual copy or a photo shot in portrait.
|
||||
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.
|
||||
- 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.
|
||||
- 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.
|
||||
|
||||
+32
-10
@@ -141,7 +141,7 @@ application's renderer and is ignored: the photo keeps LightCraft's own process,
|
||||
| `ShadowTint`, `RedHue/Saturation`, `GreenHue/Saturation`, `BlueHue/Saturation` | `calibration.shadows_tint`, `calibration.red_hue/red_sat`, … | Calibration panel, 1:1 |
|
||||
| `PerspectiveVertical/Horizontal/Rotate/Scale/Aspect/X/Y` | `geometry.vertical/horizontal/rotate/scale/aspect/offset_x/offset_y` | |
|
||||
| `PerspectiveUpright` | `geometry.upright`, `geometry.upright_transform` | 0 off, 1 auto, 2 full, 3 level, 4 vertical, 5 guided; Lightroom's saved correction for the selected mode is kept as it is when it can be (see *Saved Upright geometry*), otherwise LightCraft works the mode out itself (Guided needs guides drawn in LightCraft: Lightroom's aren't read) and photo import says so |
|
||||
| `HasCrop`, `CropLeft/Top/Right/Bottom`, `CropAngle` | `crop.geometry` | Lightroom's edges are on the stored image (before its orientation) and its straighten angle turns the other way: both go into the frame the photo is shown in (through the file's orientation and the photo's Rotate Left / Right), as the Lightroom catalog import does; the file's orientation is recorded on import (photos imported before that read it from the file); when it can't be known (a raw shown from its preview) the edges are taken as written on import, and an existing photo keeps its crop and the report says so; `HasCrop="False"` → no crop |
|
||||
| `HasCrop`, `CropLeft/Top/Right/Bottom`, `CropAngle` | `crop.geometry` | Lightroom's edges are on the stored image (before its orientation) and its straighten angle turns the other way: both go into the frame the photo is shown in (through the file's orientation and the photo's Rotate Left / Right), as the Lightroom catalog import does; the file's orientation is read from the file each time (on import, Read Metadata from File and Synchronize Folder), so a file that changed is read as it is now; when it can't be known (a raw shown from its preview, a file that can't be read) the edges are taken as written on import, and an existing photo keeps its crop and the report says so; `HasCrop="False"` → no crop |
|
||||
|
||||
Values pass through our control specs, so anything outside our slider ranges gets clamped.
|
||||
|
||||
@@ -158,8 +158,9 @@ photo is framed as in Lightroom instead of being analysed again, when
|
||||
- `UprightVersion` is `151388160` (the only version seen so far; any other is solved again);
|
||||
- `UprightTransformCount` includes the mode's matrix, and the matrix is nine finite numbers that make a sensible
|
||||
correction (no part of the photo at or beyond the horizon, no mirror, no collapse);
|
||||
- the file's orientation is known: on import, Read Metadata from File and Synchronize Folder (Lightroom catalog
|
||||
import doesn't read saved Upright yet: see [lightroom-catalog-import.md](lightroom-catalog-import.md));
|
||||
- the file's orientation can be read from the file, as it is now: on import, Read Metadata from File and
|
||||
Synchronize Folder (Lightroom catalog import doesn't read saved Upright yet: see
|
||||
[lightroom-catalog-import.md](lightroom-catalog-import.md));
|
||||
- the crop has no straighten angle (a straightened crop on top of a saved Upright isn't measured yet).
|
||||
|
||||
The matrix's convention was measured on Lightroom's exports, not assumed (issue #205): a forward homography (photo
|
||||
@@ -170,18 +171,39 @@ to fill the frame already in it. LightCraft turns it with the file's orientation
|
||||
included) and applies the crop on top, as Lightroom does. Rotate Left / Right later turns the kept correction with
|
||||
the photo, as it does LightCraft's own Upright.
|
||||
|
||||
An Upright correction belongs to its photo's frame. Paste, Paste from Previous, Sync and Auto Sync put a copied
|
||||
correction into each target's frame through the stored image (as Lightroom keeps it), so a turned virtual copy or a
|
||||
photo shot in portrait gets it the right way round, and a pasted crop that turns a photo turns the correction it
|
||||
keeps. A photo imported before the file's orientation was recorded is taken as stored the way it is shown. On three public NEF raws exported by Lightroom Classic 12 with Upright Auto (one shown in portrait,
|
||||
one cropped with Constrain Crop on), LightCraft's geometry with the kept matrix matches the exports to about half a
|
||||
pixel on average and under a pixel at worst, at full size; without it, the frames differ by 22 to 36 px on average.
|
||||
On three public NEF raws exported by Lightroom Classic 12 with Upright Auto (one shown in portrait, one cropped
|
||||
with Constrain Crop on), LightCraft's geometry with the kept matrix matches the exports to about half a pixel on
|
||||
average and under a pixel at worst, at full size; without it, the frames differ by 22 to 36 px on average.
|
||||
|
||||
### An Upright correction is the photo's own
|
||||
|
||||
A saved Upright matrix, Lightroom's or the one LightCraft's own Upright found, is the analysis of one image: the
|
||||
correction for that scene, in the frame that photo is shown in. So when settings move or change:
|
||||
|
||||
- **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.
|
||||
- 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.
|
||||
- **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 Upright mode drops the correction, to be solved again.
|
||||
- A 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
|
||||
exports (their matrices have those modes' shapes, but no export of them with its raw was available), photos with
|
||||
Lightroom lens profile corrections, and a straightened crop on top of Upright (solved again, with the warning). Guided
|
||||
Upright's guides (`UprightFourSegments_*`) aren't read: import says so whenever a packet has them.
|
||||
Upright's guides (`UprightFourSegments_*`) aren't read: import says so when the packet's mode is Guided (in other
|
||||
modes Lightroom doesn't use them).
|
||||
|
||||
**Not mapped:** camera profiles and looks (`CameraProfile`, `Look`; we have our own profile set), red eye, lens blur,
|
||||
process-version 2010 field names, and AI features. Local adjustments and spot removal are read as described below.
|
||||
|
||||
Reference in new issue
Block a user