From fa15e53820e1fc10ab51a44911a24ea69f228a32 Mon Sep 17 00:00:00 2001 From: Rua Date: Thu, 7 Dec 2023 18:13:08 +0100 Subject: [PATCH] Validate the fragment output against color blend state (#2420) * Validate the fragment output against color blend state * Remove old methods from render/subpass that are no longer needed * Better fix * Update vulkano/src/macros.rs Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com> * Update vulkano/src/pipeline/graphics/mod.rs Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com> --------- Co-authored-by: marc0246 <40955683+marc0246@users.noreply.github.com> --- examples/deferred/triangle_draw_system.rs | 4 +- .../src/command_buffer/commands/pipeline.rs | 40 +- vulkano/src/macros.rs | 6 + vulkano/src/pipeline/graphics/color_blend.rs | 309 +++++++++++ vulkano/src/pipeline/graphics/mod.rs | 97 ++-- .../src/pipeline/graphics/vertex_input/mod.rs | 519 ++++-------------- vulkano/src/pipeline/shader.rs | 354 ++++++++++-- vulkano/src/render_pass/mod.rs | 43 -- 8 files changed, 817 insertions(+), 555 deletions(-) diff --git a/examples/deferred/triangle_draw_system.rs b/examples/deferred/triangle_draw_system.rs index 8e06b0a2..f5f0e3b3 100644 --- a/examples/deferred/triangle_draw_system.rs +++ b/examples/deferred/triangle_draw_system.rs @@ -189,11 +189,11 @@ mod fs { #version 450 layout(location = 0) out vec4 f_color; - layout(location = 1) out vec3 f_normal; + layout(location = 1) out vec4 f_normal; void main() { f_color = vec4(1.0, 1.0, 1.0, 1.0); - f_normal = vec3(0.0, 0.0, 1.0); + f_normal = vec4(0.0, 0.0, 1.0, 0.0); } ", } diff --git a/vulkano/src/command_buffer/commands/pipeline.rs b/vulkano/src/command_buffer/commands/pipeline.rs index 0c7b6bab..6cc19b52 100644 --- a/vulkano/src/command_buffer/commands/pipeline.rs +++ b/vulkano/src/command_buffer/commands/pipeline.rs @@ -18,7 +18,7 @@ use crate::{ graphics::{ input_assembly::PrimitiveTopology, subpass::PipelineSubpassType, - vertex_input::{self, RequiredVertexInputsVUIDs, VertexInputRate}, + vertex_input::{RequiredVertexInputsVUIDs, VertexInputRate}, }, DynamicState, GraphicsPipeline, Pipeline, PipelineLayout, }, @@ -2163,28 +2163,28 @@ impl AutoCommandBufferBuilder { } DynamicState::VertexInput => { if let Some(vertex_input_state) = &self.builder_state.vertex_input { - vertex_input::validate_required_vertex_inputs( - &vertex_input_state.attributes, - pipeline.required_vertex_inputs().unwrap(), - RequiredVertexInputsVUIDs { - not_present: vuids!(vuid_type, "Input-07939"), - numeric_type: vuids!(vuid_type, "Input-08734"), - requires32: vuids!(vuid_type, "format-08936"), - requires64: vuids!(vuid_type, "format-08937"), - requires_second_half: vuids!(vuid_type, "None-09203"), - }, - ) - .map_err(|mut err| { - err.problem = format!( - "the currently bound graphics pipeline requires the \ + vertex_input_state + .validate_required_vertex_inputs( + pipeline.required_vertex_inputs().unwrap(), + RequiredVertexInputsVUIDs { + not_present: vuids!(vuid_type, "Input-07939"), + numeric_type: vuids!(vuid_type, "Input-08734"), + requires32: vuids!(vuid_type, "format-08936"), + requires64: vuids!(vuid_type, "format-08937"), + requires_second_half: vuids!(vuid_type, "None-09203"), + }, + ) + .map_err(|mut err| { + err.problem = format!( + "the currently bound graphics pipeline requires the \ `DynamicState::VertexInput` dynamic state, but \ the dynamic vertex input does not meet the requirements of the \ vertex shader in the pipeline: {}", - err.problem, - ) - .into(); - err - })?; + err.problem, + ) + .into(); + err + })?; } else { return Err(Box::new(ValidationError { problem: format!( diff --git a/vulkano/src/macros.rs b/vulkano/src/macros.rs index e57bc251..a8dea17b 100644 --- a/vulkano/src/macros.rs +++ b/vulkano/src/macros.rs @@ -39,6 +39,12 @@ macro_rules! vulkan_bitflags { )* } + /// Returns the number of flags set in `self`. + #[inline] + pub const fn count(self) -> u32 { + self.0.count_ones() + } + /// Returns whether no flags are set in `self`. #[inline] pub const fn is_empty(self) -> bool { diff --git a/vulkano/src/pipeline/graphics/color_blend.rs b/vulkano/src/pipeline/graphics/color_blend.rs index b0c250c4..2674839b 100644 --- a/vulkano/src/pipeline/graphics/color_blend.rs +++ b/vulkano/src/pipeline/graphics/color_blend.rs @@ -12,11 +12,15 @@ //! formats, the logic operation is applied. For normalized integer formats, the logic operation //! will take precedence if it is activated, otherwise the blending operation is applied. +use super::subpass::PipelineSubpassType; use crate::{ device::Device, + format::Format, macros::{vulkan_bitflags, vulkan_enum}, + pipeline::{ShaderInterfaceLocationInfo, ShaderInterfaceLocationWidth}, Requires, RequiresAllOf, RequiresOneOf, ValidationError, }; +use ahash::HashMap; use std::iter; /// Describes how the color output of the fragment shader is written to the attachment. See the @@ -218,6 +222,243 @@ impl ColorBlendState { Ok(()) } + + pub(crate) fn validate_required_fragment_outputs( + &self, + subpass: &PipelineSubpassType, + fragment_shader_outputs: &HashMap, + ) -> Result<(), Box> { + let validate_location = + |attachment_index: usize, format: Format| -> Result<(), Box> { + let &ColorBlendAttachmentState { + ref blend, + color_write_mask, + color_write_enable, + } = &self.attachments[attachment_index]; + + if !color_write_enable || color_write_mask.is_empty() { + return Ok(()); + } + + // An output component is written if it exists in the format, + // and is included in the write mask. + let format_components = format.components(); + let written_output_components = [ + format_components[0] != 0 && color_write_mask.intersects(ColorComponents::R), + format_components[1] != 0 && color_write_mask.intersects(ColorComponents::G), + format_components[2] != 0 && color_write_mask.intersects(ColorComponents::B), + format_components[3] != 0 && color_write_mask.intersects(ColorComponents::A), + ]; + + // Gather the input components (for source0 and source1) that are needed to + // produce the components in `written_components`. + let mut source_components_used = [ColorComponents::empty(); 2]; + + fn add_components(dst: &mut [ColorComponents; 2], src: [ColorComponents; 2]) { + for (dst, src) in dst.iter_mut().zip(src) { + *dst |= src; + } + } + + if let Some(blend) = blend { + let &AttachmentBlend { + src_color_blend_factor, + dst_color_blend_factor, + color_blend_op, + src_alpha_blend_factor, + dst_alpha_blend_factor, + alpha_blend_op, + } = blend; + + let mut add_blend = + |output_component: usize, + blend_op: BlendOp, + blend_factors: [BlendFactor; 2]| { + if written_output_components[output_component] { + add_components( + &mut source_components_used, + blend_op.source_components_used(output_component), + ); + + if blend_op.uses_blend_factors() { + for blend_factor in blend_factors { + add_components( + &mut source_components_used, + blend_factor.source_components_used(output_component), + ); + } + } + } + }; + + add_blend( + 0, + color_blend_op, + [src_color_blend_factor, dst_color_blend_factor], + ); + add_blend( + 1, + color_blend_op, + [src_color_blend_factor, dst_color_blend_factor], + ); + add_blend( + 2, + color_blend_op, + [src_color_blend_factor, dst_color_blend_factor], + ); + add_blend( + 3, + alpha_blend_op, + [src_alpha_blend_factor, dst_alpha_blend_factor], + ); + } else { + let mut add_passthrough = |output_component: usize| { + if written_output_components[output_component] { + add_components( + &mut source_components_used, + [ + ColorComponents::from_index(output_component), + ColorComponents::empty(), + ], + ) + } + }; + + add_passthrough(0); + add_passthrough(1); + add_passthrough(2); + add_passthrough(3); + } + + // If no components from either input source are used, + // then there is nothing more to check. + if source_components_used == [ColorComponents::empty(); 2] { + return Ok(()); + } + + let location = attachment_index as u32; + let location_info = fragment_shader_outputs.get(&location).ok_or_else(|| { + Box::new(ValidationError { + problem: format!( + "the subpass and color blend state of color attachment {0} use the \ + fragment shader output, but \ + the fragment shader does not have an output variable with location {0}", + location, + ) + .into(), + ..Default::default() + }) + })?; + let attachment_numeric_type = format.numeric_format_color().unwrap().numeric_type(); + + if attachment_numeric_type != location_info.numeric_type { + return Err(Box::new(ValidationError { + problem: format!( + "the subpass and color blend state of color attachment {0} use the \ + fragment shader output, but \ + the numeric type of the color attachment format ({1:?}) \ + does not equal the numeric type of the fragment shader output \ + variable with location {0} ({2:?})", + location, attachment_numeric_type, location_info.numeric_type, + ) + .into(), + ..Default::default() + })); + } + + if location_info.width != ShaderInterfaceLocationWidth::Bits32 { + return Err(Box::new(ValidationError { + problem: format!( + "the subpass and color blend state of color attachment {0} use the \ + fragment shader output, and \ + the color attachment format is not 64 bit, but \ + the format of the fragment output variable with location {0} is 64-bit", + location, + ) + .into(), + ..Default::default() + })); + } + + for (index, (source_components_provided, source_components_used)) in location_info + .components + .into_iter() + .zip(source_components_used) + .enumerate() + { + let missing_components = source_components_used - source_components_provided; + + if !missing_components.is_empty() { + let component = if missing_components.intersects(ColorComponents::R) { + 0 + } else if missing_components.intersects(ColorComponents::G) { + 1 + } else if missing_components.intersects(ColorComponents::B) { + 2 + } else if missing_components.intersects(ColorComponents::A) { + 3 + } else { + unreachable!() + }; + + return Err(Box::new(ValidationError { + problem: format!( + "the subpass and color blend state of color attachment {0} use \ + location {0}, index {1}, component {2} from the fragment shader \ + output, but the fragment shader does not have an output variable \ + with that location, index and component", + location, index, component, + ) + .into(), + ..Default::default() + })); + } + } + + Ok(()) + }; + + match subpass { + PipelineSubpassType::BeginRenderPass(subpass) => { + debug_assert_eq!( + self.attachments.len(), + subpass.subpass_desc().color_attachments.len() + ); + let render_pass_attachments = subpass.render_pass().attachments(); + + for (attachment_index, atch_ref) in + subpass.subpass_desc().color_attachments.iter().enumerate() + { + match atch_ref { + Some(atch_ref) => validate_location( + attachment_index, + render_pass_attachments[atch_ref.attachment as usize].format, + )?, + None => continue, + } + } + } + PipelineSubpassType::BeginRendering(rendering_create_info) => { + debug_assert_eq!( + self.attachments.len(), + rendering_create_info.color_attachment_formats.len() + ); + + for (attachment_index, &format) in rendering_create_info + .color_attachment_formats + .iter() + .enumerate() + { + match format { + Some(format) => validate_location(attachment_index, format)?, + None => continue, + } + } + } + } + + Ok(()) + } } vulkan_bitflags! { @@ -725,6 +966,37 @@ vulkan_enum! { OneMinusSrc1Alpha = ONE_MINUS_SRC1_ALPHA, } +impl BlendFactor { + const fn source_components_used(self, output_component: usize) -> [ColorComponents; 2] { + match self { + BlendFactor::Zero + | BlendFactor::One + | BlendFactor::DstColor + | BlendFactor::OneMinusDstColor + | BlendFactor::DstAlpha + | BlendFactor::OneMinusDstAlpha + | BlendFactor::ConstantColor + | BlendFactor::OneMinusConstantColor + | BlendFactor::ConstantAlpha + | BlendFactor::OneMinusConstantAlpha => [ColorComponents::empty(); 2], + BlendFactor::SrcColor | BlendFactor::OneMinusSrcColor => [ + ColorComponents::from_index(output_component), + ColorComponents::empty(), + ], + BlendFactor::Src1Color | BlendFactor::OneMinusSrc1Color => [ + ColorComponents::empty(), + ColorComponents::from_index(output_component), + ], + BlendFactor::SrcAlpha + | BlendFactor::OneMinusSrcAlpha + | BlendFactor::SrcAlphaSaturate => [ColorComponents::A, ColorComponents::empty()], + BlendFactor::Src1Alpha | BlendFactor::OneMinusSrc1Alpha => { + [ColorComponents::empty(), ColorComponents::A] + } + } + } +} + vulkan_enum! { #[non_exhaustive] @@ -1070,6 +1342,30 @@ vulkan_enum! { ]),*/ } +impl BlendOp { + /// Returns whether the blend op will use the specified blend factors, or ignore them. + #[inline] + pub const fn uses_blend_factors(self) -> bool { + match self { + BlendOp::Add | BlendOp::Subtract | BlendOp::ReverseSubtract => true, + BlendOp::Min | BlendOp::Max => false, + } + } + + const fn source_components_used(self, output_component: usize) -> [ColorComponents; 2] { + match self { + BlendOp::Add + | BlendOp::Subtract + | BlendOp::ReverseSubtract + | BlendOp::Min + | BlendOp::Max => [ + ColorComponents::from_index(output_component), + ColorComponents::empty(), + ], + } + } +} + vulkan_bitflags! { /// A mask specifying color components that can be written to a framebuffer attachment. ColorComponents = ColorComponentFlags(u32); @@ -1086,3 +1382,16 @@ vulkan_bitflags! { /// The alpha component. A = A, } + +impl ColorComponents { + #[inline] + pub(crate) const fn from_index(index: usize) -> Self { + match index { + 0 => ColorComponents::R, + 1 => ColorComponents::G, + 2 => ColorComponents::B, + 3 => ColorComponents::A, + _ => unreachable!(), + } + } +} diff --git a/vulkano/src/pipeline/graphics/mod.rs b/vulkano/src/pipeline/graphics/mod.rs index b787e54c..83f7d6e2 100644 --- a/vulkano/src/pipeline/graphics/mod.rs +++ b/vulkano/src/pipeline/graphics/mod.rs @@ -56,12 +56,13 @@ use self::{ rasterization::RasterizationState, subpass::PipelineSubpassType, tessellation::TessellationState, - vertex_input::{RequiredVertexInputsVUIDs, VertexInputLocationRequirements, VertexInputState}, + vertex_input::{RequiredVertexInputsVUIDs, VertexInputState}, viewport::ViewportState, }; use super::{ cache::PipelineCache, shader::validate_interfaces_compatible, DynamicState, Pipeline, PipelineBindPoint, PipelineCreateFlags, PipelineLayout, PipelineShaderStageCreateInfo, + ShaderInterfaceLocationInfo, }; use crate::{ device::{Device, DeviceOwned, DeviceOwnedDebugWrapper}, @@ -80,7 +81,7 @@ use crate::{ }, }, shader::{ - spirv::{ExecutionMode, ExecutionModel, Instruction}, + spirv::{ExecutionMode, ExecutionModel, Instruction, StorageClass}, DescriptorBindingRequirements, ShaderStage, ShaderStages, }, Requires, RequiresAllOf, RequiresOneOf, Validated, ValidationError, VulkanError, VulkanObject, @@ -137,7 +138,7 @@ pub struct GraphicsPipeline { fixed_state: HashSet, fragment_tests_stages: Option, // Note: this is only `Some` if `vertex_input_state` is `None`. - required_vertex_inputs: Option>, + required_vertex_inputs: Option>, } impl GraphicsPipeline { @@ -939,9 +940,10 @@ impl GraphicsPipeline { match entry_point_info.execution_model { ExecutionModel::Vertex => { if vertex_input_state.is_none() { - required_vertex_inputs = Some(vertex_input::required_vertex_inputs( + required_vertex_inputs = Some(super::shader_interface_location_info( entry_point.module().spirv(), entry_point.id(), + StorageClass::Input, )); } } @@ -1182,7 +1184,7 @@ impl GraphicsPipeline { #[inline] pub(crate) fn required_vertex_inputs( &self, - ) -> Option<&HashMap> { + ) -> Option<&HashMap> { self.required_vertex_inputs.as_ref() } } @@ -2421,34 +2423,35 @@ impl GraphicsPipelineCreateInfo { */ if let (Some(vertex_stage), Some(vertex_input_state)) = (vertex_stage, vertex_input_state) { - let required_vertex_inputs = vertex_input::required_vertex_inputs( + let required_vertex_inputs = super::shader_interface_location_info( vertex_stage.entry_point.module().spirv(), vertex_stage.entry_point.id(), + StorageClass::Input, ); - vertex_input::validate_required_vertex_inputs( - &vertex_input_state.attributes, - &required_vertex_inputs, - RequiredVertexInputsVUIDs { - not_present: &["VUID-VkGraphicsPipelineCreateInfo-Input-07904"], - numeric_type: &["VUID-VkGraphicsPipelineCreateInfo-Input-08733"], - requires32: &["VUID-VkGraphicsPipelineCreateInfo-pVertexInputState-08929"], - requires64: &["VUID-VkGraphicsPipelineCreateInfo-pVertexInputState-08930"], - requires_second_half: &[ - "VUID-VkGraphicsPipelineCreateInfo-pVertexInputState-09198", - ], - }, - ) - .map_err(|mut err| { - err.problem = format!( - "{}: {}", - "`vertex_input_state` does not meet the requirements \ - of the vertex shader in `stages`", - err.problem, + vertex_input_state + .validate_required_vertex_inputs( + &required_vertex_inputs, + RequiredVertexInputsVUIDs { + not_present: &["VUID-VkGraphicsPipelineCreateInfo-Input-07904"], + numeric_type: &["VUID-VkGraphicsPipelineCreateInfo-Input-08733"], + requires32: &["VUID-VkGraphicsPipelineCreateInfo-pVertexInputState-08929"], + requires64: &["VUID-VkGraphicsPipelineCreateInfo-pVertexInputState-08930"], + requires_second_half: &[ + "VUID-VkGraphicsPipelineCreateInfo-pVertexInputState-09198", + ], + }, ) - .into(); - err - })?; + .map_err(|mut err| { + err.problem = format!( + "{}: {}", + "`vertex_input_state` does not meet the requirements \ + of the vertex shader in `stages`", + err.problem, + ) + .into(); + err + })?; } if let (Some(_), Some(_)) = (tessellation_control_stage, tessellation_evaluation_stage) { @@ -2508,25 +2511,27 @@ impl GraphicsPipelineCreateInfo { } } - if let (Some(fragment_stage), Some(subpass)) = (fragment_stage, subpass) { - let entry_point_info = fragment_stage.entry_point.info(); + if let (Some(fragment_stage), Some(color_blend_state), Some(subpass)) = + (fragment_stage, color_blend_state, subpass) + { + let fragment_shader_outputs = super::shader_interface_location_info( + fragment_stage.entry_point.module().spirv(), + fragment_stage.entry_point.id(), + StorageClass::Output, + ); - // Check that the subpass can accept the output of the fragment shader. - match subpass { - PipelineSubpassType::BeginRenderPass(subpass) => { - if !subpass.is_compatible_with(&entry_point_info.output_interface) { - return Err(Box::new(ValidationError { - problem: "`subpass` is not compatible with the \ - output interface of the fragment shader" - .into(), - ..Default::default() - })); - } - } - PipelineSubpassType::BeginRendering(_) => { - // TODO: - } - } + color_blend_state + .validate_required_fragment_outputs(subpass, &fragment_shader_outputs) + .map_err(|mut err| { + err.problem = format!( + "{}: {}", + "the fragment shader in `stages` does not meet the requirements of \ + `color_blend_state` and `subpass`", + err.problem, + ) + .into(); + err + })?; // TODO: // VUID-VkGraphicsPipelineCreateInfo-pStages-01565 diff --git a/vulkano/src/pipeline/graphics/vertex_input/mod.rs b/vulkano/src/pipeline/graphics/vertex_input/mod.rs index b478e90f..36a436f4 100644 --- a/vulkano/src/pipeline/graphics/vertex_input/mod.rs +++ b/vulkano/src/pipeline/graphics/vertex_input/mod.rs @@ -97,17 +97,14 @@ pub use self::{ impl_vertex::VertexMember, vertex::{Vertex, VertexBufferDescription, VertexMemberInfo}, }; +use super::color_blend::ColorComponents; use crate::{ device::Device, - format::{Format, FormatFeatures, NumericType}, - shader::{ - reflect::get_constant, - spirv::{Decoration, Id, Instruction, Spirv, StorageClass}, - }, + format::{Format, FormatFeatures}, + pipeline::{ShaderInterfaceLocationInfo, ShaderInterfaceLocationWidth}, DeviceSize, Requires, RequiresAllOf, RequiresOneOf, ValidationError, }; use ahash::HashMap; -use std::collections::hash_map::Entry; mod buffers; mod collection; @@ -330,6 +327,125 @@ impl VertexInputState { Ok(()) } + + pub(crate) fn validate_required_vertex_inputs( + &self, + vertex_shader_inputs: &HashMap, + vuids: RequiredVertexInputsVUIDs, + ) -> Result<(), Box> { + for (&location, location_info) in vertex_shader_inputs { + let (is_previous, attribute_desc) = + (self.attributes.get(&location).map(|d| (false, d))) + .or_else(|| { + // If the previous location has at least three 64-bit components, + // then it extends into the current location, so try that instead. + location.checked_sub(1).and_then(|location| { + self.attributes + .get(&location) + .filter(|attribute_desc| { + attribute_desc + .format + .components() + .starts_with(&[64, 64, 64]) + }) + .map(|d| (true, d)) + }) + }) + .ok_or_else(|| { + Box::new(ValidationError { + problem: format!( + "the vertex shader has an input variable with location {0}, but \ + the vertex input attributes do not contain {0}", + location, + ) + .into(), + vuids: vuids.not_present, + ..Default::default() + }) + })?; + + let attribute_numeric_type = attribute_desc + .format + .numeric_format_color() + .unwrap() + .numeric_type(); + + if attribute_numeric_type != location_info.numeric_type { + return Err(Box::new(ValidationError { + problem: format!( + "the numeric type of the format of vertex input attribute {0} ({1:?}) \ + does not equal the numeric type of the vertex shader input variable with \ + location {0} ({2:?})", + location, attribute_numeric_type, location_info.numeric_type, + ) + .into(), + vuids: vuids.numeric_type, + ..Default::default() + })); + } + + let attribute_components = attribute_desc.format.components(); + + // 64-bit in the shader must match with 64-bit in the attribute. + match location_info.width { + ShaderInterfaceLocationWidth::Bits32 => { + if attribute_components[0] > 32 { + return Err(Box::new(ValidationError { + problem: format!( + "the vertex shader input variable location {0} requires a non-64-bit \ + format, but the format of vertex input attribute {0} is 64-bit", + location, + ) + .into(), + vuids: vuids.requires32, + ..Default::default() + })); + } + } + ShaderInterfaceLocationWidth::Bits64 => { + if attribute_components[0] <= 32 { + return Err(Box::new(ValidationError { + problem: format!( + "the vertex shader input variable location {0} requires a 64-bit \ + format, but the format of vertex input attribute {0} is not 64-bit", + location, + ) + .into(), + vuids: vuids.requires64, + ..Default::default() + })); + } + + // For 64-bit values, there are no default values for missing components. + // If the shader uses the 64-bit value in the second half of the location, then + // the attribute must provide it. + if location_info.components[0] + .intersects(ColorComponents::B | ColorComponents::A) + { + let second_half_attribute_component = if is_previous { 3 } else { 1 }; + + if attribute_components[second_half_attribute_component] != 64 { + return Err(Box::new(ValidationError { + problem: format!( + "the vertex shader input variable location {0} requires a format \ + with at least {1} 64-bit components, but the format of \ + vertex input attribute {0} contains only {2} components", + location, + second_half_attribute_component + 1, + attribute_components.into_iter().filter(|&c| c != 0).count(), + ) + .into(), + vuids: vuids.requires_second_half, + ..Default::default() + })); + } + } + } + } + } + + Ok(()) + } } impl Default for VertexInputState { @@ -607,279 +723,6 @@ impl From for ash::vk::VertexInputRate { } } -#[derive(Clone, Copy, Debug)] -pub(crate) struct VertexInputLocationRequirements { - pub(crate) numeric_type: NumericType, - pub(crate) width: VertexInputLocationWidth, -} - -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum VertexInputLocationWidth { - /// The shader requires a 32-bit or smaller value at this location. - Requires32, - - /// The shader requires a 64-bit value at this location. - /// The boolean indicates whether the shader requires a format that fills the second half - /// of the location. - Requires64 { requires_second_half: bool }, -} - -pub(crate) fn required_vertex_inputs( - spirv: &Spirv, - entry_point_id: Id, -) -> HashMap { - let interface = match spirv.function(entry_point_id).entry_point() { - Some(Instruction::EntryPoint { interface, .. }) => interface, - _ => unreachable!(), - }; - - let mut required_vertex_inputs = HashMap::default(); - - for &variable_id in interface { - let variable_id_info = spirv.id(variable_id); - let pointer_type_id = match *variable_id_info.instruction() { - Instruction::Variable { - result_type_id, - storage_class: StorageClass::Input, - .. - } => result_type_id, - _ => continue, - }; - let pointer_type_id_info = spirv.id(pointer_type_id); - let type_id = match *pointer_type_id_info.instruction() { - Instruction::TypePointer { ty, .. } => ty, - _ => unreachable!(), - }; - - let mut variable_location = None; - let mut variable_component = 0; - - for instruction in variable_id_info.decorations() { - if let Instruction::Decorate { ref decoration, .. } = *instruction { - match *decoration { - Decoration::Location { location } => variable_location = Some(location), - Decoration::Component { component } => variable_component = component, - _ => (), - } - } - } - - if let Some(variable_location) = variable_location { - add_type_location( - &mut required_vertex_inputs, - spirv, - variable_location, - variable_component, - type_id, - ); - } else { - let block_type_id_info = spirv.id(type_id); - let member_types = match block_type_id_info.instruction() { - Instruction::TypeStruct { member_types, .. } => member_types, - _ => continue, - }; - - for (&type_id, member_info) in member_types.iter().zip(block_type_id_info.members()) { - let mut member_location = None; - let mut member_component = 0; - - for instruction in member_info.decorations() { - if let Instruction::MemberDecorate { ref decoration, .. } = *instruction { - match *decoration { - Decoration::Location { location } => member_location = Some(location), - Decoration::Component { component } => member_component = component, - _ => (), - } - } - } - - if let Some(member_location) = member_location { - add_type_location( - &mut required_vertex_inputs, - spirv, - member_location, - member_component, - type_id, - ); - } - } - } - } - - required_vertex_inputs -} - -fn add_type_location( - required_vertex_inputs: &mut HashMap, - spirv: &Spirv, - mut location: u32, - mut component: u32, - type_id: Id, -) -> (u32, u32) { - debug_assert!(component < 4); - - let mut add_scalar = |numeric_type: NumericType, width: u32| -> (u32, u32) { - if width > 32 { - debug_assert!(component & 1 == 0); - let half_index = component as usize / 2; - - match required_vertex_inputs.entry(location) { - Entry::Occupied(mut entry) => { - let requirements = entry.get_mut(); - debug_assert_eq!(requirements.numeric_type, numeric_type); - - match &mut requirements.width { - VertexInputLocationWidth::Requires32 => unreachable!(), - VertexInputLocationWidth::Requires64 { - requires_second_half, - } => { - if component == 2 { - debug_assert!(!*requires_second_half); - *requires_second_half = true; - } - } - } - } - Entry::Vacant(entry) => { - let mut required_halves = [false; 2]; - required_halves[half_index] = true; - entry.insert(VertexInputLocationRequirements { - numeric_type, - width: VertexInputLocationWidth::Requires64 { - requires_second_half: component == 2, - }, - }); - } - } - - (1, 2) - } else { - match required_vertex_inputs.entry(location) { - Entry::Occupied(entry) => { - let requirements = *entry.get(); - debug_assert_eq!(requirements.numeric_type, numeric_type); - debug_assert_eq!(requirements.width, VertexInputLocationWidth::Requires32); - } - Entry::Vacant(entry) => { - entry.insert(VertexInputLocationRequirements { - numeric_type, - width: VertexInputLocationWidth::Requires32, - }); - } - } - - (1, 1) - } - }; - - match *spirv.id(type_id).instruction() { - Instruction::TypeInt { - width, signedness, .. - } => { - let numeric_type = if signedness == 1 { - NumericType::Int - } else { - NumericType::Uint - }; - - add_scalar(numeric_type, width) - } - Instruction::TypeFloat { width, .. } => add_scalar(NumericType::Float, width), - Instruction::TypeVector { - component_type, - component_count, - .. - } => { - let mut total_locations_added = 1; - - for _ in 0..component_count { - // Overflow into next location - if component == 4 { - component = 0; - location += 1; - total_locations_added += 1; - } else { - debug_assert!(component < 4); - } - - let (_, components_added) = add_type_location( - required_vertex_inputs, - spirv, - location, - component, - component_type, - ); - component += components_added; - } - - (total_locations_added, 0) - } - Instruction::TypeMatrix { - column_type, - column_count, - .. - } => { - let mut total_locations_added = 0; - - for _ in 0..column_count { - let (locations_added, _) = add_type_location( - required_vertex_inputs, - spirv, - location, - component, - column_type, - ); - location += locations_added; - total_locations_added += locations_added; - } - - (total_locations_added, 0) - } - Instruction::TypeArray { - element_type, - length, - .. - } => { - let length = get_constant(spirv, length).unwrap(); - let mut total_locations_added = 0; - - for _ in 0..length { - let (locations_added, _) = add_type_location( - required_vertex_inputs, - spirv, - location, - component, - element_type, - ); - location += locations_added; - total_locations_added += locations_added; - } - - (total_locations_added, 0) - } - Instruction::TypeStruct { - ref member_types, .. - } => { - let mut total_locations_added = 0; - - for &member_type in member_types { - let (locations_added, _) = add_type_location( - required_vertex_inputs, - spirv, - location, - component, - member_type, - ); - location += locations_added; - total_locations_added += locations_added; - } - - (total_locations_added, 0) - } - _ => unimplemented!(), - } -} - pub(crate) struct RequiredVertexInputsVUIDs { pub(crate) not_present: &'static [&'static str], pub(crate) numeric_type: &'static [&'static str], @@ -887,121 +730,3 @@ pub(crate) struct RequiredVertexInputsVUIDs { pub(crate) requires64: &'static [&'static str], pub(crate) requires_second_half: &'static [&'static str], } - -pub(crate) fn validate_required_vertex_inputs( - attribute_descs: &HashMap, - required_vertex_inputs: &HashMap, - vuids: RequiredVertexInputsVUIDs, -) -> Result<(), Box> { - for (&location, location_info) in required_vertex_inputs { - let (is_previous, attribute_desc) = (attribute_descs.get(&location).map(|d| (false, d))) - .or_else(|| { - // If the previous location has at least three 64-bit components, - // then it extends into the current location, so try that instead. - location.checked_sub(1).and_then(|location| { - attribute_descs - .get(&location) - .filter(|attribute_desc| { - attribute_desc - .format - .components() - .starts_with(&[64, 64, 64]) - }) - .map(|d| (true, d)) - }) - }) - .ok_or_else(|| { - Box::new(ValidationError { - problem: format!( - "the vertex shader has an input variable with location {0}, but \ - the vertex input attributes do not contain {0}", - location, - ) - .into(), - vuids: vuids.not_present, - ..Default::default() - }) - })?; - - let attribute_numeric_type = attribute_desc - .format - .numeric_format_color() - .unwrap() - .numeric_type(); - - if attribute_numeric_type != location_info.numeric_type { - return Err(Box::new(ValidationError { - problem: format!( - "the numeric type of the format of vertex input attribute {0} ({1:?}) \ - does not equal the numeric type of the vertex shader input variable with \ - location {0} ({2:?})", - location, attribute_numeric_type, location_info.numeric_type, - ) - .into(), - vuids: vuids.numeric_type, - ..Default::default() - })); - } - - let attribute_components = attribute_desc.format.components(); - - // 64-bit in the shader must match with 64-bit in the attribute. - match location_info.width { - VertexInputLocationWidth::Requires32 => { - if attribute_components[0] > 32 { - return Err(Box::new(ValidationError { - problem: format!( - "the vertex shader input variable location {0} requires a non-64-bit \ - format, but the format of vertex input attribute {0} is 64-bit", - location, - ) - .into(), - vuids: vuids.requires32, - ..Default::default() - })); - } - } - VertexInputLocationWidth::Requires64 { - requires_second_half, - } => { - if attribute_components[0] <= 32 { - return Err(Box::new(ValidationError { - problem: format!( - "the vertex shader input variable location {0} requires a 64-bit \ - format, but the format of vertex input attribute {0} is not 64-bit", - location, - ) - .into(), - vuids: vuids.requires64, - ..Default::default() - })); - } - - // For 64-bit values, there are no default values for missing components. - // If the shader uses the 64-bit value in the second half of the location, then - // the attribute must provide it. - if requires_second_half { - let second_half_attribute_component = if is_previous { 3 } else { 1 }; - - if attribute_components[second_half_attribute_component] != 64 { - return Err(Box::new(ValidationError { - problem: format!( - "the vertex shader input variable location {0} requires a format \ - with at least {1} 64-bit components, but the format of \ - vertex input attribute {0} contains only {2} components", - location, - second_half_attribute_component + 1, - attribute_components.into_iter().filter(|&c| c != 0).count(), - ) - .into(), - vuids: vuids.requires_second_half, - ..Default::default() - })); - } - } - } - } - } - - Ok(()) -} diff --git a/vulkano/src/pipeline/shader.rs b/vulkano/src/pipeline/shader.rs index 6d36fb39..26f5229a 100644 --- a/vulkano/src/pipeline/shader.rs +++ b/vulkano/src/pipeline/shader.rs @@ -1,7 +1,10 @@ use crate::{ device::Device, + format::NumericType, macros::vulkan_bitflags, + pipeline::graphics::color_blend::ColorComponents, shader::{ + reflect::get_constant, spirv::{ BuiltIn, Decoration, ExecutionMode, ExecutionModel, Id, Instruction, Spirv, StorageClass, @@ -11,6 +14,7 @@ use crate::{ Requires, RequiresAllOf, RequiresOneOf, ValidationError, }; use ahash::HashMap; +use std::collections::hash_map::Entry; /// Specifies a single shader stage when creating a pipeline. #[derive(Clone, Debug)] @@ -933,53 +937,6 @@ fn get_variables_by_location<'a>( variables_by_location } -fn strip_array( - spirv: &Spirv, - storage_class: StorageClass, - execution_model: ExecutionModel, - variable_id: Id, - pointed_type_id: Id, -) -> Id { - let variable_decorations = spirv.id(variable_id).decorations(); - let variable_has_decoration = |has_decoration: Decoration| -> bool { - variable_decorations.iter().any(|instruction| { - matches!( - instruction, - Instruction::Decorate { - decoration, - .. - } if *decoration == has_decoration - ) - }) - }; - - let must_strip_array = match storage_class { - StorageClass::Input => match execution_model { - ExecutionModel::TessellationControl | ExecutionModel::TessellationEvaluation => { - !variable_has_decoration(Decoration::Patch) - } - ExecutionModel::Geometry => true, - ExecutionModel::Fragment => variable_has_decoration(Decoration::PerVertexKHR), - _ => false, - }, - StorageClass::Output => match execution_model { - ExecutionModel::TessellationControl => !variable_has_decoration(Decoration::Patch), - ExecutionModel::MeshNV => !variable_has_decoration(Decoration::PerTaskNV), - _ => false, - }, - _ => unreachable!(), - }; - - if must_strip_array { - match spirv.id(pointed_type_id).instruction() { - &Instruction::TypeArray { element_type, .. } => element_type, - _ => pointed_type_id, - } - } else { - pointed_type_id - } -} - fn are_interface_types_compatible( out_spirv: &Spirv, out_type_id: Id, @@ -1364,3 +1321,306 @@ fn decoration_filter_variable(instruction: &Instruction) -> bool { _ => false, } } + +#[derive(Clone, Copy, Debug)] +pub(crate) struct ShaderInterfaceLocationInfo { + pub(crate) numeric_type: NumericType, + pub(crate) width: ShaderInterfaceLocationWidth, + pub(crate) components: [ColorComponents; 2], // Index 0 and 1 +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ShaderInterfaceLocationWidth { + Bits32, + Bits64, +} + +impl From for ShaderInterfaceLocationWidth { + #[inline] + fn from(value: u32) -> Self { + if value > 32 { + Self::Bits64 + } else { + Self::Bits32 + } + } +} + +pub(crate) fn shader_interface_location_info( + spirv: &Spirv, + entry_point_id: Id, + filter_storage_class: StorageClass, +) -> HashMap { + fn add_type( + locations: &mut HashMap, + spirv: &Spirv, + mut location: u32, + mut component: u32, + index: u32, + type_id: Id, + ) -> (u32, u32) { + debug_assert!(component < 4); + + let mut add_scalar = |numeric_type: NumericType, width: u32| -> (u32, u32) { + let width = ShaderInterfaceLocationWidth::from(width); + let components_to_add = match width { + ShaderInterfaceLocationWidth::Bits32 => { + ColorComponents::from_index(component as usize) + } + ShaderInterfaceLocationWidth::Bits64 => { + debug_assert!(component & 1 == 0); + ColorComponents::from_index(component as usize) + | ColorComponents::from_index(component as usize + 1) + } + }; + + let location_info = match locations.entry(location) { + Entry::Occupied(entry) => { + let location_info = entry.into_mut(); + debug_assert_eq!(location_info.numeric_type, numeric_type); + debug_assert_eq!(location_info.width, width); + location_info + } + Entry::Vacant(entry) => entry.insert(ShaderInterfaceLocationInfo { + numeric_type, + width, + components: [ColorComponents::empty(); 2], + }), + }; + + let components = &mut location_info.components[index as usize]; + debug_assert!(!components.intersects(components_to_add)); + *components |= components_to_add; + + (components_to_add.count(), 1) + }; + + match *spirv.id(type_id).instruction() { + Instruction::TypeInt { + width, signedness, .. + } => { + let numeric_type = if signedness == 1 { + NumericType::Int + } else { + NumericType::Uint + }; + + add_scalar(numeric_type, width) + } + Instruction::TypeFloat { width, .. } => add_scalar(NumericType::Float, width), + Instruction::TypeVector { + component_type, + component_count, + .. + } => { + let mut total_locations_added = 1; + + for _ in 0..component_count { + // Overflow into next location + if component == 4 { + component = 0; + location += 1; + total_locations_added += 1; + } else { + debug_assert!(component < 4); + } + + let (_, components_added) = + add_type(locations, spirv, location, component, index, component_type); + component += components_added; + } + + (total_locations_added, 0) + } + Instruction::TypeMatrix { + column_type, + column_count, + .. + } => { + let mut total_locations_added = 0; + + for _ in 0..column_count { + let (locations_added, _) = + add_type(locations, spirv, location, component, index, column_type); + location += locations_added; + total_locations_added += locations_added; + } + + (total_locations_added, 0) + } + Instruction::TypeArray { + element_type, + length, + .. + } => { + let length = get_constant(spirv, length).unwrap(); + let mut total_locations_added = 0; + + for _ in 0..length { + let (locations_added, _) = + add_type(locations, spirv, location, component, index, element_type); + location += locations_added; + total_locations_added += locations_added; + } + + (total_locations_added, 0) + } + Instruction::TypeStruct { + ref member_types, .. + } => { + let mut total_locations_added = 0; + + for &member_type in member_types { + let (locations_added, _) = + add_type(locations, spirv, location, component, index, member_type); + location += locations_added; + total_locations_added += locations_added; + } + + (total_locations_added, 0) + } + _ => unimplemented!(), + } + } + + let (execution_model, interface) = match spirv.function(entry_point_id).entry_point() { + Some(&Instruction::EntryPoint { + execution_model, + ref interface, + .. + }) => (execution_model, interface), + _ => unreachable!(), + }; + + let mut locations = HashMap::default(); + + for &variable_id in interface { + let variable_id_info = spirv.id(variable_id); + let (pointer_type_id, storage_class) = match *variable_id_info.instruction() { + Instruction::Variable { + result_type_id, + storage_class, + .. + } if storage_class == filter_storage_class => (result_type_id, storage_class), + _ => continue, + }; + let pointer_type_id_info = spirv.id(pointer_type_id); + let type_id = match *pointer_type_id_info.instruction() { + Instruction::TypePointer { ty, .. } => { + strip_array(spirv, storage_class, execution_model, variable_id, ty) + } + _ => unreachable!(), + }; + + let mut variable_location = None; + let mut variable_component = 0; + let mut variable_index = 0; + + for instruction in variable_id_info.decorations() { + if let Instruction::Decorate { ref decoration, .. } = *instruction { + match *decoration { + Decoration::Location { location } => variable_location = Some(location), + Decoration::Component { component } => variable_component = component, + Decoration::Index { index } => variable_index = index, + _ => (), + } + } + } + + if let Some(variable_location) = variable_location { + add_type( + &mut locations, + spirv, + variable_location, + variable_component, + variable_index, + type_id, + ); + } else { + let block_type_id_info = spirv.id(type_id); + let member_types = match block_type_id_info.instruction() { + Instruction::TypeStruct { member_types, .. } => member_types, + _ => continue, + }; + + for (&type_id, member_info) in member_types.iter().zip(block_type_id_info.members()) { + let mut member_location = None; + let mut member_component = 0; + let mut member_index = 0; + + for instruction in member_info.decorations() { + if let Instruction::MemberDecorate { ref decoration, .. } = *instruction { + match *decoration { + Decoration::Location { location } => member_location = Some(location), + Decoration::Component { component } => member_component = component, + Decoration::Index { index } => member_index = index, + _ => (), + } + } + } + + if let Some(member_location) = member_location { + add_type( + &mut locations, + spirv, + member_location, + member_component, + member_index, + type_id, + ); + } + } + } + } + + locations +} + +fn strip_array( + spirv: &Spirv, + storage_class: StorageClass, + execution_model: ExecutionModel, + variable_id: Id, + pointed_type_id: Id, +) -> Id { + let variable_decorations = spirv.id(variable_id).decorations(); + let variable_has_decoration = |has_decoration: Decoration| -> bool { + variable_decorations.iter().any(|instruction| { + matches!( + instruction, + Instruction::Decorate { + decoration, + .. + } if *decoration == has_decoration + ) + }) + }; + + let must_strip_array = match storage_class { + StorageClass::Output => match execution_model { + ExecutionModel::TaskEXT | ExecutionModel::MeshEXT | ExecutionModel::MeshNV => true, + ExecutionModel::TessellationControl => !variable_has_decoration(Decoration::Patch), + ExecutionModel::TaskNV => !variable_has_decoration(Decoration::PerTaskNV), + _ => false, + }, + StorageClass::Input => match execution_model { + ExecutionModel::Geometry | ExecutionModel::MeshEXT => true, + ExecutionModel::TessellationControl | ExecutionModel::TessellationEvaluation => { + !variable_has_decoration(Decoration::Patch) + } + ExecutionModel::Fragment => variable_has_decoration(Decoration::PerVertexKHR), + ExecutionModel::MeshNV => !variable_has_decoration(Decoration::PerTaskNV), + _ => false, + }, + _ => unreachable!(), + }; + + if must_strip_array { + match spirv.id(pointed_type_id).instruction() { + &Instruction::TypeArray { element_type, .. } => element_type, + _ => pointed_type_id, + } + } else { + pointed_type_id + } +} diff --git a/vulkano/src/render_pass/mod.rs b/vulkano/src/render_pass/mod.rs index d71a8f9f..c1c4ba41 100644 --- a/vulkano/src/render_pass/mod.rs +++ b/vulkano/src/render_pass/mod.rs @@ -23,7 +23,6 @@ use crate::{ image::{ImageAspects, ImageLayout, SampleCount}, instance::InstanceOwnedDebugWrapper, macros::{impl_id_counter, vulkan_bitflags, vulkan_bitflags_enum, vulkan_enum}, - shader::ShaderInterface, sync::{AccessFlags, DependencyFlags, MemoryBarrier, PipelineStages}, Requires, RequiresAllOf, RequiresOneOf, Validated, ValidationError, Version, VulkanError, VulkanObject, @@ -591,40 +590,6 @@ impl RenderPass { true } - - /// Returns `true` if the subpass of this description is compatible with the shader's fragment - /// output definition. - pub fn is_compatible_with_shader( - &self, - subpass: u32, - shader_interface: &ShaderInterface, - ) -> bool { - let subpass_descr = match self.subpasses.get(subpass as usize) { - Some(s) => s, - None => return false, - }; - - for element in shader_interface.elements() { - assert!(!element.ty.is_64bit); // TODO: implement - let location_range = element.location..element.location + element.ty.num_locations(); - - for location in location_range { - let attachment_id = match subpass_descr.color_attachments.get(location as usize) { - Some(Some(attachment_ref)) => attachment_ref.attachment, - _ => return false, - }; - - let _attachment_desc = &self.attachments[attachment_id as usize]; - - // FIXME: compare formats depending on the number of components and data type - /*if attachment_desc.format != element.format { - return false; - }*/ - } - } - - true - } } impl Drop for RenderPass { @@ -741,14 +706,6 @@ impl Subpass { .next() .map(|attachment_desc| attachment_desc.samples) } - - /// Returns `true` if this subpass is compatible with the fragment output definition. - // TODO: return proper error - #[inline] - pub fn is_compatible_with(&self, shader_interface: &ShaderInterface) -> bool { - self.render_pass - .is_compatible_with_shader(self.subpass_id, shader_interface) - } } impl From for (Arc, u32) {