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

Merge the batch export, and settle the seven-branch merge

Browse Source 
Resolves the last of the parallel work. Two conflicts worth recording,
because both were semantic rather than textual:

`render_for_export` gained a colour space on master while the batch branch
was rewriting the single-image export path around it. Kept both: the batch
request supersedes the synchronous path, and the space still has to be chosen
at render time because the conversion happens in the shader before the clip to
0..1. `render_open_frame` takes it as an argument rather than reaching for a
controller it does not hold.

The map-wait moved into `readback::await_mapping` on one branch while another
was editing the constant it used, so `READBACK_POLL_LIMIT` survived the merge
with no callers. Removed rather than left for clippy to find later.

1164 tests pass, clippy clean, fmt clean. Traceability 53.0% -> 54.3%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-17 10:11:44 +02:00
co-authored by Claude Opus 5
parent cfff6a3302 7d3c8c521f
commit a8b28136a6
8 changed files with 1447 additions and 187 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/lib.rs
+219 -137
View File
@@ -122,48 +122,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 {
@@ -203,6 +167,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();
@@ -305,93 +314,66 @@ 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> {
space: dr_types::ColourSpace,
) -> 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(stored.export.colour_space)?;
// 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,
);
// The colour space is chosen at render time because the conversion happens
// in the shader, before the clip to 0..1 — see `render_for_export`.
let frame = session.render_for_export(space)?;
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.active_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.active_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.
@@ -401,11 +383,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.
@@ -1522,33 +1512,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, settings.snapshot().export.colour_space) {
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());
}
});
}
}
{