Say a photograph is downloading, and how far, instead of failing

The develop view reported a remote original on its way through the
error message, so it read "Could not load image" over "Downloading…".
It did so on every step along the roll, including a cached frame that
was ready within a tick, so each step flashed the error.

Waiting is now its own state. On the step, the grid's thumbnail of the
photograph stands in at once. Only when a transfer is really on the
wire does it dim under "Not on this device yet", with a line like
"Downloading — 12.4 of 38.0 MB" and a progress bar.

The bytes come from a new RemoteBackend::get_reporting. The Nextcloud
backend overrides it to read the body chunk by chunk; the default
reports once at the end. Progress is kept in the in-flight registry by
path, because a step usually lands on a frame the prefetcher is already
fetching. The catalog's file length stands in when the server sends no
Content-Length.
This commit is contained in:
2026-09-26 11:02:11 -04:00
parent 3b97195b37
commit 4bec01eaf1
9 changed files with 380 additions and 63 deletions
+44
View File
@@ -415,6 +415,31 @@ pub fn describe_bytes(bytes: u64) -> String {
}
}
/// TRACES: FR-NC-6a
/// A download someone is watching, as the line under its bar and how full the
/// bar is: `received` of `total` bytes, and a fraction in 0..1 — or below
/// zero when there is no total, which the bar draws as indeterminate rather
/// than as a position it would have to invent.
pub fn describe_download(received: u64, total: Option<u64>) -> (String, f32) {
match total.filter(|&t| t > 0) {
// Nothing yet: the size alone says what the wait is for, where
// "0 kB of 38.0 MB" would read as a transfer that has stalled.
Some(t) if received == 0 => (format!("Downloading {}", describe_bytes(t)), 0.0),
// A file that grew since the scan measured it can overrun the
// catalog's length; the bar stops full rather than past its end.
Some(t) => (
format!(
"Downloading — {} of {}",
describe_bytes(received),
describe_bytes(t.max(received))
),
(received as f64 / t as f64).min(1.0) as f32,
),
None if received > 0 => (format!("Downloading — {}", describe_bytes(received)), -1.0),
None => ("Downloading…".to_string(), -1.0),
}
}
/// A running job, held by whatever is reporting on it.
///
/// Every method is idempotent and every one is a no-op once the job has
@@ -498,6 +523,25 @@ impl Drop for Activity {
mod tests {
use super::*;
#[test]
fn a_download_reads_as_what_it_knows() {
let mb = 1024 * 1024;
assert_eq!(describe_download(0, None), ("Downloading…".to_string(), -1.0));
assert_eq!(
describe_download(0, Some(38 * mb)),
("Downloading 38.0 MB".to_string(), 0.0)
);
let (text, fraction) = describe_download(19 * mb, Some(38 * mb));
assert_eq!(text, "Downloading — 19.0 MB of 38.0 MB");
assert_eq!(fraction, 0.5);
let (text, fraction) = describe_download(12 * mb, None);
assert_eq!(text, "Downloading — 12.0 MB");
assert!(fraction < 0.0, "no total, no position");
let (text, fraction) = describe_download(40 * mb, Some(38 * mb));
assert_eq!(text, "Downloading — 40.0 MB of 40.0 MB");
assert_eq!(fraction, 1.0, "an overrun stops full");
}
#[test]
fn a_running_job_makes_the_bar_busy() {
let log = ActivityLog::new();
+40 -2
View File
@@ -411,6 +411,12 @@ fn is_supported(p: &Path) -> bool {
/// rect chosen for the first — or, since local masking became a mode, would
/// open the next photograph with a mask stack it does not have.
fn reset_view_state(window: &AppWindow) {
// Whatever the last open was waiting for, this one is not — until the
// remote path says otherwise.
window.set_load_pending(false);
window.set_load_waiting("".into());
window.set_load_fraction(-1.0);
window.set_has_load_preview(false);
window.global::<Develop>().set_view_mode(ViewMode::Photo);
window.set_zoom(1.0);
window.set_zoomed(false);
@@ -2769,7 +2775,17 @@ fn wire_remote_open(
log::info!("fetching {path} for develop");
w.set_load_error("Downloading…".into());
// TRACES: FR-NC-6a
// The grid's thumbnail stands in from this moment, so the step
// lands on this photograph rather than on the last one's pixels
// or an empty frame. Whether to say anything about a download is
// decided below, once one is actually running.
let (preview, size) = library.preview_for_path(&w, &path);
if let Some(p) = preview {
w.set_load_preview(p);
w.set_has_load_preview(true);
}
w.set_load_pending(true);
let rx = library::spawn_full_fetch(conn, path.clone(), cache);
@@ -2803,7 +2819,26 @@ fn wire_remote_open(
slint::TimerMode::Repeated,
std::time::Duration::from_millis(50),
move || {
let Ok(got) = rx.try_recv() else { return };
let Ok(got) = rx.try_recv() else {
// TRACES: FR-NC-6a
// Still coming. Said only once the original is on the
// wire — a read from the cache lands before there is
// a transfer to find, and so never flashes a headline.
// Read by path, because the transfer is as often the
// prefetcher's as this open's own.
if current.get() == mine {
if let (Some(w), Some((received, declared))) =
(weak.upgrade(), library::transfer_progress(&path))
{
let (text, fraction) =
activity::describe_download(received, declared.or(size));
w.set_load_waiting(text.as_str().into());
w.set_load_fraction(fraction);
job.detail(text);
}
}
return;
};
// Landed — this timer has done its job.
held.stop();
let Some(w) = weak.upgrade() else { return };
@@ -2819,6 +2854,9 @@ fn wire_remote_open(
log::debug!("{name}: landed after the view moved on");
return;
}
w.set_load_pending(false);
w.set_load_waiting("".into());
w.set_has_load_preview(false);
let bytes = match got {
Ok(b) => b,
+67 -5
View File
@@ -588,9 +588,31 @@ pub fn spawn_full_fetch(
/// because that is the one name every caller has.
static IN_FLIGHT: std::sync::LazyLock<InFlight> = std::sync::LazyLock::new(InFlight::default);
/// TRACES: FR-NC-6a
/// How far one original's transfer has got: bytes received, and the length
/// the server declared (zero until it has, or if it never does).
///
/// Held in the registry beside the claim rather than handed to the caller,
/// because the caller watching is often not the one downloading: a step along
/// the roll usually lands on a frame the [`Prefetcher`] is already fetching,
/// and the click waits on that transfer instead of starting its own.
#[derive(Default)]
pub(super) struct Transfer {
received: std::sync::atomic::AtomicU64,
declared: std::sync::atomic::AtomicU64,
}
/// TRACES: FR-NC-6a
/// Bytes received so far and bytes expected, for an original being fetched
/// right now by anyone. `None` when nothing is fetching `path` — it is in the
/// cache, or the transfer has not reached the network yet, or it has ended.
pub fn transfer_progress(path: &str) -> Option<(u64, Option<u64>)> {
IN_FLIGHT.progress(path)
}
#[derive(Default)]
pub(super) struct InFlight {
busy: std::sync::Mutex<std::collections::HashSet<String>>,
busy: std::sync::Mutex<std::collections::HashMap<String, std::sync::Arc<Transfer>>>,
freed: std::sync::Condvar,
/// Threads parked in [`InFlight::claim`], counted under the lock so a
/// test can release the holder only once a waiter is really waiting.
@@ -607,21 +629,32 @@ impl InFlight {
/// stored what the caller was about to download.
fn claim(&self, path: &str) -> Option<InFlightGuard<'_>> {
let mut busy = self.busy.lock().unwrap_or_else(|e| e.into_inner());
if busy.insert(path.to_string()) {
if !busy.contains_key(path) {
let transfer = std::sync::Arc::new(Transfer::default());
busy.insert(path.to_string(), transfer.clone());
return Some(InFlightGuard {
of: self,
path: path.to_string(),
transfer,
});
}
#[cfg(test)]
self.waiting
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
while busy.contains(path) {
while busy.contains_key(path) {
busy = self.freed.wait(busy).unwrap_or_else(|e| e.into_inner());
}
None
}
fn progress(&self, path: &str) -> Option<(u64, Option<u64>)> {
use std::sync::atomic::Ordering::Relaxed;
let busy = self.busy.lock().unwrap_or_else(|e| e.into_inner());
let t = busy.get(path)?;
let declared = t.declared.load(Relaxed);
Some((t.received.load(Relaxed), (declared > 0).then_some(declared)))
}
fn release(&self, path: &str) {
self.busy
.lock()
@@ -636,6 +669,7 @@ impl InFlight {
pub(super) struct InFlightGuard<'a> {
of: &'a InFlight,
path: String,
transfer: std::sync::Arc<Transfer>,
}
impl Drop for InFlightGuard<'_> {
@@ -678,7 +712,7 @@ pub(super) fn fetch_original(
// Miss, claim, and if the claim had to wait, look again: the thread that
// held the path has finished with it, and what it fetched is on disk.
let _claim = loop {
let claim = loop {
if let Some(bytes) = from_cache() {
return Ok(bytes);
}
@@ -693,7 +727,14 @@ pub(super) fn fetch_original(
let backend = crate::remote::connect(&conn).map_err(FetchFailure::local)?;
let id = RemoteId::Path(RemotePath::new(path));
let bytes = backend.get(&id, None).await?;
let transfer = &claim.transfer;
let bytes = backend
.get_reporting(&id, &|received, declared| {
use std::sync::atomic::Ordering::Relaxed;
transfer.received.store(received, Relaxed);
transfer.declared.store(declared.unwrap_or(0), Relaxed);
})
.await?;
// Store before returning, so the bytes are on disk by the time the
// image is on screen. Doing it after would leave a window where
@@ -936,6 +977,27 @@ mod tests {
);
}
/// Whoever is watching a path reads the holder's progress through the
/// registry, and loses it when the transfer ends — a step onto a frame
/// the prefetcher is fetching shows that transfer's bar.
#[test]
fn progress_is_readable_by_path_while_claimed() {
use std::sync::atomic::Ordering::Relaxed;
let registry = InFlight::default();
assert_eq!(registry.progress("a.CR2"), None);
let claim = registry.claim("a.CR2").unwrap();
assert_eq!(registry.progress("a.CR2"), Some((0, None)));
claim.transfer.received.store(1024, Relaxed);
claim.transfer.declared.store(4096, Relaxed);
assert_eq!(registry.progress("a.CR2"), Some((1024, Some(4096))));
assert_eq!(registry.progress("b.CR2"), None);
drop(claim);
assert_eq!(registry.progress("a.CR2"), None);
}
/// Different photographs never wait on each other.
#[test]
fn distinct_paths_are_claimed_independently() {
+28
View File
@@ -614,6 +614,34 @@ impl LibraryController {
.map(|id| dr_types::ImageId(*id as u64))
}
/// TRACES: FR-NC-6a
/// What the grid already holds for `path`: the thumbnail its cell is
/// drawing, if it has one yet, and the file's length as the scan recorded
/// it.
///
/// For the develop view while the original comes down. The thumbnail is
/// the one already decoded for the grid — no store read, no decode — and
/// the length gives the progress bar a denominator when the server sends
/// none of its own.
pub fn preview_for_path(
&self,
window: &crate::AppWindow,
path: &str,
) -> (Option<slint::Image>, Option<u64>) {
use slint::{ComponentHandle as _, Model as _};
let Some(row) = self.paths.borrow().iter().position(|p| p == path) else {
return (None, None);
};
let size = self.sizes.borrow().get(row).copied().filter(|&s| s > 0);
let thumbnail = window
.global::<crate::Library>()
.get_library_cells()
.row_data(row)
.filter(|c| c.has_thumb)
.map(|c| c.thumbnail);
(thumbnail, size)
}
/// TRACES: FR-NC-6a | FR-UI-4
/// The photographs within `depth` of `path` on the roll, closest first
/// and working outwards: next, previous, next-but-one, previous-but-one…