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

Export a whole selection, on a thread that is not the interface's

Browse Source 
The export button rendered, resampled and encoded a 24 MP frame on the UI
thread and the window was dead for all of it. That was written down as a known
compromise, on the grounds that a batch is what makes the wait intolerable
rather than merely noticeable. This is the batch, so the compromise comes due.

The grid's selection now exports (FR-EXP-7). A worker thread takes a clone of
the `GpuContext` — an `Arc` pair over a device and a queue — and opens each
photograph for itself: fetch, sidecar, decode, demosaic, render at full size,
resample, sharpen, encode, write. Nothing of that touches the interface, which
keeps drawing throughout, and the progress goes where every other background
job's does: one row in the activity register, with a count and a bar.

Why the worker does not borrow the session it could have had. A
`DevelopSession` owns the `AdjustPass` the canvas renders from, so handing it
to a worker would stop the develop view drawing for the length of the batch —
the same freeze, moved. Opening a session per image instead costs a
`Demosaicer` and an `AdjustPass` each time round, and the pipeline cache is
per-pass so the composed shader is recompiled per image rather than once for
the run. Against a full-resolution decode, render and encode that is a few
percent, and it keeps this file out of the pipeline `develop` owns. A reusable
export pass is the obvious next economy if a profile ever says so.

The open image is the exception, and it is why the develop button is not simply
a one-image batch. Its edit lives in the interface's session and may not have
reached a sidecar yet, so a worker that re-opened the file would export the
saved version rather than the one on screen. That frame is therefore rendered
by the caller and handed over as `Source::Rendered`; everything after the
render — the Lanczos reduction, the encode, the write, which is the larger half
of the wait and all of its variance — still leaves the UI thread. So the
develop export is no longer synchronous, but it is not fully off-thread either,
and the doc comment says so rather than claiming otherwise.

Cancellation (NFR-ARCH-3) is an `AtomicBool` read between stages, and the
export button becomes the cancel button while a run is live — a batch that
could only be stopped by not touching the selection would be a trap. Waits on
another worker use `recv_timeout` rather than `recv`, so a cancelled batch
sitting on a forty-megabyte download gives up within 100 ms instead of when the
transfer finishes. The honest bound is worse than that: a frame already in
render has no interior stopping point, so the worst case is one image. Closing
that needs the render itself to become interruptible, which is NFR-ARCH-2's
scheduler and not a finer poll here.

Failures are per image and typed (NFR-ARCH-4). One unreadable body, one folder
that cannot be written, one server that went away — each is a message on the
channel, a line in the log, and a count in the summary, and the batch carries
on. A run with any failure keeps its row until it is cleared, because that is
the row somebody came to the list to find; a cancelled run does not, because
they asked for it.

Two collisions that look alike and are not. `CollisionPolicy` is the user's
answer to "a file of this name was already there", and Overwrite is a fine
answer to that. It is not an answer to "the frame I exported four seconds ago
was also called this" — two folders in a library each holding an IMG_0001 is
ordinary — so a name the run has already issued is always stepped past whatever
the policy says about the folder. Both halves are held by tests.

Supporting changes, each smaller than it sounds. `open_session` comes out of
`load_bytes` so the worker shares the JPEG-versus-RAW routing rather than
carrying a copy that would drift; the half that builds a `slint::Image` stays
behind, where it belongs. `LibraryController::selected_image_paths` answers
from the catalog rather than from the loaded window, because selection is by id
and survives a scrub — a selection made before scrolling routinely names
photographs no row holds. `cache_context_for` takes an id for the same reason,
so a batch reads the originals cache instead of re-downloading three hundred
files. `format_date` is shared so `{date}` and the timeline agree about what
day a photograph was taken.

