Merge PR #2764: LR-VIEW-PHOTOGRID: a thumbnail that can't be read is tried again only when its photo changes (#2682)

crates/ui-egui/src/render.rs

@@ -252,7 +252,7 @@ pub struct Renderer {
     /// asks every frame.
     failed: HashMap<Slot, (u64, String, u64, PhotoId)>,
     /// 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<Slot, u64>,
@@ -801,11 +801,34 @@ 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).
+    /// 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() && !missing(f.3) {
+                    f.2 = rev;
+                }
+            }
+        }
+        self.catalog_rev = rev;
+    }
+
     /// The slots with a render pending (diagnostics: `ui.inspect`).
     pub fn pending_slots(&self) -> Vec<String> {
         self.pending.keys().map(|s| format!("{s:?}")).collect()
@@ -902,7 +925,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, &session.media.availability);
         self.frame += 1;
         self.trim_stages();
         // wasm: run one job per frame on this thread, timed with the host clock

crates/ui-egui/src/render/thumbnail_tests.rs

@@ -218,6 +218,55 @@ fn larger_texture_for_another_key_does_not_count_as_current() {
     assert!(r.thumb_current(&p, 256, 10), "the recorded key's larger texture serves the smaller bucket");
 }
 
+/// 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, 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, &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, &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, &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, &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]
 fn a_fast_draft_is_drawn_in_the_frame_that_asked_for_it() {
     let mut s = Session::new();

docs/whats-new.md

@@ -38,6 +38,14 @@
   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 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`
   or `xmp`. Those foreign properties no longer become LightCraft settings or ratings when imported.