diff --git a/crates/pdfs-core/src/cache.rs b/crates/pdfs-core/src/cache.rs index 77d2e24..c9a6ed0 100644 --- a/crates/pdfs-core/src/cache.rs +++ b/crates/pdfs-core/src/cache.rs @@ -19,6 +19,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt; use std::io::{Read, Seek, SeekFrom}; +use std::os::unix::fs::PermissionsExt as _; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -415,6 +416,10 @@ pub struct ContentCache { /// Subdirectory holding cached thumbnails (`.t` + `.meta`). Kept out /// of `content_dir` so the budget scan never sees thumbnail files. thumb_dir: PathBuf, + /// Owner-protected scratch space for decrypted camera RAWs passed to + /// exiftool. Files are removed on drop and swept on the next cache open after + /// an unclean shutdown. + raw_thumb_dir: PathBuf, /// Subdirectory holding on-demand block-cache files (`.b` + /// `.meta`). Kept out of `content_dir` so the whole-file budget scan never /// sees them; blocks carry their own LRU budget. @@ -507,6 +512,19 @@ impl ContentCache { std::fs::create_dir_all(&content_dir)?; let thumb_dir = content_dir.join("thumbs"); std::fs::create_dir_all(&thumb_dir)?; + let raw_thumb_dir = content_dir.join("raw-thumbnails"); + std::fs::create_dir_all(&raw_thumb_dir)?; + std::fs::set_permissions(&raw_thumb_dir, std::fs::Permissions::from_mode(0o700))?; + if let Ok(entries) = std::fs::read_dir(&raw_thumb_dir) { + for entry in entries.flatten() { + let name = entry.file_name(); + if name.to_string_lossy().starts_with("pdfs-raw-") + && entry.file_type().is_ok_and(|kind| kind.is_file()) + { + let _ = std::fs::remove_file(entry.path()); + } + } + } let block_dir = content_dir.join("blocks"); std::fs::create_dir_all(&block_dir)?; // Scratch holds disk-backed write buffers. A previous run's leftovers are @@ -530,6 +548,7 @@ impl ContentCache { let cache = Self { content_dir, thumb_dir, + raw_thumb_dir, block_dir, scratch_dir, staging_dir, @@ -1666,6 +1685,12 @@ impl ContentCache { .join(format!("{}.t{ttype}.meta", Self::key(uid))) } + /// Owner-protected transient directory used to stage decrypted camera RAWs + /// for metadata helpers. Callers still remove individual files promptly. + pub fn raw_thumbnail_staging_dir(&self) -> &Path { + &self.raw_thumb_dir + } + /// Serve a cached thumbnail of `ttype` for `uid`, or `None` on miss/stale. /// Validated by node `mtime` alone: a new revision (which is what changes the /// thumbnail) bumps the mtime, so a stale thumbnail is detected. Thumbnails diff --git a/crates/pdfs-core/src/control.rs b/crates/pdfs-core/src/control.rs index f81601a..46b7f0e 100644 --- a/crates/pdfs-core/src/control.rs +++ b/crates/pdfs-core/src/control.rs @@ -104,6 +104,10 @@ pub enum Request { #[serde(default)] generation: u64, }, + /// Reserve a daemon-issued generation for ordinary-file thumbnail work. + /// The daemon is the ordering authority, so GUI restarts and wall-clock + /// corrections cannot make a new listing look older than an earlier one. + ReserveFileThumbGeneration, /// Stop ordinary-file thumbnail work belonging to older listings. Photos /// timeline work and an explicit recursive build are deliberately separate. CancelFileThumbs { generation: u64 }, @@ -111,6 +115,8 @@ pub enum Request { /// The request returns immediately; progress is read with /// [`Request::ThumbnailBuildStatus`]. StartThumbnailBuild { path: String }, + /// Stop the current recursive ordinary-file thumbnail build, if any. + CancelThumbnailBuild, /// Read progress for the recursive ordinary-file thumbnail build. ThumbnailBuildStatus, /// Download a photo's full content into the cache; replies with its path. @@ -1232,35 +1238,9 @@ pub fn is_thumbnail_image_name(name: &str) -> bool { let Some((_, extension)) = name.rsplit_once('.') else { return false; }; - matches!( - extension.to_ascii_lowercase().as_str(), - "bmp" - | "gif" - | "jpeg" - | "jpg" - | "png" - | "tif" - | "tiff" - | "webp" - | "arw" - | "cr2" - | "cr3" - | "crw" - | "dng" - | "k25" - | "kdc" - | "mrw" - | "nef" - | "nrw" - | "orf" - | "pef" - | "raf" - | "rw" - | "rw2" - | "sr2" - | "srf" - | "x3f" - ) + let extension = extension.to_ascii_lowercase(); + STANDARD_THUMBNAIL_EXTENSIONS.contains(&extension.as_str()) + || RAW_THUMBNAIL_EXTENSIONS.contains(&extension.as_str()) } /// Whether `name` is one of the concrete RAW formats handled by the exiftool @@ -1270,29 +1250,21 @@ pub fn is_raw_image_name(name: &str) -> bool { let Some((_, extension)) = name.rsplit_once('.') else { return false; }; - matches!( - extension.to_ascii_lowercase().as_str(), - "arw" - | "cr2" - | "cr3" - | "crw" - | "dng" - | "k25" - | "kdc" - | "mrw" - | "nef" - | "nrw" - | "orf" - | "pef" - | "raf" - | "rw" - | "rw2" - | "sr2" - | "srf" - | "x3f" - ) + let extension = extension.to_ascii_lowercase(); + RAW_THUMBNAIL_EXTENSIONS.contains(&extension.as_str()) } +const STANDARD_THUMBNAIL_EXTENSIONS: &[&str] = + &["bmp", "gif", "jpeg", "jpg", "png", "tif", "tiff", "webp"]; + +/// Concrete camera RAW formats with embedded previews handled by exiftool. +/// This is the single source used by both RAW classification and the broader +/// thumbnail capability check. +const RAW_THUMBNAIL_EXTENSIONS: &[&str] = &[ + "arw", "cr2", "cr3", "crw", "dng", "k25", "kdc", "mrw", "nef", "nrw", "orf", "pef", "raf", + "raw", "rw2", "sr2", "srf", "x3f", +]; + /// Progress of the one recursive ordinary-file thumbnail build the daemon may /// run at a time. #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq)] @@ -1467,6 +1439,13 @@ pub enum Response { PhotoMonths { months: Vec }, /// Thumbnails for a [`Request::PhotoThumbs`] batch. Thumbs { items: Vec }, + /// A [`Request::FileThumbs`] generation is no longer current. This is + /// retryable after reserving a fresh generation and must never be interpreted + /// as a permanent "no thumbnail" verdict. + FileThumbsStale, + /// Daemon-issued generation reserved by + /// [`Request::ReserveFileThumbGeneration`]. + FileThumbGeneration { generation: u64 }, /// Current recursive thumbnail-build progress. ThumbnailBuild { status: ThumbnailBuildStatus }, /// An on-disk path the front-end can open (e.g. a downloaded photo). @@ -1801,10 +1780,12 @@ mod tests { #[test] fn thumbnail_build_requests_and_progress_roundtrip() { let requests = [ + Request::ReserveFileThumbGeneration, Request::CancelFileThumbs { generation: 43 }, Request::StartThumbnailBuild { path: "pictures/events".into(), }, + Request::CancelThumbnailBuild, Request::ThumbnailBuildStatus, ]; for request in requests { @@ -1828,6 +1809,16 @@ mod tests { let line = serde_json::to_string(&response).unwrap(); let decoded: Response = serde_json::from_str(&line).unwrap(); assert_eq!(line, serde_json::to_string(&decoded).unwrap()); + + let response = Response::FileThumbGeneration { generation: 44 }; + let line = serde_json::to_string(&response).unwrap(); + let decoded: Response = serde_json::from_str(&line).unwrap(); + assert_eq!(line, serde_json::to_string(&decoded).unwrap()); + + let response = Response::FileThumbsStale; + let line = serde_json::to_string(&response).unwrap(); + let decoded: Response = serde_json::from_str(&line).unwrap(); + assert_eq!(line, serde_json::to_string(&decoded).unwrap()); } #[test] diff --git a/crates/pdfs-fuse/src/control.rs b/crates/pdfs-fuse/src/control.rs index 445384e..c5557f3 100644 --- a/crates/pdfs-fuse/src/control.rs +++ b/crates/pdfs-fuse/src/control.rs @@ -329,8 +329,14 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U items: core.photo_thumbs(&parsed), } } - Ok(CtlRequest::FileThumbs { items, generation }) => CtlResponse::Thumbs { - items: core.file_thumbs(&items, generation), + Ok(CtlRequest::FileThumbs { items, generation }) => { + match core.file_thumbs(&items, generation) { + Some(items) => CtlResponse::Thumbs { items }, + None => CtlResponse::FileThumbsStale, + } + } + Ok(CtlRequest::ReserveFileThumbGeneration) => CtlResponse::FileThumbGeneration { + generation: core.reserve_file_thumb_generation(), }, Ok(CtlRequest::CancelFileThumbs { generation }) => { core.cancel_file_thumbs(generation); @@ -339,11 +345,15 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U } } Ok(CtlRequest::StartThumbnailBuild { path }) => match rel_to_mount(mountpoint, &path) { - Ok(rel) => CtlResponse::ThumbnailBuild { - status: core.start_thumbnail_build(rel), + Ok(rel) => match core.start_thumbnail_build(rel) { + Ok(status) => CtlResponse::ThumbnailBuild { status }, + Err(error) => CtlResponse::error(error), }, Err(error) => CtlResponse::error(error), }, + Ok(CtlRequest::CancelThumbnailBuild) => CtlResponse::ThumbnailBuild { + status: core.cancel_thumbnail_build(), + }, Ok(CtlRequest::ThumbnailBuildStatus) => CtlResponse::ThumbnailBuild { status: core.thumbnail_build_status(), }, diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index f22b32e..822ecfb 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -374,6 +374,45 @@ impl SearchRoots { } } +/// Negative thumbnail knowledge has two different authorities. A remote miss +/// only says Proton has no thumbnail to download; it must never suppress local +/// generation. A local miss says this exact file revision was successfully +/// inspected and could not be decoded. Both maps are bounded independently. +#[derive(Default)] +struct ThumbnailMissCaches { + remote: HashMap<(NodeUid, i32), i64>, + local: HashMap<(NodeUid, i32), i64>, +} + +impl ThumbnailMissCaches { + fn remote_contains(&self, key: &(NodeUid, i32), tag: i64) -> bool { + self.remote.get(key) == Some(&tag) + } + + fn local_contains(&self, key: &(NodeUid, i32), tag: i64) -> bool { + self.local.get(key) == Some(&tag) + } + + fn remember_remote(&mut self, key: (NodeUid, i32), tag: i64) { + Self::remember(&mut self.remote, key, tag); + } + + fn remember_local(&mut self, key: (NodeUid, i32), tag: i64) { + Self::remember(&mut self.local, key, tag); + } + + fn forget_local(&mut self, key: &(NodeUid, i32)) { + self.local.remove(key); + } + + fn remember(map: &mut HashMap<(NodeUid, i32), i64>, key: (NodeUid, i32), tag: i64) { + if map.len() >= MAX_THUMBNAIL_MISSES && !map.contains_key(&key) { + map.clear(); + } + map.insert(key, tag); + } +} + /// `path` expressed relative to `root`, or `None` if it is not below it. The /// root itself resolves to the empty path; an empty root is the mount itself /// and covers everything. @@ -521,8 +560,12 @@ struct Core { file_thumb_cancel: Arc, /// Progress of the explicit recursive build started from the Files toolbar. thumbnail_build: Arc>, - /// Nodes the remote has told us have *no* thumbnail of a given type, keyed by - /// `(uid, thumbnail type)` and holding the mtime the answer was learned at. + /// Cancellation for the explicit recursive build. Separate from ordinary + /// listing cancellation because the two jobs have independent lifetimes. + thumbnail_build_cancelled: Arc, + thumbnail_build_cancel: Arc, + /// Negative thumbnail knowledge, separated by whether the remote lacked a + /// thumbnail or local decoding proved this revision unsupported. /// /// Absence has to be cached or it costs a round trip every time it is asked /// for, and it is asked for constantly: an `ls -l` from an xattr-aware lister @@ -530,7 +573,7 @@ struct Core { /// of videos re-probed 130 times per listing at ~186 ms each (B5). The mtime /// is the validity tag — a new revision may well have a thumbnail — matching /// how [`ContentCache::read_thumbnail`] validates the positive side. - no_thumbnail: Arc>>, + thumbnail_misses: Arc>, /// Last account quota answer and when it was learned, for `statfs(2)`. /// /// `df` and the free-space preflight of every file manager and installer ask @@ -3590,7 +3633,7 @@ impl Core { // "This file has no thumbnail" is an answer worth remembering: without // it every listing pays a round trip per file to be told nothing (B5). let key = (uid.clone(), ttype.as_i32()); - if self.no_thumbnail.lock().get(&key) == Some(&mtime) { + if self.thumbnail_misses.lock().remote_contains(&key, mtime) { return Ok(None); } let bytes = self @@ -3607,15 +3650,7 @@ impl Core { .store_thumbnail(&uid, ttype.as_i32(), mtime, bytes); } None => { - let mut misses = self.no_thumbnail.lock(); - // Bounded by clearing rather than by LRU: the entries are two - // words each, the cap is far above any plausible working set, - // and re-learning a miss costs one round trip. Not worth a - // second data structure to order them. - if misses.len() >= MAX_THUMBNAIL_MISSES { - misses.clear(); - } - misses.insert(key, mtime); + self.thumbnail_misses.lock().remember_remote(key, mtime); } } Ok(bytes) @@ -7556,6 +7591,38 @@ mod tests { } } +#[cfg(test)] +mod thumbnail_miss_tests { + use super::*; + + #[test] + fn remote_misses_never_suppress_local_generation() { + let uid = parse_uid("volume~node").expect("uid"); + let key = (uid, ThumbnailType::Thumbnail.as_i32()); + let mut misses = ThumbnailMissCaches::default(); + + misses.remember_remote(key.clone(), 42); + assert!(misses.remote_contains(&key, 42)); + assert!(!misses.local_contains(&key, 42)); + + misses.remember_local(key.clone(), 42); + assert!(misses.local_contains(&key, 42)); + misses.forget_local(&key); + assert!(!misses.local_contains(&key, 42)); + assert!(misses.remote_contains(&key, 42)); + } + + #[test] + fn local_miss_cache_is_bounded_on_gui_only_paths() { + let mut misses = ThumbnailMissCaches::default(); + for index in 0..=MAX_THUMBNAIL_MISSES { + let uid = parse_uid(&format!("volume~node-{index}")).expect("uid"); + misses.remember_local((uid, ThumbnailType::Thumbnail.as_i32()), 42); + } + assert_eq!(misses.local.len(), 1); + } +} + #[cfg(test)] mod search_root_tests { use super::*; diff --git a/crates/pdfs-fuse/src/mount.rs b/crates/pdfs-fuse/src/mount.rs index c9f392e..bea904f 100644 --- a/crates/pdfs-fuse/src/mount.rs +++ b/crates/pdfs-fuse/src/mount.rs @@ -395,7 +395,9 @@ pub fn mount( file_thumb_generation: Arc::new(AtomicU64::new(0)), file_thumb_cancel: Arc::new(tokio::sync::Notify::new()), thumbnail_build: Arc::new(Mutex::new(Default::default())), - no_thumbnail: Arc::new(Mutex::new(HashMap::new())), + thumbnail_build_cancelled: Arc::new(AtomicBool::new(false)), + thumbnail_build_cancel: Arc::new(tokio::sync::Notify::new()), + thumbnail_misses: Arc::new(Mutex::new(Default::default())), quota: Arc::new(Mutex::new(None)), size_upgrades: Arc::new(Mutex::new(HashMap::new())), size_waiters: Arc::new(Default::default()), diff --git a/crates/pdfs-fuse/src/photos.rs b/crates/pdfs-fuse/src/photos.rs index 7185be5..feb0e65 100644 --- a/crates/pdfs-fuse/src/photos.rs +++ b/crates/pdfs-fuse/src/photos.rs @@ -6,10 +6,11 @@ //! a phone) are generated locally and stored as if the server had served them. use std::collections::{HashMap, HashSet}; +use std::ffi::OsStr; use std::fs::OpenOptions; use std::io::{Read as _, Write as _}; use std::os::unix::fs::OpenOptionsExt as _; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::Once; use std::sync::atomic::{AtomicU64, Ordering}; @@ -44,6 +45,11 @@ const THUMB_QUALITY: u8 = 82; pub(crate) const THUMB_GEN_CONCURRENCY: usize = 4; /// Images processed between recursive-build progress updates. const THUMB_BUILD_CHUNK: usize = 16; +/// A recursive build may wait briefly for an opportunistic tile job that +/// already owns a uid, but never forever. +const THUMB_BUILD_CLAIM_TIMEOUT: Duration = Duration::from_secs(30); +/// Backstop for cancellation if a notification races with waiter registration. +const THUMB_CANCEL_POLL: Duration = Duration::from_millis(100); /// A broken metadata helper must not hold one of the four thumbnail permits /// indefinitely. Matches the deliberately generous KDE RAW thumbnail budget. const EXIFTOOL_TIMEOUT: Duration = Duration::from_secs(20); @@ -68,6 +74,17 @@ struct GeneratedThumb { ratio: f64, } +/// Scaling distinguishes a permanent content verdict from an environmental +/// failure. Only `Undecodable` may enter the local negative cache. +enum ScaleAttempt { + Made(GeneratedThumb), + Undecodable, + Unavailable, +} + +#[derive(Debug, Clone, Copy)] +struct RawPreviewUnavailable; + /// How one attempt at generating a missing thumbnail ended. The distinction that /// matters is *permanent* versus *transient*: only bytes we cannot decode prove /// the photo will never have a thumbnail, and only that verdict is persisted. @@ -109,13 +126,49 @@ impl ThumbBatchSummary { } } +/// `true` means publish a new build, `false` means the caller may attach to the +/// already-running build for the same root. A different root is never silently +/// accepted as if it were the requested job. +fn thumbnail_build_may_start(status: &ThumbnailBuildStatus, path: &str) -> CoreResult { + if !status.running { + return Ok(true); + } + if status.path == path { + return Ok(false); + } + Err(CoreError::invalid(format!( + "a thumbnail build is already running for {}", + if status.path.is_empty() { + "Proton Drive" + } else { + &status.path + } + ))) +} + /// Scale a full-size photo down to a thumbnail: at most [`THUMB_EDGE`] on its -/// longest side, JPEG, aspect ratio preserved. `None` when the bytes aren't an -/// image this build can decode — the caller then writes the photo off as -/// un-thumbnailable. +/// longest side, JPEG, aspect ratio preserved. Environmental RAW-extraction +/// failures remain retryable; only bytes conclusively outside the supported +/// formats return an undecodable verdict. /// /// CPU-bound (a 20 MP JPEG is real work), so callers run it on the blocking pool. -fn scale_thumbnail(bytes: &[u8], name: &str) -> Option { +fn scale_thumbnail(bytes: &[u8], name: &str, staging_dir: &Path) -> ScaleAttempt { + scale_thumbnail_with_exiftool( + bytes, + name, + staging_dir, + OsStr::new("exiftool"), + EXIFTOOL_TIMEOUT, + ) +} + +fn scale_thumbnail_with_exiftool( + bytes: &[u8], + name: &str, + staging_dir: &Path, + exiftool: &OsStr, + timeout: Duration, +) -> ScaleAttempt { let direct = image::ImageReader::new(std::io::Cursor::new(bytes)) .with_guessed_format() .ok() @@ -123,22 +176,29 @@ fn scale_thumbnail(bytes: &[u8], name: &str) -> Option { let image = match direct { Some(image) => image, None if is_raw_image_name(name) => { - let (preview, orientation) = extract_raw_preview(bytes, name)?; - let mut image = image::ImageReader::new(std::io::Cursor::new(preview)) + let (preview, orientation) = + match extract_raw_preview(bytes, name, staging_dir, exiftool, timeout) { + Ok(Some(preview)) => preview, + Ok(None) => return ScaleAttempt::Undecodable, + Err(_) => return ScaleAttempt::Unavailable, + }; + let Some(mut image) = image::ImageReader::new(std::io::Cursor::new(preview)) .with_guessed_format() - .ok()? - .decode() - .ok()?; + .ok() + .and_then(|reader| reader.decode().ok()) + else { + return ScaleAttempt::Undecodable; + }; if let Some(orientation) = image::metadata::Orientation::from_exif(orientation) { image.apply_orientation(orientation); } image } - None => return None, + None => return ScaleAttempt::Undecodable, }; let (width, height) = (image.width(), image.height()); if width == 0 || height == 0 { - return None; + return ScaleAttempt::Undecodable; } let ratio = f64::from(width) / f64::from(height); @@ -146,10 +206,13 @@ fn scale_thumbnail(bytes: &[u8], name: &str) -> Option { // THUMB_EDGE and the ratio is untouched. let thumb = image.thumbnail(THUMB_EDGE, THUMB_EDGE).to_rgb8(); let mut bytes = Vec::new(); - image::codecs::jpeg::JpegEncoder::new_with_quality(&mut bytes, THUMB_QUALITY) + if image::codecs::jpeg::JpegEncoder::new_with_quality(&mut bytes, THUMB_QUALITY) .encode_image(&thumb) - .ok()?; - Some(GeneratedThumb { bytes, ratio }) + .is_err() + { + return ScaleAttempt::Unavailable; + } + ScaleAttempt::Made(GeneratedThumb { bytes, ratio }) } /// Temporary file passed to exiftool. The helper identifies most RAWs from their @@ -158,7 +221,7 @@ fn scale_thumbnail(bytes: &[u8], name: &str) -> Option { struct RawTempFile(PathBuf); impl RawTempFile { - fn create(bytes: &[u8], name: &str) -> Option { + fn create_in(staging_dir: &Path, bytes: &[u8], name: &str) -> Option { let extension = std::path::Path::new(name) .extension() .and_then(|value| value.to_str()) @@ -174,7 +237,7 @@ impl RawTempFile { .as_nanos(); for _ in 0..16 { let nonce = RAW_TEMP_NONCE.fetch_add(1, Ordering::Relaxed); - let path = std::env::temp_dir().join(format!( + let path = staging_dir.join(format!( "pdfs-raw-{}-{started}-{nonce}.{extension}", std::process::id(), )); @@ -208,8 +271,13 @@ impl Drop for RawTempFile { /// Run one bounded exiftool JSON query while draining stdout concurrently. The /// latter matters for full-size `JpgFromRaw` fallbacks, whose base64 can exceed a /// pipe buffer long before the child exits. -fn exiftool_query(path: &std::path::Path, tags: &[&str]) -> Option { - let mut command = Command::new("exiftool"); +fn exiftool_query( + exiftool: &OsStr, + path: &Path, + tags: &[&str], + timeout: Duration, +) -> Result { + let mut command = Command::new(exiftool); command.args(["-j", "-b", "-n"]); command.args(tags); command @@ -227,10 +295,14 @@ fn exiftool_query(path: &std::path::Path, tags: &[&str]) -> Option Option break status.success(), - Ok(None) if started.elapsed() < EXIFTOOL_TIMEOUT => { + Ok(None) if started.elapsed() < timeout => { std::thread::sleep(Duration::from_millis(10)); } Ok(None) | Err(_) => { @@ -249,11 +321,15 @@ fn exiftool_query(path: &std::path::Path, tags: &[&str]) -> Option Option<(Vec, u8)> { - let file = RawTempFile::create(bytes, name)?; - let primary = exiftool_query(&file.0, &["-Orientation", "-PreviewImage"])?; - let primary = exiftool_object(&primary)?; +fn extract_raw_preview( + bytes: &[u8], + name: &str, + staging_dir: &Path, + exiftool: &OsStr, + timeout: Duration, +) -> Result, u8)>, RawPreviewUnavailable> { + let file = RawTempFile::create_in(staging_dir, bytes, name).ok_or(RawPreviewUnavailable)?; + let primary = exiftool_query( + exiftool, + &file.0, + &["-Orientation", "-PreviewImage"], + timeout, + )?; + let primary = exiftool_object(&primary).ok_or(RawPreviewUnavailable)?; let orientation = primary .get("Orientation") .and_then(serde_json::Value::as_u64) .and_then(|value| u8::try_from(value).ok()) .unwrap_or(1); if let Some(preview) = exiftool_binary(primary, "PreviewImage") { - return Some((preview, orientation)); + return Ok(Some((preview, orientation))); } const FALLBACKS: [&str; 5] = [ @@ -297,6 +384,7 @@ fn extract_raw_preview(bytes: &[u8], name: &str) -> Option<(Vec, u8)> { "ThumbnailTIFF", ]; let fallback = exiftool_query( + exiftool, &file.0, &[ "-OtherImage", @@ -305,12 +393,13 @@ fn extract_raw_preview(bytes: &[u8], name: &str) -> Option<(Vec, u8)> { "-ThumbnailImage", "-ThumbnailTIFF", ], + timeout, )?; - let fallback = exiftool_object(&fallback)?; - FALLBACKS + let fallback = exiftool_object(&fallback).ok_or(RawPreviewUnavailable)?; + Ok(FALLBACKS .iter() .find_map(|key| exiftool_binary(fallback, key)) - .map(|preview| (preview, orientation)) + .map(|preview| (preview, orientation))) } /// Formats supported by the local thumbnail decoder and exposed by the GUI. @@ -514,14 +603,12 @@ impl Core { &self, items: &[FileThumbRequest], generation: u64, - ) -> Vec { - let previous = self - .file_thumb_generation - .fetch_max(generation, Ordering::SeqCst); - if generation > previous { - self.file_thumb_cancel.notify_waiters(); + ) -> Option> { + let current = + generation != 0 && self.file_thumb_generation.load(Ordering::SeqCst) == generation; + if !current { + return None; } - let current = self.file_thumb_generation.load(Ordering::SeqCst) == generation; let ttype = ThumbnailType::Thumbnail.as_i32(); let parsed: Vec<(String, NodeUid, i64)> = items .iter() @@ -549,7 +636,10 @@ impl Core { .cache .cached_thumbnail_path(uid, ttype, *modified) .is_some() - || self.no_thumbnail.lock().get(&(uid.clone(), ttype)) == Some(modified) + || self + .thumbnail_misses + .lock() + .local_contains(&(uid.clone(), ttype), *modified) || !seen.insert(raw.clone()) { continue; @@ -557,26 +647,44 @@ impl Core { wanted.push(uid.clone()); } - if current && !wanted.is_empty() { + if !wanted.is_empty() { self.spawn_generate_thumbs(wanted, &tags, &names, ThumbJob::Files(generation)); } let pending = self.thumb_gen.lock(); - items - .iter() - .map(|item| { - let uid = parse_uid(&item.uid); - PhotoThumb { - uid: item.uid.clone(), - path: uid.as_ref().and_then(|uid| { - self.cache - .cached_thumbnail_path(uid, ttype, item.modified) - .map(|path| path.display().to_string()) - }), - pending: current && uid.as_ref().is_some_and(|uid| pending.contains(uid)), - } + Some( + items + .iter() + .map(|item| { + let uid = parse_uid(&item.uid); + PhotoThumb { + uid: item.uid.clone(), + path: uid.as_ref().and_then(|uid| { + self.cache + .cached_thumbnail_path(uid, ttype, item.modified) + .map(|path| path.display().to_string()) + }), + pending: uid.as_ref().is_some_and(|uid| pending.contains(uid)), + } + }) + .collect(), + ) + } + + /// Reserve the next ordinary-file listing generation. This daemon-owned + /// monotonic value replaces GUI wall-clock seeds, which can move backwards + /// across NTP corrections, suspend, or dual boot. + pub(crate) fn reserve_file_thumb_generation(&self) -> u64 { + let next = |current| if current == u64::MAX { 1 } else { current + 1 }; + let previous = self + .file_thumb_generation + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |current| { + Some(next(current)) }) - .collect() + .expect("generation update always succeeds"); + let generation = next(previous); + self.file_thumb_cancel.notify_waiters(); + generation } /// Advance the ordinary-file thumbnail generation. Every queued/download in @@ -644,7 +752,8 @@ impl Core { ThumbJob::Files(generation) => { self.file_thumb_generation.load(Ordering::SeqCst) == generation } - ThumbJob::Photos | ThumbJob::Build => true, + ThumbJob::Photos => true, + ThumbJob::Build => !self.thumbnail_build_cancelled.load(Ordering::SeqCst), } } @@ -684,9 +793,29 @@ impl Core { return (uid, ThumbAttempt::Cancelled); } } + _ = tokio::time::sleep(THUMB_CANCEL_POLL) => { + if !core.thumb_job_current(job) { + return (uid, ThumbAttempt::Cancelled); + } + } } }, - ThumbJob::Photos | ThumbJob::Build => acquire.await.ok(), + ThumbJob::Build => loop { + tokio::select! { + permit = &mut acquire => break permit.ok(), + _ = core.thumbnail_build_cancel.notified() => { + if !core.thumb_job_current(job) { + return (uid, ThumbAttempt::Cancelled); + } + } + _ = tokio::time::sleep(THUMB_CANCEL_POLL) => { + if !core.thumb_job_current(job) { + return (uid, ThumbAttempt::Cancelled); + } + } + } + }, + ThumbJob::Photos => acquire.await.ok(), }; let Some(_permit) = _permit else { return (uid, ThumbAttempt::Unavailable); @@ -707,9 +836,29 @@ impl Core { break None; } } + _ = tokio::time::sleep(THUMB_CANCEL_POLL) => { + if !core.thumb_job_current(job) { + break None; + } + } } }, - ThumbJob::Photos | ThumbJob::Build => Some(download.await), + ThumbJob::Build => loop { + tokio::select! { + result = &mut download => break Some(result), + _ = core.thumbnail_build_cancel.notified() => { + if !core.thumb_job_current(job) { + break None; + } + } + _ = tokio::time::sleep(THUMB_CANCEL_POLL) => { + if !core.thumb_job_current(job) { + break None; + } + } + } + }, + ThumbJob::Photos => Some(download.await), }; let Some(downloaded) = downloaded else { return (uid, ThumbAttempt::Cancelled); @@ -726,16 +875,19 @@ impl Core { } // Decoding + scaling a 20 MP JPEG is CPU-bound and would // stall the runtime's worker; hand it to the blocking pool. - let made = - tokio::task::spawn_blocking(move || scale_thumbnail(&bytes, &name)) - .await - .unwrap_or(None); + let staging_dir = core.cache.raw_thumbnail_staging_dir().to_path_buf(); + let made = tokio::task::spawn_blocking(move || { + scale_thumbnail(&bytes, &name, &staging_dir) + }) + .await + .unwrap_or(ScaleAttempt::Unavailable); if !core.thumb_job_current(job) { return (uid, ThumbAttempt::Cancelled); } match made { - Some(thumb) => (uid, ThumbAttempt::Made(thumb)), - None => (uid, ThumbAttempt::Undecodable), + ScaleAttempt::Made(thumb) => (uid, ThumbAttempt::Made(thumb)), + ScaleAttempt::Undecodable => (uid, ThumbAttempt::Undecodable), + ScaleAttempt::Unavailable => (uid, ThumbAttempt::Unavailable), } }); } @@ -760,7 +912,9 @@ impl Core { match self.cache.store_thumbnail(&uid, ttype, tag, &thumb.bytes) { Ok(()) => { summary.made += 1; - self.no_thumbnail.lock().remove(&(uid.clone(), ttype)); + self.thumbnail_misses + .lock() + .forget_local(&(uid.clone(), ttype)); self.record_thumb(&uid, db::THUMB_HAVE, Some(thumb.ratio)); } Err(e) => { @@ -774,7 +928,9 @@ impl Core { ThumbAttempt::Undecodable => { summary.undecodable += 1; if let Some(&tag) = tags.get(&uid.to_string()) { - self.no_thumbnail.lock().insert((uid.clone(), ttype), tag); + self.thumbnail_misses + .lock() + .remember_local((uid.clone(), ttype), tag); } self.record_thumb(&uid, db::THUMB_NONE, None); } @@ -792,20 +948,22 @@ impl Core { self.thumbnail_build.lock().clone() } - /// Start one recursive local-thumbnail build. A second request while it is - /// running attaches to the existing job instead of duplicating every - /// download; the caller polls [`Core::thumbnail_build_status`]. - pub(crate) fn start_thumbnail_build(&self, root: PathBuf) -> ThumbnailBuildStatus { + /// Start one recursive local-thumbnail build. A request for the same root + /// may attach to it; a different root is rejected explicitly instead of + /// silently pretending the existing job belongs to the caller. + pub(crate) fn start_thumbnail_build(&self, root: PathBuf) -> CoreResult { let path = root.to_string_lossy().into_owned(); let status = { let mut status = self.thumbnail_build.lock(); - if status.running { - return status.clone(); + if !thumbnail_build_may_start(&status, &path)? { + return Ok(status.clone()); } + self.thumbnail_build_cancelled + .store(false, Ordering::SeqCst); *status = ThumbnailBuildStatus { running: true, scanning: true, - path, + path: path.clone(), folders_scanned: 0, images_found: 0, completed: 0, @@ -826,18 +984,57 @@ impl Core { status.running = false; status.scanning = false; status.message = Some("Thumbnail generation stopped unexpectedly".to_string()); + core.thumbnail_build_cancelled + .store(false, Ordering::SeqCst); warn!(path = %status.path, "recursive thumbnail build panicked"); } }); + Ok(status) + } + + pub(crate) fn cancel_thumbnail_build(&self) -> ThumbnailBuildStatus { + let (status, cancelled) = { + let status = self.thumbnail_build.lock(); + let cancelled = status.running; + if cancelled { + self.thumbnail_build_cancelled.store(true, Ordering::SeqCst); + } + (status.clone(), cancelled) + }; + if cancelled { + self.thumbnail_build_cancel.notify_waiters(); + } status } + fn finish_thumbnail_build(&self) { + let mut status = self.thumbnail_build.lock(); + let cancelled = self.thumbnail_build_cancelled.swap(false, Ordering::SeqCst); + status.running = false; + status.scanning = false; + if cancelled { + status.message = Some("Thumbnail build cancelled".to_string()); + } + info!( + path = %status.path, + images = status.images_found, + failed = status.failed, + cancelled, + "recursive thumbnail build finished" + ); + } + fn run_thumbnail_build(&self, root: PathBuf) { let mut folders = vec![root]; let mut images: Vec<(NodeUid, i64, String)> = Vec::new(); - let mut seen = HashSet::new(); + let mut seen_folders = HashSet::new(); + let mut seen_images = HashSet::new(); while let Some(folder) = folders.pop() { + if self.thumbnail_build_cancelled.load(Ordering::SeqCst) { + self.finish_thumbnail_build(); + return; + } let listing = self.list_dir(&folder); { let mut status = self.thumbnail_build.lock(); @@ -861,14 +1058,20 @@ impl Core { let mut invalid_here = 0_u64; for entry in entries { if entry.is_dir { - if seen.insert(entry.uid) { + if seen_folders.insert(entry.uid) { folders.push(folder.join(entry.name)); } } else if is_thumbnail_image_name(&entry.name) { - found_here += 1; match parse_uid(&entry.uid) { - Some(uid) => images.push((uid, entry.modified, entry.name)), - None => invalid_here += 1, + Some(uid) if seen_images.insert(uid.clone()) => { + found_here += 1; + images.push((uid, entry.modified, entry.name)); + } + Some(_) => {} + None => { + found_here += 1; + invalid_here += 1; + } } } } @@ -879,6 +1082,10 @@ impl Core { } self.thumbnail_build.lock().scanning = false; + if self.thumbnail_build_cancelled.load(Ordering::SeqCst) { + self.finish_thumbnail_build(); + return; + } let ttype = ThumbnailType::Thumbnail.as_i32(); let mut todo = Vec::new(); for (uid, modified, name) in images { @@ -888,7 +1095,11 @@ impl Core { .is_some() { self.thumbnail_build.lock().completed += 1; - } else if self.no_thumbnail.lock().get(&(uid.clone(), ttype)) == Some(&modified) { + } else if self + .thumbnail_misses + .lock() + .local_contains(&(uid.clone(), ttype), modified) + { let mut status = self.thumbnail_build.lock(); status.completed += 1; status.failed += 1; @@ -898,6 +1109,10 @@ impl Core { } for chunk in todo.chunks(THUMB_BUILD_CHUNK) { + if self.thumbnail_build_cancelled.load(Ordering::SeqCst) { + self.finish_thumbnail_build(); + return; + } let mut claimed = Vec::with_capacity(chunk.len()); let mut tags = HashMap::with_capacity(chunk.len()); let mut names = HashMap::with_capacity(chunk.len()); @@ -905,7 +1120,12 @@ impl Core { // A visible tile may have started this image just before the // toolbar action. Wait for that one bounded job to finish rather // than downloading the full image twice. + let waiting_since = Instant::now(); loop { + if self.thumbnail_build_cancelled.load(Ordering::SeqCst) { + self.finish_thumbnail_build(); + return; + } if self .cache .cached_thumbnail_path(uid, ttype, *modified) @@ -914,7 +1134,11 @@ impl Core { self.thumbnail_build.lock().completed += 1; break; } - if self.no_thumbnail.lock().get(&(uid.clone(), ttype)) == Some(modified) { + if self + .thumbnail_misses + .lock() + .local_contains(&(uid.clone(), ttype), *modified) + { let mut status = self.thumbnail_build.lock(); status.completed += 1; status.failed += 1; @@ -926,6 +1150,17 @@ impl Core { claimed.push(uid.clone()); break; } + if waiting_since.elapsed() >= THUMB_BUILD_CLAIM_TIMEOUT { + let mut status = self.thumbnail_build.lock(); + status.completed += 1; + status.failed += 1; + if status.message.is_none() { + status.message = Some( + "Some images stayed busy and were skipped after a timeout".into(), + ); + } + break; + } std::thread::sleep(Duration::from_millis(50)); } } @@ -955,16 +1190,7 @@ impl Core { } } } - - let mut status = self.thumbnail_build.lock(); - status.running = false; - status.scanning = false; - info!( - path = %status.path, - images = status.images_found, - failed = status.failed, - "recursive thumbnail build finished" - ); + self.finish_thumbnail_build(); } /// Persist what a thumbnail attempt learned about a photo, against the @@ -1092,9 +1318,14 @@ impl Core { #[cfg(test)] mod thumb_tests { - use super::{RawTempFile, THUMB_EDGE, exiftool_binary, ratio_of, scale_thumbnail}; - use pdfs_core::control::is_thumbnail_image_name; + use super::{ + RawTempFile, ScaleAttempt, THUMB_EDGE, exiftool_binary, ratio_of, scale_thumbnail, + scale_thumbnail_with_exiftool, thumbnail_build_may_start, + }; + use pdfs_core::control::{ThumbnailBuildStatus, is_thumbnail_image_name}; + use std::ffi::OsStr; use std::os::unix::fs::PermissionsExt as _; + use std::time::Duration; /// A `width`×`height` JPEG, standing in for a camera photo the server never /// generated a thumbnail for. @@ -1112,7 +1343,10 @@ mod thumb_tests { #[test] fn scaling_fits_the_long_edge_and_keeps_the_aspect_ratio() { let photo = jpeg(4000, 3000); - let thumb = scale_thumbnail(&photo, "photo.jpg").expect("a JPEG scales"); + let ScaleAttempt::Made(thumb) = scale_thumbnail(&photo, "photo.jpg", &std::env::temp_dir()) + else { + panic!("a JPEG scales"); + }; let (width, height) = image::ImageReader::new(std::io::Cursor::new(&thumb.bytes)) .with_guessed_format() @@ -1130,7 +1364,11 @@ mod thumb_tests { #[test] fn a_portrait_photo_fits_its_long_edge_too() { - let thumb = scale_thumbnail(&jpeg(1000, 2000), "portrait.jpeg").expect("a JPEG scales"); + let ScaleAttempt::Made(thumb) = + scale_thumbnail(&jpeg(1000, 2000), "portrait.jpeg", &std::env::temp_dir()) + else { + panic!("a JPEG scales"); + }; assert!(thumb.ratio < 1.0, "portrait stays portrait"); assert_eq!(ratio_of(&thumb.bytes).map(|r| r < 1.0), Some(true)); } @@ -1139,7 +1377,10 @@ mod thumb_tests { fn undecodable_bytes_are_not_a_thumbnail() { // What a photo in a format this build has no decoder for looks like: the // caller writes it off as un-thumbnailable rather than retrying forever. - assert!(scale_thumbnail(b"not an image at all", "broken.jpg").is_none()); + assert!(matches!( + scale_thumbnail(b"not an image at all", "broken.jpg", &std::env::temp_dir()), + ScaleAttempt::Undecodable + )); assert!(ratio_of(b"not an image at all").is_none()); } @@ -1150,9 +1391,11 @@ mod thumb_tests { #[test] fn raw_staging_file_is_private_and_removed_on_drop() { - let file = - RawTempFile::create(b"private camera bytes", "shot.NEF").expect("temporary RAW file"); + let staging_dir = std::env::temp_dir(); + let file = RawTempFile::create_in(&staging_dir, b"private camera bytes", "shot.NEF") + .expect("temporary RAW file"); let path = file.0.clone(); + assert_eq!(path.parent(), Some(staging_dir.as_path())); let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; assert_eq!(mode, 0o600); assert_eq!( @@ -1172,7 +1415,10 @@ mod thumb_tests { let path = std::path::PathBuf::from(path); let name = path.file_name().unwrap().to_string_lossy(); let bytes = std::fs::read(&path).unwrap(); - let thumb = scale_thumbnail(&bytes, &name).expect("embedded RAW preview"); + let ScaleAttempt::Made(thumb) = scale_thumbnail(&bytes, &name, &std::env::temp_dir()) + else { + panic!("embedded RAW preview"); + }; let (width, height) = image::ImageReader::new(std::io::Cursor::new(&thumb.bytes)) .with_guessed_format() .unwrap() @@ -1192,6 +1438,7 @@ mod thumb_tests { "camera.NEF", "negative.cr3", "sensor.orf", + "camera.raw", ] { assert!(is_thumbnail_image_name(name), "{name}"); } @@ -1203,6 +1450,7 @@ mod thumb_tests { "unsupported.heic", "unsupported.avif", "vector.svg", + "camera.rw", ] { assert!(!is_thumbnail_image_name(name), "{name}"); } @@ -1224,4 +1472,30 @@ mod thumb_tests { assert_eq!(exiftool_binary(object, "ThumbnailImage"), None); assert_eq!(exiftool_binary(object, "Missing"), None); } + + #[test] + fn missing_exiftool_is_retryable_not_a_permanent_raw_miss() { + let attempt = scale_thumbnail_with_exiftool( + b"camera bytes that require an embedded preview", + "shot.raw", + &std::env::temp_dir(), + OsStr::new("pdfs-exiftool-deliberately-absent"), + Duration::from_millis(10), + ); + assert!(matches!(attempt, ScaleAttempt::Unavailable)); + } + + #[test] + fn a_running_build_only_accepts_the_same_root() { + let status = ThumbnailBuildStatus { + running: true, + path: "pictures/first".into(), + ..Default::default() + }; + assert!(!thumbnail_build_may_start(&status, "pictures/first").unwrap()); + assert!(thumbnail_build_may_start(&status, "pictures/second").is_err()); + + let idle = ThumbnailBuildStatus::default(); + assert!(thumbnail_build_may_start(&idle, "pictures/second").unwrap()); + } } diff --git a/crates/pdfs-gui/src/app/main.rs b/crates/pdfs-gui/src/app/main.rs index 97c942d..15caf8a 100644 --- a/crates/pdfs-gui/src/app/main.rs +++ b/crates/pdfs-gui/src/app/main.rs @@ -407,6 +407,7 @@ fn build_window(app: &adw::Application) { thumbnail_status: browser_widgets.thumbnail_status.clone(), thumbnail_poll: RefCell::new(None), thumbnail_build_running: Cell::new(false), + thumbnail_cancel_pending: Cell::new(false), search_source: RefCell::new(None), load_generation: Cell::new(0), views: browser_widgets.views.clone(), diff --git a/crates/pdfs-gui/src/app/pages/browser.rs b/crates/pdfs-gui/src/app/pages/browser.rs index 03a675f..b819a16 100644 --- a/crates/pdfs-gui/src/app/pages/browser.rs +++ b/crates/pdfs-gui/src/app/pages/browser.rs @@ -35,6 +35,7 @@ pub(crate) struct BrowserState { pub(crate) thumbnail_status: gtk4::Label, pub(crate) thumbnail_poll: RefCell>, pub(crate) thumbnail_build_running: Cell, + pub(crate) thumbnail_cancel_pending: Cell, /// Pending debounce timer for the search box; replaced on every keystroke so /// only the last pause actually fires a [`Request::Search`]. pub(crate) search_source: RefCell>, @@ -1469,7 +1470,13 @@ pub(crate) fn wire_browser_actions( let ui_uf = ui.clone(); upload_folder.connect_clicked(move |_| prompt_upload_folder(&ui_uf)); let ui_thumbs = ui.clone(); - build_thumbnails.connect_clicked(move |_| start_thumbnail_build(&ui_thumbs)); + build_thumbnails.connect_clicked(move |_| { + if ui_thumbs.browser.thumbnail_build_running.get() { + cancel_thumbnail_build(&ui_thumbs); + } else { + start_thumbnail_build(&ui_thumbs); + } + }); } const THUMBNAIL_BUILD_POLL: Duration = Duration::from_millis(500); @@ -1488,7 +1495,10 @@ fn start_thumbnail_build(ui: &Rc) { } cancel_file_thumbnails(ui); ui.browser.thumbnail_build_running.set(true); - ui.browser.build_thumbnails.set_sensitive(false); + // Do not expose Cancel until the daemon has acknowledged Start; the two + // requests use separate control connections and could otherwise reorder. + ui.browser.thumbnail_cancel_pending.set(true); + repaint_thumbnail_build_action(ui, true); ui.browser.thumbnail_build_row.set_visible(true); ui.browser.thumbnail_progress.set_fraction(0.0); ui.browser @@ -1504,21 +1514,61 @@ fn start_thumbnail_build(ui: &Rc) { glib::spawn_future_local(async move { match rx.recv().await { Ok(Ok(Response::ThumbnailBuild { status })) => { + ui.browser.thumbnail_cancel_pending.set(false); repaint_thumbnail_build(&ui, &status); } Ok(Ok(Response::Error { message, kind })) => { - thumbnail_build_failed(&ui, &message); + thumbnail_build_failed(&ui); toast_failure(&ui, "Couldn't build thumbnails", &message, kind); } _ => { let message = "The mount service didn't respond."; - thumbnail_build_failed(&ui, message); + thumbnail_build_failed(&ui); toast_error(&ui, "Couldn't build thumbnails", message); } } }); } +fn cancel_thumbnail_build(ui: &Rc) { + if ui.browser.thumbnail_cancel_pending.replace(true) { + return; + } + ui.browser.build_thumbnails.set_sensitive(false); + ui.browser + .thumbnail_status + .set_label("Stopping thumbnail build…"); + let rx = spawn_request(ui.dirs.control_socket(), Request::CancelThumbnailBuild); + let ui = ui.clone(); + glib::spawn_future_local(async move { + match rx.recv().await { + Ok(Ok(Response::ThumbnailBuild { status })) if !status.running => { + repaint_thumbnail_build(&ui, &status) + } + Ok(Ok(Response::ThumbnailBuild { .. })) => schedule_thumbnail_build_poll(&ui), + Ok(Ok(Response::Error { message, kind })) => { + thumbnail_cancel_failed(&ui); + toast_failure(&ui, "Couldn't cancel thumbnail build", &message, kind); + } + _ => { + thumbnail_cancel_failed(&ui); + toast_error( + &ui, + "Couldn't cancel thumbnail build", + "The mount service didn't respond.", + ); + } + } + }); +} + +fn thumbnail_cancel_failed(ui: &Rc) { + ui.browser.thumbnail_cancel_pending.set(false); + ui.browser.thumbnail_build_running.set(true); + repaint_thumbnail_build_action(ui, true); + schedule_thumbnail_build_poll(ui); +} + fn schedule_thumbnail_build_poll(ui: &Rc) { if let Some(source) = ui.browser.thumbnail_poll.borrow_mut().take() { source.remove(); @@ -1534,12 +1584,17 @@ fn schedule_thumbnail_build_poll(ui: &Rc) { repaint_thumbnail_build(&ui_result, &status) } Ok(Ok(Response::Error { message, .. })) => { - thumbnail_build_failed(&ui_result, &message) + thumbnail_build_failed(&ui_result); + toast_error(&ui_result, "Thumbnail build stopped", &message); + } + _ => { + thumbnail_build_failed(&ui_result); + toast_error( + &ui_result, + "Thumbnail build stopped", + "The mount service stopped reporting thumbnail progress.", + ); } - _ => thumbnail_build_failed( - &ui_result, - "The mount service stopped reporting thumbnail progress.", - ), } }); }); @@ -1593,13 +1648,12 @@ fn repaint_thumbnail_build(ui: &Rc, status: &ThumbnailBuildStatus) { if status.running { ui.browser.thumbnail_build_running.set(true); - ui.browser.build_thumbnails.set_sensitive(false); + repaint_thumbnail_build_action(ui, true); schedule_thumbnail_build_poll(ui); } else { ui.browser.thumbnail_build_running.set(false); - ui.browser - .build_thumbnails - .set_sensitive(*ui.mounted.borrow()); + ui.browser.thumbnail_cancel_pending.set(false); + repaint_thumbnail_build_action(ui, false); if ui.stack.visible_child_name().as_deref() == Some("browser") { reload_listing(ui); } @@ -1612,15 +1666,32 @@ fn show_thumbnail_build_progress(status: &ThumbnailBuildStatus) -> bool { status.running } -fn thumbnail_build_failed(ui: &Rc, message: &str) { - ui.browser.thumbnail_build_running.set(false); +fn repaint_thumbnail_build_action(ui: &Rc, running: bool) { + ui.browser.build_thumbnails.set_icon_name(if running { + "process-stop-symbolic" + } else { + "pdfs-build-thumbnails-symbolic" + }); ui.browser .build_thumbnails - .set_sensitive(*ui.mounted.borrow()); - ui.browser.thumbnail_build_row.set_visible(true); + .set_tooltip_text(Some(if running { + "Cancel thumbnail build" + } else { + "Build thumbnails in this folder and its subfolders" + })); + ui.browser.build_thumbnails.set_sensitive( + *ui.mounted.borrow() && (!running || !ui.browser.thumbnail_cancel_pending.get()), + ); +} + +fn thumbnail_build_failed(ui: &Rc) { + ui.browser.thumbnail_build_running.set(false); + ui.browser.thumbnail_cancel_pending.set(false); + repaint_thumbnail_build_action(ui, false); + ui.browser.thumbnail_build_row.set_visible(false); ui.browser.thumbnail_progress.set_fraction(0.0); - ui.browser.thumbnail_status.set_label(message); - ui.browser.thumbnail_status.set_tooltip_text(Some(message)); + ui.browser.thumbnail_status.set_label(""); + ui.browser.thumbnail_status.set_tooltip_text(None); } /// Send a mutating request (rename / move / delete / mkdir / upload, or a trash diff --git a/crates/pdfs-gui/src/app/pages/status.rs b/crates/pdfs-gui/src/app/pages/status.rs index 8f7c1a4..2f32734 100644 --- a/crates/pdfs-gui/src/app/pages/status.rs +++ b/crates/pdfs-gui/src/app/pages/status.rs @@ -683,9 +683,11 @@ pub(crate) fn set_mounted(ui: &Rc, mounted: bool) { ui.browser.new_folder.set_sensitive(mounted); ui.browser.upload.set_sensitive(mounted); ui.browser.upload_folder.set_sensitive(mounted); - ui.browser - .build_thumbnails - .set_sensitive(mounted && !ui.browser.thumbnail_build_running.get()); + ui.browser.build_thumbnails.set_sensitive( + mounted + && (!ui.browser.thumbnail_build_running.get() + || !ui.browser.thumbnail_cancel_pending.get()), + ); ui.gallery.upload.set_sensitive(mounted); ui.details.details.pin_row.set_sensitive(mounted); ui.details.details.rename_button.set_sensitive(mounted); diff --git a/crates/pdfs-gui/src/app/widgets/thumbnails.rs b/crates/pdfs-gui/src/app/widgets/thumbnails.rs index f69614b..d6124ab 100644 --- a/crates/pdfs-gui/src/app/widgets/thumbnails.rs +++ b/crates/pdfs-gui/src/app/widgets/thumbnails.rs @@ -7,7 +7,6 @@ use crate::*; use pdfs_core::control::{FileThumbRequest, is_thumbnail_image_name}; -use std::time::{SystemTime, UNIX_EPOCH}; const THUMB_BATCH: usize = 32; const THUMB_CACHE_MAX: usize = 256; @@ -25,6 +24,27 @@ pub(crate) struct FileThumbnailState { inflight: Cell, source: RefCell>, generation: Cell, + generation_loading: Cell, +} + +#[derive(Debug, PartialEq, Eq)] +enum ReplyDisposition { + Ignore, + Load, + Retry, + Missing, +} + +fn reply_disposition(wanted: bool, has_path: bool, pending: bool) -> ReplyDisposition { + if !wanted { + ReplyDisposition::Ignore + } else if has_path { + ReplyDisposition::Load + } else if pending { + ReplyDisposition::Retry + } else { + ReplyDisposition::Missing + } } impl FileThumbnailState { @@ -38,13 +58,8 @@ impl FileThumbnailState { queue: RefCell::new(VecDeque::new()), inflight: Cell::new(false), source: RefCell::new(None), - generation: Cell::new( - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() - .min(u128::from(u64::MAX)) as u64, - ), + generation: Cell::new(0), + generation_loading: Cell::new(false), } } @@ -84,14 +99,17 @@ pub(crate) fn cancel_file_thumbnails(ui: &Rc) { } ui.file_thumbs.queue.borrow_mut().clear(); ui.file_thumbs.wanted.borrow_mut().clear(); - let generation = ui.file_thumbs.generation.get().saturating_add(1); - ui.file_thumbs.generation.set(generation); + let generation = ui.file_thumbs.generation.replace(0); // The request is useful even when its reply is not. `spawn_request` owns the // socket round-trip on a worker thread, so dropping the receiver is safe. - drop(spawn_request( - ui.dirs.control_socket(), - Request::CancelFileThumbs { generation }, - )); + if generation != 0 { + drop(spawn_request( + ui.dirs.control_socket(), + Request::CancelFileThumbs { + generation: generation.saturating_add(1), + }, + )); + } } /// A square thumbnail surface with a generic file icon underneath the picture. @@ -244,6 +262,10 @@ fn schedule_file_thumbs(ui: &Rc) { if ui.file_thumbs.queue.borrow().is_empty() || ui.file_thumbs.inflight.get() { return; } + if ui.file_thumbs.generation.get() == 0 { + reserve_file_thumb_generation(ui); + return; + } if let Some(source) = ui.file_thumbs.source.borrow_mut().take() { source.remove(); } @@ -255,6 +277,40 @@ fn schedule_file_thumbs(ui: &Rc) { *ui.file_thumbs.source.borrow_mut() = Some(source); } +fn reserve_file_thumb_generation(ui: &Rc) { + if ui.file_thumbs.generation_loading.replace(true) { + return; + } + let rx = spawn_request( + ui.dirs.control_socket(), + Request::ReserveFileThumbGeneration, + ); + let ui = ui.clone(); + glib::spawn_future_local(async move { + let result = rx.recv().await; + ui.file_thumbs.generation_loading.set(false); + match result { + Ok(Ok(Response::FileThumbGeneration { generation })) if generation != 0 => { + ui.file_thumbs.generation.set(generation); + schedule_file_thumbs(&ui); + } + _ => retry_file_thumb_generation(&ui), + } + }); +} + +fn retry_file_thumb_generation(ui: &Rc) { + if let Some(source) = ui.file_thumbs.source.borrow_mut().take() { + source.remove(); + } + let ui_retry = ui.clone(); + let source = glib::timeout_add_local_once(THUMB_RETRY, move || { + ui_retry.file_thumbs.source.borrow_mut().take(); + schedule_file_thumbs(&ui_retry); + }); + *ui.file_thumbs.source.borrow_mut() = Some(source); +} + fn flush_file_thumbs(ui: &Rc) { if ui.file_thumbs.inflight.get() { return; @@ -291,51 +347,63 @@ fn flush_file_thumbs(ui: &Rc) { ui.file_thumbs.inflight.set(false); match result { Ok(Ok(Response::Thumbs { items: replies })) => { - let requested: HashMap = items + let mut requested: HashMap = items .iter() .cloned() .map(|item| (item.uid.clone(), item)) .collect(); let mut pending = Vec::new(); for reply in replies { - let Some(request) = requested.get(&reply.uid) else { + let Some(request) = requested.remove(&reply.uid) else { continue; }; let key = thumbnail_key(&request.uid, request.modified); - match reply.path { - Some(path) => match gtk4::gdk::Texture::from_filename(&path) { - Ok(texture) => { - ui.file_thumbs.store_texture(&key, texture.clone()); - if let Some(pictures) = - ui.file_thumbs.wanted.borrow_mut().remove(&key) - { - for picture in pictures { - paint(&picture, &key, &texture); + let wanted = ui.file_thumbs.wanted.borrow().contains_key(&key); + match reply_disposition(wanted, reply.path.is_some(), reply.pending) { + ReplyDisposition::Ignore => continue, + ReplyDisposition::Load => { + let Some(path) = reply.path else { continue }; + match gtk4::gdk::Texture::from_filename(&path) { + Ok(texture) => { + ui.file_thumbs.store_texture(&key, texture.clone()); + if let Some(pictures) = + ui.file_thumbs.wanted.borrow_mut().remove(&key) + { + for picture in pictures { + paint(&picture, &key, &texture); + } } } + Err(e) => { + tracing::debug!("cannot decode file thumbnail {path}: {e}"); + ui.file_thumbs.store_missing(key.clone()); + ui.file_thumbs.wanted.borrow_mut().remove(&key); + } } - Err(e) => { - tracing::debug!("cannot decode file thumbnail {path}: {e}"); - ui.file_thumbs.store_missing(key.clone()); - ui.file_thumbs.wanted.borrow_mut().remove(&key); - } - }, - None if reply.pending => pending.push(request.clone()), - None => { + } + ReplyDisposition::Retry => pending.push(request), + ReplyDisposition::Missing => { ui.file_thumbs.store_missing(key.clone()); ui.file_thumbs.wanted.borrow_mut().remove(&key); } } } + pending.extend(requested.into_values()); if !pending.is_empty() { retry_file_thumbs(&ui, pending); } } + Ok(Ok(Response::FileThumbsStale)) => { + ui.file_thumbs.generation.set(0); + retry_file_thumbs(&ui, items); + } Ok(Ok(Response::Error { message, .. })) => { tracing::debug!("file thumbnails failed: {message}"); + retry_file_thumbs(&ui, items); } Ok(Ok(_)) | Ok(Err(_)) | Err(_) => { tracing::debug!("file thumbnails: no reply"); + retry_file_thumbs(&ui, items); } } schedule_file_thumbs(&ui); @@ -358,7 +426,7 @@ fn retry_file_thumbs(ui: &Rc, items: Vec) { #[cfg(test)] mod tests { - use super::is_image_name; + use super::{ReplyDisposition, is_image_name, reply_disposition}; #[test] fn image_extensions_are_case_insensitive_and_specific() { @@ -369,6 +437,7 @@ mod tests { "archive.tiff", "camera.nef", "negative.CR3", + "camera.RAW", ] { assert!(is_image_name(name), "{name}"); } @@ -380,8 +449,21 @@ mod tests { "camera.heic", "frame.avif", "graphic.svg", + "camera.rw", ] { assert!(!is_image_name(name), "{name}"); } } + + #[test] + fn cancelled_listing_replies_cannot_become_permanent_misses() { + assert_eq!( + reply_disposition(false, false, false), + ReplyDisposition::Ignore + ); + assert_eq!( + reply_disposition(true, false, false), + ReplyDisposition::Missing + ); + } }