From 25a371a86aeb21bf3833d69f3a822dea1080e1ff Mon Sep 17 00:00:00 2001 From: Rua Date: Mon, 3 Jul 2023 21:17:36 +0200 Subject: [PATCH] ValidationError-ify `Buffer` and `BufferView` (#2244) * ValidationError-ify `Buffer` and `BufferView` * Simpler pub use --- vulkano/src/buffer/allocator.rs | 4 +- vulkano/src/buffer/mod.rs | 428 ++++++++--------------------- vulkano/src/buffer/subbuffer.rs | 109 +++----- vulkano/src/buffer/sys.rs | 468 +++++++++++++++++++------------- vulkano/src/buffer/view.rs | 399 ++++++++++++++------------- vulkano/src/image/immutable.rs | 6 +- vulkano/src/image/sys.rs | 19 +- vulkano/src/swapchain/mod.rs | 1 + vulkano/src/sync/mod.rs | 79 +++++- 9 files changed, 747 insertions(+), 766 deletions(-) diff --git a/vulkano/src/buffer/allocator.rs b/vulkano/src/buffer/allocator.rs index 7ff10d43..fc060ebd 100644 --- a/vulkano/src/buffer/allocator.rs +++ b/vulkano/src/buffer/allocator.rs @@ -10,7 +10,7 @@ //! Efficiently suballocates buffers into smaller subbuffers. use super::{ - sys::BufferCreateInfo, Buffer, BufferContents, BufferError, BufferMemory, BufferUsage, + sys::BufferCreateInfo, Buffer, BufferAllocateError, BufferContents, BufferMemory, BufferUsage, Subbuffer, }; use crate::{ @@ -355,7 +355,7 @@ where DeviceLayout::from_size_alignment(self.arena_size, 1).unwrap(), ) .map_err(|err| match err { - BufferError::AllocError(err) => err, + BufferAllocateError::AllocateMemory(err) => err, // We don't use sparse-binding, concurrent sharing or external memory, therefore the // other errors can't happen. _ => unreachable!(), diff --git a/vulkano/src/buffer/mod.rs b/vulkano/src/buffer/mod.rs index ce9ec5f0..58beb06d 100644 --- a/vulkano/src/buffer/mod.rs +++ b/vulkano/src/buffer/mod.rs @@ -101,15 +101,8 @@ //! [the `view` module]: self::view //! [the `shader` module documentation]: crate::shader -pub use self::{ - subbuffer::{BufferContents, BufferContentsLayout, Subbuffer}, - sys::BufferCreateInfo, - usage::BufferUsage, -}; -use self::{ - subbuffer::{ReadLockError, WriteLockError}, - sys::RawBuffer, -}; +use self::sys::RawBuffer; +pub use self::{subbuffer::*, sys::*, usage::*}; use crate::{ device::{physical::PhysicalDevice, Device, DeviceOwned}, macros::{vulkan_bitflags, vulkan_enum}, @@ -118,21 +111,20 @@ use crate::{ AllocationCreateInfo, AllocationType, DeviceLayout, MemoryAlloc, MemoryAllocator, MemoryAllocatorError, }, - is_aligned, DedicatedAllocation, DeviceAlignment, ExternalMemoryHandleType, - ExternalMemoryHandleTypes, ExternalMemoryProperties, MemoryRequirements, + is_aligned, DedicatedAllocation, ExternalMemoryHandleType, ExternalMemoryHandleTypes, + ExternalMemoryProperties, MemoryRequirements, }, range_map::RangeMap, - sync::{future::AccessError, CurrentAccess, Sharing}, - DeviceSize, NonZeroDeviceSize, RequirementNotMet, Requires, RequiresAllOf, RequiresOneOf, - RuntimeError, ValidationError, Version, VulkanObject, + sync::{future::AccessError, AccessConflict, CurrentAccess, Sharing}, + DeviceSize, NonZeroDeviceSize, Requires, RequiresAllOf, RequiresOneOf, RuntimeError, + ValidationError, Version, VulkanError, VulkanObject, }; use parking_lot::{Mutex, MutexGuard}; use smallvec::SmallVec; use std::{ error::Error, - fmt::{Display, Error as FmtError, Formatter}, + fmt::{Display, Formatter}, hash::{Hash, Hasher}, - mem::size_of_val, ops::Range, sync::Arc, }; @@ -273,13 +265,18 @@ impl Buffer { buffer_info: BufferCreateInfo, allocation_info: AllocationCreateInfo, data: T, - ) -> Result, BufferError> + ) -> Result, BufferAllocateError> where T: BufferContents, { let buffer = Buffer::new_sized(allocator, buffer_info, allocation_info)?; - *buffer.write()? = data; + { + let mut write_guard = buffer + .write() + .expect("the buffer is somehow in use before we returned it to the user"); + *write_guard = data; + } Ok(buffer) } @@ -301,7 +298,7 @@ impl Buffer { buffer_info: BufferCreateInfo, allocation_info: AllocationCreateInfo, iter: I, - ) -> Result, BufferError> + ) -> Result, BufferAllocateError> where T: BufferContents, I: IntoIterator, @@ -315,8 +312,14 @@ impl Buffer { iter.len().try_into().unwrap(), )?; - for (o, i) in buffer.write()?.iter_mut().zip(iter) { - *o = i; + { + let mut write_guard = buffer + .write() + .expect("the buffer is somehow in use before we returned it to the user"); + + for (o, i) in write_guard.iter_mut().zip(iter) { + *o = i; + } } Ok(buffer) @@ -332,7 +335,7 @@ impl Buffer { allocator: &(impl MemoryAllocator + ?Sized), buffer_info: BufferCreateInfo, allocation_info: AllocationCreateInfo, - ) -> Result, BufferError> + ) -> Result, BufferAllocateError> where T: BufferContents, { @@ -359,7 +362,7 @@ impl Buffer { buffer_info: BufferCreateInfo, allocation_info: AllocationCreateInfo, len: DeviceSize, - ) -> Result, BufferError> + ) -> Result, BufferAllocateError> where T: BufferContents, { @@ -378,7 +381,7 @@ impl Buffer { buffer_info: BufferCreateInfo, allocation_info: AllocationCreateInfo, len: DeviceSize, - ) -> Result, BufferError> + ) -> Result, BufferAllocateError> where T: BufferContents + ?Sized, { @@ -405,7 +408,7 @@ impl Buffer { mut buffer_info: BufferCreateInfo, allocation_info: AllocationCreateInfo, layout: DeviceLayout, - ) -> Result, BufferError> { + ) -> Result, BufferAllocateError> { assert!(layout.alignment().as_devicesize() <= 64); // TODO: Enable once sparse binding materializes // assert!(!allocate_info.flags.contains(BufferCreateFlags::SPARSE_BINDING)); @@ -418,18 +421,21 @@ impl Buffer { buffer_info.size = layout.size(); - let raw_buffer = RawBuffer::new(allocator.device().clone(), buffer_info)?; + let raw_buffer = RawBuffer::new(allocator.device().clone(), buffer_info) + .map_err(BufferAllocateError::CreateBuffer)?; let mut requirements = *raw_buffer.memory_requirements(); requirements.layout = requirements.layout.align_to(layout.alignment()).unwrap(); let mut allocation = unsafe { - allocator.allocate_unchecked( - requirements, - AllocationType::Linear, - allocation_info, - Some(DedicatedAllocation::Buffer(&raw_buffer)), - ) - }?; + allocator + .allocate_unchecked( + requirements, + AllocationType::Linear, + allocation_info, + Some(DedicatedAllocation::Buffer(&raw_buffer)), + ) + .map_err(BufferAllocateError::AllocateMemory)? + }; debug_assert!(is_aligned( allocation.offset(), requirements.layout.alignment(), @@ -442,7 +448,7 @@ impl Buffer { unsafe { raw_buffer.bind_memory_unchecked(allocation) } .map(Arc::new) - .map_err(|(err, _, _)| err.into()) + .map_err(|(err, _, _)| BufferAllocateError::BindMemory(err)) } fn from_raw(inner: RawBuffer, memory: BufferMemory) -> Self { @@ -499,39 +505,59 @@ impl Buffer { /// Returns the device address for this buffer. // TODO: Caching? - pub fn device_address(&self) -> Result { + pub fn device_address(&self) -> Result { + self.validate_device_address()?; + + unsafe { Ok(self.device_address_unchecked()) } + } + + fn validate_device_address(&self) -> Result<(), ValidationError> { let device = self.device(); - // VUID-vkGetBufferDeviceAddress-bufferDeviceAddress-03324 if !device.enabled_features().buffer_device_address { - return Err(BufferError::RequirementNotMet { - required_for: "`Buffer::device_address`", + return Err(ValidationError { requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( "buffer_device_address", )])]), + vuids: &["VUID-vkGetBufferDeviceAddress-bufferDeviceAddress-03324"], + ..Default::default() }); } - // VUID-VkBufferDeviceAddressInfo-buffer-02601 if !self.usage().intersects(BufferUsage::SHADER_DEVICE_ADDRESS) { - return Err(BufferError::BufferMissingUsage); + return Err(ValidationError { + context: "self.usage()".into(), + problem: "does not contain `BufferUsage::SHADER_DEVICE_ADDRESS`".into(), + vuids: &["VUID-VkBufferDeviceAddressInfo-buffer-02601"], + ..Default::default() + }); } - let info = ash::vk::BufferDeviceAddressInfo { + Ok(()) + } + + #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] + pub unsafe fn device_address_unchecked(&self) -> NonZeroDeviceSize { + let device = self.device(); + + let info_vk = ash::vk::BufferDeviceAddressInfo { buffer: self.handle(), ..Default::default() }; - let fns = device.fns(); - let f = if device.api_version() >= Version::V1_2 { - fns.v1_2.get_buffer_device_address - } else if device.enabled_extensions().khr_buffer_device_address { - fns.khr_buffer_device_address.get_buffer_device_address_khr - } else { - fns.ext_buffer_device_address.get_buffer_device_address_ext - }; - let ptr = unsafe { f(device.handle(), &info) }; - Ok(NonZeroDeviceSize::new(ptr).unwrap()) + let ptr = { + let fns = device.fns(); + let f = if device.api_version() >= Version::V1_2 { + fns.v1_2.get_buffer_device_address + } else if device.enabled_extensions().khr_buffer_device_address { + fns.khr_buffer_device_address.get_buffer_device_address_khr + } else { + fns.ext_buffer_device_address.get_buffer_device_address_ext + }; + f(device.handle(), &info_vk) + }; + + NonZeroDeviceSize::new(ptr).unwrap() } pub(crate) fn state(&self) -> MutexGuard<'_, BufferState> { @@ -570,6 +596,34 @@ impl Hash for Buffer { } } +/// Error that can happen when allocating a new buffer. +#[derive(Clone, Debug)] +pub enum BufferAllocateError { + CreateBuffer(VulkanError), + AllocateMemory(MemoryAllocatorError), + BindMemory(RuntimeError), +} + +impl Error for BufferAllocateError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::CreateBuffer(err) => Some(err), + Self::AllocateMemory(err) => Some(err), + Self::BindMemory(err) => Some(err), + } + } +} + +impl Display for BufferAllocateError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::CreateBuffer(_) => write!(f, "creating the buffer failed"), + Self::AllocateMemory(_) => write!(f, "allocating memory for the buffer failed"), + Self::BindMemory(_) => write!(f, "binding memory to the buffer failed"), + } + } +} + /// The current state of a buffer. #[derive(Debug)] pub(crate) struct BufferState { @@ -593,11 +647,11 @@ impl BufferState { } } - pub(crate) fn check_cpu_read(&self, range: Range) -> Result<(), ReadLockError> { + pub(crate) fn check_cpu_read(&self, range: Range) -> Result<(), AccessConflict> { for (_range, state) in self.ranges.range(&range) { match &state.current_access { - CurrentAccess::CpuExclusive { .. } => return Err(ReadLockError::CpuWriteLocked), - CurrentAccess::GpuExclusive { .. } => return Err(ReadLockError::GpuWriteLocked), + CurrentAccess::CpuExclusive { .. } => return Err(AccessConflict::HostWrite), + CurrentAccess::GpuExclusive { .. } => return Err(AccessConflict::DeviceWrite), CurrentAccess::Shared { .. } => (), } } @@ -631,19 +685,19 @@ impl BufferState { } } - pub(crate) fn check_cpu_write(&self, range: Range) -> Result<(), WriteLockError> { + pub(crate) fn check_cpu_write(&self, range: Range) -> Result<(), AccessConflict> { for (_range, state) in self.ranges.range(&range) { match &state.current_access { - CurrentAccess::CpuExclusive => return Err(WriteLockError::CpuLocked), - CurrentAccess::GpuExclusive { .. } => return Err(WriteLockError::GpuLocked), + CurrentAccess::CpuExclusive => return Err(AccessConflict::HostWrite), + CurrentAccess::GpuExclusive { .. } => return Err(AccessConflict::DeviceWrite), CurrentAccess::Shared { cpu_reads: 0, gpu_reads: 0, } => (), CurrentAccess::Shared { cpu_reads, .. } if *cpu_reads > 0 => { - return Err(WriteLockError::CpuLocked) + return Err(AccessConflict::HostRead) } - CurrentAccess::Shared { .. } => return Err(WriteLockError::GpuLocked), + CurrentAccess::Shared { .. } => return Err(AccessConflict::DeviceRead), } } @@ -776,264 +830,10 @@ struct BufferRangeState { current_access: CurrentAccess, } -/// Error that can happen in buffer functions. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum BufferError { - RuntimeError(RuntimeError), - - /// Allocating memory failed. - AllocError(MemoryAllocatorError), - - RequirementNotMet { - required_for: &'static str, - requires_one_of: RequiresOneOf, - }, - - /// The buffer is missing the `SHADER_DEVICE_ADDRESS` usage. - BufferMissingUsage, - - /// The memory was created dedicated to a resource, but not to this buffer. - DedicatedAllocationMismatch, - - /// A dedicated allocation is required for this buffer, but one was not provided. - DedicatedAllocationRequired, - - /// The host is already using this buffer in a way that is incompatible with the - /// requested access. - InUseByHost, - - /// The device is already using this buffer in a way that is incompatible with the - /// requested access. - InUseByDevice, - - /// The specified size exceeded the value of the `max_buffer_size` limit. - MaxBufferSizeExceeded { - size: DeviceSize, - max: DeviceSize, - }, - - /// The offset of the allocation does not have the required alignment. - MemoryAllocationNotAligned { - allocation_offset: DeviceSize, - required_alignment: DeviceAlignment, - }, - - /// The size of the allocation is smaller than what is required. - MemoryAllocationTooSmall { - allocation_size: DeviceSize, - required_size: DeviceSize, - }, - - /// The buffer was created with the `SHADER_DEVICE_ADDRESS` usage, but the memory does not - /// support this usage. - MemoryBufferDeviceAddressNotSupported, - - /// The memory was created with export handle types, but none of these handle types were - /// enabled on the buffer. - MemoryExternalHandleTypesDisjoint { - buffer_handle_types: ExternalMemoryHandleTypes, - memory_export_handle_types: ExternalMemoryHandleTypes, - }, - - /// The memory was created with an import, but the import's handle type was not enabled on - /// the buffer. - MemoryImportedHandleTypeNotEnabled { - buffer_handle_types: ExternalMemoryHandleTypes, - memory_imported_handle_type: ExternalMemoryHandleType, - }, - - /// The memory backing this buffer is not visible to the host. - MemoryNotHostVisible, - - /// The protection of buffer and memory are not equal. - MemoryProtectedMismatch { - buffer_protected: bool, - memory_protected: bool, - }, - - /// The provided memory type is not one of the allowed memory types that can be bound to this - /// buffer. - MemoryTypeNotAllowed { - provided_memory_type_index: u32, - allowed_memory_type_bits: u32, - }, - - /// The sharing mode was set to `Concurrent`, but one of the specified queue family indices was - /// out of range. - SharingQueueFamilyIndexOutOfRange { - queue_family_index: u32, - queue_family_count: u32, - }, -} - -impl Error for BufferError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self { - Self::RuntimeError(err) => Some(err), - Self::AllocError(err) => Some(err), - _ => None, - } - } -} - -impl Display for BufferError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { - match self { - Self::RuntimeError(_) => write!(f, "a runtime error occurred"), - Self::AllocError(_) => write!(f, "allocating memory failed"), - Self::RequirementNotMet { - required_for, - requires_one_of, - } => write!( - f, - "a requirement was not met for: {}; requires one of: {}", - required_for, requires_one_of, - ), - Self::BufferMissingUsage => { - write!(f, "the buffer is missing the `SHADER_DEVICE_ADDRESS` usage") - } - Self::DedicatedAllocationMismatch => write!( - f, - "the memory was created dedicated to a resource, but not to this buffer", - ), - Self::DedicatedAllocationRequired => write!( - f, - "a dedicated allocation is required for this buffer, but one was not provided" - ), - Self::InUseByHost => write!( - f, - "the host is already using this buffer in a way that is incompatible with the \ - requested access", - ), - Self::InUseByDevice => write!( - f, - "the device is already using this buffer in a way that is incompatible with the \ - requested access" - ), - Self::MaxBufferSizeExceeded { .. } => write!( - f, - "the specified size exceeded the value of the `max_buffer_size` limit", - ), - Self::MemoryAllocationNotAligned { - allocation_offset, - required_alignment, - } => write!( - f, - "the offset of the allocation ({}) does not have the required alignment ({:?})", - allocation_offset, required_alignment, - ), - Self::MemoryAllocationTooSmall { - allocation_size, - required_size, - } => write!( - f, - "the size of the allocation ({}) is smaller than what is required ({})", - allocation_size, required_size, - ), - Self::MemoryBufferDeviceAddressNotSupported => write!( - f, - "the buffer was created with the `SHADER_DEVICE_ADDRESS` usage, but the memory \ - does not support this usage", - ), - Self::MemoryExternalHandleTypesDisjoint { .. } => write!( - f, - "the memory was created with export handle types, but none of these handle types \ - were enabled on the buffer", - ), - Self::MemoryImportedHandleTypeNotEnabled { .. } => write!( - f, - "the memory was created with an import, but the import's handle type was not \ - enabled on the buffer", - ), - Self::MemoryNotHostVisible => write!( - f, - "the memory backing this buffer is not visible to the host", - ), - Self::MemoryProtectedMismatch { - buffer_protected, - memory_protected, - } => write!( - f, - "the protection of buffer ({}) and memory ({}) are not equal", - buffer_protected, memory_protected, - ), - Self::MemoryTypeNotAllowed { - provided_memory_type_index, - allowed_memory_type_bits, - } => write!( - f, - "the provided memory type ({}) is not one of the allowed memory types (", - provided_memory_type_index, - ) - .and_then(|_| { - let mut first = true; - - for i in (0..size_of_val(allowed_memory_type_bits)) - .filter(|i| allowed_memory_type_bits & (1 << i) != 0) - { - if first { - write!(f, "{}", i)?; - first = false; - } else { - write!(f, ", {}", i)?; - } - } - - Ok(()) - }) - .and_then(|_| write!(f, ") that can be bound to this buffer")), - Self::SharingQueueFamilyIndexOutOfRange { .. } => write!( - f, - "the sharing mode was set to `Concurrent`, but one of the specified queue family \ - indices was out of range", - ), - } - } -} - -impl From for BufferError { - fn from(err: RuntimeError) -> Self { - Self::RuntimeError(err) - } -} - -impl From for BufferError { - fn from(err: MemoryAllocatorError) -> Self { - Self::AllocError(err) - } -} - -impl From for BufferError { - fn from(err: RequirementNotMet) -> Self { - Self::RequirementNotMet { - required_for: err.required_for, - requires_one_of: err.requires_one_of, - } - } -} - -impl From for BufferError { - fn from(err: ReadLockError) -> Self { - match err { - ReadLockError::CpuWriteLocked => Self::InUseByHost, - ReadLockError::GpuWriteLocked => Self::InUseByDevice, - } - } -} - -impl From for BufferError { - fn from(err: WriteLockError) -> Self { - match err { - WriteLockError::CpuLocked => Self::InUseByHost, - WriteLockError::GpuLocked => Self::InUseByDevice, - } - } -} - vulkan_bitflags! { #[non_exhaustive] - /// Flags to be set when creating a buffer. + /// Flags specifying additional properties of a buffer. BufferCreateFlags = BufferCreateFlags(u32); /* TODO: enable diff --git a/vulkano/src/buffer/subbuffer.rs b/vulkano/src/buffer/subbuffer.rs index 7e95016f..dc4bc081 100644 --- a/vulkano/src/buffer/subbuffer.rs +++ b/vulkano/src/buffer/subbuffer.rs @@ -9,7 +9,7 @@ //! A subpart of a buffer. -use super::{allocator::Arena, Buffer, BufferError, BufferMemory}; +use super::{allocator::Arena, Buffer, BufferMemory}; use crate::{ device::{Device, DeviceOwned}, macros::try_opt, @@ -18,15 +18,14 @@ use crate::{ allocator::{align_down, align_up, DeviceLayout}, is_aligned, DeviceAlignment, }, - DeviceSize, NonZeroDeviceSize, + sync::HostAccessError, + DeviceSize, NonZeroDeviceSize, ValidationError, }; use bytemuck::AnyBitPattern; use std::{ alloc::Layout, cmp, - error::Error, ffi::c_void, - fmt::{Display, Error as FmtError, Formatter}, hash::{Hash, Hasher}, marker::PhantomData, mem::{self, align_of, size_of}, @@ -135,7 +134,7 @@ impl Subbuffer { } /// Returns the device address for this subbuffer. - pub fn device_address(&self) -> Result { + pub fn device_address(&self) -> Result { self.buffer().device_address().map(|ptr| { // SAFETY: The original address came from the Vulkan implementation, and allocation // sizes are guaranteed to not exceed `DeviceLayout::MAX_SIZE`, so the offset better be @@ -144,6 +143,16 @@ impl Subbuffer { }) } + #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] + pub unsafe fn device_address_unchecked(&self) -> NonZeroDeviceSize { + // SAFETY: The original address came from the Vulkan implementation, and allocation + // sizes are guaranteed to not exceed `DeviceLayout::MAX_SIZE`, so the offset better be + // in range. + NonZeroDeviceSize::new_unchecked( + self.buffer().device_address_unchecked().get() + self.offset, + ) + } + /// Casts the subbuffer to a slice of raw bytes. pub fn into_bytes(self) -> Subbuffer<[u8]> { unsafe { self.reinterpret_unchecked_inner() } @@ -292,7 +301,7 @@ where /// [`non_coherent_atom_size`]: crate::device::Properties::non_coherent_atom_size /// [`write`]: Self::write /// [`SubbufferAllocator`]: super::allocator::SubbufferAllocator - pub fn read(&self) -> Result, BufferError> { + pub fn read(&self) -> Result, HostAccessError> { let allocation = match self.buffer().memory() { BufferMemory::Normal(a) => a, BufferMemory::Sparse => todo!("`Subbuffer::read` doesn't support sparse binding yet"), @@ -313,7 +322,9 @@ where }; let mut state = self.buffer().state(); - state.check_cpu_read(range.clone())?; + state + .check_cpu_read(range.clone()) + .map_err(HostAccessError::AccessConflict)?; unsafe { state.cpu_read_lock(range.clone()) }; if allocation.atom_size().is_some() { @@ -322,10 +333,19 @@ where // lock, so there will be no new data and this call will do nothing. // TODO: probably still more efficient to call it only if we're the first to acquire a // read lock, but the number of CPU locks isn't currently tracked anywhere. - unsafe { allocation.invalidate_range(range.clone()) }?; + unsafe { + allocation + .invalidate_range(range.clone()) + .map_err(HostAccessError::Invalidate)? + }; } - let mapped_ptr = self.mapped_ptr().ok_or(BufferError::MemoryNotHostVisible)?; + let mapped_ptr = + self.mapped_ptr() + .ok_or(HostAccessError::ValidationError(ValidationError { + problem: "the memory of this subbuffer is not host-visible".into(), + ..Default::default() + }))?; // SAFETY: `Subbuffer` guarantees that its contents are laid out correctly for `T`. let data = unsafe { &*T::from_ffi(mapped_ptr.as_ptr(), self.size as usize) }; @@ -361,7 +381,7 @@ where /// [`non_coherent_atom_size`]: crate::device::Properties::non_coherent_atom_size /// [`read`]: Self::read /// [`SubbufferAllocator`]: super::allocator::SubbufferAllocator - pub fn write(&self) -> Result, BufferError> { + pub fn write(&self) -> Result, HostAccessError> { let allocation = match self.buffer().memory() { BufferMemory::Normal(a) => a, BufferMemory::Sparse => todo!("`Subbuffer::write` doesn't support sparse binding yet"), @@ -382,14 +402,25 @@ where }; let mut state = self.buffer().state(); - state.check_cpu_write(range.clone())?; + state + .check_cpu_write(range.clone()) + .map_err(HostAccessError::AccessConflict)?; unsafe { state.cpu_write_lock(range.clone()) }; if allocation.atom_size().is_some() { - unsafe { allocation.invalidate_range(range.clone()) }?; + unsafe { + allocation + .invalidate_range(range.clone()) + .map_err(HostAccessError::Invalidate)? + }; } - let mapped_ptr = self.mapped_ptr().ok_or(BufferError::MemoryNotHostVisible)?; + let mapped_ptr = + self.mapped_ptr() + .ok_or(HostAccessError::ValidationError(ValidationError { + problem: "the memory of this subbuffer is not host-visible".into(), + ..Default::default() + }))?; // SAFETY: `Subbuffer` guarantees that its contents are laid out correctly for `T`. let data = unsafe { &mut *T::from_ffi(mapped_ptr.as_ptr(), self.size as usize) }; @@ -652,58 +683,6 @@ impl DerefMut for BufferWriteGuard<'_, T> { } } -/// Error when attempting to CPU-read a buffer. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum ReadLockError { - /// The buffer is already locked for write mode by the CPU. - CpuWriteLocked, - /// The buffer is already locked for write mode by the GPU. - GpuWriteLocked, -} - -impl Error for ReadLockError {} - -impl Display for ReadLockError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { - write!( - f, - "{}", - match self { - ReadLockError::CpuWriteLocked => { - "the buffer is already locked for write mode by the CPU" - } - ReadLockError::GpuWriteLocked => { - "the buffer is already locked for write mode by the GPU" - } - } - ) - } -} - -/// Error when attempting to CPU-write a buffer. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum WriteLockError { - /// The buffer is already locked by the CPU. - CpuLocked, - /// The buffer is already locked by the GPU. - GpuLocked, -} - -impl Error for WriteLockError {} - -impl Display for WriteLockError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { - write!( - f, - "{}", - match self { - WriteLockError::CpuLocked => "the buffer is already locked by the CPU", - WriteLockError::GpuLocked => "the buffer is already locked by the GPU", - } - ) - } -} - /// Trait for types of data that can be put in a buffer. /// /// This trait is not intended to be implemented manually (ever) and attempting so will make you diff --git a/vulkano/src/buffer/sys.rs b/vulkano/src/buffer/sys.rs index f2f63498..3fbddf17 100644 --- a/vulkano/src/buffer/sys.rs +++ b/vulkano/src/buffer/sys.rs @@ -13,7 +13,7 @@ //! other buffer types of this library, and all custom buffer types //! that you create must wrap around the types in this module. -use super::{Buffer, BufferCreateFlags, BufferError, BufferMemory, BufferUsage}; +use super::{Buffer, BufferCreateFlags, BufferMemory, BufferUsage}; use crate::{ device::{Device, DeviceOwned}, macros::impl_id_counter, @@ -23,7 +23,8 @@ use crate::{ MemoryPropertyFlags, MemoryRequirements, }, sync::Sharing, - DeviceSize, Requires, RequiresAllOf, RequiresOneOf, RuntimeError, Version, VulkanObject, + DeviceSize, Requires, RequiresAllOf, RequiresOneOf, RuntimeError, ValidationError, Version, + VulkanError, VulkanObject, }; use smallvec::SmallVec; use std::{mem::MaybeUninit, num::NonZeroU64, ptr, sync::Arc}; @@ -58,130 +59,19 @@ impl RawBuffer { /// - Panics if `create_info.size` is zero. /// - Panics if `create_info.usage` is empty. #[inline] - pub fn new( - device: Arc, - mut create_info: BufferCreateInfo, - ) -> Result { - match &mut create_info.sharing { - Sharing::Exclusive => (), - Sharing::Concurrent(queue_family_indices) => { - // VUID-VkBufferCreateInfo-sharingMode-01419 - queue_family_indices.sort_unstable(); - queue_family_indices.dedup(); - } - } - + pub fn new(device: Arc, create_info: BufferCreateInfo) -> Result { Self::validate_new(&device, &create_info)?; unsafe { Ok(Self::new_unchecked(device, create_info)?) } } - fn validate_new(device: &Device, create_info: &BufferCreateInfo) -> Result<(), BufferError> { - let &BufferCreateInfo { - flags, - ref sharing, - size, - usage, - external_memory_handle_types, - _ne: _, - } = create_info; - - // VUID-VkBufferCreateInfo-flags-parameter - flags.validate_device(device)?; - - // VUID-VkBufferCreateInfo-usage-parameter - usage.validate_device(device)?; - - // VUID-VkBufferCreateInfo-usage-requiredbitmask - assert!(!usage.is_empty()); - - // VUID-VkBufferCreateInfo-size-00912 - assert!(size != 0); - - /* Enable when sparse binding is properly handled - if let Some(sparse_level) = sparse { - // VUID-VkBufferCreateInfo-flags-00915 - if !device.enabled_features().sparse_binding { - return Err(BufferError::RequirementNotMet { - required_for: "`create_info.sparse` is `Some`", - requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature("sparse_binding")])]), - }); - } - - // VUID-VkBufferCreateInfo-flags-00916 - if sparse_level.sparse_residency && !device.enabled_features().sparse_residency_buffer { - return Err(BufferError::RequirementNotMet { - required_for: "`create_info.sparse` is `Some(sparse_level)`, where \ - `sparse_level` contains `BufferCreateFlags::SPARSE_RESIDENCY`", - requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature("sparse_residency_buffer")])]), - }); - } - - // VUID-VkBufferCreateInfo-flags-00917 - if sparse_level.sparse_aliased && !device.enabled_features().sparse_residency_aliased { - return Err(BufferError::RequirementNotMet { - required_for: "`create_info.sparse` is `Some(sparse_level)`, where \ - `sparse_level` contains `BufferCreateFlags::SPARSE_ALIASED`", - requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature("sparse_residency_aliased")])]), - }); - } - - // VUID-VkBufferCreateInfo-flags-00918 - } - */ - - match sharing { - Sharing::Exclusive => (), - Sharing::Concurrent(queue_family_indices) => { - // VUID-VkBufferCreateInfo-sharingMode-00914 - assert!(queue_family_indices.len() >= 2); - - for &queue_family_index in queue_family_indices.iter() { - // VUID-VkBufferCreateInfo-sharingMode-01419 - if queue_family_index - >= device.physical_device().queue_family_properties().len() as u32 - { - return Err(BufferError::SharingQueueFamilyIndexOutOfRange { - queue_family_index, - queue_family_count: device - .physical_device() - .queue_family_properties() - .len() as u32, - }); - } - } - } - } - - if let Some(max_buffer_size) = device.physical_device().properties().max_buffer_size { - // VUID-VkBufferCreateInfo-size-06409 - if size > max_buffer_size { - return Err(BufferError::MaxBufferSizeExceeded { - size, - max: max_buffer_size, - }); - } - } - - if !external_memory_handle_types.is_empty() { - if !(device.api_version() >= Version::V1_1 - || device.enabled_extensions().khr_external_memory) - { - return Err(BufferError::RequirementNotMet { - required_for: "`create_info.external_memory_handle_types` is not empty", - requires_one_of: RequiresOneOf(&[ - RequiresAllOf(&[Requires::APIVersion(Version::V1_1)]), - RequiresAllOf(&[Requires::DeviceExtension("khr_external_memory")]), - ]), - }); - } - - // VUID-VkExternalMemoryBufferCreateInfo-handleTypes-parameter - external_memory_handle_types.validate_device(device)?; - - // VUID-VkBufferCreateInfo-pNext-00920 - // TODO: - } + fn validate_new( + device: &Device, + create_info: &BufferCreateInfo, + ) -> Result<(), ValidationError> { + create_info + .validate(device) + .map_err(|err| err.add_context("create_info"))?; Ok(()) } @@ -387,16 +277,16 @@ impl RawBuffer { pub fn bind_memory( self, allocation: MemoryAlloc, - ) -> Result { + ) -> Result { if let Err(err) = self.validate_bind_memory(&allocation) { - return Err((err, self, allocation)); + return Err((err.into(), self, allocation)); } unsafe { self.bind_memory_unchecked(allocation) } .map_err(|(err, buffer, allocation)| (err.into(), buffer, allocation)) } - fn validate_bind_memory(&self, allocation: &MemoryAlloc) -> Result<(), BufferError> { + fn validate_bind_memory(&self, allocation: &MemoryAlloc) -> Result<(), ValidationError> { assert_ne!(allocation.allocation_type(), AllocationType::NonLinear); let memory_requirements = &self.memory_requirements; @@ -421,87 +311,117 @@ impl RawBuffer { // VUID-VkBindBufferMemoryInfo-memoryOffset-01031 // Assume that `allocation` was created correctly. - // VUID-VkBindBufferMemoryInfo-memory-01035 if memory_requirements.memory_type_bits & (1 << memory.memory_type_index()) == 0 { - return Err(BufferError::MemoryTypeNotAllowed { - provided_memory_type_index: memory.memory_type_index(), - allowed_memory_type_bits: memory_requirements.memory_type_bits, + return Err(ValidationError { + problem: "`allocation.device_memory().memory_type_index()` is not a bit set in \ + `self.memory_requirements().memory_type_bits`" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-memory-01035"], + ..Default::default() }); } - // VUID-VkBindBufferMemoryInfo-memoryOffset-01036 if !is_aligned(memory_offset, memory_requirements.layout.alignment()) { - return Err(BufferError::MemoryAllocationNotAligned { - allocation_offset: memory_offset, - required_alignment: memory_requirements.layout.alignment(), + return Err(ValidationError { + problem: "`allocation.offset()` is not aligned according to \ + `self.memory_requirements().layout.alignment()`" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-memoryOffset-01036"], + ..Default::default() }); } - // VUID-VkBindBufferMemoryInfo-size-01037 if allocation.size() < memory_requirements.layout.size() { - return Err(BufferError::MemoryAllocationTooSmall { - allocation_size: allocation.size(), - required_size: memory_requirements.layout.size(), + return Err(ValidationError { + problem: "`allocation.size()` is less than \ + `self.memory_requirements().layout.size()`" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-size-01037"], + ..Default::default() }); } if let Some(dedicated_to) = memory.dedicated_to() { - // VUID-VkBindBufferMemoryInfo-memory-01508 match dedicated_to { DedicatedTo::Buffer(id) if id == self.id => {} - _ => return Err(BufferError::DedicatedAllocationMismatch), + _ => { + return Err(ValidationError { + problem: "`allocation.device_memory()` is a dedicated allocation, but \ + it is not dedicated to this buffer" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-memory-01508"], + ..Default::default() + }); + } } debug_assert!(memory_offset == 0); // This should be ensured by the allocator } else { - // VUID-VkBindBufferMemoryInfo-buffer-01444 if memory_requirements.requires_dedicated_allocation { - return Err(BufferError::DedicatedAllocationRequired); - } - } - - // VUID-VkBindBufferMemoryInfo-None-01899 - if memory_type - .property_flags - .intersects(MemoryPropertyFlags::PROTECTED) - { - return Err(BufferError::MemoryProtectedMismatch { - buffer_protected: false, - memory_protected: true, - }); - } - - // VUID-VkBindBufferMemoryInfo-memory-02726 - if !memory.export_handle_types().is_empty() - && !memory - .export_handle_types() - .intersects(self.external_memory_handle_types) - { - return Err(BufferError::MemoryExternalHandleTypesDisjoint { - buffer_handle_types: self.external_memory_handle_types, - memory_export_handle_types: memory.export_handle_types(), - }); - } - - if let Some(handle_type) = memory.imported_handle_type() { - // VUID-VkBindBufferMemoryInfo-memory-02985 - if !ExternalMemoryHandleTypes::from(handle_type) - .intersects(self.external_memory_handle_types) - { - return Err(BufferError::MemoryImportedHandleTypeNotEnabled { - buffer_handle_types: self.external_memory_handle_types, - memory_imported_handle_type: handle_type, + return Err(ValidationError { + problem: "`self.memory_requirements().requires_dedicated_allocation` is \ + `true`, but `allocation.device_memory()` is not a dedicated allocation" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-buffer-01444"], + ..Default::default() + }); + } + } + + if memory_type + .property_flags + .intersects(MemoryPropertyFlags::PROTECTED) + { + return Err(ValidationError { + problem: "the `property_flags` of the memory type of \ + `allocation.device_memory()` contains `MemoryPropertyFlags::PROTECTED`" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-None-01899"], + ..Default::default() + }); + } + + if !memory.export_handle_types().is_empty() + && !self + .external_memory_handle_types + .intersects(memory.export_handle_types()) + { + return Err(ValidationError { + problem: "`allocation.device_memory().export_handle_types()` is not empty, but \ + it does not share at least one memory type with \ + `self.external_memory_handle_types()`" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-memory-02726"], + ..Default::default() + }); + } + + if let Some(handle_type) = memory.imported_handle_type() { + if !self.external_memory_handle_types.contains_enum(handle_type) { + return Err(ValidationError { + problem: "`allocation.device_memory()` is imported, but \ + `self.external_memory_handle_types()` does not contain the imported \ + handle type" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-memory-02985"], + ..Default::default() }); } } - // VUID-VkBindBufferMemoryInfo-bufferDeviceAddress-03339 if !self.device.enabled_extensions().ext_buffer_device_address && self.usage.intersects(BufferUsage::SHADER_DEVICE_ADDRESS) && !memory .flags() .intersects(MemoryAllocateFlags::DEVICE_ADDRESS) { - return Err(BufferError::MemoryBufferDeviceAddressNotSupported); + return Err(ValidationError { + problem: "`self.usage()` contains `BufferUsage::SHADER_DEVICE_ADDRESS`, but \ + `allocation.device_memory().flags()` does not contain \ + `MemoryAllocateFlags::DEVICE_ADDRESS`" + .into(), + vuids: &["VUID-VkBindBufferMemoryInfo-bufferDeviceAddress-03339"], + ..Default::default() + }); } Ok(()) @@ -624,9 +544,9 @@ impl_id_counter!(RawBuffer); /// Parameters to create a new [`Buffer`]. #[derive(Clone, Debug)] pub struct BufferCreateInfo { - /// Flags to enable. + /// Additional properties of the buffer. /// - /// The default value is [`BufferCreateFlags::empty()`]. + /// The default value is empty. pub flags: BufferCreateFlags, /// Whether the buffer can be shared across multiple queues, or is limited to a single queue. @@ -676,6 +596,176 @@ impl Default for BufferCreateInfo { } } +impl BufferCreateInfo { + pub(crate) fn validate(&self, device: &Device) -> Result<(), ValidationError> { + let &Self { + flags, + ref sharing, + size, + usage, + external_memory_handle_types, + _ne: _, + } = self; + + flags + .validate_device(device) + .map_err(|err| ValidationError { + context: "flags".into(), + vuids: &["VUID-VkBufferCreateInfo-flags-parameter"], + ..ValidationError::from_requirement(err) + })?; + + usage + .validate_device(device) + .map_err(|err| ValidationError { + context: "usage".into(), + vuids: &["VUID-VkBufferCreateInfo-usage-parameter"], + ..ValidationError::from_requirement(err) + })?; + + if usage.is_empty() { + return Err(ValidationError { + context: "usage".into(), + problem: "is empty".into(), + vuids: &["VUID-VkBufferCreateInfo-usage-requiredbitmask"], + ..Default::default() + }); + } + + if size == 0 { + return Err(ValidationError { + context: "size".into(), + problem: "is zero".into(), + vuids: &["VUID-VkBufferCreateInfo-size-00912"], + ..Default::default() + }); + } + + /* Enable when sparse binding is properly handled + if let Some(sparse_level) = sparse { + if !device.enabled_features().sparse_binding { + return Err(ValidationError { + context: "sparse".into(), + problem: "is `Some`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "sparse_binding", + )])]), + vuids: &["VUID-VkBufferCreateInfo-flags-00915"], + }); + } + + if sparse_level.sparse_residency && !device.enabled_features().sparse_residency_buffer { + return Err(ValidationError { + context: "sparse".into(), + problem: "contains `BufferCreateFlags::SPARSE_RESIDENCY`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "sparse_residency_buffer", + )])]), + vuids: &["VUID-VkBufferCreateInfo-flags-00916"], + }); + } + + if sparse_level.sparse_aliased && !device.enabled_features().sparse_residency_aliased { + return Err(ValidationError { + context: "sparse".into(), + problem: "contains `BufferCreateFlags::SPARSE_ALIASED`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "sparse_residency_aliased", + )])]), + vuids: &["VUID-VkBufferCreateInfo-flags-00917"], + }); + } + + // TODO: + // VUID-VkBufferCreateInfo-flags-00918 + }*/ + + match sharing { + Sharing::Exclusive => (), + Sharing::Concurrent(queue_family_indices) => { + if queue_family_indices.len() < 2 { + return Err(ValidationError { + context: "sharing".into(), + problem: "is `Sharing::Concurrent`, but contains less than 2 elements" + .into(), + vuids: &["VUID-VkBufferCreateInfo-sharingMode-00914"], + ..Default::default() + }); + } + + let queue_family_count = + device.physical_device().queue_family_properties().len() as u32; + + for (index, &queue_family_index) in queue_family_indices.iter().enumerate() { + if queue_family_indices[..index].contains(&queue_family_index) { + return Err(ValidationError { + context: "queue_family_indices".into(), + problem: format!( + "the queue family index in the list at index {} is contained in \ + the list more than once", + index, + ) + .into(), + vuids: &["VUID-VkBufferCreateInfo-sharingMode-01419"], + ..Default::default() + }); + } + + if queue_family_index >= queue_family_count { + return Err(ValidationError { + context: format!("sharing[{}]", index).into(), + problem: "is not less than the number of queue families in the device" + .into(), + vuids: &["VUID-VkBufferCreateInfo-sharingMode-01419"], + ..Default::default() + }); + } + } + } + } + + if let Some(max_buffer_size) = device.physical_device().properties().max_buffer_size { + if size > max_buffer_size { + return Err(ValidationError { + context: "size".into(), + problem: "exceeds the `max_buffer_size` limit".into(), + vuids: &["VUID-VkBufferCreateInfo-size-06409"], + ..Default::default() + }); + } + } + + if !external_memory_handle_types.is_empty() { + if !(device.api_version() >= Version::V1_1 + || device.enabled_extensions().khr_external_memory) + { + return Err(ValidationError { + context: "external_memory_handle_types".into(), + problem: "is not empty".into(), + requires_one_of: RequiresOneOf(&[ + RequiresAllOf(&[Requires::APIVersion(Version::V1_1)]), + RequiresAllOf(&[Requires::DeviceExtension("khr_external_memory")]), + ]), + ..Default::default() + }); + } + + external_memory_handle_types + .validate_device(device) + .map_err(|err| ValidationError { + context: "external_memory_handle_types".into(), + vuids: &["VUID-VkExternalMemoryBufferCreateInfo-handleTypes-parameter"], + ..ValidationError::from_requirement(err) + })?; + + // TODO: + // VUID-VkBufferCreateInfo-pNext-00920 + } + + Ok(()) + } +} + #[cfg(test)] mod tests { use super::{BufferCreateInfo, BufferUsage, RawBuffer}; @@ -774,15 +864,17 @@ mod tests { fn create_empty_buffer() { let (device, _) = gfx_dev_and_queue!(); - assert_should_panic!({ - RawBuffer::new( - device, - BufferCreateInfo { - size: 0, - usage: BufferUsage::TRANSFER_DST, - ..Default::default() - }, - ) - }); + if RawBuffer::new( + device, + BufferCreateInfo { + size: 0, + usage: BufferUsage::TRANSFER_DST, + ..Default::default() + }, + ) + .is_ok() + { + panic!() + } } } diff --git a/vulkano/src/buffer/view.rs b/vulkano/src/buffer/view.rs index f25b7c60..83505eca 100644 --- a/vulkano/src/buffer/view.rs +++ b/vulkano/src/buffer/view.rs @@ -53,17 +53,9 @@ use crate::{ format::{Format, FormatFeatures}, macros::impl_id_counter, memory::{is_aligned, DeviceAlignment}, - DeviceSize, OomError, RequirementNotMet, RequiresOneOf, RuntimeError, Version, VulkanObject, -}; -use std::{ - error::Error, - fmt::{Display, Error as FmtError, Formatter}, - mem::MaybeUninit, - num::NonZeroU64, - ops::Range, - ptr, - sync::Arc, + DeviceSize, RuntimeError, ValidationError, Version, VulkanError, VulkanObject, }; +use std::{mem::MaybeUninit, num::NonZeroU64, ops::Range, ptr, sync::Arc}; /// Represents a way for the GPU to interpret buffer data. See the documentation of the /// `view` module. @@ -84,37 +76,29 @@ impl BufferView { pub fn new( subbuffer: Subbuffer, create_info: BufferViewCreateInfo, - ) -> Result, BufferViewCreationError> { - Self::new_inner(subbuffer.into_bytes(), create_info) + ) -> Result, VulkanError> { + let subbuffer = subbuffer.into_bytes(); + Self::validate_new(&subbuffer, &create_info)?; + + unsafe { Ok(Self::new_unchecked(subbuffer, create_info)?) } } - fn new_inner( - subbuffer: Subbuffer<[u8]>, - create_info: BufferViewCreateInfo, - ) -> Result, BufferViewCreationError> { + fn validate_new( + subbuffer: &Subbuffer<[u8]>, + create_info: &BufferViewCreateInfo, + ) -> Result<(), ValidationError> { + let device = subbuffer.device(); + + create_info + .validate(device) + .map_err(|err| err.add_context("create_info"))?; + let BufferViewCreateInfo { format, _ne: _ } = create_info; let buffer = subbuffer.buffer(); - let device = buffer.device(); let properties = device.physical_device().properties(); - let size = subbuffer.size(); - let offset = subbuffer.offset(); - // No VUID, but seems sensible? let format = format.unwrap(); - - // VUID-VkBufferViewCreateInfo-format-parameter - format.validate_device(device)?; - - // VUID-VkBufferViewCreateInfo-buffer-00932 - if !buffer - .usage() - .intersects(BufferUsage::UNIFORM_TEXEL_BUFFER | BufferUsage::STORAGE_TEXEL_BUFFER) - { - return Err(BufferViewCreationError::BufferMissingUsage); - } - - // Use unchecked, because all validation has been done above. let format_features = unsafe { device .physical_device() @@ -122,36 +106,70 @@ impl BufferView { .buffer_features }; - // VUID-VkBufferViewCreateInfo-buffer-00933 + if !buffer + .usage() + .intersects(BufferUsage::UNIFORM_TEXEL_BUFFER | BufferUsage::STORAGE_TEXEL_BUFFER) + { + return Err(ValidationError { + context: "subbuffer".into(), + problem: "was not created with the `BufferUsage::UNIFORM_TEXEL_BUFFER` \ + or `BufferUsage::STORAGE_TEXEL_BUFFER` usage" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-buffer-00932"], + ..Default::default() + }); + } + if buffer.usage().intersects(BufferUsage::UNIFORM_TEXEL_BUFFER) && !format_features.intersects(FormatFeatures::UNIFORM_TEXEL_BUFFER) { - return Err(BufferViewCreationError::UnsupportedFormat); + return Err(ValidationError { + problem: "`subbuffer` was created with the `BufferUsage::UNIFORM_TEXEL_BUFFER` \ + usage, but the format features of `create_info.format` do not include \ + `FormatFeatures::UNIFORM_TEXEL_BUFFER`" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-buffer-00933"], + ..Default::default() + }); } - // VUID-VkBufferViewCreateInfo-buffer-00934 if buffer.usage().intersects(BufferUsage::STORAGE_TEXEL_BUFFER) && !format_features.intersects(FormatFeatures::STORAGE_TEXEL_BUFFER) { - return Err(BufferViewCreationError::UnsupportedFormat); + return Err(ValidationError { + problem: "`subbuffer` was created with the `BufferUsage::STORAGE_TEXEL_BUFFER` \ + usage, but the format features of `create_info.format` do not include \ + `FormatFeatures::STORAGE_TEXEL_BUFFER`" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-buffer-00934"], + ..Default::default() + }); } let block_size = format.block_size().unwrap(); let texels_per_block = format.texels_per_block(); - // VUID-VkBufferViewCreateInfo-range-00929 - if size % block_size != 0 { - return Err(BufferViewCreationError::RangeNotAligned { - range: size, - required_alignment: block_size, + if subbuffer.size() % block_size != 0 { + return Err(ValidationError { + problem: "`subbuffer.size()` is not a multiple of \ + `create_info.format.block_size()`" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-range-00929"], + ..Default::default() }); } - // VUID-VkBufferViewCreateInfo-range-00930 - if ((size / block_size) * texels_per_block as DeviceSize) as u32 + if ((subbuffer.size() / block_size) * texels_per_block as DeviceSize) as u32 > properties.max_texel_buffer_elements { - return Err(BufferViewCreationError::MaxTexelBufferElementsExceeded); + return Err(ValidationError { + problem: "`subbuffer.size() / create_info.format.block_size() * \ + create_info.format.texels_per_block()` is greater than the \ + `max_texel_buffer_elements` limit" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-range-00930"], + ..Default::default() + }); } if device.api_version() >= Version::V1_3 || device.enabled_features().texel_buffer_alignment @@ -164,64 +182,131 @@ impl BufferView { .unwrap(); if buffer.usage().intersects(BufferUsage::STORAGE_TEXEL_BUFFER) { - let mut required_alignment = properties - .storage_texel_buffer_offset_alignment_bytes - .unwrap(); - if properties .storage_texel_buffer_offset_single_texel_alignment .unwrap() { - required_alignment = required_alignment.min(element_size); - } - - // VUID-VkBufferViewCreateInfo-buffer-02750 - if !is_aligned(offset, required_alignment) { - return Err(BufferViewCreationError::OffsetNotAligned { - offset, - required_alignment, - }); + if !is_aligned( + subbuffer.offset(), + properties + .storage_texel_buffer_offset_alignment_bytes + .unwrap() + .min(element_size), + ) { + return Err(ValidationError { + problem: "`subbuffer` was created with the \ + `BufferUsage::STORAGE_TEXEL_BUFFER` usage, and the \ + `storage_texel_buffer_offset_single_texel_alignment` \ + property is `true`, but \ + `subbuffer.offset()` is not a multiple of the \ + minimum of `create_info.format.block_size()` and the \ + `storage_texel_buffer_offset_alignment_bytes` limit" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-buffer-02750"], + ..Default::default() + }); + } + } else { + if !is_aligned( + subbuffer.offset(), + properties + .storage_texel_buffer_offset_alignment_bytes + .unwrap(), + ) { + return Err(ValidationError { + problem: "`subbuffer` was created with the \ + `BufferUsage::STORAGE_TEXEL_BUFFER` usage, and the \ + `storage_texel_buffer_offset_single_texel_alignment` \ + property is `false`, but \ + `subbuffer.offset()` is not a multiple of the \ + `storage_texel_buffer_offset_alignment_bytes` limit" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-buffer-02750"], + ..Default::default() + }); + } } } if buffer.usage().intersects(BufferUsage::UNIFORM_TEXEL_BUFFER) { - let mut required_alignment = properties - .uniform_texel_buffer_offset_alignment_bytes - .unwrap(); - if properties .uniform_texel_buffer_offset_single_texel_alignment .unwrap() { - required_alignment = required_alignment.min(element_size); - } - - // VUID-VkBufferViewCreateInfo-buffer-02751 - if !is_aligned(offset, required_alignment) { - return Err(BufferViewCreationError::OffsetNotAligned { - offset, - required_alignment, - }); + if !is_aligned( + subbuffer.offset(), + properties + .uniform_texel_buffer_offset_alignment_bytes + .unwrap() + .min(element_size), + ) { + return Err(ValidationError { + problem: "`subbuffer` was created with the \ + `BufferUsage::UNIFORM_TEXEL_BUFFER` usage, and the \ + `uniform_texel_buffer_offset_single_texel_alignment` \ + property is `false`, but \ + `subbuffer.offset()` is not a multiple of the \ + minimum of `create_info.format.block_size()` and the \ + `uniform_texel_buffer_offset_alignment_bytes` limit" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-buffer-02751"], + ..Default::default() + }); + } + } else { + if !is_aligned( + subbuffer.offset(), + properties + .uniform_texel_buffer_offset_alignment_bytes + .unwrap(), + ) { + return Err(ValidationError { + problem: "`subbuffer` was created with the \ + `BufferUsage::UNIFORM_TEXEL_BUFFER` usage, and the \ + `uniform_texel_buffer_offset_single_texel_alignment` \ + property is `false`, but \ + `subbuffer.offset()` is not a multiple of the \ + `uniform_texel_buffer_offset_alignment_bytes` limit" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-buffer-02751"], + ..Default::default() + }); + } } } } else { - let required_alignment = properties.min_texel_buffer_offset_alignment; - - // VUID-VkBufferViewCreateInfo-offset-02749 - if !is_aligned(offset, required_alignment) { - return Err(BufferViewCreationError::OffsetNotAligned { - offset, - required_alignment, + if !is_aligned( + subbuffer.offset(), + properties.min_texel_buffer_offset_alignment, + ) { + return Err(ValidationError { + problem: "`subbuffer.offset()` is not a multiple of the \ + `min_texel_buffer_offset_alignment` limit" + .into(), + vuids: &["VUID-VkBufferViewCreateInfo-offset-02749"], + ..Default::default() }); } } - let create_info = ash::vk::BufferViewCreateInfo { + Ok(()) + } + + #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] + pub unsafe fn new_unchecked( + subbuffer: Subbuffer, + create_info: BufferViewCreateInfo, + ) -> Result, RuntimeError> { + let &BufferViewCreateInfo { format, _ne: _ } = &create_info; + + let device = subbuffer.device(); + + let create_info_vk = ash::vk::BufferViewCreateInfo { flags: ash::vk::BufferViewCreateFlags::empty(), - buffer: buffer.handle(), - format: format.into(), - offset, - range: size, + buffer: subbuffer.buffer().handle(), + format: format.unwrap().into(), + offset: subbuffer.offset(), + range: subbuffer.size(), ..Default::default() }; @@ -230,7 +315,7 @@ impl BufferView { let mut output = MaybeUninit::uninit(); (fns.v1_0.create_buffer_view)( device.handle(), - &create_info, + &create_info_vk, ptr::null(), output.as_mut_ptr(), ) @@ -239,14 +324,39 @@ impl BufferView { output.assume_init() }; - Ok(Arc::new(BufferView { + Ok(Self::from_handle(subbuffer, handle, create_info)) + } + + /// Creates a new `BufferView` from a raw object handle. + /// + /// # Safety + /// + /// - `handle` must be a valid Vulkan object handle created from `device`. + /// - `subbuffer` and `create_info` must match the info used to create the object. + pub unsafe fn from_handle( + subbuffer: Subbuffer, + handle: ash::vk::BufferView, + create_info: BufferViewCreateInfo, + ) -> Arc { + let &BufferViewCreateInfo { format, _ne: _ } = &create_info; + let format = format.unwrap(); + let size = subbuffer.size(); + let format_features = unsafe { + subbuffer + .device() + .physical_device() + .format_properties_unchecked(format) + .buffer_features + }; + + Arc::new(BufferView { handle, - subbuffer, + subbuffer: subbuffer.into_bytes(), id: Self::next_id(), format: Some(format), format_features, range: 0..size, - })) + }) } /// Returns the buffer associated to this view. @@ -327,108 +437,31 @@ impl Default for BufferViewCreateInfo { } } -/// Error that can happen when creating a buffer view. -#[derive(Debug, Copy, Clone)] -pub enum BufferViewCreationError { - /// Out of memory. - OomError(OomError), +impl BufferViewCreateInfo { + pub(crate) fn validate(&self, device: &Device) -> Result<(), ValidationError> { + let Self { format, _ne: _ } = self; - RequirementNotMet { - required_for: &'static str, - requires_one_of: RequiresOneOf, - }, + let format = format.ok_or(ValidationError { + context: "format".into(), + problem: "is `None`".into(), + ..Default::default() + })?; - /// The buffer was not created with one of the `storage_texel_buffer` or - /// `uniform_texel_buffer` usages. - BufferMissingUsage, + format + .validate_device(device) + .map_err(|err| ValidationError { + context: "format".into(), + vuids: &["VUID-VkBufferViewCreateInfo-format-parameter"], + ..ValidationError::from_requirement(err) + })?; - /// The offset within the buffer is not a multiple of the required alignment. - OffsetNotAligned { - offset: DeviceSize, - required_alignment: DeviceAlignment, - }, - - /// The range within the buffer is not a multiple of the required alignment. - RangeNotAligned { - range: DeviceSize, - required_alignment: DeviceSize, - }, - - /// The requested format is not supported for this usage. - UnsupportedFormat, - - /// The `max_texel_buffer_elements` limit has been exceeded. - MaxTexelBufferElementsExceeded, -} - -impl Error for BufferViewCreationError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self { - BufferViewCreationError::OomError(err) => Some(err), - _ => None, - } - } -} - -impl Display for BufferViewCreationError { - fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { - match self { - Self::OomError(_) => write!(f, "out of memory when creating buffer view"), - Self::RequirementNotMet { - required_for, - requires_one_of, - } => write!( - f, - "a requirement was not met for: {}; requires one of: {}", - required_for, requires_one_of, - ), - Self::BufferMissingUsage => write!( - f, - "the buffer was not created with one of the `storage_texel_buffer` or \ - `uniform_texel_buffer` usages", - ), - Self::OffsetNotAligned { .. } => write!( - f, - "the offset within the buffer is not a multiple of the required alignment", - ), - Self::RangeNotAligned { .. } => write!( - f, - "the range within the buffer is not a multiple of the required alignment", - ), - Self::UnsupportedFormat => { - write!(f, "the requested format is not supported for this usage") - } - Self::MaxTexelBufferElementsExceeded => { - write!(f, "the `max_texel_buffer_elements` limit has been exceeded") - } - } - } -} - -impl From for BufferViewCreationError { - fn from(err: OomError) -> Self { - Self::OomError(err) - } -} - -impl From for BufferViewCreationError { - fn from(err: RuntimeError) -> Self { - OomError::from(err).into() - } -} - -impl From for BufferViewCreationError { - fn from(err: RequirementNotMet) -> Self { - Self::RequirementNotMet { - required_for: err.required_for, - requires_one_of: err.requires_one_of, - } + Ok(()) } } #[cfg(test)] mod tests { - use super::{BufferView, BufferViewCreateInfo, BufferViewCreationError}; + use super::{BufferView, BufferViewCreateInfo}; use crate::{ buffer::{Buffer, BufferCreateInfo, BufferUsage}, format::Format, @@ -540,7 +573,7 @@ mod tests { ..Default::default() }, ) { - Err(BufferViewCreationError::BufferMissingUsage) => (), + Err(_) => (), _ => panic!(), } } @@ -569,7 +602,7 @@ mod tests { ..Default::default() }, ) { - Err(BufferViewCreationError::UnsupportedFormat) => (), + Err(_) => (), _ => panic!(), } } diff --git a/vulkano/src/image/immutable.rs b/vulkano/src/image/immutable.rs index d4341ba2..7a509bee 100644 --- a/vulkano/src/image/immutable.rs +++ b/vulkano/src/image/immutable.rs @@ -14,7 +14,9 @@ use super::{ ImageSubresourceLayers, ImageUsage, MipmapsCount, }; use crate::{ - buffer::{Buffer, BufferContents, BufferCreateInfo, BufferError, BufferUsage, Subbuffer}, + buffer::{ + Buffer, BufferAllocateError, BufferContents, BufferCreateInfo, BufferUsage, Subbuffer, + }, command_buffer::{ allocator::CommandBufferAllocator, auto::CommandBufferBeginError, AutoCommandBufferBuilder, BlitImageInfo, BufferImageCopy, CopyBufferToImageInfo, ImageBlit, @@ -204,7 +206,7 @@ impl ImmutableImage { iter, ) .map_err(|err| match err { - BufferError::AllocError(err) => err, + BufferAllocateError::AllocateMemory(err) => err, // We don't use sparse-binding, concurrent sharing or external memory, therefore the // other errors can't happen. _ => unreachable!(), diff --git a/vulkano/src/image/sys.rs b/vulkano/src/image/sys.rs index e786aa42..eb09fd31 100644 --- a/vulkano/src/image/sys.rs +++ b/vulkano/src/image/sys.rs @@ -19,7 +19,6 @@ use super::{ SampleCounts, SparseImageMemoryRequirements, }; use crate::{ - buffer::subbuffer::{ReadLockError, WriteLockError}, cache::OnceCache, device::{Device, DeviceOwned}, format::{ChromaSampling, Format, FormatFeatures, NumericType}, @@ -35,7 +34,7 @@ use crate::{ }, range_map::RangeMap, swapchain::Swapchain, - sync::{future::AccessError, CurrentAccess, Sharing}, + sync::{future::AccessError, AccessConflict, CurrentAccess, Sharing}, DeviceSize, RequirementNotMet, Requires, RequiresAllOf, RequiresOneOf, RuntimeError, Version, VulkanObject, }; @@ -2409,11 +2408,11 @@ impl ImageState { } #[allow(dead_code)] - pub(crate) fn check_cpu_read(&self, range: Range) -> Result<(), ReadLockError> { + pub(crate) fn check_cpu_read(&self, range: Range) -> Result<(), AccessConflict> { for (_range, state) in self.ranges.range(&range) { match &state.current_access { - CurrentAccess::CpuExclusive { .. } => return Err(ReadLockError::CpuWriteLocked), - CurrentAccess::GpuExclusive { .. } => return Err(ReadLockError::GpuWriteLocked), + CurrentAccess::CpuExclusive { .. } => return Err(AccessConflict::HostWrite), + CurrentAccess::GpuExclusive { .. } => return Err(AccessConflict::DeviceWrite), CurrentAccess::Shared { .. } => (), } } @@ -2450,19 +2449,19 @@ impl ImageState { } #[allow(dead_code)] - pub(crate) fn check_cpu_write(&self, range: Range) -> Result<(), WriteLockError> { + pub(crate) fn check_cpu_write(&self, range: Range) -> Result<(), AccessConflict> { for (_range, state) in self.ranges.range(&range) { match &state.current_access { - CurrentAccess::CpuExclusive => return Err(WriteLockError::CpuLocked), - CurrentAccess::GpuExclusive { .. } => return Err(WriteLockError::GpuLocked), + CurrentAccess::CpuExclusive => return Err(AccessConflict::HostWrite), + CurrentAccess::GpuExclusive { .. } => return Err(AccessConflict::DeviceWrite), CurrentAccess::Shared { cpu_reads: 0, gpu_reads: 0, } => (), CurrentAccess::Shared { cpu_reads, .. } if *cpu_reads > 0 => { - return Err(WriteLockError::CpuLocked) + return Err(AccessConflict::HostRead) } - CurrentAccess::Shared { .. } => return Err(WriteLockError::GpuLocked), + CurrentAccess::Shared { .. } => return Err(AccessConflict::DeviceRead), } } diff --git a/vulkano/src/swapchain/mod.rs b/vulkano/src/swapchain/mod.rs index 74d9db1e..f38e1d49 100644 --- a/vulkano/src/swapchain/mod.rs +++ b/vulkano/src/swapchain/mod.rs @@ -1855,6 +1855,7 @@ impl SwapchainCreateInfo { for (index, &queue_family_index) in queue_family_indices.iter().enumerate() { if queue_family_indices[..index].contains(&queue_family_index) { return Err(ValidationError { + context: "queue_family_indices".into(), problem: format!( "the queue family index in the list at index {} is contained in \ the list more than once", diff --git a/vulkano/src/sync/mod.rs b/vulkano/src/sync/mod.rs index 932a6621..539850dd 100644 --- a/vulkano/src/sync/mod.rs +++ b/vulkano/src/sync/mod.rs @@ -25,8 +25,12 @@ pub use self::{ MemoryBarrier, PipelineStage, PipelineStages, QueueFamilyOwnershipTransfer, }, }; -use crate::device::Queue; -use std::sync::Arc; +use crate::{device::Queue, RuntimeError, ValidationError}; +use std::{ + error::Error, + fmt::{Display, Formatter}, + sync::Arc, +}; pub mod event; pub mod fence; @@ -94,3 +98,74 @@ pub(crate) enum CurrentAccess { /// The resource is not currently being accessed, or is being accessed for reading only. Shared { cpu_reads: usize, gpu_reads: usize }, } + +/// Error when attempting to read or write a resource from the host (CPU). +#[derive(Clone, Debug)] +pub enum HostAccessError { + AccessConflict(AccessConflict), + Invalidate(RuntimeError), + ValidationError(ValidationError), +} + +impl Error for HostAccessError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::AccessConflict(err) => Some(err), + Self::Invalidate(err) => Some(err), + Self::ValidationError(err) => Some(err), + } + } +} + +impl Display for HostAccessError { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { + match self { + Self::AccessConflict(_) => { + write!(f, "the resource is already in use in a conflicting way") + } + HostAccessError::Invalidate(_) => write!(f, "invalidating the device memory failed"), + HostAccessError::ValidationError(_) => write!(f, "validation error"), + } + } +} + +/// Conflict when attempting to access a resource. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum AccessConflict { + /// The resource is already locked for reading by the host (CPU). + HostRead, + + /// The resource is already locked for writing by the host (CPU). + HostWrite, + + /// The resource is already locked for reading by the device (GPU). + DeviceRead, + + /// The resource is already locked for writing by the device (GPU). + DeviceWrite, +} + +impl Error for AccessConflict {} + +impl Display for AccessConflict { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { + match self { + AccessConflict::HostRead => write!( + f, + "the resource is already locked for reading by the host (CPU)" + ), + AccessConflict::HostWrite => write!( + f, + "the resource is already locked for writing by the host (CPU)" + ), + AccessConflict::DeviceRead => write!( + f, + "the resource is already locked for reading by the device (GPU)" + ), + AccessConflict::DeviceWrite => write!( + f, + "the resource is already locked for writing by the device (GPU)" + ), + } + } +}