From c731f79f81d0c79ae53520a6b9158c8097053207 Mon Sep 17 00:00:00 2001 From: janis Date: Fri, 10 Apr 2026 01:47:34 +0200 Subject: [PATCH] switching to more generic DeviceObject impl --- crates/renderer/src/buffers.rs | 4 +- crates/renderer/src/commands.rs | 5 +- crates/renderer/src/device.rs | 182 +++++++++++++------------------ crates/renderer/src/images.rs | 23 ++-- crates/renderer/src/pipeline.rs | 73 +++++++++---- crates/renderer/src/swapchain.rs | 6 +- crates/renderer/src/sync.rs | 15 ++- 7 files changed, 157 insertions(+), 151 deletions(-) diff --git a/crates/renderer/src/buffers.rs b/crates/renderer/src/buffers.rs index 7bcd9ef..95469b0 100644 --- a/crates/renderer/src/buffers.rs +++ b/crates/renderer/src/buffers.rs @@ -113,9 +113,9 @@ impl Buffer { } Ok(Self { - buffer: DeviceObject::new(buffer, device.clone(), desc.name.clone()), + buffer: DeviceObject::new_debug_named(device.clone(), buffer, desc.name.clone()), desc, - alloc: Allocation::Owned(DeviceObject::new_without_name(alloc, device)), + alloc: Allocation::Owned(DeviceObject::new(device, alloc)), }) } diff --git a/crates/renderer/src/commands.rs b/crates/renderer/src/commands.rs index 1fbc883..e9afe9c 100644 --- a/crates/renderer/src/commands.rs +++ b/crates/renderer/src/commands.rs @@ -534,7 +534,7 @@ pub mod traits { unsafe { self.device().dev().cmd_push_constants( self.handle(), - layout.handle(), + layout.raw(), stage, offset, bytes, @@ -575,12 +575,11 @@ pub mod traits { descriptor_sets: &[vk::DescriptorSet], ) { // assert_eq!(self.state(), CommandBufferState::Recording); - use crate::device::DeviceOwned; unsafe { self.device().dev().cmd_bind_descriptor_sets( self.handle(), bind_point, - layout.handle(), + layout.raw(), 0, descriptor_sets, &[], diff --git a/crates/renderer/src/device.rs b/crates/renderer/src/device.rs index 233475a..5a51fed 100644 --- a/crates/renderer/src/device.rs +++ b/crates/renderer/src/device.rs @@ -16,6 +16,7 @@ use raw_window_handle::RawDisplayHandle; use crate::{ Instance, PhysicalDeviceFeatures, PhysicalDeviceInfo, Result, + device::asdf::traits::ExternallyManagedObject, pipeline::pipeline_cache::PipelineCache, queue::{DeviceQueueInfos, DeviceQueues, Queue}, sync::{self, BinarySemaphore, TimelineSemaphore}, @@ -54,14 +55,6 @@ pub(crate) struct DeviceExtensions { type GpuAllocation = gpu_allocator::vulkan::Allocation; -impl DeviceHandle for GpuAllocation { - unsafe fn destroy(&mut self, device: &Device) { - let mut swapped = GpuAllocation::default(); - std::mem::swap(self, &mut swapped); - _ = device.alloc2.lock().free(swapped); - } -} - #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] pub enum AllocationStrategy { #[default] @@ -93,7 +86,7 @@ impl Allocation { pub(crate) fn allocation_mut(&mut self) -> Option<&mut GpuAllocation> { match self { Allocation::Owned(obj) => Some(obj), - Allocation::Shared(arc) => Arc::get_mut(arc).map(|alloc| &mut alloc.inner), + Allocation::Shared(arc) => Arc::get_mut(arc).map(DerefMut::deref_mut), Allocation::Unmanaged => None, } } @@ -700,76 +693,7 @@ impl DeviceOwnedDebugObject { } } -#[derive(Debug)] -pub struct DeviceObject { - inner: T, - device: Device, - #[cfg(debug_assertions)] - name: Option>, -} - -impl Deref for DeviceObject { - type Target = T; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} - -impl DerefMut for DeviceObject { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.inner - } -} - -impl DeviceObject { - pub fn new(inner: T, device: Device, name: Option>) -> Self - where - T: vk::Handle + Clone, - { - unsafe { - if let Some(name) = name.as_ref() { - device.debug_name_object(inner.clone(), name); - } - } - - Self { - inner, - device, - #[cfg(debug_assertions)] - name, - } - } - pub fn new_without_name(inner: T, device: Device) -> Self { - Self { - inner, - device, - #[cfg(debug_assertions)] - name: None, - } - } - pub fn device(&self) -> &Device { - &self.device - } - pub fn name(&self) -> Option<&str> { - #[cfg(debug_assertions)] - { - self.name.as_deref() - } - #[cfg(not(debug_assertions))] - { - None - } - } -} - -impl Drop for DeviceObject { - fn drop(&mut self) { - unsafe { - self.inner.destroy(&self.device); - } - } -} +pub use asdf::{DeviceObject, InnerDeviceObject}; pub trait DeviceHandle { /// # Safety @@ -777,35 +701,26 @@ pub trait DeviceHandle { unsafe fn destroy(&mut self, device: &Device); } -impl DeviceHandle for vk::Semaphore { - unsafe fn destroy(&mut self, device: &Device) { +impl> ExternallyManagedObject for Mutex { + unsafe fn destroy(self, owner: &O) { + // Safety guarantee is upheld by the caller. + unsafe { self.into_inner().destroy(owner) }; + } +} + +impl> ExternallyManagedObject for vk::Buffer { + unsafe fn destroy(self, device: &T) { unsafe { - device.dev().destroy_semaphore(*self, None); + device.as_ref().raw.destroy_buffer(self, None); } } } -impl DeviceHandle for vk::Fence { - unsafe fn destroy(&mut self, device: &Device) { +impl> ExternallyManagedObject for vk::SwapchainKHR { + unsafe fn destroy(self, device: &T) { unsafe { - device.dev().destroy_fence(*self, None); - } - } -} - -impl DeviceHandle for vk::Buffer { - unsafe fn destroy(&mut self, device: &Device) { - unsafe { - device.dev().destroy_buffer(*self, None); - } - } -} - -impl DeviceHandle for vk::SwapchainKHR { - unsafe fn destroy(&mut self, device: &Device) { - unsafe { - if let Some(swapchain) = device.device_extensions.swapchain.as_ref() { - swapchain.destroy_swapchain(*self, None) + if let Some(swapchain) = device.as_ref().device_extensions.swapchain.as_ref() { + swapchain.destroy_swapchain(self, None) } } } @@ -1042,11 +957,13 @@ pub(crate) mod asdf { } } + pub type InnerDeviceObject = DeviceObject>; + /// A wrapper for vulkan types which are owned by the device, taking care of destruction. #[derive(Debug)] pub struct DeviceObject< T: traits::ExternallyManagedObject, - O: AsRef = Arc, + O: AsRef = super::Device, > { inner: ExternallyManagedObject, #[allow(dead_code)] @@ -1076,9 +993,54 @@ pub(crate) mod asdf { Self { inner, name: None } } + + pub fn new_debug_named_with( + owner: O, + inner: T, + name: Option>, + debug_namable: impl FnOnce(&T) -> D, + ) -> Self + where + D: traits::DebugNameable, + { + let name = name.map(Into::into); + if let Some(ref name) = name { + traits::DebugNameable::debug_name(&debug_namable(&inner), owner.as_ref(), name); + } + + let obj = ExternallyManagedObject::new(inner, owner); + + Self { inner: obj, name } + } + pub fn device(&self) -> &O { self.inner.owner() } + + pub fn name(&self) -> Option<&str> { + self.name.as_ref().map(|n| &**n) + } + + pub fn map_inner(self, f: impl FnOnce(T) -> U) -> DeviceObject + where + U: traits::ExternallyManagedObject, + { + DeviceObject { + inner: self.inner.map_inner(f), + name: self.name, + } + } + + pub fn map_owner(self, f: impl FnOnce(O) -> U) -> DeviceObject + where + T: traits::ExternallyManagedObject, + U: AsRef, + { + DeviceObject { + inner: self.inner.map_owner(f), + name: self.name, + } + } } impl Deref for DeviceObject @@ -1089,7 +1051,17 @@ pub(crate) mod asdf { type Target = T; fn deref(&self) -> &Self::Target { - &self.inner + &*self.inner + } + } + + impl DerefMut for DeviceObject + where + T: traits::ExternallyManagedObject, + O: AsRef, + { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut *self.inner } } @@ -1098,7 +1070,7 @@ pub(crate) mod asdf { use super::*; - impl> traits::ExternallyManagedObject for ash::vk::Semaphore { + impl> traits::ExternallyManagedObject for vk::Semaphore { unsafe fn destroy(self, device: &T) { unsafe { device.as_ref().raw.destroy_semaphore(self, None); diff --git a/crates/renderer/src/images.rs b/crates/renderer/src/images.rs index 140e31b..dce665e 100644 --- a/crates/renderer/src/images.rs +++ b/crates/renderer/src/images.rs @@ -1,7 +1,10 @@ use std::{borrow::Cow, mem::ManuallyDrop, sync::Arc}; use crate::{ - device::{Allocation, AllocationStrategy, DeviceHandle, DeviceObject, QueueFlags}, + device::{ + Allocation, AllocationStrategy, DeviceInner, DeviceObject, QueueFlags, + asdf::traits::ExternallyManagedObject, + }, swapchain::Swapchain, util::weak_vec::WeakVec, }; @@ -114,10 +117,10 @@ enum ImageInner { Allocated(DeviceObject, Allocation), } -impl DeviceHandle for vk::Image { - unsafe fn destroy(&mut self, device: &Device) { +impl> ExternallyManagedObject for vk::Image { + unsafe fn destroy(self, device: &T) { unsafe { - device.dev().destroy_image(*self, None); + device.as_ref().raw.destroy_image(self, None); } } } @@ -175,7 +178,7 @@ impl Image { Self::new_with_allocation_unchecked( device.clone(), image, - Allocation::Owned(DeviceObject::new_without_name(alloc, device)), + Allocation::Owned(DeviceObject::new(device, alloc)), desc, ) } @@ -197,7 +200,7 @@ impl Image { Ok(Self { image: ImageInner::Allocated( - DeviceObject::new(image, device, desc.name.clone()), + DeviceObject::new_debug_named(device, image, desc.name.clone()), allocation, ), desc, @@ -475,7 +478,7 @@ impl Image { let view = unsafe { device.raw.create_image_view(&create_info, None)? }; Ok(ManuallyDrop::new(ImageView { - view: DeviceObject::new(view, device.clone(), desc.name.clone()), + view: DeviceObject::new_debug_named(device.clone(), view, desc.name.clone()), desc, image: self.clone(), })) @@ -682,10 +685,10 @@ impl ImageView { } } -impl DeviceHandle for vk::ImageView { - unsafe fn destroy(&mut self, device: &Device) { +impl> ExternallyManagedObject for vk::ImageView { + unsafe fn destroy(self, device: &T) { unsafe { - device.dev().destroy_image_view(*self, None); + device.as_ref().raw.destroy_image_view(self, None); } } } diff --git a/crates/renderer/src/pipeline.rs b/crates/renderer/src/pipeline.rs index d40a96c..c960c90 100644 --- a/crates/renderer/src/pipeline.rs +++ b/crates/renderer/src/pipeline.rs @@ -1,10 +1,13 @@ use std::{borrow::Cow, path::Path, sync::Arc}; use ash::{ext, prelude::*, vk}; +use parking_lot::Mutex; use crate::{ define_device_owned_handle, - device::{Device, DeviceHandle, DeviceObject}, + device::{ + Device, DeviceHandle, DeviceInner, DeviceObject, asdf::traits::ExternallyManagedObject, + }, make_extension, }; @@ -222,10 +225,19 @@ impl DeviceHandle for vk::DescriptorPool { unsafe { device.dev().destroy_descriptor_pool(*self, None) }; } } +impl> ExternallyManagedObject for vk::DescriptorPool { + unsafe fn destroy(self, device: &T) { + // SAFETY: We have exclusive ownership of the descriptor pool, so it's safe to destroy it. + unsafe { + device.as_ref().raw.destroy_descriptor_pool(self, None); + } + } +} #[derive(Debug)] pub struct DescriptorPool { pool: DeviceObject, + lock: Mutex<()>, } impl DescriptorPool { @@ -238,20 +250,25 @@ impl DescriptorPool { let handle = unsafe { device.dev().create_descriptor_pool(info, None)? }; Ok(Self { - pool: DeviceObject::new(handle, device, desc.name), + pool: DeviceObject::new_debug_named(device, handle, desc.name), + lock: Mutex::new(()), }) } pub fn allocate(&self, descs: &[DescriptorSetAllocDesc]) -> VkResult> { let layouts = descs .iter() - .map(|desc| desc.layout.handle()) + .map(|desc| desc.layout.raw()) .collect::>(); let info = &vk::DescriptorSetAllocateInfo::default() .descriptor_pool(*self.pool) .set_layouts(&layouts); - let sets = unsafe { self.pool.device().raw.allocate_descriptor_sets(info)? }; + + let sets = unsafe { + let _lock = self.lock.lock(); + self.pool.device().raw.allocate_descriptor_sets(info)? + }; for (&set, desc) in sets.iter().zip(descs) { if let Some(name) = desc.name.as_ref() { @@ -264,6 +281,7 @@ impl DescriptorPool { #[allow(dead_code)] pub fn reset(&self) -> VkResult<()> { + let _lock = self.lock.lock(); unsafe { self.pool .device() @@ -273,9 +291,14 @@ impl DescriptorPool { } } -impl DeviceHandle for vk::DescriptorSetLayout { - unsafe fn destroy(&mut self, device: &Device) { - unsafe { device.raw.destroy_descriptor_set_layout(*self, None) }; +impl> ExternallyManagedObject for vk::DescriptorSetLayout { + unsafe fn destroy(self, device: &T) { + unsafe { + device + .as_ref() + .raw + .destroy_descriptor_set_layout(self, None); + } } } @@ -319,7 +342,7 @@ impl DescriptorSetLayout { let layout = unsafe { device.raw.create_descriptor_set_layout(&info, None)? }; Ok(Self { - layout: DeviceObject::new(layout, device, desc.name), + layout: DeviceObject::new_debug_named(device, layout, desc.name), }) } @@ -330,9 +353,11 @@ impl DescriptorSetLayout { use crate::device::DeviceOwned; -impl DeviceHandle for vk::PipelineLayout { - unsafe fn destroy(&mut self, device: &Device) { - unsafe { device.raw.destroy_pipeline_layout(*self, None) }; +impl> ExternallyManagedObject for vk::PipelineLayout { + unsafe fn destroy(self, device: &T) { + unsafe { + device.as_ref().raw.destroy_pipeline_layout(self, None); + } } } @@ -354,7 +379,7 @@ impl PipelineLayout { let layout = unsafe { device.raw.create_pipeline_layout(info, None)? }; Ok(Self { - layout: DeviceObject::new(layout, device, desc.name), + layout: DeviceObject::new_debug_named(device, layout, desc.name), }) } @@ -458,9 +483,11 @@ impl Sampler { } } -impl DeviceHandle for vk::ShaderModule { - unsafe fn destroy(&mut self, device: &Device) { - unsafe { device.raw.destroy_shader_module(*self, None) }; +impl> ExternallyManagedObject for vk::ShaderModule { + unsafe fn destroy(self, device: &T) { + unsafe { + device.as_ref().raw.destroy_shader_module(self, None); + } } } @@ -487,7 +514,7 @@ impl ShaderModule { let size = reader.read(bytemuck::cast_slice_mut(buffer.as_mut_slice()))?; buffer.resize(size / 4, 0); - Ok(Self::new_from_memory(device, &buffer)?) + Self::new_from_memory(device, &buffer) } pub fn new_from_memory(device: Device, buffer: &[u32]) -> crate::Result { @@ -496,7 +523,7 @@ impl ShaderModule { let module = unsafe { device.dev().create_shader_module(info, None)? }; Ok(Self { - module: DeviceObject::new(module, device, None), + module: DeviceObject::new(device, module), }) } } @@ -507,9 +534,11 @@ pub struct Pipeline { bind_point: vk::PipelineBindPoint, } -impl DeviceHandle for vk::Pipeline { - unsafe fn destroy(&mut self, device: &Device) { - unsafe { device.raw.destroy_pipeline(*self, None) }; +impl> ExternallyManagedObject for vk::Pipeline { + unsafe fn destroy(self, device: &T) { + unsafe { + device.as_ref().raw.destroy_pipeline(self, None); + } } } @@ -550,7 +579,7 @@ impl Pipeline { }; Ok(Self { - pipeline: DeviceObject::new(pipeline, device, desc.name), + pipeline: DeviceObject::new_debug_named(device, pipeline, desc.name), bind_point: vk::PipelineBindPoint::COMPUTE, }) } @@ -718,7 +747,7 @@ impl Pipeline { }; Ok(Self { - pipeline: DeviceObject::new(pipeline, device, desc.name), + pipeline: DeviceObject::new_debug_named(device, pipeline, desc.name), bind_point: vk::PipelineBindPoint::GRAPHICS, }) } diff --git a/crates/renderer/src/swapchain.rs b/crates/renderer/src/swapchain.rs index 1d33162..2a2152f 100644 --- a/crates/renderer/src/swapchain.rs +++ b/crates/renderer/src/swapchain.rs @@ -488,10 +488,10 @@ impl Swapchain { .swapchain .clone() .expect("swapchain extension not loaded"), - swapchain: DeviceObject::new( - swapchain, + swapchain: DeviceObject::new_debug_named( device, - Some(format!("swapchain-{:x}", swapchain.as_raw()).into()), + swapchain, + Some(format!("swapchain-{:x}", swapchain.as_raw())), ), images, config, diff --git a/crates/renderer/src/sync.rs b/crates/renderer/src/sync.rs index a3a2c4b..05dbe97 100644 --- a/crates/renderer/src/sync.rs +++ b/crates/renderer/src/sync.rs @@ -7,7 +7,10 @@ use std::{ use crate::device::{ DevicePools, Pool, PoolObject, Pooled, - asdf::{DeviceObject, traits::ExternallyManagedObject as ExternallyManagedObjectTrait}, + asdf::{ + DeviceObject, InnerDeviceObject, + traits::ExternallyManagedObject as ExternallyManagedObjectTrait, + }, }; use crate::{Result, device::DeviceInner}; @@ -157,7 +160,7 @@ impl SyncThreadpool { pub enum Fence { Dedicated { - fence: DeviceObject, + fence: InnerDeviceObject, }, Pooled { fence: PoolObject>>, @@ -182,10 +185,10 @@ impl ExternallyManagedObjectTrait>> for vk::Fence { } } -impl ExternallyManagedObjectTrait> for vk::Fence { - unsafe fn destroy(self, device: &Arc) { +impl> ExternallyManagedObjectTrait for vk::Fence { + unsafe fn destroy(self, device: &T) { unsafe { - device.raw.destroy_fence(self, None); + device.as_ref().raw.destroy_fence(self, None); } } } @@ -333,7 +336,7 @@ impl From for SemaphoreInner { pub enum Semaphore { Dedicated { - semaphore: DeviceObject, + semaphore: InnerDeviceObject, }, Pooled { #[allow(private_interfaces)]