diff --git a/Cargo.lock b/Cargo.lock index 17f738ea5..0a3e9f5e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3593,6 +3593,7 @@ dependencies = [ "lightcraft-merge", "lightcraft-raster", "lightcraft-sysmem", + "lightcraft-testkit", "lightcraft-ui-egui", "serde_json", ] diff --git a/apps/lightcraft-cli/Cargo.toml b/apps/lightcraft-cli/Cargo.toml index b82aca1d5..e55d458b3 100644 --- a/apps/lightcraft-cli/Cargo.toml +++ b/apps/lightcraft-cli/Cargo.toml @@ -25,6 +25,9 @@ lightcraft-merge = { workspace = true } serde_json = { workspace = true } dhat = { workspace = true, optional = true } +[dev-dependencies] +lightcraft-testkit = { workspace = true } + [features] default = ["denoise"] denoise = ["lightcraft-engine/denoise"] diff --git a/apps/lightcraft-cli/tests/it/cli.rs b/apps/lightcraft-cli/tests/it/cli.rs index 8a11f5c9a..70f7143e6 100644 --- a/apps/lightcraft-cli/tests/it/cli.rs +++ b/apps/lightcraft-cli/tests/it/cli.rs @@ -443,7 +443,12 @@ fn snapshot_defaults_to_cpu_without_gpu_environment_overrides() { let script = tmp("default-cpu.jsonl"); std::fs::write( &script, - format!("{}\n{}\n", json!({"method": "engine.execute", "params": {"command": "app.gpu"}}), json!({"method": "ui.screenshot"})), + format!( + "{}\n{}\n{}\n", + json!({"method": "engine.execute", "params": {"command": "app.gpu", "params": {"wait": false}}}), + json!({"method": "engine.execute", "params": {"command": "app.gpu"}}), + json!({"method": "ui.screenshot"}) + ), ) .unwrap(); let image = tmp("default-cpu.png"); @@ -457,15 +462,52 @@ fn snapshot_defaults_to_cpu_without_gpu_environment_overrides() { assert!(output.status.success(), "status {}: {}", output.status, String::from_utf8_lossy(&output.stderr)); let replies: Vec = String::from_utf8_lossy(&output.stdout).lines().map(|line| serde_json::from_str(line).unwrap()).collect(); assert!(replies.iter().all(|reply| reply["ok"] == true), "{replies:?}"); - let gpu = &replies[0]["result"]; - assert_eq!(gpu["enabled"], false, "{gpu}"); - assert_eq!(gpu["available"], false, "{gpu}"); - assert_eq!(gpu["adapter"], Value::Null, "no device is created: {gpu}"); - assert!(gpu["reason"].as_str().is_some_and(|reason| reason.contains("preference")), "{gpu}"); + // `wait: false` (what the UI sends) still says why the GPU is off: no device is being created + for gpu in [&replies[0]["result"], &replies[1]["result"]] { + assert_eq!(gpu["enabled"], false, "{gpu}"); + assert_eq!(gpu["available"], false, "{gpu}"); + assert_eq!(gpu["adapter"], Value::Null, "no device is created: {gpu}"); + assert!(gpu["reason"].as_str().is_some_and(|reason| reason.contains("preference")), "{gpu}"); + } let decoded = lightcraft_codecs::decode(&std::fs::read(image).unwrap(), Default::default()).unwrap(); assert_eq!((decoded.width, decoded.height), (480, 320)); } +/// Issue #136: `app.gpu {enabled: true, wait: false}` (what the UI sends on its first frame) answers +/// at once while the device is still being created: enabled, and nothing known yet (`null`), instead +/// of waiting for the driver. It starts creating the device (off the calling thread), so this test +/// touches the GPU driver and skips when the environment turns the GPU off. +#[test] +fn enabling_the_gpu_without_waiting_answers_before_the_device_exists() { + if std::env::var("LIGHTCRAFT_GPU").is_ok_and(|v| matches!(v.trim(), "0" | "off" | "false" | "no")) { + lightcraft_testkit::skip!(Gpu, "LIGHTCRAFT_GPU=0: enabling the GPU starts creating a device"); + return; + } + let script = tmp("gpu-starting.jsonl"); + std::fs::write( + &script, + format!("{}\n", json!({"method": "engine.execute", "params": {"command": "app.gpu", "params": {"enabled": true, "wait": false}}})), + ) + .unwrap(); + let image = tmp("gpu-starting.png"); + let output = Command::new(binary()) + .env_remove("LIGHTCRAFT_GPU") + .env_remove("LIGHTCRAFT_GPU_BACKEND") + .env_remove("WGPU_BACKEND") + .args(["snapshot", "--demo", "--script", script.to_str().unwrap(), "-o", image.to_str().unwrap(), "--size", "480x320"]) + .output() + .unwrap(); + assert!(output.status.success(), "status {}: {}", output.status, String::from_utf8_lossy(&output.stderr)); + let replies: Vec = String::from_utf8_lossy(&output.stdout).lines().map(|line| serde_json::from_str(line).unwrap()).collect(); + assert_eq!(replies.len(), 1, "{replies:?}"); + assert_eq!(replies[0]["ok"], true, "{replies:?}"); + let gpu = &replies[0]["result"]; + assert_eq!(gpu["enabled"], true, "{gpu}"); + for field in ["available", "adapter", "reason"] { + assert_eq!(gpu[field], Value::Null, "{field} is not known while the device is created: {gpu}"); + } +} + /// Issue #136: with the GPU switched off from the environment the UI starts and renders on the CPU, /// without creating a GPU device (no driver is loaded), and says why. #[test] diff --git a/crates/engine/src/cmd/query.rs b/crates/engine/src/cmd/query.rs index a3926bc51..469253805 100644 --- a/crates/engine/src/cmd/query.rs +++ b/crates/engine/src/cmd/query.rs @@ -230,10 +230,29 @@ pub fn specs() -> Vec { "versions": ph.versions.iter().map(|v| json!({"name": v.name, "created": v.created})).collect::>(), })) }), - cmd!(query "app.gpu", "GPU Rendering", [], None, "{enabled?: bool} — allow/forbid GPU rendering (CPU fallback; LIGHTCRAFT_GPU=0 forbids it for the process); returns {enabled, available, adapter, reason (why the GPU is off), lastFallback (latest render redone on the CPU, and why)}", always, |_, p| { + cmd!(query "app.gpu", "GPU Rendering", [], None, "{enabled?: bool, wait?: bool} — allow/forbid GPU rendering (CPU fallback; LIGHTCRAFT_GPU=0 forbids it for the process); returns {enabled, available, adapter, reason (why the GPU is off), lastFallback (latest render redone on the CPU, and why)}. wait: false never waits for a device still being created (available, adapter and reason are then null and the device is created in the background; with GPU rendering off they say so at once)", always, |_, p| { if let Some(on) = p.get("enabled").and_then(Value::as_bool) { lightcraft_gpu::set_enabled(on); } + // the UI applies the setting on its first frame: it must not wait seconds for the + // device there (issue #136; `warm_up` creates it off the UI thread) + if p.get("wait").and_then(Value::as_bool) == Some(false) && !lightcraft_gpu::ready() { + // off (preference, environment): known at once, and asking the adapter would + // create the device; on: still being created, so nothing is known yet + let on = lightcraft_gpu::enabled(); + if on { + // a caller that never waits still gets an answer later (off the UI thread; + // starts at most once) + lightcraft_gpu::warm_up(); + } + return Ok(json!({ + "enabled": on, + "available": if on { Value::Null } else { Value::Bool(false) }, + "adapter": null, + "reason": if on { None } else { lightcraft_gpu::unavailable_reason() }, + "lastFallback": lightcraft_gpu::last_fallback(), + })); + } Ok(json!({ "enabled": lightcraft_gpu::enabled(), "available": lightcraft_gpu::available(), diff --git a/crates/gpu/src/lib.rs b/crates/gpu/src/lib.rs index 452e2dd67..e66a8fadd 100644 --- a/crates/gpu/src/lib.rs +++ b/crates/gpu/src/lib.rs @@ -193,7 +193,9 @@ pub fn reset_failures() { /// Allow or forbid GPU rendering at runtime (a preference). `LIGHTCRAFT_GPU=0` forbids it for the /// whole process regardless. pub fn set_enabled(on: bool) { - ENABLED.store(on, Ordering::Relaxed); + // (SeqCst, as `warm_up`'s thread reads it after releasing its start: a switch back on is + // never missed there) + ENABLED.store(on, Ordering::SeqCst); } /// Whether GPU rendering is allowed (environment, preference, no earlier failure — real, or @@ -308,13 +310,33 @@ pub(crate) fn existing_device() -> Option<&'static ctx::Gpu> { /// Create the device and compile the kernels on a background thread now (once the window is up), /// so the first render doesn't wait ~0.3–0.4 s for it and the UI thread never does. Does nothing -/// while GPU rendering is turned off. +/// while GPU rendering is turned off. Repeated calls start one thread at most. pub fn warm_up() { #[cfg(not(target_arch = "wasm32"))] - if !env_disabled() && ENABLED.load(Ordering::Relaxed) && GPU.get().is_none() { - let _ = std::thread::Builder::new().name("lc-gpu-init".into()).spawn(|| { - let _ = device(); + if !env_disabled() && ENABLED.load(Ordering::SeqCst) && GPU.get().is_none() { + static STARTED: AtomicBool = AtomicBool::new(false); + if STARTED.swap(true, Ordering::SeqCst) { + return; + } + let spawned = std::thread::Builder::new().name("lc-gpu-init".into()).spawn(|| { + loop { + let _ = device(); + if GPU.get().is_some() || shutting_down() || env_disabled() { + break; + } + // turned off before the thread ran (nothing created): a later call may start + // again. Released first, then the preference read again: a call made while this + // thread still held the flag (turned back on meanwhile) returned without starting + // one, so this thread tries again itself rather than leave "starting" for good. + STARTED.store(false, Ordering::SeqCst); + if !ENABLED.load(Ordering::SeqCst) || STARTED.swap(true, Ordering::SeqCst) { + break; + } + } }); + if spawned.is_err() { + STARTED.store(false, Ordering::SeqCst); + } } } diff --git a/crates/ui-egui/src/app/prefs.rs b/crates/ui-egui/src/app/prefs.rs index 3dcafeb14..9a5c612f1 100644 --- a/crates/ui-egui/src/app/prefs.rs +++ b/crates/ui-egui/src/app/prefs.rs @@ -34,7 +34,7 @@ impl LightcraftApp { pub(super) fn apply_settings(&mut self, ctx: &egui::Context) { if self.gpu_applied != Some(self.ui.settings.gpu) { self.gpu_applied = Some(self.ui.settings.gpu); - let _ = self.session.execute("app.gpu", &serde_json::json!({"enabled": self.ui.settings.gpu})); + let _ = self.session.execute("app.gpu", &serde_json::json!({"enabled": self.ui.settings.gpu, "wait": false})); // GPU device + kernels off the UI thread, once the window is up and only when GPU // rendering is on: a broken driver must not keep the window from appearing (issue #136) if self.ui.settings.gpu { diff --git a/crates/ui-egui/src/dialogs/settings/performance.rs b/crates/ui-egui/src/dialogs/settings/performance.rs index 88ff55f6e..370cf08ba 100644 --- a/crates/ui-egui/src/dialogs/settings/performance.rs +++ b/crates/ui-egui/src/dialogs/settings/performance.rs @@ -11,7 +11,15 @@ pub(super) fn performance_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &To #[cfg(target_os = "windows")] check(ui, "settings.composeWindow", &mut app.ui.settings.compose_window, "Prevent G-SYNC/FreeSync flicker (applies at next launch)"); check(ui, "settings.gpu", &mut app.ui.settings.gpu, "Use the GPU for rendering"); - let status = if !gpu::available() { + // `available` would wait for a device still being created (issue #136) + let status = if gpu::enabled() && !gpu::ready() { + // the device is created on another thread, which doesn't wake the UI when it's done; + // make sure that thread runs (cheap and idempotent: a start lost to an off/on switch + // can't leave "starting" for good) + gpu::warm_up(); + ui.ctx().request_repaint_after(std::time::Duration::from_millis(250)); + "GPU: starting…".to_string() + } else if !gpu::available() { match gpu::unavailable_reason() { Some(why) => crate::i18n::tr_format!("Rendering on the CPU: {why}", why = why), None => "No usable GPU found: rendering on the CPU".to_string(),