From e90c27ebbd163a7ddadf057f9e97d79e0f8d7c5f Mon Sep 17 00:00:00 2001 From: chyyran Date: Sat, 24 Aug 2024 23:26:00 -0400 Subject: [PATCH] rt(vk): use manuallydrop for VulkanBuffer rather than Option --- librashader-runtime-vk/src/filter_chain.rs | 6 +- librashader-runtime-vk/src/memory.rs | 69 +++++++++++++++++----- librashader-runtime-vk/src/util.rs | 49 --------------- 3 files changed, 56 insertions(+), 68 deletions(-) diff --git a/librashader-runtime-vk/src/filter_chain.rs b/librashader-runtime-vk/src/filter_chain.rs index a084711..ca13c1f 100644 --- a/librashader-runtime-vk/src/filter_chain.rs +++ b/librashader-runtime-vk/src/filter_chain.rs @@ -9,7 +9,7 @@ use crate::options::{FilterChainOptionsVulkan, FrameOptionsVulkan}; use crate::queue_selection::get_graphics_queue; use crate::samplers::SamplerSet; use crate::texture::{InputImage, OwnedImage, OwnedImageLayout, VulkanImage}; -use crate::{error, util}; +use crate::{error, memory, util}; use ash::vk; use librashader_common::{ImageFormat, Size, Viewport}; @@ -80,7 +80,7 @@ impl TryFrom for VulkanObjects { // let memory_properties = // instance.get_physical_device_memory_properties(vulkan.physical_device); - let alloc = util::create_allocator(device.clone(), instance, vulkan.physical_device)?; + let alloc = memory::create_allocator(device.clone(), instance, vulkan.physical_device)?; Ok(VulkanObjects { device: Arc::new(device), @@ -103,7 +103,7 @@ impl TryFrom<(vk::PhysicalDevice, ash::Instance, ash::Device)> for VulkanObjects // let memory_properties = value.1.get_physical_device_memory_properties(value.0); - let alloc = util::create_allocator(device.clone(), value.1, value.0)?; + let alloc = memory::create_allocator(device.clone(), value.1, value.0)?; Ok(VulkanObjects { alloc, diff --git a/librashader-runtime-vk/src/memory.rs b/librashader-runtime-vk/src/memory.rs index d810492..fdce8f4 100644 --- a/librashader-runtime-vk/src/memory.rs +++ b/librashader-runtime-vk/src/memory.rs @@ -1,8 +1,10 @@ use crate::error; use crate::error::FilterChainError; use ash::vk; -use gpu_allocator::vulkan::{Allocation, AllocationCreateDesc, AllocationScheme, Allocator}; -use gpu_allocator::MemoryLocation; +use gpu_allocator::vulkan::{ + Allocation, AllocationCreateDesc, AllocationScheme, Allocator, AllocatorCreateDesc, +}; +use gpu_allocator::{AllocationSizes, MemoryLocation}; use librashader_runtime::uniforms::UniformStorageAccess; use parking_lot::Mutex; @@ -56,7 +58,7 @@ impl Drop for VulkanImageMemory { pub struct VulkanBuffer { pub handle: vk::Buffer, device: Arc, - memory: Option, + allocation: ManuallyDrop, allocator: Arc>, size: vk::DeviceSize, } @@ -85,12 +87,11 @@ impl VulkanBuffer { allocation_scheme: AllocationScheme::DedicatedBuffer(buffer), })?; - // let alloc = device.allocate_memory(&alloc_info, None)?; device.bind_buffer_memory(buffer, alloc.memory(), 0)?; Ok(VulkanBuffer { handle: buffer, - memory: Some(alloc), + allocation: ManuallyDrop::new(alloc), allocator: Arc::clone(allocator), size: size as vk::DeviceSize, device: device.clone(), @@ -99,10 +100,7 @@ impl VulkanBuffer { } pub fn as_mut_slice(&mut self) -> error::Result<&mut [u8]> { - let Some(allocation) = self.memory.as_mut() else { - return Err(FilterChainError::AllocationDoesNotExist); - }; - let Some(allocation) = allocation.mapped_slice_mut() else { + let Some(allocation) = self.allocation.mapped_slice_mut() else { return Err(FilterChainError::AllocationDoesNotExist); }; Ok(allocation) @@ -112,12 +110,10 @@ impl VulkanBuffer { impl Drop for VulkanBuffer { fn drop(&mut self) { unsafe { - if let Some(allocation) = self.memory.take() { - if let Err(e) = self.allocator.lock().free(allocation) { - println!( - "librashader-runtime-vk: [warn] failed to deallocate buffer memory {e}" - ) - } + // SAFETY: things can not be double dropped. + let allocation = ManuallyDrop::take(&mut self.allocation); + if let Err(e) = self.allocator.lock().free(allocation) { + println!("librashader-runtime-vk: [warn] failed to deallocate buffer memory {e}") } if self.handle != vk::Buffer::null() { @@ -147,7 +143,7 @@ impl RawVulkanBuffer { ) -> error::Result { let buffer = ManuallyDrop::new(VulkanBuffer::new(device, allocator, usage, size)?); - let Some(ptr) = buffer.memory.as_ref().map(|m| m.mapped_ptr()).flatten() else { + let Some(ptr) = buffer.allocation.mapped_ptr() else { return Err(FilterChainError::AllocationDoesNotExist); }; @@ -204,3 +200,44 @@ impl DerefMut for RawVulkanBuffer { } } } + +#[allow(unused)] +pub fn find_vulkan_memory_type( + props: &vk::PhysicalDeviceMemoryProperties, + device_reqs: u32, + host_reqs: vk::MemoryPropertyFlags, +) -> error::Result { + for i in 0..vk::MAX_MEMORY_TYPES { + if device_reqs & (1 << i) != 0 + && props.memory_types[i].property_flags & host_reqs == host_reqs + { + return Ok(i as u32); + } + } + + if host_reqs == vk::MemoryPropertyFlags::empty() { + Err(FilterChainError::VulkanMemoryError(device_reqs)) + } else { + Ok(find_vulkan_memory_type( + props, + device_reqs, + vk::MemoryPropertyFlags::empty(), + )?) + } +} + +pub fn create_allocator( + device: ash::Device, + instance: ash::Instance, + physical_device: vk::PhysicalDevice, +) -> error::Result>> { + let alloc = Allocator::new(&AllocatorCreateDesc { + instance, + device, + physical_device, + debug_settings: Default::default(), + buffer_device_address: false, + allocation_sizes: AllocationSizes::default(), + })?; + Ok(Arc::new(Mutex::new(alloc))) +} diff --git a/librashader-runtime-vk/src/util.rs b/librashader-runtime-vk/src/util.rs index 090bcd9..3a06d60 100644 --- a/librashader-runtime-vk/src/util.rs +++ b/librashader-runtime-vk/src/util.rs @@ -1,12 +1,4 @@ use ash::vk; -use gpu_allocator::vulkan::{Allocator, AllocatorCreateDesc}; - -use gpu_allocator::AllocationSizes; -use parking_lot::Mutex; -use std::sync::Arc; - -use crate::error; -use crate::error::FilterChainError; use librashader_reflect::reflect::semantics::BindingStage; pub fn binding_stage_to_vulkan_stage(stage_mask: BindingStage) -> vk::ShaderStageFlags { @@ -22,31 +14,6 @@ pub fn binding_stage_to_vulkan_stage(stage_mask: BindingStage) -> vk::ShaderStag mask } -#[allow(unused)] -pub fn find_vulkan_memory_type( - props: &vk::PhysicalDeviceMemoryProperties, - device_reqs: u32, - host_reqs: vk::MemoryPropertyFlags, -) -> error::Result { - for i in 0..vk::MAX_MEMORY_TYPES { - if device_reqs & (1 << i) != 0 - && props.memory_types[i].property_flags & host_reqs == host_reqs - { - return Ok(i as u32); - } - } - - if host_reqs == vk::MemoryPropertyFlags::empty() { - Err(FilterChainError::VulkanMemoryError(device_reqs)) - } else { - Ok(find_vulkan_memory_type( - props, - device_reqs, - vk::MemoryPropertyFlags::empty(), - )?) - } -} - #[inline(always)] pub unsafe fn vulkan_image_layout_transition_levels( device: &ash::Device, @@ -90,19 +57,3 @@ pub unsafe fn vulkan_image_layout_transition_levels( ) } } - -pub fn create_allocator( - device: ash::Device, - instance: ash::Instance, - physical_device: vk::PhysicalDevice, -) -> error::Result>> { - let alloc = Allocator::new(&AllocatorCreateDesc { - instance, - device, - physical_device, - debug_settings: Default::default(), - buffer_device_address: false, - allocation_sizes: AllocationSizes::default(), - })?; - Ok(Arc::new(Mutex::new(alloc))) -}