From 9475fbad77512c5a5c8b8fba87b6d9b639f3f7b6 Mon Sep 17 00:00:00 2001 From: Rua Date: Sun, 8 Oct 2023 17:51:53 +0200 Subject: [PATCH 1/4] Fix the documentation for `DescriptorPool` methods --- vulkano/src/descriptor_set/pool.rs | 73 ++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/vulkano/src/descriptor_set/pool.rs b/vulkano/src/descriptor_set/pool.rs index eef0753083..422f3636e2 100644 --- a/vulkano/src/descriptor_set/pool.rs +++ b/vulkano/src/descriptor_set/pool.rs @@ -53,6 +53,18 @@ impl DescriptorPool { unsafe { Ok(Self::new_unchecked(device, create_info)?) } } + fn validate_new( + device: &Device, + create_info: &DescriptorPoolCreateInfo, + ) -> Result<(), Box> { + // VUID-vkCreateDescriptorPool-pCreateInfo-parameter + create_info + .validate(device) + .map_err(|err| err.add_context("create_info"))?; + + Ok(()) + } + #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] pub unsafe fn new_unchecked( device: Arc, @@ -113,18 +125,6 @@ impl DescriptorPool { unsafe { Ok(Self::from_handle(device, handle, create_info)) } } - fn validate_new( - device: &Device, - create_info: &DescriptorPoolCreateInfo, - ) -> Result<(), Box> { - // VUID-vkCreateDescriptorPool-pCreateInfo-parameter - create_info - .validate(device) - .map_err(|err| err.add_context("create_info"))?; - - Ok(()) - } - /// Creates a new `DescriptorPool` from a raw object handle. /// /// # Safety @@ -184,7 +184,7 @@ impl DescriptorPool { self.max_inline_uniform_block_bindings } - /// Allocates descriptor sets from the pool, one for each element in `create_info`. + /// Allocates descriptor sets from the pool, one for each element in `allocate_info`. /// Returns an iterator to the allocated sets, or an error. /// /// The `FragmentedPool` errors often can't be prevented. If the function returns this error, @@ -196,12 +196,8 @@ impl DescriptorPool { /// /// # Safety /// - /// See also the `new` function. - /// - /// - The total descriptors of the layouts must fit in the pool. - /// - The total number of descriptor sets allocated from the pool must not overflow the pool. - /// - You must ensure that the allocated descriptor sets are no longer in use when the pool - /// is destroyed, as destroying the pool is equivalent to freeing all the sets. + /// - When the pool is dropped, the returned descriptor sets must not be in use by either + /// the host or device. pub unsafe fn allocate_descriptor_sets<'a>( &self, allocate_info: impl IntoIterator>, @@ -281,13 +277,39 @@ impl DescriptorPool { /// /// # Safety /// - /// - The pool must have been created with `free_descriptor_set_bit` set to `true`. - /// - The descriptor sets must have been allocated from the pool. - /// - The descriptor sets must not be free'd twice. - /// - The descriptor sets must not be in use by the GPU. + /// - All elements of `descriptor_sets` must have been allocated from `self`, + /// and not freed previously. + /// - All elements of `descriptor_sets` must not be in use by the host or device. + #[inline] pub unsafe fn free_descriptor_sets( &self, descriptor_sets: impl IntoIterator, + ) -> Result<(), Validated> { + self.validate_free_descriptor_sets()?; + + Ok(self.free_descriptor_sets_unchecked(descriptor_sets)?) + } + + fn validate_free_descriptor_sets(&self) -> Result<(), Box> { + if !self + .flags + .intersects(DescriptorPoolCreateFlags::FREE_DESCRIPTOR_SET) + { + return Err(Box::new(ValidationError { + context: "self.flags()".into(), + problem: "does not contain `DescriptorPoolCreateFlags::FREE_DESCRIPTOR_SET`".into(), + vuids: &["VUID-vkFreeDescriptorSets-descriptorPool-00312"], + ..Default::default() + })); + } + + Ok(()) + } + + #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] + pub unsafe fn free_descriptor_sets_unchecked( + &self, + descriptor_sets: impl IntoIterator, ) -> Result<(), VulkanError> { let sets: SmallVec<[_; 8]> = descriptor_sets.into_iter().map(|s| s.handle()).collect(); if !sets.is_empty() { @@ -308,6 +330,11 @@ impl DescriptorPool { /// Resets the pool. /// /// This destroys all descriptor sets and empties the pool. + /// + /// # Safety + /// + /// - All descriptor sets that were previously allocated from `self` must not be in use by the + /// host or device. #[inline] pub unsafe fn reset(&self) -> Result<(), VulkanError> { let fns = self.device.fns(); From 091f3aa99f6c0978dd4034d1747860a217cd8364 Mon Sep 17 00:00:00 2001 From: Rua Date: Tue, 10 Oct 2023 16:12:54 +0200 Subject: [PATCH 2/4] Pool allocate validation --- vulkano/src/descriptor_set/allocator.rs | 140 ++++++++++---------- vulkano/src/descriptor_set/pool.rs | 164 +++++++++++++++++++----- 2 files changed, 198 insertions(+), 106 deletions(-) diff --git a/vulkano/src/descriptor_set/allocator.rs b/vulkano/src/descriptor_set/allocator.rs index 108c80651c..c2cef89f3a 100644 --- a/vulkano/src/descriptor_set/allocator.rs +++ b/vulkano/src/descriptor_set/allocator.rs @@ -23,7 +23,7 @@ use super::{ sys::UnsafeDescriptorSet, }; use crate::{ - descriptor_set::layout::{DescriptorSetLayoutCreateFlags, DescriptorType}, + descriptor_set::layout::DescriptorType, device::{Device, DeviceOwned}, instance::InstanceOwnedDebugWrapper, Validated, VulkanError, @@ -60,7 +60,7 @@ pub unsafe trait DescriptorSetAllocator: DeviceOwned { &self, layout: &Arc, variable_descriptor_count: u32, - ) -> Result; + ) -> Result>; } /// An allocated descriptor set. @@ -70,6 +70,9 @@ pub trait DescriptorSetAlloc: Send + Sync { /// Returns the inner unsafe descriptor set object. fn inner_mut(&mut self) -> &mut UnsafeDescriptorSet; + + /// Returns the descriptor pool that the descriptor set was allocated from. + fn pool(&self) -> &DescriptorPool; } /// Standard implementation of a descriptor set allocator. @@ -142,38 +145,15 @@ unsafe impl DescriptorSetAllocator for StandardDescriptorSetAllocator { type Alloc = StandardDescriptorSetAlloc; /// Allocates a descriptor set. - /// - /// # Panics - /// - /// - Panics if the provided `layout` is for push descriptors rather than regular descriptor - /// sets. - /// - Panics if the provided `variable_descriptor_count` is greater than the maximum number of - /// variable count descriptors in the set. #[inline] fn allocate( &self, layout: &Arc, variable_descriptor_count: u32, - ) -> Result { - assert!( - !layout - .flags() - .intersects(DescriptorSetLayoutCreateFlags::PUSH_DESCRIPTOR), - "the provided descriptor set layout is for push descriptors, and cannot be used to \ - build a descriptor set object", - ); - + ) -> Result> { let max_count = layout.variable_descriptor_count(); - - assert!( - variable_descriptor_count <= max_count, - "the provided variable_descriptor_count ({}) is greater than the maximum number of \ - variable count descriptors in the set ({})", - variable_descriptor_count, - max_count, - ); - let pools = self.pools.get_or(Default::default); + let entry = unsafe { &mut *pools.get() }.get_or_try_insert(layout.id(), || { if max_count == 0 { FixedEntry::new(layout.clone()).map(Entry::Fixed) @@ -197,7 +177,7 @@ unsafe impl DescriptorSetAllocator for Arc { &self, layout: &Arc, variable_descriptor_count: u32, - ) -> Result { + ) -> Result> { (**self).allocate(layout, variable_descriptor_count) } } @@ -221,7 +201,7 @@ struct FixedEntry { } impl FixedEntry { - fn new(layout: Arc) -> Result { + fn new(layout: Arc) -> Result> { Ok(FixedEntry { pool: FixedPool::new(&layout, MAX_SETS)?, set_count: MAX_SETS, @@ -229,7 +209,7 @@ impl FixedEntry { }) } - fn allocate(&mut self) -> Result { + fn allocate(&mut self) -> Result> { let inner = if let Some(inner) = self.pool.reserve.pop() { inner } else { @@ -250,14 +230,17 @@ impl FixedEntry { struct FixedPool { // The actual Vulkan descriptor pool. This field isn't actually used anywhere, but we need to // keep the pool alive in order to keep the descriptor sets valid. - _inner: DescriptorPool, + inner: DescriptorPool, // List of descriptor sets. When `alloc` is called, a descriptor will be extracted from this // list. When a `SingleLayoutPoolAlloc` is dropped, its descriptor set is put back in this list. reserve: ArrayQueue, } impl FixedPool { - fn new(layout: &Arc, set_count: usize) -> Result, VulkanError> { + fn new( + layout: &Arc, + set_count: usize, + ) -> Result, Validated> { let inner = DescriptorPool::new( layout.device().clone(), DescriptorPoolCreateInfo { @@ -275,28 +258,28 @@ impl FixedPool { ) .map_err(Validated::unwrap)?; - let allocate_infos = (0..set_count).map(|_| DescriptorSetAllocateInfo { - layout, - variable_descriptor_count: 0, - }); + let allocate_infos = (0..set_count).map(|_| DescriptorSetAllocateInfo::new(layout)); let allocs = unsafe { inner .allocate_descriptor_sets(allocate_infos) .map_err(|err| match err { - VulkanError::OutOfHostMemory | VulkanError::OutOfDeviceMemory => err, - VulkanError::FragmentedPool => { - // This can't happen as we don't free individual sets. - unreachable!(); - } - VulkanError::OutOfPoolMemory => { - // We created the pool with an exact size. - unreachable!(); - } - _ => { - // Shouldn't ever be returned. - unreachable!(); - } + Validated::ValidationError(_) => err, + Validated::Error(vk_err) => match vk_err { + VulkanError::OutOfHostMemory | VulkanError::OutOfDeviceMemory => err, + VulkanError::FragmentedPool => { + // This can't happen as we don't free individual sets. + unreachable!(); + } + VulkanError::OutOfPoolMemory => { + // We created the pool with an exact size. + unreachable!(); + } + _ => { + // Shouldn't ever be returned. + unreachable!(); + } + }, })? }; @@ -305,10 +288,7 @@ impl FixedPool { let _ = reserve.push(alloc); } - Ok(Arc::new(FixedPool { - _inner: inner, - reserve, - })) + Ok(Arc::new(FixedPool { inner, reserve })) } } @@ -326,7 +306,7 @@ struct VariableEntry { } impl VariableEntry { - fn new(layout: Arc) -> Result { + fn new(layout: Arc) -> Result> { let reserve = Arc::new(ArrayQueue::new(MAX_POOLS)); Ok(VariableEntry { @@ -340,7 +320,7 @@ impl VariableEntry { fn allocate( &mut self, variable_descriptor_count: u32, - ) -> Result { + ) -> Result> { if self.allocations >= MAX_SETS { self.pool = if let Some(inner) = self.reserve.pop() { Arc::new(VariablePool { @@ -354,8 +334,8 @@ impl VariableEntry { } let allocate_info = DescriptorSetAllocateInfo { - layout: &self.layout, variable_descriptor_count, + ..DescriptorSetAllocateInfo::new(&self.layout) }; let mut sets = unsafe { @@ -363,19 +343,22 @@ impl VariableEntry { .inner .allocate_descriptor_sets([allocate_info]) .map_err(|err| match err { - VulkanError::OutOfHostMemory | VulkanError::OutOfDeviceMemory => err, - VulkanError::FragmentedPool => { - // This can't happen as we don't free individual sets. - unreachable!(); - } - VulkanError::OutOfPoolMemory => { - // We created the pool to fit the maximum variable descriptor count. - unreachable!(); - } - _ => { - // Shouldn't ever be returned. - unreachable!(); - } + Validated::ValidationError(_) => err, + Validated::Error(vk_err) => match vk_err { + VulkanError::OutOfHostMemory | VulkanError::OutOfDeviceMemory => err, + VulkanError::FragmentedPool => { + // This can't happen as we don't free individual sets. + unreachable!(); + } + VulkanError::OutOfPoolMemory => { + // We created the pool to fit the maximum variable descriptor count. + unreachable!(); + } + _ => { + // Shouldn't ever be returned. + unreachable!(); + } + }, })? }; self.allocations += 1; @@ -457,6 +440,16 @@ enum AllocParent { Variable(Arc), } +impl AllocParent { + #[inline] + fn pool(&self) -> &DescriptorPool { + match self { + Self::Fixed(pool) => &pool.inner, + Self::Variable(pool) => &pool.inner, + } + } +} + // This is needed because of the blanket impl of `Send` on `Arc`, which requires that `T` is // `Send + Sync`. `FixedPool` and `VariablePool` are `Send + !Sync` because `DescriptorPool` is // `!Sync`. That's fine however because we never access the `DescriptorPool` concurrently. @@ -473,6 +466,11 @@ impl DescriptorSetAlloc for StandardDescriptorSetAlloc { fn inner_mut(&mut self) -> &mut UnsafeDescriptorSet { &mut self.inner } + + #[inline] + fn pool(&self) -> &DescriptorPool { + self.parent.pool() + } } impl Drop for StandardDescriptorSetAlloc { @@ -567,7 +565,7 @@ mod tests { let pool1 = if let AllocParent::Fixed(pool) = &allocator.allocate(&layout, 0).unwrap().parent { - pool._inner.handle() + pool.inner.handle() } else { unreachable!() }; @@ -575,7 +573,7 @@ mod tests { thread::spawn(move || { let pool2 = if let AllocParent::Fixed(pool) = &allocator.allocate(&layout, 0).unwrap().parent { - pool._inner.handle() + pool.inner.handle() } else { unreachable!() }; diff --git a/vulkano/src/descriptor_set/pool.rs b/vulkano/src/descriptor_set/pool.rs index 422f3636e2..cc3edaabdc 100644 --- a/vulkano/src/descriptor_set/pool.rs +++ b/vulkano/src/descriptor_set/pool.rs @@ -190,33 +190,50 @@ impl DescriptorPool { /// The `FragmentedPool` errors often can't be prevented. If the function returns this error, /// you should just create a new pool. /// - /// # Panics - /// - /// - Panics if one of the layouts wasn't created with the same device as the pool. - /// /// # Safety /// /// - When the pool is dropped, the returned descriptor sets must not be in use by either /// the host or device. + /// - If the device API version is less than 1.1, and the [`khr_maintenance1`] extension is not + /// enabled on the device, then + /// the length of `allocate_infos` must not be greater than the number of descriptor sets + /// remaining in the pool, and + /// the total number of descriptors of each type being allocated must not be greater than the + /// number of descriptors of that type remaining in the pool. + /// + /// [`khr_maintenance1`]: crate::device::DeviceExtensions::khr_maintenance1 + #[inline] pub unsafe fn allocate_descriptor_sets<'a>( &self, - allocate_info: impl IntoIterator>, - ) -> Result, VulkanError> { + allocate_infos: impl IntoIterator>, + ) -> Result> { + let allocate_infos: SmallVec<[_; 1]> = allocate_infos.into_iter().collect(); + self.validate_allocate_descriptor_sets(&allocate_infos)?; + + Ok(self.allocate_descriptor_sets_unchecked(allocate_infos)?) + } + + fn validate_allocate_descriptor_sets( + &self, + allocate_infos: &[DescriptorSetAllocateInfo<'_>], + ) -> Result<(), Box> { + for (index, info) in allocate_infos.iter().enumerate() { + info.validate(self.device()) + .map_err(|err| err.add_context(format!("allocate_infos[{}]", index)))?; + } + + Ok(()) + } + + #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] + pub unsafe fn allocate_descriptor_sets_unchecked<'a>( + &self, + allocate_infos: impl IntoIterator>, + ) -> Result { let (layouts, variable_descriptor_counts): (SmallVec<[_; 1]>, SmallVec<[_; 1]>) = - allocate_info + allocate_infos .into_iter() - .map(|info| { - assert_eq!(self.device.handle(), info.layout.device().handle(),); - debug_assert!(!info - .layout - .flags() - .intersects(DescriptorSetLayoutCreateFlags::PUSH_DESCRIPTOR)); - debug_assert!( - info.variable_descriptor_count <= info.layout.variable_descriptor_count() - ); - - (info.layout.handle(), info.variable_descriptor_count) - }) + .map(|info| (info.layout.handle(), info.variable_descriptor_count)) .unzip(); let output = if layouts.is_empty() { @@ -267,7 +284,9 @@ impl DescriptorPool { output }; - Ok(output.into_iter().map(UnsafeDescriptorSet::new)) + Ok(DescriptorSetIter( + output.into_iter().map(UnsafeDescriptorSet::new), + )) } /// Frees some descriptor sets. @@ -516,29 +535,108 @@ vulkan_bitflags! { /* TODO: enable // TODO: document - UPDATE_AFTER_BIND = UPDATE_AFTER_BIND { - api_version: V1_2, - device_extensions: [ext_descriptor_indexing], - }, */ + UPDATE_AFTER_BIND = UPDATE_AFTER_BIND + RequiresOneOf([ + RequiresAllOf([APIVersion(V1_2)]), + RequiresAllOf([DeviceExtension(ext_descriptor_indexing)]), + ]), */ /* TODO: enable // TODO: document - HOST_ONLY = HOST_ONLY_EXT { - device_extensions: [ext_mutable_descriptor_type, valve_mutable_descriptor_type], - }, */ + HOST_ONLY = HOST_ONLY_EXT + RequiresOneOf([ + RequiresAllOf([DeviceExtension(ext_mutable_descriptor_type)]), + RequiresAllOf([DeviceExtension(valve_mutable_descriptor_type)]), + ]), */ } /// Parameters to allocate a new `UnsafeDescriptorSet` from an `UnsafeDescriptorPool`. #[derive(Clone, Debug)] pub struct DescriptorSetAllocateInfo<'a> { /// The descriptor set layout to create the set for. + /// + /// There is no default value. pub layout: &'a DescriptorSetLayout, /// For layouts with a variable-count binding, the number of descriptors to allocate for that /// binding. This should be 0 for layouts that don't have a variable-count binding. + /// + /// The default value is 0. pub variable_descriptor_count: u32, + + pub _ne: crate::NonExhaustive, +} + +impl<'a> DescriptorSetAllocateInfo<'a> { + /// Returns a `DescriptorSetAllocateInfo` with the specified `layout`. + #[inline] + pub fn new(layout: &'a DescriptorSetLayout) -> Self { + Self { + layout, + variable_descriptor_count: 0, + _ne: crate::NonExhaustive(()), + } + } + + pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { + let &Self { + layout, + variable_descriptor_count, + _ne, + } = self; + + // VUID-VkDescriptorSetAllocateInfo-commonparent + assert_eq!(device, layout.device().as_ref()); + + if layout + .flags() + .intersects(DescriptorSetLayoutCreateFlags::PUSH_DESCRIPTOR) + { + return Err(Box::new(ValidationError { + context: "layout.flags()".into(), + problem: "contains `DescriptorSetLayoutCreateFlags::PUSH_DESCRIPTOR`".into(), + vuids: &["VUID-VkDescriptorSetAllocateInfo-pSetLayouts-00308"], + ..Default::default() + })); + } + + if variable_descriptor_count >= layout.variable_descriptor_count() { + return Err(Box::new(ValidationError { + problem: "`variable_descriptor_count` is greater than + `layout.variable_descriptor_count()`" + .into(), + // vuids: https://github.com/KhronosGroup/Vulkan-Docs/issues/2244 + ..Default::default() + })); + } + + Ok(()) + } +} + +// TODO: Exists only due to an issue with returning `impl Trait` when the input has a lifetime: +// https://github.com/rust-lang/rust/issues/42940 +pub struct DescriptorSetIter( + std::iter::Map< + std::vec::IntoIter, + fn(ash::vk::DescriptorSet) -> UnsafeDescriptorSet, + >, +); + +impl Iterator for DescriptorSetIter { + type Item = UnsafeDescriptorSet; + + fn next(&mut self) -> Option { + self.0.next() + } + + fn size_hint(&self) -> (usize, Option) { + self.0.size_hint() + } } +impl ExactSizeIterator for DescriptorSetIter {} + #[cfg(test)] mod tests { use super::{DescriptorPool, DescriptorPoolCreateInfo}; @@ -628,10 +726,7 @@ mod tests { .unwrap(); unsafe { let sets = pool - .allocate_descriptor_sets([DescriptorSetAllocateInfo { - layout: set_layout.as_ref(), - variable_descriptor_count: 0, - }]) + .allocate_descriptor_sets([DescriptorSetAllocateInfo::new(set_layout.as_ref())]) .unwrap(); assert_eq!(sets.count(), 1); } @@ -670,10 +765,9 @@ mod tests { .unwrap(); unsafe { - let _ = pool.allocate_descriptor_sets([DescriptorSetAllocateInfo { - layout: set_layout.as_ref(), - variable_descriptor_count: 0, - }]); + let _ = pool.allocate_descriptor_sets([DescriptorSetAllocateInfo::new( + set_layout.as_ref(), + )]); } }); } From b105d3ae1d4cb47aaa8b8af7d8698f83cef5b047 Mon Sep 17 00:00:00 2001 From: Rua Date: Tue, 10 Oct 2023 18:18:07 +0200 Subject: [PATCH 3/4] Add `DescriptorPoolAlloc`, make `UnsafeDescriptorSet` more useful --- .../src/command_buffer/commands/bind_push.rs | 2 +- vulkano/src/descriptor_set/allocator.rs | 27 +-- vulkano/src/descriptor_set/mod.rs | 181 ++--------------- vulkano/src/descriptor_set/persistent.rs | 106 +++++----- vulkano/src/descriptor_set/pool.rs | 173 ++++++++++------ vulkano/src/descriptor_set/sys.rs | 188 +++++++++++++----- 6 files changed, 329 insertions(+), 348 deletions(-) diff --git a/vulkano/src/command_buffer/commands/bind_push.rs b/vulkano/src/command_buffer/commands/bind_push.rs index cdcda8c430..4ca61159e3 100644 --- a/vulkano/src/command_buffer/commands/bind_push.rs +++ b/vulkano/src/command_buffer/commands/bind_push.rs @@ -694,7 +694,7 @@ where let descriptor_sets_vk: SmallVec<[_; 12]> = descriptor_sets .iter() - .map(|x| x.as_ref().0.inner().handle()) + .map(|x| x.as_ref().0.handle()) .collect(); let dynamic_offsets_vk: SmallVec<[_; 32]> = descriptor_sets .iter() diff --git a/vulkano/src/descriptor_set/allocator.rs b/vulkano/src/descriptor_set/allocator.rs index c2cef89f3a..bc1dd4bb86 100644 --- a/vulkano/src/descriptor_set/allocator.rs +++ b/vulkano/src/descriptor_set/allocator.rs @@ -19,8 +19,9 @@ use self::sorted_map::SortedMap; use super::{ layout::DescriptorSetLayout, - pool::{DescriptorPool, DescriptorPoolCreateInfo, DescriptorSetAllocateInfo}, - sys::UnsafeDescriptorSet, + pool::{ + DescriptorPool, DescriptorPoolAlloc, DescriptorPoolCreateInfo, DescriptorSetAllocateInfo, + }, }; use crate::{ descriptor_set::layout::DescriptorType, @@ -65,11 +66,8 @@ pub unsafe trait DescriptorSetAllocator: DeviceOwned { /// An allocated descriptor set. pub trait DescriptorSetAlloc: Send + Sync { - /// Returns the inner unsafe descriptor set object. - fn inner(&self) -> &UnsafeDescriptorSet; - - /// Returns the inner unsafe descriptor set object. - fn inner_mut(&mut self) -> &mut UnsafeDescriptorSet; + /// Returns the internal object that contains the descriptor set. + fn inner(&self) -> &DescriptorPoolAlloc; /// Returns the descriptor pool that the descriptor set was allocated from. fn pool(&self) -> &DescriptorPool; @@ -233,7 +231,7 @@ struct FixedPool { inner: DescriptorPool, // List of descriptor sets. When `alloc` is called, a descriptor will be extracted from this // list. When a `SingleLayoutPoolAlloc` is dropped, its descriptor set is put back in this list. - reserve: ArrayQueue, + reserve: ArrayQueue, } impl FixedPool { @@ -258,7 +256,7 @@ impl FixedPool { ) .map_err(Validated::unwrap)?; - let allocate_infos = (0..set_count).map(|_| DescriptorSetAllocateInfo::new(layout)); + let allocate_infos = (0..set_count).map(|_| DescriptorSetAllocateInfo::new(layout.clone())); let allocs = unsafe { inner @@ -335,7 +333,7 @@ impl VariableEntry { let allocate_info = DescriptorSetAllocateInfo { variable_descriptor_count, - ..DescriptorSetAllocateInfo::new(&self.layout) + ..DescriptorSetAllocateInfo::new(self.layout.clone()) }; let mut sets = unsafe { @@ -429,7 +427,7 @@ impl Drop for VariablePool { #[derive(Debug)] pub struct StandardDescriptorSetAlloc { // The actual descriptor set. - inner: ManuallyDrop, + inner: ManuallyDrop, // The pool where we allocated from. Needed for our `Drop` impl. parent: AllocParent, } @@ -458,15 +456,10 @@ unsafe impl Sync for StandardDescriptorSetAlloc {} impl DescriptorSetAlloc for StandardDescriptorSetAlloc { #[inline] - fn inner(&self) -> &UnsafeDescriptorSet { + fn inner(&self) -> &DescriptorPoolAlloc { &self.inner } - #[inline] - fn inner_mut(&mut self) -> &mut UnsafeDescriptorSet { - &mut self.inner - } - #[inline] fn pool(&self) -> &DescriptorPool { self.parent.pool() diff --git a/vulkano/src/descriptor_set/mod.rs b/vulkano/src/descriptor_set/mod.rs index a99977b4e3..4f13c17229 100644 --- a/vulkano/src/descriptor_set/mod.rs +++ b/vulkano/src/descriptor_set/mod.rs @@ -87,16 +87,16 @@ pub use self::{ WriteDescriptorSetElements, }, }; -use self::{layout::DescriptorSetLayout, sys::UnsafeDescriptorSet}; +use self::{layout::DescriptorSetLayout, pool::DescriptorPoolAlloc}; use crate::{ acceleration_structure::AccelerationStructure, buffer::view::BufferView, descriptor_set::layout::{ DescriptorBindingFlags, DescriptorSetLayoutCreateFlags, DescriptorType, }, - device::{DeviceOwned, DeviceOwnedDebugWrapper}, + device::DeviceOwned, image::{sampler::Sampler, ImageLayout}, - ValidationError, VulkanObject, + VulkanObject, }; use ahash::HashMap; use smallvec::{smallvec, SmallVec}; @@ -116,15 +116,23 @@ mod update; /// Trait for objects that contain a collection of resources that will be accessible by shaders. /// /// Objects of this type can be passed when submitting a draw command. -pub unsafe trait DescriptorSet: DeviceOwned + Send + Sync { - /// Returns the inner `UnsafeDescriptorSet`. - fn inner(&self) -> &UnsafeDescriptorSet; +pub unsafe trait DescriptorSet: + VulkanObject + DeviceOwned + Send + Sync +{ + /// Returns the allocation of the descriptor set. + fn alloc(&self) -> &DescriptorPoolAlloc; /// Returns the layout of this descriptor set. - fn layout(&self) -> &Arc; + #[inline] + fn layout(&self) -> &Arc { + self.alloc().layout() + } /// Returns the variable descriptor count that this descriptor set was allocated with. - fn variable_descriptor_count(&self) -> u32; + #[inline] + fn variable_descriptor_count(&self) -> u32 { + self.alloc().variable_descriptor_count() + } /// Creates a [`DescriptorSetWithOffsets`] with the given dynamic offsets. fn offsets( @@ -144,7 +152,7 @@ pub unsafe trait DescriptorSet: DeviceOwned + Send + Sync { impl PartialEq for dyn DescriptorSet { #[inline] fn eq(&self, other: &Self) -> bool { - self.inner() == other.inner() + self.alloc() == other.alloc() } } @@ -152,160 +160,7 @@ impl Eq for dyn DescriptorSet {} impl Hash for dyn DescriptorSet { fn hash(&self, state: &mut H) { - self.inner().hash(state); - } -} - -pub(crate) struct DescriptorSetInner { - layout: DeviceOwnedDebugWrapper>, - variable_descriptor_count: u32, - resources: DescriptorSetResources, -} - -impl DescriptorSetInner { - pub(crate) fn new( - handle: ash::vk::DescriptorSet, - layout: Arc, - variable_descriptor_count: u32, - descriptor_writes: impl IntoIterator, - descriptor_copies: impl IntoIterator, - ) -> Result> { - assert!( - !layout - .flags() - .intersects(DescriptorSetLayoutCreateFlags::PUSH_DESCRIPTOR), - "the provided descriptor set layout is for push descriptors, and cannot be used to \ - build a descriptor set object", - ); - - let max_variable_descriptor_count = layout.variable_descriptor_count(); - - assert!( - variable_descriptor_count <= max_variable_descriptor_count, - "the provided variable_descriptor_count ({}) is greater than the maximum number of \ - variable count descriptors in the layout ({})", - variable_descriptor_count, - max_variable_descriptor_count, - ); - - let mut resources = DescriptorSetResources::new(&layout, variable_descriptor_count); - - struct PerDescriptorWrite { - write_info: DescriptorWriteInfo, - acceleration_structures: ash::vk::WriteDescriptorSetAccelerationStructureKHR, - inline_uniform_block: ash::vk::WriteDescriptorSetInlineUniformBlock, - } - - let writes_iter = descriptor_writes.into_iter(); - let (lower_size_bound, _) = writes_iter.size_hint(); - let mut writes_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(lower_size_bound); - let mut per_writes_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(lower_size_bound); - - for (index, write) in writes_iter.enumerate() { - write - .validate(&layout, variable_descriptor_count) - .map_err(|err| err.add_context(format!("descriptor_writes[{}]", index)))?; - resources.write(&write, &layout); - - let layout_binding = &layout.bindings()[&write.binding()]; - writes_vk.push(write.to_vulkan(handle, layout_binding.descriptor_type)); - per_writes_vk.push(PerDescriptorWrite { - write_info: write.to_vulkan_info(layout_binding.descriptor_type), - acceleration_structures: Default::default(), - inline_uniform_block: Default::default(), - }); - } - - if !writes_vk.is_empty() { - for (write_vk, per_write_vk) in writes_vk.iter_mut().zip(per_writes_vk.iter_mut()) { - match &mut per_write_vk.write_info { - DescriptorWriteInfo::Image(info) => { - write_vk.descriptor_count = info.len() as u32; - write_vk.p_image_info = info.as_ptr(); - } - DescriptorWriteInfo::Buffer(info) => { - write_vk.descriptor_count = info.len() as u32; - write_vk.p_buffer_info = info.as_ptr(); - } - DescriptorWriteInfo::BufferView(info) => { - write_vk.descriptor_count = info.len() as u32; - write_vk.p_texel_buffer_view = info.as_ptr(); - } - DescriptorWriteInfo::InlineUniformBlock(data) => { - write_vk.descriptor_count = data.len() as u32; - write_vk.p_next = &per_write_vk.inline_uniform_block as *const _ as _; - per_write_vk.inline_uniform_block.data_size = write_vk.descriptor_count; - per_write_vk.inline_uniform_block.p_data = data.as_ptr() as *const _; - } - DescriptorWriteInfo::AccelerationStructure(info) => { - write_vk.descriptor_count = info.len() as u32; - write_vk.p_next = &per_write_vk.acceleration_structures as *const _ as _; - per_write_vk - .acceleration_structures - .acceleration_structure_count = write_vk.descriptor_count; - per_write_vk - .acceleration_structures - .p_acceleration_structures = info.as_ptr(); - } - } - } - } - - let copies_iter = descriptor_copies.into_iter(); - let (lower_size_bound, _) = copies_iter.size_hint(); - let mut copies_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(lower_size_bound); - - for (index, copy) in copies_iter.enumerate() { - copy.validate(&layout, variable_descriptor_count) - .map_err(|err| err.add_context(format!("descriptor_copies[{}]", index)))?; - resources.copy(©); - - let &CopyDescriptorSet { - ref src_set, - src_binding, - src_first_array_element, - dst_binding, - dst_first_array_element, - descriptor_count, - _ne: _, - } = © - - copies_vk.push(ash::vk::CopyDescriptorSet { - src_set: src_set.inner().handle(), - src_binding, - src_array_element: src_first_array_element, - dst_set: handle, - dst_binding, - dst_array_element: dst_first_array_element, - descriptor_count, - ..Default::default() - }); - } - - unsafe { - let fns = layout.device().fns(); - (fns.v1_0.update_descriptor_sets)( - layout.device().handle(), - writes_vk.len() as u32, - writes_vk.as_ptr(), - copies_vk.len() as u32, - copies_vk.as_ptr(), - ); - } - - Ok(DescriptorSetInner { - layout: DeviceOwnedDebugWrapper(layout), - variable_descriptor_count, - resources, - }) - } - - pub(crate) fn layout(&self) -> &Arc { - &self.layout - } - - pub(crate) fn resources(&self) -> &DescriptorSetResources { - &self.resources + self.alloc().hash(state); } } diff --git a/vulkano/src/descriptor_set/persistent.rs b/vulkano/src/descriptor_set/persistent.rs index 3444a8ff63..23bd6b72dc 100644 --- a/vulkano/src/descriptor_set/persistent.rs +++ b/vulkano/src/descriptor_set/persistent.rs @@ -21,18 +21,17 @@ //! # Examples //! TODO: -use super::CopyDescriptorSet; +use super::{pool::DescriptorPoolAlloc, sys::UnsafeDescriptorSet, CopyDescriptorSet}; use crate::{ descriptor_set::{ allocator::{DescriptorSetAlloc, DescriptorSetAllocator, StandardDescriptorSetAlloc}, - layout::DescriptorSetLayoutCreateFlags, update::WriteDescriptorSet, - DescriptorSet, DescriptorSetInner, DescriptorSetLayout, DescriptorSetResources, - UnsafeDescriptorSet, + DescriptorSet, DescriptorSetLayout, DescriptorSetResources, }, device::{Device, DeviceOwned}, - Validated, VulkanError, VulkanObject, + Validated, ValidationError, VulkanError, VulkanObject, }; +use smallvec::SmallVec; use std::{ hash::{Hash, Hasher}, sync::Arc, @@ -40,8 +39,8 @@ use std::{ /// A simple, immutable descriptor set that is expected to be long-lived. pub struct PersistentDescriptorSet

{ - alloc: P, - inner: DescriptorSetInner, + inner: UnsafeDescriptorSet

, + resources: DescriptorSetResources, } impl PersistentDescriptorSet { @@ -78,55 +77,68 @@ impl PersistentDescriptorSet { where A: DescriptorSetAllocator + ?Sized, { - assert!( - !layout - .flags() - .intersects(DescriptorSetLayoutCreateFlags::PUSH_DESCRIPTOR), - "the provided descriptor set layout is for push descriptors, and cannot be used to \ - build a descriptor set object", - ); - - let max_count = layout.variable_descriptor_count(); - - assert!( - variable_descriptor_count <= max_count, - "the provided variable_descriptor_count ({}) is greater than the maximum number of \ - variable count descriptors in the set ({})", - variable_descriptor_count, - max_count, - ); - - let alloc = allocator.allocate(&layout, variable_descriptor_count)?; - let inner = DescriptorSetInner::new( - alloc.inner().handle(), - layout, - variable_descriptor_count, - descriptor_writes, - descriptor_copies, - )?; - - Ok(Arc::new(PersistentDescriptorSet { alloc, inner })) + let mut set = PersistentDescriptorSet { + inner: UnsafeDescriptorSet::new(allocator, &layout, variable_descriptor_count)?, + resources: DescriptorSetResources::new(&layout, variable_descriptor_count), + }; + + unsafe { + set.update(descriptor_writes, descriptor_copies)?; + } + + Ok(Arc::new(set)) } } -unsafe impl

DescriptorSet for PersistentDescriptorSet

+impl

PersistentDescriptorSet

where P: DescriptorSetAlloc, { - fn inner(&self) -> &UnsafeDescriptorSet { - self.alloc.inner() - } + unsafe fn update( + &mut self, + descriptor_writes: impl IntoIterator, + descriptor_copies: impl IntoIterator, + ) -> Result<(), Box> { + let descriptor_writes: SmallVec<[_; 8]> = descriptor_writes.into_iter().collect(); + let descriptor_copies: SmallVec<[_; 8]> = descriptor_copies.into_iter().collect(); - fn layout(&self) -> &Arc { - self.inner.layout() + unsafe { + self.inner.update(&descriptor_writes, &descriptor_copies)?; + } + + for write in descriptor_writes { + self.resources.write(&write, self.inner.layout()); + } + + for copy in descriptor_copies { + self.resources.copy(©); + } + + Ok(()) } +} - fn variable_descriptor_count(&self) -> u32 { - self.inner.variable_descriptor_count +unsafe impl

DescriptorSet for PersistentDescriptorSet

+where + P: DescriptorSetAlloc, +{ + fn alloc(&self) -> &DescriptorPoolAlloc { + self.inner.alloc().inner() } fn resources(&self) -> &DescriptorSetResources { - self.inner.resources() + &self.resources + } +} + +unsafe impl

VulkanObject for PersistentDescriptorSet

+where + P: DescriptorSetAlloc, +{ + type Handle = ash::vk::DescriptorSet; + + fn handle(&self) -> Self::Handle { + self.inner.handle() } } @@ -135,7 +147,7 @@ where P: DescriptorSetAlloc, { fn device(&self) -> &Arc { - self.inner.layout().device() + self.layout().device() } } @@ -144,7 +156,7 @@ where P: DescriptorSetAlloc, { fn eq(&self, other: &Self) -> bool { - self.inner() == other.inner() + self.inner == other.inner } } @@ -155,6 +167,6 @@ where P: DescriptorSetAlloc, { fn hash(&self, state: &mut H) { - self.inner().hash(state); + self.inner.hash(state); } } diff --git a/vulkano/src/descriptor_set/pool.rs b/vulkano/src/descriptor_set/pool.rs index cc3edaabdc..708d755418 100644 --- a/vulkano/src/descriptor_set/pool.rs +++ b/vulkano/src/descriptor_set/pool.rs @@ -12,7 +12,7 @@ use crate::{ layout::{DescriptorSetLayout, DescriptorSetLayoutCreateFlags, DescriptorType}, sys::UnsafeDescriptorSet, }, - device::{Device, DeviceOwned}, + device::{Device, DeviceOwned, DeviceOwnedDebugWrapper}, instance::InstanceOwnedDebugWrapper, macros::{impl_id_counter, vulkan_bitflags}, Requires, RequiresAllOf, RequiresOneOf, Validated, ValidationError, Version, VulkanError, @@ -203,10 +203,10 @@ impl DescriptorPool { /// /// [`khr_maintenance1`]: crate::device::DeviceExtensions::khr_maintenance1 #[inline] - pub unsafe fn allocate_descriptor_sets<'a>( + pub unsafe fn allocate_descriptor_sets( &self, - allocate_infos: impl IntoIterator>, - ) -> Result> { + allocate_infos: impl IntoIterator, + ) -> Result, Validated> { let allocate_infos: SmallVec<[_; 1]> = allocate_infos.into_iter().collect(); self.validate_allocate_descriptor_sets(&allocate_infos)?; @@ -215,7 +215,7 @@ impl DescriptorPool { fn validate_allocate_descriptor_sets( &self, - allocate_infos: &[DescriptorSetAllocateInfo<'_>], + allocate_infos: &[DescriptorSetAllocateInfo], ) -> Result<(), Box> { for (index, info) in allocate_infos.iter().enumerate() { info.validate(self.device()) @@ -226,25 +226,38 @@ impl DescriptorPool { } #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] - pub unsafe fn allocate_descriptor_sets_unchecked<'a>( + pub unsafe fn allocate_descriptor_sets_unchecked( &self, - allocate_infos: impl IntoIterator>, - ) -> Result { - let (layouts, variable_descriptor_counts): (SmallVec<[_; 1]>, SmallVec<[_; 1]>) = - allocate_infos - .into_iter() - .map(|info| (info.layout.handle(), info.variable_descriptor_count)) - .unzip(); - - let output = if layouts.is_empty() { - vec![] - } else { + allocate_infos: impl IntoIterator, + ) -> Result, VulkanError> { + let allocate_infos = allocate_infos.into_iter(); + + let (lower_size_bound, _) = allocate_infos.size_hint(); + let mut layouts_vk = Vec::with_capacity(lower_size_bound); + let mut variable_descriptor_counts = Vec::with_capacity(lower_size_bound); + let mut layouts = Vec::with_capacity(lower_size_bound); + + for info in allocate_infos { + let DescriptorSetAllocateInfo { + layout, + variable_descriptor_count, + _ne: _, + } = info; + + layouts_vk.push(layout.handle()); + variable_descriptor_counts.push(variable_descriptor_count); + layouts.push(layout); + } + + let mut output = vec![]; + + if !layouts_vk.is_empty() { let variable_desc_count_alloc_info = if (self.device.api_version() >= Version::V1_2 || self.device.enabled_extensions().ext_descriptor_indexing) && variable_descriptor_counts.iter().any(|c| *c != 0) { Some(ash::vk::DescriptorSetVariableDescriptorCountAllocateInfo { - descriptor_set_count: layouts.len() as u32, + descriptor_set_count: layouts_vk.len() as u32, p_descriptor_counts: variable_descriptor_counts.as_ptr(), ..Default::default() }) @@ -252,10 +265,10 @@ impl DescriptorPool { None }; - let infos = ash::vk::DescriptorSetAllocateInfo { + let info_vk = ash::vk::DescriptorSetAllocateInfo { descriptor_pool: self.handle, - descriptor_set_count: layouts.len() as u32, - p_set_layouts: layouts.as_ptr(), + descriptor_set_count: layouts_vk.len() as u32, + p_set_layouts: layouts_vk.as_ptr(), p_next: if let Some(next) = variable_desc_count_alloc_info.as_ref() { next as *const _ as *const _ } else { @@ -264,29 +277,41 @@ impl DescriptorPool { ..Default::default() }; - let mut output = Vec::with_capacity(layouts.len()); + output.reserve(layouts_vk.len()); let fns = self.device.fns(); - (fns.v1_0.allocate_descriptor_sets)(self.device.handle(), &infos, output.as_mut_ptr()) - .result() - .map_err(VulkanError::from) - .map_err(|err| match err { - VulkanError::OutOfHostMemory - | VulkanError::OutOfDeviceMemory - | VulkanError::OutOfPoolMemory => err, - // According to the specs, because `VK_ERROR_FRAGMENTED_POOL` was added after - // version 1.0 of Vulkan, any negative return value except out-of-memory errors - // must be considered as a fragmented pool error. - _ => VulkanError::FragmentedPool, - })?; - - output.set_len(layouts.len()); - output - }; + (fns.v1_0.allocate_descriptor_sets)( + self.device.handle(), + &info_vk, + output.as_mut_ptr(), + ) + .result() + .map_err(VulkanError::from) + .map_err(|err| match err { + VulkanError::OutOfHostMemory + | VulkanError::OutOfDeviceMemory + | VulkanError::OutOfPoolMemory => err, + // According to the specs, because `VK_ERROR_FRAGMENTED_POOL` was added after + // version 1.0 of Vulkan, any negative return value except out-of-memory errors + // must be considered as a fragmented pool error. + _ => VulkanError::FragmentedPool, + })?; + + output.set_len(layouts_vk.len()); + } - Ok(DescriptorSetIter( - output.into_iter().map(UnsafeDescriptorSet::new), - )) + Ok(output + .into_iter() + .zip(layouts) + .zip(variable_descriptor_counts) + .map( + move |((handle, layout), variable_descriptor_count)| DescriptorPoolAlloc { + handle, + id: DescriptorPoolAlloc::next_id(), + layout: DeviceOwnedDebugWrapper(layout), + variable_descriptor_count, + }, + )) } /// Frees some descriptor sets. @@ -552,11 +577,11 @@ vulkan_bitflags! { /// Parameters to allocate a new `UnsafeDescriptorSet` from an `UnsafeDescriptorPool`. #[derive(Clone, Debug)] -pub struct DescriptorSetAllocateInfo<'a> { +pub struct DescriptorSetAllocateInfo { /// The descriptor set layout to create the set for. /// /// There is no default value. - pub layout: &'a DescriptorSetLayout, + pub layout: Arc, /// For layouts with a variable-count binding, the number of descriptors to allocate for that /// binding. This should be 0 for layouts that don't have a variable-count binding. @@ -567,10 +592,10 @@ pub struct DescriptorSetAllocateInfo<'a> { pub _ne: crate::NonExhaustive, } -impl<'a> DescriptorSetAllocateInfo<'a> { +impl DescriptorSetAllocateInfo { /// Returns a `DescriptorSetAllocateInfo` with the specified `layout`. #[inline] - pub fn new(layout: &'a DescriptorSetLayout) -> Self { + pub fn new(layout: Arc) -> Self { Self { layout, variable_descriptor_count: 0, @@ -580,7 +605,7 @@ impl<'a> DescriptorSetAllocateInfo<'a> { pub(crate) fn validate(&self, device: &Device) -> Result<(), Box> { let &Self { - layout, + ref layout, variable_descriptor_count, _ne, } = self; @@ -600,7 +625,7 @@ impl<'a> DescriptorSetAllocateInfo<'a> { })); } - if variable_descriptor_count >= layout.variable_descriptor_count() { + if variable_descriptor_count > layout.variable_descriptor_count() { return Err(Box::new(ValidationError { problem: "`variable_descriptor_count` is greater than `layout.variable_descriptor_count()`" @@ -614,28 +639,46 @@ impl<'a> DescriptorSetAllocateInfo<'a> { } } -// TODO: Exists only due to an issue with returning `impl Trait` when the input has a lifetime: -// https://github.com/rust-lang/rust/issues/42940 -pub struct DescriptorSetIter( - std::iter::Map< - std::vec::IntoIter, - fn(ash::vk::DescriptorSet) -> UnsafeDescriptorSet, - >, -); +/// Opaque type that represents a descriptor set allocated from a pool. +#[derive(Debug)] +pub struct DescriptorPoolAlloc { + handle: ash::vk::DescriptorSet, + id: NonZeroU64, + layout: DeviceOwnedDebugWrapper>, + variable_descriptor_count: u32, +} + +impl DescriptorPoolAlloc { + /// Returns the descriptor set layout of the descriptor set. + #[inline] + pub fn layout(&self) -> &Arc { + &self.layout + } + + /// Returns the variable descriptor count that this descriptor set was allocated with. + #[inline] + pub fn variable_descriptor_count(&self) -> u32 { + self.variable_descriptor_count + } +} -impl Iterator for DescriptorSetIter { - type Item = UnsafeDescriptorSet; +unsafe impl VulkanObject for DescriptorPoolAlloc { + type Handle = ash::vk::DescriptorSet; - fn next(&mut self) -> Option { - self.0.next() + #[inline] + fn handle(&self) -> Self::Handle { + self.handle } +} - fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() +unsafe impl DeviceOwned for DescriptorPoolAlloc { + #[inline] + fn device(&self) -> &Arc { + self.layout.device() } } -impl ExactSizeIterator for DescriptorSetIter {} +impl_id_counter!(DescriptorPoolAlloc); #[cfg(test)] mod tests { @@ -726,7 +769,7 @@ mod tests { .unwrap(); unsafe { let sets = pool - .allocate_descriptor_sets([DescriptorSetAllocateInfo::new(set_layout.as_ref())]) + .allocate_descriptor_sets([DescriptorSetAllocateInfo::new(set_layout)]) .unwrap(); assert_eq!(sets.count(), 1); } @@ -765,9 +808,7 @@ mod tests { .unwrap(); unsafe { - let _ = pool.allocate_descriptor_sets([DescriptorSetAllocateInfo::new( - set_layout.as_ref(), - )]); + let _ = pool.allocate_descriptor_sets([DescriptorSetAllocateInfo::new(set_layout)]); } }); } diff --git a/vulkano/src/descriptor_set/sys.rs b/vulkano/src/descriptor_set/sys.rs index ec36324599..16985f45a0 100644 --- a/vulkano/src/descriptor_set/sys.rs +++ b/vulkano/src/descriptor_set/sys.rs @@ -9,56 +9,113 @@ //! Low-level descriptor set. -use super::CopyDescriptorSet; +use super::{ + allocator::{DescriptorSetAlloc, DescriptorSetAllocator, StandardDescriptorSetAlloc}, + CopyDescriptorSet, +}; use crate::{ descriptor_set::{ layout::DescriptorSetLayout, update::{DescriptorWriteInfo, WriteDescriptorSet}, }, - device::DeviceOwned, - macros::impl_id_counter, - VulkanObject, + device::{Device, DeviceOwned}, + Validated, ValidationError, VulkanError, VulkanObject, }; use smallvec::SmallVec; -use std::{fmt::Debug, num::NonZeroU64}; +use std::{fmt::Debug, hash::Hash, sync::Arc}; /// Low-level descriptor set. /// -/// Contrary to most other objects in this library, this one doesn't free itself automatically and -/// doesn't hold the pool or the device it is associated to. -/// Instead it is an object meant to be used with the [`DescriptorPool`]. -/// -/// [`DescriptorPool`]: super::pool::DescriptorPool +/// This descriptor set does not keep track of synchronization, +/// nor does it store any information on what resources have been written to each descriptor. #[derive(Debug)] -pub struct UnsafeDescriptorSet { - handle: ash::vk::DescriptorSet, - id: NonZeroU64, +pub struct UnsafeDescriptorSet

{ + alloc: P, } impl UnsafeDescriptorSet { - pub(crate) fn new(handle: ash::vk::DescriptorSet) -> Self { - Self { - handle, - id: Self::next_id(), - } + /// Allocates a new descriptor set and returns it. + #[inline] + pub fn new( + allocator: &A, + layout: &Arc, + variable_descriptor_count: u32, + ) -> Result, Validated> + where + A: DescriptorSetAllocator + ?Sized, + { + Ok(UnsafeDescriptorSet { + alloc: allocator.allocate(layout, variable_descriptor_count)?, + }) + } +} + +impl

UnsafeDescriptorSet

+where + P: DescriptorSetAlloc, +{ + /// Returns the allocation of this descriptor set. + #[inline] + pub fn alloc(&self) -> &P { + &self.alloc + } + + /// Returns the layout of this descriptor set. + #[inline] + pub fn layout(&self) -> &Arc { + self.alloc.inner().layout() } - /// Modifies a descriptor set. Doesn't check that the writes or copies are correct, and - /// doesn't check whether the descriptor set is in use. + /// Returns the variable descriptor count that this descriptor set was allocated with. + #[inline] + pub fn variable_descriptor_count(&self) -> u32 { + self.alloc.inner().variable_descriptor_count() + } + + /// Updates the descriptor set with new values. /// /// # Safety /// - /// - The `Device` must be the device the pool of this set was created with. - /// - Doesn't verify that the things you write in the descriptor set match its layout. - /// - Doesn't keep the resources alive. You have to do that yourself. - /// - Updating a descriptor set obeys synchronization rules that aren't checked here. Once a - /// command buffer contains a pointer/reference to a descriptor set, it is illegal to write - /// to it. - pub unsafe fn update<'a>( + /// - The resources in `descriptor_writes` and `descriptor_copies` must be kept alive for as + /// long as `self` is in use. + /// - The descriptor set must not be in use by the device, + /// or be recorded to a command buffer as part of a bind command. + #[inline] + pub unsafe fn update( &mut self, - layout: &DescriptorSetLayout, - descriptor_writes: impl IntoIterator, - descriptor_copies: impl IntoIterator, + descriptor_writes: &[WriteDescriptorSet], + descriptor_copies: &[CopyDescriptorSet], + ) -> Result<(), Box> { + self.validate_update(descriptor_writes, descriptor_copies)?; + + self.update_unchecked(descriptor_writes, descriptor_copies); + Ok(()) + } + + fn validate_update( + &self, + descriptor_writes: &[WriteDescriptorSet], + descriptor_copies: &[CopyDescriptorSet], + ) -> Result<(), Box> { + for (index, write) in descriptor_writes.iter().enumerate() { + write + .validate(self.layout(), self.variable_descriptor_count()) + .map_err(|err| err.add_context(format!("descriptor_writes[{}]", index)))?; + } + + for (index, copy) in descriptor_copies.iter().enumerate() { + copy.validate(self.layout(), self.variable_descriptor_count()) + .map_err(|err| err.add_context(format!("descriptor_copies[{}]", index)))?; + } + + Ok(()) + } + + #[cfg_attr(not(feature = "document_unchecked"), doc(hidden))] + pub unsafe fn update_unchecked( + &mut self, + descriptor_writes: &[WriteDescriptorSet], + descriptor_copies: &[CopyDescriptorSet], ) { struct PerDescriptorWrite { write_info: DescriptorWriteInfo, @@ -66,14 +123,12 @@ impl UnsafeDescriptorSet { inline_uniform_block: ash::vk::WriteDescriptorSetInlineUniformBlock, } - let writes_iter = descriptor_writes.into_iter(); - let (lower_size_bound, _) = writes_iter.size_hint(); - let mut writes_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(lower_size_bound); - let mut per_writes_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(lower_size_bound); + let mut writes_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(descriptor_writes.len()); + let mut per_writes_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(descriptor_writes.len()); - for write in writes_iter { - let layout_binding = &layout.bindings()[&write.binding()]; - writes_vk.push(write.to_vulkan(self.handle, layout_binding.descriptor_type)); + for write in descriptor_writes { + let layout_binding = &self.layout().bindings()[&write.binding()]; + writes_vk.push(write.to_vulkan(self.handle(), layout_binding.descriptor_type)); per_writes_vk.push(PerDescriptorWrite { write_info: write.to_vulkan_info(layout_binding.descriptor_type), acceleration_structures: Default::default(), @@ -81,12 +136,6 @@ impl UnsafeDescriptorSet { }); } - // It is forbidden to call `vkUpdateDescriptorSets` with 0 writes, so we need to perform - // this emptiness check. - if writes_vk.is_empty() { - return; - } - for (write_vk, per_write_vk) in writes_vk.iter_mut().zip(per_writes_vk.iter_mut()) { match &mut per_write_vk.write_info { DescriptorWriteInfo::Image(info) => { @@ -122,11 +171,9 @@ impl UnsafeDescriptorSet { debug_assert!(write_vk.descriptor_count != 0); } - let copies_iter = descriptor_copies.into_iter(); - let (lower_size_bound, _) = copies_iter.size_hint(); - let mut copies_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(lower_size_bound); + let mut copies_vk: SmallVec<[_; 8]> = SmallVec::with_capacity(descriptor_copies.len()); - for copy in copies_iter { + for copy in descriptor_copies { let &CopyDescriptorSet { ref src_set, src_binding, @@ -138,10 +185,10 @@ impl UnsafeDescriptorSet { } = copy; copies_vk.push(ash::vk::CopyDescriptorSet { - src_set: src_set.inner().handle(), + src_set: src_set.handle(), src_binding, src_array_element: src_first_array_element, - dst_set: self.handle, + dst_set: self.handle(), dst_binding, dst_array_element: dst_first_array_element, descriptor_count, @@ -149,9 +196,9 @@ impl UnsafeDescriptorSet { }); } - let fns = layout.device().fns(); + let fns = self.device().fns(); (fns.v1_0.update_descriptor_sets)( - layout.device().handle(), + self.device().handle(), writes_vk.len() as u32, writes_vk.as_ptr(), copies_vk.len() as u32, @@ -160,13 +207,46 @@ impl UnsafeDescriptorSet { } } -unsafe impl VulkanObject for UnsafeDescriptorSet { +unsafe impl

VulkanObject for UnsafeDescriptorSet

+where + P: DescriptorSetAlloc, +{ type Handle = ash::vk::DescriptorSet; #[inline] fn handle(&self) -> Self::Handle { - self.handle + self.alloc.inner().handle() } } -impl_id_counter!(UnsafeDescriptorSet); +unsafe impl

DeviceOwned for UnsafeDescriptorSet

+where + P: DescriptorSetAlloc, +{ + #[inline] + fn device(&self) -> &Arc { + self.alloc.inner().device() + } +} + +impl

PartialEq for UnsafeDescriptorSet

+where + P: DescriptorSetAlloc, +{ + #[inline] + fn eq(&self, other: &Self) -> bool { + self.alloc.inner() == other.alloc.inner() + } +} + +impl

Eq for UnsafeDescriptorSet

where P: DescriptorSetAlloc {} + +impl

Hash for UnsafeDescriptorSet

+where + P: DescriptorSetAlloc, +{ + #[inline] + fn hash(&self, state: &mut H) { + self.alloc.inner().hash(state); + } +} From 7e9fa3be9cd233a5ad853fd31b52e0f1999e6acd Mon Sep 17 00:00:00 2001 From: Rua Date: Wed, 11 Oct 2023 15:57:39 +0200 Subject: [PATCH 4/4] Add remaining `ext_descriptor_indexing` flags --- vulkano/src/descriptor_set/layout.rs | 315 +++++++++++++++++++++-- vulkano/src/descriptor_set/mod.rs | 8 +- vulkano/src/descriptor_set/persistent.rs | 10 +- vulkano/src/descriptor_set/pool.rs | 38 ++- vulkano/src/descriptor_set/sys.rs | 9 +- vulkano/src/descriptor_set/update.rs | 89 ++++++- vulkano/src/pipeline/layout.rs | 311 ++++++++++++++++------ 7 files changed, 670 insertions(+), 110 deletions(-) diff --git a/vulkano/src/descriptor_set/layout.rs b/vulkano/src/descriptor_set/layout.rs index 41c4e438ef..95dcae42aa 100644 --- a/vulkano/src/descriptor_set/layout.rs +++ b/vulkano/src/descriptor_set/layout.rs @@ -339,6 +339,8 @@ impl DescriptorSetLayoutCreateInfo { let mut total_descriptor_count = 0; let highest_binding_num = bindings.keys().copied().next_back(); + let mut update_after_bind_binding = None; + let mut buffer_dynamic_binding = None; for (&binding_num, binding) in bindings.iter() { binding @@ -381,11 +383,17 @@ impl DescriptorSetLayoutCreateInfo { })); } - if binding_flags.intersects(DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT) { + if binding_flags.intersects( + DescriptorBindingFlags::UPDATE_AFTER_BIND + | DescriptorBindingFlags::UPDATE_UNUSED_WHILE_PENDING + | DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT, + ) { return Err(Box::new(ValidationError { problem: format!( "`flags` contains `DescriptorSetLayoutCreateFlags::PUSH_DESCRIPTOR`, \ - and `bindings[{}].flags` contains \ + and `bindings[{}].binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, \ + `DescriptorBindingFlags::UPDATE_UNUSED_WHILE_PENDING` or \ `DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT`", binding_num ) @@ -401,7 +409,7 @@ impl DescriptorSetLayoutCreateInfo { { return Err(Box::new(ValidationError { problem: format!( - "`bindings[{}].flags` contains \ + "`bindings[{}].binding_flags` contains \ `DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT`, but {0} is not the highest binding number in `bindings`", binding_num @@ -413,6 +421,32 @@ impl DescriptorSetLayoutCreateInfo { ..Default::default() })); } + + if binding_flags.intersects(DescriptorBindingFlags::UPDATE_AFTER_BIND) { + if !flags.intersects(DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL) { + return Err(Box::new(ValidationError { + problem: format!( + "`bindings[{}].binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, but \ + `flags` does not contain \ + `DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL`", + binding_num + ) + .into(), + vuids: &["VUID-VkDescriptorSetLayoutCreateInfo-flags-03000"], + ..Default::default() + })); + } + + update_after_bind_binding.get_or_insert(binding_num); + } + + if matches!( + descriptor_type, + DescriptorType::UniformBufferDynamic | DescriptorType::StorageBufferDynamic + ) { + buffer_dynamic_binding.get_or_insert(binding_num); + } } let max_push_descriptors = device @@ -434,6 +468,24 @@ impl DescriptorSetLayoutCreateInfo { })); } + if let (Some(update_after_bind_binding), Some(buffer_dynamic_binding)) = + (update_after_bind_binding, buffer_dynamic_binding) + { + return Err(Box::new(ValidationError { + problem: format!( + "`bindings[{}].binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `bindings[{}].descriptor_type` is \ + `DescriptorType::UniformBufferDynamic` or \ + `DescriptorType::StorageBufferDynamic`", + update_after_bind_binding, buffer_dynamic_binding + ) + .into(), + vuids: &["VUID-VkDescriptorSetLayoutCreateInfo-descriptorType-03001"], + ..Default::default() + })); + } + Ok(()) } } @@ -455,13 +507,21 @@ vulkan_bitflags! { /// Flags that control how a descriptor set layout is created. DescriptorSetLayoutCreateFlags = DescriptorSetLayoutCreateFlags(u32); - /* TODO: enable - // TODO: document + /// Whether descriptor sets using this descriptor set layout must be allocated from a + /// descriptor pool whose flags contain [`DescriptorPoolCreateFlags::UPDATE_AFTER_BIND`]. + /// Descriptor set layouts with this flag use alternative (typically higher) device limits on + /// per-stage and total descriptor counts, which have `_update_after_bind_` in their names. + /// + /// This flag must be specified whenever the layout contains one or more bindings that have + /// the [`DescriptorBindingFlags::UPDATE_AFTER_BIND`] flag, but can be specified also if none + /// of the bindings have this flag, purely to use the alternative device limits. + /// + /// [`DescriptorPoolCreateFlags::UPDATE_AFTER_BIND`]: crate::descriptor_set::pool::DescriptorPoolCreateFlags::UPDATE_AFTER_BIND UPDATE_AFTER_BIND_POOL = UPDATE_AFTER_BIND_POOL RequiresOneOf([ RequiresAllOf([APIVersion(V1_2)]), RequiresAllOf([DeviceExtension(ext_descriptor_indexing)]), - ]), */ + ]), /// Whether the descriptor set layout should be created for push descriptors. /// @@ -506,6 +566,8 @@ vulkan_bitflags! { #[derive(Clone, Debug, PartialEq, Eq)] pub struct DescriptorSetLayoutBinding { /// Specifies how to create the binding. + /// + /// The default value is empty. pub binding_flags: DescriptorBindingFlags, /// The content and layout of each array element of a binding. @@ -733,6 +795,193 @@ impl DescriptorSetLayoutBinding { } } + if binding_flags.intersects(DescriptorBindingFlags::UPDATE_AFTER_BIND) { + match descriptor_type { + DescriptorType::UniformBuffer => { + if !device + .enabled_features() + .descriptor_binding_uniform_buffer_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::UniformBuffer`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_uniform_buffer_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingUniformBufferUpdateAfterBind-03005"], + ..Default::default() + })); + } + } + DescriptorType::Sampler + | DescriptorType::CombinedImageSampler + | DescriptorType::SampledImage => { + if !device + .enabled_features() + .descriptor_binding_sampled_image_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::Sampler`, \ + `DescriptorType::CombinedImageSampler` or \ + `DescriptorType::SampledImage`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_sampled_image_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingSampledImageUpdateAfterBind-03006"], + ..Default::default() + })); + } + } + DescriptorType::StorageImage => { + if !device + .enabled_features() + .descriptor_binding_storage_image_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::StorageImage`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_storage_image_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingStorageImageUpdateAfterBind-03007"], + ..Default::default() + })); + } + } + DescriptorType::StorageBuffer => { + if !device + .enabled_features() + .descriptor_binding_storage_buffer_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::StorageBuffer`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_storage_buffer_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingStorageBufferUpdateAfterBind-03008"], + ..Default::default() + })); + } + } + DescriptorType::UniformTexelBuffer => { + if !device + .enabled_features() + .descriptor_binding_uniform_texel_buffer_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::UniformTexelBuffer`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_uniform_texel_buffer_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingUniformTexelBufferUpdateAfterBind-03009"], + ..Default::default() + })); + } + } + DescriptorType::StorageTexelBuffer => { + if !device + .enabled_features() + .descriptor_binding_storage_texel_buffer_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::StorageTexelBuffer`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_storage_texel_buffer_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingStorageTexelBufferUpdateAfterBind-03010"], + ..Default::default() + })); + } + } + DescriptorType::InlineUniformBlock => { + if !device + .enabled_features() + .descriptor_binding_inline_uniform_block_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::InlineUniformBlock`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_inline_uniform_block_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingInlineUniformBlockUpdateAfterBind-02211"], + ..Default::default() + })); + } + } + DescriptorType::AccelerationStructure => { + if !device + .enabled_features() + .descriptor_binding_acceleration_structure_update_after_bind + { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::AccelerationStructure`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_acceleration_structure_update_after_bind" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingAccelerationStructureUpdateAfterBind-03570"], + ..Default::default() + })); + } + } + DescriptorType::InputAttachment + | DescriptorType::UniformBufferDynamic + | DescriptorType::StorageBufferDynamic => { + return Err(Box::new(ValidationError { + problem: "`binding_flags` contains \ + `DescriptorBindingFlags::UPDATE_AFTER_BIND`, and \ + `descriptor_type` is `DescriptorType::InputAttachment`, \ + `DescriptorType::UniformBufferDynamic` or \ + `DescriptorType::StorageBufferDynamic`" + .into(), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-None-03011"], + ..Default::default() + })); + } + } + } + + if binding_flags.intersects(DescriptorBindingFlags::UPDATE_UNUSED_WHILE_PENDING) + && !device + .enabled_features() + .descriptor_binding_update_unused_while_pending + { + return Err(Box::new(ValidationError { + context: "binding_flags".into(), + problem: "contains `DescriptorBindingFlags::UPDATE_UNUSED_WHILE_PENDING`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_update_unused_while_pending" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingUpdateUnusedWhilePending-03012"], + })); + } + + if binding_flags.intersects(DescriptorBindingFlags::PARTIALLY_BOUND) + && !device.enabled_features().descriptor_binding_partially_bound + { + return Err(Box::new(ValidationError { + context: "binding_flags".into(), + problem: "contains `DescriptorBindingFlags::PARTIALLY_BOUND`".into(), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_partially_bound" + )])]), + vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingPartiallyBound-03013"], + })); + } + if binding_flags.intersects(DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT) { if !device .enabled_features() @@ -741,9 +990,9 @@ impl DescriptorSetLayoutBinding { return Err(Box::new(ValidationError { context: "binding_flags".into(), problem: "contains `DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT`".into(), - requires_one_of: RequiresOneOf(&[ - RequiresAllOf(&[Requires::Feature("descriptor_binding_variable_descriptor_count")]) - ]), + requires_one_of: RequiresOneOf(&[RequiresAllOf(&[Requires::Feature( + "descriptor_binding_variable_descriptor_count" + )])]), vuids: &["VUID-VkDescriptorSetLayoutBindingFlagsCreateInfo-descriptorBindingVariableDescriptorCount-03014"], })); } @@ -790,29 +1039,59 @@ vulkan_bitflags! { /// Flags that control how a binding in a descriptor set layout is created. DescriptorBindingFlags = DescriptorBindingFlags(u32); - /* TODO: enable - // TODO: document + /// Allows descriptors in this binding to be updated after a command buffer has already + /// recorded a bind command containing a descriptor set with this layout, as long as the + /// command buffer is not executing. Each descriptor can also be updated concurrently. + /// + /// If a binding has this flag, then the descriptor set layout must be created with the + /// [`DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL`] flag, and descriptor sets using + /// it must be allocated from a descriptor pool that has the + /// [`DescriptorPoolCreateFlags::UPDATE_AFTER_BIND`] flag. + /// In addition, the `descriptor_binding_*_update_after_bind` feature corresponding to the + /// descriptor type of the binding must be enabled. + /// + /// [`DescriptorPoolCreateFlags::UPDATE_AFTER_BIND`]: crate::descriptor_set::pool::DescriptorPoolCreateFlags::UPDATE_AFTER_BIND UPDATE_AFTER_BIND = UPDATE_AFTER_BIND RequiresOneOf([ RequiresAllOf([APIVersion(V1_2)]), RequiresAllOf([DeviceExtension(ext_descriptor_indexing)]), - ]), */ + ]), - /* TODO: enable - // TODO: document + /// Allows descriptors in this binding to be updated after a command buffer has already + /// recorded a bind command containing a descriptor set with this layout, as long as the + /// command buffer is not executing, and no shader invocation recorded in the command buffer + /// *uses* the descriptor. Each descriptor can also be updated concurrently. + /// + /// This is a subset of what is allowed by [`DescriptorBindingFlags::UPDATE_AFTER_BIND`], but + /// has much less strict requirements. It does not require any additional flags to be present + /// on the descriptor set layout or the descriptor pool, and instead requires the + /// [`descriptor_binding_update_unused_while_pending`] feature. + /// + /// What counts as "used" depends on whether the [`DescriptorBindingFlags::PARTIALLY_BOUND`] + /// flag is also set. If it is set, then only *dynamic use* by a shader invocation counts as + /// being used, otherwise all *static use* by a shader invocation is considered used. + /// + /// [`descriptor_binding_update_unused_while_pending`]: crate::device::Features::descriptor_binding_update_unused_while_pending UPDATE_UNUSED_WHILE_PENDING = UPDATE_UNUSED_WHILE_PENDING RequiresOneOf([ RequiresAllOf([APIVersion(V1_2)]), RequiresAllOf([DeviceExtension(ext_descriptor_indexing)]), - ]), */ + ]), - /* TODO: enable - // TODO: document + /// Allows descriptors to be left empty or invalid even if they are *statically used* by a + /// shader invocation, as long as they are not *dynamically used* . Additionally, if + /// [`DescriptorBindingFlags::UPDATE_UNUSED_WHILE_PENDING`] is set, allows updating descriptors + /// if they are statically used by a command buffer they are recorded in, as long as are not + /// dynamically used. + /// + /// The [`descriptor_binding_partially_bound`] feature must be enabled on the device. + /// + /// [`descriptor_binding_partially_bound`]: crate::device::Features::descriptor_binding_partially_bound PARTIALLY_BOUND = PARTIALLY_BOUND RequiresOneOf([ RequiresAllOf([APIVersion(V1_2)]), RequiresAllOf([DeviceExtension(ext_descriptor_indexing)]), - ]), */ + ]), /// Whether the binding has a variable number of descriptors. /// diff --git a/vulkano/src/descriptor_set/mod.rs b/vulkano/src/descriptor_set/mod.rs index 4f13c17229..7f2b057890 100644 --- a/vulkano/src/descriptor_set/mod.rs +++ b/vulkano/src/descriptor_set/mod.rs @@ -87,7 +87,10 @@ pub use self::{ WriteDescriptorSetElements, }, }; -use self::{layout::DescriptorSetLayout, pool::DescriptorPoolAlloc}; +use self::{ + layout::DescriptorSetLayout, + pool::{DescriptorPool, DescriptorPoolAlloc}, +}; use crate::{ acceleration_structure::AccelerationStructure, buffer::view::BufferView, @@ -122,6 +125,9 @@ pub unsafe trait DescriptorSet: /// Returns the allocation of the descriptor set. fn alloc(&self) -> &DescriptorPoolAlloc; + /// Returns the descriptor pool that the descriptor set was allocated from. + fn pool(&self) -> &DescriptorPool; + /// Returns the layout of this descriptor set. #[inline] fn layout(&self) -> &Arc { diff --git a/vulkano/src/descriptor_set/persistent.rs b/vulkano/src/descriptor_set/persistent.rs index 23bd6b72dc..b4f30cd01e 100644 --- a/vulkano/src/descriptor_set/persistent.rs +++ b/vulkano/src/descriptor_set/persistent.rs @@ -21,7 +21,11 @@ //! # Examples //! TODO: -use super::{pool::DescriptorPoolAlloc, sys::UnsafeDescriptorSet, CopyDescriptorSet}; +use super::{ + pool::{DescriptorPool, DescriptorPoolAlloc}, + sys::UnsafeDescriptorSet, + CopyDescriptorSet, +}; use crate::{ descriptor_set::{ allocator::{DescriptorSetAlloc, DescriptorSetAllocator, StandardDescriptorSetAlloc}, @@ -126,6 +130,10 @@ where self.inner.alloc().inner() } + fn pool(&self) -> &DescriptorPool { + self.inner.alloc().pool() + } + fn resources(&self) -> &DescriptorSetResources { &self.resources } diff --git a/vulkano/src/descriptor_set/pool.rs b/vulkano/src/descriptor_set/pool.rs index 708d755418..f21b19ccc6 100644 --- a/vulkano/src/descriptor_set/pool.rs +++ b/vulkano/src/descriptor_set/pool.rs @@ -220,6 +220,33 @@ impl DescriptorPool { for (index, info) in allocate_infos.iter().enumerate() { info.validate(self.device()) .map_err(|err| err.add_context(format!("allocate_infos[{}]", index)))?; + + let &DescriptorSetAllocateInfo { + ref layout, + variable_descriptor_count: _, + _ne: _, + } = info; + + if layout + .flags() + .intersects(DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL) + && !self + .flags + .intersects(DescriptorPoolCreateFlags::UPDATE_AFTER_BIND) + { + return Err(Box::new(ValidationError { + problem: format!( + "`allocate_infos[{}].layout.flags()` contains \ + `DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL`, but \ + `self.flags` does not contain \ + `DescriptorPoolCreateFlags::UPDATE_AFTER_BIND`", + index + ) + .into(), + vuids: &["VUID-VkDescriptorSetAllocateInfo-pSetLayouts-03044"], + ..Default::default() + })); + } } Ok(()) @@ -558,13 +585,18 @@ vulkan_bitflags! { /// destroy the whole pool at once. FREE_DESCRIPTOR_SET = FREE_DESCRIPTOR_SET, - /* TODO: enable - // TODO: document + /// The pool can allocate descriptor sets with a layout whose flags include + /// [`DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL`]. + /// + /// A pool created with this flag can still allocate descriptor sets without the flag. + /// However, descriptor copy operations are only allowed between pools of the same type; + /// it is not possible to copy between a descriptor set whose pool has `UPDATE_AFTER_BIND`, + /// and a descriptor set whose pool does not have this flag. UPDATE_AFTER_BIND = UPDATE_AFTER_BIND RequiresOneOf([ RequiresAllOf([APIVersion(V1_2)]), RequiresAllOf([DeviceExtension(ext_descriptor_indexing)]), - ]), */ + ]), /* TODO: enable // TODO: document diff --git a/vulkano/src/descriptor_set/sys.rs b/vulkano/src/descriptor_set/sys.rs index 16985f45a0..53d13f56b1 100644 --- a/vulkano/src/descriptor_set/sys.rs +++ b/vulkano/src/descriptor_set/sys.rs @@ -11,6 +11,7 @@ use super::{ allocator::{DescriptorSetAlloc, DescriptorSetAllocator, StandardDescriptorSetAlloc}, + pool::DescriptorPool, CopyDescriptorSet, }; use crate::{ @@ -60,6 +61,12 @@ where &self.alloc } + /// Returns the descriptor pool that the descriptor set was allocated from. + #[inline] + pub fn pool(&self) -> &DescriptorPool { + self.alloc.pool() + } + /// Returns the layout of this descriptor set. #[inline] pub fn layout(&self) -> &Arc { @@ -104,7 +111,7 @@ where } for (index, copy) in descriptor_copies.iter().enumerate() { - copy.validate(self.layout(), self.variable_descriptor_count()) + copy.validate(self) .map_err(|err| err.add_context(format!("descriptor_copies[{}]", index)))?; } diff --git a/vulkano/src/descriptor_set/update.rs b/vulkano/src/descriptor_set/update.rs index e6b9ebb883..b71ef4253e 100644 --- a/vulkano/src/descriptor_set/update.rs +++ b/vulkano/src/descriptor_set/update.rs @@ -8,13 +8,18 @@ // according to those terms. use super::{ + allocator::DescriptorSetAlloc, layout::{DescriptorSetLayout, DescriptorType}, + sys::UnsafeDescriptorSet, DescriptorSet, }; use crate::{ acceleration_structure::{AccelerationStructure, AccelerationStructureType}, buffer::{view::BufferView, BufferUsage, Subbuffer}, - descriptor_set::layout::{DescriptorBindingFlags, DescriptorSetLayoutCreateFlags}, + descriptor_set::{ + layout::{DescriptorBindingFlags, DescriptorSetLayoutCreateFlags}, + pool::DescriptorPoolCreateFlags, + }, device::DeviceOwned, image::{ sampler::Sampler, @@ -1629,11 +1634,13 @@ impl CopyDescriptorSet { } } - pub(crate) fn validate( + pub(crate) fn validate

( &self, - dst_set_layout: &DescriptorSetLayout, - dst_set_variable_descriptor_count: u32, - ) -> Result<(), Box> { + dst_set: &UnsafeDescriptorSet

, + ) -> Result<(), Box> + where + P: DescriptorSetAlloc, + { let &Self { ref src_set, src_binding, @@ -1645,7 +1652,73 @@ impl CopyDescriptorSet { } = self; // VUID-VkCopyDescriptorSet-commonparent - assert_eq!(src_set.device(), dst_set_layout.device()); + assert_eq!(src_set.device(), dst_set.device()); + + match ( + src_set + .layout() + .flags() + .intersects(DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL), + dst_set + .layout() + .flags() + .intersects(DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL), + ) { + (true, false) => { + return Err(Box::new(ValidationError { + problem: "`src_set.layout().flags()` contains \ + `DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL`, but \ + `dst_set.layout().flags()` does not also contain it" + .into(), + vuids: &["VUID-VkCopyDescriptorSet-srcSet-01918"], + ..Default::default() + })); + } + (false, true) => { + return Err(Box::new(ValidationError { + problem: "`src_set.layout().flags()` does not contain \ + `DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL`, but \ + `dst_set.layout().flags()` does contain it" + .into(), + vuids: &["VUID-VkCopyDescriptorSet-srcSet-04885"], + ..Default::default() + })); + } + _ => (), + } + + match ( + src_set + .pool() + .flags() + .intersects(DescriptorPoolCreateFlags::UPDATE_AFTER_BIND), + dst_set + .pool() + .flags() + .intersects(DescriptorPoolCreateFlags::UPDATE_AFTER_BIND), + ) { + (true, false) => { + return Err(Box::new(ValidationError { + problem: "`src_set.pool().flags()` contains \ + `DescriptorPoolCreateFlags::UPDATE_AFTER_BIND`, but \ + `dst_set.pool().flags()` does not also contain it" + .into(), + vuids: &["VUID-VkCopyDescriptorSet-srcSet-01920"], + ..Default::default() + })); + } + (false, true) => { + return Err(Box::new(ValidationError { + problem: "`src_set.pool().flags()` does not contain \ + `DescriptorPoolCreateFlags::UPDATE_AFTER_BIND`, but \ + `dst_set.pool().flags()` does contain it" + .into(), + vuids: &["VUID-VkCopyDescriptorSet-srcSet-04887"], + ..Default::default() + })); + } + _ => (), + } let src_layout_binding = match src_set.layout().bindings().get(&src_binding) { Some(layout_binding) => layout_binding, @@ -1679,7 +1752,7 @@ impl CopyDescriptorSet { })); } - let dst_layout_binding = match dst_set_layout.bindings().get(&dst_binding) { + let dst_layout_binding = match dst_set.layout().bindings().get(&dst_binding) { Some(layout_binding) => layout_binding, None => { return Err(Box::new(ValidationError { @@ -1696,7 +1769,7 @@ impl CopyDescriptorSet { .binding_flags .intersects(DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT) { - dst_set_variable_descriptor_count + dst_set.variable_descriptor_count() } else { dst_layout_binding.descriptor_count }; diff --git a/vulkano/src/pipeline/layout.rs b/vulkano/src/pipeline/layout.rs index e7808e177e..e687d208ae 100644 --- a/vulkano/src/pipeline/layout.rs +++ b/vulkano/src/pipeline/layout.rs @@ -473,9 +473,12 @@ impl PipelineLayoutCreateInfo { struct DescriptorLimit { descriptor_types: &'static [DescriptorType], - get_limit: fn(&Properties) -> u32, - limit_name: &'static str, - vuids: &'static [&'static str], + get_limit_all: fn(&Properties) -> Option, + limit_name_all: &'static str, + vuids_all: &'static [&'static str], + get_limit_not_uab: fn(&Properties) -> u32, + limit_name_not_uab: &'static str, + vuids_not_uab: &'static [&'static str], } const PER_STAGE_DESCRIPTOR_LIMITS: [DescriptorLimit; 8] = [ @@ -484,27 +487,36 @@ impl PipelineLayoutCreateInfo { DescriptorType::Sampler, DescriptorType::CombinedImageSampler, ], - get_limit: |p| p.max_per_stage_descriptor_samplers, - limit_name: "max_per_stage_descriptor_samplers", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03016"], + get_limit_all: |p| p.max_per_stage_descriptor_update_after_bind_samplers, + limit_name_all: "max_per_stage_descriptor_update_after_bind_samplers", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03022"], + get_limit_not_uab: |p| p.max_per_stage_descriptor_samplers, + limit_name_not_uab: "max_per_stage_descriptor_samplers", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03016"], }, DescriptorLimit { descriptor_types: &[ DescriptorType::UniformBuffer, DescriptorType::UniformBufferDynamic, ], - get_limit: |p| p.max_per_stage_descriptor_uniform_buffers, - limit_name: "max_per_stage_descriptor_uniform_buffers", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03017"], + get_limit_all: |p| p.max_per_stage_descriptor_update_after_bind_uniform_buffers, + limit_name_all: "max_per_stage_descriptor_update_after_bind_uniform_buffers", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03023"], + get_limit_not_uab: |p| p.max_per_stage_descriptor_uniform_buffers, + limit_name_not_uab: "max_per_stage_descriptor_uniform_buffers", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03017"], }, DescriptorLimit { descriptor_types: &[ DescriptorType::StorageBuffer, DescriptorType::StorageBufferDynamic, ], - get_limit: |p| p.max_per_stage_descriptor_storage_buffers, - limit_name: "max_per_stage_descriptor_storage_buffers", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03018"], + get_limit_all: |p| p.max_per_stage_descriptor_update_after_bind_storage_buffers, + limit_name_all: "max_per_stage_descriptor_update_after_bind_storage_buffers", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03024"], + get_limit_not_uab: |p| p.max_per_stage_descriptor_storage_buffers, + limit_name_not_uab: "max_per_stage_descriptor_storage_buffers", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03018"], }, DescriptorLimit { descriptor_types: &[ @@ -512,42 +524,62 @@ impl PipelineLayoutCreateInfo { DescriptorType::SampledImage, DescriptorType::UniformTexelBuffer, ], - get_limit: |p| p.max_per_stage_descriptor_sampled_images, - limit_name: "max_per_stage_descriptor_sampled_images", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-06939"], + get_limit_all: |p| p.max_per_stage_descriptor_update_after_bind_sampled_images, + limit_name_all: "max_per_stage_descriptor_update_after_bind_sampled_images", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03025"], + get_limit_not_uab: |p| p.max_per_stage_descriptor_sampled_images, + limit_name_not_uab: "max_per_stage_descriptor_sampled_images", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-06939"], }, DescriptorLimit { descriptor_types: &[ DescriptorType::StorageImage, DescriptorType::StorageTexelBuffer, ], - get_limit: |p| p.max_per_stage_descriptor_storage_images, - limit_name: "max_per_stage_descriptor_storage_images", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03020"], + get_limit_all: |p| p.max_per_stage_descriptor_update_after_bind_storage_images, + limit_name_all: "max_per_stage_descriptor_update_after_bind_storage_images", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03026"], + get_limit_not_uab: |p| p.max_per_stage_descriptor_storage_images, + limit_name_not_uab: "max_per_stage_descriptor_storage_images", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03020"], }, DescriptorLimit { descriptor_types: &[DescriptorType::InputAttachment], - get_limit: |p| p.max_per_stage_descriptor_input_attachments, - limit_name: "max_per_stage_descriptor_input_attachments", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03021"], + get_limit_all: |p| p.max_per_stage_descriptor_update_after_bind_input_attachments, + limit_name_all: "max_per_stage_descriptor_update_after_bind_input_attachments", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03027"], + get_limit_not_uab: |p| p.max_per_stage_descriptor_input_attachments, + limit_name_not_uab: "max_per_stage_descriptor_input_attachments", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03021"], }, DescriptorLimit { descriptor_types: &[DescriptorType::InlineUniformBlock], - get_limit: |p| { + get_limit_all: |p| { + p.max_per_stage_descriptor_update_after_bind_inline_uniform_blocks + }, + limit_name_all: "max_per_stage_descriptor_update_after_bind_inline_uniform_blocks", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-02215"], + get_limit_not_uab: |p| { p.max_per_stage_descriptor_inline_uniform_blocks .unwrap_or(0) }, - limit_name: "max_per_stage_descriptor_inline_uniform_blocks", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-02214"], + limit_name_not_uab: "max_per_stage_descriptor_inline_uniform_blocks", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-02214"], }, DescriptorLimit { descriptor_types: &[DescriptorType::AccelerationStructure], - get_limit: |p| { + get_limit_all: |p| { + p.max_per_stage_descriptor_update_after_bind_acceleration_structures + }, + limit_name_all: + "max_per_stage_descriptor_update_after_bind_acceleration_structures", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03572"], + get_limit_not_uab: |p| { p.max_per_stage_descriptor_acceleration_structures .unwrap_or(0) }, - limit_name: "max_per_stage_descriptor_acceleration_structures", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03571"], + limit_name_not_uab: "max_per_stage_descriptor_acceleration_structures", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03571"], }, ]; @@ -557,33 +589,48 @@ impl PipelineLayoutCreateInfo { DescriptorType::Sampler, DescriptorType::CombinedImageSampler, ], - get_limit: |p| p.max_descriptor_set_samplers, - limit_name: "max_descriptor_set_samplers", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03028"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_samplers, + limit_name_all: "max_descriptor_set_update_after_bind_samplers", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03036"], + get_limit_not_uab: |p| p.max_descriptor_set_samplers, + limit_name_not_uab: "max_descriptor_set_samplers", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03028"], }, DescriptorLimit { descriptor_types: &[DescriptorType::UniformBuffer], - get_limit: |p| p.max_descriptor_set_uniform_buffers, - limit_name: "max_descriptor_set_uniform_buffers", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03029"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_uniform_buffers, + limit_name_all: "max_descriptor_set_update_after_bind_uniform_buffers", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03037"], + get_limit_not_uab: |p| p.max_descriptor_set_uniform_buffers, + limit_name_not_uab: "max_descriptor_set_uniform_buffers", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03029"], }, DescriptorLimit { descriptor_types: &[DescriptorType::UniformBufferDynamic], - get_limit: |p| p.max_descriptor_set_uniform_buffers_dynamic, - limit_name: "max_descriptor_set_uniform_buffers_dynamic", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03030"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_uniform_buffers_dynamic, + limit_name_all: "max_descriptor_set_update_after_bind_uniform_buffers_dynamic", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03038"], + get_limit_not_uab: |p| p.max_descriptor_set_uniform_buffers_dynamic, + limit_name_not_uab: "max_descriptor_set_uniform_buffers_dynamic", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03030"], }, DescriptorLimit { descriptor_types: &[DescriptorType::StorageBuffer], - get_limit: |p| p.max_descriptor_set_storage_buffers, - limit_name: "max_descriptor_set_storage_buffers", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03031"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_storage_buffers, + limit_name_all: "max_descriptor_set_update_after_bind_storage_buffers", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03039"], + get_limit_not_uab: |p| p.max_descriptor_set_storage_buffers, + limit_name_not_uab: "max_descriptor_set_storage_buffers", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03031"], }, DescriptorLimit { descriptor_types: &[DescriptorType::StorageBufferDynamic], - get_limit: |p| p.max_descriptor_set_storage_buffers_dynamic, - limit_name: "max_descriptor_set_storage_buffers_dynamic", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03032"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_storage_buffers_dynamic, + limit_name_all: "max_descriptor_set_update_after_bind_storage_buffers_dynamic", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03040"], + get_limit_not_uab: |p| p.max_descriptor_set_storage_buffers_dynamic, + limit_name_not_uab: "max_descriptor_set_storage_buffers_dynamic", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03032"], }, DescriptorLimit { descriptor_types: &[ @@ -591,42 +638,60 @@ impl PipelineLayoutCreateInfo { DescriptorType::SampledImage, DescriptorType::UniformTexelBuffer, ], - get_limit: |p| p.max_descriptor_set_sampled_images, - limit_name: "max_descriptor_set_sampled_images", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03033"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_sampled_images, + limit_name_all: "max_descriptor_set_update_after_bind_sampled_images", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03041"], + get_limit_not_uab: |p| p.max_descriptor_set_sampled_images, + limit_name_not_uab: "max_descriptor_set_sampled_images", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03033"], }, DescriptorLimit { descriptor_types: &[ DescriptorType::StorageImage, DescriptorType::StorageTexelBuffer, ], - get_limit: |p| p.max_descriptor_set_storage_images, - limit_name: "max_descriptor_set_storage_images", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03034"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_storage_images, + limit_name_all: "max_descriptor_set_update_after_bind_storage_images", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03042"], + get_limit_not_uab: |p| p.max_descriptor_set_storage_images, + limit_name_not_uab: "max_descriptor_set_storage_images", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03034"], }, DescriptorLimit { descriptor_types: &[DescriptorType::InputAttachment], - get_limit: |p| p.max_descriptor_set_input_attachments, - limit_name: "max_descriptor_set_input_attachments", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03035"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_input_attachments, + limit_name_all: "max_descriptor_set_update_after_bind_input_attachments", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-pSetLayouts-03043"], + get_limit_not_uab: |p| p.max_descriptor_set_input_attachments, + limit_name_not_uab: "max_descriptor_set_input_attachments", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03035"], }, DescriptorLimit { descriptor_types: &[DescriptorType::InlineUniformBlock], - get_limit: |p| p.max_descriptor_set_inline_uniform_blocks.unwrap_or(0), - limit_name: "max_descriptor_set_inline_uniform_blocks", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-02216"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_inline_uniform_blocks, + limit_name_all: "max_descriptor_set_update_after_bind_inline_uniform_blocks", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-02217"], + get_limit_not_uab: |p| p.max_descriptor_set_inline_uniform_blocks.unwrap_or(0), + limit_name_not_uab: "max_descriptor_set_inline_uniform_blocks", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-02216"], }, DescriptorLimit { descriptor_types: &[DescriptorType::AccelerationStructure], - get_limit: |p| p.max_descriptor_set_acceleration_structures.unwrap_or(0), - limit_name: "max_descriptor_set_acceleration_structures", - vuids: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03573"], + get_limit_all: |p| p.max_descriptor_set_update_after_bind_acceleration_structures, + limit_name_all: "max_descriptor_set_update_after_bind_acceleration_structures", + vuids_all: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03574"], + get_limit_not_uab: |p| p.max_descriptor_set_acceleration_structures.unwrap_or(0), + limit_name_not_uab: "max_descriptor_set_acceleration_structures", + vuids_not_uab: &["VUID-VkPipelineLayoutCreateInfo-descriptorType-03573"], }, ]; - let mut per_stage_descriptors: [HashMap; + let mut per_stage_descriptors_all: [HashMap; PER_STAGE_DESCRIPTOR_LIMITS.len()] = array::from_fn(|_| HashMap::default()); - let mut total_descriptors = [0; TOTAL_DESCRIPTOR_LIMITS.len()]; + let mut per_stage_descriptors_not_uab: [HashMap; + PER_STAGE_DESCRIPTOR_LIMITS.len()] = array::from_fn(|_| HashMap::default()); + let mut total_descriptors_all = [0; TOTAL_DESCRIPTOR_LIMITS.len()]; + let mut total_descriptors_not_uab = [0; TOTAL_DESCRIPTOR_LIMITS.len()]; let mut has_push_descriptor_set = false; for (_set_num, set_layout) in set_layouts.iter().enumerate() { @@ -650,6 +715,10 @@ impl PipelineLayoutCreateInfo { has_push_descriptor_set = true; } + let is_not_uab = !set_layout + .flags() + .intersects(DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL); + for layout_binding in set_layout.bindings().values() { let &DescriptorSetLayoutBinding { binding_flags: _, @@ -660,36 +729,86 @@ impl PipelineLayoutCreateInfo { _ne: _, } = layout_binding; - for (limit, count) in PER_STAGE_DESCRIPTOR_LIMITS + for ((limit, count_all), count_not_uab) in PER_STAGE_DESCRIPTOR_LIMITS .iter() - .zip(&mut per_stage_descriptors) + .zip(&mut per_stage_descriptors_all) + .zip(&mut per_stage_descriptors_not_uab) { if limit.descriptor_types.contains(&descriptor_type) { for stage in stages { - *count.entry(stage).or_default() += descriptor_count; + *count_all.entry(stage).or_default() += descriptor_count; + + if is_not_uab { + *count_not_uab.entry(stage).or_default() += descriptor_count; + } } } } - for (limit, count) in TOTAL_DESCRIPTOR_LIMITS.iter().zip(&mut total_descriptors) { + for ((limit, count_all), count_not_uab) in TOTAL_DESCRIPTOR_LIMITS + .iter() + .zip(&mut total_descriptors_all) + .zip(&mut total_descriptors_not_uab) + { if limit.descriptor_types.contains(&descriptor_type) { - *count += descriptor_count; + *count_all += descriptor_count; + + if is_not_uab { + *count_not_uab += descriptor_count; + } } } } } - for (limit, count) in PER_STAGE_DESCRIPTOR_LIMITS + for ((limit, count_all), count_not_uab) in PER_STAGE_DESCRIPTOR_LIMITS .iter() - .zip(per_stage_descriptors) + .zip(per_stage_descriptors_all) + .zip(per_stage_descriptors_not_uab) { - if let Some((max_stage, max_count)) = count.into_iter().max_by_key(|(_, c)| *c) { - if max_count > (limit.get_limit)(properties) { + let limit_not_uab = (limit.get_limit_not_uab)(properties); + + if let Some((max_stage, max_count)) = count_not_uab.into_iter().max_by_key(|(_, c)| *c) + { + if max_count > limit_not_uab { + return Err(Box::new(ValidationError { + context: "set_layouts".into(), + problem: format!( + "the combined number of {} descriptors, \ + belonging to descriptor set layouts without the \ + `DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL` flag, \ + accessible to the `ShaderStage::{:?}` stage, \ + exceeds the `{}` limit", + limit.descriptor_types[1..].iter().fold( + format!("`DescriptorType::{:?}`", limit.descriptor_types[0]), + |mut s, dt| { + write!(s, " + `DescriptorType::{:?}`", dt).unwrap(); + s + } + ), + max_stage, + limit.limit_name_not_uab, + ) + .into(), + vuids: limit.vuids_not_uab, + ..Default::default() + })); + } + } + + let limit_all = match (limit.get_limit_all)(properties) { + Some(x) => x, + None => continue, + }; + + if let Some((max_stage, max_count)) = count_all.into_iter().max_by_key(|(_, c)| *c) { + if max_count > limit_all { return Err(Box::new(ValidationError { context: "set_layouts".into(), problem: format!( - "the combined number of {} descriptors accessible to the \ - `ShaderStage::{:?}` stage exceeds the `{}` limit", + "the combined number of {} descriptors, \ + accessible to the `ShaderStage::{:?}` stage, \ + exceeds the `{}` limit", limit.descriptor_types[1..].iter().fold( format!("`DescriptorType::{:?}`", limit.descriptor_types[0]), |mut s, dt| { @@ -698,23 +817,59 @@ impl PipelineLayoutCreateInfo { } ), max_stage, - limit.limit_name, + limit.limit_name_all, ) .into(), - vuids: limit.vuids, + vuids: limit.vuids_all, ..Default::default() })); } } } - for (limit, count) in TOTAL_DESCRIPTOR_LIMITS.iter().zip(total_descriptors) { - if count > (limit.get_limit)(properties) { + for ((limit, count_all), count_not_uab) in TOTAL_DESCRIPTOR_LIMITS + .iter() + .zip(total_descriptors_all) + .zip(total_descriptors_not_uab) + { + let limit_not_uab = (limit.get_limit_not_uab)(properties); + + if count_not_uab > limit_not_uab { + return Err(Box::new(ValidationError { + context: "set_layouts".into(), + problem: format!( + "the combined number of {} descriptors, \ + belonging to descriptor set layouts without the \ + `DescriptorSetLayoutCreateFlags::UPDATE_AFTER_BIND_POOL` flag, \ + accessible across all shader stages, \ + exceeds the `{}` limit", + limit.descriptor_types[1..].iter().fold( + format!("`DescriptorType::{:?}`", limit.descriptor_types[0]), + |mut s, dt| { + write!(s, " + `DescriptorType::{:?}`", dt).unwrap(); + s + } + ), + limit.limit_name_not_uab, + ) + .into(), + vuids: limit.vuids_not_uab, + ..Default::default() + })); + } + + let limit_all = match (limit.get_limit_all)(properties) { + Some(x) => x, + None => continue, + }; + + if count_all > limit_all { return Err(Box::new(ValidationError { context: "set_layouts".into(), problem: format!( - "the combined number of {} descriptors accessible across all \ - shader stages exceeds the `{}` limit", + "the combined number of {} descriptors, \ + accessible across all shader stages, \ + exceeds the `{}` limit", limit.descriptor_types[1..].iter().fold( format!("`DescriptorType::{:?}`", limit.descriptor_types[0]), |mut s, dt| { @@ -722,10 +877,10 @@ impl PipelineLayoutCreateInfo { s } ), - limit.limit_name, + limit.limit_name_all, ) .into(), - vuids: limit.vuids, + vuids: limit.vuids_all, ..Default::default() })); }