diff --git a/crates/gpu/src/context.rs b/crates/gpu/src/context.rs index e220a146b..f598df4af 100644 --- a/crates/gpu/src/context.rs +++ b/crates/gpu/src/context.rs @@ -73,6 +73,8 @@ struct Lease { pool: Weak>, /// The [`Enc`] that took it (see [`Pool::owned`]). owner: u64, + /// The pool's [`Pool::generation`] when it was taken. + generation: u64, } impl std::fmt::Debug for Lease { @@ -86,7 +88,7 @@ impl Drop for Lease { if let Some(pool) = self.pool.upgrade() && let Ok(mut p) = pool.lock() { - p.release_owned(self.texture.clone(), self.owner); + p.release_owned(self.texture.clone(), self.owner, self.generation); } } } @@ -117,6 +119,8 @@ pub(crate) struct Pool { /// working texture of another `Enc` before it is gone. owned: HashMap>>, next_owner: u64, + /// Raised by [`Pool::discard`]: a texture taken before it never comes back to the pool. + generation: u64, } /// In-`Enc` texture reuse ([`Pool::owned`]); tests turn it off to compare. @@ -207,7 +211,10 @@ impl Pool { } } - fn release_owned(&mut self, t: wgpu::Texture, owner: u64) { + fn release_owned(&mut self, t: wgpu::Texture, owner: u64, generation: u64) { + if generation != self.generation { + return; + } if let Some(m) = self.owned.get_mut(&owner) { m.entry((t.width(), t.height())).or_default().push(t); return; @@ -237,6 +244,18 @@ impl Pool { self.free.clear(); self.bytes = 0; } + + /// After a failed frame (#1319): drop every kept texture, the free ones and those waiting + /// for an open encoder or kept by one, and the textures still out never return. The + /// encoders' bookkeeping stays, so reuse remains ordered. + fn discard(&mut self) { + self.clear(); + self.released.clear(); + for kept in self.owned.values_mut() { + kept.clear(); + } + self.generation = self.generation.wrapping_add(1); + } } /// Uniform parameter block shared by every kernel (see `shaders/common.wgsl`). @@ -867,10 +886,10 @@ impl GpuContext { /// one is zeroed. fn pooled(&self, w: u32, h: u32, owner: u64) -> (GpuImage, bool) { let (w, h) = (w.max(1), h.max(1)); - let reused = self.pool.lock().ok().and_then(|mut p| p.take_owned(owner, w, h).or_else(|| p.take(w, h))); + let (reused, generation) = self.pool.lock().map(|mut p| (p.take_owned(owner, w, h).or_else(|| p.take(w, h)), p.generation)).unwrap_or((None, u64::MAX)); let was_reused = reused.is_some(); let texture = reused.unwrap_or_else(|| self.texture(w, h)); - let lease = Arc::new(Lease { texture: texture.clone(), pool: Arc::downgrade(&self.pool), owner }); + let lease = Arc::new(Lease { texture: texture.clone(), pool: Arc::downgrade(&self.pool), owner, generation }); (GpuImage { texture, width: w, height: h, _lease: Some(lease), levels: None }, was_reused) } @@ -985,6 +1004,9 @@ impl GpuContext { /// may be one the device rejected (#1319). Later frames allocate afresh. pub fn discard_kept(&self) { self.clear_uploads(); + if let Ok(mut p) = self.pool.lock() { + p.discard(); + } if let Ok(mut z) = self.zeros.lock() { z.clear(); } diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index f9d62b901..a5df345d5 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -780,6 +780,49 @@ fn a_rejected_texture_fails_the_frame_not_the_compositor() { assert_eq!(g.ctx.pool_stats().0, hits, "the failed frame's texture wasn't reused"); } +/// A texture the device rejected (deliberately too wide), made outside any frame under a scope +/// of its own: the view a frame then makes of it is the reported validation error. +fn rejected_texture(g: &Gpu) -> wgpu::Texture { + let scope = g.ctx.device.push_error_scope(wgpu::ErrorFilter::Validation); + let too_wide = g.ctx.device.limits().max_texture_dimension_2d.saturating_add(1); + let t = g.ctx.device.create_texture(&wgpu::TextureDescriptor { + label: Some("deliberately too wide"), + size: wgpu::Extent3d { width: too_wide, height: 1, depth_or_array_layers: 1 }, + mip_level_count: 1, + sample_count: 1, + dimension: wgpu::TextureDimension::D2, + format: wgpu::TextureFormat::Rgba8Unorm, + usage: wgpu::TextureUsages::TEXTURE_BINDING, + view_formats: &[], + }); + assert!(pollster::block_on(scope.pop()).is_some(), "the device rejected the texture"); + t +} + +/// No texture of a failed frame is reused: not one released while another encoder is still +/// open (it waits for that encoder), nor one taken before the failure and handed back after. +#[test] +fn a_failed_frame_returns_no_texture_to_the_pool() { + hold_gpu_lock(); + let Some(g) = Gpu::headless() else { return }; + let rejected = rejected_texture(&g); + let earlier = g.within_memory(|| crate::context::Enc::new(&g.ctx).image(48, 16)).expect("an earlier frame"); + let mut other = crate::context::Enc::new(&g.ctx); + other.encoder(); + g.within_memory(|| (crate::context::Enc::new(&g.ctx).image(64, 32), rejected.create_view(&Default::default()))).expect_err("the frame fails"); + drop(earlier); + drop(other); + let (hits, _) = g.ctx.pool_stats(); + let (a, b) = g + .within_memory(|| { + let mut e = crate::context::Enc::new(&g.ctx); + (e.image(64, 32), e.image(48, 16)) + }) + .expect("the next frame"); + assert_eq!(((a.width, a.height), (b.width, b.height)), ((64, 32), (48, 16))); + assert_eq!(g.ctx.pool_stats().0, hits, "no texture of the failed frame was reused"); +} + /// A buffer whose allocation fails (here: over the device's buffer size limit; on Adreno an /// internal `ERROR_INITIALIZATION_FAILED`) is reported to the caller's error scope instead of /// panicking in `create_buffer_init`'s mapped range on a frame thread (#591).