From 2d73eb396785370c25bcd811f7f2940a51b7b1de Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 23 Apr 2025 16:13:04 +0200 Subject: [PATCH] [vk] move framebuffer cache into `CommandEncoder` This makes texture view destruction cheap since users of wgpu-hal are required to keep resources referenced by encoders alive for at least as long as the encoder is alive. This is also a prerequisite to implement rendering to slices of 3D textures (by creating temporary texture views). On the other hand the cache won't be as effective but that is probably ok, we can reevaluate the implementation if it turns out to be a problem. --- wgpu-hal/src/vulkan/adapter.rs | 1 - wgpu-hal/src/vulkan/command.rs | 25 ++++++++++++++++++++++--- wgpu-hal/src/vulkan/device.rs | 30 +----------------------------- wgpu-hal/src/vulkan/mod.rs | 25 ++++++++++++++----------- 4 files changed, 37 insertions(+), 44 deletions(-) diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index a2bc5479d..7ac9b3629 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -2054,7 +2054,6 @@ impl super::Adapter { features, workarounds: self.workarounds, render_passes: Mutex::new(Default::default()), - framebuffers: Mutex::new(Default::default()), sampler_cache: Mutex::new(super::sampler::SamplerCache::new( self.private_caps.maximum_samplers, )), diff --git a/wgpu-hal/src/vulkan/command.rs b/wgpu-hal/src/vulkan/command.rs index d9120e625..b8ce08068 100644 --- a/wgpu-hal/src/vulkan/command.rs +++ b/wgpu-hal/src/vulkan/command.rs @@ -1,9 +1,8 @@ use super::conv; - use arrayvec::ArrayVec; use ash::vk; - use core::{mem, ops::Range}; +use hashbrown::hash_map::Entry; const ALLOCATION_GRANULARITY: u32 = 16; const DST_IMAGE_LAYOUT: vk::ImageLayout = vk::ImageLayout::TRANSFER_DST_OPTIMAL; @@ -52,6 +51,26 @@ impl super::CommandEncoder { } } } + + fn make_framebuffer( + &mut self, + key: super::FramebufferKey, + ) -> Result { + Ok(match self.framebuffers.entry(key) { + Entry::Occupied(e) => *e.get(), + Entry::Vacant(e) => { + let vk_info = vk::FramebufferCreateInfo::default() + .render_pass(e.key().raw_pass) + .width(e.key().extent.width) + .height(e.key().extent.height) + .layers(e.key().extent.depth_or_array_layers) + .attachments(&e.key().attachments); + + let raw = unsafe { self.device.raw.create_framebuffer(&vk_info, None).unwrap() }; + *e.insert(raw) + } + }) + } } impl crate::CommandEncoder for super::CommandEncoder { @@ -796,7 +815,7 @@ impl crate::CommandEncoder for super::CommandEncoder { let raw_pass = self.device.make_render_pass(rp_key).unwrap(); fb_key.raw_pass = raw_pass; - let raw_framebuffer = self.device.make_framebuffer(fb_key).unwrap(); + let raw_framebuffer = self.make_framebuffer(fb_key).unwrap(); let vk_info = vk::RenderPassBeginInfo::default() .render_pass(raw_pass) diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index 7c4bec78e..2b5323a04 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -202,26 +202,6 @@ impl super::DeviceShared { }) } - pub fn make_framebuffer( - &self, - key: super::FramebufferKey, - ) -> Result { - Ok(match self.framebuffers.lock().entry(key) { - Entry::Occupied(e) => *e.get(), - Entry::Vacant(e) => { - let vk_info = vk::FramebufferCreateInfo::default() - .render_pass(e.key().raw_pass) - .width(e.key().extent.width) - .height(e.key().extent.height) - .layers(e.key().extent.depth_or_array_layers) - .attachments(&e.key().attachments); - - let raw = unsafe { self.raw.create_framebuffer(&vk_info, None).unwrap() }; - *e.insert(raw) - } - }) - } - fn make_memory_ranges<'a, I: 'a + Iterator>( &self, buffer: &'a super::Buffer, @@ -1312,15 +1292,6 @@ impl crate::Device for super::Device { }) } unsafe fn destroy_texture_view(&self, view: super::TextureView) { - { - let mut fbuf_lock = self.shared.framebuffers.lock(); - for (key, &raw_fbuf) in fbuf_lock.iter() { - if key.attachments.iter().any(|at_view| *at_view == view.raw) { - unsafe { self.shared.raw.destroy_framebuffer(raw_fbuf, None) }; - } - } - fbuf_lock.retain(|key, _| !key.attachments.iter().any(|at_view| *at_view == view.raw)); - } unsafe { self.shared.raw.destroy_image_view(view.raw, None) }; self.counters.texture_views.sub(1); @@ -1413,6 +1384,7 @@ impl crate::Device for super::Device { discarded: Vec::new(), rpass_debug_marker_active: false, end_of_pass_timer_query: None, + framebuffers: Default::default(), counters: Arc::clone(&self.counters), }) } diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index 42e9b2c68..f10e3901f 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -602,13 +602,6 @@ struct RenderPassKey { multiview: Option, } -#[derive(Clone, Eq, Hash, PartialEq)] -struct FramebufferKey { - raw_pass: vk::RenderPass, - attachments: ArrayVec, - extent: wgt::Extent3d, -} - struct DeviceShared { raw: ash::Device, family_index: u32, @@ -626,7 +619,6 @@ struct DeviceShared { workarounds: Workarounds, features: wgt::Features, render_passes: Mutex>, - framebuffers: Mutex>, sampler_cache: Mutex, memory_allocations_counter: InternalCounter, } @@ -636,9 +628,6 @@ impl Drop for DeviceShared { for &raw in self.render_passes.lock().values() { unsafe { self.raw.destroy_render_pass(raw, None) }; } - for &raw in self.framebuffers.lock().values() { - unsafe { self.raw.destroy_framebuffer(raw, None) }; - } if self.drop_guard.is_none() { unsafe { self.raw.destroy_device(None) }; } @@ -874,6 +863,13 @@ impl Temp { } } +#[derive(Clone, Eq, Hash, PartialEq)] +struct FramebufferKey { + raw_pass: vk::RenderPass, + attachments: ArrayVec, + extent: wgt::Extent3d, +} + pub struct CommandEncoder { raw: vk::CommandPool, device: Arc, @@ -910,6 +906,8 @@ pub struct CommandEncoder { /// the given pool & location. end_of_pass_timer_query: Option<(vk::QueryPool, u32)>, + framebuffers: FastHashMap, + counters: Arc, } @@ -931,6 +929,11 @@ impl Drop for CommandEncoder { // fields. self.device.raw.destroy_command_pool(self.raw, None); } + + for (_, fb) in self.framebuffers.drain() { + unsafe { self.device.raw.destroy_framebuffer(fb, None) }; + } + self.counters.command_encoders.sub(1); } }