From 3aca5dc1905c584a3df6e7c017340a16c2be12ff Mon Sep 17 00:00:00 2001 From: Rua Date: Fri, 16 Dec 2022 11:40:46 +0100 Subject: [PATCH] Fix #1643 (#2103) * Fix #1643 * Remove old code that was incorrect anyway --- examples/src/bin/dynamic-buffers.rs | 12 +- .../src/command_buffer/commands/bind_push.rs | 165 ++- .../src/command_buffer/commands/pipeline.rs | 50 +- .../standard/builder/bind_push.rs | 92 +- .../command_buffer/standard/builder/mod.rs | 8 + .../standard/builder/pipeline.rs | 63 +- vulkano/src/command_buffer/synced/builder.rs | 8 + vulkano/src/descriptor_set/mod.rs | 74 +- vulkano/src/descriptor_set/persistent.rs | 4 + vulkano/src/descriptor_set/update.rs | 956 ++++++++++-------- 10 files changed, 914 insertions(+), 518 deletions(-) diff --git a/examples/src/bin/dynamic-buffers.rs b/examples/src/bin/dynamic-buffers.rs index c56a4799a..cf34eb5c5 100644 --- a/examples/src/bin/dynamic-buffers.rs +++ b/examples/src/bin/dynamic-buffers.rs @@ -32,7 +32,7 @@ use vulkano::{ memory::allocator::StandardMemoryAllocator, pipeline::{ComputePipeline, Pipeline, PipelineBindPoint}, sync::{self, GpuFuture}, - VulkanLibrary, + DeviceSize, VulkanLibrary, }; fn main() { @@ -188,7 +188,15 @@ fn main() { &descriptor_set_allocator, layout.clone(), [ - WriteDescriptorSet::buffer(0, input_buffer), + // When writing to the dynamic buffer binding, the range of the buffer that the shader + // will access must also be provided. We specify the size of the `InData` struct here. + // When dynamic offsets are provided later, they get added to the start and end of + // this range. + WriteDescriptorSet::buffer_with_range( + 0, + input_buffer, + 0..size_of::() as DeviceSize, + ), WriteDescriptorSet::buffer(1, output_buffer.clone()), ], ) diff --git a/vulkano/src/command_buffer/commands/bind_push.rs b/vulkano/src/command_buffer/commands/bind_push.rs index 636049666..4fc0370d8 100644 --- a/vulkano/src/command_buffer/commands/bind_push.rs +++ b/vulkano/src/command_buffer/commands/bind_push.rs @@ -17,9 +17,10 @@ use crate::{ AutoCommandBufferBuilder, }, descriptor_set::{ - check_descriptor_write, sys::UnsafeDescriptorSet, DescriptorSetResources, - DescriptorSetUpdateError, DescriptorSetWithOffsets, DescriptorSetsCollection, - DescriptorWriteInfo, WriteDescriptorSet, + check_descriptor_write, layout::DescriptorType, sys::UnsafeDescriptorSet, + DescriptorBindingResources, DescriptorSetResources, DescriptorSetUpdateError, + DescriptorSetWithOffsets, DescriptorSetsCollection, DescriptorWriteInfo, + WriteDescriptorSet, }, device::{DeviceOwned, QueueFlags}, pipeline::{ @@ -36,6 +37,7 @@ use crate::{ use parking_lot::Mutex; use smallvec::SmallVec; use std::{ + cmp::min, error, fmt::{Display, Error as FmtError, Formatter}, mem::{size_of, size_of_val}, @@ -129,24 +131,93 @@ where }); } + let properties = self.device().physical_device().properties(); + let uniform_alignment = properties.min_uniform_buffer_offset_alignment as u32; + let storage_alignment = properties.min_storage_buffer_offset_alignment as u32; + for (i, set) in descriptor_sets.iter().enumerate() { let set_num = first_set + i as u32; + let (set, dynamic_offsets) = set.as_ref(); // VUID-vkCmdBindDescriptorSets-commonparent - assert_eq!(self.device(), set.as_ref().0.device()); + assert_eq!(self.device(), set.device()); - let pipeline_layout_set = &pipeline_layout.set_layouts()[set_num as usize]; + let set_layout = set.layout(); + let pipeline_set_layout = &pipeline_layout.set_layouts()[set_num as usize]; // VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358 - if !pipeline_layout_set.is_compatible_with(set.as_ref().0.layout()) { + if !pipeline_set_layout.is_compatible_with(set_layout) { return Err(BindPushError::DescriptorSetNotCompatible { set_num }); } - // TODO: see https://github.com/vulkano-rs/vulkano/issues/1643 - // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01971 - // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01972 - // VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979 - // VUID-vkCmdBindDescriptorSets-pDescriptorSets-06715 + let mut dynamic_offsets_remaining = dynamic_offsets; + let mut required_dynamic_offset_count = 0; + + for (&binding_num, binding) in set_layout.bindings() { + let required_alignment = match binding.descriptor_type { + DescriptorType::UniformBufferDynamic => uniform_alignment, + DescriptorType::StorageBufferDynamic => storage_alignment, + _ => continue, + }; + + let count = if binding.variable_descriptor_count { + set.variable_descriptor_count() + } else { + binding.descriptor_count + } as usize; + + required_dynamic_offset_count += count; + + if !dynamic_offsets_remaining.is_empty() { + let split_index = min(count, dynamic_offsets_remaining.len()); + let dynamic_offsets = &dynamic_offsets_remaining[..split_index]; + dynamic_offsets_remaining = &dynamic_offsets_remaining[split_index..]; + + let elements = match set.resources().binding(binding_num) { + Some(DescriptorBindingResources::Buffer(elements)) => elements.as_slice(), + _ => unreachable!(), + }; + + for (index, (&offset, element)) in + dynamic_offsets.iter().zip(elements).enumerate() + { + // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01971 + // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01972 + if offset % required_alignment != 0 { + return Err(BindPushError::DynamicOffsetNotAligned { + set_num, + binding_num, + index: index as u32, + offset, + required_alignment, + }); + } + + if let Some((buffer, range)) = element { + // VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979 + if offset as DeviceSize + range.end > buffer.size() { + return Err(BindPushError::DynamicOffsetOutOfBufferBounds { + set_num, + binding_num, + index: index as u32, + offset, + range_end: range.end, + buffer_size: buffer.size(), + }); + } + } + } + } + } + + // VUID-vkCmdBindDescriptorSets-dynamicOffsetCount-00359 + if dynamic_offsets.len() != required_dynamic_offset_count { + return Err(BindPushError::DynamicOffsetCountMismatch { + set_num, + provided_count: dynamic_offsets.len(), + required_count: required_dynamic_offset_count, + }); + } } Ok(()) @@ -1246,6 +1317,39 @@ pub(in super::super) enum BindPushError { pipeline_layout_set_count: u32, }, + /// In an element of `descriptor_sets`, the number of provided dynamic offsets does not match + /// the number required by the descriptor set. + DynamicOffsetCountMismatch { + set_num: u32, + provided_count: usize, + required_count: usize, + }, + + /// In an element of `descriptor_sets`, a provided dynamic offset + /// is not a multiple of the value of the [`min_uniform_buffer_offset_alignment`] or + /// [`min_storage_buffer_offset_alignment`] property. + /// + /// min_uniform_buffer_offset_alignment: crate::device::Properties::min_uniform_buffer_offset_alignment + /// min_storage_buffer_offset_alignment: crate::device::Properties::min_storage_buffer_offset_alignment + DynamicOffsetNotAligned { + set_num: u32, + binding_num: u32, + index: u32, + offset: u32, + required_alignment: u32, + }, + + /// In an element of `descriptor_sets`, a provided dynamic offset, when added to the end of the + /// buffer range bound to the descriptor set, is greater than the size of the buffer. + DynamicOffsetOutOfBufferBounds { + set_num: u32, + binding_num: u32, + index: u32, + offset: u32, + range_end: DeviceSize, + buffer_size: DeviceSize, + }, + /// An index buffer is missing the `index_buffer` usage. IndexBufferMissingUsage, @@ -1328,6 +1432,45 @@ impl Display for BindPushError { sets in `pipeline_layout` ({})", set_num, pipeline_layout_set_count, ), + Self::DynamicOffsetCountMismatch { + set_num, + provided_count, + required_count, + } => write!( + f, + "in the element of `descriptor_sets` being bound to slot {}, the number of \ + provided dynamic offsets ({}) does not match the number required by the \ + descriptor set ({})", + set_num, provided_count, required_count, + ), + Self::DynamicOffsetNotAligned { + set_num, + binding_num, + index, + offset, + required_alignment, + } => write!( + f, + "in the element of `descriptor_sets` being bound to slot {}, the dynamic offset \ + provided for binding {} index {} ({}) is not a multiple of the value of the \ + `min_uniform_buffer_offset_alignment` or `min_storage_buffer_offset_alignment` \ + property ({})", + set_num, binding_num, index, offset, required_alignment, + ), + Self::DynamicOffsetOutOfBufferBounds { + set_num, + binding_num, + index, + offset, + range_end, + buffer_size, + } => write!( + f, + "in the element of `descriptor_sets` being bound to slot {}, the dynamic offset \ + provided for binding {} index {} ({}), when added to the end of the buffer range \ + bound to the descriptor set ({}), is greater than the size of the buffer ({})", + set_num, binding_num, index, offset, range_end, buffer_size, + ), Self::IndexBufferMissingUsage => { write!(f, "an index buffer is missing the `index_buffer` usage") } diff --git a/vulkano/src/command_buffer/commands/pipeline.rs b/vulkano/src/command_buffer/commands/pipeline.rs index eec9ad86a..21e6b3823 100644 --- a/vulkano/src/command_buffer/commands/pipeline.rs +++ b/vulkano/src/command_buffer/commands/pipeline.rs @@ -647,7 +647,7 @@ where let layout_binding = &pipeline.layout().set_layouts()[set_num as usize].bindings()[&binding_num]; - let check_buffer = |_index: u32, _buffer: &Arc| Ok(()); + let check_buffer = |_index: u32, (_buffer, _range): &(Arc, Range)| Ok(()); let check_buffer_view = |index: u32, buffer_view: &Arc| { for desc_reqs in (binding_reqs.descriptors.get(&Some(index)).into_iter()) @@ -2144,26 +2144,46 @@ impl SyncCommandBufferBuilder { }) }; - match descriptor_sets_state.descriptor_sets[&set] - .resources() + let descriptor_set_state = &descriptor_sets_state.descriptor_sets[&set]; + + match descriptor_set_state.resources() .binding(binding) .unwrap() { DescriptorBindingResources::None(_) => continue, DescriptorBindingResources::Buffer(elements) => { - resources.extend( - (elements.iter().enumerate()) - .filter_map(|(index, element)| { - element.as_ref().map(|buffer| { - ( - index as u32, - buffer.clone(), - 0..buffer.size(), // TODO: - ) + if matches!(descriptor_type, DescriptorType::UniformBufferDynamic | DescriptorType::StorageBufferDynamic) { + let dynamic_offsets = descriptor_set_state.dynamic_offsets(); + resources.extend( + (elements.iter().enumerate()) + .filter_map(|(index, element)| { + element.as_ref().map(|(buffer, range)| { + let dynamic_offset = dynamic_offsets[index] as DeviceSize; + + ( + index as u32, + buffer.clone(), + dynamic_offset + range.start..dynamic_offset + range.end, + ) + }) }) - }) - .flat_map(buffer_resource), - ); + .flat_map(buffer_resource), + ); + } else { + resources.extend( + (elements.iter().enumerate()) + .filter_map(|(index, element)| { + element.as_ref().map(|(buffer, range)| { + ( + index as u32, + buffer.clone(), + range.clone(), + ) + }) + }) + .flat_map(buffer_resource), + ); + } } DescriptorBindingResources::BufferView(elements) => { resources.extend( diff --git a/vulkano/src/command_buffer/standard/builder/bind_push.rs b/vulkano/src/command_buffer/standard/builder/bind_push.rs index 26a27a8a7..009c1e10c 100644 --- a/vulkano/src/command_buffer/standard/builder/bind_push.rs +++ b/vulkano/src/command_buffer/standard/builder/bind_push.rs @@ -12,8 +12,9 @@ use crate::{ buffer::{BufferAccess, BufferContents, BufferUsage, TypedBufferAccess}, command_buffer::{allocator::CommandBufferAllocator, commands::bind_push::BindPushError}, descriptor_set::{ - check_descriptor_write, DescriptorSetResources, DescriptorSetWithOffsets, - DescriptorSetsCollection, DescriptorWriteInfo, WriteDescriptorSet, + check_descriptor_write, layout::DescriptorType, DescriptorBindingResources, + DescriptorSetResources, DescriptorSetWithOffsets, DescriptorSetsCollection, + DescriptorWriteInfo, WriteDescriptorSet, }, device::{DeviceOwned, QueueFlags}, pipeline::{ @@ -24,7 +25,7 @@ use crate::{ }, ComputePipeline, GraphicsPipeline, PipelineBindPoint, PipelineLayout, }, - RequiresOneOf, VulkanObject, + DeviceSize, RequiresOneOf, VulkanObject, }; use smallvec::SmallVec; use std::{cmp::min, sync::Arc}; @@ -108,24 +109,93 @@ where }); } + let properties = self.device().physical_device().properties(); + let uniform_alignment = properties.min_uniform_buffer_offset_alignment as u32; + let storage_alignment = properties.min_storage_buffer_offset_alignment as u32; + for (i, set) in descriptor_sets.iter().enumerate() { let set_num = first_set + i as u32; + let (set, dynamic_offsets) = set.as_ref(); // VUID-vkCmdBindDescriptorSets-commonparent - assert_eq!(self.device(), set.as_ref().0.device()); + assert_eq!(self.device(), set.device()); - let pipeline_layout_set = &pipeline_layout.set_layouts()[set_num as usize]; + let set_layout = set.layout(); + let pipeline_set_layout = &pipeline_layout.set_layouts()[set_num as usize]; // VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358 - if !pipeline_layout_set.is_compatible_with(set.as_ref().0.layout()) { + if !pipeline_set_layout.is_compatible_with(set_layout) { return Err(BindPushError::DescriptorSetNotCompatible { set_num }); } - // TODO: see https://github.com/vulkano-rs/vulkano/issues/1643 - // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01971 - // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01972 - // VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979 - // VUID-vkCmdBindDescriptorSets-pDescriptorSets-06715 + let mut dynamic_offsets_remaining = dynamic_offsets; + let mut required_dynamic_offset_count = 0; + + for (&binding_num, binding) in set_layout.bindings() { + let required_alignment = match binding.descriptor_type { + DescriptorType::UniformBufferDynamic => uniform_alignment, + DescriptorType::StorageBufferDynamic => storage_alignment, + _ => continue, + }; + + let count = if binding.variable_descriptor_count { + set.variable_descriptor_count() + } else { + binding.descriptor_count + } as usize; + + required_dynamic_offset_count += count; + + if !dynamic_offsets_remaining.is_empty() { + let split_index = min(count, dynamic_offsets_remaining.len()); + let dynamic_offsets = &dynamic_offsets_remaining[..split_index]; + dynamic_offsets_remaining = &dynamic_offsets_remaining[split_index..]; + + let elements = match set.resources().binding(binding_num) { + Some(DescriptorBindingResources::Buffer(elements)) => elements.as_slice(), + _ => unreachable!(), + }; + + for (index, (&offset, element)) in + dynamic_offsets.iter().zip(elements).enumerate() + { + // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01971 + // VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01972 + if offset % required_alignment != 0 { + return Err(BindPushError::DynamicOffsetNotAligned { + set_num, + binding_num, + index: index as u32, + offset, + required_alignment, + }); + } + + if let Some((buffer, range)) = element { + // VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979 + if offset as DeviceSize + range.end > buffer.size() { + return Err(BindPushError::DynamicOffsetOutOfBufferBounds { + set_num, + binding_num, + index: index as u32, + offset, + range_end: range.end, + buffer_size: buffer.size(), + }); + } + } + } + } + } + + // VUID-vkCmdBindDescriptorSets-dynamicOffsetCount-00359 + if dynamic_offsets.len() != required_dynamic_offset_count { + return Err(BindPushError::DynamicOffsetCountMismatch { + set_num, + provided_count: dynamic_offsets.len(), + required_count: required_dynamic_offset_count, + }); + } } Ok(()) diff --git a/vulkano/src/command_buffer/standard/builder/mod.rs b/vulkano/src/command_buffer/standard/builder/mod.rs index 4533c3dd7..a8268bdef 100644 --- a/vulkano/src/command_buffer/standard/builder/mod.rs +++ b/vulkano/src/command_buffer/standard/builder/mod.rs @@ -1124,6 +1124,14 @@ impl SetOrPush { Self::Push(resources) => resources, } } + + #[inline] + pub fn dynamic_offsets(&self) -> &[u32] { + match self { + Self::Set(set) => set.as_ref().1, + Self::Push(_) => &[], + } + } } #[derive(Clone, Copy, Debug, Default)] diff --git a/vulkano/src/command_buffer/standard/builder/pipeline.rs b/vulkano/src/command_buffer/standard/builder/pipeline.rs index c33339b7e..45f125742 100644 --- a/vulkano/src/command_buffer/standard/builder/pipeline.rs +++ b/vulkano/src/command_buffer/standard/builder/pipeline.rs @@ -37,10 +37,10 @@ use crate::{ ShaderStages, }, sync::PipelineStageAccess, - RequiresOneOf, VulkanObject, + RequiresOneOf, VulkanObject, DeviceSize, }; use ahash::HashMap; -use std::{cmp::min, mem::size_of, sync::Arc}; +use std::{cmp::min, mem::size_of, sync::Arc, ops::Range}; impl CommandBufferBuilder where @@ -995,7 +995,7 @@ where let layout_binding = &pipeline.layout().set_layouts()[set_num as usize].bindings()[&binding_num]; - let check_buffer = |_index: u32, _buffer: &Arc| Ok(()); + let check_buffer = |_index: u32, (_buffer, _range): &(Arc, Range)| Ok(()); let check_buffer_view = |index: u32, buffer_view: &Arc| { for desc_reqs in (binding_reqs.descriptors.get(&Some(index)).into_iter()) @@ -2058,29 +2058,56 @@ fn record_descriptor_sets_access( (use_ref, stage_access_iter) }; - match descriptor_sets_state.descriptor_sets[&set] + let descriptor_set_state = &descriptor_sets_state.descriptor_sets[&set]; + + match descriptor_set_state .resources() .binding(binding) .unwrap() { DescriptorBindingResources::None(_) => continue, DescriptorBindingResources::Buffer(elements) => { - for (index, element) in elements.iter().enumerate() { - if let Some(buffer) = element { - let buffer_inner = buffer.inner(); - let (use_ref, stage_access_iter) = use_iter(index as u32); + if matches!(descriptor_type, DescriptorType::UniformBufferDynamic | DescriptorType::StorageBufferDynamic) { + let dynamic_offsets = descriptor_set_state.dynamic_offsets(); - let mut range = 0..buffer.size(); // TODO: - range.start += buffer_inner.offset; - range.end += buffer_inner.offset; + for (index, element) in elements.iter().enumerate() { + if let Some((buffer, range)) = element { + let buffer_inner = buffer.inner(); + let dynamic_offset = dynamic_offsets[index] as DeviceSize; + let (use_ref, stage_access_iter) = use_iter(index as u32); - for stage_access in stage_access_iter { - resources_usage_state.record_buffer_access( - &use_ref, - buffer_inner.buffer, - range.clone(), - stage_access, - ); + let mut range = range.clone(); + range.start += buffer_inner.offset + dynamic_offset; + range.end += buffer_inner.offset + dynamic_offset; + + for stage_access in stage_access_iter { + resources_usage_state.record_buffer_access( + &use_ref, + buffer_inner.buffer, + range.clone(), + stage_access, + ); + } + } + } + } else { + for (index, element) in elements.iter().enumerate() { + if let Some((buffer, range)) = element { + let buffer_inner = buffer.inner(); + let (use_ref, stage_access_iter) = use_iter(index as u32); + + let mut range = range.clone(); + range.start += buffer_inner.offset; + range.end += buffer_inner.offset; + + for stage_access in stage_access_iter { + resources_usage_state.record_buffer_access( + &use_ref, + buffer_inner.buffer, + range.clone(), + stage_access, + ); + } } } } diff --git a/vulkano/src/command_buffer/synced/builder.rs b/vulkano/src/command_buffer/synced/builder.rs index d09f70415..e88d3582a 100644 --- a/vulkano/src/command_buffer/synced/builder.rs +++ b/vulkano/src/command_buffer/synced/builder.rs @@ -1137,6 +1137,14 @@ impl SetOrPush { Self::Push(resources) => resources, } } + + #[inline] + pub fn dynamic_offsets(&self) -> &[u32] { + match self { + Self::Set(set) => set.as_ref().1, + Self::Push(_) => &[], + } + } } /// Allows you to retrieve the current state of a command buffer builder. diff --git a/vulkano/src/descriptor_set/mod.rs b/vulkano/src/descriptor_set/mod.rs index e468c02b4..9f930e33e 100644 --- a/vulkano/src/descriptor_set/mod.rs +++ b/vulkano/src/descriptor_set/mod.rs @@ -91,7 +91,7 @@ use crate::{ device::DeviceOwned, image::view::ImageViewAbstract, sampler::Sampler, - OomError, VulkanObject, + DeviceSize, OomError, VulkanObject, }; use ahash::HashMap; use smallvec::{smallvec, SmallVec}; @@ -99,6 +99,7 @@ use std::{ error::Error, fmt::{Display, Error as FmtError, Formatter}, hash::{Hash, Hasher}, + ops::Range, ptr, sync::Arc, }; @@ -121,6 +122,9 @@ pub unsafe trait DescriptorSet: DeviceOwned + Send + Sync { /// Returns the layout of this descriptor set. fn layout(&self) -> &Arc; + /// Returns the variable descriptor count that this descriptor set was allocated with. + fn variable_descriptor_count(&self) -> u32; + /// Creates a [`DescriptorSetWithOffsets`] with the given dynamic offsets. fn offsets( self: Arc, @@ -153,6 +157,7 @@ impl Hash for dyn DescriptorSet { pub(crate) struct DescriptorSetInner { layout: Arc, + variable_descriptor_count: u32, resources: DescriptorSetResources, } @@ -229,7 +234,11 @@ impl DescriptorSetInner { ); } - Ok(DescriptorSetInner { layout, resources }) + Ok(DescriptorSetInner { + layout, + variable_descriptor_count, + resources, + }) } pub(crate) fn layout(&self) -> &Arc { @@ -333,7 +342,7 @@ impl DescriptorSetResources { #[derive(Clone)] pub enum DescriptorBindingResources { None(Elements<()>), - Buffer(Elements>), + Buffer(Elements<(Arc, Range)>), BufferView(Elements>), ImageView(Elements>), ImageViewSampler(Elements<(Arc, Arc)>), @@ -416,64 +425,9 @@ impl DescriptorSetWithOffsets { descriptor_set: Arc, dynamic_offsets: impl IntoIterator, ) -> Self { - let dynamic_offsets: SmallVec<_> = dynamic_offsets.into_iter().collect(); - let layout = descriptor_set.layout(); - let properties = layout.device().physical_device().properties(); - let min_uniform_off_align = properties.min_uniform_buffer_offset_alignment as u32; - let min_storage_off_align = properties.min_storage_buffer_offset_alignment as u32; - let mut dynamic_offset_index = 0; - - // Ensure that the number of dynamic_offsets is correct and that each - // dynamic offset is a multiple of the minimum offset alignment specified - // by the physical device. - for binding in layout.bindings().values() { - match binding.descriptor_type { - DescriptorType::StorageBufferDynamic => { - // Don't check alignment if there are not enough offsets anyway - if dynamic_offsets.len() > dynamic_offset_index { - assert!( - dynamic_offsets[dynamic_offset_index] % min_storage_off_align == 0, - "Dynamic storage buffer offset must be a multiple of \ - min_storage_buffer_offset_alignment: got {}, expected a multiple of {}", - dynamic_offsets[dynamic_offset_index], - min_storage_off_align, - ); - } - dynamic_offset_index += 1; - } - DescriptorType::UniformBufferDynamic => { - // Don't check alignment if there are not enough offsets anyway - if dynamic_offsets.len() > dynamic_offset_index { - assert!( - dynamic_offsets[dynamic_offset_index] % min_uniform_off_align == 0, - "Dynamic uniform buffer offset must be a multiple of \ - min_uniform_buffer_offset_alignment: got {}, expected a multiple of {}", - dynamic_offsets[dynamic_offset_index], - min_uniform_off_align, - ); - } - dynamic_offset_index += 1; - } - _ => (), - } - } - - assert!( - dynamic_offsets.len() >= dynamic_offset_index, - "Too few dynamic offsets: got {}, expected {}", - dynamic_offsets.len(), - dynamic_offset_index, - ); - assert!( - dynamic_offsets.len() <= dynamic_offset_index, - "Too many dynamic offsets: got {}, expected {}", - dynamic_offsets.len(), - dynamic_offset_index, - ); - - DescriptorSetWithOffsets { + Self { descriptor_set, - dynamic_offsets, + dynamic_offsets: dynamic_offsets.into_iter().collect(), } } diff --git a/vulkano/src/descriptor_set/persistent.rs b/vulkano/src/descriptor_set/persistent.rs index b16d401b3..cfa3f0768 100644 --- a/vulkano/src/descriptor_set/persistent.rs +++ b/vulkano/src/descriptor_set/persistent.rs @@ -114,6 +114,10 @@ where self.inner.layout() } + fn variable_descriptor_count(&self) -> u32 { + self.inner.variable_descriptor_count + } + fn resources(&self) -> &DescriptorSetResources { self.inner.resources() } diff --git a/vulkano/src/descriptor_set/update.rs b/vulkano/src/descriptor_set/update.rs index c4aa6b1a3..6b6d06b2d 100644 --- a/vulkano/src/descriptor_set/update.rs +++ b/vulkano/src/descriptor_set/update.rs @@ -19,6 +19,7 @@ use smallvec::SmallVec; use std::{ error::Error, fmt::{Display, Error as FmtError, Formatter}, + ops::Range, ptr, sync::Arc, }; @@ -41,7 +42,12 @@ pub struct WriteDescriptorSet { impl WriteDescriptorSet { /// Write an empty element to array element 0. /// - /// See `none_array` for more information. + /// This is used for push descriptors in combination with `Sampler` descriptors that have + /// immutable samplers in the layout. The Vulkan spec requires these elements to be explicitly + /// written, but since there is no data to write, a dummy write is provided instead. + /// + /// For regular descriptor sets, the data for such descriptors is automatically valid, and dummy + /// writes are not allowed. #[inline] pub fn none(binding: u32) -> Self { Self::none_array(binding, 0, 1) @@ -49,12 +55,7 @@ impl WriteDescriptorSet { /// Write a number of consecutive empty elements. /// - /// This is used for push descriptors in combination with `Sampler` descriptors that have - /// immutable samplers in the layout. The Vulkan spec requires these elements to be explicitly - /// written, but since there is no data to write, a dummy write is provided instead. - /// - /// For regular descriptor sets, the data for such descriptors is automatically valid, and dummy - /// writes are not allowed. + /// See [`none`](Self::none) for more information. #[inline] pub fn none_array(binding: u32, first_array_element: u32, num_elements: u32) -> Self { assert!(num_elements != 0); @@ -65,17 +66,63 @@ impl WriteDescriptorSet { } } - /// Write a single buffer to array element 0. + /// Write a single buffer to array element 0, with the bound range covering the whole buffer. + /// + /// For dynamic buffer bindings, this will bind the whole buffer, and only a dynamic offset + /// of zero will be valid, which is probably not what you want. + /// Use [`buffer_with_range`](Self::buffer_with_range) instead. #[inline] pub fn buffer(binding: u32, buffer: Arc) -> Self { - Self::buffer_array(binding, 0, [buffer]) + let range = 0..buffer.size(); + Self::buffer_with_range_array(binding, 0, [(buffer, range)]) } /// Write a number of consecutive buffer elements. + /// + /// See [`buffer`](Self::buffer) for more information. + #[inline] pub fn buffer_array( binding: u32, first_array_element: u32, elements: impl IntoIterator>, + ) -> Self { + Self::buffer_with_range_array( + binding, + first_array_element, + elements.into_iter().map(|buffer| { + let range = 0..buffer.size(); + (buffer, range) + }), + ) + } + + /// Write a single buffer to array element 0, specifying the range of the buffer to be bound. + /// + /// `range` is the slice of bytes in `buffer` that will be made available to the shader. + /// `range` must not be outside the range `buffer`. + /// + /// For dynamic buffer bindings, `range` specifies the slice that is to be bound if the + /// dynamic offset were zero. When binding the descriptor set, the effective value of `range` + /// shifts forward by the offset that was provided. For example, if `range` is specified as + /// `0..8` when writing the descriptor set, and then when binding the descriptor set the + /// offset `16` is used, then the range of `buffer` that will actually be bound is `16..24`. + #[inline] + pub fn buffer_with_range( + binding: u32, + buffer: Arc, + range: Range, + ) -> Self { + Self::buffer_with_range_array(binding, 0, [(buffer, range)]) + } + + /// Write a number of consecutive buffer elements, specifying the ranges of the buffers to be + /// bound. + /// + /// See [`buffer_with_range`](Self::buffer_with_range) for more information. + pub fn buffer_with_range_array( + binding: u32, + first_array_element: u32, + elements: impl IntoIterator, Range)>, ) -> Self { let elements: SmallVec<_> = elements.into_iter().collect(); assert!(!elements.is_empty()); @@ -217,31 +264,16 @@ impl WriteDescriptorSet { DescriptorWriteInfo::Buffer( elements .iter() - .map(|buffer| { - let size = buffer.size(); + .map(|(buffer, range)| { let BufferInner { buffer, offset } = buffer.inner(); - debug_assert_eq!( - offset - % buffer - .device() - .physical_device() - .properties() - .min_storage_buffer_offset_alignment, - 0 - ); - debug_assert!( - size <= buffer - .device() - .physical_device() - .properties() - .max_storage_buffer_range - as DeviceSize - ); + debug_assert!(!range.is_empty()); + debug_assert!(range.end <= buffer.size()); + ash::vk::DescriptorBufferInfo { buffer: buffer.handle(), - offset, - range: size, + offset: offset + range.start, + range: range.end - range.start, } }) .collect(), @@ -343,7 +375,7 @@ impl WriteDescriptorSet { /// The elements held by a `WriteDescriptorSet`. pub enum WriteDescriptorSetElements { None(u32), - Buffer(SmallVec<[Arc; 1]>), + Buffer(SmallVec<[(Arc, Range); 1]>), BufferView(SmallVec<[Arc; 1]>), ImageView(SmallVec<[Arc; 1]>), ImageViewSampler(SmallVec<[(Arc, Arc); 1]>), @@ -377,6 +409,17 @@ pub(crate) fn check_descriptor_write<'a>( layout: &'a DescriptorSetLayout, variable_descriptor_count: u32, ) -> Result<&'a DescriptorSetLayoutBinding, DescriptorSetUpdateError> { + fn provided_element_type(elements: &WriteDescriptorSetElements) -> &'static str { + match elements { + WriteDescriptorSetElements::None(_) => "none", + WriteDescriptorSetElements::Buffer(_) => "buffer", + WriteDescriptorSetElements::BufferView(_) => "buffer_view", + WriteDescriptorSetElements::ImageView(_) => "image_view", + WriteDescriptorSetElements::ImageViewSampler(_) => "image_view_sampler", + WriteDescriptorSetElements::Sampler(_) => "sampler", + } + } + let device = layout.device(); let layout_binding = match layout.bindings().get(&write.binding()) { @@ -394,6 +437,7 @@ pub(crate) fn check_descriptor_write<'a>( layout_binding.descriptor_count }; + let binding = write.binding(); let elements = write.elements(); let num_elements = elements.len(); debug_assert!(num_elements != 0); @@ -403,353 +447,68 @@ pub(crate) fn check_descriptor_write<'a>( if descriptor_range_end > max_descriptor_count { return Err(DescriptorSetUpdateError::ArrayIndexOutOfBounds { - binding: write.binding(), + binding, available_count: max_descriptor_count, written_count: descriptor_range_end, }); } - match elements { - WriteDescriptorSetElements::None(_num_elements) => match layout_binding.descriptor_type { - DescriptorType::Sampler - if layout.push_descriptor() && !layout_binding.immutable_samplers.is_empty() => {} - _ => { - return Err(DescriptorSetUpdateError::IncompatibleDescriptorType { - binding: write.binding(), - }) - } - }, - WriteDescriptorSetElements::Buffer(elements) => { - match layout_binding.descriptor_type { - DescriptorType::StorageBuffer | DescriptorType::StorageBufferDynamic => { - for (index, buffer) in elements.iter().enumerate() { - assert_eq!(device, buffer.device()); + match layout_binding.descriptor_type { + DescriptorType::Sampler => { + if layout_binding.immutable_samplers.is_empty() { + let elements = if let WriteDescriptorSetElements::Sampler(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["sampler"], + }); + }; - if !buffer.usage().intersects(BufferUsage::STORAGE_BUFFER) { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "storage_buffer", - }); - } - } - } - DescriptorType::UniformBuffer | DescriptorType::UniformBufferDynamic => { - for (index, buffer) in elements.iter().enumerate() { - assert_eq!(device, buffer.device()); + for (index, sampler) in elements.iter().enumerate() { + assert_eq!(device, sampler.device()); - if !buffer.usage().intersects(BufferUsage::UNIFORM_BUFFER) { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "uniform_buffer", - }); - } - } - } - _ => { - return Err(DescriptorSetUpdateError::IncompatibleDescriptorType { - binding: write.binding(), - }) - } - } - - // Note that the buffer content is not checked. This is technically not unsafe as - // long as the data in the buffer has no invalid memory representation (ie. no - // bool, no enum, no pointer, no str) and as long as the robust buffer access - // feature is enabled. - // TODO: this is not checked ^ - - // TODO: eventually shouldn't be an assert ; for now robust_buffer_access is always - // enabled so this assert should never fail in practice, but we put it anyway - // in case we forget to adjust this code - assert!(device.enabled_features().robust_buffer_access); - } - WriteDescriptorSetElements::BufferView(elements) => { - match layout_binding.descriptor_type { - DescriptorType::StorageTexelBuffer => { - for (index, buffer_view) in elements.iter().enumerate() { - assert_eq!(device, buffer_view.device()); - - // TODO: storage_texel_buffer_atomic - if !buffer_view - .buffer() - .usage() - .intersects(BufferUsage::STORAGE_TEXEL_BUFFER) - { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "storage_texel_buffer", - }); - } - } - } - DescriptorType::UniformTexelBuffer => { - for (index, buffer_view) in elements.iter().enumerate() { - assert_eq!(device, buffer_view.device()); - - if !buffer_view - .buffer() - .usage() - .intersects(BufferUsage::UNIFORM_TEXEL_BUFFER) - { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "uniform_texel_buffer", - }); - } - } - } - _ => { - return Err(DescriptorSetUpdateError::IncompatibleDescriptorType { - binding: write.binding(), - }) - } - } - } - WriteDescriptorSetElements::ImageView(elements) => match layout_binding.descriptor_type { - DescriptorType::CombinedImageSampler - if !layout_binding.immutable_samplers.is_empty() => - { - let immutable_samplers = &layout_binding.immutable_samplers - [descriptor_range_start as usize..descriptor_range_end as usize]; - - for (index, (image_view, sampler)) in - elements.iter().zip(immutable_samplers).enumerate() - { - assert_eq!(device, image_view.device()); - - // VUID-VkWriteDescriptorSet-descriptorType-00337 - if !image_view.usage().intersects(ImageUsage::SAMPLED) { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "sampled", - }); - } - - // VUID-VkDescriptorImageInfo-imageView-00343 - if matches!( - image_view.view_type(), - ImageViewType::Dim2d | ImageViewType::Dim2dArray - ) && image_view.image().inner().image.dimensions().image_type() - == ImageType::Dim3d - { - return Err(DescriptorSetUpdateError::ImageView2dFrom3d { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID-VkDescriptorImageInfo-imageView-01976 - if image_view - .subresource_range() - .aspects - .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) - { - return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - if let Err(error) = sampler.check_can_sample(image_view.as_ref()) { - return Err(DescriptorSetUpdateError::ImageViewIncompatibleSampler { - binding: write.binding(), - index: descriptor_range_start + index as u32, - error, - }); - } - } - } - DescriptorType::SampledImage => { - for (index, image_view) in elements.iter().enumerate() { - assert_eq!(device, image_view.device()); - - // VUID-VkWriteDescriptorSet-descriptorType-00337 - if !image_view.usage().intersects(ImageUsage::SAMPLED) { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "sampled", - }); - } - - // VUID-VkDescriptorImageInfo-imageView-00343 - if matches!( - image_view.view_type(), - ImageViewType::Dim2d | ImageViewType::Dim2dArray - ) && image_view.image().inner().image.dimensions().image_type() - == ImageType::Dim3d - { - return Err(DescriptorSetUpdateError::ImageView2dFrom3d { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID-VkDescriptorImageInfo-imageView-01976 - if image_view - .subresource_range() - .aspects - .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) - { - return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID-VkWriteDescriptorSet-descriptorType-01946 - if image_view.sampler_ycbcr_conversion().is_some() { - return Err( - DescriptorSetUpdateError::ImageViewHasSamplerYcbcrConversion { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }, - ); - } - } - } - DescriptorType::StorageImage => { - for (index, image_view) in elements.iter().enumerate() { - assert_eq!(device, image_view.device()); - - // VUID-VkWriteDescriptorSet-descriptorType-00339 - if !image_view.usage().intersects(ImageUsage::STORAGE) { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "storage", - }); - } - - // VUID-VkDescriptorImageInfo-imageView-00343 - if matches!( - image_view.view_type(), - ImageViewType::Dim2d | ImageViewType::Dim2dArray - ) && image_view.image().inner().image.dimensions().image_type() - == ImageType::Dim3d - { - return Err(DescriptorSetUpdateError::ImageView2dFrom3d { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID-VkDescriptorImageInfo-imageView-01976 - if image_view - .subresource_range() - .aspects - .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) - { - return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID-VkWriteDescriptorSet-descriptorType-00336 - if !image_view.component_mapping().is_identity() { - return Err(DescriptorSetUpdateError::ImageViewNotIdentitySwizzled { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID?? - if image_view.sampler_ycbcr_conversion().is_some() { - return Err( - DescriptorSetUpdateError::ImageViewHasSamplerYcbcrConversion { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }, - ); - } - } - } - DescriptorType::InputAttachment => { - for (index, image_view) in elements.iter().enumerate() { - assert_eq!(device, image_view.device()); - - // VUID-VkWriteDescriptorSet-descriptorType-00338 - if !image_view.usage().intersects(ImageUsage::INPUT_ATTACHMENT) { - return Err(DescriptorSetUpdateError::MissingUsage { - binding: write.binding(), - index: descriptor_range_start + index as u32, - usage: "input_attachment", - }); - } - - // VUID-VkDescriptorImageInfo-imageView-00343 - if matches!( - image_view.view_type(), - ImageViewType::Dim2d | ImageViewType::Dim2dArray - ) && image_view.image().inner().image.dimensions().image_type() - == ImageType::Dim3d - { - return Err(DescriptorSetUpdateError::ImageView2dFrom3d { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID-VkDescriptorImageInfo-imageView-01976 - if image_view - .subresource_range() - .aspects - .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) - { - return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID-VkWriteDescriptorSet-descriptorType-00336 - if !image_view.component_mapping().is_identity() { - return Err(DescriptorSetUpdateError::ImageViewNotIdentitySwizzled { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }); - } - - // VUID?? - if image_view.sampler_ycbcr_conversion().is_some() { - return Err( - DescriptorSetUpdateError::ImageViewHasSamplerYcbcrConversion { - binding: write.binding(), - index: descriptor_range_start + index as u32, - }, - ); - } - - // VUID?? - if image_view.view_type().is_arrayed() { - return Err(DescriptorSetUpdateError::ImageViewIsArrayed { + if sampler.sampler_ycbcr_conversion().is_some() { + return Err(DescriptorSetUpdateError::SamplerHasSamplerYcbcrConversion { binding: write.binding(), index: descriptor_range_start + index as u32, }); } } - } - _ => { - return Err(DescriptorSetUpdateError::IncompatibleDescriptorType { - binding: write.binding(), - }) - } - }, - WriteDescriptorSetElements::ImageViewSampler(elements) => match layout_binding - .descriptor_type - { - DescriptorType::CombinedImageSampler => { - if !layout_binding.immutable_samplers.is_empty() { - return Err(DescriptorSetUpdateError::SamplerIsImmutable { - binding: write.binding(), + } else if layout.push_descriptor() { + // For push descriptors, we must write a dummy element. + if let WriteDescriptorSetElements::None(_) = elements { + // Do nothing + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["none"], }); } + } else { + // For regular descriptors, no element must be written. + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &[], + }); + } + } + + DescriptorType::CombinedImageSampler => { + if layout_binding.immutable_samplers.is_empty() { + let elements = + if let WriteDescriptorSetElements::ImageViewSampler(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["image_view_sampler"], + }); + }; for (index, (image_view, sampler)) in elements.iter().enumerate() { assert_eq!(device, image_view.device()); @@ -830,38 +589,400 @@ pub(crate) fn check_descriptor_write<'a>( }); } } - } - _ => { - return Err(DescriptorSetUpdateError::IncompatibleDescriptorType { - binding: write.binding(), - }) - } - }, - WriteDescriptorSetElements::Sampler(elements) => match layout_binding.descriptor_type { - DescriptorType::Sampler => { - if !layout_binding.immutable_samplers.is_empty() { - return Err(DescriptorSetUpdateError::SamplerIsImmutable { - binding: write.binding(), + } else { + let elements = if let WriteDescriptorSetElements::ImageView(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["image_view"], }); - } + }; - for (index, sampler) in elements.iter().enumerate() { - assert_eq!(device, sampler.device()); + let immutable_samplers = &layout_binding.immutable_samplers + [descriptor_range_start as usize..descriptor_range_end as usize]; - if sampler.sampler_ycbcr_conversion().is_some() { - return Err(DescriptorSetUpdateError::SamplerHasSamplerYcbcrConversion { + for (index, (image_view, sampler)) in + elements.iter().zip(immutable_samplers).enumerate() + { + assert_eq!(device, image_view.device()); + + // VUID-VkWriteDescriptorSet-descriptorType-00337 + if !image_view.usage().intersects(ImageUsage::SAMPLED) { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "sampled", + }); + } + + // VUID-VkDescriptorImageInfo-imageView-00343 + if matches!( + image_view.view_type(), + ImageViewType::Dim2d | ImageViewType::Dim2dArray + ) && image_view.image().inner().image.dimensions().image_type() + == ImageType::Dim3d + { + return Err(DescriptorSetUpdateError::ImageView2dFrom3d { binding: write.binding(), index: descriptor_range_start + index as u32, }); } + + // VUID-VkDescriptorImageInfo-imageView-01976 + if image_view + .subresource_range() + .aspects + .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) + { + return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + if let Err(error) = sampler.check_can_sample(image_view.as_ref()) { + return Err(DescriptorSetUpdateError::ImageViewIncompatibleSampler { + binding: write.binding(), + index: descriptor_range_start + index as u32, + error, + }); + } } } - _ => { - return Err(DescriptorSetUpdateError::IncompatibleDescriptorType { - binding: write.binding(), - }) + } + + DescriptorType::SampledImage => { + let elements = if let WriteDescriptorSetElements::ImageView(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["image_view"], + }); + }; + + for (index, image_view) in elements.iter().enumerate() { + assert_eq!(device, image_view.device()); + + // VUID-VkWriteDescriptorSet-descriptorType-00337 + if !image_view.usage().intersects(ImageUsage::SAMPLED) { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "sampled", + }); + } + + // VUID-VkDescriptorImageInfo-imageView-00343 + if matches!( + image_view.view_type(), + ImageViewType::Dim2d | ImageViewType::Dim2dArray + ) && image_view.image().inner().image.dimensions().image_type() + == ImageType::Dim3d + { + return Err(DescriptorSetUpdateError::ImageView2dFrom3d { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID-VkDescriptorImageInfo-imageView-01976 + if image_view + .subresource_range() + .aspects + .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) + { + return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID-VkWriteDescriptorSet-descriptorType-01946 + if image_view.sampler_ycbcr_conversion().is_some() { + return Err( + DescriptorSetUpdateError::ImageViewHasSamplerYcbcrConversion { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }, + ); + } } - }, + } + + DescriptorType::StorageImage => { + let elements = if let WriteDescriptorSetElements::ImageView(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["image_view"], + }); + }; + + for (index, image_view) in elements.iter().enumerate() { + assert_eq!(device, image_view.device()); + + // VUID-VkWriteDescriptorSet-descriptorType-00339 + if !image_view.usage().intersects(ImageUsage::STORAGE) { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "storage", + }); + } + + // VUID-VkDescriptorImageInfo-imageView-00343 + if matches!( + image_view.view_type(), + ImageViewType::Dim2d | ImageViewType::Dim2dArray + ) && image_view.image().inner().image.dimensions().image_type() + == ImageType::Dim3d + { + return Err(DescriptorSetUpdateError::ImageView2dFrom3d { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID-VkDescriptorImageInfo-imageView-01976 + if image_view + .subresource_range() + .aspects + .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) + { + return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID-VkWriteDescriptorSet-descriptorType-00336 + if !image_view.component_mapping().is_identity() { + return Err(DescriptorSetUpdateError::ImageViewNotIdentitySwizzled { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID?? + if image_view.sampler_ycbcr_conversion().is_some() { + return Err( + DescriptorSetUpdateError::ImageViewHasSamplerYcbcrConversion { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }, + ); + } + } + } + + DescriptorType::UniformTexelBuffer => { + let elements = if let WriteDescriptorSetElements::BufferView(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["buffer_view"], + }); + }; + + for (index, buffer_view) in elements.iter().enumerate() { + assert_eq!(device, buffer_view.device()); + + if !buffer_view + .buffer() + .usage() + .intersects(BufferUsage::UNIFORM_TEXEL_BUFFER) + { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "uniform_texel_buffer", + }); + } + } + } + + DescriptorType::StorageTexelBuffer => { + let elements = if let WriteDescriptorSetElements::BufferView(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["buffer_view"], + }); + }; + + for (index, buffer_view) in elements.iter().enumerate() { + assert_eq!(device, buffer_view.device()); + + // TODO: storage_texel_buffer_atomic + if !buffer_view + .buffer() + .usage() + .intersects(BufferUsage::STORAGE_TEXEL_BUFFER) + { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "storage_texel_buffer", + }); + } + } + } + + DescriptorType::UniformBuffer | DescriptorType::UniformBufferDynamic => { + let elements = if let WriteDescriptorSetElements::Buffer(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["buffer"], + }); + }; + + for (index, (buffer, range)) in elements.iter().enumerate() { + assert_eq!(device, buffer.device()); + + if !buffer.usage().intersects(BufferUsage::UNIFORM_BUFFER) { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "uniform_buffer", + }); + } + + assert!(!range.is_empty()); + + if range.end > buffer.size() { + return Err(DescriptorSetUpdateError::RangeOutOfBufferBounds { + binding: write.binding(), + index: descriptor_range_start + index as u32, + range_end: range.end, + buffer_size: buffer.size(), + }); + } + } + } + + DescriptorType::StorageBuffer | DescriptorType::StorageBufferDynamic => { + let elements = if let WriteDescriptorSetElements::Buffer(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["buffer"], + }); + }; + + for (index, (buffer, range)) in elements.iter().enumerate() { + assert_eq!(device, buffer.device()); + + if !buffer.usage().intersects(BufferUsage::STORAGE_BUFFER) { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "storage_buffer", + }); + } + + assert!(!range.is_empty()); + + if range.end > buffer.size() { + return Err(DescriptorSetUpdateError::RangeOutOfBufferBounds { + binding: write.binding(), + index: descriptor_range_start + index as u32, + range_end: range.end, + buffer_size: buffer.size(), + }); + } + } + } + + DescriptorType::InputAttachment => { + let elements = if let WriteDescriptorSetElements::ImageView(elements) = elements { + elements + } else { + return Err(DescriptorSetUpdateError::IncompatibleElementType { + binding, + provided_element_type: provided_element_type(elements), + allowed_element_types: &["image_view"], + }); + }; + + for (index, image_view) in elements.iter().enumerate() { + assert_eq!(device, image_view.device()); + + // VUID-VkWriteDescriptorSet-descriptorType-00338 + if !image_view.usage().intersects(ImageUsage::INPUT_ATTACHMENT) { + return Err(DescriptorSetUpdateError::MissingUsage { + binding: write.binding(), + index: descriptor_range_start + index as u32, + usage: "input_attachment", + }); + } + + // VUID-VkDescriptorImageInfo-imageView-00343 + if matches!( + image_view.view_type(), + ImageViewType::Dim2d | ImageViewType::Dim2dArray + ) && image_view.image().inner().image.dimensions().image_type() + == ImageType::Dim3d + { + return Err(DescriptorSetUpdateError::ImageView2dFrom3d { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID-VkDescriptorImageInfo-imageView-01976 + if image_view + .subresource_range() + .aspects + .contains(ImageAspects::DEPTH | ImageAspects::STENCIL) + { + return Err(DescriptorSetUpdateError::ImageViewDepthAndStencil { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID-VkWriteDescriptorSet-descriptorType-00336 + if !image_view.component_mapping().is_identity() { + return Err(DescriptorSetUpdateError::ImageViewNotIdentitySwizzled { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + + // VUID?? + if image_view.sampler_ycbcr_conversion().is_some() { + return Err( + DescriptorSetUpdateError::ImageViewHasSamplerYcbcrConversion { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }, + ); + } + + // VUID?? + if image_view.view_type().is_arrayed() { + return Err(DescriptorSetUpdateError::ImageViewIsArrayed { + binding: write.binding(), + index: descriptor_range_start + index as u32, + }); + } + } + } } Ok(layout_binding) @@ -914,7 +1035,11 @@ pub enum DescriptorSetUpdateError { /// Tried to write an element type that was not compatible with the descriptor type in the /// layout. - IncompatibleDescriptorType { binding: u32 }, + IncompatibleElementType { + binding: u32, + provided_element_type: &'static str, + allowed_element_types: &'static [&'static str], + }, /// Tried to write to a nonexistent binding. InvalidBinding { binding: u32 }, @@ -926,11 +1051,16 @@ pub enum DescriptorSetUpdateError { usage: &'static str, }, + /// The end of the provided `range` for a buffer is larger than the size of the buffer. + RangeOutOfBufferBounds { + binding: u32, + index: u32, + range_end: DeviceSize, + buffer_size: DeviceSize, + }, + /// Tried to write a sampler that has an attached sampler YCbCr conversion. SamplerHasSamplerYcbcrConversion { binding: u32, index: u32 }, - - /// Tried to write a sampler to a binding with immutable samplers. - SamplerIsImmutable { binding: u32 }, } impl Error for DescriptorSetUpdateError { @@ -1002,12 +1132,31 @@ impl Display for DescriptorSetUpdateError { but this binding has a descriptor type that requires it to be identity swizzled", binding, index, ), - Self::IncompatibleDescriptorType { binding } => write!( - f, - "tried to write a resource to binding {} whose type was not compatible with the \ - descriptor type", + Self::IncompatibleElementType { binding, - ), + provided_element_type, + allowed_element_types, + } => write!( + f, + "tried to write a resource to binding {} whose type ({}) was not one of the \ + resource types allowed for the descriptor type (", + binding, provided_element_type, + ) + .and_then(|_| { + let mut first = true; + + for elem_type in *allowed_element_types { + if first { + write!(f, "{}", elem_type)?; + first = false; + } else { + write!(f, ", {}", elem_type)?; + } + } + + Ok(()) + }) + .and_then(|_| write!(f, ") that can be bound to this buffer")), Self::InvalidBinding { binding } => { write!(f, "tried to write to a nonexistent binding {}", binding,) } @@ -1021,18 +1170,23 @@ impl Display for DescriptorSetUpdateError { usage {} enabled", binding, index, usage, ), + Self::RangeOutOfBufferBounds { + binding, + index, + range_end, + buffer_size, + } => write!( + f, + "the end of the provided `range` for the buffer at binding {} index {} ({:?}) is + larger than the size of the buffer ({})", + binding, index, range_end, buffer_size, + ), Self::SamplerHasSamplerYcbcrConversion { binding, index } => write!( f, "tried to write a sampler to binding {} index {} that has an attached sampler \ YCbCr conversion", binding, index, ), - Self::SamplerIsImmutable { binding } => write!( - f, - "tried to write a sampler to binding {}, which already contains immutable samplers \ - in the descriptor set layout", - binding, - ), } } }