From 70a2109de2f7dbeb4fa4b759e235952afa3e3daf Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Fri, 23 Oct 2026 19:04:24 -0700 Subject: [PATCH 1/2] LR-VIEW-PHOTOGRID: a thumbnail that can't be read is tried again only when its photo changes (#2682) The renderer remembered a failed render until the catalog revision moved, and any op moved it: face results and metadata reads for other photos at launch sent every undecodable file back to the decoder (issue #2682: 37 tries each in 154 s of idle). Renderer::follow_catalog asks the catalog which photos changed (touched_since) and keeps a failure parked unless its own photo is among them; when the catalog can't say, every failure is retried as before. The failure record keeps its shape. Test: render::thumbnail_tests::a_failed_thumbnail_is_retried_only_when_its_photo_changes (fails on main at the "another photo's change retries nothing" check). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/render.rs | 61 ++++++++++++++++++++++++++++++++++-- docs/whats-new.md | 7 +++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/render.rs b/crates/ui-egui/src/render.rs index 1ed1071c2..d6cd35a9c 100644 --- a/crates/ui-egui/src/render.rs +++ b/crates/ui-egui/src/render.rs @@ -229,7 +229,7 @@ pub struct Renderer { /// asks every frame. failed: HashMap, /// The catalog revision seen at the last poll: a failure is retried once the catalog changes - /// (a relinked or re-imported file). + /// its photo (a relinked or re-imported file), see [`Self::follow_catalog`]. catalog_rev: u64, /// Prefetch slot → key of the last job submitted (each runs once). prefetched: HashMap, @@ -674,11 +674,30 @@ impl Renderer { } /// Whether `slot`'s pending render asks again for the key that last failed (a library change - /// retries failures): what is on screen stays what the failure left until it answers. + /// to its photo retries it): what is on screen stays what the failure left until it answers. pub fn retrying(&self, slot: Slot) -> bool { self.pending.get(&slot).zip(self.failed.get(&slot)).is_some_and(|(p, f)| p.0 == f.0) } + /// Follow the library to `catalog`'s revision. A failure stands while its photo is unchanged: + /// only the photos the ops since the last poll touched are tried again, and every photo when + /// the catalog can't say which. So faces found or metadata written for other photos don't + /// send each undecodable file back to the decoder (issue #2682: 37 tries each in 154 s idle). + fn follow_catalog(&mut self, catalog: &lightcraft_catalog::Catalog) { + let rev = catalog.revision; + if rev == self.catalog_rev { + return; + } + if let Some(touched) = catalog.touched_since(self.catalog_rev) { + for f in self.failed.values_mut() { + if f.2 == self.catalog_rev && touched.binary_search(&f.3).is_err() { + f.2 = rev; + } + } + } + self.catalog_rev = rev; + } + /// The slots with a render pending (diagnostics: `ui.inspect`). pub fn pending_slots(&self) -> Vec { self.pending.keys().map(|s| format!("{s:?}")).collect() @@ -775,7 +794,7 @@ impl Renderer { self.forget_all(); } self.preview_generation = Some((Arc::downgrade(cache), generation)); - self.catalog_rev = session.catalog.revision; + self.follow_catalog(&session.catalog); self.frame += 1; self.trim_stages(); // wasm: run one job per frame on this thread, timed with the host clock @@ -1228,6 +1247,42 @@ mod thumbnail_tests { assert!(!r.thumb_current(&p, 384, 10)); } + /// Issue #2682: writes to other photos in the background (faces found, metadata read) moved + /// the catalog revision on and sent every undecodable file back to the decoder, 37 times each + /// in 154 s of idle. A failure is tried again only when its own photo changes, or when the + /// catalog can't say which photos did. + #[test] + fn a_failed_thumbnail_is_retried_only_when_its_photo_changes() { + let mut catalog = Catalog::default(); + let p = photo(); + let other = PhotoId(2); + catalog.apply(Op::AddPhoto { photo: Box::new((*p).clone()) }).unwrap(); + let other_photo = Photo::new(other, Source::File { path: "other.jpg".into() }, "other.jpg", "JPEG", 60, 40, "2026-01-01"); + catalog.apply(Op::AddPhoto { photo: Box::new(other_photo) }).unwrap(); + let slot = Slot::Thumb(p.id); + let mut r = Renderer::default(); + r.follow_catalog(&catalog); + r.failed.insert(slot, (77, "unrecognised image format".into(), r.catalog_rev, p.id)); + assert!(!r.request_needed(slot, 77, 10), "a failure stands"); + // the other photo changes, op after op, as background work does + for rating in 1..=5 { + catalog.apply(Op::SetRating { id: other, rating }).unwrap(); + r.follow_catalog(&catalog); + assert!(!r.request_needed(slot, 77, 10), "another photo's change (rating {rating}) retries nothing"); + } + // a change to the photo itself is a reason to try again + catalog.apply(Op::SetRating { id: p.id, rating: 2 }).unwrap(); + r.follow_catalog(&catalog); + assert!(r.request_needed(slot, 77, 10), "its own photo changed"); + // so is a change the catalog can't attribute to photos + r.failed.insert(slot, (77, "unrecognised image format".into(), r.catalog_rev, p.id)); + assert!(!r.request_needed(slot, 77, 10)); + let album = catalog.alloc_album_id(); + catalog.apply(Op::AddAlbum { album: lightcraft_catalog::Album::new(album, "Trip") }).unwrap(); + r.follow_catalog(&catalog); + assert!(r.request_needed(slot, 77, 10), "which photos changed is unknown"); + } + #[test] fn a_fast_draft_is_drawn_in_the_frame_that_asked_for_it() { let mut s = Session::new(); diff --git a/docs/whats-new.md b/docs/whats-new.md index c09dddc09..1d8c0ed1d 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -8,6 +8,13 @@ Before, the field took the focus back every frame, so only Return and Esc got you out of it. Return still confirms and Esc still cancels. +### A thumbnail that can't be read is tried again only when its photo changes +- A file the grid can't draw (a damaged JPEG, a file the decoders can't read) was decoded again after every change to + the library, even a change to another photo: face recognition and metadata reads at launch made each such file + fail 20 to 40 times in a few minutes, one log warning each time (issue #2682). It is now tried again when that photo + changes (Locate Missing File, Reload, an edit) or when the library can't say which photos changed. A file that comes + back on its own, such as a drive plugged in again, is tried again at the next launch or when its photo is touched. + ### Foreign XMP metadata survives namespace-prefix collisions - Saving a sidecar keeps another application's properties even when its namespace uses prefixes such as `lc` or `xmp`. Those foreign properties no longer become LightCraft settings or ratings when imported. From e4014b700c6d51f0d7663050a90e800430277d17 Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Fri, 23 Oct 2026 19:39:30 -0700 Subject: [PATCH 2/2] LR-VIEW-PHOTOGRID: a failure whose file is missing is still tried at every library change (#2682) Parking every failure until its photo changed also parked photos whose drive was unplugged: on main they came back at the next library change once the drive did, and with the first commit only at the next launch. follow_catalog now asks the availability cache (non-blocking in the app) about the failures it would park, and leaves one whose file is known to be missing to be retried as before. Undecodable files that are there stay parked. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/render.rs | 35 ++++++++++++++++++++++++++--------- docs/whats-new.md | 5 +++-- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/render.rs b/crates/ui-egui/src/render.rs index d6cd35a9c..751b41b85 100644 --- a/crates/ui-egui/src/render.rs +++ b/crates/ui-egui/src/render.rs @@ -683,14 +683,18 @@ impl Renderer { /// only the photos the ops since the last poll touched are tried again, and every photo when /// the catalog can't say which. So faces found or metadata written for other photos don't /// send each undecodable file back to the decoder (issue #2682: 37 tries each in 154 s idle). - fn follow_catalog(&mut self, catalog: &lightcraft_catalog::Catalog) { + /// A file `files` knows to be missing (its drive unplugged) is still tried at every change, so + /// its photo comes back with the drive, as before. + fn follow_catalog(&mut self, catalog: &lightcraft_catalog::Catalog, files: &lightcraft_engine::availability::Availability) { let rev = catalog.revision; if rev == self.catalog_rev { return; } if let Some(touched) = catalog.touched_since(self.catalog_rev) { + let missing = + |id| matches!(catalog.photo(id).map(|p| &p.source), Some(lightcraft_catalog::Source::File { path }) if files.is_offline(path)); for f in self.failed.values_mut() { - if f.2 == self.catalog_rev && touched.binary_search(&f.3).is_err() { + if f.2 == self.catalog_rev && touched.binary_search(&f.3).is_err() && !missing(f.3) { f.2 = rev; } } @@ -794,7 +798,7 @@ impl Renderer { self.forget_all(); } self.preview_generation = Some((Arc::downgrade(cache), generation)); - self.follow_catalog(&session.catalog); + self.follow_catalog(&session.catalog, &session.media.availability); self.frame += 1; self.trim_stages(); // wasm: run one job per frame on this thread, timed with the host clock @@ -1249,38 +1253,51 @@ mod thumbnail_tests { /// Issue #2682: writes to other photos in the background (faces found, metadata read) moved /// the catalog revision on and sent every undecodable file back to the decoder, 37 times each - /// in 154 s of idle. A failure is tried again only when its own photo changes, or when the - /// catalog can't say which photos did. + /// in 154 s of idle. A failure is tried again only when its own photo changes, when the + /// catalog can't say which photos did, or, at any change, when its file is known to be missing. #[test] fn a_failed_thumbnail_is_retried_only_when_its_photo_changes() { + use std::sync::atomic::{AtomicBool, Ordering}; let mut catalog = Catalog::default(); let p = photo(); let other = PhotoId(2); catalog.apply(Op::AddPhoto { photo: Box::new((*p).clone()) }).unwrap(); let other_photo = Photo::new(other, Source::File { path: "other.jpg".into() }, "other.jpg", "JPEG", 60, 40, "2026-01-01"); catalog.apply(Op::AddPhoto { photo: Box::new(other_photo) }).unwrap(); + // the files are there (an undecodable file is still a file) + let here = Arc::new(AtomicBool::new(true)); + let mut files = lightcraft_engine::availability::Availability::default(); + let h = here.clone(); + files.set_probe(Arc::new(move |_| h.load(Ordering::SeqCst))); let slot = Slot::Thumb(p.id); let mut r = Renderer::default(); - r.follow_catalog(&catalog); + r.follow_catalog(&catalog, &files); r.failed.insert(slot, (77, "unrecognised image format".into(), r.catalog_rev, p.id)); assert!(!r.request_needed(slot, 77, 10), "a failure stands"); // the other photo changes, op after op, as background work does for rating in 1..=5 { catalog.apply(Op::SetRating { id: other, rating }).unwrap(); - r.follow_catalog(&catalog); + r.follow_catalog(&catalog, &files); assert!(!r.request_needed(slot, 77, 10), "another photo's change (rating {rating}) retries nothing"); } // a change to the photo itself is a reason to try again catalog.apply(Op::SetRating { id: p.id, rating: 2 }).unwrap(); - r.follow_catalog(&catalog); + r.follow_catalog(&catalog, &files); assert!(r.request_needed(slot, 77, 10), "its own photo changed"); // so is a change the catalog can't attribute to photos r.failed.insert(slot, (77, "unrecognised image format".into(), r.catalog_rev, p.id)); assert!(!r.request_needed(slot, 77, 10)); let album = catalog.alloc_album_id(); catalog.apply(Op::AddAlbum { album: lightcraft_catalog::Album::new(album, "Trip") }).unwrap(); - r.follow_catalog(&catalog); + r.follow_catalog(&catalog, &files); assert!(r.request_needed(slot, 77, 10), "which photos changed is unknown"); + // a file known to be missing (its drive unplugged) is tried at every change, as before, so + // the photo comes back with its drive + r.failed.insert(slot, (77, "file not found".into(), r.catalog_rev, p.id)); + here.store(false, Ordering::SeqCst); + catalog.apply(Op::SetRating { id: other, rating: 1 }).unwrap(); + r.follow_catalog(&catalog, &files); + assert!(r.request_needed(slot, 77, 10), "a missing file is tried again"); } #[test] diff --git a/docs/whats-new.md b/docs/whats-new.md index 1d8c0ed1d..73d9290f8 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -12,8 +12,9 @@ - A file the grid can't draw (a damaged JPEG, a file the decoders can't read) was decoded again after every change to the library, even a change to another photo: face recognition and metadata reads at launch made each such file fail 20 to 40 times in a few minutes, one log warning each time (issue #2682). It is now tried again when that photo - changes (Locate Missing File, Reload, an edit) or when the library can't say which photos changed. A file that comes - back on its own, such as a drive plugged in again, is tried again at the next launch or when its photo is touched. + changes (Locate Missing File, Reload, an edit) or when the library can't say which photos changed. A photo whose + file is missing, such as one on a drive that is unplugged, is still tried again at every change, so it comes back + once the drive does. ### Foreign XMP metadata survives namespace-prefix collisions - Saving a sidecar keeps another application's properties even when its namespace uses prefixes such as `lc`