diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index a0717ccfd..b95b55b19 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -412,7 +412,10 @@ fn performance_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { } else { format!("GPU: {}", gpu::adapter_name().unwrap_or_else(|| "starting…".into())) }; - hint(ui, t, &status); + // One line, the whole text on hover: the reason can be a long driver message (no adapter + // among the backends …), and the GPU state is process-wide, so it may change between frames; + // a hint that wrapped would move every control below it under the pointer. + ui.add(egui::Label::new(RichText::new(crate::i18n::tr(&status)).size(11.0).color(t.text_dim)).truncate()).on_hover_text(&status); row(ui, t, crate::i18n::tr("Preview size"), |ui| { let opts: Vec<(u32, String)> = PREVIEW_LIMITS.iter().map(|e| (*e, if *e == 0 { crate::i18n::tr("Automatic").to_string() } else { format!("{e} px") })).collect(); diff --git a/crates/ui-egui/src/tests_dialog_fit.rs b/crates/ui-egui/src/tests_dialog_fit.rs index 0014c6bdb..29d0401c2 100644 --- a/crates/ui-egui/src/tests_dialog_fit.rs +++ b/crates/ui-egui/src/tests_dialog_fit.rs @@ -134,3 +134,26 @@ fn a_dialog_list_shrinks_to_fit_a_short_window() { let dialog = rect(&h, "dialog:window"); assert!(dialog.height() <= 320.0, "{dialog:?}"); } + +/// Settings ▸ Performance: the GPU status line above the controls stays one line whatever it says +/// (without a driver it is a long wgpu message, e.g. on the FreeBSD CI), so the controls below it +/// don't move when the process-wide GPU state changes between two frames (a click aimed at +/// "Memory for caches" then landed on the gap above it). +#[test] +fn gpu_status_never_moves_the_performance_controls() { + let mut h = demo([1300.0, 820.0]); + let r = h.request("ui.key", json!({"key": ",", "cmd": true}), T); + assert_eq!(r["ok"], true, "{r}"); + let r = h.request("ui.clickWidget", json!({"id": "button:settingsTab-performance"}), T); + assert_eq!(r["ok"], true, "{r}"); + let mut seen = vec![]; + for gpu in [true, false, true] { + // the setting is applied process-wide at the next frame (`app.gpu`), as other tests do + h.app.ui.settings.gpu = gpu; + h.step(); + h.step(); + seen.push((gpu, rect(&h, "button:settingsMemory-2"), lightcraft_engine::gpu::unavailable_reason())); + } + let first = seen[0].1; + assert!(seen.iter().all(|(_, r, _)| *r == first), "the memory buttons moved: {seen:?}"); +} diff --git a/crates/ui-egui/src/tests_offline.rs b/crates/ui-egui/src/tests_offline.rs index 3e3ed49d6..e06fe1401 100644 --- a/crates/ui-egui/src/tests_offline.rs +++ b/crates/ui-egui/src/tests_offline.rs @@ -1,10 +1,14 @@ //! Slow or offline drives never stall a frame (issue #104): with an injected file-system probe -//! that takes far longer than a frame (a sleeping NAS), the grid, the Info panel, the Missing -//! Photos view, the Rename dialog and an import keep drawing frames quickly, the probe never runs -//! on the UI thread, and the background answers still arrive. +//! that does not answer at all until the test lets it (a sleeping NAS), the grid, the Info panel, +//! the Rename dialog and the Missing Photos view keep drawing frames, the probe never runs on the +//! UI thread, and the background answers arrive once the drive wakes up. +//! +//! The NAS is a gate, not a sleep: a frame that waited for the drive would wait for good (until a +//! watchdog gives up and records it), so the test needs no frame-time budget and holds on a slow +//! or loaded machine (the FreeBSD CI VM) as it does on a fast one. -use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Arc, Condvar, Mutex}; use std::thread::ThreadId; use std::time::{Duration, Instant}; @@ -16,11 +20,11 @@ use crate::headless::Headless; use crate::state::{RightPanel, ViewMode}; use crate::{LightcraftApp, Services}; -/// What one file-system call takes on the "NAS". -const SLOW: Duration = Duration::from_millis(400); -/// A frame that waited for one such call would take at least this long. +/// The import test's frame budget (its files take 150 ms each to read). const FRAME_LIMIT: Duration = Duration::from_millis(300); const N: u64 = 12; +/// How long the drive stays asleep at most: only a frame stuck waiting for it gets this far. +const WATCHDOG: Duration = Duration::from_secs(120); /// Threads the probe ran on, and how often. #[derive(Clone, Default)] @@ -39,9 +43,49 @@ impl Calls { } } +/// The sleeping drive: every file-system call waits until [`Nas::wake`]. A watchdog wakes it +/// after [`WATCHDOG`] and records that it had to (some frame was waiting for the drive). +#[derive(Default)] +struct Nas { + awake: Mutex, + changed: Condvar, + forced: AtomicBool, +} + +impl Nas { + fn asleep() -> Arc { + let nas = Arc::new(Nas::default()); + let dog = nas.clone(); + std::thread::spawn(move || { + let awake = dog.awake.lock().unwrap(); + let (mut awake, _) = dog.changed.wait_timeout_while(awake, WATCHDOG, |a| !*a).unwrap(); + if !*awake { + dog.forced.store(true, Ordering::SeqCst); + *awake = true; + dog.changed.notify_all(); + } + }); + nas + } + fn wait(&self) { + let awake = self.awake.lock().unwrap(); + drop(self.changed.wait_while(awake, |a| !*a).unwrap()); + } + fn wake(&self) { + *self.awake.lock().unwrap() = true; + self.changed.notify_all(); + } + /// The watchdog had to wake the drive: a frame waited for it. + fn forced(&self) -> bool { + self.forced.load(Ordering::SeqCst) + } +} + /// `N` photos whose originals are on an unreachable share, a smart previews folder, and a probe -/// that sleeps like a NAS waking up before saying "not there". -fn offline_library(calls: &Calls) -> Session { +/// that waits for the NAS to wake up before saying "not there". Called on the UI thread: a call +/// from that thread is recorded and answered at once, so a regression fails the thread check +/// instead of hanging the test. +fn offline_library(calls: &Calls, nas: &Arc) -> Session { let mut s = Session::new(); for i in 0..N { let path = format!("/lightcraft-offline-nas/IMG_{i:03}.jpg"); @@ -49,72 +93,71 @@ fn offline_library(calls: &Calls) -> Session { s.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); } s.media.smart_dir = Some(std::env::temp_dir().join("lightcraft-offline-smart-previews-none")); - let c = calls.clone(); + let (c, nas, ui_thread) = (calls.clone(), nas.clone(), std::thread::current().id()); s.media.availability.set_probe(Arc::new(move |_| { c.record(); - std::thread::sleep(SLOW); + if std::thread::current().id() != ui_thread { + nas.wait(); + } false })); s } -fn max_frame(h: &mut Headless, frames: usize) -> Duration { - let mut worst = Duration::ZERO; +/// Run `frames` frames while the drive sleeps; each one has to finish without it. +fn frames_while_asleep(h: &mut Headless, nas: &Nas, frames: usize, what: &str) { for _ in 0..frames { - let t0 = Instant::now(); h.step(); - worst = worst.max(t0.elapsed()); + assert!(!nas.forced(), "a {what} frame waited for the sleeping drive"); } - worst } #[test] fn offline_originals_never_block_a_frame() { let calls = Calls::default(); - let app = LightcraftApp::new(offline_library(&calls), Services { png: None, ..Default::default() }); + let nas = Nas::asleep(); + let app = LightcraftApp::new(offline_library(&calls, &nas), Services { png: None, ..Default::default() }); let mut h = Headless::new(app, [1200.0, 800.0], 1.0); let ui_thread = std::thread::current().id(); h.app.ui.view = ViewMode::PhotoGrid; h.step(); - // The first frame installs fonts; finish the first layout before timing the views. + // The first frame installs fonts; finish the first layout before the drive matters. h.step(); - assert!(h.app.fonts_ready, "the UI is initialized before frame timing starts"); + assert!(h.app.fonts_ready, "the UI is initialized"); assert!(h.app.session.media.availability.is_background(), "the app checks files off the UI thread"); // the grid: every cell asks for its thumbnail (which asks whether the original is there) h.app.session.execute("library.select", &json!({"ids": [1]})).unwrap(); - let worst = max_frame(&mut h, 20); - assert!(worst < FRAME_LIMIT, "grid frame took {worst:?}"); + frames_while_asleep(&mut h, &nas, 20, "grid"); // the Info panel of the active photo, in the loupe h.app.ui.view = ViewMode::Detail; h.app.ui.right = RightPanel::Info; - let worst = max_frame(&mut h, 20); - assert!(worst < FRAME_LIMIT, "Info panel frame took {worst:?}"); + frames_while_asleep(&mut h, &nas, 20, "Info panel"); - // the Missing Photos view fills in as the background checks find the files gone + // the Rename dialog's preview (it checks which names are taken on disk) h.app.ui.view = ViewMode::PhotoGrid; h.app.ui.right = RightPanel::None; + h.app.session.execute("library.select", &json!({"ids": [1, 2, 3]})).unwrap(); + h.app.ui.dialog = Some(crate::state::Dialog::Rename { template: "Trip-{seq:3}".into(), start: 1 }); + frames_while_asleep(&mut h, &nas, 20, "Rename dialog"); + h.app.ui.dialog = None; + + // the Missing Photos view: nothing is known yet, so nothing is listed, and frames go on h.app.session.execute("library.source", &json!({"kind": "missing"})).unwrap(); assert_eq!(h.app.session.source, LibrarySource::Missing); + frames_while_asleep(&mut h, &nas, 20, "Missing Photos"); + assert!(calls.n.load(Ordering::SeqCst) > 0, "the probe was asked"); + assert_eq!(h.app.session.visible().len(), 0, "unknown files count as there"); + + // the drive answers: the view fills in as the background checks find the files gone + nas.wake(); let t0 = Instant::now(); - let mut worst = Duration::ZERO; while h.app.session.visible().len() < N as usize && t0.elapsed() < Duration::from_secs(60) { - let f = Instant::now(); h.step(); - worst = worst.max(f.elapsed()); std::thread::sleep(Duration::from_millis(5)); } assert_eq!(h.app.session.visible().len(), N as usize, "every photo found missing in the end"); - assert!(worst < FRAME_LIMIT, "Missing Photos frame took {worst:?}"); - - // the Rename dialog's preview (it checks which names are taken on disk) - h.app.session.execute("library.select", &json!({"ids": [1, 2, 3]})).unwrap(); - h.app.ui.dialog = Some(crate::state::Dialog::Rename { template: "Trip-{seq:3}".into(), start: 1 }); - let worst = max_frame(&mut h, 20); - assert!(worst < FRAME_LIMIT, "Rename dialog frame took {worst:?}"); - - assert!(calls.n.load(Ordering::SeqCst) > 0, "the probe was asked"); assert_eq!(calls.on(ui_thread), 0, "the slow probe never ran on the UI thread"); }