dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Let a photograph leave: an export button, and a cache to leave from

Browse Source 
dr-export could turn a frame into bytes and nothing could ask it to. This is
the button, and the place the bytes go.

**Everything is staged first.** An export bound for the server is written to a
local outbox and uploaded afterwards; offline is not a special case, it is the
same path with a drain that finds the server absent. Doing it the other way —
upload directly, stage only on failure — makes the failure path the one that
is rarely exercised and always broken, and a network drop mid-batch leaves
some exports existing and some not with nothing recording which. Staged first,
an export is finished the moment it is written and the upload is a promise
kept later.

The outbox sits beside the catalog rather than under the cache. dr_catalog's
cache already draws that line: passive entries are a convenience and go under
LRU, pinned ones are a promise and never do. An export awaiting upload is a
promise — the user was told it succeeded — and sweeping it for disk would
destroy the only copy. Bytes are written before the destination record, so a
kill between the two leaves an orphan the drain ignores rather than a record
pointing at nothing.

The status line says "Queued for Exports/2026", never "Exported to Nextcloud",
until it has actually landed. There is a test asserting that wording, because
the tempting shorter sentence is a claim the app cannot keep.

The drain runs on the sync pass, before the shards: a thumbnail shard can be
rebuilt from the originals and the catalog is an index, but a queued export
exists nowhere else.

`DevelopSession::render_for_export` renders the framed size rather than reusing
the frame on screen, which is deliberately viewport-sized (FR-DSP-1) — encoding
that would hand the user a soft, screen-sized file with nothing to say anything
had been lost (FR-EXP-9).

One compromise, recorded rather than hidden: the export runs synchronously on
the UI thread, so the window is unresponsive for the few hundred milliseconds
a full-resolution render and encode takes. Moving a DevelopSession and its GPU
pass to a worker is a larger change than one button earns, and it is batch
export that makes the wait intolerable rather than merely noticeable.

Still missing: the Nextcloud folder *picker*. The destination is typed into
Settings for now. `FolderBrowser` in launch.rs is already the reusable model
for it — it browses a remote tree and nothing about it is specific to choosing
a library root — but wiring it into the settings page needs a listing worker
and browser UI there, which is its own piece of work.

Carries in-flight work from a parallel session — presets, the develop copy and
paste, and the node schema's `presentation` and `enum` support. One misplaced
callback in settings_ui.rs is moved from `render` to `wire`: registered in
`render` it borrowed a `&SettingsController` into a 'static closure and would
not compile, and that file's own docs say render pushes properties while wire
connects callbacks.

