Bring a Lightroom library across, and start with something in the list
Two halves of the same complaint: a preset sheet that opens on "No presets yet" is homework, and a photographer with ten years of presets in Lightroom has no way to bring them. `dr-preset-xmp` reads Camera Raw `.xmp`. The mapping turned out to be mostly a rename rather than a conversion, because Adobe and this pipeline already agree: exposure is in stops in both, and contrast, the four recovery controls, clarity, texture, vibrance and saturation are all ±100 in both. That is not imitation, it is the convention raw developers converged on — `highlights_shadows.yaml` cites it in as many words. Only sharpening needed arithmetic, Adobe's 0…150 against our 0…100. The white balance does not come across, and says so rather than guessing. Adobe writes absolute Kelvin for a raw file where ours is a relative nudge from what the camera recorded, so converting needs the *target image's* as-shot white balance — exactly what a preset cannot carry, since the same preset lands on a frame shot at 3200K and one shot at 7000K. A guess would be wrong on most images and invisibly so. A folder is read as readily as a file, nested, because that is the shape an exported preset folder is in and importing ninety files one at a time is asking someone not to bother. `dr_pipeline::starter` is six presets a first run begins with, written against this pipeline in its units and deliberately mild — a starting point, not a caricature. They are seeded when the library *file* does not exist rather than when the library is empty, so deleting all six does not hand them back on the next launch. Both of these name operations, and `ui_names_no_operation` was right to stop them living in `ui/`. That test exists because the failure is silent and cumulative, and it caught exactly what it was written for: a preset called "Punch" is a statement about contrast, clarity and vibrance, and a table mapping Adobe's vocabulary to ours is a statement about the pipeline. Neither is a fact about an interface. So the starter set went into `dr-pipeline`, and the importer into its own crate — between two walls, since `dr-pipeline` depends on nothing on purpose and XMP is real XML not worth hand-rolling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -7,6 +7,10 @@ license.workspace = true
|
||||
|
||||
[dependencies]
|
||||
dr-types.workspace = true
|
||||
# TRACES: FR-DEV-6
|
||||
# Reading Lightroom `.xmp` presets. Its own crate because the translation names
|
||||
# operations, which the interface may not — see `ui_names_no_operation.rs`.
|
||||
dr-preset-xmp.workspace = true
|
||||
# No `readback`. S1 wired Slint's texture import, so the develop view hands
|
||||
# the compositor the texture itself and there is no display round-trip left to
|
||||
# gate (ARCH §6.1, AC-8). The export path reads pixels back through
|
||||
|
||||
+213
-5
@@ -688,8 +688,74 @@ impl NamedPresets {
|
||||
}
|
||||
|
||||
fn at(store: PresetStore) -> Rc<Self> {
|
||||
let library = RefCell::new(store.load());
|
||||
Rc::new(Self { store, library })
|
||||
// TRACES: FR-DEV-6
|
||||
// Seed the starter set on a device that has never had a library, and
|
||||
// only then. See `preset_starter` for why this is not a merge:
|
||||
// re-adding on every start would resurrect one deleted on purpose.
|
||||
//
|
||||
// "Never had one" is the *file* not existing rather than the library
|
||||
// being empty — a photographer who deleted all six must not be handed
|
||||
// them again on the next launch.
|
||||
let seeding = !store.path().exists();
|
||||
let library = if seeding {
|
||||
let starters = dr_pipeline::starter::library();
|
||||
if let Err(e) = store.save(&starters) {
|
||||
// Not fatal. The presets are in memory and usable this
|
||||
// session; what is lost is their persistence, and refusing to
|
||||
// start over it would be absurd.
|
||||
log::warn!("could not write the starter presets: {e}");
|
||||
}
|
||||
starters
|
||||
} else {
|
||||
store.load()
|
||||
};
|
||||
Rc::new(Self {
|
||||
store,
|
||||
library: RefCell::new(library),
|
||||
})
|
||||
}
|
||||
|
||||
/// Read presets from a file or a folder and store them.
|
||||
///
|
||||
/// Returns what to tell the user. Names already in the library are
|
||||
/// replaced, the same rule a save follows — importing the same folder
|
||||
/// twice leaves one copy of each rather than `Warm Portrait 2`.
|
||||
fn import(&self, path: &Path) -> String {
|
||||
let report = dr_preset_xmp::read_path(path);
|
||||
if report.presets.is_empty() {
|
||||
return if report.failed > 0 {
|
||||
format!("Found {} file(s), but none could be read.", report.failed)
|
||||
} else {
|
||||
"No .xmp presets there.".to_string()
|
||||
};
|
||||
}
|
||||
|
||||
let added = report.presets.len();
|
||||
{
|
||||
let mut library = self.library.borrow_mut();
|
||||
for (name, preset) in report.presets {
|
||||
if let Err(e) = library.insert(&name, preset) {
|
||||
log::warn!("imported preset {name:?} is unusable ({e:?}); skipping");
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Err(e) = self.persist(|_| {}) {
|
||||
return e.message();
|
||||
}
|
||||
|
||||
let mut message = format!(
|
||||
"Imported {added} preset{}.",
|
||||
if added == 1 { "" } else { "s" }
|
||||
);
|
||||
if report.failed > 0 {
|
||||
message.push_str(&format!(" {} could not be read.", report.failed));
|
||||
}
|
||||
// Said once, however many files carried it: after forty presets the
|
||||
// useful sentence is that the white balance did not come across.
|
||||
if !report.unsupported.is_empty() {
|
||||
message.push_str(" White balance and tone curves do not carry across.");
|
||||
}
|
||||
message
|
||||
}
|
||||
|
||||
/// The stored names, in the order they are written.
|
||||
@@ -702,6 +768,12 @@ impl NamedPresets {
|
||||
self.library.borrow().get(name).cloned()
|
||||
}
|
||||
|
||||
/// Whether a preset is stored under `name`.
|
||||
#[cfg(test)]
|
||||
pub fn contains(&self, name: &str) -> bool {
|
||||
self.library.borrow().contains(name)
|
||||
}
|
||||
|
||||
/// Store `preset` under `name` and persist.
|
||||
///
|
||||
/// The in-memory library is updated first and rolled back if the write
|
||||
@@ -726,6 +798,13 @@ impl NamedPresets {
|
||||
}
|
||||
|
||||
/// Save the library, undoing the in-memory change if the write fails.
|
||||
///
|
||||
/// The rollback is the caller's because only the caller knows what the
|
||||
/// previous state was. An import passes an empty one deliberately: it may
|
||||
/// have replaced any number of names, restoring them all would mean
|
||||
/// snapshotting the whole library, and the honest failure there is to say
|
||||
/// the write failed and leave what is on screen matching what is in
|
||||
/// memory until the next successful save.
|
||||
fn persist(&self, rollback: impl FnOnce(&mut PresetLibrary)) -> Result<(), SaveError> {
|
||||
let result = self.store.save(&self.library.borrow());
|
||||
match result {
|
||||
@@ -745,6 +824,7 @@ impl NamedPresets {
|
||||
}
|
||||
|
||||
/// Why a preset could not be saved.
|
||||
#[derive(Debug)]
|
||||
enum SaveError {
|
||||
/// The name itself was refused.
|
||||
Name(NameError),
|
||||
@@ -899,6 +979,25 @@ pub fn wire_named(
|
||||
});
|
||||
}
|
||||
|
||||
// --- import from Lightroom --------------------------------------------
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let named = named.clone();
|
||||
window.on_import_presets(move |path| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let path = path.trim();
|
||||
if path.is_empty() {
|
||||
return;
|
||||
}
|
||||
// Synchronous, deliberately. A preset folder is a few hundred
|
||||
// small text files and the read is milliseconds; a worker and a
|
||||
// progress bar would be machinery around a wait nobody sees.
|
||||
let report = named.import(Path::new(path));
|
||||
w.set_preset_import_report(report.into());
|
||||
render_named(&w, &named);
|
||||
});
|
||||
}
|
||||
|
||||
// --- rename -----------------------------------------------------------
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
@@ -1275,11 +1374,115 @@ mod tests {
|
||||
// Named presets
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
/// A library that has *already been started*, so the starter presets are
|
||||
/// not in the way.
|
||||
///
|
||||
/// Writing an empty library first is what makes the file exist, which is
|
||||
/// the condition seeding tests — see `NamedPresets::at`. The seeding
|
||||
/// itself has its own tests below rather than being switched off with a
|
||||
/// flag that only tests would set.
|
||||
fn named(name: &str) -> (Rc<NamedPresets>, PathBuf) {
|
||||
let dir = tempdir(name);
|
||||
let path = dir.join("presets.drpl");
|
||||
PresetStore::open_at(path.clone())
|
||||
.save(&PresetLibrary::default())
|
||||
.unwrap();
|
||||
(NamedPresets::open_at(path), dir)
|
||||
}
|
||||
|
||||
/// A device that has never had a preset library.
|
||||
fn fresh(name: &str) -> (Rc<NamedPresets>, PathBuf) {
|
||||
let dir = tempdir(name);
|
||||
(NamedPresets::open_at(dir.join("presets.drpl")), dir)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_first_run_is_handed_the_starter_presets() {
|
||||
// A sheet that opens on "No presets yet" teaches the photographer that
|
||||
// the feature is homework.
|
||||
let (presets, _dir) = fresh("first-run");
|
||||
assert_eq!(presets.names().len(), dr_pipeline::starter::library().len());
|
||||
assert!(!presets.names().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_starters_are_written_out_so_they_survive_a_restart() {
|
||||
let (_presets, dir) = fresh("starters-persist");
|
||||
let path = dir.join("presets.drpl");
|
||||
assert!(path.exists(), "the starter library was never written");
|
||||
assert!(!PresetStore::open_at(path).load().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_starter_the_photographer_deleted_stays_deleted() {
|
||||
// The reason seeding keys on the file existing rather than on the
|
||||
// library being empty: deleting all six must not hand them back on the
|
||||
// next launch.
|
||||
let dir = tempdir("deleted-starters");
|
||||
let path = dir.join("presets.drpl");
|
||||
{
|
||||
let presets = NamedPresets::open_at(path.clone());
|
||||
for name in presets.names() {
|
||||
let mut library = presets.library.borrow_mut();
|
||||
library.remove(&name);
|
||||
}
|
||||
presets.persist(|_| {}).unwrap();
|
||||
}
|
||||
|
||||
let again = NamedPresets::open_at(path);
|
||||
assert!(
|
||||
again.names().is_empty(),
|
||||
"the starters came back: {:?}",
|
||||
again.names()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn importing_a_folder_stores_what_it_finds() {
|
||||
let (presets, dir) = named("import-folder");
|
||||
let from = dir.join("from-lightroom");
|
||||
std::fs::create_dir_all(&from).unwrap();
|
||||
std::fs::write(
|
||||
from.join("warm.xmp"),
|
||||
r#"<x:xmpmeta xmlns:x="adobe:ns:meta/">
|
||||
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
|
||||
<rdf:Description rdf:about="" xmlns:crs="http://ns.adobe.com/camera-raw-settings/1.0/">
|
||||
<crs:Exposure2012>+0.5</crs:Exposure2012>
|
||||
</rdf:Description>
|
||||
</rdf:RDF>
|
||||
</x:xmpmeta>"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let report = presets.import(&from);
|
||||
assert!(report.starts_with("Imported 1 preset."), "{report}");
|
||||
assert_eq!(presets.names(), vec!["warm".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn importing_the_same_folder_twice_leaves_one_copy() {
|
||||
let (presets, dir) = named("import-twice");
|
||||
let from = dir.join("from-lightroom");
|
||||
std::fs::create_dir_all(&from).unwrap();
|
||||
std::fs::write(
|
||||
from.join("warm.xmp"),
|
||||
r#"<rdf:Description xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
|
||||
xmlns:crs="http://ns.adobe.com/camera-raw-settings/1.0/" crs:Exposure2012="+0.5"/>"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
presets.import(&from);
|
||||
presets.import(&from);
|
||||
assert_eq!(presets.names(), vec!["warm".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn importing_from_nowhere_says_so_rather_than_claiming_success() {
|
||||
let (presets, _dir) = named("import-nothing");
|
||||
let report = presets.import(Path::new("/definitely/not/here"));
|
||||
assert!(report.contains("No .xmp presets"), "{report}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_saved_preset_is_on_disk_before_the_call_returns() {
|
||||
// Not on the way out, and not on a timer: a preset the user named and
|
||||
@@ -1287,7 +1490,6 @@ mod tests {
|
||||
let (presets, dir) = named("saved-immediately");
|
||||
presets
|
||||
.insert("Warm", Preset::capture(&edited()))
|
||||
.ok()
|
||||
.expect("saved");
|
||||
|
||||
let reloaded = NamedPresets::open_at(dir.join("presets.drpl"));
|
||||
@@ -1331,9 +1533,15 @@ mod tests {
|
||||
|
||||
let presets = NamedPresets::open_at(blocked.join("presets.drpl"));
|
||||
assert!(presets.insert("Warm", Preset::default()).is_err());
|
||||
// Not "the library is empty": the starter set is held in memory even
|
||||
// on a device whose config directory cannot be written, which is the
|
||||
// right behaviour — the presets are usable this session and only their
|
||||
// persistence is lost. What must not survive is the preset whose write
|
||||
// failed.
|
||||
assert!(
|
||||
presets.names().is_empty(),
|
||||
"the failed save left a preset behind"
|
||||
!presets.contains("Warm"),
|
||||
"the failed save left a preset behind: {:?}",
|
||||
presets.names()
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -680,6 +680,11 @@ export component AppWindow inherits Window {
|
||||
in property <bool> copy-scope-empty: false;
|
||||
callback copy-scope-toggled(string);
|
||||
|
||||
/// TRACES: FR-DEV-6
|
||||
/// Reading Lightroom presets in, and what came of the last attempt.
|
||||
in property <string> preset-import-report;
|
||||
callback import-presets(string);
|
||||
|
||||
// --- settings (FR-EXP-1, FR-EXP-3, FR-NC-6a) ---
|
||||
//
|
||||
// A page rather than an overlay, and the outermost of the view conditions
|
||||
@@ -2473,6 +2478,8 @@ in property <bool> panel-visible: true;
|
||||
scope-kinds: root.copy-scope-kinds;
|
||||
scope-empty: root.copy-scope-empty;
|
||||
scope-toggled(name) => { root.copy-scope-toggled(name); }
|
||||
import-report: root.preset-import-report;
|
||||
import-presets(path) => { root.import-presets(path); }
|
||||
save(name) => { root.save-preset(name); }
|
||||
apply(name) => { root.apply-preset(name); }
|
||||
rename(from, to) => { root.rename-preset(from, to); }
|
||||
|
||||
@@ -110,6 +110,14 @@ export component PresetSheet inherits Rectangle {
|
||||
in property <bool> scope-empty: false;
|
||||
callback scope-toggled(string);
|
||||
|
||||
/// TRACES: FR-DEV-6
|
||||
/// Read Lightroom presets from a typed path, and what came of the last
|
||||
/// attempt. A typed path rather than a file dialogue because that is the
|
||||
/// idiom this application already has — the launch screen takes a library
|
||||
/// the same way, and there is no native picker anywhere in it.
|
||||
in property <string> import-report;
|
||||
callback import-presets(string);
|
||||
|
||||
background: #000000CC;
|
||||
|
||||
// Swallows the taps that miss the card, and closes. First, so the card's
|
||||
@@ -265,6 +273,27 @@ export component PresetSheet inherits Rectangle {
|
||||
|
||||
Rectangle { height: 1px; background: Theme.rule; }
|
||||
|
||||
// TRACES: FR-DEV-6
|
||||
// Bringing a Lightroom library across. A folder rather than a
|
||||
// file is the expected case — an exported preset folder, nested
|
||||
// one level per group — so the field says so.
|
||||
VerticalLayout {
|
||||
spacing: Theme.gap-sm;
|
||||
|
||||
PanelHeading { text: "IMPORT"; }
|
||||
|
||||
Field {
|
||||
placeholder: "Path to a .xmp file or folder";
|
||||
accepted(path) => { root.import-presets(path); }
|
||||
}
|
||||
|
||||
if root.import-report != "": Caption {
|
||||
text: root.import-report;
|
||||
}
|
||||
}
|
||||
|
||||
Rectangle { height: 1px; background: Theme.rule; }
|
||||
|
||||
// TRACES: FR-DEV-6
|
||||
// What an apply carries. At the bottom, because it qualifies every
|
||||
// action above it rather than being one of them — and because the
|
||||
|
||||
Reference in New Issue
Block a user