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.