From 9a50d6ca5637663d4cd473c61039aba1ee718da5 Mon Sep 17 00:00:00 2001 From: Rua Date: Thu, 22 Sep 2022 09:11:30 +0200 Subject: [PATCH] Fix handling of runtime sized arrays in shaders (#1992) * Fix handling of runtime sized arrays in shaders * Small fix --- vulkano-shaders/src/entry_point.rs | 4 ++++ vulkano/src/buffer/cpu_pool.rs | 2 +- .../src/command_buffer/commands/pipeline.rs | 23 +++++++++++++++---- vulkano/src/descriptor_set/layout.rs | 22 ++++++++++-------- vulkano/src/shader/mod.rs | 5 +++- vulkano/src/shader/reflect.rs | 9 +++++--- 6 files changed, 46 insertions(+), 19 deletions(-) diff --git a/vulkano-shaders/src/entry_point.rs b/vulkano-shaders/src/entry_point.rs index b199f32e..463786bb 100644 --- a/vulkano-shaders/src/entry_point.rs +++ b/vulkano-shaders/src/entry_point.rs @@ -110,6 +110,10 @@ fn write_descriptor_requirements( let ident = format_ident!("{}", format!("{:?}", ty)); quote! { ::vulkano::descriptor_set::layout::DescriptorType::#ident } }); + let descriptor_count = match descriptor_count { + Some(descriptor_count) => quote! { Some(#descriptor_count) }, + None => quote! { None }, + }; let image_format = match image_format { Some(image_format) => { let ident = format_ident!("{}", format!("{:?}", image_format)); diff --git a/vulkano/src/buffer/cpu_pool.rs b/vulkano/src/buffer/cpu_pool.rs index 440ea3cc..b4bec3ba 100644 --- a/vulkano/src/buffer/cpu_pool.rs +++ b/vulkano/src/buffer/cpu_pool.rs @@ -196,7 +196,7 @@ where #[inline] pub fn new(device: Arc, usage: BufferUsage) -> CpuBufferPool { assert!(size_of::() > 0); - let pool = device.standard_memory_pool().clone(); + let pool = device.standard_memory_pool(); CpuBufferPool { device, diff --git a/vulkano/src/command_buffer/commands/pipeline.rs b/vulkano/src/command_buffer/commands/pipeline.rs index 7e145643..b3a8734b 100644 --- a/vulkano/src/command_buffer/commands/pipeline.rs +++ b/vulkano/src/command_buffer/commands/pipeline.rs @@ -573,10 +573,25 @@ impl AutoCommandBufferBuilder { elements: &[Option], mut extra_check: impl FnMut(u32, &T) -> Result<(), DescriptorResourceInvalidError>, ) -> Result<(), PipelineExecutionError> { - for (index, element) in elements[0..reqs.descriptor_count as usize] - .iter() - .enumerate() - { + let elements_to_check = if let Some(descriptor_count) = reqs.descriptor_count { + // The shader has a fixed-sized array, so it will never access more than + // the first `descriptor_count` elements. + elements.get(..descriptor_count as usize).ok_or({ + // There are less than `descriptor_count` elements in `elements` + PipelineExecutionError::DescriptorResourceInvalid { + set_num, + binding_num, + index: elements.len() as u32, + error: DescriptorResourceInvalidError::Missing, + } + })? + } else { + // The shader has a runtime-sized array, so any element could potentially + // be accessed. We must check them all. + elements + }; + + for (index, element) in elements_to_check.iter().enumerate() { let index = index as u32; // VUID-vkCmdDispatch-None-02699 diff --git a/vulkano/src/descriptor_set/layout.rs b/vulkano/src/descriptor_set/layout.rs index e7dde816..63ff496b 100644 --- a/vulkano/src/descriptor_set/layout.rs +++ b/vulkano/src/descriptor_set/layout.rs @@ -722,8 +722,8 @@ impl DescriptorSetLayoutBinding { &self, descriptor_requirements: &DescriptorRequirements, ) -> Result<(), DescriptorRequirementsNotMet> { - let DescriptorRequirements { - descriptor_types, + let &DescriptorRequirements { + ref descriptor_types, descriptor_count, image_format: _, image_multisampled: _, @@ -746,16 +746,18 @@ impl DescriptorSetLayoutBinding { }); } - if self.descriptor_count < *descriptor_count { - return Err(DescriptorRequirementsNotMet::DescriptorCount { - required: *descriptor_count, - obtained: self.descriptor_count, - }); + if let Some(required) = descriptor_count { + if self.descriptor_count < required { + return Err(DescriptorRequirementsNotMet::DescriptorCount { + required, + obtained: self.descriptor_count, + }); + } } - if !self.stages.contains(stages) { + if !self.stages.contains(&stages) { return Err(DescriptorRequirementsNotMet::ShaderStages { - required: *stages, + required: stages, obtained: self.stages, }); } @@ -768,7 +770,7 @@ impl From<&DescriptorRequirements> for DescriptorSetLayoutBinding { fn from(reqs: &DescriptorRequirements) -> Self { Self { descriptor_type: reqs.descriptor_types[0], - descriptor_count: reqs.descriptor_count, + descriptor_count: reqs.descriptor_count.unwrap_or(0), variable_descriptor_count: false, stages: reqs.stages, immutable_samplers: Vec::new(), diff --git a/vulkano/src/shader/mod.rs b/vulkano/src/shader/mod.rs index 6bc3c31b..b5407d4c 100644 --- a/vulkano/src/shader/mod.rs +++ b/vulkano/src/shader/mod.rs @@ -542,7 +542,10 @@ pub struct DescriptorRequirements { /// The number of descriptors (array elements) that the shader requires. The descriptor set /// layout can declare more than this, but never less. - pub descriptor_count: u32, + /// + /// `None` means that the shader declares this as a runtime-sized array, and could potentially + /// access every array element provided in the descriptor set. + pub descriptor_count: Option, /// The image format that is required for image views bound to this descriptor. If this is /// `None`, then any image format is allowed. diff --git a/vulkano/src/shader/reflect.rs b/vulkano/src/shader/reflect.rs index e523deb5..bd4543d8 100644 --- a/vulkano/src/shader/reflect.rs +++ b/vulkano/src/shader/reflect.rs @@ -711,7 +711,7 @@ fn descriptor_requirements_of(spirv: &Spirv, variable_id: Id) -> DescriptorVaria let variable_id_info = spirv.id(variable_id); let mut reqs = DescriptorRequirements { - descriptor_count: 1, + descriptor_count: Some(1), ..Default::default() }; @@ -878,12 +878,15 @@ fn descriptor_requirements_of(spirv: &Spirv, variable_id: Id) -> DescriptorVaria _ => panic!("failed to find array length"), }; - reqs.descriptor_count *= len as u32; + if let Some(count) = reqs.descriptor_count.as_mut() { + *count *= len as u32 + } + Some(element_type) } &Instruction::TypeRuntimeArray { element_type, .. } => { - reqs.descriptor_count = 0; + reqs.descriptor_count = None; Some(element_type) }