mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-01 14:07:34 -06:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
445dd3106e
commit
142ddcff29
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
|
||||
|
||||
@@ -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<AtomicUsize>,
|
||||
@@ -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()));
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user