From b6b423c3d344b441b0c84f7fac4a41f69848da38 Mon Sep 17 00:00:00 2001 From: Rua Date: Tue, 24 Aug 2021 11:16:09 +0200 Subject: [PATCH] Refactoring of DescriptorDesc and related changes (#1680) * Various changes to DescriptorSetDesc * More changes to DescriptorSetDesc * Small comment fix --- vulkano-shaders/src/codegen.rs | 10 +- vulkano-shaders/src/descriptor_sets.rs | 133 ++- vulkano/src/command_buffer/synced/commands.rs | 13 +- vulkano/src/command_buffer/synced/mod.rs | 4 +- .../validity/descriptor_sets.rs | 89 +- vulkano/src/descriptor_set/layout/desc.rs | 847 ++++++++---------- vulkano/src/descriptor_set/layout/mod.rs | 9 +- vulkano/src/descriptor_set/layout/sys.rs | 28 +- vulkano/src/descriptor_set/mod.rs | 25 +- vulkano/src/descriptor_set/persistent.rs | 354 ++++---- vulkano/src/descriptor_set/pool/standard.rs | 4 +- vulkano/src/descriptor_set/pool/sys.rs | 19 +- vulkano/src/image/view.rs | 20 +- vulkano/src/pipeline/compute_pipeline.rs | 12 +- .../src/pipeline/graphics_pipeline/builder.rs | 44 +- vulkano/src/pipeline/layout/limits_check.rs | 62 +- vulkano/src/pipeline/layout/sys.rs | 31 +- vulkano/src/pipeline/shader.rs | 35 +- 18 files changed, 765 insertions(+), 974 deletions(-) diff --git a/vulkano-shaders/src/codegen.rs b/vulkano-shaders/src/codegen.rs index 40d4408a..1d0bbe12 100644 --- a/vulkano-shaders/src/codegen.rs +++ b/vulkano-shaders/src/codegen.rs @@ -343,13 +343,7 @@ where #[allow(unused_imports)] use vulkano::descriptor_set::layout::DescriptorDescTy; #[allow(unused_imports)] - use vulkano::descriptor_set::layout::DescriptorBufferDesc; - #[allow(unused_imports)] - use vulkano::descriptor_set::layout::DescriptorImageDesc; - #[allow(unused_imports)] - use vulkano::descriptor_set::layout::DescriptorImageDescDimensions; - #[allow(unused_imports)] - use vulkano::descriptor_set::layout::DescriptorImageDescArray; + use vulkano::descriptor_set::layout::DescriptorDescImage; #[allow(unused_imports)] use vulkano::descriptor_set::layout::DescriptorSetDesc; #[allow(unused_imports)] @@ -359,6 +353,8 @@ where #[allow(unused_imports)] use vulkano::format::Format; #[allow(unused_imports)] + use vulkano::image::view::ImageViewType; + #[allow(unused_imports)] use vulkano::pipeline::layout::PipelineLayout; #[allow(unused_imports)] use vulkano::pipeline::layout::PipelineLayoutPcRange; diff --git a/vulkano-shaders/src/descriptor_sets.rs b/vulkano-shaders/src/descriptor_sets.rs index e7f20840..17c1ea46 100644 --- a/vulkano-shaders/src/descriptor_sets.rs +++ b/vulkano-shaders/src/descriptor_sets.rs @@ -19,8 +19,8 @@ struct Descriptor { set: u32, binding: u32, desc_ty: TokenStream, - array_count: u64, - readonly: bool, + descriptor_count: u64, + mutable: bool, } pub(super) fn write_descriptor_set_layout_descs( @@ -51,14 +51,14 @@ pub(super) fn write_descriptor_set_layout_descs( { Some(d) => { let desc_ty = &d.desc_ty; - let array_count = d.array_count as u32; - let readonly = d.readonly; + let descriptor_count = d.descriptor_count as u32; + let mutable = d.mutable; quote! { Some(DescriptorDesc { ty: #desc_ty, - array_count: #array_count, + descriptor_count: #descriptor_count, stages: #stages, - readonly: #readonly, + mutable: #mutable, }), } } @@ -84,7 +84,11 @@ pub(super) fn write_descriptor_set_layout_descs( } } -pub(super) fn write_push_constant_ranges(doc: &Spirv, stage: &TokenStream, types_meta: &TypesMeta) -> TokenStream { +pub(super) fn write_push_constant_ranges( + doc: &Spirv, + stage: &TokenStream, + types_meta: &TypesMeta, +) -> TokenStream { // TODO: somewhat implemented correctly // Looping to find all the push constant structs. @@ -173,7 +177,7 @@ fn find_descriptors( .is_some(); // Find information about the kind of binding for this descriptor. - let (desc_ty, readonly, array_count) = + let (desc_ty, mutable, descriptor_count) = descriptor_infos(doc, pointed_ty, storage_class, false).expect(&format!( "Couldn't find relevant type for uniform `{}` (type {}, maybe unimplemented)", name, pointed_ty @@ -182,8 +186,8 @@ fn find_descriptors( desc_ty, set, binding, - array_count, - readonly: nonwritable || readonly, + descriptor_count, + mutable: !nonwritable && mutable, }); } @@ -385,26 +389,20 @@ fn descriptor_infos( "Structs in shader interface are expected to be decorated with one of Block or BufferBlock" ); - // false -> VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER - // true -> VK_DESCRIPTOR_TYPE_STORAGE_BUFFER - let storage = decoration_buffer_block || decoration_block && pointer_storage == StorageClass::StorageBuffer; + let (ty, mutable) = if decoration_buffer_block || decoration_block && pointer_storage == StorageClass::StorageBuffer { + // VK_DESCRIPTOR_TYPE_STORAGE_BUFFER + // Determine whether all members have a NonWritable decoration. + let nonwritable = (0..member_types.len() as u32).all(|i| { + doc.get_member_decoration_params(pointed_ty, i, Decoration::NonWritable).is_some() + }); - // Determine whether all members have a NonWritable decoration. - let nonwritable = (0..member_types.len() as u32).all(|i| { - doc.get_member_decoration_params(pointed_ty, i, Decoration::NonWritable).is_some() - }); - - // Uniforms are never writable. - let readonly = !storage || nonwritable; - - let desc = quote! { - DescriptorDescTy::Buffer(DescriptorBufferDesc { - dynamic: None, - storage: #storage, - }) + (quote! { DescriptorDescTy::StorageBuffer }, !nonwritable) + } else { + // VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER + (quote! { DescriptorDescTy::UniformBuffer }, false) // Uniforms are never mutable. }; - - Some((desc, readonly, 1)) + + Some((ty, mutable, 1)) } &Instruction::TypeImage { result_id, @@ -422,11 +420,6 @@ fn descriptor_infos( let vulkan_format = to_vulkan_format(*format); - let arrayed = match arrayed { - true => quote! { DescriptorImageDescArray::Arrayed { max_layers: None } }, - false => quote! { DescriptorImageDescArray::NonArrayed }, - }; - match dim { Dim::DimSubpassData => { // VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT @@ -441,67 +434,71 @@ fn descriptor_infos( "If Dim is SubpassData, Image Format must be Unknown" ); assert!(!sampled, "If Dim is SubpassData, Sampled must be 2"); + assert!(!arrayed, "If Dim is SubpassData, Arrayed must be 0"); let desc = quote! { DescriptorDescTy::InputAttachment { multisampled: #ms, - array_layers: #arrayed } }; Some((desc, true, 1)) // Never writable. } Dim::DimBuffer => { - // false -> VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER - // true -> VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER - let storage = !sampled; + let (ty, mutable) = if sampled { + // VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER + (quote! { DescriptorDescTy::UniformTexelBuffer }, false) // Uniforms are never mutable. + } else { + // VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER + (quote! { DescriptorDescTy::StorageTexelBuffer }, true) + }; + let desc = quote! { - DescriptorDescTy::TexelBuffer { - storage: #storage, + #ty { format: #vulkan_format, } }; - Some((desc, !storage, 1)) // Uniforms are never writable. + Some((desc, mutable, 1)) + } _ => { - let (ty, readonly) = match force_combined_image_sampled { + let (ty, mutable) = if force_combined_image_sampled { // VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER // Never writable. - true => (quote! { DescriptorDescTy::CombinedImageSampler }, true), - false => { - // false -> VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE - // true -> VK_DESCRIPTOR_TYPE_STORAGE_IMAGE - let storage = !sampled; - (quote! { DescriptorDescTy::Image }, !storage) // Sampled images are never writable. - }, + assert!(sampled, "A combined image sampler must not reference a storage image"); + (quote! { DescriptorDescTy::CombinedImageSampler }, false) // Sampled images are never mutable. + } else { + if sampled { + // VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE + (quote! { DescriptorDescTy::SampledImage }, false) // Sampled images are never mutable. + } else { + // VK_DESCRIPTOR_TYPE_STORAGE_IMAGE + (quote! { DescriptorDescTy::StorageImage }, true) + } }; - let dim = match *dim { - Dim::Dim1D => { - quote! { DescriptorImageDescDimensions::OneDimensional } - } - Dim::Dim2D => { - quote! { DescriptorImageDescDimensions::TwoDimensional } - } - Dim::Dim3D => { - quote! { DescriptorImageDescDimensions::ThreeDimensional } - } - Dim::DimCube => quote! { DescriptorImageDescDimensions::Cube }, - Dim::DimRect => panic!("Vulkan doesn't support rectangle textures"), + let view_type = match (dim, arrayed) { + (Dim::Dim1D, false) => quote! { ImageViewType::Dim1d }, + (Dim::Dim1D, true) => quote! { ImageViewType::Dim1dArray }, + (Dim::Dim2D, false) => quote! { ImageViewType::Dim2d }, + (Dim::Dim2D, true) => quote! { ImageViewType::Dim2dArray }, + (Dim::Dim3D, false) => quote! { ImageViewType::Dim3d }, + (Dim::Dim3D, true) => panic!("Vulkan doesn't support arrayed 3D textures"), + (Dim::DimCube, false) => quote! { ImageViewType::Cube }, + (Dim::DimCube, true) => quote! { ImageViewType::CubeArray }, + (Dim::DimRect, _) => panic!("Vulkan doesn't support rectangle textures"), _ => unreachable!(), }; let desc = quote! { - #ty(DescriptorImageDesc { - sampled: #sampled, - dimensions: #dim, + #ty(DescriptorDescImage { format: #vulkan_format, multisampled: #ms, - array_layers: #arrayed, + view_type: #view_type, }) }; - Some((desc, readonly, 1)) + Some((desc, mutable, 1)) } } } @@ -515,14 +512,14 @@ fn descriptor_infos( &Instruction::TypeSampler { result_id } if result_id == pointed_ty => { let desc = quote! { DescriptorDescTy::Sampler }; - Some((desc, true, 1)) + Some((desc, false, 1)) } &Instruction::TypeArray { result_id, type_id, length_id, } if result_id == pointed_ty => { - let (desc, readonly, arr) = + let (desc, mutable, arr) = match descriptor_infos(doc, type_id, pointer_storage.clone(), false) { None => return None, Some(v) => v, @@ -542,7 +539,7 @@ fn descriptor_infos( .next() .expect("failed to find array length"); let len = len.iter().rev().fold(0, |a, &b| (a << 32) | b as u64); - Some((desc, readonly, len)) + Some((desc, mutable, len)) } _ => None, // TODO: other types } diff --git a/vulkano/src/command_buffer/synced/commands.rs b/vulkano/src/command_buffer/synced/commands.rs index eb82cfbd..4df471a3 100644 --- a/vulkano/src/command_buffer/synced/commands.rs +++ b/vulkano/src/command_buffer/synced/commands.rs @@ -2631,7 +2631,7 @@ impl SyncCommandBufferBuilder { .layout() .descriptor(ds.buffer(buf_num).unwrap().1 as usize) .unwrap(); - let exclusive = !desc.readonly; + let exclusive = desc.mutable; let (stages, access) = desc.pipeline_stages_and_access(); resources.push(( KeyTy::Buffer, @@ -2650,7 +2650,7 @@ impl SyncCommandBufferBuilder { for img_num in 0..ds.num_images() { let (image_view, desc_num) = ds.image(img_num).unwrap(); let desc = ds.layout().descriptor(desc_num as usize).unwrap(); - let exclusive = !desc.readonly; + let exclusive = desc.mutable; let (stages, access) = desc.pipeline_stages_and_access(); let mut ignore_me_hack = false; let layouts = image_view @@ -2659,13 +2659,8 @@ impl SyncCommandBufferBuilder { .expect("descriptor_layouts must return Some when used in an image view"); let layout = match desc.ty { DescriptorDescTy::CombinedImageSampler(_) => layouts.combined_image_sampler, - DescriptorDescTy::Image(ref img) => { - if img.sampled { - layouts.sampled_image - } else { - layouts.storage_image - } - } + DescriptorDescTy::SampledImage(_) => layouts.sampled_image, + DescriptorDescTy::StorageImage(_) => layouts.storage_image, DescriptorDescTy::InputAttachment { .. } => { // FIXME: This is tricky. Since we read from the input attachment // and this input attachment is being written in an earlier pass, diff --git a/vulkano/src/command_buffer/synced/mod.rs b/vulkano/src/command_buffer/synced/mod.rs index 7a6b98a3..8f003f2c 100644 --- a/vulkano/src/command_buffer/synced/mod.rs +++ b/vulkano/src/command_buffer/synced/mod.rs @@ -711,9 +711,9 @@ mod tests { device.clone(), [Some(DescriptorDesc { ty: DescriptorDescTy::Sampler, - array_count: 1, + descriptor_count: 1, stages: ShaderStages::all(), - readonly: true, + mutable: false, })], ) .unwrap(), diff --git a/vulkano/src/command_buffer/validity/descriptor_sets.rs b/vulkano/src/command_buffer/validity/descriptor_sets.rs index 227101bf..ec5649d0 100644 --- a/vulkano/src/command_buffer/validity/descriptor_sets.rs +++ b/vulkano/src/command_buffer/validity/descriptor_sets.rs @@ -7,46 +7,31 @@ // notice may not be copied, modified, or distributed except // according to those terms. -use std::error; -use std::fmt; - -use crate::descriptor_set::layout::DescriptorDescSupersetError; +use crate::descriptor_set::layout::DescriptorSetCompatibilityError; use crate::descriptor_set::DescriptorSetWithOffsets; use crate::pipeline::layout::PipelineLayout; +use std::error; +use std::fmt; /// Checks whether descriptor sets are compatible with the pipeline. pub fn check_descriptor_sets_validity( pipeline_layout: &PipelineLayout, descriptor_sets: &[DescriptorSetWithOffsets], ) -> Result<(), CheckDescriptorSetsValidityError> { - // What's important is not that the pipeline layout and the descriptor sets *match*. Instead - // what's important is that the descriptor sets are a superset of the pipeline layout. It's not - // a problem if the descriptor sets provide more elements than expected. + for (set_index, pipeline_set) in pipeline_layout.descriptor_set_layouts().iter().enumerate() { + let set_num = set_index as u32; - for (set_num, set) in pipeline_layout.descriptor_set_layouts().iter().enumerate() { - for (binding_num, pipeline_desc) in - (0..set.num_bindings()).filter_map(|i| set.descriptor(i).map(|d| (i, d))) - { - let set_desc = descriptor_sets - .get(set_num) - .and_then(|so| so.as_ref().0.layout().descriptor(binding_num)); + let descriptor_set = match descriptor_sets.get(set_index) { + Some(s) => s, + None => return Err(CheckDescriptorSetsValidityError::MissingDescriptorSet { set_num }), + }; - let set_desc = match set_desc { - Some(s) => s, - None => { - return Err(CheckDescriptorSetsValidityError::MissingDescriptor { - set_num: set_num, - binding_num: binding_num, - }) - } - }; - - if let Err(err) = set_desc.ensure_superset_of(&pipeline_desc) { - return Err(CheckDescriptorSetsValidityError::IncompatibleDescriptor { - error: err, - set_num: set_num, - binding_num: binding_num, - }); + match pipeline_set.ensure_compatible_with_bind(descriptor_set.as_ref().0.layout()) { + Ok(_) => (), + Err(error) => { + return Err( + CheckDescriptorSetsValidityError::IncompatibleDescriptorSet { error, set_num }, + ); } } } @@ -57,32 +42,22 @@ pub fn check_descriptor_sets_validity( /// Error that can happen when checking descriptor sets validity. #[derive(Debug, Clone)] pub enum CheckDescriptorSetsValidityError { - /// A descriptor is missing in the descriptor sets that were provided. - MissingDescriptor { - /// The index of the set of the descriptor. - set_num: usize, - /// The binding number of the descriptor. - binding_num: usize, + MissingDescriptorSet { + set_num: u32, }, - - /// A descriptor in the provided sets is not compatible with what is expected. - IncompatibleDescriptor { - /// The reason why the two descriptors aren't compatible. - error: DescriptorDescSupersetError, + IncompatibleDescriptorSet { + /// The error returned by the descriptor set. + error: DescriptorSetCompatibilityError, /// The index of the set of the descriptor. - set_num: usize, - /// The binding number of the descriptor. - binding_num: usize, + set_num: u32, }, } impl error::Error for CheckDescriptorSetsValidityError { #[inline] fn source(&self) -> Option<&(dyn error::Error + 'static)> { - match *self { - CheckDescriptorSetsValidityError::IncompatibleDescriptor { ref error, .. } => { - Some(error) - } + match self { + Self::IncompatibleDescriptorSet { error, .. } => Some(error), _ => None, } } @@ -91,18 +66,14 @@ impl error::Error for CheckDescriptorSetsValidityError { impl fmt::Display for CheckDescriptorSetsValidityError { #[inline] fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { - write!( - fmt, - "{}", - match *self { - CheckDescriptorSetsValidityError::MissingDescriptor { .. } => { - "a descriptor is missing in the descriptor sets that were provided" - } - CheckDescriptorSetsValidityError::IncompatibleDescriptor { .. } => { - "a descriptor in the provided sets is not compatible with what is expected" - } + match self { + Self::MissingDescriptorSet { set_num } => { + write!(fmt, "descriptor set {} has not been not bound, but is required by the pipeline layout", set_num) } - ) + Self::IncompatibleDescriptorSet { set_num, .. } => { + write!(fmt, "compatibility error in descriptor set {}", set_num) + } + } } } diff --git a/vulkano/src/descriptor_set/layout/desc.rs b/vulkano/src/descriptor_set/layout/desc.rs index 15168613..751b2453 100644 --- a/vulkano/src/descriptor_set/layout/desc.rs +++ b/vulkano/src/descriptor_set/layout/desc.rs @@ -44,7 +44,6 @@ use crate::format::Format; use crate::image::view::ImageViewType; use crate::pipeline::shader::ShaderStages; -use crate::pipeline::shader::ShaderStagesSupersetError; use crate::sync::AccessFlags; use crate::sync::PipelineStages; use smallvec::SmallVec; @@ -146,11 +145,10 @@ impl DescriptorSetDesc { { for binding_num in dynamic_buffers { debug_assert!( - self.descriptor(binding_num) - .map_or(false, |desc| match desc.ty { - DescriptorDescTy::Buffer(_) => true, - _ => false, - }), + self.descriptor(binding_num).map_or(false, |desc| matches!( + desc.ty, + DescriptorDescTy::StorageBuffer | DescriptorDescTy::UniformBuffer + )), "tried to make the non-buffer descriptor at binding {} a dynamic buffer", binding_num ); @@ -161,12 +159,15 @@ impl DescriptorSetDesc { .and_then(|b| b.as_mut()); if let Some(desc) = binding { - if let DescriptorDescTy::Buffer(ref buffer_desc) = desc.ty { - desc.ty = DescriptorDescTy::Buffer(DescriptorBufferDesc { - dynamic: Some(true), - ..*buffer_desc - }); - } + match &desc.ty { + DescriptorDescTy::StorageBuffer => { + desc.ty = DescriptorDescTy::StorageBufferDynamic; + } + DescriptorDescTy::UniformBuffer => { + desc.ty = DescriptorDescTy::UniformBufferDynamic; + } + _ => (), + }; } } } @@ -186,13 +187,14 @@ impl DescriptorSetDesc { } } - /// Returns whether `self` is a superset of `other`. - pub fn ensure_superset_of( + /// Checks whether the descriptor of a pipeline layout `self` is compatible with the descriptor + /// of a shader `other`. + pub fn ensure_compatible_with_shader( &self, other: &DescriptorSetDesc, - ) -> Result<(), DescriptorSetDescSupersetError> { + ) -> Result<(), DescriptorSetCompatibilityError> { if self.descriptors.len() < other.descriptors.len() { - return Err(DescriptorSetDescSupersetError::DescriptorsCountMismatch { + return Err(DescriptorSetCompatibilityError::DescriptorsCountMismatch { self_num: self.descriptors.len() as u32, other_num: other.descriptors.len() as u32, }); @@ -204,15 +206,19 @@ impl DescriptorSetDesc { match (self_desc, other_desc) { (Some(mine), Some(other)) => { - if let Err(err) = mine.ensure_superset_of(&other) { - return Err(DescriptorSetDescSupersetError::IncompatibleDescriptors { + if let Err(err) = mine.ensure_compatible_with_shader(&other) { + return Err(DescriptorSetCompatibilityError::IncompatibleDescriptors { error: err, binding_num: binding_num as u32, }); } } (None, Some(_)) => { - return Err(DescriptorSetDescSupersetError::ExpectedEmptyDescriptor { + return Err(DescriptorSetCompatibilityError::IncompatibleDescriptors { + error: DescriptorCompatibilityError::Empty { + first: true, + second: false, + }, binding_num: binding_num as u32, }) } @@ -222,6 +228,48 @@ impl DescriptorSetDesc { Ok(()) } + + /// Checks whether the descriptor of a pipeline layout `self` is compatible with the descriptor + /// of a descriptor set being bound `other`. + pub fn ensure_compatible_with_bind( + &self, + other: &DescriptorSetDesc, + ) -> Result<(), DescriptorSetCompatibilityError> { + if self.descriptors.len() != other.descriptors.len() { + return Err(DescriptorSetCompatibilityError::DescriptorsCountMismatch { + self_num: self.descriptors.len() as u32, + other_num: other.descriptors.len() as u32, + }); + } + + for binding_num in 0..other.descriptors.len() { + let self_desc = self.descriptor(binding_num); + let other_desc = self.descriptor(binding_num); + + match (self_desc, other_desc) { + (Some(mine), Some(other)) => { + if let Err(err) = mine.ensure_compatible_with_bind(&other) { + return Err(DescriptorSetCompatibilityError::IncompatibleDescriptors { + error: err, + binding_num: binding_num as u32, + }); + } + } + (None, None) => (), + (a, b) => { + return Err(DescriptorSetCompatibilityError::IncompatibleDescriptors { + error: DescriptorCompatibilityError::Empty { + first: a.is_none(), + second: b.is_none(), + }, + binding_num: binding_num as u32, + }) + } + } + } + + Ok(()) + } } impl From for DescriptorSetDesc @@ -249,64 +297,81 @@ pub struct DescriptorDesc { /// How many array elements this descriptor is made of. The value 0 is invalid and may trigger /// a panic depending on the situation. - pub array_count: u32, + pub descriptor_count: u32, /// Which shader stages are going to access this descriptor. pub stages: ShaderStages, - /// True if the attachment is only ever read by the shader. False if it is also written. - pub readonly: bool, + /// True if the attachment can be written by the shader. + pub mutable: bool, } impl DescriptorDesc { - /// Checks whether we are a superset of another descriptor. - /// - /// Returns true if `self` is the same descriptor as `other`, or if `self` is the same as - /// `other` but with a larger array elements count and/or more shader stages. - /// - ///# Example - ///``` - ///use vulkano::descriptor_set::layout::DescriptorDesc; - ///use vulkano::descriptor_set::layout::DescriptorDescTy::*; - ///use vulkano::pipeline::shader::ShaderStages; - /// - ///let desc_super = DescriptorDesc{ ty: Sampler, array_count: 2, stages: ShaderStages{ - /// vertex: true, - /// tessellation_control: true, - /// tessellation_evaluation: true, - /// geometry: true, - /// fragment: true, - /// compute: true - ///}, readonly: false }; - ///let desc_sub = DescriptorDesc{ ty: Sampler, array_count: 1, stages: ShaderStages{ - /// vertex: true, - /// tessellation_control: false, - /// tessellation_evaluation: false, - /// geometry: false, - /// fragment: true, - /// compute: false - ///}, readonly: true }; - /// - ///assert_eq!(desc_super.ensure_superset_of(&desc_sub).unwrap(), ()); - /// - ///``` + /// Checks whether the descriptor of a pipeline layout `self` is compatible with the descriptor + /// of a shader `other`. #[inline] - pub fn ensure_superset_of( + pub fn ensure_compatible_with_shader( &self, other: &DescriptorDesc, - ) -> Result<(), DescriptorDescSupersetError> { - self.ty.ensure_superset_of(&other.ty)?; - self.stages.ensure_superset_of(&other.stages)?; + ) -> Result<(), DescriptorCompatibilityError> { + match (self.ty.ty(), other.ty.ty()) { + (DescriptorType::UniformBufferDynamic, DescriptorType::UniformBuffer) => (), + (DescriptorType::StorageBufferDynamic, DescriptorType::StorageBuffer) => (), + _ => self.ty.ensure_superset_of(&other.ty)?, + } - if self.array_count < other.array_count { - return Err(DescriptorDescSupersetError::ArrayTooSmall { - len: self.array_count, - required: other.array_count, + if !self.stages.is_superset_of(&other.stages) { + return Err(DescriptorCompatibilityError::ShaderStages { + first: self.stages, + second: other.stages, }); } - if self.readonly && !other.readonly { - return Err(DescriptorDescSupersetError::MutabilityRequired); + if self.descriptor_count < other.descriptor_count { + return Err(DescriptorCompatibilityError::DescriptorCount { + first: self.descriptor_count, + second: other.descriptor_count, + }); + } + + if !self.mutable && other.mutable { + return Err(DescriptorCompatibilityError::Mutability { + first: self.mutable, + second: other.mutable, + }); + } + + Ok(()) + } + + /// Checks whether the descriptor of a pipeline layout `self` is compatible with the descriptor + /// of a descriptor set being bound `other`. + #[inline] + pub fn ensure_compatible_with_bind( + &self, + other: &DescriptorDesc, + ) -> Result<(), DescriptorCompatibilityError> { + other.ty.ensure_superset_of(&self.ty)?; + + if self.stages != other.stages { + return Err(DescriptorCompatibilityError::ShaderStages { + first: self.stages, + second: other.stages, + }); + } + + if self.descriptor_count != other.descriptor_count { + return Err(DescriptorCompatibilityError::DescriptorCount { + first: self.descriptor_count, + second: other.descriptor_count, + }); + } + + if self.mutable && !other.mutable { + return Err(DescriptorCompatibilityError::Mutability { + first: self.mutable, + second: other.mutable, + }); } Ok(()) @@ -325,32 +390,32 @@ impl DescriptorDesc { ///use vulkano::descriptor_set::layout::DescriptorDescTy::*; ///use vulkano::pipeline::shader::ShaderStages; /// - ///let desc_part1 = DescriptorDesc{ ty: Sampler, array_count: 2, stages: ShaderStages{ + ///let desc_part1 = DescriptorDesc{ ty: Sampler, descriptor_count: 2, stages: ShaderStages{ /// vertex: true, /// tessellation_control: true, /// tessellation_evaluation: false, /// geometry: true, /// fragment: false, /// compute: true - ///}, readonly: false }; + ///}, mutable: true }; /// - ///let desc_part2 = DescriptorDesc{ ty: Sampler, array_count: 1, stages: ShaderStages{ + ///let desc_part2 = DescriptorDesc{ ty: Sampler, descriptor_count: 1, stages: ShaderStages{ /// vertex: true, /// tessellation_control: false, /// tessellation_evaluation: true, /// geometry: false, /// fragment: true, /// compute: true - ///}, readonly: true }; + ///}, mutable: false }; /// - ///let desc_union = DescriptorDesc{ ty: Sampler, array_count: 2, stages: ShaderStages{ + ///let desc_union = DescriptorDesc{ ty: Sampler, descriptor_count: 2, stages: ShaderStages{ /// vertex: true, /// tessellation_control: true, /// tessellation_evaluation: true, /// geometry: true, /// fragment: true, /// compute: true - ///}, readonly: false }; + ///}, mutable: true }; /// ///assert_eq!(DescriptorDesc::union(Some(&desc_part1), Some(&desc_part2)), Ok(Some(desc_union))); ///``` @@ -366,9 +431,9 @@ impl DescriptorDesc { Ok(Some(DescriptorDesc { ty: first.ty.clone(), - array_count: cmp::max(first.array_count, second.array_count), + descriptor_count: cmp::max(first.descriptor_count, second.descriptor_count), stages: first.stages | second.stages, - readonly: first.readonly && second.readonly, + mutable: first.mutable || second.mutable, })) } else { Ok(first.or(second).cloned()) @@ -384,347 +449,41 @@ impl DescriptorDesc { pub fn pipeline_stages_and_access(&self) -> (PipelineStages, AccessFlags) { let stages: PipelineStages = self.stages.into(); - let access = match self.ty { - DescriptorDescTy::Sampler => panic!(), - DescriptorDescTy::CombinedImageSampler(_) | DescriptorDescTy::Image(_) => AccessFlags { + let access = match self.ty.ty() { + DescriptorType::Sampler => panic!(), + DescriptorType::CombinedImageSampler + | DescriptorType::SampledImage + | DescriptorType::StorageImage => AccessFlags { shader_read: true, - shader_write: !self.readonly, + shader_write: self.mutable, ..AccessFlags::none() }, - DescriptorDescTy::TexelBuffer { .. } => AccessFlags { - shader_read: true, - shader_write: !self.readonly, - ..AccessFlags::none() - }, - DescriptorDescTy::InputAttachment { .. } => AccessFlags { + DescriptorType::InputAttachment => AccessFlags { input_attachment_read: true, ..AccessFlags::none() }, - DescriptorDescTy::Buffer(ref buf) => { - if buf.storage { - AccessFlags { - shader_read: true, - shader_write: !self.readonly, - ..AccessFlags::none() - } - } else { - AccessFlags { - uniform_read: true, - ..AccessFlags::none() - } + DescriptorType::UniformTexelBuffer | DescriptorType::StorageTexelBuffer => { + AccessFlags { + shader_read: true, + shader_write: self.mutable, + ..AccessFlags::none() } } + DescriptorType::UniformBuffer | DescriptorType::UniformBufferDynamic => AccessFlags { + uniform_read: true, + ..AccessFlags::none() + }, + DescriptorType::StorageBuffer | DescriptorType::StorageBufferDynamic => AccessFlags { + shader_read: true, + shader_write: self.mutable, + ..AccessFlags::none() + }, }; (stages, access) } } -/// Describes the content and layout of each array element of a descriptor. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum DescriptorDescTy { - Sampler, // TODO: the sampler has some restrictions as well - CombinedImageSampler(DescriptorImageDesc), // TODO: the sampler has some restrictions as well - Image(DescriptorImageDesc), - TexelBuffer { - /// If `true`, this describes a storage texel buffer. - storage: bool, - - /// The format of the content, or `None` if the format is unknown. Depending on the - /// context, it may be invalid to have a `None` value here. If the format is `Some`, only - /// buffer views that have this exact format can be attached to this descriptor. - format: Option, - }, - InputAttachment { - /// If `true`, the input attachment is multisampled. Only multisampled images can be - /// attached to this descriptor. If `false`, only single-sampled images can be attached. - multisampled: bool, - array_layers: DescriptorImageDescArray, - }, - Buffer(DescriptorBufferDesc), -} - -impl DescriptorDescTy { - /// Returns the type of descriptor. - // TODO: add example - pub fn ty(&self) -> DescriptorType { - match *self { - DescriptorDescTy::Sampler => DescriptorType::Sampler, - DescriptorDescTy::CombinedImageSampler(_) => DescriptorType::CombinedImageSampler, - DescriptorDescTy::Image(ref desc) => { - if desc.sampled { - DescriptorType::SampledImage - } else { - DescriptorType::StorageImage - } - } - DescriptorDescTy::InputAttachment { .. } => DescriptorType::InputAttachment, - DescriptorDescTy::Buffer(ref desc) => { - let dynamic = desc.dynamic.unwrap_or(false); - match (desc.storage, dynamic) { - (false, false) => DescriptorType::UniformBuffer, - (true, false) => DescriptorType::StorageBuffer, - (false, true) => DescriptorType::UniformBufferDynamic, - (true, true) => DescriptorType::StorageBufferDynamic, - } - } - DescriptorDescTy::TexelBuffer { storage, .. } => { - if storage { - DescriptorType::StorageTexelBuffer - } else { - DescriptorType::UniformTexelBuffer - } - } - } - } - - /// Checks whether we are a superset of another descriptor type. - // TODO: add example - #[inline] - pub fn ensure_superset_of( - &self, - other: &DescriptorDescTy, - ) -> Result<(), DescriptorDescSupersetError> { - match (self, other) { - (&DescriptorDescTy::Sampler, &DescriptorDescTy::Sampler) => Ok(()), - - ( - &DescriptorDescTy::CombinedImageSampler(ref me), - &DescriptorDescTy::CombinedImageSampler(ref other), - ) => me.ensure_superset_of(other), - - (&DescriptorDescTy::Image(ref me), &DescriptorDescTy::Image(ref other)) => { - me.ensure_superset_of(other) - } - - ( - &DescriptorDescTy::InputAttachment { - multisampled: me_multisampled, - array_layers: me_array_layers, - }, - &DescriptorDescTy::InputAttachment { - multisampled: other_multisampled, - array_layers: other_array_layers, - }, - ) => { - if me_multisampled != other_multisampled { - return Err(DescriptorDescSupersetError::MultisampledMismatch { - provided: me_multisampled, - expected: other_multisampled, - }); - } - - if me_array_layers != other_array_layers { - return Err(DescriptorDescSupersetError::IncompatibleArrayLayers { - provided: me_array_layers, - required: other_array_layers, - }); - } - - Ok(()) - } - - (&DescriptorDescTy::Buffer(ref me), &DescriptorDescTy::Buffer(ref other)) => { - if me.storage != other.storage { - return Err(DescriptorDescSupersetError::TypeMismatch); - } - - match (me.dynamic, other.dynamic) { - (Some(_), None) => Ok(()), - (Some(m), Some(o)) => { - if m == o { - Ok(()) - } else { - Err(DescriptorDescSupersetError::TypeMismatch) - } - } - (None, None) => Ok(()), - (None, Some(_)) => Err(DescriptorDescSupersetError::TypeMismatch), - } - } - - ( - &DescriptorDescTy::TexelBuffer { - storage: me_storage, - format: me_format, - }, - &DescriptorDescTy::TexelBuffer { - storage: other_storage, - format: other_format, - }, - ) => { - if me_storage != other_storage { - return Err(DescriptorDescSupersetError::TypeMismatch); - } - - match (me_format, other_format) { - (Some(_), None) => Ok(()), - (Some(m), Some(o)) => { - if m == o { - Ok(()) - } else { - Err(DescriptorDescSupersetError::FormatMismatch { - provided: Some(m), - expected: o, - }) - } - } - (None, None) => Ok(()), - (None, Some(a)) => Err(DescriptorDescSupersetError::FormatMismatch { - provided: Some(a), - expected: a, - }), - } - } - - // Any other combination is invalid. - _ => Err(DescriptorDescSupersetError::TypeMismatch), - } - } -} - -/// Additional description for descriptors that contain images. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct DescriptorImageDesc { - /// If `true`, the image can be sampled by the shader. Only images that were created with the - /// `sampled` usage can be attached to the descriptor. - pub sampled: bool, - /// The kind of image: one-dimensional, two-dimensional, three-dimensional, or cube. - pub dimensions: DescriptorImageDescDimensions, - /// The format of the image, or `None` if the format is unknown. If `Some`, only images with - /// exactly that format can be attached. - pub format: Option, - /// True if the image is multisampled. - pub multisampled: bool, - /// Whether the descriptor contains one or more array layers of an image. - pub array_layers: DescriptorImageDescArray, -} - -impl DescriptorImageDesc { - /// Checks whether we are a superset of another image. - // TODO: add example - #[inline] - pub fn ensure_superset_of( - &self, - other: &DescriptorImageDesc, - ) -> Result<(), DescriptorDescSupersetError> { - if self.dimensions != other.dimensions { - return Err(DescriptorDescSupersetError::DimensionsMismatch { - provided: self.dimensions, - expected: other.dimensions, - }); - } - - if self.multisampled != other.multisampled { - return Err(DescriptorDescSupersetError::MultisampledMismatch { - provided: self.multisampled, - expected: other.multisampled, - }); - } - - match (self.format, other.format) { - (Some(a), Some(b)) => { - if a != b { - return Err(DescriptorDescSupersetError::FormatMismatch { - provided: Some(a), - expected: b, - }); - } - } - (Some(_), None) => (), - (None, None) => (), - (None, Some(a)) => { - return Err(DescriptorDescSupersetError::FormatMismatch { - provided: None, - expected: a, - }); - } - }; - - match (self.array_layers, other.array_layers) { - (DescriptorImageDescArray::NonArrayed, DescriptorImageDescArray::NonArrayed) => (), - ( - DescriptorImageDescArray::Arrayed { max_layers: my_max }, - DescriptorImageDescArray::Arrayed { - max_layers: other_max, - }, - ) => { - match (my_max, other_max) { - (Some(m), Some(o)) => { - if m < o { - return Err(DescriptorDescSupersetError::IncompatibleArrayLayers { - provided: DescriptorImageDescArray::Arrayed { max_layers: my_max }, - required: DescriptorImageDescArray::Arrayed { - max_layers: other_max, - }, - }); - } - } - (Some(_), None) => (), - (None, Some(m)) => { - return Err(DescriptorDescSupersetError::IncompatibleArrayLayers { - provided: DescriptorImageDescArray::Arrayed { max_layers: my_max }, - required: DescriptorImageDescArray::Arrayed { - max_layers: other_max, - }, - }); - } - (None, None) => (), // TODO: is this correct? - }; - } - (a, b) => { - return Err(DescriptorDescSupersetError::IncompatibleArrayLayers { - provided: a, - required: b, - }) - } - }; - - Ok(()) - } -} - -// TODO: documentation -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum DescriptorImageDescArray { - NonArrayed, - Arrayed { max_layers: Option }, -} - -// TODO: documentation -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub enum DescriptorImageDescDimensions { - OneDimensional, - TwoDimensional, - ThreeDimensional, - Cube, -} - -impl DescriptorImageDescDimensions { - /// Builds the `DescriptorImageDescDimensions` that corresponds to the view type. - #[inline] - pub fn from_image_view_type(ty: ImageViewType) -> DescriptorImageDescDimensions { - match ty { - ImageViewType::Dim1d => DescriptorImageDescDimensions::OneDimensional, - ImageViewType::Dim1dArray => DescriptorImageDescDimensions::OneDimensional, - ImageViewType::Dim2d => DescriptorImageDescDimensions::TwoDimensional, - ImageViewType::Dim2dArray => DescriptorImageDescDimensions::TwoDimensional, - ImageViewType::Dim3d => DescriptorImageDescDimensions::ThreeDimensional, - ImageViewType::Cubemap => DescriptorImageDescDimensions::Cube, - ImageViewType::CubemapArray => DescriptorImageDescDimensions::Cube, - } - } -} - -/// Additional description for descriptors that contain buffers. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct DescriptorBufferDesc { - /// If `true`, this buffer is a dynamic buffer. Assumes false if `None`. - pub dynamic: Option, - /// If `true`, this buffer is a storage buffer. - pub storage: bool, -} - /// Describes what kind of resource may later be bound to a descriptor. /// /// This is mostly the same as a `DescriptorDescTy` but with less precise information. @@ -751,27 +510,182 @@ impl From for ash::vk::DescriptorType { } } -/// Error when checking whether a descriptor set is a superset of another one. +/// Describes the content and layout of each array element of a descriptor. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum DescriptorSetDescSupersetError { - /// There are more descriptors in the child than in the parent layout. - DescriptorsCountMismatch { self_num: u32, other_num: u32 }, +pub enum DescriptorDescTy { + Sampler, // TODO: the sampler has some restrictions as well + CombinedImageSampler(DescriptorDescImage), // TODO: the sampler has some restrictions as well + SampledImage(DescriptorDescImage), + StorageImage(DescriptorDescImage), + UniformTexelBuffer { + /// The format of the content, or `None` if the format is unknown. Depending on the + /// context, it may be invalid to have a `None` value here. If the format is `Some`, only + /// buffer views that have this exact format can be attached to this descriptor. + format: Option, + }, + StorageTexelBuffer { + /// The format of the content, or `None` if the format is unknown. Depending on the + /// context, it may be invalid to have a `None` value here. If the format is `Some`, only + /// buffer views that have this exact format can be attached to this descriptor. + format: Option, + }, + UniformBuffer, + StorageBuffer, + UniformBufferDynamic, + StorageBufferDynamic, + InputAttachment { + /// If `true`, the input attachment is multisampled. Only multisampled images can be + /// attached to this descriptor. If `false`, only single-sampled images can be attached. + multisampled: bool, + }, +} - /// Expected an empty descriptor, but got something instead. - ExpectedEmptyDescriptor { binding_num: u32 }, +impl DescriptorDescTy { + /// Returns the type of descriptor. + // TODO: add example + pub fn ty(&self) -> DescriptorType { + match *self { + DescriptorDescTy::Sampler => DescriptorType::Sampler, + DescriptorDescTy::CombinedImageSampler(_) => DescriptorType::CombinedImageSampler, + DescriptorDescTy::SampledImage(_) => DescriptorType::SampledImage, + DescriptorDescTy::StorageImage(_) => DescriptorType::StorageImage, + DescriptorDescTy::UniformTexelBuffer { .. } => DescriptorType::UniformTexelBuffer, + DescriptorDescTy::StorageTexelBuffer { .. } => DescriptorType::StorageTexelBuffer, + DescriptorDescTy::UniformBuffer => DescriptorType::UniformBuffer, + DescriptorDescTy::StorageBuffer => DescriptorType::StorageBuffer, + DescriptorDescTy::UniformBufferDynamic => DescriptorType::UniformBufferDynamic, + DescriptorDescTy::StorageBufferDynamic => DescriptorType::StorageBufferDynamic, + DescriptorDescTy::InputAttachment { .. } => DescriptorType::InputAttachment, + } + } + + /// Checks whether we are a superset of another descriptor type. + // TODO: add example + #[inline] + pub fn ensure_superset_of( + &self, + other: &DescriptorDescTy, + ) -> Result<(), DescriptorCompatibilityError> { + if self.ty() != other.ty() { + return Err(DescriptorCompatibilityError::Type { + first: self.ty(), + second: other.ty(), + }); + } + + match (self, other) { + ( + DescriptorDescTy::CombinedImageSampler(ref me) + | DescriptorDescTy::SampledImage(ref me) + | DescriptorDescTy::StorageImage(ref me), + DescriptorDescTy::CombinedImageSampler(ref other) + | DescriptorDescTy::SampledImage(ref other) + | DescriptorDescTy::StorageImage(ref other), + ) => me.ensure_superset_of(other)?, + + ( + DescriptorDescTy::UniformTexelBuffer { format: me_format } + | DescriptorDescTy::StorageTexelBuffer { format: me_format }, + DescriptorDescTy::UniformTexelBuffer { + format: other_format, + } + | DescriptorDescTy::StorageTexelBuffer { + format: other_format, + }, + ) => { + if other_format.is_some() && me_format != other_format { + return Err(DescriptorCompatibilityError::Format { + first: *me_format, + second: other_format.unwrap(), + }); + } + } + + ( + DescriptorDescTy::InputAttachment { + multisampled: me_multisampled, + }, + DescriptorDescTy::InputAttachment { + multisampled: other_multisampled, + }, + ) => { + if me_multisampled != other_multisampled { + return Err(DescriptorCompatibilityError::Multisampling { + first: *me_multisampled, + second: *other_multisampled, + }); + } + } + + _ => (), + } + + Ok(()) + } +} + +/// Additional description for descriptors that contain images. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct DescriptorDescImage { + /// The image format that is required for an attached image, or `None` no particular format is required. + pub format: Option, + /// True if the image is multisampled. + pub multisampled: bool, + /// The type of image view that must be attached to this descriptor. + pub view_type: ImageViewType, +} + +impl DescriptorDescImage { + /// Checks whether we are a superset of another image. + // TODO: add example + #[inline] + pub fn ensure_superset_of( + &self, + other: &DescriptorDescImage, + ) -> Result<(), DescriptorCompatibilityError> { + if other.format.is_some() && self.format != other.format { + return Err(DescriptorCompatibilityError::Format { + first: self.format, + second: other.format.unwrap(), + }); + } + + if self.multisampled != other.multisampled { + return Err(DescriptorCompatibilityError::Multisampling { + first: self.multisampled, + second: other.multisampled, + }); + } + + if self.view_type != other.view_type { + return Err(DescriptorCompatibilityError::ImageViewType { + first: self.view_type, + second: other.view_type, + }); + } + + Ok(()) + } +} + +/// Error when checking whether a descriptor set is compatible with another one. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DescriptorSetCompatibilityError { + /// The number of descriptors in the two sets is not compatible. + DescriptorsCountMismatch { self_num: u32, other_num: u32 }, /// Two descriptors are incompatible. IncompatibleDescriptors { - error: DescriptorDescSupersetError, + error: DescriptorCompatibilityError, binding_num: u32, }, } -impl error::Error for DescriptorSetDescSupersetError { +impl error::Error for DescriptorSetCompatibilityError { #[inline] fn source(&self) -> Option<&(dyn error::Error + 'static)> { match *self { - DescriptorSetDescSupersetError::IncompatibleDescriptors { ref error, .. } => { + DescriptorSetCompatibilityError::IncompatibleDescriptors { ref error, .. } => { Some(error) } _ => None, @@ -779,20 +693,17 @@ impl error::Error for DescriptorSetDescSupersetError { } } -impl fmt::Display for DescriptorSetDescSupersetError { +impl fmt::Display for DescriptorSetCompatibilityError { #[inline] fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { write!( fmt, "{}", match *self { - DescriptorSetDescSupersetError::DescriptorsCountMismatch { .. } => { - "there are more descriptors in the child than in the parent layout" + DescriptorSetCompatibilityError::DescriptorsCountMismatch { .. } => { + "the number of descriptors in the two sets is not compatible." } - DescriptorSetDescSupersetError::ExpectedEmptyDescriptor { .. } => { - "expected an empty descriptor, but got something instead" - } - DescriptorSetDescSupersetError::IncompatibleDescriptors { .. } => { + DescriptorSetCompatibilityError::IncompatibleDescriptors { .. } => { "two descriptors are incompatible" } } @@ -800,90 +711,92 @@ impl fmt::Display for DescriptorSetDescSupersetError { } } -/// Error when checking whether a descriptor is a superset of another one. +/// Error when checking whether a descriptor compatible with another one. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum DescriptorDescSupersetError { - /// The number of array elements of the descriptor is smaller than expected. - ArrayTooSmall { - len: u32, - required: u32, +pub enum DescriptorCompatibilityError { + /// The number of descriptors is not compatible. + DescriptorCount { + first: u32, + second: u32, }, - DimensionsMismatch { - provided: DescriptorImageDescDimensions, - expected: DescriptorImageDescDimensions, + /// The presence or absence of a descriptor in a binding is not compatible. + Empty { + first: bool, + second: bool, }, - FormatMismatch { - provided: Option, - expected: Format, + // The formats of an image descriptor are not compatible. + Format { + first: Option, + second: Format, }, - IncompatibleArrayLayers { - provided: DescriptorImageDescArray, - required: DescriptorImageDescArray, + // The image view types of an image descriptor are not compatible. + ImageViewType { + first: ImageViewType, + second: ImageViewType, }, - MultisampledMismatch { - provided: bool, - expected: bool, + // The multisampling of an image descriptor is not compatible. + Multisampling { + first: bool, + second: bool, }, - /// The descriptor is marked as read-only, but the other is not. - MutabilityRequired, + /// The mutability of the descriptors is not compatible. + Mutability { + first: bool, + second: bool, + }, - /// The shader stages are not a superset of one another. - ShaderStagesNotSuperset, + /// The shader stages of the descriptors are not compatible. + ShaderStages { + first: ShaderStages, + second: ShaderStages, + }, - /// The descriptor type doesn't match the type of the other descriptor. - TypeMismatch, + /// The types of the two descriptors are not compatible. + Type { + first: DescriptorType, + second: DescriptorType, + }, } -impl error::Error for DescriptorDescSupersetError {} +impl error::Error for DescriptorCompatibilityError {} -impl fmt::Display for DescriptorDescSupersetError { +impl fmt::Display for DescriptorCompatibilityError { #[inline] fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { write!( fmt, "{}", match *self { - DescriptorDescSupersetError::ArrayTooSmall { .. } => { - "the number of array elements of the descriptor is smaller than expected" + DescriptorCompatibilityError::DescriptorCount { .. } => { + "the number of descriptors is not compatible" } - DescriptorDescSupersetError::TypeMismatch => { - "the descriptor type doesn't match the type of the other descriptor" + DescriptorCompatibilityError::Empty { .. } => { + "the presence or absence of a descriptor in a binding is not compatible" } - DescriptorDescSupersetError::MutabilityRequired => { - "the descriptor is marked as read-only, but the other is not" + DescriptorCompatibilityError::Format { .. } => { + "the formats of an image descriptor are not compatible" } - DescriptorDescSupersetError::ShaderStagesNotSuperset => { - "the shader stages are not a superset of one another" + DescriptorCompatibilityError::ImageViewType { .. } => { + "the image view types of an image descriptor are not compatible" } - DescriptorDescSupersetError::DimensionsMismatch { .. } => { - "mismatch between the dimensions of the two descriptors" + DescriptorCompatibilityError::Multisampling { .. } => { + "the multisampling of an image descriptor is not compatible" } - DescriptorDescSupersetError::FormatMismatch { .. } => { - "mismatch between the format of the two descriptors" + DescriptorCompatibilityError::Mutability { .. } => { + "the mutability of the descriptors is not compatible" } - DescriptorDescSupersetError::MultisampledMismatch { .. } => { - "mismatch between whether the descriptors are multisampled" + DescriptorCompatibilityError::ShaderStages { .. } => { + "the shader stages of the descriptors are not compatible" } - DescriptorDescSupersetError::IncompatibleArrayLayers { .. } => { - "the array layers of the descriptors aren't compatible" + DescriptorCompatibilityError::Type { .. } => { + "the types of the two descriptors are not compatible" } } ) } } - -impl From for DescriptorDescSupersetError { - #[inline] - fn from(err: ShaderStagesSupersetError) -> DescriptorDescSupersetError { - match err { - ShaderStagesSupersetError::NotSuperset => { - DescriptorDescSupersetError::ShaderStagesNotSuperset - } - } - } -} diff --git a/vulkano/src/descriptor_set/layout/mod.rs b/vulkano/src/descriptor_set/layout/mod.rs index 272e480f..527570ec 100644 --- a/vulkano/src/descriptor_set/layout/mod.rs +++ b/vulkano/src/descriptor_set/layout/mod.rs @@ -13,15 +13,12 @@ //! can create a descriptor set layout manually, but it is normally created automatically by each //! pipeline layout. -pub use self::desc::DescriptorBufferDesc; +pub use self::desc::DescriptorCompatibilityError; pub use self::desc::DescriptorDesc; -pub use self::desc::DescriptorDescSupersetError; +pub use self::desc::DescriptorDescImage; pub use self::desc::DescriptorDescTy; -pub use self::desc::DescriptorImageDesc; -pub use self::desc::DescriptorImageDescArray; -pub use self::desc::DescriptorImageDescDimensions; +pub use self::desc::DescriptorSetCompatibilityError; pub use self::desc::DescriptorSetDesc; -pub use self::desc::DescriptorSetDescSupersetError; pub use self::desc::DescriptorType; pub use self::sys::DescriptorSetLayout; diff --git a/vulkano/src/descriptor_set/layout/sys.rs b/vulkano/src/descriptor_set/layout/sys.rs index 656f767c..d4361b89 100644 --- a/vulkano/src/descriptor_set/layout/sys.rs +++ b/vulkano/src/descriptor_set/layout/sys.rs @@ -9,6 +9,7 @@ use crate::check_errors; use crate::descriptor_set::layout::DescriptorDesc; +use crate::descriptor_set::layout::DescriptorSetCompatibilityError; use crate::descriptor_set::layout::DescriptorSetDesc; use crate::descriptor_set::pool::DescriptorsCount; use crate::device::Device; @@ -60,12 +61,12 @@ impl DescriptorSetLayout { // doesn't have tess shaders enabled let ty = desc.ty.ty(); - descriptors_count.add_num(ty, desc.array_count); + descriptors_count.add_num(ty, desc.descriptor_count); Some(ash::vk::DescriptorSetLayoutBinding { binding: binding as u32, descriptor_type: ty.into(), - descriptor_count: desc.array_count, + descriptor_count: desc.descriptor_count, stage_flags: desc.stages.into(), p_immutable_samplers: ptr::null(), // FIXME: not yet implemented }) @@ -122,6 +123,19 @@ impl DescriptorSetLayout { pub fn descriptor(&self, binding: usize) -> Option { self.desc.bindings().get(binding).cloned().unwrap_or(None) } + + /// Checks whether the descriptor of a pipeline layout `self` is compatible with the descriptor + /// of a descriptor set being bound `other`. + pub fn ensure_compatible_with_bind( + &self, + other: &DescriptorSetLayout, + ) -> Result<(), DescriptorSetCompatibilityError> { + if self.internal_object() == other.internal_object() { + return Ok(()); + } + + self.desc.ensure_compatible_with_bind(&other.desc) + } } unsafe impl DeviceOwned for DescriptorSetLayout { @@ -156,7 +170,6 @@ impl Drop for DescriptorSetLayout { #[cfg(test)] mod tests { - use crate::descriptor_set::layout::DescriptorBufferDesc; use crate::descriptor_set::layout::DescriptorDesc; use crate::descriptor_set::layout::DescriptorDescTy; use crate::descriptor_set::layout::DescriptorSetDesc; @@ -176,13 +189,10 @@ mod tests { let (device, _) = gfx_dev_and_queue!(); let layout = DescriptorDesc { - ty: DescriptorDescTy::Buffer(DescriptorBufferDesc { - dynamic: Some(false), - storage: false, - }), - array_count: 1, + ty: DescriptorDescTy::UniformBuffer, + descriptor_count: 1, stages: ShaderStages::all_graphics(), - readonly: true, + mutable: false, }; let sl = DescriptorSetLayout::new( diff --git a/vulkano/src/descriptor_set/mod.rs b/vulkano/src/descriptor_set/mod.rs index b69deeed..b0202824 100644 --- a/vulkano/src/descriptor_set/mod.rs +++ b/vulkano/src/descriptor_set/mod.rs @@ -81,7 +81,7 @@ pub use self::persistent::PersistentDescriptorSetBuildError; pub use self::persistent::PersistentDescriptorSetError; use self::sys::UnsafeDescriptorSet; use crate::buffer::BufferAccess; -use crate::descriptor_set::layout::{DescriptorBufferDesc, DescriptorDescTy}; +use crate::descriptor_set::layout::DescriptorDescTy; use crate::device::DeviceOwned; use crate::image::view::ImageViewAbstract; use crate::SafeDeref; @@ -213,22 +213,22 @@ impl DescriptorSetWithOffsets { // dynamic offset is a multiple of the minimum offset alignment specified // by the physical device. for desc in layout.desc().bindings() { - let desc = desc.as_ref().unwrap(); - if let DescriptorDescTy::Buffer(DescriptorBufferDesc { - dynamic: Some(true), - storage, - }) = desc.ty - { - // Don't check alignment if there are not enough offsets anyway - if dynamic_offsets.len() > dynamic_offset_index { - if storage { + match desc.as_ref().unwrap().ty { + DescriptorDescTy::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 ); - } else { + } + dynamic_offset_index += 1; + } + DescriptorDescTy::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 {}", @@ -236,8 +236,9 @@ impl DescriptorSetWithOffsets { min_uniform_off_align ); } + dynamic_offset_index += 1; } - dynamic_offset_index += 1; + _ => (), } } diff --git a/vulkano/src/descriptor_set/persistent.rs b/vulkano/src/descriptor_set/persistent.rs index c949f882..320fb21e 100644 --- a/vulkano/src/descriptor_set/persistent.rs +++ b/vulkano/src/descriptor_set/persistent.rs @@ -29,10 +29,8 @@ use crate::buffer::BufferAccess; use crate::buffer::BufferViewRef; use crate::descriptor_set::layout::DescriptorDesc; +use crate::descriptor_set::layout::DescriptorDescImage; use crate::descriptor_set::layout::DescriptorDescTy; -use crate::descriptor_set::layout::DescriptorImageDesc; -use crate::descriptor_set::layout::DescriptorImageDescArray; -use crate::descriptor_set::layout::DescriptorImageDescDimensions; use crate::descriptor_set::layout::DescriptorSetLayout; use crate::descriptor_set::layout::DescriptorType; use crate::descriptor_set::pool::standard::StdDescriptorPoolAlloc; @@ -45,6 +43,7 @@ use crate::device::Device; use crate::device::DeviceOwned; use crate::format::Format; use crate::image::view::ImageViewAbstract; +use crate::image::view::ImageViewType; use crate::image::SampleCount; use crate::sampler::Sampler; use crate::OomError; @@ -398,14 +397,14 @@ impl PersistentDescriptorSetBuilderArray { pub fn leave_array( mut self, ) -> Result, PersistentDescriptorSetError> { - if self.desc.array_count > self.array_element as u32 { + if self.desc.descriptor_count > self.array_element as u32 { return Err(PersistentDescriptorSetError::MissingArrayElements { - expected: self.desc.array_count, + expected: self.desc.descriptor_count, obtained: self.array_element as u32, }); } - debug_assert_eq!(self.desc.array_count, self.array_element as u32); + debug_assert_eq!(self.desc.descriptor_count, self.array_element as u32); self.builder.binding_id += 1; Ok(self.builder) @@ -434,21 +433,21 @@ impl PersistentDescriptorSetBuilderArray { buffer.inner().buffer.device().internal_object() ); - if self.array_element as u32 >= self.desc.array_count { + if self.array_element as u32 >= self.desc.descriptor_count { return Err(PersistentDescriptorSetError::ArrayOutOfBounds); } - self.builder.writes.push(match self.desc.ty { - DescriptorDescTy::Buffer(ref buffer_desc) => { - // 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 ^ + // 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 + // 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 + self.builder.writes.push(match self.desc.ty { + DescriptorDescTy::StorageBuffer | DescriptorDescTy::StorageBufferDynamic => { assert!( self.builder .layout @@ -457,44 +456,64 @@ impl PersistentDescriptorSetBuilderArray { .robust_buffer_access ); - if buffer_desc.storage { - if !buffer.inner().buffer.usage().storage_buffer { - return Err(PersistentDescriptorSetError::MissingBufferUsage( - MissingBufferUsage::StorageBuffer, - )); - } + if !buffer.inner().buffer.usage().storage_buffer { + return Err(PersistentDescriptorSetError::MissingBufferUsage( + MissingBufferUsage::StorageBuffer, + )); + } - unsafe { - DescriptorWrite::storage_buffer( - self.builder.binding_id as u32, - self.array_element as u32, - &buffer, - ) - } - } else { - if !buffer.inner().buffer.usage().uniform_buffer { - return Err(PersistentDescriptorSetError::MissingBufferUsage( - MissingBufferUsage::UniformBuffer, - )); - } + unsafe { + DescriptorWrite::storage_buffer( + self.builder.binding_id as u32, + self.array_element as u32, + &buffer, + ) + } + } + DescriptorDescTy::UniformBuffer => { + assert!( + self.builder + .layout + .device() + .enabled_features() + .robust_buffer_access + ); - if buffer_desc.dynamic.unwrap_or(false) { - unsafe { - DescriptorWrite::dynamic_uniform_buffer( - self.builder.binding_id as u32, - self.array_element as u32, - &buffer, - ) - } - } else { - unsafe { - DescriptorWrite::uniform_buffer( - self.builder.binding_id as u32, - self.array_element as u32, - &buffer, - ) - } - } + if !buffer.inner().buffer.usage().uniform_buffer { + return Err(PersistentDescriptorSetError::MissingBufferUsage( + MissingBufferUsage::UniformBuffer, + )); + } + + unsafe { + DescriptorWrite::uniform_buffer( + self.builder.binding_id as u32, + self.array_element as u32, + &buffer, + ) + } + } + DescriptorDescTy::UniformBufferDynamic => { + assert!( + self.builder + .layout + .device() + .enabled_features() + .robust_buffer_access + ); + + if !buffer.inner().buffer.usage().uniform_buffer { + return Err(PersistentDescriptorSetError::MissingBufferUsage( + MissingBufferUsage::UniformBuffer, + )); + } + + unsafe { + DescriptorWrite::dynamic_uniform_buffer( + self.builder.binding_id as u32, + self.array_element as u32, + &buffer, + ) } } ref d => { @@ -543,39 +562,38 @@ impl PersistentDescriptorSetBuilderArray { view.view().device().internal_object() ); - if self.array_element as u32 >= self.desc.array_count { + if self.array_element as u32 >= self.desc.descriptor_count { return Err(PersistentDescriptorSetError::ArrayOutOfBounds); } self.builder.writes.push(match self.desc.ty { - DescriptorDescTy::TexelBuffer { storage, .. } => { - if storage { - // TODO: storage_texel_buffer_atomic + DescriptorDescTy::StorageTexelBuffer { .. } => { + // TODO: storage_texel_buffer_atomic - if !view.view().storage_texel_buffer() { - return Err(PersistentDescriptorSetError::MissingBufferUsage( - MissingBufferUsage::StorageTexelBuffer, - )); - } - - DescriptorWrite::storage_texel_buffer( - self.builder.binding_id as u32, - self.array_element as u32, - view.view(), - ) - } else { - if !view.view().uniform_texel_buffer() { - return Err(PersistentDescriptorSetError::MissingBufferUsage( - MissingBufferUsage::UniformTexelBuffer, - )); - } - - DescriptorWrite::uniform_texel_buffer( - self.builder.binding_id as u32, - self.array_element as u32, - view.view(), - ) + if !view.view().storage_texel_buffer() { + return Err(PersistentDescriptorSetError::MissingBufferUsage( + MissingBufferUsage::StorageTexelBuffer, + )); } + + DescriptorWrite::storage_texel_buffer( + self.builder.binding_id as u32, + self.array_element as u32, + view.view(), + ) + } + DescriptorDescTy::UniformTexelBuffer { .. } => { + if !view.view().uniform_texel_buffer() { + return Err(PersistentDescriptorSetError::MissingBufferUsage( + MissingBufferUsage::UniformTexelBuffer, + )); + } + + DescriptorWrite::uniform_texel_buffer( + self.builder.binding_id as u32, + self.array_element as u32, + view.view(), + ) } ref d => { return Err(PersistentDescriptorSetError::WrongDescriptorTy { expected: d.ty() }); @@ -623,7 +641,7 @@ impl PersistentDescriptorSetBuilderArray { image_view.image().inner().image.device().internal_object() ); - if self.array_element as u32 >= self.desc.array_count { + if self.array_element as u32 >= self.desc.descriptor_count { return Err(PersistentDescriptorSetError::ArrayOutOfBounds); } @@ -633,31 +651,41 @@ impl PersistentDescriptorSetBuilderArray { }; self.builder.writes.push(match desc.ty { - DescriptorDescTy::Image(ref desc) => { + DescriptorDescTy::SampledImage(ref desc) => { + if !image_view.image().inner().image.usage().sampled { + return Err(PersistentDescriptorSetError::MissingImageUsage( + MissingImageUsage::Sampled, + )); + } + image_match_desc(&image_view, &desc)?; - if desc.sampled { - DescriptorWrite::sampled_image( - self.builder.binding_id as u32, - self.array_element as u32, - &image_view, - ) - } else { - if !image_view.component_mapping().is_identity() { - return Err(PersistentDescriptorSetError::NotIdentitySwizzled); - } - - DescriptorWrite::storage_image( - self.builder.binding_id as u32, - self.array_element as u32, - &image_view, - ) - } + DescriptorWrite::sampled_image( + self.builder.binding_id as u32, + self.array_element as u32, + &image_view, + ) } - DescriptorDescTy::InputAttachment { - multisampled, - array_layers, - } => { + DescriptorDescTy::StorageImage(ref desc) => { + if !image_view.image().inner().image.usage().storage { + return Err(PersistentDescriptorSetError::MissingImageUsage( + MissingImageUsage::Storage, + )); + } + + image_match_desc(&image_view, &desc)?; + + if !image_view.component_mapping().is_identity() { + return Err(PersistentDescriptorSetError::NotIdentitySwizzled); + } + + DescriptorWrite::storage_image( + self.builder.binding_id as u32, + self.array_element as u32, + &image_view, + ) + } + DescriptorDescTy::InputAttachment { multisampled } => { if !image_view.image().inner().image.usage().input_attachment { return Err(PersistentDescriptorSetError::MissingImageUsage( MissingImageUsage::InputAttachment, @@ -677,28 +705,9 @@ impl PersistentDescriptorSetBuilderArray { let image_layers = image_view.array_layers(); let num_layers = image_layers.end - image_layers.start; - match array_layers { - DescriptorImageDescArray::NonArrayed => { - if num_layers != 1 { - return Err(PersistentDescriptorSetError::ArrayLayersMismatch { - expected: 1, - obtained: num_layers, - }); - } - } - DescriptorImageDescArray::Arrayed { - max_layers: Some(max_layers), - } => { - if num_layers > max_layers { - // TODO: is this correct? "max" layers? or is it in fact min layers? - return Err(PersistentDescriptorSetError::ArrayLayersMismatch { - expected: max_layers, - obtained: num_layers, - }); - } - } - DescriptorImageDescArray::Arrayed { max_layers: None } => {} - }; + if image_view.ty().is_arrayed() { + return Err(PersistentDescriptorSetError::UnexpectedArrayed); + } DescriptorWrite::input_attachment( self.builder.binding_id as u32, @@ -760,7 +769,7 @@ impl PersistentDescriptorSetBuilderArray { sampler.device().internal_object() ); - if self.array_element as u32 >= self.desc.array_count { + if self.array_element as u32 >= self.desc.descriptor_count { return Err(PersistentDescriptorSetError::ArrayOutOfBounds); } @@ -769,6 +778,12 @@ impl PersistentDescriptorSetBuilderArray { None => return Err(PersistentDescriptorSetError::EmptyExpected), }; + if !image_view.image().inner().image.usage().sampled { + return Err(PersistentDescriptorSetError::MissingImageUsage( + MissingImageUsage::Sampled, + )); + } + if !image_view.can_be_sampled(&sampler) { return Err(PersistentDescriptorSetError::IncompatibleImageViewSampler); } @@ -829,7 +844,7 @@ impl PersistentDescriptorSetBuilderArray { sampler.device().internal_object() ); - if self.array_element as u32 >= self.desc.array_count { + if self.array_element as u32 >= self.desc.descriptor_count { return Err(PersistentDescriptorSetError::ArrayOutOfBounds); } @@ -868,26 +883,15 @@ impl PersistentDescriptorSetBuilderArray { // Checks whether an image view matches the descriptor. fn image_match_desc( image_view: &I, - desc: &DescriptorImageDesc, + desc: &DescriptorDescImage, ) -> Result<(), PersistentDescriptorSetError> where I: ?Sized + ImageViewAbstract, { - if desc.sampled && !image_view.image().inner().image.usage().sampled { - return Err(PersistentDescriptorSetError::MissingImageUsage( - MissingImageUsage::Sampled, - )); - } else if !desc.sampled && !image_view.image().inner().image.usage().storage { - return Err(PersistentDescriptorSetError::MissingImageUsage( - MissingImageUsage::Storage, - )); - } - - let image_view_ty = DescriptorImageDescDimensions::from_image_view_type(image_view.ty()); - if image_view_ty != desc.dimensions { + if image_view.ty() != desc.view_type { return Err(PersistentDescriptorSetError::ImageViewTypeMismatch { - expected: desc.dimensions, - obtained: image_view_ty, + expected: desc.view_type, + obtained: image_view.ty(), }); } @@ -906,46 +910,6 @@ where return Err(PersistentDescriptorSetError::UnexpectedMultisampled); } - let image_layers = image_view.array_layers(); - let num_layers = image_layers.end - image_layers.start; - - match desc.array_layers { - DescriptorImageDescArray::NonArrayed => { - // TODO: when a non-array is expected, can we pass an image view that is in fact an - // array with one layer? need to check - let required_layers = if desc.dimensions == DescriptorImageDescDimensions::Cube { - 6 - } else { - 1 - }; - - if num_layers != required_layers { - return Err(PersistentDescriptorSetError::ArrayLayersMismatch { - expected: 1, - obtained: num_layers, - }); - } - } - DescriptorImageDescArray::Arrayed { - max_layers: Some(max_layers), - } => { - let required_layers = if desc.dimensions == DescriptorImageDescDimensions::Cube { - max_layers * 6 - } else { - max_layers - }; - - // TODO: is this correct? "max" layers? or is it in fact min layers? - if num_layers > required_layers { - return Err(PersistentDescriptorSetError::ArrayLayersMismatch { - expected: max_layers, - obtained: num_layers, - }); - } - } - DescriptorImageDescArray::Arrayed { max_layers: None } => {} - }; - Ok(()) } @@ -1147,12 +1111,12 @@ pub enum MissingImageUsage { /// Error related to the persistent descriptor set. #[derive(Debug, Clone)] pub enum PersistentDescriptorSetError { - /// The number of array layers of an image doesn't match what was expected. - ArrayLayersMismatch { - /// Number of expected array layers for the image. - expected: u32, - /// Number of array layers of the image that was added. - obtained: u32, + /// The arrayed-ness of an image view doesn't match what was expected. + ArrayedMismatch { + /// Whether the shader expects an arrayed image. + expected: bool, + /// Whether an arrayed image view was provided. + obtained: bool, }, /// Tried to add too many elements to an array. @@ -1175,9 +1139,9 @@ pub enum PersistentDescriptorSetError { /// The type of an image view doesn't match what was expected. ImageViewTypeMismatch { /// Expected type. - expected: DescriptorImageDescDimensions, + expected: ImageViewType, /// Type of the image view that was passed. - obtained: DescriptorImageDescDimensions, + obtained: ImageViewType, }, /// The image view isn't compatible with the sampler. @@ -1200,6 +1164,9 @@ pub enum PersistentDescriptorSetError { /// The image view has a component swizzle that is different from identity. NotIdentitySwizzled, + /// Expected a non-arrayed image, but got an arrayed image. + UnexpectedArrayed, + /// Expected a single-sampled image, but got a multisampled image. UnexpectedMultisampled, @@ -1219,8 +1186,8 @@ impl fmt::Display for PersistentDescriptorSetError { fmt, "{}", match *self { - PersistentDescriptorSetError::ArrayLayersMismatch { .. } => { - "the number of array layers of an image doesn't match what was expected" + PersistentDescriptorSetError::ArrayedMismatch { .. } => { + "the arrayed-ness of an image view doesn't match what was expected" } PersistentDescriptorSetError::ArrayOutOfBounds => { "tried to add too many elements to an array" @@ -1252,6 +1219,9 @@ impl fmt::Display for PersistentDescriptorSetError { PersistentDescriptorSetError::NotIdentitySwizzled => { "the image view's component mapping is not identity swizzled" } + PersistentDescriptorSetError::UnexpectedArrayed => { + "expected a non-arrayed image, but got an arrayed image" + } PersistentDescriptorSetError::UnexpectedMultisampled => { "expected a single-sampled image, but got a multisampled image" } diff --git a/vulkano/src/descriptor_set/pool/standard.rs b/vulkano/src/descriptor_set/pool/standard.rs index 287cac1f..c08dc127 100644 --- a/vulkano/src/descriptor_set/pool/standard.rs +++ b/vulkano/src/descriptor_set/pool/standard.rs @@ -194,9 +194,9 @@ mod tests { let desc = DescriptorDesc { ty: DescriptorDescTy::Sampler, - array_count: 1, + descriptor_count: 1, stages: ShaderStages::all(), - readonly: false, + mutable: true, }; let layout = DescriptorSetLayout::new( device.clone(), diff --git a/vulkano/src/descriptor_set/pool/sys.rs b/vulkano/src/descriptor_set/pool/sys.rs index 134ddc7c..863d0a31 100644 --- a/vulkano/src/descriptor_set/pool/sys.rs +++ b/vulkano/src/descriptor_set/pool/sys.rs @@ -373,7 +373,6 @@ impl ExactSizeIterator for UnsafeDescriptorPoolAllocIter {} #[cfg(test)] mod tests { - use crate::descriptor_set::layout::DescriptorBufferDesc; use crate::descriptor_set::layout::DescriptorDesc; use crate::descriptor_set::layout::DescriptorDescTy; use crate::descriptor_set::layout::DescriptorSetDesc; @@ -421,13 +420,10 @@ mod tests { let (device, _) = gfx_dev_and_queue!(); let layout = DescriptorDesc { - ty: DescriptorDescTy::Buffer(DescriptorBufferDesc { - dynamic: Some(false), - storage: false, - }), - array_count: 1, + ty: DescriptorDescTy::UniformBuffer, + descriptor_count: 1, stages: ShaderStages::all_graphics(), - readonly: true, + mutable: false, }; let set_layout = DescriptorSetLayout::new( @@ -454,13 +450,10 @@ mod tests { let (device2, _) = gfx_dev_and_queue!(); let layout = DescriptorDesc { - ty: DescriptorDescTy::Buffer(DescriptorBufferDesc { - dynamic: Some(false), - storage: false, - }), - array_count: 1, + ty: DescriptorDescTy::UniformBuffer, + descriptor_count: 1, stages: ShaderStages::all_graphics(), - readonly: true, + mutable: false, }; let set_layout = diff --git a/vulkano/src/image/view.rs b/vulkano/src/image/view.rs index 894a3433..099af22c 100644 --- a/vulkano/src/image/view.rs +++ b/vulkano/src/image/view.rs @@ -181,12 +181,10 @@ where (ImageViewType::Dim1dArray, ImageDimensions::Dim1d { .. }, _, _) => (), (ImageViewType::Dim2d, ImageDimensions::Dim2d { .. }, 1, _) => (), (ImageViewType::Dim2dArray, ImageDimensions::Dim2d { .. }, _, _) => (), - (ImageViewType::Cubemap, ImageDimensions::Dim2d { .. }, 6, _) - if flags.cube_compatible => - { + (ImageViewType::Cube, ImageDimensions::Dim2d { .. }, 6, _) if flags.cube_compatible => { () } - (ImageViewType::CubemapArray, ImageDimensions::Dim2d { .. }, n, _) + (ImageViewType::CubeArray, ImageDimensions::Dim2d { .. }, n, _) if flags.cube_compatible && n % 6 == 0 => { () @@ -407,8 +405,18 @@ pub enum ImageViewType { Dim2d = ash::vk::ImageViewType::TYPE_2D.as_raw(), Dim2dArray = ash::vk::ImageViewType::TYPE_2D_ARRAY.as_raw(), Dim3d = ash::vk::ImageViewType::TYPE_3D.as_raw(), - Cubemap = ash::vk::ImageViewType::CUBE.as_raw(), - CubemapArray = ash::vk::ImageViewType::CUBE_ARRAY.as_raw(), + Cube = ash::vk::ImageViewType::CUBE.as_raw(), + CubeArray = ash::vk::ImageViewType::CUBE_ARRAY.as_raw(), +} + +impl ImageViewType { + #[inline] + pub fn is_arrayed(&self) -> bool { + match self { + Self::Dim1d | Self::Dim2d | Self::Dim3d | Self::Cube => false, + Self::Dim1dArray | Self::Dim2dArray | Self::CubeArray => true, + } + } } impl From for ash::vk::ImageViewType { diff --git a/vulkano/src/pipeline/compute_pipeline.rs b/vulkano/src/pipeline/compute_pipeline.rs index 124eff36..812bdc3e 100644 --- a/vulkano/src/pipeline/compute_pipeline.rs +++ b/vulkano/src/pipeline/compute_pipeline.rs @@ -107,7 +107,7 @@ impl ComputePipeline { } unsafe { - pipeline_layout.ensure_superset_of( + pipeline_layout.ensure_compatible_with_shader( shader.descriptor_set_layout_descs(), shader.push_constant_range(), )?; @@ -343,7 +343,6 @@ mod tests { use crate::buffer::CpuAccessibleBuffer; use crate::command_buffer::AutoCommandBufferBuilder; use crate::command_buffer::CommandBufferUsage; - use crate::descriptor_set::layout::DescriptorBufferDesc; use crate::descriptor_set::layout::DescriptorDesc; use crate::descriptor_set::layout::DescriptorDescTy; use crate::descriptor_set::layout::DescriptorSetDesc; @@ -414,16 +413,13 @@ mod tests { module.compute_entry_point( CStr::from_ptr(NAME.as_ptr() as *const _), [DescriptorSetDesc::new([Some(DescriptorDesc { - ty: DescriptorDescTy::Buffer(DescriptorBufferDesc { - dynamic: Some(false), - storage: true, - }), - array_count: 1, + ty: DescriptorDescTy::StorageBuffer, + descriptor_count: 1, stages: ShaderStages { compute: true, ..ShaderStages::none() }, - readonly: true, + mutable: false, })])], None, SpecConsts::descriptors(), diff --git a/vulkano/src/pipeline/graphics_pipeline/builder.rs b/vulkano/src/pipeline/graphics_pipeline/builder.rs index 1223a59b..427d4a3d 100644 --- a/vulkano/src/pipeline/graphics_pipeline/builder.rs +++ b/vulkano/src/pipeline/graphics_pipeline/builder.rs @@ -192,17 +192,19 @@ where match range_map.entry((range.offset, range.size)) { Entry::Vacant(entry) => { entry.insert(range.stages); - }, + } Entry::Occupied(mut entry) => { *entry.get_mut() = *entry.get() | range.stages; - }, + } } } } let push_constant_ranges: Vec<_> = range_map .iter() - .map(|((offset, size), stages)| { - PipelineLayoutPcRange { offset: *offset, size: *size, stages: *stages } + .map(|((offset, size), stages)| PipelineLayoutPcRange { + offset: *offset, + size: *size, + stages: *stages, }) .collect(); @@ -239,7 +241,7 @@ where { let shader = &self.vertex_shader.as_ref().unwrap().0; - pipeline_layout.ensure_superset_of( + pipeline_layout.ensure_compatible_with_shader( shader.descriptor_set_layout_descs(), shader.push_constant_range(), )?; @@ -247,7 +249,7 @@ where if let Some(ref geometry_shader) = self.geometry_shader { let shader = &geometry_shader.0; - pipeline_layout.ensure_superset_of( + pipeline_layout.ensure_compatible_with_shader( shader.descriptor_set_layout_descs(), shader.push_constant_range(), )?; @@ -256,7 +258,7 @@ where if let Some(ref tess) = self.tessellation { { let shader = &tess.tessellation_control_shader.0; - pipeline_layout.ensure_superset_of( + pipeline_layout.ensure_compatible_with_shader( shader.descriptor_set_layout_descs(), shader.push_constant_range(), )?; @@ -264,7 +266,7 @@ where { let shader = &tess.tessellation_evaluation_shader.0; - pipeline_layout.ensure_superset_of( + pipeline_layout.ensure_compatible_with_shader( shader.descriptor_set_layout_descs(), shader.push_constant_range(), )?; @@ -273,7 +275,7 @@ where if let Some(ref fragment_shader) = self.fragment_shader { let shader = &fragment_shader.0; - pipeline_layout.ensure_superset_of( + pipeline_layout.ensure_compatible_with_shader( shader.descriptor_set_layout_descs(), shader.push_constant_range(), )?; @@ -795,26 +797,10 @@ where return Err(GraphicsPipelineCreationError::MaxViewportDimensionsExceeded); } - if vp.x - < device - .physical_device() - .properties() - .viewport_bounds_range[0] - || vp.x + vp.width - > device - .physical_device() - .properties() - .viewport_bounds_range[1] - || vp.y - < device - .physical_device() - .properties() - .viewport_bounds_range[0] - || vp.y + vp.height - > device - .physical_device() - .properties() - .viewport_bounds_range[1] + if vp.x < device.physical_device().properties().viewport_bounds_range[0] + || vp.x + vp.width > device.physical_device().properties().viewport_bounds_range[1] + || vp.y < device.physical_device().properties().viewport_bounds_range[0] + || vp.y + vp.height > device.physical_device().properties().viewport_bounds_range[1] { return Err(GraphicsPipelineCreationError::ViewportBoundsExceeded); } diff --git a/vulkano/src/pipeline/layout/limits_check.rs b/vulkano/src/pipeline/layout/limits_check.rs index 8f98f260..0f2bb874 100644 --- a/vulkano/src/pipeline/layout/limits_check.rs +++ b/vulkano/src/pipeline/layout/limits_check.rs @@ -36,39 +36,40 @@ pub fn check_desc_against_limits( for set in descriptor_set_layouts { for descriptor in (0..set.num_bindings()).filter_map(|i| set.descriptor(i).map(|d| d)) { - num_resources.increment(descriptor.array_count, &descriptor.stages); + num_resources.increment(descriptor.descriptor_count, &descriptor.stages); match descriptor.ty.ty() { // TODO: DescriptorType::Sampler => { - num_samplers.increment(descriptor.array_count, &descriptor.stages); + num_samplers.increment(descriptor.descriptor_count, &descriptor.stages); } DescriptorType::CombinedImageSampler => { - num_samplers.increment(descriptor.array_count, &descriptor.stages); - num_sampled_images.increment(descriptor.array_count, &descriptor.stages); + num_samplers.increment(descriptor.descriptor_count, &descriptor.stages); + num_sampled_images.increment(descriptor.descriptor_count, &descriptor.stages); } DescriptorType::SampledImage | DescriptorType::UniformTexelBuffer => { - num_sampled_images.increment(descriptor.array_count, &descriptor.stages); + num_sampled_images.increment(descriptor.descriptor_count, &descriptor.stages); } DescriptorType::StorageImage | DescriptorType::StorageTexelBuffer => { - num_storage_images.increment(descriptor.array_count, &descriptor.stages); + num_storage_images.increment(descriptor.descriptor_count, &descriptor.stages); } DescriptorType::UniformBuffer => { - num_uniform_buffers.increment(descriptor.array_count, &descriptor.stages); + num_uniform_buffers.increment(descriptor.descriptor_count, &descriptor.stages); } DescriptorType::UniformBufferDynamic => { - num_uniform_buffers.increment(descriptor.array_count, &descriptor.stages); + num_uniform_buffers.increment(descriptor.descriptor_count, &descriptor.stages); num_uniform_buffers_dynamic += 1; } DescriptorType::StorageBuffer => { - num_storage_buffers.increment(descriptor.array_count, &descriptor.stages); + num_storage_buffers.increment(descriptor.descriptor_count, &descriptor.stages); } DescriptorType::StorageBufferDynamic => { - num_storage_buffers.increment(descriptor.array_count, &descriptor.stages); + num_storage_buffers.increment(descriptor.descriptor_count, &descriptor.stages); num_storage_buffers_dynamic += 1; } DescriptorType::InputAttachment => { - num_input_attachments.increment(descriptor.array_count, &descriptor.stages); + num_input_attachments + .increment(descriptor.descriptor_count, &descriptor.stages); } } } @@ -98,9 +99,7 @@ pub fn check_desc_against_limits( }, ); } - if num_uniform_buffers.max_per_stage() - > properties.max_per_stage_descriptor_uniform_buffers - { + if num_uniform_buffers.max_per_stage() > properties.max_per_stage_descriptor_uniform_buffers { return Err( PipelineLayoutLimitsError::MaxPerStageDescriptorUniformBuffersLimitExceeded { limit: properties.max_per_stage_descriptor_uniform_buffers, @@ -108,9 +107,7 @@ pub fn check_desc_against_limits( }, ); } - if num_storage_buffers.max_per_stage() - > properties.max_per_stage_descriptor_storage_buffers - { + if num_storage_buffers.max_per_stage() > properties.max_per_stage_descriptor_storage_buffers { return Err( PipelineLayoutLimitsError::MaxPerStageDescriptorStorageBuffersLimitExceeded { limit: properties.max_per_stage_descriptor_storage_buffers, @@ -118,9 +115,7 @@ pub fn check_desc_against_limits( }, ); } - if num_sampled_images.max_per_stage() - > properties.max_per_stage_descriptor_sampled_images - { + if num_sampled_images.max_per_stage() > properties.max_per_stage_descriptor_sampled_images { return Err( PipelineLayoutLimitsError::MaxPerStageDescriptorSampledImagesLimitExceeded { limit: properties.max_per_stage_descriptor_sampled_images, @@ -128,9 +123,7 @@ pub fn check_desc_against_limits( }, ); } - if num_storage_images.max_per_stage() - > properties.max_per_stage_descriptor_storage_images - { + if num_storage_images.max_per_stage() > properties.max_per_stage_descriptor_storage_images { return Err( PipelineLayoutLimitsError::MaxPerStageDescriptorStorageImagesLimitExceeded { limit: properties.max_per_stage_descriptor_storage_images, @@ -138,14 +131,11 @@ pub fn check_desc_against_limits( }, ); } - if num_input_attachments.max_per_stage() - > properties - .max_per_stage_descriptor_input_attachments + if num_input_attachments.max_per_stage() > properties.max_per_stage_descriptor_input_attachments { return Err( PipelineLayoutLimitsError::MaxPerStageDescriptorInputAttachmentsLimitExceeded { - limit: properties - .max_per_stage_descriptor_input_attachments, + limit: properties.max_per_stage_descriptor_input_attachments, requested: num_input_attachments.max_per_stage(), }, ); @@ -167,14 +157,10 @@ pub fn check_desc_against_limits( }, ); } - if num_uniform_buffers_dynamic - > properties - .max_descriptor_set_uniform_buffers_dynamic - { + if num_uniform_buffers_dynamic > properties.max_descriptor_set_uniform_buffers_dynamic { return Err( PipelineLayoutLimitsError::MaxDescriptorSetUniformBuffersDynamicLimitExceeded { - limit: properties - .max_descriptor_set_uniform_buffers_dynamic, + limit: properties.max_descriptor_set_uniform_buffers_dynamic, requested: num_uniform_buffers_dynamic, }, ); @@ -187,14 +173,10 @@ pub fn check_desc_against_limits( }, ); } - if num_storage_buffers_dynamic - > properties - .max_descriptor_set_storage_buffers_dynamic - { + if num_storage_buffers_dynamic > properties.max_descriptor_set_storage_buffers_dynamic { return Err( PipelineLayoutLimitsError::MaxDescriptorSetStorageBuffersDynamicLimitExceeded { - limit: properties - .max_descriptor_set_storage_buffers_dynamic, + limit: properties.max_descriptor_set_storage_buffers_dynamic, requested: num_storage_buffers_dynamic, }, ); diff --git a/vulkano/src/pipeline/layout/sys.rs b/vulkano/src/pipeline/layout/sys.rs index 7d6d33bb..4efda658 100644 --- a/vulkano/src/pipeline/layout/sys.rs +++ b/vulkano/src/pipeline/layout/sys.rs @@ -9,8 +9,8 @@ use super::limits_check; use crate::check_errors; +use crate::descriptor_set::layout::DescriptorSetCompatibilityError; use crate::descriptor_set::layout::DescriptorSetDesc; -use crate::descriptor_set::layout::DescriptorSetDescSupersetError; use crate::descriptor_set::layout::DescriptorSetLayout; use crate::device::Device; use crate::device::DeviceOwned; @@ -167,7 +167,7 @@ impl PipelineLayout { /// Makes sure that `self` is a superset of the provided descriptor set layouts and push /// constant ranges. Returns an `Err` if this is not the case. - pub fn ensure_superset_of( + pub fn ensure_compatible_with_shader( &self, descriptor_set_layout_descs: &[DescriptorSetDesc], push_constant_range: &Option, @@ -189,7 +189,7 @@ impl PipelineLayout { .get(set_num) .unwrap_or_else(|| &empty); - if let Err(error) = first.ensure_superset_of(second) { + if let Err(error) = first.ensure_compatible_with_shader(second) { return Err(PipelineLayoutSupersetError::DescriptorSet { error, set_num: set_num as u32, @@ -202,7 +202,8 @@ impl PipelineLayout { for own_range in self.push_constant_ranges.as_ref().into_iter() { if range.stages.intersects(&own_range.stages) && // check if it shares any stages (range.offset < own_range.offset || // our range must start before and end after the given range - own_range.offset + own_range.size < range.offset + range.size) { + own_range.offset + own_range.size < range.offset + range.size) + { return Err(PipelineLayoutSupersetError::PushConstantRange { first_range: *own_range, second_range: *range, @@ -338,7 +339,7 @@ impl From for PipelineLayoutCreationError { #[derive(Clone, Debug, PartialEq, Eq)] pub enum PipelineLayoutSupersetError { DescriptorSet { - error: DescriptorSetDescSupersetError, + error: DescriptorSetCompatibilityError, set_num: u32, }, PushConstantRange { @@ -362,13 +363,16 @@ impl fmt::Display for PipelineLayoutSupersetError { fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { match *self { PipelineLayoutSupersetError::DescriptorSet { .. } => { - write!( + write!(fmt, "the descriptor set was not a superset of the other") + } + PipelineLayoutSupersetError::PushConstantRange { + first_range, + second_range, + } => { + writeln!( fmt, - "the descriptor set was not a superset of the other" - ) - }, - PipelineLayoutSupersetError::PushConstantRange { first_range, second_range } => { - writeln!(fmt, "our range did not completely encompass the other range")?; + "our range did not completely encompass the other range" + )?; writeln!(fmt, " our stages: {:?}", first_range.stages)?; writeln!( fmt, @@ -377,12 +381,13 @@ impl fmt::Display for PipelineLayoutSupersetError { first_range.offset + first_range.size )?; writeln!(fmt, " other stages: {:?}", second_range.stages)?; - write!(fmt, + write!( + fmt, " other range: {} - {}", second_range.offset, second_range.offset + second_range.size ) - }, + } } } } diff --git a/vulkano/src/pipeline/shader.rs b/vulkano/src/pipeline/shader.rs index c5ba1e35..5838dd06 100644 --- a/vulkano/src/pipeline/shader.rs +++ b/vulkano/src/pipeline/shader.rs @@ -681,24 +681,16 @@ impl ShaderStages { } } - /// Checks whether we have more stages enabled than `other`. + /// Returns whether `self` contains all the stages of `other`. // TODO: add example #[inline] - pub const fn ensure_superset_of( - &self, - other: &ShaderStages, - ) -> Result<(), ShaderStagesSupersetError> { - if (self.vertex || !other.vertex) + pub const fn is_superset_of(&self, other: &ShaderStages) -> bool { + (self.vertex || !other.vertex) && (self.tessellation_control || !other.tessellation_control) && (self.tessellation_evaluation || !other.tessellation_evaluation) && (self.geometry || !other.geometry) && (self.fragment || !other.fragment) && (self.compute || !other.compute) - { - Ok(()) - } else { - Err(ShaderStagesSupersetError::NotSuperset) - } } /// Checks whether any of the stages in `self` are also present in `other`. @@ -785,24 +777,3 @@ impl From for PipelineStages { } } } - -/// Error when checking that a `ShaderStages` object is a superset of another. -#[derive(Debug, Clone)] -pub enum ShaderStagesSupersetError { - NotSuperset, -} - -impl error::Error for ShaderStagesSupersetError {} - -impl fmt::Display for ShaderStagesSupersetError { - #[inline] - fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> { - write!( - fmt, - "{}", - match *self { - ShaderStagesSupersetError::NotSuperset => "shader stages not a superset", - } - ) - } -}