Open a photograph with the edit it already carries, when DarkRoom has none
The library's DNGs carry the photographer's earlier develop settings in their embedded XMP — the house style their photographs were made with. A photograph opened with no edit of DarkRoom's now starts from that earlier edit, translated (HSL bands, highlights, blacks and the rest), as one undoable step named "Earlier Edit"; from there it is an ordinary edit, saved with the photograph. Export does the same, so a photograph never opened exports as opening it would show. Only on positive evidence that there is no DarkRoom edit: a local file with no sidecar beside it, or a server that answered "no such file" with nothing in the cache. The stored-edit fetch now says which (FetchedSidecar::absent). Offline, unreachable or unreadable never counts — the earlier edit would otherwise be saved over a real edit that merely failed to arrive.
This commit is contained in:
+55
-55
File diff suppressed because one or more lines are too long
@@ -76,6 +76,11 @@ pub struct DevelopSession {
|
|||||||
/// the info panel offers to save for every photograph from that camera
|
/// the info panel offers to save for every photograph from that camera
|
||||||
/// (D20). `None` once taken up.
|
/// (D20). `None` once taken up.
|
||||||
pub(super) profile_offer: Option<Arc<dr_decode::dcp::Dcp>>,
|
pub(super) profile_offer: Option<Arc<dr_decode::dcp::Dcp>>,
|
||||||
|
/// TRACES: FR-DEV-6
|
||||||
|
/// The edit the file already carries from the photographer's earlier work, translated, waiting for the
|
||||||
|
/// word that this photograph has no edit of DarkRoom's own
|
||||||
|
/// ([`Self::adopt_earlier_edit`]).
|
||||||
|
pub(super) earlier_edit: Option<Preset>,
|
||||||
/// Kept so the session can build GPU resources after construction.
|
/// Kept so the session can build GPU resources after construction.
|
||||||
///
|
///
|
||||||
/// The distance fields behind a subject mask are made when a layer is
|
/// The distance fields behind a subject mask are made when a layer is
|
||||||
@@ -439,6 +444,7 @@ impl DevelopSession {
|
|||||||
// and not found" — `lens_summary` distinguishes them.
|
// and not found" — `lens_summary` distinguishes them.
|
||||||
lens_profile_found: false,
|
lens_profile_found: false,
|
||||||
profile_offer: None,
|
profile_offer: None,
|
||||||
|
earlier_edit: None,
|
||||||
ctx: ctx.clone(),
|
ctx: ctx.clone(),
|
||||||
graph,
|
graph,
|
||||||
history,
|
history,
|
||||||
@@ -610,6 +616,33 @@ impl DevelopSession {
|
|||||||
.map(Arc::new);
|
.map(Arc::new);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// TRACES: FR-DEV-6
|
||||||
|
/// Remember the earlier edit stored in the file this session opened.
|
||||||
|
pub fn set_earlier_edit(&mut self, edit: Option<Preset>) {
|
||||||
|
self.earlier_edit = edit;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// TRACES: FR-DEV-6
|
||||||
|
/// Apply the file's earlier edit, as the photograph's starting point, so it
|
||||||
|
/// keeps the style it was given before it came to DarkRoom.
|
||||||
|
///
|
||||||
|
/// Only for a photograph DarkRoom has no edit of: the caller says so, and
|
||||||
|
/// must know it rather than guess — a stored edit that could not be
|
||||||
|
/// fetched is not an absent one, and the earlier edit would then be
|
||||||
|
/// saved over it on the way out. One undoable step, so undo shows the
|
||||||
|
/// photograph without it; from there it is an ordinary edit, saved with
|
||||||
|
/// the photograph. Taken, so a second call does nothing.
|
||||||
|
pub fn adopt_earlier_edit(&mut self) -> bool {
|
||||||
|
let Some(edit) = self.earlier_edit.take() else {
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
let rebake = edit.apply(&mut self.graph, Scope::adjustments());
|
||||||
|
self.pay_film_debt(&rebake);
|
||||||
|
self.history
|
||||||
|
.record(&self.graph, Edit::Action(labels::step::EARLIER_EDIT));
|
||||||
|
true
|
||||||
|
}
|
||||||
|
|
||||||
/// TRACES: FR-DEV-3e
|
/// TRACES: FR-DEV-3e
|
||||||
/// What to tell the photographer about the camera profile (D20).
|
/// What to tell the photographer about the camera profile (D20).
|
||||||
///
|
///
|
||||||
@@ -888,6 +921,34 @@ mod tests {
|
|||||||
assert_eq!(jpeg.profile_summary(), "");
|
assert_eq!(jpeg.profile_summary(), "");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_earlier_edit_is_adopted_once_and_can_be_undone() {
|
||||||
|
// TRACES: FR-DEV-6
|
||||||
|
let Some(ctx) = headless() else { return };
|
||||||
|
let mut s = DevelopSession::open(&ctx, &raw_with(None), dr_types::Orientation::NORMAL)
|
||||||
|
.expect("session");
|
||||||
|
assert!(!s.adopt_earlier_edit(), "nothing to adopt yet");
|
||||||
|
|
||||||
|
let earlier = dr_preset_xmp::read_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:SaturationAdjustmentBlue="+58" crs:Highlights2012="-40"/></rdf:RDF></x:xmpmeta>"#,
|
||||||
|
)
|
||||||
|
.expect("an edit")
|
||||||
|
.preset;
|
||||||
|
s.set_earlier_edit(Some(earlier));
|
||||||
|
assert!(s.adopt_earlier_edit());
|
||||||
|
let blue = || {
|
||||||
|
s.graph.param(
|
||||||
|
dr_pipeline::ops::colour_mixer::ID,
|
||||||
|
dr_pipeline::ParamId("blue_sat"),
|
||||||
|
)
|
||||||
|
};
|
||||||
|
assert_eq!(blue(), Some(58.0));
|
||||||
|
assert!(!s.adopt_earlier_edit(), "taken: a second call does nothing");
|
||||||
|
assert!(!s.graph.is_neutral());
|
||||||
|
s.undo();
|
||||||
|
assert!(s.graph.is_neutral(), "undo shows the photograph without it");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn only_a_copyable_profile_is_offered() {
|
fn only_a_copyable_profile_is_offered() {
|
||||||
// TRACES: FR-DEV-3e
|
// TRACES: FR-DEV-3e
|
||||||
|
|||||||
+10
-5
@@ -1293,15 +1293,15 @@ fn render_from_library(
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let sidecar = match wait_for(&sidecar_rx, cancel) {
|
let fetched = match wait_for(&sidecar_rx, cancel) {
|
||||||
Waited::Got(sidecar) => sidecar,
|
Waited::Got(fetched) => fetched,
|
||||||
Waited::Cancelled => return None,
|
Waited::Cancelled => return None,
|
||||||
// An unedited photograph has no sidecar and a fetch that died looks
|
// A fetch that died. Exporting at defaults is what opening it would
|
||||||
// identical from here. Exporting at defaults is what opening it would
|
|
||||||
// do, and refusing the image over a missing edit it may never have had
|
// do, and refusing the image over a missing edit it may never have had
|
||||||
// would fail the common case.
|
// would fail the common case.
|
||||||
Waited::Silent => None,
|
Waited::Silent => crate::library::FetchedSidecar::default(),
|
||||||
};
|
};
|
||||||
|
let sidecar = fetched.sidecar;
|
||||||
|
|
||||||
let (meta, mut session) = match open_for_export(gpu, request.decoder, &bytes) {
|
let (meta, mut session) = match open_for_export(gpu, request.decoder, &bytes) {
|
||||||
Ok(opened) => opened,
|
Ok(opened) => opened,
|
||||||
@@ -1318,6 +1318,11 @@ fn render_from_library(
|
|||||||
.and_then(dr_pipeline::Sidecar::default_version)
|
.and_then(dr_pipeline::Sidecar::default_version)
|
||||||
{
|
{
|
||||||
session.apply_version(version);
|
session.apply_version(version);
|
||||||
|
} else if fetched.absent {
|
||||||
|
// TRACES: FR-DEV-6
|
||||||
|
// No edit of DarkRoom's, by the server's word: export what opening it
|
||||||
|
// would show — the photograph's earlier edit, where it carries one.
|
||||||
|
session.adopt_earlier_edit();
|
||||||
}
|
}
|
||||||
|
|
||||||
// The last cheap place to stop. Everything past here is a full-resolution
|
// The last cheap place to stop. Everything past here is a full-resolution
|
||||||
|
|||||||
@@ -27,6 +27,9 @@ pub mod step {
|
|||||||
use dr_pipeline::LocalizedKey;
|
use dr_pipeline::LocalizedKey;
|
||||||
|
|
||||||
pub const PASTE: LocalizedKey = LocalizedKey("history.paste");
|
pub const PASTE: LocalizedKey = LocalizedKey("history.paste");
|
||||||
|
/// TRACES: FR-DEV-6
|
||||||
|
/// The edit a photograph already carried, brought across on first opening.
|
||||||
|
pub const EARLIER_EDIT: LocalizedKey = LocalizedKey("history.earlier_edit");
|
||||||
pub const FILM: LocalizedKey = LocalizedKey("history.film");
|
pub const FILM: LocalizedKey = LocalizedKey("history.film");
|
||||||
/// TRACES: FR-DEV-5
|
/// TRACES: FR-DEV-5
|
||||||
/// The photograph put back to a named snapshot, as one step.
|
/// The photograph put back to a named snapshot, as one step.
|
||||||
@@ -100,6 +103,7 @@ pub mod step {
|
|||||||
dr_pipeline::history::OPENED,
|
dr_pipeline::history::OPENED,
|
||||||
dr_pipeline::history::UNNAMED,
|
dr_pipeline::history::UNNAMED,
|
||||||
PASTE,
|
PASTE,
|
||||||
|
EARLIER_EDIT,
|
||||||
SNAPSHOT_RESTORED,
|
SNAPSHOT_RESTORED,
|
||||||
FILM,
|
FILM,
|
||||||
SAMPLED_NEUTRAL,
|
SAMPLED_NEUTRAL,
|
||||||
@@ -322,6 +326,7 @@ fn catalogued(key: &str) -> Option<&'static str> {
|
|||||||
"history.opened" => "Opened",
|
"history.opened" => "Opened",
|
||||||
"history.edit" => "Edit",
|
"history.edit" => "Edit",
|
||||||
"history.paste" => "Paste Settings",
|
"history.paste" => "Paste Settings",
|
||||||
|
"history.earlier_edit" => "Earlier Edit",
|
||||||
"history.snapshot_restored" => "Restore A Snapshot",
|
"history.snapshot_restored" => "Restore A Snapshot",
|
||||||
"history.film" => "Film Stock",
|
"history.film" => "Film Stock",
|
||||||
"history.sampled_neutral" => "Sample Neutral",
|
"history.sampled_neutral" => "Sample Neutral",
|
||||||
|
|||||||
+48
-8
@@ -334,6 +334,10 @@ pub(crate) fn open_session(
|
|||||||
if dr_decode::probe(bytes) != Some(dr_types::Format::Jpeg) {
|
if dr_decode::probe(bytes) != Some(dr_types::Format::Jpeg) {
|
||||||
session.set_embedded_profile(dr_decode::dcp::embedded_in(bytes));
|
session.set_embedded_profile(dr_decode::dcp::embedded_in(bytes));
|
||||||
}
|
}
|
||||||
|
// TRACES: FR-DEV-6
|
||||||
|
// And the earlier edit stored in it, applied by the caller only once it
|
||||||
|
// knows DarkRoom has no edit of this photograph.
|
||||||
|
session.set_earlier_edit(dr_preset_xmp::read_embedded(bytes).map(|i| i.preset));
|
||||||
Ok(session)
|
Ok(session)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -820,7 +824,7 @@ pub(crate) fn refresh_export_label(window: &AppWindow) {
|
|||||||
/// whose session this sidecar must not be applied to.
|
/// whose session this sidecar must not be applied to.
|
||||||
fn apply_when_ready(
|
fn apply_when_ready(
|
||||||
window: &AppWindow,
|
window: &AppWindow,
|
||||||
rx: Rc<std::sync::mpsc::Receiver<Option<dr_pipeline::Sidecar>>>,
|
rx: Rc<std::sync::mpsc::Receiver<library::FetchedSidecar>>,
|
||||||
session: &Rc<RefCell<Option<DevelopSession>>>,
|
session: &Rc<RefCell<Option<DevelopSession>>>,
|
||||||
rows: &Rc<slint::VecModel<ParamRow>>,
|
rows: &Rc<slint::VecModel<ParamRow>>,
|
||||||
redraw: &Rc<dyn Fn(&AppWindow)>,
|
redraw: &Rc<dyn Fn(&AppWindow)>,
|
||||||
@@ -830,9 +834,7 @@ fn apply_when_ready(
|
|||||||
|
|
||||||
// Already here — the common case.
|
// Already here — the common case.
|
||||||
if let Ok(got) = rx.try_recv() {
|
if let Ok(got) = rx.try_recv() {
|
||||||
if let Some(sidecar) = got {
|
apply_fetched(window, got, session, rows);
|
||||||
presets::apply_stored_edit(window, &sidecar, session, rows);
|
|
||||||
}
|
|
||||||
redraw(window);
|
redraw(window);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -867,10 +869,7 @@ fn apply_when_ready(
|
|||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
held.stop();
|
held.stop();
|
||||||
let applied = match got {
|
let applied = apply_fetched(&w, got, &session, &rows);
|
||||||
Some(sidecar) => presets::apply_stored_edit(&w, &sidecar, &session, &rows),
|
|
||||||
None => false,
|
|
||||||
};
|
|
||||||
if applied || !drawn.get() {
|
if applied || !drawn.get() {
|
||||||
redraw(&w);
|
redraw(&w);
|
||||||
}
|
}
|
||||||
@@ -878,6 +877,42 @@ fn apply_when_ready(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// TRACES: FR-CAT-8 | FR-DEV-6
|
||||||
|
/// Apply what the stored-edit fetch answered: the edit, or — only where the
|
||||||
|
/// server said there is none — the photograph's earlier edit. An answer
|
||||||
|
/// that could not be had (offline, unreachable, unreadable) applies nothing,
|
||||||
|
/// so a real edit that failed to arrive is never saved over.
|
||||||
|
fn apply_fetched(
|
||||||
|
window: &AppWindow,
|
||||||
|
got: library::FetchedSidecar,
|
||||||
|
session: &Rc<RefCell<Option<DevelopSession>>>,
|
||||||
|
rows: &Rc<slint::VecModel<ParamRow>>,
|
||||||
|
) -> bool {
|
||||||
|
match got.sidecar {
|
||||||
|
Some(sidecar) => presets::apply_stored_edit(window, &sidecar, session, rows),
|
||||||
|
None if got.absent => adopt_earlier_edit(window, session, rows),
|
||||||
|
None => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// TRACES: FR-DEV-6
|
||||||
|
/// Start the open photograph from its earlier edit, if it has one.
|
||||||
|
fn adopt_earlier_edit(
|
||||||
|
window: &AppWindow,
|
||||||
|
session: &Rc<RefCell<Option<DevelopSession>>>,
|
||||||
|
rows: &Rc<slint::VecModel<ParamRow>>,
|
||||||
|
) -> bool {
|
||||||
|
let adopted = session
|
||||||
|
.borrow_mut()
|
||||||
|
.as_mut()
|
||||||
|
.is_some_and(DevelopSession::adopt_earlier_edit);
|
||||||
|
if adopted {
|
||||||
|
log::info!("started from the photograph's earlier edit");
|
||||||
|
sync_rows(window, rows, session);
|
||||||
|
}
|
||||||
|
adopted
|
||||||
|
}
|
||||||
|
|
||||||
/// TRACES: FR-DEV-3f
|
/// TRACES: FR-DEV-3f
|
||||||
/// Push the film choice out to the panel.
|
/// Push the film choice out to the panel.
|
||||||
///
|
///
|
||||||
@@ -2623,6 +2658,11 @@ fn build_show(
|
|||||||
*open_image.borrow_mut() = presets::Stored::Local(path.to_path_buf());
|
*open_image.borrow_mut() = presets::Stored::Local(path.to_path_buf());
|
||||||
if let Some(sidecar) = presets::load_local(path) {
|
if let Some(sidecar) = presets::load_local(path) {
|
||||||
presets::apply_stored_edit(window, &sidecar, &session, &rows);
|
presets::apply_stored_edit(window, &sidecar, &session, &rows);
|
||||||
|
} else {
|
||||||
|
// TRACES: FR-DEV-6
|
||||||
|
// No edit of DarkRoom's beside the file: start
|
||||||
|
// from the earlier edit stored in it.
|
||||||
|
adopt_earlier_edit(window, &session, &rows);
|
||||||
}
|
}
|
||||||
|
|
||||||
// TRACES: FR-UI-4
|
// TRACES: FR-UI-4
|
||||||
|
|||||||
@@ -440,7 +440,7 @@ pub fn spawn_sidecar_fetch(
|
|||||||
image_path: String,
|
image_path: String,
|
||||||
cache_dir: PathBuf,
|
cache_dir: PathBuf,
|
||||||
offline: bool,
|
offline: bool,
|
||||||
) -> Receiver<Option<dr_pipeline::Sidecar>> {
|
) -> Receiver<FetchedSidecar> {
|
||||||
let (tx, rx) = std::sync::mpsc::channel();
|
let (tx, rx) = std::sync::mpsc::channel();
|
||||||
|
|
||||||
executors::spawn(Executor::Network, "sidecars", move || {
|
executors::spawn(Executor::Network, "sidecars", move || {
|
||||||
@@ -455,7 +455,7 @@ pub fn spawn_sidecar_fetch(
|
|||||||
// which the next save would then write back over the top of.
|
// which the next save would then write back over the top of.
|
||||||
if cache.is_pending(&path_str) {
|
if cache.is_pending(&path_str) {
|
||||||
log::debug!("{path_str} has queued local edits; opening from the cache");
|
log::debug!("{path_str} has queued local edits; opening from the cache");
|
||||||
let _ = tx.send(cache.load(&path_str));
|
let _ = tx.send(FetchedSidecar::unknown(cache.load(&path_str)));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -472,7 +472,7 @@ pub fn spawn_sidecar_fetch(
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
let Some(rt) = rt else {
|
let Some(rt) = rt else {
|
||||||
let _ = tx.send(cache.load(&path_str));
|
let _ = tx.send(FetchedSidecar::unknown(cache.load(&path_str)));
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -481,7 +481,7 @@ pub fn spawn_sidecar_fetch(
|
|||||||
Ok(b) => b,
|
Ok(b) => b,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
log::debug!("sidecar fetch backend: {e}");
|
log::debug!("sidecar fetch backend: {e}");
|
||||||
let _ = tx.send(cache.load(&path_str));
|
let _ = tx.send(FetchedSidecar::unknown(cache.load(&path_str)));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@@ -491,12 +491,21 @@ pub fn spawn_sidecar_fetch(
|
|||||||
|
|
||||||
// A 404 is the normal case on a library that has never been
|
// A 404 is the normal case on a library that has never been
|
||||||
// edited, so this is `ok()` rather than an error path.
|
// edited, so this is `ok()` rather than an error path.
|
||||||
let Ok(bytes) = backend.get(&id, None).await else {
|
let bytes = match backend.get(&id, None).await {
|
||||||
// Unreachable, or no such file. The cache cannot tell those
|
Ok(bytes) => bytes,
|
||||||
// apart and does not need to: either way it holds the best
|
Err(e) => {
|
||||||
// answer this device has.
|
// Unreachable, or no such file. The cache holds the best
|
||||||
let _ = tx.send(cache.load(&path_str));
|
// answer this device has either way — but only a server
|
||||||
return;
|
// that said "no such file", with nothing in the cache,
|
||||||
|
// is the word that there is no edit (FR-DEV-6).
|
||||||
|
let cached = cache.load(&path_str);
|
||||||
|
let absent = cached.is_none() && matches!(e, dr_sync::RemoteError::NotFound(_));
|
||||||
|
let _ = tx.send(FetchedSidecar {
|
||||||
|
sidecar: cached,
|
||||||
|
absent,
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let text = String::from_utf8_lossy(&bytes).into_owned();
|
let text = String::from_utf8_lossy(&bytes).into_owned();
|
||||||
@@ -530,13 +539,36 @@ pub fn spawn_sidecar_fetch(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let _ = tx.send(parsed);
|
let _ = tx.send(FetchedSidecar::unknown(parsed));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
rx
|
rx
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// TRACES: FR-CAT-8 | FR-DEV-6
|
||||||
|
/// What a stored-edit fetch answered.
|
||||||
|
///
|
||||||
|
/// `absent` is true only when the server said there is no such file and no
|
||||||
|
/// cached copy stood in — the one answer that means "DarkRoom has no edit of
|
||||||
|
/// this photograph", and so the only one that lets its earlier edit in.
|
||||||
|
/// Every other way of arriving at no sidecar — offline, unreachable, a file
|
||||||
|
/// that would not parse — leaves it false.
|
||||||
|
#[derive(Debug, Default)]
|
||||||
|
pub struct FetchedSidecar {
|
||||||
|
pub sidecar: Option<dr_pipeline::Sidecar>,
|
||||||
|
pub absent: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl FetchedSidecar {
|
||||||
|
fn unknown(sidecar: Option<dr_pipeline::Sidecar>) -> Self {
|
||||||
|
Self {
|
||||||
|
sidecar,
|
||||||
|
absent: false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Fetch one file in full, for opening it in develop.
|
/// Fetch one file in full, for opening it in develop.
|
||||||
///
|
///
|
||||||
/// Deliberately *not* the preview path. Browsing fetches a range and decodes
|
/// Deliberately *not* the preview path. Browsing fetches a range and decodes
|
||||||
|
|||||||
Reference in New Issue
Block a user