diff --git a/apps/photocraft/src/gpu_startup.rs b/apps/photocraft/src/gpu_startup.rs index 3b85984360..03d8b11bc6 100644 --- a/apps/photocraft/src/gpu_startup.rs +++ b/apps/photocraft/src/gpu_startup.rs @@ -130,6 +130,14 @@ pub fn next_safer(tried: GpuBackend, os: Os) -> GpuBackend { } } +/// Whether a start that returned an error (so it didn't crash in the driver) keeps its marker, so +/// the next start tries a safer backend. Not when there is none: the CPU path is the end of every +/// chain, and a marker left there pinned every later start to the same failing path. Not under +/// `WGPU_BACKEND` either (no plan reads that marker). +pub fn keep_marker_after_error(plan: &Plan, os: Os) -> bool { + plan.env.is_none() && next_safer(plan.backend, os) != plan.backend +} + /// Decide this launch's backend from the preference, a marker left by a start that crashed, /// `WGPU_BACKEND` and `--safe-gpu`. pub fn plan(pref: GpuBackend, crashed: Option<&Marker>, env: Option<&str>, safe_gpu: bool, os: Os) -> Plan { @@ -562,6 +570,43 @@ mod tests { assert_eq!(Marker::parse(&long).adapter.len(), 256); } + #[test] + fn a_failed_start_keeps_its_marker_only_when_a_safer_backend_exists() { + // GPU backends move down the chain on the next start. + assert!(keep_marker_after_error(&plan(Auto, None, None, false, Os::Other), Os::Other)); + assert!(keep_marker_after_error(&plan(Gl, None, None, false, Os::Other), Os::Other)); + assert!(keep_marker_after_error(&plan(Dx12, None, None, false, Os::Windows), Os::Windows)); + assert!(keep_marker_after_error(&plan(Auto, None, None, false, Os::Mac), Os::Mac)); + // The CPU path is the end of every chain, including the `--safe-gpu` retry. + for os in [Os::Windows, Os::Mac, Os::Other] { + assert!(!keep_marker_after_error(&plan(Cpu, None, None, false, os), os)); + assert!(!keep_marker_after_error(&plan(Auto, None, None, true, os), os)); + } + // `WGPU_BACKEND` is the user's choice; no plan reads its marker. + assert!(!keep_marker_after_error(&plan(Auto, None, Some("vulkan"), false, Os::Other), Os::Other)); + } + + /// A failed CPU start (no usable GL adapter under XWayland) left a `cpu` marker, so every + /// later start was planned on CPU and failed the same way until the marker was deleted by hand. + #[test] + fn a_failed_cpu_start_does_not_pin_later_starts() { + let dir = temp_dir("cpu-error"); + let (_, s) = Sentinel::begin(&dir); + let mut s = s.expect("sentinel"); + let failed = plan(Auto, None, None, true, Os::Other); + s.write(Marker { backend: failed.backend.name().into(), ..Default::default() }).unwrap(); + // eframe returned an error: what `main` does with the marker. + if keep_marker_after_error(&failed, Os::Other) { + drop(s); + } else { + s.finish(); + } + let (prev, _s) = Sentinel::begin(&dir); + assert_eq!(prev, Previous::Clean); + assert_eq!(plan(Auto, prev.crashed(), None, false, Os::Other).backend, Auto); + let _ = std::fs::remove_dir_all(&dir); + } + fn temp_dir(name: &str) -> PathBuf { let d = std::env::temp_dir().join(format!("photocraft-gpu-startup-{name}-{}", std::process::id())); let _ = std::fs::remove_dir_all(&d); diff --git a/apps/photocraft/src/main.rs b/apps/photocraft/src/main.rs index 08f52b08a8..82ed93bf10 100644 --- a/apps/photocraft/src/main.rs +++ b/apps/photocraft/src/main.rs @@ -261,6 +261,7 @@ fn main() -> eframe::Result { let sentinel_ms = t_sentinel.elapsed().as_secs_f64() * 1000.0; log::info!("GPU startup: {:?} ({sentinel_ms:.2} ms)", plan); let retry_cpu = !safe_gpu && plan.backend != photocraft_engine::prefs::GpuBackend::Cpu; + let keep_marker = gpu_startup::keep_marker_after_error(&plan, os); let app_created = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); let created_in_callback = app_created.clone(); let started_sentinel = sentinel.clone(); @@ -390,12 +391,15 @@ fn main() -> eframe::Result { Ok(Box::new(app)) }), ); - // Closed or failed outside graphics initialization: not a driver crash. A renderer - // error keeps the marker, so the next start tries a safer backend. - if !gpu_startup::keep_marker_after_run(&result) + // Closed or failed outside graphics initialization: not a driver crash. A renderer error + // keeps the marker, so the next start tries a safer backend, unless there is no safer one: + // a failed CPU start would otherwise pin every later start to CPU. + let renderer_error = gpu_startup::keep_marker_after_run(&result); + if !(renderer_error && keep_marker) && let Some(mut s) = sentinel.lock().unwrap_or_else(std::sync::PoisonError::into_inner).take() { if result.is_err() + && !renderer_error && let Some(marker) = previous.crashed() { // This failure supplies no new graphics-crash evidence: retain the previous