Compare commits

...
4 Commits
Author SHA1 Message Date
dtourolle d5c93ae795 Step to a library photograph without flashing frames in between
Benchmarks / Frame budget (on demand) (push) Canceled after 0s
Benchmarks / CPU and I/O (per commit) (push) Canceled after 5s
Traceability / Requirement traces (push) Canceled after 0s
Build and test / Desktop (Linux) (push) Successful in 1h21m7s
Build and test / Layer separation (push) Successful in 46s
🐳 Android image / Build and push (push) Successful in 4s
Build and test / android-image (push) Successful in 5s
🐳 Windows image / Build and push (push) Successful in 1s
Build and test / windows-image (push) Successful in 2s
Build and test / Android (aarch64) (push) Successful in 47m42s
Build and test / Windows (x86_64, cross) (push) Successful in 54m42s
Build and test / Publish the release (push) Successful in 52s
A step along the roll showed up to four pictures: the grid thumbnail,
the previous photograph again (the thumbnail was dropped when the bytes
landed, while the canvas still held the last texture through the decode
and first render), the new one at its defaults when a cached original
beat the sidecar, and then its edit.

The thumbnail now stays up until render_now draws the new photograph's
first frame, and that first frame waits up to 400 ms for the stored edit
before drawing at the defaults. A later arrival still redraws.
2026-10-02 20:46:05 -04:00
dtourolle 379dd1afcc Keep a late sidecar off the next photograph
A stored edit that arrived after the view had stepped on was applied to
whatever session was open by then — the next photograph's. The wait now
stops once the open it belongs to is no longer the current one.
2026-10-02 20:44:43 -04:00
dtourolle 825c5af20a Release 0.19.4
Benchmarks / Frame budget (on demand) (push) Canceled after 0s
Benchmarks / CPU and I/O (per commit) (push) Canceled after 5m49s
Traceability / Requirement traces (push) Canceled after 0s
Build and test / android-image (push) Canceled after 0s
🐳 Android image / Build and push (push) Canceled after 0s
Build and test / Android (aarch64) (push) Canceled after 0s
Build and test / windows-image (push) Canceled after 0s
🐳 Windows image / Build and push (push) Canceled after 0s
Build and test / Windows (x86_64, cross) (push) Canceled after 0s
Build and test / Layer separation (push) Canceled after 0s
Build and test / Publish the release (push) Canceled after 0s
Build and test / Desktop (Linux) (push) Canceled after 17s
2026-10-02 20:40:01 -04:00
dtourolle 23a2f13b46 Apply the collision policy to exports bound for the server
A queued export could not see the server, so its name check always
answered "free" and the upload PUT over whatever was there: Increment
and Skip behaved as Overwrite on Nextcloud, and two exports of the same
name queued before either uploaded landed on one file.

