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

@@ -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);
             }
         ",
     }

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,8 +2163,8 @@ impl<L> AutoCommandBufferBuilder<L> {
                 }
                 DynamicState::VertexInput => {
                     if let Some(vertex_input_state) = &self.builder_state.vertex_input {
-                        vertex_input::validate_required_vertex_inputs(
+                        vertex_input_state
-                            &vertex_input_state.attributes,
+                            .validate_required_vertex_inputs(
                                 pipeline.required_vertex_inputs().unwrap(),
                                 RequiredVertexInputsVUIDs {
                                     not_present: vuids!(vuid_type, "Input-07939"),

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 {

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<u32, ShaderInterfaceLocationInfo>,
+    ) -> Result<(), Box<ValidationError>> {
+        let validate_location =
+            |attachment_index: usize, format: Format| -> Result<(), Box<ValidationError>> {
+                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!(),
+        }
+    }
+}

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<DynamicState>,
     fragment_tests_stages: Option<FragmentTestsStages>,
     // Note: this is only `Some` if `vertex_input_state` is `None`.
-    required_vertex_inputs: Option<HashMap<u32, VertexInputLocationRequirements>>,
+    required_vertex_inputs: Option<HashMap<u32, ShaderInterfaceLocationInfo>>,
 }
 
 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<u32, VertexInputLocationRequirements>> {
+    ) -> Option<&HashMap<u32, ShaderInterfaceLocationInfo>> {
         self.required_vertex_inputs.as_ref()
     }
 }
@@ -2421,13 +2423,14 @@ 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
-                &vertex_input_state.attributes,
+                .validate_required_vertex_inputs(
                     &required_vertex_inputs,
                     RequiredVertexInputsVUIDs {
                         not_present: &["VUID-VkGraphicsPipelineCreateInfo-Input-07904"],
@@ -2508,25 +2511,27 @@ impl GraphicsPipelineCreateInfo {
             }
         }
 
-        if let (Some(fragment_stage), Some(subpass)) = (fragment_stage, subpass) {
+        if let (Some(fragment_stage), Some(color_blend_state), Some(subpass)) =
-            let entry_point_info = fragment_stage.entry_point.info();
+            (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.
+            color_blend_state
-            match subpass {
+                .validate_required_fragment_outputs(subpass, &fragment_shader_outputs)
-                PipelineSubpassType::BeginRenderPass(subpass) => {
+                .map_err(|mut err| {
-                    if !subpass.is_compatible_with(&entry_point_info.output_interface) {
+                    err.problem = format!(
-                        return Err(Box::new(ValidationError {
+                        "{}: {}",
-                            problem: "`subpass` is not compatible with the \
+                        "the fragment shader in `stages` does not meet the requirements of \
-                                output interface of the fragment shader"
+                        `color_blend_state` and `subpass`",
-                                .into(),
+                        err.problem,
-                            ..Default::default()
+                    )
-                        }));
+                    .into();
-                    }
+                    err
-                }
+                })?;
-                PipelineSubpassType::BeginRendering(_) => {
-                    // TODO:
-                }
-            }
 
             // TODO:
             // VUID-VkGraphicsPipelineCreateInfo-pStages-01565

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},
+    format::{Format, FormatFeatures},
-    shader::{
+    pipeline::{ShaderInterfaceLocationInfo, ShaderInterfaceLocationWidth},
-        reflect::get_constant,
-        spirv::{Decoration, Id, Instruction, Spirv, StorageClass},
-    },
     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<u32, ShaderInterfaceLocationInfo>,
+        vuids: RequiredVertexInputsVUIDs,
+    ) -> Result<(), Box<ValidationError>> {
+        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<VertexInputRate> 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<u32, VertexInputLocationRequirements> {
-    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<u32, VertexInputLocationRequirements>,
-    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<u32, VertexInputAttributeDescription>,
-    required_vertex_inputs: &HashMap<u32, VertexInputLocationRequirements>,
-    vuids: RequiredVertexInputsVUIDs,
-) -> Result<(), Box<ValidationError>> {
-    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(())
-}

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<u32> 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<u32, ShaderInterfaceLocationInfo> {
+    fn add_type(
+        locations: &mut HashMap<u32, ShaderInterfaceLocationInfo>,
+        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
+    }
+}

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<Subpass> for (Arc<RenderPass>, u32) {