Support creating buffers and images that are not backed by vulkano-ma…

…naged memory (#2413) * Support creating images that are not backed by vulkano managed memory * Support creating buffers that are not backed by vulkano managed memory * Add a way to create a RawImage that doesn't own the inner image * Add a way to create a RawBuffer that doesn't own the inner buffer * Apply suggestions from code review Co-authored-by: marc0246 <[email protected]> * fixup! Support creating images that are not backed by vulkano managed memory * fixup! Support creating buffers that are not backed by vulkano managed memory --------- Co-authored-by: marc0246 <[email protected]>
vulkano-rs · Dec 18, 2023 · 22276cd · 22276cd
1 parent b8a9e94
commit 22276cd
Show file tree

Hide file tree

Showing 8 changed files with 148 additions and 23 deletions.
diff --git a/examples/gl-interop/main.rs b/examples/gl-interop/main.rs
@@ -160,12 +160,13 @@ mod linux {
             .export_fd(ExternalMemoryHandleType::OpaqueFd)
             .unwrap();
 
-        let image = Arc::new(
+        // 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)])
                 .map_err(|(err, _, _)| err)
-                .unwrap(),
-        );
+                .unwrap()
+        });
 
         let image_view = ImageView::new_default(image).unwrap();
 

diff --git a/vulkano/src/buffer/allocator.rs b/vulkano/src/buffer/allocator.rs
@@ -308,7 +308,7 @@ where
             let arena = self.arena.as_ref().unwrap();
             let allocation = match arena.buffer.memory() {
                 BufferMemory::Normal(a) => a,
-                BufferMemory::Sparse => unreachable!(),
+                BufferMemory::Sparse | BufferMemory::External => unreachable!(),
             };
             let arena_offset = allocation.offset();
             let atom_size = allocation.atom_size().unwrap_or(DeviceAlignment::MIN);

diff --git a/vulkano/src/buffer/mod.rs b/vulkano/src/buffer/mod.rs
@@ -203,6 +203,7 @@ pub struct Buffer {
 
 /// The type of backing memory that a buffer can have.
 #[derive(Debug)]
+#[non_exhaustive]
 pub enum BufferMemory {
     /// The buffer is backed by normal memory, bound with [`bind_memory`].
     ///
@@ -213,6 +214,9 @@ pub enum BufferMemory {
     ///
     /// [`bind_sparse`]: crate::device::QueueGuard::bind_sparse
     Sparse,
+
+    /// The buffer is backed by memory not managed by vulkano.
+    External,
 }
 
 impl Buffer {
@@ -402,10 +406,13 @@ impl Buffer {
             .map_err(AllocateBufferError::AllocateMemory)?;
         let allocation = unsafe { ResourceMemory::from_allocation(allocator, allocation) };
 
-        let buffer = raw_buffer.bind_memory(allocation).map_err(|(err, _, _)| {
-            err.map(AllocateBufferError::BindMemory)
-                .map_validation(|err| err.add_context("RawBuffer::bind_memory"))
-        })?;
+        // 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, _, _)| {
+                err.map(AllocateBufferError::BindMemory)
+                    .map_validation(|err| err.add_context("RawBuffer::bind_memory"))
+            })?
+        };
 
         Ok(Arc::new(buffer))
     }

diff --git a/vulkano/src/buffer/subbuffer.rs b/vulkano/src/buffer/subbuffer.rs
@@ -85,7 +85,7 @@ impl<T: ?Sized> Subbuffer<T> {
     fn memory_offset(&self) -> DeviceSize {
         let allocation = match self.buffer().memory() {
             BufferMemory::Normal(a) => a,
-            BufferMemory::Sparse => unreachable!(),
+            BufferMemory::Sparse | BufferMemory::External => unreachable!(),
         };
 
         allocation.offset() + self.offset
@@ -124,7 +124,7 @@ impl<T: ?Sized> Subbuffer<T> {
                 // SAFETY: `self.range()` is in bounds of the allocation.
                 unsafe { allocation.mapped_slice_unchecked(self.range()) }
             }
-            BufferMemory::Sparse => unreachable!(),
+            BufferMemory::Sparse | BufferMemory::External => unreachable!(),
         }
     }
 
@@ -291,15 +291,20 @@ where
     /// non-coherent atom size, so in this case one would be at offset 0 and the other at offset
     /// 64. [`SubbufferAllocator`] does this automatically.
     ///
+    /// If the memory backing the buffer is not managed by vulkano, (i.e. this buffer was created
+    /// from [`RawBuffer::assume_bound`]), then it can't be read from using this function.
+    ///
     /// [host-coherent]: crate::memory::MemoryPropertyFlags::HOST_COHERENT
     /// [`invalidate_range`]: crate::memory::ResourceMemory::invalidate_range
     /// [`non_coherent_atom_size`]: crate::device::Properties::non_coherent_atom_size
     /// [`write`]: Self::write
     /// [`SubbufferAllocator`]: super::allocator::SubbufferAllocator
