Merge PR #3392: LRC-LIB-MISSING: settling waits for the Missing Photos count, which can't panic on spawn

crates/ui-egui/src/app.rs

@@ -64,6 +64,8 @@ impl LightcraftApp {
             ui_passes: 0,
             #[cfg(test)]
             library_passes: vec![],
+            #[cfg(test)]
+            missing_count_test_delay: None,
             shadow: None,
             synthetic: vec![],
             pointer_cleanup: false,

crates/ui-egui/src/lib.rs

@@ -216,6 +216,10 @@ pub struct LightcraftApp {
     /// effects (which the global shortcut guard also hides).
     #[cfg(test)]
     pub(crate) library_passes: Vec<panels::library_problem::LibraryPass>,
+    /// Test-only: a delay for the sidebar's Missing Photos count worker (a slow disk). Kept here,
+    /// not in `ui`, because `ui.set` rebuilds `ui` and would drop it.
+    #[cfg(test)]
+    pub(crate) missing_count_test_delay: Option<std::time::Duration>,
     /// Offscreen context for headless screenshots of the windowed app.
     shadow: Option<headless::HeadlessView>,
     /// Synthetic input events (from the control channel) injected one step per frame.

crates/ui-egui/src/panels/left/local.rs

@@ -224,14 +224,17 @@ fn list_subfolders(path: &str) -> std::sync::Arc<Vec<(String, String)>> {
     v.into()
 }
 
-/// How many [`fs_cached`] answers for `ctx` are being worked out right now. Rows appear (and
-/// the sidebar below them moves) when they land, so the headless driver counts them as pending
-/// work and waits for them before acting on widget positions.
+/// How many file-system answers for `ctx` are being worked out right now: [`fs_cached`]'s and the
+/// Missing Photos count's. Rows appear (and the sidebar below them moves) when they land, so the
+/// headless driver counts them as pending work and waits for them before acting on widget
+/// positions.
 pub(crate) fn fs_cached_running(ctx: &egui::Context) -> usize {
     fs_running_counter(ctx).load(std::sync::atomic::Ordering::Acquire)
 }
 
-fn fs_running_counter(ctx: &egui::Context) -> std::sync::Arc<std::sync::atomic::AtomicUsize> {
+/// The counter behind [`fs_cached_running`]: a worker adds one before it starts and takes it off
+/// after its answer is stored.
+pub(super) fn fs_running_counter(ctx: &egui::Context) -> std::sync::Arc<std::sync::atomic::AtomicUsize> {
     ctx.data_mut(|d| d.get_temp_mut_or_default::<std::sync::Arc<std::sync::atomic::AtomicUsize>>(egui::Id::unique("fs-cached-running")).clone())
 }
 

crates/ui-egui/src/panels/left/sections.rs

@@ -381,36 +381,77 @@ fn new_album_menu(app: &mut LightcraftApp, ui: &mut egui::Ui) {
 /// How many library photos have no file (Local browse records are not checked; see
 /// `cmd::missing::checked_path`). Checking stats every file, which on a network share takes
 /// seconds, so it runs on a worker thread: the count shown is the last finished one, refreshed
-/// at most every 5 s, and at once (after the running check) when the catalog changed.
+/// at most every 5 s, and at once (after the running check) when the catalog changed. The first
+/// count and a recount after a catalog change are pending sidebar work
+/// ([`super::fs_cached_running`]): the Missing Photos row they add or take away moves every row
+/// below it. The timed refresh is not: it rarely changes the count, and on a share where counting
+/// takes seconds, a headless run's fast simulated clock would start the next one before settling.
+/// A catalog change during a timed refresh is pending from the frame it is seen, although its
+/// recount starts only when the refresh ends.
 pub(super) fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize {
-    use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering::Relaxed};
+    use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering::AcqRel, Ordering::Relaxed};
     #[derive(Clone, Default)]
     struct Job {
         n: std::sync::Arc<AtomicUsize>,
         running: std::sync::Arc<AtomicBool>,
         at: f64,
         rev: u64,
+        /// A catalog change arrived while a count was running: its recount is pending work
+        /// already, held until the recount stores its result.
+        owed: bool,
     }
     let id = egui::Id::unique("missing-count");
     let now = ui.input(|i| i.time);
     let rev = app.session.catalog.revision;
     let mut job: Job = ui.data(|d| d.get_temp(id)).unwrap_or(Job { rev: u64::MAX, at: f64::MIN, ..Default::default() });
+    let running = job.running.load(Relaxed);
+    if running && job.rev != rev && !job.owed {
+        // the recount waits for the running count (a timed refresh, not waited for), but settling
+        // must wait for it from now on
+        super::local::fs_running_counter(ui.ctx()).fetch_add(1, AcqRel);
+        job.owed = true;
+        ui.data_mut(|d| d.insert_temp(id, job.clone()));
+    }
     let fresh = now - job.at < 5.0 && job.rev == rev;
-    if !fresh && !job.running.load(Relaxed) {
+    if !fresh && !running {
         // the same scope as the Missing Photos view: library photos only, never Local browse records
         let paths = lightcraft_engine::cmd::missing::candidates(&app.session.catalog);
+        let pending = (job.rev != rev).then(|| super::local::fs_running_counter(ui.ctx()));
+        if let Some(p) = &pending
+            && !job.owed
+        {
+            p.fetch_add(1, AcqRel);
+        }
+        // the recount takes over the entry held while it waited
+        job.owed = false;
         job.running.store(true, Relaxed);
         job.at = now;
         job.rev = rev;
-        let (n, running, ctx) = (job.n.clone(), job.running.clone(), ui.ctx().clone());
+        #[cfg(test)]
+        let test_delay = app.missing_count_test_delay;
+        let (n, running, ctx, done) = (job.n.clone(), job.running.clone(), ui.ctx().clone(), pending.clone());
         let work = move || {
+            #[cfg(test)]
+            if let Some(d) = test_delay {
+                std::thread::sleep(d);
+            }
             let missing = if cfg!(target_arch = "wasm32") { 0 } else { paths.iter().filter(|p| !std::path::Path::new(p).exists()).count() };
             n.store(missing, Relaxed);
             running.store(false, Relaxed);
+            // after the count is stored: a frame that sees the pending count drop also sees the row
+            if let Some(d) = done {
+                d.fetch_sub(1, AcqRel);
+            }
             ctx.request_repaint();
         };
         #[cfg(not(target_arch = "wasm32"))]
-        std::thread::spawn(work);
+        if std::thread::Builder::new().name("lc-missing-count".into()).spawn(work).is_err() {
+            // no thread to spare: keep the last count and try again after the next 5 s
+            job.running.store(false, Relaxed);
+            if let Some(p) = &pending {
+                p.fetch_sub(1, AcqRel);
+            }
+        }
         #[cfg(target_arch = "wasm32")]
         work();
         ui.data_mut(|d| d.insert_temp(id, job.clone()));

crates/ui-egui/src/tests/panels.rs

@@ -1495,6 +1495,11 @@ fn folders_app_sized(paths: &[&str], size: [f32; 2]) -> Headless {
 }
 
 fn folders_app_with_services(paths: &[&str], size: [f32; 2], services: Services) -> Headless {
+    folders_app_prepared(paths, size, services, |_| {})
+}
+
+/// [`folders_app_with_services`], with `prepare` run on the app before its first frame.
+fn folders_app_prepared(paths: &[&str], size: [f32; 2], services: Services, prepare: impl FnOnce(&mut LightcraftApp)) -> Headless {
     use lightcraft_catalog::{Op, Photo, Source};
     let mut session = lightcraft_engine::Session::new();
     for path in paths {
@@ -1502,7 +1507,8 @@ fn folders_app_with_services(paths: &[&str], size: [f32; 2], services: Services)
         let p = Photo::new(id, Source::File { path: (*path).into() }, "x.jpg", "JPEG", 60, 40, "2026-01-01T10:00:00");
         session.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
     }
-    let app = LightcraftApp::new(session, services);
+    let mut app = LightcraftApp::new(session, services);
+    prepare(&mut app);
     let mut h = Headless::new(app, size, 1.0);
     let r = h.request("ui.set", json!({"view": "photoGrid", "leftPanel": true}), T);
     assert_eq!(r["ok"], true, "{r}");
@@ -1857,6 +1863,55 @@ fn a_folder_menu_locates_its_missing_tree_only() {
     assert!(sources.contains(&format!("{old}/other/c.jpg")), "another missing folder stays untouched: {sources:?}");
 }
 
+/// The Missing Photos row appears when the background count of missing files lands, and moves
+/// every row below it down. Settling waits for that count, so a click aimed at a folder row from
+/// the settled layout doesn't land on the row above it (on a busy machine, Locate Missing Files…
+/// above ran for `other` instead of `trip`).
+#[test]
+fn settling_waits_for_the_missing_photos_count() {
+    // longer than settling takes without the count (a few hundred ms here)
+    let delay = Duration::from_secs(1);
+    let paths = ["/lc-missing-count/other/a.jpg", "/lc-missing-count/trip/b.jpg"];
+    let services = Services { png: None, ..Default::default() };
+    let mut h = folders_app_prepared(&paths, [1400.0, 900.0], services, |app| app.missing_count_test_delay = Some(delay));
+    assert!(has(&h, "source:missing"), "the Missing Photos row is on screen once the sidebar settled");
+    click(&mut h, "libraryFolderToggle:/lc-missing-count");
+    let settled = widget(&h, "source:libfolder:/lc-missing-count/trip");
+    let t0 = std::time::Instant::now();
+    while t0.elapsed() < delay / 4 {
+        h.step();
+        std::thread::sleep(Duration::from_millis(5));
+    }
+    assert_eq!(widget(&h, "source:libfolder:/lc-missing-count/trip"), settled, "nothing moves the folder rows after settling");
+    // the timed refresh of a slow count is not waited for: the fast simulated clock would start
+    // the next one before the last finished, and settling would never end
+    assert!(h.settle(Duration::from_secs(30)), "the sidebar settles while a slow count refreshes");
+}
+
+/// A catalog change while a slow timed refresh of the count runs: its recount starts only when
+/// the refresh ends, but settling waits for it from the frame the change is seen, so the Missing
+/// Photos row it adds is on screen once the sidebar settled.
+#[test]
+fn settling_waits_for_a_recount_owed_during_a_timed_refresh() {
+    use lightcraft_catalog::{Op, Photo, Source};
+    let dir = std::env::temp_dir().join(format!("lc-ui-missing-owed-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let here = dir.join("a.jpg");
+    std::fs::write(&here, b"photo").unwrap();
+    // settling takes the first count (2 s), in which the simulated clock passes the 5 s refresh
+    // interval: a timed refresh is running when it returns
+    let delay = Duration::from_secs(2);
+    let services = Services { png: None, ..Default::default() };
+    let mut h = folders_app_prepared(&[&here.to_string_lossy()], [1400.0, 900.0], services, |app| app.missing_count_test_delay = Some(delay));
+    assert!(!has(&h, "source:missing"), "no photo is missing yet");
+    let id = h.app.session.catalog.alloc_photo_id();
+    let gone = Photo::new(id, Source::File { path: "/lc-missing-owed/gone.jpg".into() }, "gone.jpg", "JPEG", 60, 40, "2026-01-01T10:00:00");
+    h.app.session.catalog.apply(Op::AddPhoto { photo: Box::new(gone) }).unwrap();
+    assert!(h.settle(Duration::from_secs(30)), "the sidebar settles");
+    assert!(has(&h, "source:missing"), "the Missing Photos row is on screen once the sidebar settled");
+    let _ = std::fs::remove_dir_all(dir);
+}
+
 /// Right-click a folder ▸ Set Color Label ▸ a colour: the folder's row shows the label's dot (a
 /// row without a label shows none), and None takes it off again.
 #[test]