992 tests pass, clippy and fmt clean. Traceability 48.3% -> 51.0%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-16 23:58:17 +02:00
co-authored by Claude Opus 5
parent 23f0c4b76a
commit e00c99b864
19 changed files with 2845 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/presets.rs
+653
View File
@@ -0,0 +1,653 @@
//! TRACES: FR-DEV-6
//! Copying develop settings from one photograph to others.
//!
//! # The three pieces
//!
//! * A [`Clipboard`] — one [`Preset`] held for the life of the window. Not
//! the system clipboard: these are typed values addressed by `op.param`,
//! and putting them on a text clipboard would mean anything that happened
//! to be copied afterwards silently disarmed the paste.
//! * An [`OpenImage`] — where the develop view's edit is *stored*, which is
//! what a copy is taken from and what a paste has to be written back to.
//! * The batch, in [`paste_to_selection`], which never opens an image at all.
//!
//! # Why a paste can reach images that are not open
//!
//! Applying settings to forty frames by opening forty develop sessions would
//! mean forty downloads of a whole RAW file and forty demosaics, to change
//! some numbers. The edit is *data* — [`Preset::amend`] operates on the
//! parameter map a sidecar already stores — so the batch is a read-modify-write
//! per file and never touches a GPU. That is what makes it usable on a phone.
//!
//! The cost is that a batch paste is only visible once the target is opened
//! and its sidecar read, which is the load path this module also provides.
//!
//! # Two stores, because there are two ways an image is opened
//!
//! An image from the library grid lives on the server and its sidecar goes
//! beside it, through the same writer a rating uses. An image named on the
//! command line is a local file with no library behind it, and its sidecar is
//! written beside it on disk. Both are the same document in the same format —
//! only the transport differs, which is why [`Stored`] is an enum over where
//! rather than two notions of what.
use std::cell::RefCell;
use std::path::{Path, PathBuf};
use std::rc::Rc;
use dr_pipeline::{Preset, Scope, Sidecar};
use slint::ComponentHandle;
use crate::develop::DevelopSession;
use crate::{library, library_ui, settings_ui, AppWindow, ParamRow};
/// Where the develop view's current edit is stored.
///
/// `None` is not an error state: an image that failed to decode, or one opened
/// before a library was chosen, has nowhere to persist to and simply does not.
/// Copy still works from it — the graph is in memory either way — and it is
/// only the *saving* that has no destination.
#[derive(Debug, Clone, Default)]
pub enum Stored {
#[default]
Nowhere,
/// A file on this device, named on the command line.
Local(PathBuf),
/// An image in the library. The uuid comes from the catalog so that this
/// device and every other name the same version (FR-NC-8).
Remote { path: String, version_uuid: String },
}
/// Which image the develop view is showing, and where its edit belongs.
pub type OpenImage = Rc<RefCell<Stored>>;
/// The settings clipboard.
///
/// Holds the preset at full scope; the [`Scope`] is applied at paste time from
/// the user's setting, so changing that setting after copying takes effect on
/// the next paste rather than requiring a fresh copy.
#[derive(Default)]
pub struct Clipboard {
preset: RefCell<Option<Preset>>,
}
impl Clipboard {
pub fn new() -> Rc<Self> {
Rc::new(Self::default())
}
/// Take a copy of a session's edit.
pub fn copy_from(&self, session: &DevelopSession) {
self.put(session.copy_settings());
}
/// Hold an already-captured preset.
pub fn put(&self, preset: Preset) {
*self.preset.borrow_mut() = Some(preset);
}
/// The held preset, if there is one.
pub fn peek(&self) -> Option<Preset> {
self.preset.borrow().clone()
}
/// Whether anything has been copied.
///
/// True even for a *neutral* copy. Copying an unedited photograph and
/// pasting it onto an edited one is a meaningful action — it clears the
/// target — so "has something been copied" is a different question from
/// "does the copy contain any values", and this is the first.
pub fn is_armed(&self) -> bool {
self.preset.borrow().is_some()
}
/// Whether the held preset carries framing that the current scope drops.
///
/// What the interface uses to explain a setting's effect on *this*
/// clipboard rather than in the abstract.
pub fn holds_framing(&self) -> bool {
self.preset
.borrow()
.as_ref()
.is_some_and(|p| p.touches_framing())
}
/// A short description of what would be pasted, for a button's label.
///
/// Counts operations rather than parameters: the colour mixer alone
/// declares thirty-six, and "36 settings" would say something about the
/// mixer's shape rather than about how much was copied.
pub fn describe(&self, scope: Scope) -> String {
let Some(preset) = self.preset.borrow().clone() else {
return String::new();
};
match preset.op_count(scope) {
0 => "Neutral".to_string(),
1 => "1 adjustment".to_string(),
n => format!("{n} adjustments"),
}
}
}
/// The scope a paste should use, from the user's preference.
///
/// One function rather than the boolean read at each call site, so "the
/// setting is off by default and means framing stays behind" is stated once.
pub fn scope_for(settings: &dr_types::Settings) -> Scope {
if settings.develop.copy_includes_framing {
Scope::Everything
} else {
Scope::Adjustments
}
}
// ---------------------------------------------------------------------------
// Local sidecars
// ---------------------------------------------------------------------------
/// The sidecar path for a local image — the file's own path with the
/// extension replaced.
///
/// The same rule [`crate::library::sidecar_path`] applies to remote paths, so
/// a folder synced between the two is read identically from either side.
pub fn local_sidecar_path(image: &Path) -> PathBuf {
image.with_extension(dr_pipeline::sidecar::EXTENSION)
}
/// Read a local sidecar, if there is one.
///
/// Absence is the common case and is not an error. An *unreadable* file yields
/// `None` too, and [`save_local`] independently refuses to overwrite one — so
/// a sidecar this build cannot parse costs the edit being shown, never the
/// edit being kept.
pub fn load_local(image: &Path) -> Option<Sidecar> {
let path = local_sidecar_path(image);
let text = std::fs::read_to_string(&path).ok()?;
match Sidecar::parse(&text) {
Ok(s) => Some(s),
Err(e) => {
log::warn!("sidecar at {} is unreadable ({e})", path.display());
None
}
}
}
/// Read-modify-write a local sidecar.
///
/// Read first for the same reason the remote writer does: the file may already
/// hold a rating, or an operation this build does not know about, and writing
/// a fresh document containing only the current edit would delete both.
pub fn save_local(image: &Path, preset: &Preset, scope: Scope) -> Result<(), String> {
let path = local_sidecar_path(image);
// Distinguish "no sidecar yet" from "a sidecar this build cannot read".
// Only the second is a refusal — overwriting it would destroy an edit we
// merely failed to understand.
let existing = std::fs::read_to_string(&path).ok();
let mut sidecar = match existing.as_deref() {
None => Sidecar::new(),
Some(text) => Sidecar::parse(text).map_err(|e| {
format!(
"existing sidecar at {} is unreadable ({e}); not overwriting",
path.display()
)
})?,
};
// A local file has no catalog behind it to supply a version identity, so
// the file's own default version is used and one is created if absent.
// Deterministic rather than random: reopening the same photograph must
// amend the version it wrote last time, not accumulate one per save.
let uuid = sidecar
.default_version()
.map(|v| v.uuid.clone())
.unwrap_or_else(|| "local".to_string());
let mut version =
sidecar
.versions
.get(&uuid)
.cloned()
.unwrap_or_else(|| dr_pipeline::sidecar::Version {
uuid: uuid.clone(),
name: "Default".to_string(),
is_default: true,
revision: 0,
..Default::default()
});
preset.amend(&mut version.params, scope);
version.revision = version.revision.saturating_add(1);
version.modified = now_secs();
sidecar.put(version);
let text = sidecar.to_text();
// Write and rename, so an interrupted save cannot truncate an edit that
// was already safely on disk.
let tmp = path.with_extension("drsc.tmp");
std::fs::write(&tmp, text).map_err(|e| e.to_string())?;
std::fs::rename(&tmp, &path).map_err(|e| e.to_string())
}
// ---------------------------------------------------------------------------
// Saving and loading the open image
// ---------------------------------------------------------------------------
/// Persist the develop view's current edit to wherever it belongs.
///
/// Called when the image is about to stop being the open one — on leaving for
/// the library, on stepping to the next frame, and on closing the window —
/// rather than on every slider move. A save is a network round-trip on the
/// library path, and one per drag frame would put an upload inside the gesture
/// that NFR-P5 is about.
///
/// Silent on success and logged on failure, deliberately. There is no
/// acknowledgement worth interrupting a photographer for, and the failure that
/// matters — a sidecar this build could not parse — is refused by the writer
/// rather than resolved here.
pub fn save_open_edit(
window: &AppWindow,
stored: &Stored,
session: &Rc<RefCell<Option<DevelopSession>>>,
library: &Rc<library_ui::LibraryController>,
) {
let Some(preset) = session.borrow().as_ref().map(|s| s.copy_settings()) else {
return;
};
match stored {
// Nothing to save to. An image that failed to decode, or one opened
// before a library was chosen.
Stored::Nowhere => {}
Stored::Local(path) => {
// `Everything`: this is the image's *own* edit being written back,
// not a paste onto someone else's. Excluding framing here would
// make a crop the one adjustment that never survived a restart.
if let Err(e) = save_local(path, &preset, Scope::Everything) {
log::warn!("saving {}: {e}", path.display());
}
}
Stored::Remote { path, version_uuid } => {
let write = library::SidecarWrite {
image_path: path.clone(),
version_uuid: version_uuid.clone(),
amendment: library::Amendment::Settings {
preset,
scope: Scope::Everything,
},
};
library_ui::start_sidecar_writes(window, library, vec![write]);
}
}
}
/// Load a stored edit into the open session and refresh the panel.
///
/// Returns whether anything was applied, so the caller can skip a redraw for
/// the common case of a photograph that has never been edited.
pub fn apply_stored_edit(
window: &AppWindow,
sidecar: &Sidecar,
session: &Rc<RefCell<Option<DevelopSession>>>,
rows: &Rc<slint::VecModel<ParamRow>>,
) -> bool {
let Some(version) = sidecar.default_version() else {
return false;
};
{
let mut slot = session.borrow_mut();
let Some(s) = slot.as_mut() else { return false };
s.apply_version(version);
}
crate::sync_rows(window, rows, session);
true
}
// ---------------------------------------------------------------------------
// Wiring
// ---------------------------------------------------------------------------
/// Push the clipboard's state onto the window.
///
/// One function rather than three `set_` calls at each site, because the three
/// properties have to agree: a summary describing a scope the button is not
/// using would be worse than no summary.
pub fn render(
window: &AppWindow,
clipboard: &Rc<Clipboard>,
settings: &Rc<settings_ui::SettingsController>,
) {
let scope = scope_for(&settings.snapshot());
window.set_settings_armed(clipboard.is_armed());
window.set_settings_summary(clipboard.describe(scope).into());
// Only worth saying when it is actually true of *this* copy: a clipboard
// holding no crop loses nothing to the setting, and saying so anyway would
// train the user to ignore the line.
window.set_settings_framing_withheld(clipboard.holds_framing() && scope == Scope::Adjustments);
}
/// The develop view's shared state, as this module needs it.
///
/// Bundled rather than passed one by one because these four always travel
/// together — a paste changes the graph, so it must rebuild the panel, redraw
/// the canvas and know where to save, and a call site holding three of the
/// four is a call site with a bug in it.
#[derive(Clone)]
pub struct Develop {
pub session: Rc<RefCell<Option<DevelopSession>>>,
pub rows: Rc<slint::VecModel<ParamRow>>,
pub redraw: Rc<dyn Fn(&AppWindow)>,
/// Where the open image's edit is stored.
pub open: OpenImage,
}
/// Wire copy, paste and batch-paste.
pub fn wire(
window: &AppWindow,
clipboard: Rc<Clipboard>,
develop: Develop,
settings: Rc<settings_ui::SettingsController>,
library: Rc<library_ui::LibraryController>,
collections: Rc<crate::collections_ui::CollectionsController>,
) {
let Develop {
session,
rows,
redraw,
open,
} = develop;
// --- copy ------------------------------------------------------------
{
let weak = window.as_weak();
let clipboard = clipboard.clone();
let session = session.clone();
let settings = settings.clone();
window.on_copy_settings(move || {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow().as_ref() {
clipboard.copy_from(s);
}
render(&w, &clipboard, &settings);
});
}
// --- paste onto the open image ---------------------------------------
{
let weak = window.as_weak();
let clipboard = clipboard.clone();
let session = session.clone();
let settings = settings.clone();
let rows = rows.clone();
let redraw = redraw.clone();
let open = open.clone();
let library = library.clone();
window.on_paste_settings(move || {
let Some(w) = weak.upgrade() else { return };
let Some(preset) = clipboard.peek() else {
return;
};
let scope = scope_for(&settings.snapshot());
{
let mut slot = session.borrow_mut();
let Some(s) = slot.as_mut() else { return };
s.apply_settings(&preset, scope);
}
// The sliders are showing the values that just moved, so the panel
// has to be rebuilt — a paste is the one edit that changes many
// controls without any of them having been touched.
crate::sync_rows(&w, &rows, &session);
redraw(&w);
// Saved immediately rather than on the way out. A paste is a
// deliberate, discrete action, unlike a drag, and the cost of one
// write is nothing beside the surprise of it not having stuck.
save_open_edit(&w, &open.borrow(), &session, &library);
});
}
// --- paste onto the selection ----------------------------------------
{
let weak = window.as_weak();
let clipboard = clipboard.clone();
let settings = settings.clone();
let library = library.clone();
let collections = collections.clone();
window.on_paste_settings_to_selection(move || {
let Some(w) = weak.upgrade() else { return };
let Some(preset) = clipboard.peek() else {
return;
};
let scope = scope_for(&settings.snapshot());
library_ui::paste_settings_to_selection(
&w,
&library,
&collections.selected(),
&preset,
scope,
);
});
}
}
/// Seconds since the epoch, or zero if the clock is before it.
///
/// Zero rather than a panic: a wrong timestamp costs a tie-break in the merge,
/// which resolves on `revision` first anyway (FR-NC-9), and refusing to save
/// because a clock is unset would be far worse.
fn now_secs() -> i64 {
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_secs() as i64)
.unwrap_or(0)
}
#[cfg(test)]
mod tests {
use super::*;
use dr_pipeline::EditGraph;
fn tempdir(name: &str) -> PathBuf {
let dir = std::env::temp_dir().join(format!(
"dr-presets-test-{name}-{}-{:?}",
std::process::id(),
std::thread::current().id()
));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
dir
}
fn edited() -> EditGraph {
use dr_pipeline::ops::exposure;
let mut g = EditGraph::default_chain();
g.set_param(exposure::ID, exposure::EXPOSURE, 1.5);
g
}
#[test]
fn a_sidecar_sits_beside_its_image_with_the_extension_replaced() {
// Replaced, not appended, so a RAW and the JPEG beside it share one
// sidecar — they are the same photograph (FR-CAT-11).
assert_eq!(
local_sidecar_path(Path::new("/photos/a.CR2")),
PathBuf::from("/photos/a.drsc")
);
}
#[test]
fn an_edit_survives_a_save_and_a_load() {
use dr_pipeline::ops::exposure;
let dir = tempdir("round-trip");
let image = dir.join("a.CR2");
save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
let sidecar = load_local(&image).expect("a sidecar was written");
let mut restored = EditGraph::default_chain();
sidecar
.default_version()
.expect("a version")
.apply(&mut restored);
assert_eq!(restored.param(exposure::ID, exposure::EXPOSURE), Some(1.5));
}
#[test]
fn an_image_with_no_sidecar_loads_as_nothing() {
let dir = tempdir("absent");
assert!(load_local(&dir.join("never-edited.CR2")).is_none());
}
#[test]
fn saving_twice_amends_one_version_rather_than_accumulating_them() {
// A random uuid per save would leave the file growing a version every
// time the user left the develop view, and `default_version` would
// start answering with whichever sorted first.
let dir = tempdir("one-version");
let image = dir.join("a.CR2");
save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
assert_eq!(load_local(&image).expect("a sidecar").versions.len(), 1);
}
#[test]
fn a_save_bumps_the_revision() {
// FR-NC-9 resolves conflicts by revision; a write that did not bump it
// would lose to a stale remote copy at the next sync.
let dir = tempdir("revision");
let image = dir.join("a.CR2");
save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
let first = load_local(&image)
.unwrap()
.default_version()
.unwrap()
.revision;
save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
let second = load_local(&image)
.unwrap()
.default_version()
.unwrap()
.revision;
assert!(second > first, "{second} did not follow {first}");
}
#[test]
fn a_save_does_not_destroy_a_rating_it_never_read() {
// The read-modify-write property. A cull is stored in the same version
// as the edit, and leaving the develop view must not wipe it.
let dir = tempdir("keeps-rating");
let image = dir.join("a.CR2");
let mut sidecar = Sidecar::new();
sidecar.put(dr_pipeline::sidecar::Version {
uuid: "local".to_string(),
name: "Default".to_string(),
is_default: true,
rating: 4,
flag: 1,
..Default::default()
});
std::fs::write(local_sidecar_path(&image), sidecar.to_text()).unwrap();
save_local(&image, &Preset::capture(&edited()), Scope::Everything).unwrap();
let back = load_local(&image).unwrap();
let v = back.default_version().unwrap();
assert_eq!(v.rating, 4, "the cull was destroyed by an edit");
assert_eq!(v.flag, 1);
}
#[test]
fn a_save_refuses_to_overwrite_an_unreadable_sidecar() {
// The file may hold an edit written by a newer build. Losing it
// because this one could not parse it is the worst available failure
// for an authoritative store.
let dir = tempdir("refuse");
let image = dir.join("a.CR2");
std::fs::write(local_sidecar_path(&image), "drsc 99\n").unwrap();
assert!(save_local(&image, &Preset::capture(&edited()), Scope::Everything).is_err());
assert_eq!(
std::fs::read_to_string(local_sidecar_path(&image)).unwrap(),
"drsc 99\n",
"the file was modified despite the refusal"
);
}
#[test]
fn saving_leaves_no_temporary_file_behind() {
let dir = tempdir("no-temp");
save_local(
&dir.join("a.CR2"),
&Preset::capture(&edited()),
Scope::Everything,
)
.unwrap();
let leftovers: Vec<_> = std::fs::read_dir(&dir)
.unwrap()
.filter_map(|e| e.ok())
.map(|e| e.file_name().to_string_lossy().to_string())
.filter(|n| n.ends_with(".tmp"))
.collect();
assert!(leftovers.is_empty(), "left {leftovers:?} behind");
}
// --- the clipboard ------------------------------------------------------
#[test]
fn a_fresh_clipboard_is_not_armed() {
assert!(!Clipboard::default().is_armed());
}
#[test]
fn the_scope_follows_the_setting_and_defaults_to_sparing_the_crop() {
let mut settings = dr_types::Settings::default();
assert_eq!(
scope_for(&settings),
Scope::Adjustments,
"a fresh install must not carry crops between photographs"
);
settings.develop.copy_includes_framing = true;
assert_eq!(scope_for(&settings), Scope::Everything);
}
#[test]
fn the_description_counts_operations_and_not_parameters() {
use dr_pipeline::ops::{exposure, white_balance};
let clipboard = Clipboard::default();
let mut g = EditGraph::default_chain();
g.set_param(exposure::ID, exposure::EXPOSURE, 1.0);
g.set_param(white_balance::ID, white_balance::TEMPERATURE, 20.0);
g.set_param(white_balance::ID, white_balance::TINT, 5.0);
clipboard.put(Preset::capture(&g));
// Three parameters, two operations.
assert_eq!(clipboard.describe(Scope::Adjustments), "2 adjustments");
}
#[test]
fn a_neutral_copy_is_armed_and_says_so() {
// Copying an unedited frame and pasting it clears the target, which is
// a real action — so the button must be live rather than looking like
// nothing was copied.
let clipboard = Clipboard::default();
clipboard.put(Preset::capture(&EditGraph::default_chain()));
assert!(clipboard.is_armed());
assert_eq!(clipboard.describe(Scope::Adjustments), "Neutral");
}
}