+    /// [`RawBuffer::assume_bound`]: crate::buffer::sys::RawBuffer::assume_bound
     pub fn read(&self) -> Result<BufferReadGuard<'_, T>, HostAccessError> {
         let allocation = match self.buffer().memory() {
             BufferMemory::Normal(a) => a,
             BufferMemory::Sparse => todo!("`Subbuffer::read` doesn't support sparse binding yet"),
+            BufferMemory::External => return Err(HostAccessError::Unmanaged),
         };
 
         let range = if let Some(atom_size) = allocation.atom_size() {
@@ -377,15 +382,20 @@ where
     /// in this case one would be at offset 0 and the other at offset 64. [`SubbufferAllocator`]
     /// does this automatically.
     ///
+    /// If the memory backing the buffer is not managed by vulkano, (i.e. this buffer was created
+    /// from [`RawBuffer::assume_bound`]), then it can't be written to using this function.
+    ///
     /// [host-coherent]: crate::memory::MemoryPropertyFlags::HOST_COHERENT
     /// [`flush_range`]: crate::memory::ResourceMemory::flush_range
     /// [`non_coherent_atom_size`]: crate::device::Properties::non_coherent_atom_size
     /// [`read`]: Self::read
     /// [`SubbufferAllocator`]: super::allocator::SubbufferAllocator
+    /// [`RawBuffer::assume_bound`]: crate::buffer::sys::RawBuffer::assume_bound
     pub fn write(&self) -> Result<BufferWriteGuard<'_, T>, HostAccessError> {
         let allocation = match self.buffer().memory() {
             BufferMemory::Normal(a) => a,
             BufferMemory::Sparse => todo!("`Subbuffer::write` doesn't support sparse binding yet"),
+            BufferMemory::External => return Err(HostAccessError::Unmanaged),
         };
 
         let range = if let Some(atom_size) = allocation.atom_size() {
@@ -664,6 +674,7 @@ impl<T: ?Sized> Drop for BufferWriteGuard<'_, T> {
         let allocation = match self.subbuffer.buffer().memory() {
             BufferMemory::Normal(a) => a,
             BufferMemory::Sparse => unreachable!(),
+            BufferMemory::External => unreachable!(),
         };
 
         if allocation.atom_size().is_some() && !thread::panicking() {

diff --git a/vulkano/src/buffer/sys.rs b/vulkano/src/buffer/sys.rs
@@ -40,6 +40,7 @@ pub struct RawBuffer {
     external_memory_handle_types: ExternalMemoryHandleTypes,
 
     memory_requirements: MemoryRequirements,
+    needs_destruction: bool,
 }
 
 impl RawBuffer {
@@ -140,13 +141,39 @@ impl RawBuffer {
     /// # Safety
     ///
     /// - `handle` must be a valid Vulkan object handle created from `device`.
-    /// - `handle` must refer to a buffer that has not yet had memory bound to it.
     /// - `create_info` must match the info used to create the object.
     #[inline]
     pub unsafe fn from_handle(
         device: Arc<Device>,
         handle: ash::vk::Buffer,
         create_info: BufferCreateInfo,
+    ) -> Self {
+        Self::from_handle_with_destruction(device, handle, create_info, true)
+    }
+
+    /// Creates a new `RawBuffer` from a raw object handle. Unlike `from_handle`, the created
+    /// `RawBuffer` does not destroy the inner buffer when dropped.
+    ///
+    /// # Safety
+    ///
+    /// - `handle` must be a valid Vulkan object handle created from `device`.
+    /// - `create_info` must match the info used to create the object.
+    /// - Caller must ensure that the handle will not be destroyed for the lifetime of returned
+    ///   `RawBuffer`.
+    #[inline]
+    pub unsafe fn from_handle_borrowed(
+        device: Arc<Device>,
+        handle: ash::vk::Buffer,
+        create_info: BufferCreateInfo,
+    ) -> Self {
+        Self::from_handle_with_destruction(device, handle, create_info, false)
+    }
+
+    unsafe fn from_handle_with_destruction(
+        device: Arc<Device>,
+        handle: ash::vk::Buffer,
+        create_info: BufferCreateInfo,
+        needs_destruction: bool,
     ) -> Self {
         let BufferCreateInfo {
             flags,
@@ -195,6 +222,7 @@ impl RawBuffer {
             sharing,
             external_memory_handle_types,
             memory_requirements,
+            needs_destruction,
         }
     }
 
@@ -266,7 +294,11 @@ impl RawBuffer {
     }
 
     /// Binds device memory to this buffer.
-    pub fn bind_memory(
+    ///
+    /// # Safety
+    ///
+    /// - The buffer must not already have memory bound to it.
+    pub unsafe fn bind_memory(
         self,
         allocation: ResourceMemory,
     ) -> Result<Buffer, (Validated<VulkanError>, RawBuffer, ResourceMemory)> {
@@ -278,6 +310,15 @@ 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,
@@ -577,9 +618,11 @@ impl RawBuffer {
 impl Drop for RawBuffer {
     #[inline]
     fn drop(&mut self) {
-        unsafe {
-            let fns = self.device.fns();
-            (fns.v1_0.destroy_buffer)(self.device.handle(), self.handle, ptr::null());
+        if self.needs_destruction {
+            unsafe {
+                let fns = self.device.fns();
+                (fns.v1_0.destroy_buffer)(self.device.handle(), self.handle, ptr::null());
+            }
         }
     }
 }

diff --git a/vulkano/src/image/mod.rs b/vulkano/src/image/mod.rs
@@ -115,6 +115,7 @@ pub struct Image {
 
 /// The type of backing memory that an image can have.
 #[derive(Debug)]
+#[non_exhaustive]
 pub enum ImageMemory {
     /// The image is backed by normal memory, bound with [`bind_memory`].
     ///
@@ -131,6 +132,9 @@ pub enum ImageMemory {
         swapchain: Arc<Swapchain>,
         image_index: u32,
     },
+
+    /// The image is backed by external memory not managed by vulkano.
+    External,
 }
 
 impl Image {
@@ -161,10 +165,13 @@ impl Image {
             .map_err(AllocateImageError::AllocateMemory)?;
         let allocation = unsafe { ResourceMemory::from_allocation(allocator, allocation) };
 
-        let image = raw_image.bind_memory([allocation]).map_err(|(err, _, _)| {
-            err.map(AllocateImageError::BindMemory)
-                .map_validation(|err| err.add_context("RawImage::bind_memory"))
-        })?;
+        // 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, _, _)| {
+                err.map(AllocateImageError::BindMemory)
+                    .map_validation(|err| err.add_context("RawImage::bind_memory"))
+            })?
+        };
 
         Ok(Arc::new(image))
     }
@@ -505,7 +512,7 @@ impl Image {
 
     pub(crate) unsafe fn layout_initialized(&self) {
         match &self.memory {
-            ImageMemory::Normal(..) | ImageMemory::Sparse(..) => {
+            ImageMemory::Normal(..) | ImageMemory::Sparse(..) | ImageMemory::External => {
                 self.is_layout_initialized.store(true, Ordering::Release);
             }
             ImageMemory::Swapchain {
@@ -519,7 +526,7 @@ impl Image {
 
     pub(crate) fn is_layout_initialized(&self) -> bool {
         match &self.memory {
-            ImageMemory::Normal(..) | ImageMemory::Sparse(..) => {
+            ImageMemory::Normal(..) | ImageMemory::Sparse(..) | ImageMemory::External => {
                 self.is_layout_initialized.load(Ordering::Acquire)
             }
             ImageMemory::Swapchain {

diff --git a/vulkano/src/image/sys.rs b/vulkano/src/image/sys.rs
@@ -265,7 +265,6 @@ impl RawImage {
     /// # Safety
     ///
     /// - `handle` must be a valid Vulkan object handle created from `device`.
-    /// - `handle` must refer to an image that has not yet had memory bound to it.
     /// - `create_info` must match the info used to create the object.
     #[inline]
     pub unsafe fn from_handle(
@@ -276,6 +275,24 @@ impl RawImage {
         Self::from_handle_with_destruction(device, handle, create_info, true)
     }
 
+    /// Creates a new `RawImage` from a raw object handle. Unlike `from_handle`, the created
+    /// `RawImage` will not destroy the inner image when dropped.
+    ///
+    /// # Safety
+    ///
+    /// - `handle` must be a valid Vulkan object handle created from `device`.
+    /// - `create_info` must match the info used to create the object.
+    /// - Caller must ensure the handle will not be destroyed for the lifetime of returned
+    ///   `RawImage`.
+    #[inline]
+    pub unsafe fn from_handle_borrowed(
+        device: Arc<Device>,
+        handle: ash::vk::Image,
+        create_info: ImageCreateInfo,
+    ) -> Result<Self, VulkanError> {
+        Self::from_handle_with_destruction(device, handle, create_info, false)
+    }
+
     pub(super) unsafe fn from_handle_with_destruction(
         device: Arc<Device>,
         handle: ash::vk::Image,
@@ -696,7 +713,11 @@ 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.
-    pub fn bind_memory(
+    ///
+    /// # Safety
+    ///
+    /// - The image must not already have memory bound to it.
+    pub unsafe fn bind_memory(
         self,
         allocations: impl IntoIterator<Item = ResourceMemory>,
     ) -> Result<
@@ -1050,6 +1071,39 @@ 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`,

diff --git a/vulkano/src/sync/mod.rs b/vulkano/src/sync/mod.rs
@@ -95,6 +95,7 @@ pub(crate) enum CurrentAccess {
 pub enum HostAccessError {
     AccessConflict(AccessConflict),
     Invalidate(VulkanError),
+    Unmanaged,
     NotHostMapped,
     OutOfMappedRange,
 }
@@ -115,6 +116,7 @@ impl Display for HostAccessError {
             Self::AccessConflict(_) => {
                 write!(f, "the resource is already in use in a conflicting way")
             }
+            Self::Unmanaged => write!(f, "the resource is not managed by vulkano"),
             HostAccessError::Invalidate(_) => write!(f, "invalidating the device memory failed"),
             HostAccessError::NotHostMapped => {
                 write!(f, "the device memory is not current host-mapped")