Left undone, deliberately: the batch is sequential, where FR-EXP-7 asks for all
available cores. Four full-resolution frames in flight is tens of megabytes
each and a straightforward way to exhaust a tablet, and the GPU is shared with
the interface in any case. Also undone: exporting with a chosen preset rather
than the current export settings — that is FR-EXP-5's machinery, which does not
exist yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-17 09:59:57 +02:00
co-authored by Claude Opus 5
parent 2330ed25e9
commit 7d3c8c521f
6 changed files with 1410 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/lib.rs
+215 -136
View File
@@ -116,48 +116,12 @@ fn load(ctx: Option<&dr_gpu::GpuContext>, path: &Path) -> Result<Loaded, String>
fn load_bytes(ctx: Option<&dr_gpu::GpuContext>, bytes: &[u8]) -> Result<Loaded, String> {
let meta = dr_decode::metadata(bytes).unwrap_or_default();
// Route by what the bytes actually are, not by extension (M-9).
//
// A JPEG has no sensor data and never will, so trying the RAW decoder
// first would be a guaranteed failure whose log line reads like a fault.
// It goes straight to the RGB path instead, which is what makes develop
// mode work on the JPEGs the library already indexes.
let is_jpeg = dr_decode::probe(bytes) == Some(dr_types::Format::Jpeg);
// How the file stored its pixels. A file that says nothing is taken as
// upright — see `Orientation::from_exif`.
let orientation = meta.orientation.unwrap_or_default();
if let Some(ctx) = ctx {
let opened = if is_jpeg {
dr_decode::decode_jpeg(bytes)
.map_err(|e| e.to_string())
.and_then(|mut p| {
// Fit the device before uploading. A film scan runs to
// 13728×8928, well past the 8192 a typical GPU can hold,
// and refusing it would drop the image back to a
// read-only preview — the very thing this path exists to
// avoid. 8192 is still four times a 4K long edge.
let limit = dr_gpu::DemosaicedImage::max_dimension(ctx);
if p.width.max(p.height) > limit {
log::info!(
"{}×{} exceeds the {limit} texture limit; fitting to it",
p.width,
p.height
);
p.downscale_to(limit);
}
DevelopSession::open_rgb(ctx, &p.rgba, p.width, p.height, orientation)
})
} else {
// A failure here is expected for bodies rawler does not know, and
// must not stop the image displaying (FR-RAW-4).
dr_decode::decode(bytes)
.map_err(|e| e.to_string())
.and_then(|raw| DevelopSession::open(ctx, &raw, orientation))
};
match opened {
match open_session(ctx, bytes, orientation) {
Ok(session) => {
let (width, height) = session.source_size();
return Ok(Loaded {
@@ -197,6 +161,51 @@ fn load_bytes(ctx: Option<&dr_gpu::GpuContext>, bytes: &[u8]) -> Result<Loaded,
})
}
/// TRACES: FR-EXP-7 | FR-RAW-4
/// Open bytes for editing, with no interface types involved.
///
/// Split out of [`load_bytes`] so the batch exporter can share it. That runs on
/// a worker thread, where a `slint::Image` has no business being constructed —
/// and a second copy of the JPEG-versus-RAW routing is a second copy that would
/// drift, which is exactly how a batch comes to export something the viewer
/// would have shown differently.
pub(crate) fn open_session(
ctx: &dr_gpu::GpuContext,
bytes: &[u8],
orientation: dr_types::Orientation,
) -> Result<DevelopSession, String> {
// Route by what the bytes actually are, not by extension (M-9). A JPEG has
// no sensor data and never will, so trying the RAW decoder first would be a
// guaranteed failure whose log line reads like a fault.
if dr_decode::probe(bytes) == Some(dr_types::Format::Jpeg) {
return dr_decode::decode_jpeg(bytes)
.map_err(|e| e.to_string())
.and_then(|mut p| {
// Fit the device before uploading. A film scan runs to
// 13728×8928, well past the 8192 a typical GPU can hold, and
// refusing it would drop the image back to a read-only preview
// — the very thing this path exists to avoid. 8192 is still
// four times a 4K long edge.
let limit = dr_gpu::DemosaicedImage::max_dimension(ctx);
if p.width.max(p.height) > limit {
log::info!(
"{}×{} exceeds the {limit} texture limit; fitting to it",
p.width,
p.height
);
p.downscale_to(limit);
}
DevelopSession::open_rgb(ctx, &p.rgba, p.width, p.height, orientation)
});
}
// A failure here is expected for bodies rawler does not know, and must not
// stop the image displaying (FR-RAW-4).
dr_decode::decode(bytes)
.map_err(|e| e.to_string())
.and_then(|raw| DevelopSession::open(ctx, &raw, orientation))
}
/// Collect displayable images from file or directory arguments.
fn collect(paths: &[PathBuf]) -> Vec<PathBuf> {
let mut out = Vec::new();
@@ -289,93 +298,63 @@ fn sync_framing(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSession>>
window.set_crop_h(crop.height);
}
/// TRACES: FR-EXP-6 | FR-EXP-9
/// Render the open image at full resolution and place the result.
/// TRACES: FR-EXP-6 | FR-EXP-9 | FR-EXP-7
/// Render the open image at full resolution, ready to be handed to the worker.
///
/// Synchronous, on the UI thread, and that is a known compromise rather than
/// an oversight. A full-resolution render plus a Lanczos reduction plus an
/// encode is hundreds of milliseconds on a 24 MP frame, and the window is
/// unresponsive for all of it. It is done this way because the alternative —
/// moving a `DevelopSession` and its GPU pass onto a worker — is a larger
/// change than the button is worth before batch export exists, and a batch is
/// what makes the wait intolerable rather than merely noticeable. The status
/// line says what is happening in the meantime.
fn export_now(
/// The render stays here, on the UI thread, and everything after it does not.
/// That split is deliberate rather than the remains of the synchronous version
/// this replaced: the frame belongs to the `DevelopSession` the interface owns,
/// and the edit in it may not have reached a sidecar yet, so a worker that
/// re-opened the photograph for itself would export the *saved* version rather
/// than the one on screen.
///
/// What is left on this thread is a GPU pass and a readback. The Lanczos
/// reduction, the encode and the write — the larger half of the wait, and all
/// of its variance — leave with the frame.
fn render_open_frame(
window: &AppWindow,
session: &Rc<RefCell<Option<DevelopSession>>>,
settings: &Rc<settings_ui::SettingsController>,
library: &Rc<library_ui::LibraryController>,
) -> Result<export::Placed, String> {
) -> Result<export::Source, String> {
let mut borrowed = session.borrow_mut();
let Some(session) = borrowed.as_mut() else {
return Err("nothing is open".into());
};
let stored = settings.snapshot();
let frame = session.render_for_export()?;
// The size has to be resolved before the name, because `{dimensions}` is
// one of the tokens the template can carry.
let (tw, th) = dr_export::target_size(
frame.width,
frame.height,
stored.export.sizing,
stored.export.allow_upscaling,
);
let filename = window.get_filename().to_string();
let stem = std::path::Path::new(&filename)
.file_stem()
.map(|s| s.to_string_lossy().into_owned())
.unwrap_or_else(|| "export".into());
let outbox = match library.session() {
Some((_, s)) => export::outbox_dir(&s.server, &s.user_id),
// No account, so no outbox — a device export still works, and a
// remote one is refused below by `place` rather than here, so the
// message names the setting rather than the plumbing.
None => std::env::temp_dir().join("darkroom-outbox"),
};
Ok(export::Source::Rendered { stem, frame })
}
let ctx = dr_export::NameContext {
source_stem: &stem,
sequence: 1,
date: "",
width: tw,
height: th,
preset: "",
};
/// TRACES: FR-EXP-7
/// Everything a batch needs that only the UI thread can assemble.
fn batch_request(
sources: Vec<export::Source>,
settings: &Rc<settings_ui::SettingsController>,
library: &Rc<library_ui::LibraryController>,
gpu: Option<&dr_gpu::GpuContext>,
) -> export::BatchRequest {
let stored = settings.snapshot();
// What counts as "taken" depends on where this is going. A device export
// can look at the folder; a queued one is checked against the outbox,
// since the server cannot be reached from here and may not be reachable
// at all — see `export::stage` for why two queued exports of one name
// both survive regardless.
let target_dir = std::path::PathBuf::from(&stored.export.destination);
let taken = |name: &str| -> bool {
match stored.export.target {
dr_types::ExportTarget::Device => target_dir.join(name).exists(),
dr_types::ExportTarget::Remote => false,
}
};
let name = dr_export::resolve_name(
&stored.export.filename_template,
&ctx,
stored.export.format,
stored.export.collision,
&taken,
)
.ok_or("a file of that name is already there, and the collision setting is Skip")?;
let encoded = dr_export::export(&frame, &stored.export, name).map_err(|e| e.to_string())?;
export::place(
&encoded,
stored.export.target,
&stored.export.destination,
&outbox,
)
export::BatchRequest {
sources,
creds: library.credentials(),
settings: stored.export,
outbox: match library.session() {
Some((_, s)) => export::outbox_dir(&s.server, &s.user_id),
// No account, so no outbox — a device export still works, and a
// remote one is refused by `place` rather than here, so the message
// names the setting rather than the plumbing.
None => std::env::temp_dir().join("darkroom-outbox"),
},
sidecar_cache: library.sidecar_cache_dir().unwrap_or_default(),
offline: library.is_offline(),
gpu: gpu.cloned(),
}
}
/// What the export button should say, given where an export would go.
@@ -385,11 +364,19 @@ fn export_now(
/// lands on this device or is queued for a server that may be unreachable.
fn refresh_export_label(window: &AppWindow, settings: &Rc<settings_ui::SettingsController>) {
let stored = settings.snapshot();
let label = match stored.export.target {
dr_types::ExportTarget::Device => "Export".to_string(),
dr_types::ExportTarget::Remote => "Export to Nextcloud".to_string(),
};
window.set_export_label(label.into());
let remote = stored.export.target == dr_types::ExportTarget::Remote;
window.set_export_label(
if remote {
"Export to Nextcloud"
} else {
"Export"
}
.into(),
);
// The grid's button says the same thing about a selection, but it composes
// its own label around a count — so it is given the fact rather than the
// sentence (FR-EXP-7).
window.set_export_to_server(remote);
}
/// Push current parameter values back to the interface.
@@ -1416,33 +1403,125 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
//
// Generic by construction: they carry indices into the capability list,
// so adding an operation needs no change here (FR-DEV-3c).
// --- export ---------------------------------------------------------
// --- export (FR-EXP-6, FR-EXP-7, FR-EXP-9) ---------------------------
//
// Renders its own frame at full resolution rather than encoding what is
// on screen: the display render is deliberately viewport-sized
// (FR-DSP-1), and exporting that would hand the user a soft, screen-sized
// file with no indication anything had been lost (FR-EXP-9).
// Two buttons, one worker. The develop view exports the image on screen and
// the grid exports its selection; the only difference between them is who
// renders the frames, so both hand a `Vec<Source>` to the same batch and
// both report through the same activity row.
//
// A frame is always rendered at full resolution rather than taken from the
// canvas: the display render is deliberately viewport-sized (FR-DSP-1), and
// exporting that would hand the user a soft, screen-sized file with no
// indication anything had been lost (FR-EXP-9).
{
let weak = window.as_weak();
let session = session.clone();
let settings = settings.clone();
let library = library.clone();
window.on_export_image(move || {
let Some(w) = weak.upgrade() else { return };
w.set_export_busy(true);
// Pushed before the work rather than after: the render blocks the
// UI thread, so a label set afterwards would never be drawn in
// the "Exporting…" state at all.
w.set_export_status("Rendering…".into());
// Replaced at each start, so the button always cancels the run it is
// sitting on and a stale token cancels nothing.
let cancel: Rc<RefCell<export::Cancel>> = Rc::new(RefCell::new(export::Cancel::default()));
// Holds the drain. Assigning a new timer drops the previous one, which
// is what takes a superseded batch's row out of the register.
let drain: Rc<RefCell<Option<slint::Timer>>> = Rc::new(RefCell::new(None));
let result = export_now(&w, &session, &settings, &library);
match result {
Ok(placed) => w.set_export_status(placed.describe().into()),
Err(e) => w.set_export_status(format!("Export failed: {e}").into()),
}
w.set_export_busy(false);
refresh_export_label(&w, &settings);
});
let start = {
let settings = settings.clone();
let library = library.clone();
let activity = activity.clone();
let gpu = gpu.clone();
let cancel = cancel.clone();
let drain = drain.clone();
Rc::new(
move |window: &AppWindow, sources: Vec<export::Source>, to: export::Reporting| {
let total = sources.len();
let request = batch_request(sources, &settings, &library, gpu.as_ref());
let token = export::Cancel::default();
*cancel.borrow_mut() = token.clone();
let rx = export::spawn_batch(request, token);
export::drain_batch(window.as_weak(), &activity, &drain, rx, total, to);
},
)
};
{
let weak = window.as_weak();
let session = session.clone();
let settings = settings.clone();
let start = start.clone();
window.on_export_image(move || {
let Some(w) = weak.upgrade() else { return };
// One batch at a time, and the grid's counts as one. The two
// buttons share a worker slot, so starting a second run would
// drop the first's drain — leaving a batch still writing files
// with no progress row and a button that never comes back.
if w.get_export_busy() || w.get_library_exporting() {
w.set_export_status("An export is already running".into());
return;
}
w.set_export_busy(true);
// Set before the render rather than after: the render is the
// one part still on this thread, so a label written afterwards
// would never be drawn in the "Rendering…" state at all.
w.set_export_status("Rendering…".into());
match render_open_frame(&w, &session) {
Ok(source) => start(&w, vec![source], export::Reporting::Develop),
Err(e) => {
w.set_export_status(format!("Export failed: {e}").into());
w.set_export_busy(false);
}
}
refresh_export_label(&w, &settings);
});
}
// TRACES: FR-EXP-7
// The grid's selection. Nothing is rendered here — the worker fetches,
// decodes and renders each photograph itself, so this returns to the
// event loop immediately and a batch of three hundred is a progress bar
// rather than a frozen window.
{
let weak = window.as_weak();
let library = library.clone();
let collections = collections.clone();
let start = start.clone();
window.on_library_export_selection(move || {
let Some(w) = weak.upgrade() else { return };
// See the develop button above for why one run excludes the
// other. The button is a cancel by then, so this only catches
// a batch started from develop and left running.
if w.get_library_exporting() || w.get_export_busy() {
w.set_library_status("An export is already running".into());
return;
}
let sources = library.export_sources(&collections.selected());
if sources.is_empty() {
// Said out loud rather than ignored, matching what a paste
// or a judgement keystroke does with an empty selection.
w.set_library_status("Select an image first".into());
return;
}
w.set_library_exporting(true);
w.set_library_status(format!("Exporting {} images…", sources.len()).into());
start(&w, sources, export::Reporting::Library);
});
}
// TRACES: NFR-ARCH-3
{
let weak = window.as_weak();
window.on_library_cancel_export(move || {
cancel.borrow().cancel();
if let Some(w) = weak.upgrade() {
// The worker stops at the next point it is safe to — which
// may be a frame away — so the button says "asked for" and
// not "done". The drain writes the real answer.
w.set_library_status("Cancelling the export…".into());
}
});
}
}
{