The batch now names around what the album records of earlier exports
and what the outbox already holds for that folder. The outbox record
carries the policy, and the drain lists each destination folder once
and applies it against what the server holds: Increment steps past a
taken name and re-points the album's row, Skip drops the entry. A
record without a policy (older builds, a merge's composite) is sent as
named, as before. The album is recorded before the drain starts so a
rename has a row to move.
2026-10-02 19:40:27 -04:00
10 changed files with 652 additions and 118 deletions
Generated
+25 -25
View File
@@ -1265,7 +1265,7 @@ checksum = "f27ae1dd37df86211c42e150270f82743308803d90a6f6e6651cd730d5e1732f"
[[package]] [[package]]
name = "darkroom-android" name = "darkroom-android"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"android_logger", "android_logger",
"dr-plat", "dr-plat",
@@ -1278,7 +1278,7 @@ dependencies = [
[[package]] [[package]]
name = "darkroom-desktop" name = "darkroom-desktop"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"dr-plat", "dr-plat",
@@ -1454,7 +1454,7 @@ checksum = "d8b14ccef22fc6f5a8f4d7d768562a182c04ce9a3b3157b91390b52ddfdf1a76"
[[package]] [[package]]
name = "dr-bench" name = "dr-bench"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"dr-catalog", "dr-catalog",
@@ -1471,7 +1471,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-catalog" name = "dr-catalog"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-face", "dr-face",
"dr-plat", "dr-plat",
@@ -1486,7 +1486,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-decode" name = "dr-decode"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-types", "dr-types",
"env_logger", "env_logger",
@@ -1500,7 +1500,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-export" name = "dr-export"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-decode", "dr-decode",
"dr-gpu", "dr-gpu",
@@ -1519,7 +1519,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-face" name = "dr-face"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-inference-engine", "dr-inference-engine",
"env_logger", "env_logger",
@@ -1532,7 +1532,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-film" name = "dr-film"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"log", "log",
"serde", "serde",
@@ -1541,7 +1541,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-gpu" name = "dr-gpu"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"bytemuck", "bytemuck",
"dr-decode", "dr-decode",
@@ -1559,7 +1559,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-inference-engine" name = "dr-inference-engine"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"env_logger", "env_logger",
"libloading", "libloading",
@@ -1574,7 +1574,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-ingest" name = "dr-ingest"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-plat", "dr-plat",
"dr-types", "dr-types",
@@ -1586,7 +1586,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-lens" name = "dr-lens"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"lensfun", "lensfun",
"log", "log",
@@ -1594,7 +1594,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-pano" name = "dr-pano"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-decode", "dr-decode",
"dr-inference-engine", "dr-inference-engine",
@@ -1608,7 +1608,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-pipeline" name = "dr-pipeline"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-types", "dr-types",
"log", "log",
@@ -1617,7 +1617,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-plat" name = "dr-plat"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"android-native-keyring-store", "android-native-keyring-store",
"dr-types", "dr-types",
@@ -1633,7 +1633,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-preset-xmp" name = "dr-preset-xmp"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-pipeline", "dr-pipeline",
"log", "log",
@@ -1643,7 +1643,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-segment" name = "dr-segment"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-inference-engine", "dr-inference-engine",
"env_logger", "env_logger",
@@ -1656,7 +1656,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-sync" name = "dr-sync"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"async-trait", "async-trait",
"dr-plat", "dr-plat",
@@ -1670,7 +1670,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-sync-folder" name = "dr-sync-folder"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"async-trait", "async-trait",
"dr-sync", "dr-sync",
@@ -1682,7 +1682,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-sync-nextcloud" name = "dr-sync-nextcloud"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"async-trait", "async-trait",
"dr-decode", "dr-decode",
@@ -1704,7 +1704,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-thumbs" name = "dr-thumbs"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-types", "dr-types",
"jpeg-encoder", "jpeg-encoder",
@@ -1716,7 +1716,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-types" name = "dr-types"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"serde", "serde",
"serde_json", "serde_json",
@@ -1725,7 +1725,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-ui" name = "dr-ui"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"async-trait", "async-trait",
@@ -1773,7 +1773,7 @@ dependencies = [
[[package]] [[package]]
name = "dr-xmp" name = "dr-xmp"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"dr-types", "dr-types",
"log", "log",
@@ -7107,7 +7107,7 @@ checksum = "8df9b6e13f2d32c91b9bd719c00d1958837bc7dec474d94952798cc8e69eeec3"
[[package]] [[package]]
name = "traceability" name = "traceability"
version = "0.19.3" version = "0.19.4"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"proc-macro2", "proc-macro2",
+1 -1
View File
@@ -32,7 +32,7 @@ members = [
exclude = ["third_party"] exclude = ["third_party"]
[workspace.package] [workspace.package]
version = "0.19.3" version = "0.19.4"
edition = "2021" edition = "2021"
rust-version = "1.92" rust-version = "1.92"
license = "GPL-3.0-or-later" license = "GPL-3.0-or-later"
+1 -1
View File
@@ -201,7 +201,7 @@ controls, its place in the chain and its tests.
## Where it stands ## Where it stands
**0.19.3**, thirty-two tagged releases in. 193 numbered requirements in **0.19.4**, thirty-three tagged releases in. 193 numbered requirements in
scope, 85% of them claimed by code and [traced to it](docs/dev/traceability.md); scope, 85% of them claimed by code and [traced to it](docs/dev/traceability.md);
the rest are written down rather than merely absent. the rest are written down rather than merely absent.
+64
View File
@@ -306,6 +306,48 @@ pub fn record_exports(
Ok(()) Ok(())
} }
/// Every file name an album records, for an export choosing a name to know
/// what it would land on.
///
/// A server album cannot be asked while the export is queued offline, and
/// the names this app put there are the ones a second export of the same
/// photographs will collide with. One read of the album's rows, not one per
/// candidate name.
pub fn file_names(
conn: &Connection,
id: AlbumId,
) -> Result<std::collections::HashSet<String>, CatalogError> {
ensure_tables(conn)?;
let mut stmt = conn.prepare("SELECT file_name FROM album_exports WHERE album_id = ?1")?;
let rows = stmt
.query_map([id.0 as i64], |r| r.get(0))?
.collect::<Result<_, _>>()?;
Ok(rows)
}
/// A file the upload had to give another name: the server held one by the
/// name the export recorded, put there by something this catalog never
/// saw. The album row follows the file to the name it was given.
///
/// By the album's server folder, because that is all an outbox entry knows.
/// `folder` is spelled as [`Place::Server`] spells it, without slashes at
/// either end.
pub fn rename_export(
conn: &Connection,
folder: &str,
from: &str,
to: &str,
) -> Result<(), CatalogError> {
ensure_tables(conn)?;
conn.execute(
"UPDATE OR REPLACE album_exports SET file_name = ?3
WHERE file_name = ?2
AND album_id IN (SELECT id FROM albums WHERE server_path = ?1 AND deleted = 0)",
rusqlite::params![folder.trim_matches('/'), from, to],
)?;
Ok(())
}
/// The photographs behind an album's files, most recently exported first — /// The photographs behind an album's files, most recently exported first —
/// what the grid shows when the album is opened. /// what the grid shows when the album is opened.
pub fn sources(conn: &Connection, id: AlbumId) -> Result<Vec<ImageId>, CatalogError> { pub fn sources(conn: &Connection, id: AlbumId) -> Result<Vec<ImageId>, CatalogError> {
@@ -463,6 +505,28 @@ mod tests {
assert_eq!(sources(conn, album).unwrap(), vec![b]); assert_eq!(sources(conn, album).unwrap(), vec![b]);
} }
#[test]
fn a_renamed_upload_moves_the_row_of_the_server_album_only() {
let cat = catalog();
let conn = cat.connection();
let web = create(conn, "Web", &Place::Server("Albums/Web".into())).unwrap();
let other = create(conn, "Other", &Place::Server("Albums/Other".into())).unwrap();
let a = image(conn, "a.cr3");
record_exports(conn, web, &[(a, "a.jpg".into())]).unwrap();
record_exports(conn, other, &[(a, "a.jpg".into())]).unwrap();
rename_export(conn, "/Albums/Web", "a.jpg", "a-1.jpg").unwrap();
assert_eq!(
file_names(conn, web).unwrap(),
["a-1.jpg".to_string()].into()
);
assert_eq!(
file_names(conn, other).unwrap(),
["a.jpg".to_string()].into()
);
}
#[test] #[test]
fn moving_to_the_server_forgets_the_local_folder() { fn moving_to_the_server_forgets_the_local_folder() {
let cat = catalog(); let cat = catalog();
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -4,7 +4,7 @@
# makes `makepkg -si` in this directory install what you are actually working # makes `makepkg -si` in this directory install what you are actually working
# on. Swap `source` for a tagged tarball when there is something to release. # on. Swap `source` for a tagged tarball when there is something to release.
pkgname=darkroom pkgname=darkroom
pkgver=0.19.3 pkgver=0.19.4
# Back to 1 with the version: a new pkgver is a new archive name, so there is # Back to 1 with the version: a new pkgver is a new archive name, so there is
# nothing for makepkg to reuse and nothing for a release number to disambiguate. # nothing for makepkg to reuse and nothing for a release number to disambiguate.
pkgrel=1 pkgrel=1
+14
View File
@@ -121,6 +121,20 @@ impl AlbumsController {
} }
} }
/// The file names an album already records, for a batch bound for its
/// server folder to name around. Empty where the catalog cannot say.
pub fn file_names(&self, album: AlbumId) -> std::collections::HashSet<String> {
let catalog = self.library.catalog();
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else {
return Default::default();
};
albums::file_names(cat.connection(), album).unwrap_or_else(|e| {
log::warn!("reading the files of album {}: {e}", album.0);
Default::default()
})
}
/// Record what a batch wrote into an album, and redraw what counts it. /// Record what a batch wrote into an album, and redraw what counts it.
pub fn record(&self, window: &AppWindow, album: AlbumId, files: Vec<(ImageId, String)>) { pub fn record(&self, window: &AppWindow, album: AlbumId, files: Vec<(ImageId, String)>) {
{ {
+9 -2
View File
@@ -293,6 +293,11 @@ fn wire_export(window: &AppWindow, w: &DevelopWiring) {
request.settings.target = target; request.settings.target = target;
request.settings.destination = folder; request.settings.destination = folder;
request.images = images; request.images = images;
if target == dr_types::ExportTarget::Remote {
if let Some(a) = albums.as_ref() {
request.remote_names = a.file_names(album);
}
}
let token = export::Cancel::default(); let token = export::Cancel::default();
*cancel.borrow_mut() = token.clone(); *cancel.borrow_mut() = token.clone();
@@ -315,12 +320,14 @@ fn wire_export(window: &AppWindow, w: &DevelopWiring) {
total, total,
to, to,
move |written| { move |written| {
drain_outbox(&library_for_drain, weak.clone());
// What the album now holds, and which photograph // What the album now holds, and which photograph
// each file came from. // each file came from. Before the drain starts: an
// upload that finds a name taken on the server
// renames the album's row, which must be there.
if let (Some(albums), Some(w)) = (albums.as_ref(), weak.upgrade()) { if let (Some(albums), Some(w)) = (albums.as_ref(), weak.upgrade()) {
albums.record(&w, album, written); albums.record(&w, album, written);
} }
drain_outbox(&library_for_drain, weak.clone());
}, },
); );
}, },
+422 -22
View File
@@ -56,7 +56,7 @@
//! allow (FR-EXP-8). //! allow (FR-EXP-8).
use crate::executors::{self, Executor}; use crate::executors::{self, Executor};
use std::collections::HashSet; use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::rc::Rc; use std::rc::Rc;
use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
@@ -67,7 +67,7 @@ use std::time::Duration;
use dr_export::{Encoded, NameContext}; use dr_export::{Encoded, NameContext};
use dr_sync::RemotePath; use dr_sync::RemotePath;
use dr_sync::{Account, Connection}; use dr_sync::{Account, Connection};
use dr_types::{ExportSettings, ExportTarget}; use dr_types::{CollisionPolicy, ExportSettings, ExportTarget};
use slint::ComponentHandle as _; use slint::ComponentHandle as _;
use crate::{AppWindow, Library}; use crate::{AppWindow, Library};
@@ -106,6 +106,13 @@ pub struct Pending {
pub account: bool, pub account: bool,
/// The filename to give it there. /// The filename to give it there.
pub name: String, pub name: String,
/// What to do if the server already holds `name`. Decided when the
/// export was made, applied when it uploads: the batch could not see
/// the server, so the name it chose is a request, not a fact. `None`
/// for a record written before the policy travelled with it, and for
/// a merge's composite, which the catalog has already recorded under
/// its name — both are sent as named.
pub collision: Option<CollisionPolicy>,
} }
impl Pending { impl Pending {
@@ -156,6 +163,7 @@ pub(crate) fn staged_remote_path(root: &str, remote_dir: &str, name: &str) -> Re
remote_dir: remote_dir.to_string(), remote_dir: remote_dir.to_string(),
account: false, account: false,
name: name.to_string(), name: name.to_string(),
collision: None,
} }
.remote_path(root) .remote_path(root)
} }
@@ -222,6 +230,7 @@ pub fn place(
target: ExportTarget, target: ExportTarget,
destination: &str, destination: &str,
outbox: &Path, outbox: &Path,
collision: CollisionPolicy,
) -> Result<Placed, String> { ) -> Result<Placed, String> {
match target { match target {
ExportTarget::Device => { ExportTarget::Device => {
@@ -251,7 +260,7 @@ pub fn place(
Ok(Placed::Device(path)) Ok(Placed::Device(path))
} }
ExportTarget::Remote => { ExportTarget::Remote => {
let local = stage(encoded, destination, outbox)?; let local = stage(encoded, destination, outbox, collision)?;
Ok(Placed::Queued { Ok(Placed::Queued {
local, local,
remote_dir: destination.to_string(), remote_dir: destination.to_string(),
@@ -286,7 +295,12 @@ fn mime_for(name: &str) -> &'static str {
} }
/// Write bytes and their destination record into the outbox. /// Write bytes and their destination record into the outbox.
fn stage(encoded: &Encoded, remote_dir: &str, outbox: &Path) -> Result<PathBuf, String> { fn stage(
encoded: &Encoded,
remote_dir: &str,
outbox: &Path,
collision: CollisionPolicy,
) -> Result<PathBuf, String> {
std::fs::create_dir_all(outbox).map_err(|e| format!("{}: {e}", outbox.display()))?; std::fs::create_dir_all(outbox).map_err(|e| format!("{}: {e}", outbox.display()))?;
// The staged name is the export's name, deduplicated against the outbox // The staged name is the export's name, deduplicated against the outbox
@@ -323,11 +337,18 @@ fn stage(encoded: &Encoded, remote_dir: &str, outbox: &Path) -> Result<PathBuf,
// strings do not need a parser, and a format a human can repair by hand // strings do not need a parser, and a format a human can repair by hand
// is worth something for a queue holding the only copy of someone's work. // is worth something for a queue holding the only copy of someone's work.
// A folder spelled from `/` is an album's, relative to the account; it // A folder spelled from `/` is an album's, relative to the account; it
// is recorded as such on a third line — see `Pending::account`. // is recorded as such on a third line — see `Pending::account`. The
let text = match remote_dir.strip_prefix('/') { // fourth is the collision policy, which an older build stops before
Some(dir) => format!("{dir}\n{}\naccount\n", encoded.name), // reading and so uploads as named, as it always did.
None => format!("{remote_dir}\n{}\n", encoded.name), let (dir, base) = match remote_dir.strip_prefix('/') {
Some(dir) => (dir, "account"),
None => (remote_dir, "library"),
}; };
let text = format!(
"{dir}\n{}\n{base}\n{}\n",
encoded.name,
policy_word(collision)
);
std::fs::write(&record, text).map_err(|e| format!("{}: {e}", record.display()))?; std::fs::write(&record, text).map_err(|e| format!("{}: {e}", record.display()))?;
Ok(candidate) Ok(candidate)
@@ -442,6 +463,7 @@ pub fn pending(outbox: &Path) -> Vec<Pending> {
let remote_dir = lines.next().unwrap_or("").to_string(); let remote_dir = lines.next().unwrap_or("").to_string();
let name = lines.next().unwrap_or("").to_string(); let name = lines.next().unwrap_or("").to_string();
let account = lines.next() == Some("account"); let account = lines.next() == Some("account");
let collision = lines.next().and_then(policy_from_word);
if name.is_empty() { if name.is_empty() {
continue; continue;
} }
@@ -450,6 +472,7 @@ pub fn pending(outbox: &Path) -> Vec<Pending> {
remote_dir, remote_dir,
name, name,
account, account,
collision,
}); });
} }
// Stable order so a drain is reproducible and a stuck entry is obvious // Stable order so a drain is reproducible and a stuck entry is obvious
@@ -458,6 +481,24 @@ pub fn pending(outbox: &Path) -> Vec<Pending> {
out out
} }
/// A collision policy as an outbox record spells it.
fn policy_word(policy: CollisionPolicy) -> &'static str {
match policy {
CollisionPolicy::Overwrite => "overwrite",
CollisionPolicy::Skip => "skip",
CollisionPolicy::Increment => "increment",
}
}
fn policy_from_word(word: &str) -> Option<CollisionPolicy> {
match word {
"overwrite" => Some(CollisionPolicy::Overwrite),
"skip" => Some(CollisionPolicy::Skip),
"increment" => Some(CollisionPolicy::Increment),
_ => None,
}
}
/// How many exports are waiting. For the interface to show, and cheap enough /// How many exports are waiting. For the interface to show, and cheap enough
/// to call on a redraw. /// to call on a redraw.
pub fn pending_count(outbox: &Path) -> usize { pub fn pending_count(outbox: &Path) -> usize {
@@ -543,14 +584,23 @@ enum Sent {
Uploaded, Uploaded,
/// Its payload had gone; the record went with it. /// Its payload had gone; the record went with it.
Gone, Gone,
/// The server held its name and the policy was Skip.
Skipped,
} }
/// What each destination folder holds, as far as this drain knows: listed
/// once on the first entry bound for it, and added to as files land. One
/// request per folder per drain rather than one per file — a batch of
/// three hundred into one album is one listing.
type Listings = HashMap<String, HashSet<String>>;
/// Send one outbox entry, and clear it once it is on the server. /// Send one outbox entry, and clear it once it is on the server.
async fn send( async fn send(
backend: &dyn dr_sync::RemoteBackend, backend: &dyn dr_sync::RemoteBackend,
library: &LibraryFiles, library: &LibraryFiles,
root: &str, root: &str,
entry: &Pending, entry: &Pending,
listings: &mut Listings,
) -> Result<Sent, String> { ) -> Result<Sent, String> {
let Ok(bytes) = std::fs::read(&entry.local) else { let Ok(bytes) = std::fs::read(&entry.local) else {
// The payload vanished under us. Drop the record too; retrying // The payload vanished under us. Drop the record too; retrying
@@ -562,20 +612,85 @@ async fn send(
// The folder may not exist — this is the first export into it — and // The folder may not exist — this is the first export into it — and
// `create_dir` treats "already there" as success, so it is unconditional // `create_dir` treats "already there" as success, so it is unconditional
// rather than guarded by a check that would cost a request every time. // rather than guarded by a check that would cost a request every time.
let folder = entry.remote_folder(root);
backend backend
.create_dir(&entry.remote_folder(root)) .create_dir(&folder)
.await .await
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
// TRACES: FR-EXP-6
// The batch named this file without seeing the server, so the policy it
// was made under is applied here, against what the folder holds now.
// Overwrite, and a record that carries no policy, put it as named.
let names = match entry.collision {
Some(CollisionPolicy::Increment | CollisionPolicy::Skip) => {
let key = folder.as_str().to_string();
if !listings.contains_key(&key) {
let listed = backend
.list(&folder, None)
.await
.map_err(|e| e.to_string())?;
let held = listed.iter().map(|e| e.path.name().to_string()).collect();
listings.insert(key.clone(), held);
}
listings.get_mut(&key)
}
_ => None,
};
let mut renamed = None;
if let Some(held) = names.as_deref() {
if held.contains(&entry.name) {
if entry.collision == Some(CollisionPolicy::Skip) {
log::info!("upload: {} is already on the server; skipped", entry.name);
clear(entry);
return Ok(Sent::Skipped);
}
let free = step_past(&entry.name, &|n| held.contains(n))
.ok_or_else(|| format!("{}: ten thousand names taken", entry.name))?;
renamed = Some(Pending {
name: free,
..entry.clone()
});
}
}
let sent = renamed.as_ref().unwrap_or(entry);
backend backend
.put(&entry.remote_path(root), bytes, None) .put(&sent.remote_path(root), bytes, None)
.await .await
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
if let Some(held) = names {
held.insert(sent.name.clone());
}
if renamed.is_some() {
log::info!(
"upload: {} was taken on the server; sent as {}",
entry.name,
sent.name
);
rename_in_album(library, entry, &sent.name);
}
let thumbnails = take_thumbnails(&entry.local); let thumbnails = take_thumbnails(&entry.local);
register_upload(backend, library, root, entry, thumbnails).await; register_upload(backend, library, root, sent, thumbnails).await;
clear(entry); clear(entry);
Ok(Sent::Uploaded) Ok(Sent::Uploaded)
} }
/// TRACES: FR-EXP-10
/// Tell the album a file it recorded arrived under another name. Only an
/// album's entries are recorded by name; a library export is the scan's.
fn rename_in_album(library: &LibraryFiles, entry: &Pending, to: &str) {
if !entry.account {
return;
}
let result = dr_catalog::Catalog::open(&library.catalog).and_then(|catalog| {
dr_catalog::albums::rename_export(catalog.connection(), &entry.remote_dir, &entry.name, to)
});
if let Err(e) = result {
log::warn!("upload: recording {} as {to} in its album: {e}", entry.name);
}
}
/// TRACES: FR-MRG-6 /// TRACES: FR-MRG-6
/// Tell the catalog what the server made of a file it has just been given. /// Tell the catalog what the server made of a file it has just been given.
/// ///
@@ -703,6 +818,7 @@ pub fn spawn_upload(
let mut uploaded = 0; let mut uploaded = 0;
let mut landed = 0; let mut landed = 0;
let mut error = None; let mut error = None;
let mut listings = Listings::new();
for (i, entry) in queue.iter().enumerate() { for (i, entry) in queue.iter().enumerate() {
let _ = tx.send(UploadMessage::Status(format!( let _ = tx.send(UploadMessage::Status(format!(
@@ -711,8 +827,8 @@ pub fn spawn_upload(
i + 1 i + 1
))); )));
match send(&*backend, &library, &root, entry).await { match send(&*backend, &library, &root, entry, &mut listings).await {
Ok(Sent::Gone) => {} Ok(Sent::Gone | Sent::Skipped) => {}
Ok(Sent::Uploaded) => { Ok(Sent::Uploaded) => {
uploaded += 1; uploaded += 1;
if !entry.account { if !entry.account {
@@ -840,6 +956,13 @@ pub struct BatchRequest {
/// anything that has to be fetched. /// anything that has to be fetched.
pub conn: Option<Connection>, pub conn: Option<Connection>,
pub settings: ExportSettings, pub settings: ExportSettings,
/// TRACES: FR-EXP-6
/// Names a server destination is known to hold — what the album records
/// of earlier exports. A queued export cannot ask the server, so this is
/// what [`resolve_batch_name`] checks instead; the upload checks the
/// server itself for anything put there some other way. Unused for a
/// device export, which looks at the folder.
pub remote_names: HashSet<String>,
pub outbox: PathBuf, pub outbox: PathBuf,
pub sidecar_cache: PathBuf, pub sidecar_cache: PathBuf,
pub offline: bool, pub offline: bool,
@@ -933,6 +1056,13 @@ fn run(mut request: BatchRequest, cancel: &Cancel, tx: &Sender<BatchMessage>) {
let mut issued: HashSet<String> = HashSet::new(); let mut issued: HashSet<String> = HashSet::new();
let (mut exported, mut failed) = (0usize, 0usize); let (mut exported, mut failed) = (0usize, 0usize);
// What earlier batches queued for the same folder and has not uploaded
// yet is as much in the way as what is already there.
if request.settings.target == ExportTarget::Remote {
let names = queued_for(&request.outbox, &request.settings.destination);
request.remote_names.extend(names);
}
for (i, source) in sources.into_iter().enumerate() { for (i, source) in sources.into_iter().enumerate() {
if cancel.is_cancelled() { if cancel.is_cancelled() {
break; break;
@@ -974,6 +1104,18 @@ fn run(mut request: BatchRequest, cancel: &Cancel, tx: &Sender<BatchMessage>) {
}); });
} }
/// The names waiting in the outbox for `destination`, spelled as a batch
/// spells a remote folder: from `/` for an album, relative to the library
/// otherwise.
fn queued_for(outbox: &Path, destination: &str) -> impl Iterator<Item = String> {
let account = destination.starts_with('/');
let folder = destination.trim_matches('/').to_string();
pending(outbox)
.into_iter()
.filter(move |p| p.account == account && p.remote_dir.trim_matches('/') == folder)
.map(|p| p.name)
}
/// One image, start to finish. `None` where the run was cancelled part way. /// One image, start to finish. `None` where the run was cancelled part way.
fn export_one( fn export_one(
request: &BatchRequest, request: &BatchRequest,
@@ -1237,7 +1379,8 @@ fn place_frame(
preset: "", preset: "",
}; };
let name = resolve_batch_name(&request.settings, &ctx, issued).ok_or(ItemError::NameTaken)?; let name = resolve_batch_name(&request.settings, &request.remote_names, &ctx, issued)
.ok_or(ItemError::NameTaken)?;
let encoded = dr_export::export(frame, &request.settings, name, source)?; let encoded = dr_export::export(frame, &request.settings, name, source)?;
place( place(
@@ -1245,6 +1388,7 @@ fn place_frame(
request.settings.target, request.settings.target,
&request.settings.destination, &request.settings.destination,
&request.outbox, &request.outbox,
request.settings.collision,
) )
.map_err(ItemError::Place) .map_err(ItemError::Place)
} }
@@ -1263,6 +1407,7 @@ fn place_frame(
/// the folder. /// the folder.
fn resolve_batch_name( fn resolve_batch_name(
settings: &ExportSettings, settings: &ExportSettings,
remote_names: &HashSet<String>,
ctx: &NameContext<'_>, ctx: &NameContext<'_>,
issued: &mut HashSet<String>, issued: &mut HashSet<String>,
) -> Option<String> { ) -> Option<String> {
@@ -1275,8 +1420,9 @@ fn resolve_batch_name(
} }
dr_types::ExportTarget::Device => dir.join(name).exists(), dr_types::ExportTarget::Device => dir.join(name).exists(),
// A queued export cannot see the server, and may never be able to. // A queued export cannot see the server, and may never be able to.
// Names are kept apart in the outbox instead — see [`stage`]. // What the album records stands in for it here, and the upload
dr_types::ExportTarget::Remote => false, // checks the server itself — see [`send`].
dr_types::ExportTarget::Remote => remote_names.contains(name),
} }
}; };
@@ -1745,9 +1891,15 @@ mod tests {
let server = Assigning::default(); let server = Assigning::default();
assert_eq!(file_id_of(&s.library, s.image), None); assert_eq!(file_id_of(&s.library, s.image), None);
let sent = send(&server, &s.library, "PhotosRaw", &s.entry) let sent = send(
.await &server,
.unwrap(); &s.library,
"PhotosRaw",
&s.entry,
&mut Listings::new(),
)
.await
.unwrap();
assert_eq!(sent, Sent::Uploaded); assert_eq!(sent, Sent::Uploaded);
// The id the server assigned, on the row the merge wrote, and the // The id the server assigned, on the row the merge wrote, and the
@@ -1789,7 +1941,9 @@ mod tests {
std::fs::create_dir_all(library_dir.join("Alps")).unwrap(); std::fs::create_dir_all(library_dir.join("Alps")).unwrap();
let folder = dr_sync_folder::FolderBackend::new(&library_dir).unwrap(); let folder = dr_sync_folder::FolderBackend::new(&library_dir).unwrap();
send(&folder, &s.library, "", &s.entry).await.unwrap(); send(&folder, &s.library, "", &s.entry, &mut Listings::new())
.await
.unwrap();
assert_eq!( assert_eq!(
std::fs::read(library_dir.join("Alps/_MG_8320-pano.dng")).unwrap(), std::fs::read(library_dir.join("Alps/_MG_8320-pano.dng")).unwrap(),
@@ -1823,9 +1977,163 @@ mod tests {
.find(|p| p.name == "print.jpg") .find(|p| p.name == "print.jpg")
.unwrap(); .unwrap();
let server = Assigning::default(); let server = Assigning::default();
send(&server, &s.library, "PhotosRaw", &entry) send(
&server,
&s.library,
"PhotosRaw",
&entry,
&mut Listings::new(),
)
.await
.unwrap();
assert_eq!(server.lists.load(Ordering::SeqCst), 0);
let _ = std::fs::remove_dir_all(&s.dir);
}
/// An album export queued under `policy`, the album recording it, and a
/// server that already holds a file of that name.
async fn queued_over_a_taken_name(
tag: &str,
policy: CollisionPolicy,
) -> (Staged, Assigning, dr_catalog::albums::AlbumId) {
let s = staged(tag, "PhotosRaw");
let catalog = dr_catalog::Catalog::open(&s.library.catalog).unwrap();
let album = dr_catalog::albums::create(
catalog.connection(),
"Web",
&dr_catalog::albums::Place::Server("Shared/Web".into()),
)
.unwrap();
dr_catalog::albums::record_exports(
catalog.connection(),
album,
&[(dr_types::ImageId(s.image as u64), "a.jpg".into())],
)
.unwrap();
let server = Assigning::default();
use dr_sync::RemoteBackend as _;
server
.put(
&RemotePath::new("Shared/Web/a.jpg"),
b"theirs".to_vec(),
None,
)
.await .await
.unwrap(); .unwrap();
place(
&encoded("a.jpg", b"ours"),
ExportTarget::Remote,
"/Shared/Web",
&s.dir.join("outbox"),
policy,
)
.unwrap();
(s, server, album)
}
fn queued_named(s: &Staged, name: &str) -> Vec<Pending> {
pending(&s.dir.join("outbox"))
.into_iter()
.filter(|p| p.name == name)
.collect()
}
fn held(server: &Assigning, path: &str) -> Option<Vec<u8>> {
server
.files
.lock()
.unwrap()
.get(path)
.map(|(_, body)| body.clone())
}
#[tokio::test]
async fn an_increment_export_steps_past_a_name_the_server_holds() {
let (s, server, album) =
queued_over_a_taken_name("increment", CollisionPolicy::Increment).await;
// A second export of the same name, queued before either uploaded.
place(
&encoded("a.jpg", b"ours too"),
ExportTarget::Remote,
"/Shared/Web",
&s.dir.join("outbox"),
CollisionPolicy::Increment,
)
.unwrap();
let mut listings = Listings::new();
for entry in queued_named(&s, "a.jpg") {
let sent = send(&server, &s.library, "PhotosRaw", &entry, &mut listings)
.await
.unwrap();
assert_eq!(sent, Sent::Uploaded);
}
assert_eq!(held(&server, "Shared/Web/a.jpg").unwrap(), b"theirs");
let mut ours = vec![
held(&server, "Shared/Web/a-1.jpg").unwrap(),
held(&server, "Shared/Web/a-2.jpg").unwrap(),
];
ours.sort();
assert_eq!(ours, vec![b"ours".to_vec(), b"ours too".to_vec()]);
assert_eq!(
server.lists.load(Ordering::SeqCst),
1,
"one listing for the folder, however many files go into it"
);
// The album's row followed its file to the name it was given.
let catalog = dr_catalog::Catalog::open(&s.library.catalog).unwrap();
let names = dr_catalog::albums::file_names(catalog.connection(), album).unwrap();
assert!(!names.contains("a.jpg"), "{names:?}");
assert!(
queued_named(&s, "a.jpg").is_empty(),
"the outbox is cleared"
);
let _ = std::fs::remove_dir_all(&s.dir);
}
#[tokio::test]
async fn a_skip_export_leaves_a_name_the_server_holds() {
let (s, server, _) = queued_over_a_taken_name("skip", CollisionPolicy::Skip).await;
let entry = queued_named(&s, "a.jpg").pop().unwrap();
let sent = send(
&server,
&s.library,
"PhotosRaw",
&entry,
&mut Listings::new(),
)
.await
.unwrap();
assert_eq!(sent, Sent::Skipped);
assert_eq!(held(&server, "Shared/Web/a.jpg").unwrap(), b"theirs");
assert_eq!(server.files.lock().unwrap().len(), 1);
assert!(
queued_named(&s, "a.jpg").is_empty(),
"the outbox is cleared"
);
let _ = std::fs::remove_dir_all(&s.dir);
}
#[tokio::test]
async fn an_overwrite_export_replaces_a_name_the_server_holds() {
let (s, server, _) =
queued_over_a_taken_name("overwrite", CollisionPolicy::Overwrite).await;
let entry = queued_named(&s, "a.jpg").pop().unwrap();
send(
&server,
&s.library,
"PhotosRaw",
&entry,
&mut Listings::new(),
)
.await
.unwrap();
assert_eq!(held(&server, "Shared/Web/a.jpg").unwrap(), b"ours");
assert_eq!(server.lists.load(Ordering::SeqCst), 0); assert_eq!(server.lists.load(Ordering::SeqCst), 0);
let _ = std::fs::remove_dir_all(&s.dir); let _ = std::fs::remove_dir_all(&s.dir);
} }
@@ -1878,6 +2186,7 @@ mod tests {
ExportTarget::Device, ExportTarget::Device,
target.to_str().unwrap(), target.to_str().unwrap(),
&dir.join("outbox"), &dir.join("outbox"),
CollisionPolicy::Increment,
) )
.unwrap(); .unwrap();
@@ -1896,6 +2205,7 @@ mod tests {
ExportTarget::Device, ExportTarget::Device,
target.to_str().unwrap(), target.to_str().unwrap(),
&dir, &dir,
CollisionPolicy::Increment,
) )
.is_ok()); .is_ok());
assert!(target.join("a.jpg").exists()); assert!(target.join("a.jpg").exists());
@@ -1906,7 +2216,14 @@ mod tests {
// Rather than writing to the process's working directory, which is // Rather than writing to the process's working directory, which is
// wherever the app happened to be launched from. // wherever the app happened to be launched from.
let dir = tmp(); let dir = tmp();
let err = place(&encoded("a.jpg", b"x"), ExportTarget::Device, " ", &dir).unwrap_err(); let err = place(
&encoded("a.jpg", b"x"),
ExportTarget::Device,
" ",
&dir,
CollisionPolicy::Increment,
)
.unwrap_err();
// Says what to do about it: the destination is an album now. // Says what to do about it: the destination is an album now.
assert!(err.contains("album"), "unhelpful message: {err}"); assert!(err.contains("album"), "unhelpful message: {err}");
} }
@@ -1922,6 +2239,7 @@ mod tests {
ExportTarget::Remote, ExportTarget::Remote,
"Exports/2026", "Exports/2026",
&outbox, &outbox,
CollisionPolicy::Increment,
) )
.unwrap(); .unwrap();
@@ -1947,6 +2265,7 @@ mod tests {
ExportTarget::Remote, ExportTarget::Remote,
"Exports", "Exports",
&outbox, &outbox,
CollisionPolicy::Increment,
) )
.unwrap(); .unwrap();
@@ -1967,6 +2286,7 @@ mod tests {
ExportTarget::Remote, ExportTarget::Remote,
"E", "E",
&outbox, &outbox,
CollisionPolicy::Increment,
) )
.unwrap(); .unwrap();
place( place(
@@ -1974,6 +2294,7 @@ mod tests {
ExportTarget::Remote, ExportTarget::Remote,
"E", "E",
&outbox, &outbox,
CollisionPolicy::Increment,
) )
.unwrap(); .unwrap();
@@ -1985,6 +2306,79 @@ mod tests {
assert_ne!(queue[0].local, queue[1].local); assert_ne!(queue[0].local, queue[1].local);
} }
#[test]
fn a_remote_batch_names_around_the_album_and_the_outbox() {
let dir = tmp();
let outbox = dir.join("outbox");
// Queued by an earlier batch, not uploaded yet.
place(
&encoded("IMG_0001-1.png", b"x"),
ExportTarget::Remote,
"/Shared/Web",
&outbox,
CollisionPolicy::Increment,
)
.unwrap();
// Elsewhere, so in nobody's way.
place(
&encoded("IMG_0001-2.png", b"x"),
ExportTarget::Remote,
"Shared/Web",
&outbox,
CollisionPolicy::Increment,
)
.unwrap();
let settings = ExportSettings {
format: dr_types::ExportFormat::Png,
target: ExportTarget::Remote,
destination: "/Shared/Web".into(),
collision: CollisionPolicy::Increment,
..Default::default()
};
// What the album records of an earlier export.
let mut known: HashSet<String> = ["IMG_0001.png".to_string()].into();
known.extend(queued_for(&outbox, &settings.destination));
let name = resolve_batch_name(
&settings,
&known,
&NameContext {
source_stem: "IMG_0001",
sequence: 1,
..Default::default()
},
&mut HashSet::new(),
);
assert_eq!(name.as_deref(), Some("IMG_0001-2.png"));
}
#[test]
fn a_record_carries_its_policy_and_an_old_one_has_none() {
let dir = tmp();
let outbox = dir.join("outbox");
place(
&encoded("a.jpg", b"x"),
ExportTarget::Remote,
"E",
&outbox,
CollisionPolicy::Skip,
)
.unwrap();
let queue = pending(&outbox);
assert_eq!(queue[0].collision, Some(CollisionPolicy::Skip));
assert!(!queue[0].account);
std::fs::write(outbox.join("old.jpg"), b"x").unwrap();
std::fs::write(outbox.join("old.jpg.dest"), "E\nold.jpg\naccount\n").unwrap();
let old = pending(&outbox)
.into_iter()
.find(|p| p.name == "old.jpg")
.unwrap();
assert_eq!(old.collision, None);
assert!(old.account);
}
#[test] #[test]
fn a_payload_with_no_record_is_ignored() { fn a_payload_with_no_record_is_ignored() {
// The window a kill between the two writes leaves behind. It must not // The window a kill between the two writes leaves behind. It must not
@@ -2020,6 +2414,7 @@ mod tests {
remote_dir: "Exports/2026".into(), remote_dir: "Exports/2026".into(),
name: "a.jpg".into(), name: "a.jpg".into(),
account: false, account: false,
collision: None,
}; };
assert_eq!( assert_eq!(
entry.remote_path("Photos").as_str(), entry.remote_path("Photos").as_str(),
@@ -2040,6 +2435,7 @@ mod tests {
remote_dir: String::new(), remote_dir: String::new(),
name: "a.jpg".into(), name: "a.jpg".into(),
account: false, account: false,
collision: None,
}; };
assert_eq!(entry.remote_path("Photos").as_str(), "Photos/a.jpg"); assert_eq!(entry.remote_path("Photos").as_str(), "Photos/a.jpg");
} }
@@ -2053,6 +2449,7 @@ mod tests {
remote_dir: "/Exports/".into(), remote_dir: "/Exports/".into(),
name: "a.jpg".into(), name: "a.jpg".into(),
account: false, account: false,
collision: None,
}; };
assert_eq!( assert_eq!(
entry.remote_path("/Photos/").as_str(), entry.remote_path("/Photos/").as_str(),
@@ -2077,6 +2474,7 @@ mod tests {
ExportTarget::Remote, ExportTarget::Remote,
"/Shared/Web", "/Shared/Web",
&dir, &dir,
CollisionPolicy::Increment,
) )
.unwrap(); .unwrap();
@@ -2125,6 +2523,7 @@ mod tests {
images: Vec::new(), images: Vec::new(),
conn: None, conn: None,
settings, settings,
remote_names: HashSet::new(),
outbox: std::env::temp_dir().join("dr-batch-test-outbox"), outbox: std::env::temp_dir().join("dr-batch-test-outbox"),
sidecar_cache: std::env::temp_dir().join("dr-batch-test-sidecars"), sidecar_cache: std::env::temp_dir().join("dr-batch-test-sidecars"),
offline: true, offline: true,
@@ -2432,6 +2831,7 @@ mod tests {
let mut issued = HashSet::new(); let mut issued = HashSet::new();
let name = resolve_batch_name( let name = resolve_batch_name(
&settings, &settings,
&HashSet::new(),
&NameContext { &NameContext {
source_stem: "IMG_0001", source_stem: "IMG_0001",
sequence: 1, sequence: 1,
+68 -19
View File
@@ -712,6 +712,7 @@ fn batch_request(
images: Vec::new(), images: Vec::new(),
conn: library.credentials(), conn: library.credentials(),
settings: stored.export, settings: stored.export,
remote_names: Default::default(),
outbox: match library.session() { outbox: match library.session() {
Some(c) => export::outbox_dir(&c.account), Some(c) => export::outbox_dir(&c.account),
// No account, so no outbox — a device export still works, and a // No account, so no outbox — a device export still works, and a
@@ -793,29 +794,36 @@ pub(crate) fn refresh_export_label(window: &AppWindow) {
/// dragged, because each move event destroys the thing that would deliver /// dragged, because each move event destroys the thing that would deliver
/// the next one. /// the next one.
/// TRACES: FR-CAT-8 /// TRACES: FR-CAT-8
/// Apply a fetched sidecar to the open session, now or as soon as it arrives. /// Apply a fetched sidecar to the open session, then draw its first frame.
/// ///
/// The sidecar fetch is started beside the image fetch and is three orders of /// The sidecar fetch is started beside the image fetch and is three orders of
/// magnitude smaller, so it has almost always landed by the time there is a /// magnitude smaller, so it has usually landed by the time there is a session
/// session to apply it to — and this takes it straight from the channel. The /// to apply it to — and this takes it straight from the channel. An original
/// timer covers the case where it has not, which is why this is not simply a /// read from the cache can still beat it, and drawing then showed the
/// blocking receive: a slow or stalled sidecar request must not freeze the /// photograph at its defaults and changed it a moment later. So the first
/// window with the photograph already decoded and on screen. /// frame waits up to `SIDECAR_GRACE` for the edit, with the grid's thumbnail
/// still standing in; past that it is drawn at its defaults, and a later
/// arrival redraws. Not a blocking receive: a stalled request must not freeze
/// the window.
/// ///
/// A late arrival redraws, so the image is correct either way; the only /// `still_current` is false once the view has moved to another photograph,
/// difference is whether it was ever briefly shown at its defaults. /// 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<Option<dr_pipeline::Sidecar>>>,
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)>,
still_current: impl Fn() -> bool + 'static,
) { ) {
// Already here — the overwhelmingly common case. const SIDECAR_GRACE: std::time::Duration = std::time::Duration::from_millis(400);
// Already here — the common case.
if let Ok(got) = rx.try_recv() { if let Ok(got) = rx.try_recv() {
if let Some(sidecar) = got { if let Some(sidecar) = got {
presets::apply_stored_edit(window, &sidecar, session, rows); presets::apply_stored_edit(window, &sidecar, session, rows);
} }
redraw(window);
return; return;
} }
@@ -823,17 +831,37 @@ fn apply_when_ready(
let session = session.clone(); let session = session.clone();
let rows = rows.clone(); let rows = rows.clone();
let redraw = redraw.clone(); let redraw = redraw.clone();
let deadline = std::time::Instant::now() + SIDECAR_GRACE;
let drawn = Cell::new(false);
let timer = Rc::new(slint::Timer::default()); let timer = Rc::new(slint::Timer::default());
let held = timer.clone(); let held = timer.clone();
timer.start( timer.start(
slint::TimerMode::Repeated, slint::TimerMode::Repeated,
std::time::Duration::from_millis(50), std::time::Duration::from_millis(50),
move || { move || {
let Ok(got) = rx.try_recv() else { return }; let Some(w) = weak.upgrade() else {
held.stop();
return;
};
if !still_current() {
held.stop();
return;
}
let Ok(got) = rx.try_recv() else {
// Waited long enough: draw at the defaults and keep
// listening, so a late edit still lands.
if !drawn.get() && std::time::Instant::now() >= deadline {
drawn.set(true);
redraw(&w);
}
return;
};
held.stop(); held.stop();
let Some(w) = weak.upgrade() else { return }; let applied = match got {
let Some(sidecar) = got else { return }; Some(sidecar) => presets::apply_stored_edit(&w, &sidecar, &session, &rows),
if presets::apply_stored_edit(&w, &sidecar, &session, &rows) { None => false,
};
if applied || !drawn.get() {
redraw(&w); redraw(&w);
} }
}, },
@@ -2243,6 +2271,13 @@ fn build_render_now(
window.set_canvas_draft(draft); window.set_canvas_draft(draft);
window.global::<Levels>().set_provisional(draft); window.global::<Levels>().set_provisional(draft);
window.set_load_error("".into()); window.set_load_error("".into());
// TRACES: FR-NC-6a
// The library path keeps the grid's thumbnail up until
// here, the first frame of the new photograph, because
// the canvas still holds the last photograph's texture
// until this line replaces it.
window.set_load_pending(false);
window.set_has_load_preview(false);
// The readout and the "Fit" button follow the session // The readout and the "Fit" button follow the session
// rather than the gesture, so a clamped zoom shows the // rather than the gesture, so a clamped zoom shows the
// value that was actually applied. // value that was actually applied.
@@ -2778,13 +2813,18 @@ fn wire_remote_open(
log::debug!("{name}: landed after the view moved on"); log::debug!("{name}: landed after the view moved on");
return; return;
} }
w.set_load_pending(false); // The bar goes, and the thumbnail goes back to full
// strength; the thumbnail itself stays until the first
// frame of this photograph replaces it in `render_now`.
// Dropping it here showed the last photograph's texture,
// still in the canvas, for the length of the decode.
w.set_load_waiting("".into()); w.set_load_waiting("".into());
w.set_has_load_preview(false);
let bytes = match got { let bytes = match got {
Ok(b) => b, Ok(b) => b,
Err(e) => { Err(e) => {
w.set_load_pending(false);
w.set_has_load_preview(false);
job.fail(e.message.clone()); job.fail(e.message.clone());
log::warn!("{name}: {e}"); log::warn!("{name}: {e}");
// Offline needs its own words. "network error: // Offline needs its own words. "network error:
@@ -2853,16 +2893,19 @@ fn wire_remote_open(
// TRACES: FR-CAT-8 // TRACES: FR-CAT-8
// The stored edit, if it has landed. It // The stored edit, if it has landed. It
// was started before the download of a // was started before the download of a
// file thousands of times its size, so in // file thousands of times its size, but
// practice it has; `apply_when_ready` // an original read from the cache can
// covers the case where it has not rather // beat it; `apply_when_ready` holds the
// first frame back a moment for it rather
// than blocking the UI thread on a socket. // than blocking the UI thread on a socket.
let still = current.clone();
apply_when_ready( apply_when_ready(
&w, &w,
sidecar_rx.clone(), sidecar_rx.clone(),
&session, &session,
&rows, &rows,
&redraw, &redraw,
move || still.get() == mine,
); );
// TRACES: FR-UI-4 // TRACES: FR-UI-4
// Under the same magnifier as the last // Under the same magnifier as the last
@@ -2875,12 +2918,16 @@ fn wire_remote_open(
// moves the point rather than losing it. // moves the point rather than losing it.
resume_inspection(&session, &viewport, &inspection); resume_inspection(&session, &viewport, &inspection);
sync_rows(&w, &rows, &session); sync_rows(&w, &rows, &session);
redraw(&w); // No redraw here: `apply_when_ready` makes
// the first one, once the edit is applied
// or has been waited for long enough.
} }
None => { None => {
*session.borrow_mut() = None; *session.borrow_mut() = None;
rows.set_vec(Vec::<ParamRow>::new()); rows.set_vec(Vec::<ParamRow>::new());
w.global::<Develop>().set_enabled(false); w.global::<Develop>().set_enabled(false);
w.set_load_pending(false);
w.set_has_load_preview(false);
if let Some(image) = l.fallback { if let Some(image) = l.fallback {
w.set_canvas(image); w.set_canvas(image);
} }
@@ -2892,6 +2939,8 @@ fn wire_remote_open(
log::warn!("{name}: {e}"); log::warn!("{name}: {e}");
*session.borrow_mut() = None; *session.borrow_mut() = None;
w.global::<Develop>().set_enabled(false); w.global::<Develop>().set_enabled(false);
w.set_load_pending(false);
w.set_has_load_preview(false);
w.set_load_error(e.into()); w.set_load_error(e.into());
} }
} }