From 1a6a9de7eedb72d95a87dca1e16fff71e56a7661 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:50:29 -0400 Subject: [PATCH] Issue #33: Devices: serve the cached card list and rescan in the background MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Menus are rebuilt every frame, and File → Add from Device called devices(), which scanned every mount (D:..Z: on Windows) while holding the cache mutex once its 2 s cache expired. devices() now returns the last scan at once and starts at most one background rescan when it is stale; the scan runs without the lock. A changed result calls an on_change hook (the UI requests a repaint), so hot-plugged cards still appear. library.devices and Add from Device without a path scan synchronously (devices_now). Volume roots are probed for DCIM directly instead of listing them (Windows never lists a root). No threads or clock on wasm. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/library.rs | 2 +- crates/engine/src/devices.rs | 221 ++++++++++++++++++++++++++++--- crates/ui-egui/src/lib.rs | 3 + crates/ui-egui/src/menus.rs | 2 +- docs/parity.md | 2 +- 5 files changed, 207 insertions(+), 23 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index b4c3b2925..d8fad84b4 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -250,7 +250,7 @@ pub fn specs() -> Vec { Ok(json!({"selected": s.selection.ids.len()})) }), cmd!(query "library.devices", "Cameras and Cards", [], None, "{} → [{name, path (its DCIM folder), root}] — mounted volumes with a DCIM folder", always, |_, _| { - Ok(serde_json::to_value(crate::devices::devices()).unwrap_or_default()) + Ok(serde_json::to_value(crate::devices::devices_now()).unwrap_or_default()) }), cmd!(query "photo.copyMetadata", "Copy Metadata", ["Photo"], None, "{} — title, caption, copyright, creator, location and keywords of the active photo", has_active, |s, _| { let id = s.active().ok_or_else(|| bad("photo.copyMetadata", "no active photo"))?; diff --git a/crates/engine/src/devices.rs b/crates/engine/src/devices.rs index 0ad269224..bc82a727d 100644 --- a/crates/engine/src/devices.rs +++ b/crates/engine/src/devices.rs @@ -1,6 +1,15 @@ //! Cameras and memory cards: mounted volumes with a `DCIM` folder (the DCF layout every camera //! writes). Listed by `library.devices`; the app offers them under File → Add from Device, and //! the import review copies from them into the library. +//! +//! Menus are rebuilt every frame (even with their popups closed), so [`devices`] never touches the +//! disk: it serves the last scan and, when that is older than [`MAX_AGE`], starts one background +//! rescan (coalesced; no lock is held while it runs). A changed result calls the [`on_change`] +//! hook, which the UI uses to repaint, so hot-plugged cards still appear. [`devices_now`] scans +//! synchronously for explicit requests (`library.devices`, Add from Device without a path). + +use std::sync::{Arc, Mutex}; +use std::time::Duration; use serde::Serialize; @@ -14,6 +23,9 @@ pub struct Device { pub root: String, } +/// How long a scan is served before [`devices`] starts another one. +pub const MAX_AGE: Duration = Duration::from_secs(2); + /// Folders whose children are mounted volumes on this platform. `LIGHTCRAFT_DEVICE_ROOTS` /// (paths joined like `PATH`) replaces them (tests, unusual mounts). fn mount_parents() -> Vec { @@ -34,21 +46,114 @@ fn mount_parents() -> Vec { out } -/// Mounted volumes that look like a camera or a memory card. Cached for a couple of seconds: -/// menus ask often, and listing mounts can be slow (network volumes). -pub fn devices() -> Vec { - use std::sync::Mutex; - use std::time::{Duration, Instant}; - static CACHE: Mutex)>> = Mutex::new(None); - let mut c = CACHE.lock().unwrap_or_else(|e| e.into_inner()); - if let Some((t, d)) = c.as_ref() - && t.elapsed() < Duration::from_secs(2) - { - return d.clone(); +type Scanner = dyn Fn() -> Vec + Send + Sync; +type Notify = dyn Fn() + Send + Sync; + +#[derive(Default)] +struct WatchState { + list: Vec, + /// When the served list was scanned (`None`: never). No clock (and no threads) on wasm, where + /// there are no mounted volumes either. + #[cfg(not(target_arch = "wasm32"))] + scanned: Option, + /// A background scan is running. + refreshing: bool, +} + +/// A cached device list, refreshed off the calling thread. +pub struct DeviceWatch { + scanner: Box, + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + max_age: Duration, + state: Mutex, + notify: Mutex>>, +} + +impl DeviceWatch { + pub fn new(scanner: impl Fn() -> Vec + Send + Sync + 'static, max_age: Duration) -> Arc { + Arc::new(DeviceWatch { scanner: Box::new(scanner), max_age, state: Mutex::default(), notify: Mutex::new(None) }) } - let d = scan(); - *c = Some((Instant::now(), d.clone())); - d + + fn state(&self) -> std::sync::MutexGuard<'_, WatchState> { + self.state.lock().unwrap_or_else(|e| e.into_inner()) + } + + /// Called (on the scanning thread) whenever a scan changes the list. + pub fn on_change(&self, f: impl Fn() + Send + Sync + 'static) { + *self.notify.lock().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(f)); + } + + /// The last scan, immediately; starts a background rescan when it is stale and none is running. + pub fn get(self: &Arc) -> Vec { + let mut s = self.state(); + #[cfg(not(target_arch = "wasm32"))] + if !s.refreshing && s.scanned.is_none_or(|t| t.elapsed() >= self.max_age) { + s.refreshing = true; + let me = self.clone(); + // the scan runs without the state lock; `get` only ever waits for a list clone + let spawned = std::thread::Builder::new().name("lightcraft-devices".into()).spawn(move || { + let list = (me.scanner)(); + me.store(list); + }); + if spawned.is_err() { + s.refreshing = false; + } + } + s.list.clone() + } + + /// Scan now on this thread, and serve the result to later [`DeviceWatch::get`] calls. + pub fn scan_now(&self) -> Vec { + let list = (self.scanner)(); + self.store(list.clone()); + list + } + + fn store(&self, list: Vec) { + let changed = { + let mut s = self.state(); + s.refreshing = false; + #[cfg(not(target_arch = "wasm32"))] + { + s.scanned = Some(std::time::Instant::now()); + } + let changed = s.list != list; + s.list = list; + changed + }; + if changed { + let f = self.notify.lock().unwrap_or_else(|e| e.into_inner()).clone(); + if let Some(f) = f { + f(); + } + } + } + + /// A background scan is running. + pub fn refreshing(&self) -> bool { + self.state().refreshing + } +} + +fn global() -> &'static Arc { + static WATCH: std::sync::OnceLock> = std::sync::OnceLock::new(); + WATCH.get_or_init(|| DeviceWatch::new(scan, MAX_AGE)) +} + +/// Mounted volumes that look like a camera or a memory card, without blocking: the last scan, +/// refreshed in the background when older than [`MAX_AGE`] (empty until the first scan lands). +pub fn devices() -> Vec { + global().get() +} + +/// Scan the mounted volumes now (blocking), for explicit requests. +pub fn devices_now() -> Vec { + global().scan_now() +} + +/// Run `f` whenever a scan changes the device list (the UI requests a repaint). +pub fn on_change(f: impl Fn() + Send + Sync + 'static) { + global().on_change(f); } fn scan() -> Vec { @@ -73,10 +178,7 @@ pub fn devices_in(parents: &[std::path::PathBuf], extra: Vec roots.extend(extra); let mut out: Vec = Vec::new(); for root in roots { - let Ok(rd) = std::fs::read_dir(&root) else { continue }; - let Some(dcim) = rd.flatten().map(|e| e.path()).find(|p| p.is_dir() && p.file_name().is_some_and(|n| n.eq_ignore_ascii_case("DCIM"))) else { - continue; - }; + let Some(dcim) = dcim_in(&root) else { continue }; let name = root.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| root.to_string_lossy().to_string()); if !out.iter().any(|d| d.root == root.to_string_lossy()) { out.push(Device { name, path: dcim.to_string_lossy().to_string(), root: root.to_string_lossy().to_string() }); @@ -85,9 +187,22 @@ pub fn devices_in(parents: &[std::path::PathBuf], extra: Vec out } +/// The volume's `DCIM` folder: the usual spellings are checked directly, so a volume's root is +/// only listed (other spellings on a case-sensitive volume) off Windows, whose drives aren't. +fn dcim_in(root: &std::path::Path) -> Option { + if let Some(p) = ["DCIM", "dcim"].iter().map(|n| root.join(n)).find(|p| p.is_dir()) { + return Some(p); + } + if cfg!(windows) { + return None; + } + std::fs::read_dir(root).ok()?.flatten().map(|e| e.path()).find(|p| p.file_name().is_some_and(|n| n.eq_ignore_ascii_case("DCIM")) && p.is_dir()) +} + #[cfg(test)] mod tests { use super::*; + use std::sync::atomic::{AtomicUsize, Ordering}; #[test] fn volumes_with_a_dcim_folder_are_devices() { @@ -96,10 +211,76 @@ mod tests { std::fs::create_dir_all(base.join("EOS_DIGITAL/DCIM/100CANON")).unwrap(); std::fs::create_dir_all(base.join("Backup/Photos")).unwrap(); std::fs::create_dir_all(base.join("SD/dcim")).unwrap(); + std::fs::create_dir_all(base.join("X/Dcim")).unwrap(); let d = devices_in(std::slice::from_ref(&base), Vec::new()); let names: Vec<&str> = d.iter().map(|x| x.name.as_str()).collect(); - assert_eq!(names, ["EOS_DIGITAL", "SD"], "{d:?}"); - assert!(d[0].path.ends_with("DCIM") && d[1].path.ends_with("dcim")); + assert_eq!(names, ["EOS_DIGITAL", "SD", "X"], "{d:?}"); + // the DCIM folder itself, in a spelling that opens on that volume + for x in &d { + let p = std::path::Path::new(&x.path); + assert!(p.is_dir() && p.file_name().is_some_and(|n| n.eq_ignore_ascii_case("DCIM")), "{x:?}"); + } let _ = std::fs::remove_dir_all(&base); } + + fn card(name: &str) -> Device { + Device { name: name.into(), path: format!("/v/{name}/DCIM"), root: format!("/v/{name}") } + } + + /// Wait (bounded) for `f`. + fn until(f: impl Fn() -> bool) -> bool { + let t = std::time::Instant::now(); + while t.elapsed() < Duration::from_secs(20) { + if f() { + return true; + } + std::thread::sleep(Duration::from_millis(5)); + } + false + } + + #[test] + fn slow_scans_never_block_and_hot_plug_still_arrives() { + // a scanner that takes 1.5 s (a sleeping network drive) and counts its runs + const SLOW: Duration = Duration::from_millis(1500); + let mounted = Arc::new(Mutex::new(Vec::::new())); + let runs = Arc::new(AtomicUsize::new(0)); + let (m, r) = (mounted.clone(), runs.clone()); + let watch = DeviceWatch::new( + move || { + r.fetch_add(1, Ordering::SeqCst); + std::thread::sleep(SLOW); + m.lock().unwrap().clone() + }, + Duration::from_millis(20), + ); + let changes = Arc::new(AtomicUsize::new(0)); + let c = changes.clone(); + watch.on_change(move || { + c.fetch_add(1, Ordering::SeqCst); + }); + // cold: served at once (empty) while one scan runs in the background + let t = std::time::Instant::now(); + for _ in 0..50 { + assert!(watch.get().is_empty()); + } + assert!(t.elapsed() < SLOW / 2, "get() waited for the scan: {:?}", t.elapsed()); + assert!(watch.refreshing()); + assert!(until(|| !watch.refreshing())); + assert_eq!(runs.load(Ordering::SeqCst), 1, "requests while scanning are coalesced"); + assert_eq!(changes.load(Ordering::SeqCst), 0, "nothing changed"); + // a card is plugged in: the next stale request starts a rescan, still without waiting + mounted.lock().unwrap().push(card("EOS_DIGITAL")); + std::thread::sleep(Duration::from_millis(40)); + let t = std::time::Instant::now(); + assert!(watch.get().is_empty(), "the old list is served while rescanning"); + assert!(t.elapsed() < SLOW / 2, "warm get() waited: {:?}", t.elapsed()); + assert!(until(|| changes.load(Ordering::SeqCst) == 1), "a changed scan notifies"); + assert_eq!(watch.get(), [card("EOS_DIGITAL")]); + // explicit requests scan synchronously + assert!(until(|| !watch.refreshing())); + mounted.lock().unwrap().clear(); + assert!(watch.scan_now().is_empty()); + assert_eq!(changes.load(Ordering::SeqCst), 2); + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e42ed5ea4..684fed5b1 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -404,6 +404,9 @@ impl LightcraftApp { if !self.styled { theme::install_fonts(ctx); theme::apply(ctx); + // File → Add from Device lists cards scanned in the background: show hot-plugs + let repaint = ctx.clone(); + lightcraft_engine::devices::on_change(move || repaint.request_repaint()); self.styled = true; } else { self.fonts_ready = true; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index fde1e087a..7fd93b80d 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -915,7 +915,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option x.to_string(), - None => match lightcraft_engine::devices::devices().into_iter().next() { + None => match lightcraft_engine::devices::devices_now().into_iter().next() { Some(d) => d.path, None => return Some(Err("no camera or memory card found".into())), }, diff --git a/docs/parity.md b/docs/parity.md index a33caa461..45f32a511 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -102,7 +102,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/import.rs` | review dialog: candidate grid with thumbnails and checkboxes (duplicates by path/content marked and unchecked), destination, album (existing/new), preset, keywords; the folder scan runs on a background thread with a progress window and Cancel (a NAS folder no longer freezes the window); batched import with a progress window; one undo step. Drag-and-drop still imports directly | | LR-IMP-DRAGDROP | Drop files/folders to import | P0 | ✅ | `crates/ui-egui/src/lib.rs` (dropped files → `cmd:library.import`) | dropping onto a specific album not supported | | LR-IMP-DUPES | Skip duplicates by content | P1 | ✅ | `crates/engine/src/import.rs`, `crates/engine/src/tests_import.rs` | | -| LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Add from Device → the import review, copying into the library by default; no PTP/MTP (cameras that don't mount as a disk) | +| LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Add from Device → the import review, copying into the library by default; menus serve the last scan and rescan in the background (never blocking a frame; hot-plugs repaint); no PTP/MTP (cameras that don't mount as a disk) | | LR-IMP-AUTO | Watched-folder auto import | P2 | ✅ | `cmd:library.autoImport`, `cmd:library.autoImportScan`, `crates/ui-egui/src/panels/settings.rs` | Settings ▸ Import ▸ Auto Import: a watched folder whose new photos are added (in place or copied) once complete, into an optional album; scanned every 3 s | | LR-IMP-PRESET | Preset on import | P2 | ✅ | `cmd:library.import` (`preset`) | chosen in the import review; one History entry | | LR-IMP-RAWDEFAULT | Raw defaults | P1 | ✅ | `cmd:library.preferences`, `crates/engine/src/import.rs`, `crates/ui-egui/src/panels/settings.rs` | LightCraft default / a preset / per camera (make + model); non-raw default too; the preset look counts as unedited and Reset returns to it |