From 87d28ac6fe5f6170bd22b47d2e59aeaeb6549839 Mon Sep 17 00:00:00 2001 From: tylerjenningsw Date: Tue, 13 Oct 2026 13:43:33 -0400 Subject: [PATCH] GPU: a frame the device rejected isn't reported as out of memory (refs #1319) --- crates/gpu/src/lib.rs | 35 ++++++++++++++++++++++++++++++----- crates/gpu/src/tests.rs | 2 +- crates/ui-egui/src/frames.rs | 9 +++++++-- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/crates/gpu/src/lib.rs b/crates/gpu/src/lib.rs index ee6970de1..08806b654 100644 --- a/crates/gpu/src/lib.rs +++ b/crates/gpu/src/lib.rs @@ -96,6 +96,29 @@ pub use wgpu; use crate::context::Enc; +/// Why [`Gpu::within_memory`] discarded a frame. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum FrameError { + /// The GPU context is retired (device lost, a readback timed out, an uncaptured error). + Retired(String), + /// Out of video memory (or a driver reporting a failed allocation as an internal error), + /// with any validation errors it went on to cause. + Memory(String), + /// The device rejected a command with no allocation failure before it (an invalid texture + /// or parameters, #1319): a compositor bug, not memory pressure. + Validation(String), +} + +impl std::fmt::Display for FrameError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + FrameError::Retired(e) | FrameError::Memory(e) | FrameError::Validation(e) => f.write_str(e), + } + } +} + +impl std::error::Error for FrameError {} + /// The GPU compositor (cheap to clone; one device shared by all clones). #[derive(Clone)] pub struct Gpu { @@ -273,8 +296,8 @@ impl Gpu { /// instead of retiring the GPU compositor for the session as an uncaptured error. /// Browser callers use the host/owned-device failure bridge and bounded readbacks instead /// of blocking their event loop on an error-scope future. - pub fn within_memory(&self, f: impl FnOnce() -> T) -> Result { - self.ctx.check_health()?; + pub fn within_memory(&self, f: impl FnOnce() -> T) -> Result { + self.ctx.check_health().map_err(FrameError::Retired)?; #[cfg(not(target_arch = "wasm32"))] { let oom = self.ctx.device.push_error_scope(wgpu::ErrorFilter::OutOfMemory); @@ -284,20 +307,22 @@ impl Gpu { let validation = pollster::block_on(validation.pop()); let internal = pollster::block_on(internal.pop()); let oom = pollster::block_on(oom.pop()); + let memory = oom.is_some() || internal.is_some(); // The allocation failure first: a validation error is often its consequence. let errors: Vec = [oom, internal, validation].into_iter().flatten().map(|e| e.to_string()).collect(); if errors.is_empty() { - return self.ctx.check_health().map(|()| out); + return self.ctx.check_health().map(|()| out).map_err(FrameError::Retired); } // Its textures go back to the pool when dropped: drop them before emptying it. drop(out); self.ctx.discard_kept(); - Err(errors.join("; then: ")) + let errors = errors.join("; then: "); + Err(if memory { FrameError::Memory(errors) } else { FrameError::Validation(errors) }) } #[cfg(target_arch = "wasm32")] { let out = f(); - self.ctx.check_health().map(|()| out) + self.ctx.check_health().map(|()| out).map_err(FrameError::Retired) } } diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 2ccd438bc..f9d62b901 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -772,7 +772,7 @@ fn a_rejected_texture_fails_the_frame_not_the_compositor() { (kept, rejected.create_view(&Default::default())) }) .expect_err("the rejected texture fails the frame"); - assert!(err.contains("deliberately too wide"), "{err}"); + assert!(matches!(&err, crate::FrameError::Validation(e) if e.contains("deliberately too wide")), "{err:?}"); assert!(g.ctx.check_health().is_ok(), "the compositor stays on: {:?}", g.ctx.check_health()); let (hits, _) = g.ctx.pool_stats(); let img = g.within_memory(|| crate::context::Enc::new(&g.ctx).image(64, 32)).expect("the next frame renders"); diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 059bc3a03..2d29dd0af 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -31,7 +31,7 @@ use effectcraft_engine::project::{Comp, ItemId, ItemKind, LayerSource, Node, Pro use effectcraft_engine::render::disk_cache::{self, DiskCache}; use effectcraft_engine::render::{CancellableFootage, ExprHost, FootageSource, LayerCache, RenderOpts, Renderer}; use effectcraft_engine::time::Tick; -use effectcraft_gpu::{DisplayFrame, Gpu}; +use effectcraft_gpu::{DisplayFrame, FrameError, Gpu}; use rayon::prelude::*; /// Comp identities (and the project snapshots behind them) the RAM preview keeps frames of. @@ -1223,11 +1223,16 @@ impl Worker { /// A failed scoped attempt retries on CPU. Retired contexts cannot recover by reducing /// their budget; only a healthy device's allocation failure uses the upstream backoff. - fn gpu_attempt_failed(&self, gpu: &Gpu, e: &str) { + fn gpu_attempt_failed(&self, gpu: &Gpu, e: &FrameError) { if gpu.context().check_health().is_err() { log::warn!("the GPU context is retired ({e}); the frame renders on the CPU"); return; } + // Not memory pressure: keeping fewer GPU frames wouldn't help (#1319). + if let FrameError::Validation(_) = e { + log::warn!("the GPU rejected the frame ({e}); it renders on the CPU, the next frame tries the GPU again"); + return; + } log::warn!("the GPU ran out of memory ({e}); the frame renders on the CPU and fewer GPU frames are kept"); if let Ok(mut c) = self.cache.lock() { c.gpu_out_of_memory();