diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index b7f45fe15..4f9c73a6d 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..76550d14c 100644 --- a/crates/ui-egui/src/panels/left/sections.rs +++ b/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, running: std::sync::Arc, 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())); diff --git a/crates/ui-egui/src/tests/panels.rs b/crates/ui-egui/src/tests/panels.rs index 5208de9b7..194e235e4 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,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]