From a44a77869a0eb5452494f00f17359db4fa2ad3f0 Mon Sep 17 00:00:00 2001 From: marc0246 <40955683+marc0246@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:22:55 +0100 Subject: [PATCH] Revert making `Raw{Buffer,Image}::bind_memory` unsafe (#2595) --- examples/gl-interop/main.rs | 4 +- vulkano/src/buffer/mod.rs | 3 +- vulkano/src/buffer/sys.rs | 28 +++++++------- vulkano/src/image/mod.rs | 2 +- vulkano/src/image/sys.rs | 76 ++++++++++++++++++------------------- 5 files changed, 56 insertions(+), 57 deletions(-) diff --git a/examples/gl-interop/main.rs b/examples/gl-interop/main.rs index 42ca256e..df6a8b98 100644 --- a/examples/gl-interop/main.rs +++ b/examples/gl-interop/main.rs @@ -325,9 +325,9 @@ mod linux { .export_fd(ExternalMemoryHandleType::OpaqueFd) .unwrap(); - // SAFETY: we just created this raw image and hasn't bound any memory to it. let image = Arc::new( - unsafe { raw_image.bind_memory([ResourceMemory::new_dedicated(image_memory)]) } + raw_image + .bind_memory([ResourceMemory::new_dedicated(image_memory)]) .map_err(|(err, _, _)| err) .unwrap(), ); diff --git a/vulkano/src/buffer/mod.rs b/vulkano/src/buffer/mod.rs index a6f15e66..fa4d40a8 100644 --- a/vulkano/src/buffer/mod.rs +++ b/vulkano/src/buffer/mod.rs @@ -404,8 +404,7 @@ impl Buffer { .map_err(AllocateBufferError::AllocateMemory)?; let allocation = unsafe { ResourceMemory::from_allocation(allocator, allocation) }; - // SAFETY: we just created this raw buffer and hasn't bound any memory to it. - let buffer = unsafe { raw_buffer.bind_memory(allocation) }.map_err(|(err, _, _)| { + let buffer = raw_buffer.bind_memory(allocation).map_err(|(err, _, _)| { err.map(AllocateBufferError::BindMemory) .map_validation(|err| err.add_context("RawBuffer::bind_memory")) })?; diff --git a/vulkano/src/buffer/sys.rs b/vulkano/src/buffer/sys.rs index f063792d..22a24600 100644 --- a/vulkano/src/buffer/sys.rs +++ b/vulkano/src/buffer/sys.rs @@ -103,6 +103,8 @@ impl RawBuffer { /// /// - `handle` must be a valid Vulkan object handle created from `device`. /// - `create_info` must match the info used to create the object. + /// - If the buffer has memory bound to it, `bind_memory` must not be called on the returned + /// `RawBuffer`. #[inline] pub unsafe fn from_handle( device: Arc, @@ -119,6 +121,8 @@ impl RawBuffer { /// /// - `handle` must be a valid Vulkan object handle created from `device`. /// - `create_info` must match the info used to create the object. + /// - If the buffer has memory bound to it, `bind_memory` must not be called on the returned + /// `RawBuffer`. /// - Caller must ensure that the handle will not be destroyed for the lifetime of returned /// `RawBuffer`. #[inline] @@ -241,11 +245,7 @@ impl RawBuffer { } /// Binds device memory to this buffer. - /// - /// # Safety - /// - /// - The buffer must not already have memory bound to it. - pub unsafe fn bind_memory( + pub fn bind_memory( self, allocation: ResourceMemory, ) -> Result, RawBuffer, ResourceMemory)> { @@ -257,15 +257,6 @@ impl RawBuffer { .map_err(|(err, buffer, allocation)| (err.into(), buffer, allocation)) } - /// Assume this buffer has memory bound to it. - /// - /// # Safety - /// - /// - The buffer must have memory bound to it. - pub unsafe fn assume_bound(self) -> Buffer { - Buffer::from_raw(self, BufferMemory::External) - } - fn validate_bind_memory( &self, allocation: &ResourceMemory, @@ -520,6 +511,15 @@ impl RawBuffer { Ok(Buffer::from_raw(self, BufferMemory::Normal(allocation))) } + /// Assume this buffer has memory bound to it. + /// + /// # Safety + /// + /// - The buffer must have memory bound to it. + pub unsafe fn assume_bound(self) -> Buffer { + Buffer::from_raw(self, BufferMemory::External) + } + /// Returns the memory requirements for this buffer. pub fn memory_requirements(&self) -> &MemoryRequirements { &self.memory_requirements diff --git a/vulkano/src/image/mod.rs b/vulkano/src/image/mod.rs index f2158787..e76e8808 100644 --- a/vulkano/src/image/mod.rs +++ b/vulkano/src/image/mod.rs @@ -166,7 +166,7 @@ impl Image { let allocation = unsafe { ResourceMemory::from_allocation(allocator, allocation) }; // SAFETY: we just created this raw image and hasn't bound any memory to it. - let image = unsafe { raw_image.bind_memory([allocation]) }.map_err(|(err, _, _)| { + let image = raw_image.bind_memory([allocation]).map_err(|(err, _, _)| { err.map(AllocateImageError::BindMemory) .map_validation(|err| err.add_context("RawImage::bind_memory")) })?; diff --git a/vulkano/src/image/sys.rs b/vulkano/src/image/sys.rs index 1362b9b7..2226c03b 100644 --- a/vulkano/src/image/sys.rs +++ b/vulkano/src/image/sys.rs @@ -129,6 +129,8 @@ impl RawImage { /// /// - `handle` must be a valid Vulkan object handle created from `device`. /// - `create_info` must match the info used to create the object. + /// - If the image has memory bound to it, `bind_memory` must not be called on the returned + /// `RawImage`. #[inline] pub unsafe fn from_handle( device: Arc, @@ -145,6 +147,8 @@ impl RawImage { /// /// - `handle` must be a valid Vulkan object handle created from `device`. /// - `create_info` must match the info used to create the object. + /// - If the image has memory bound to it, `bind_memory` must not be called on the returned + /// `RawImage`. /// - Caller must ensure the handle will not be destroyed for the lifetime of returned /// `RawImage`. #[inline] @@ -475,11 +479,7 @@ impl RawImage { /// - If `self.flags()` contains `ImageCreateFlags::DISJOINT`, and `self.tiling()` is /// `ImageTiling::DrmFormatModifier`, then `allocations` must contain exactly /// `self.drm_format_modifier().unwrap().1` elements. - /// - /// # Safety - /// - /// - The image must not already have memory bound to it. - pub unsafe fn bind_memory( + pub fn bind_memory( self, allocations: impl IntoIterator, ) -> Result< @@ -833,39 +833,6 @@ impl RawImage { Ok(()) } - /// Assume that this image already has memory backing it. - /// - /// # Safety - /// - /// - The image must be backed by suitable memory allocations. - pub unsafe fn assume_bound(self) -> Image { - let usage = self - .usage - .difference(ImageUsage::TRANSFER_SRC | ImageUsage::TRANSFER_DST); - - let layout = if usage.intersects(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT) - && usage - .difference(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT) - .is_empty() - { - ImageLayout::ShaderReadOnlyOptimal - } else if usage.intersects(ImageUsage::COLOR_ATTACHMENT) - && usage.difference(ImageUsage::COLOR_ATTACHMENT).is_empty() - { - ImageLayout::ColorAttachmentOptimal - } else if usage.intersects(ImageUsage::DEPTH_STENCIL_ATTACHMENT) - && usage - .difference(ImageUsage::DEPTH_STENCIL_ATTACHMENT) - .is_empty() - { - ImageLayout::DepthStencilAttachmentOptimal - } else { - ImageLayout::General - }; - - Image::from_raw(self, ImageMemory::External, layout) - } - /// # Safety /// /// - If `self.flags()` does not contain `ImageCreateFlags::DISJOINT`, then `allocations` must @@ -1011,6 +978,39 @@ impl RawImage { )) } + /// Assume that this image already has memory backing it. + /// + /// # Safety + /// + /// - The image must be backed by suitable memory allocations. + pub unsafe fn assume_bound(self) -> Image { + let usage = self + .usage + .difference(ImageUsage::TRANSFER_SRC | ImageUsage::TRANSFER_DST); + + let layout = if usage.intersects(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT) + && usage + .difference(ImageUsage::SAMPLED | ImageUsage::INPUT_ATTACHMENT) + .is_empty() + { + ImageLayout::ShaderReadOnlyOptimal + } else if usage.intersects(ImageUsage::COLOR_ATTACHMENT) + && usage.difference(ImageUsage::COLOR_ATTACHMENT).is_empty() + { + ImageLayout::ColorAttachmentOptimal + } else if usage.intersects(ImageUsage::DEPTH_STENCIL_ATTACHMENT) + && usage + .difference(ImageUsage::DEPTH_STENCIL_ATTACHMENT) + .is_empty() + { + ImageLayout::DepthStencilAttachmentOptimal + } else { + ImageLayout::General + }; + + Image::from_raw(self, ImageMemory::External, layout) + } + /// Returns the memory requirements for this image. /// /// - If the image is a swapchain image, this returns a slice with a length of 0.