From 142ddcff29838603fa153bc1497457f10becfb75 Mon Sep 17 00:00:00 2001 From: mpratsaki <110930793+mpratsaki@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:05:18 +0200 Subject: [PATCH] LRC-LIB-MISSING: the Missing Photos count is pending sidebar work The sidebar's Missing Photos row appears when the background count of missing files lands, and moves every row below it down. That worker was not counted in fs_cached_running, so Headless::settle() did not wait for it: on a busy machine the row appeared after settling, and a click aimed at a folder row from the settled layout landed on the row above it (a_folder_menu_locates_its_missing_tree_only located `other` instead of `trip`). - missing_count adds itself to the fs_cached_running counter before it starts and takes itself off after the count is stored, as fs_cached does - the worker starts with thread::Builder::spawn instead of thread::spawn, which panics when the OS refuses a thread; on failure the last count stays and the next refresh tries again - settling_waits_for_the_missing_photos_count delays the worker by 1 s (test-only field on LightcraftApp, which ui.set does not rebuild) and fails without the fix Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/app.rs | 2 ++ crates/ui-egui/src/lib.rs | 4 +++ crates/ui-egui/src/panels/left/local.rs | 11 +++++--- crates/ui-egui/src/panels/left/sections.rs | 24 ++++++++++++++--- crates/ui-egui/src/tests/panels.rs | 30 +++++++++++++++++++++- 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index fa10210f0..f93020281 100644 --- a/crates/ui-egui/src/app.rs +++ b/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, diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8bf57b341..d1c1d342d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/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, + /// 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, /// Offscreen context for headless screenshots of the windowed app. shadow: Option, /// Synthetic input events (from the control channel) injected one step per frame. diff --git a/crates/ui-egui/src/panels/left/local.rs b/crates/ui-egui/src/panels/left/local.rs index 1308059c8..744ca422a 100644 --- a/crates/ui-egui/src/panels/left/local.rs +++ b/crates/ui-egui/src/panels/left/local.rs @@ -224,14 +224,17 @@ fn list_subfolders(path: &str) -> std::sync::Arc> { 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 { +/// 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 { ctx.data_mut(|d| d.get_temp_mut_or_default::>(egui::Id::unique("fs-cached-running")).clone()) } diff --git a/crates/ui-egui/src/panels/left/sections.rs b/crates/ui-egui/src/panels/left/sections.rs index 5597904a8..16a4d652c 100644 --- a/crates/ui-egui/src/panels/left/sections.rs +++ b/crates/ui-egui/src/panels/left/sections.rs @@ -381,9 +381,11 @@ 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 check +/// counts as pending sidebar work ([`super::fs_cached_running`]): the Missing Photos row it adds +/// moves every row below it. 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, @@ -402,15 +404,29 @@ pub(super) fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize job.running.store(true, Relaxed); job.at = now; job.rev = rev; - let (n, running, ctx) = (job.n.clone(), job.running.clone(), ui.ctx().clone()); + let pending = super::local::fs_running_counter(ui.ctx()); + pending.fetch_add(1, AcqRel); + #[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 + done.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); + pending.fetch_sub(1, AcqRel); + } #[cfg(target_arch = "wasm32")] work(); ui.data_mut(|d| d.insert_temp(id, job.clone())); diff --git a/crates/ui-egui/src/tests/panels.rs b/crates/ui-egui/src/tests/panels.rs index 5208de9b7..79b8267fd 100644 --- a/crates/ui-egui/src/tests/panels.rs +++ b/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,28 @@ 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"); +} + /// 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]