[spv-out] Properly combine the fixes for #2035 and #2038.

The Vulkan decoration rules require us to distinguish vertex shader inputs, fragment shader inputs, and everything else, so just pass the stage to `Writer::write_varying`. Together with the SPIRV storage class, this is sufficient to distinguish all the cases in a way that closely follows the spec language.
gfx-rs · Sep 1, 2022 · b209d91 · b209d91
1 parent 5be898e
commit b209d91
Show file tree

Hide file tree

Showing 12 changed files with 23 additions and 44 deletions.
diff --git a/src/back/spv/writer.rs b/src/back/spv/writer.rs
@@ -340,21 +340,16 @@ impl Writer {
             };
 
             if let Some(ref mut iface) = interface {
-                use crate::ShaderStage;
-                let interpolation_decoration = match iface.stage {
-                    ShaderStage::Vertex | ShaderStage::Compute => false,
-                    ShaderStage::Fragment => true,
-                };
                 let id = if let Some(ref binding) = argument.binding {
                     let name = argument.name.as_deref();
 
                     let varying_id = self.write_varying(
                         ir_module,
+                        iface.stage,
                         class,
                         name,
                         argument.ty,
                         binding,
-                        interpolation_decoration,
                     )?;
                     iface.varying_ids.push(varying_id);
                     let id = self.id_gen.next();
@@ -373,11 +368,11 @@ impl Writer {
                         let binding = member.binding.as_ref().unwrap();
                         let varying_id = self.write_varying(
                             ir_module,
+                            iface.stage,
                             class,
                             name,
                             member.ty,
                             binding,
-                            interpolation_decoration,
                         )?;
                         iface.varying_ids.push(varying_id);
                         let id = self.id_gen.next();
@@ -426,11 +421,6 @@ impl Writer {
         let return_type_id = match ir_function.result {
             Some(ref result) => {
                 if let Some(ref mut iface) = interface {
-                    use crate::ShaderStage;
-                    let interpolation_decoration = match iface.stage {
-                        ShaderStage::Fragment | ShaderStage::Compute => false,
-                        ShaderStage::Vertex => true,
-                    };
                     let mut has_point_size = false;
                     let class = spirv::StorageClass::Output;
                     if let Some(ref binding) = result.binding {
@@ -439,11 +429,11 @@ impl Writer {
                         let type_id = self.get_type_id(LookupType::Handle(result.ty));
                         let varying_id = self.write_varying(
                             ir_module,
+                            iface.stage,
                             class,
                             None,
                             result.ty,
                             binding,
-                            interpolation_decoration,
                         )?;
                         iface.varying_ids.push(varying_id);
                         ep_context.results.push(ResultMember {
@@ -462,11 +452,11 @@ impl Writer {
                                 *binding == crate::Binding::BuiltIn(crate::BuiltIn::PointSize);
                             let varying_id = self.write_varying(
                                 ir_module,
+                                iface.stage,
                                 class,
                                 name,
                                 member.ty,
                                 binding,
-                                interpolation_decoration,
                             )?;
                             iface.varying_ids.push(varying_id);
                             ep_context.results.push(ResultMember {
@@ -1150,11 +1140,11 @@ impl Writer {
     fn write_varying(
         &mut self,
         ir_module: &crate::Module,
+        stage: crate::ShaderStage,
         class: spirv::StorageClass,
         debug_name: Option<&str>,
         ty: Handle<crate::Type>,
         binding: &crate::Binding,
-        interpolation_decoration: bool,
     ) -> Result<Word, Error> {
         let id = self.id_gen.next();
         let pointer_type_id = self.get_pointer_id(&ir_module.types, ty, class)?;
@@ -1180,7 +1170,12 @@ impl Writer {
             } => {
                 self.decorate(id, Decoration::Location, &[location]);
 
-                if interpolation_decoration {
+                // The Vulkan spec says: VUID-StandaloneSpirv-Flat-06202
+                //
+                // > The Flat, NoPerspective, Sample, and Centroid decorations
+                // > must not be used on variables with the Input storage class in
+                // > a vertex shader
+                if class != spirv::StorageClass::Input || stage != crate::ShaderStage::Vertex {
                     match interpolation {
                         // Perspective-correct interpolation is the default in SPIR-V.
                         None | Some(crate::Interpolation::Perspective) => (),
@@ -1271,17 +1266,19 @@ impl Writer {
                 // > Any variable with integer or double-precision floating-
                 // > point type and with Input storage class in a fragment
                 // > shader, must be decorated Flat
-                let is_flat = match ir_module.types[ty].inner {
-                    crate::TypeInner::Scalar { kind, .. }
-                    | crate::TypeInner::Vector { kind, .. } => match kind {
-                        Sk::Uint | Sk::Sint | Sk::Bool => true,
-                        Sk::Float => false,
-                    },
-                    _ => false,
-                };
+                if class == spirv::StorageClass::Input && stage == crate::ShaderStage::Fragment {
+                    let is_flat = match ir_module.types[ty].inner {
+                        crate::TypeInner::Scalar { kind, .. }
+                        | crate::TypeInner::Vector { kind, .. } => match kind {
+                            Sk::Uint | Sk::Sint | Sk::Bool => true,
+                            Sk::Float => false,
+                        },
+                        _ => false,
+                    };
 
-                if is_flat {
-                    self.decorate(id, Decoration::Flat, &[]);
+                    if is_flat {
+                        self.decorate(id, Decoration::Flat, &[]);
+                    }
                 }
             }
         }

diff --git a/tests/out/spv/access.spvasm b/tests/out/spv/access.spvasm
@@ -98,7 +98,6 @@ OpDecorate %78 Binding 3
 OpDecorate %79 Block
 OpMemberDecorate %79 0 Offset 0
 OpDecorate %228 BuiltIn VertexIndex
-OpDecorate %228 Flat
 OpDecorate %231 BuiltIn Position
 OpDecorate %273 Location 0
 %2 = OpTypeVoid

diff --git a/tests/out/spv/boids.spvasm b/tests/out/spv/boids.spvasm
@@ -61,7 +61,6 @@ OpDecorate %26 DescriptorSet 0
 OpDecorate %26 Binding 2
 OpDecorate %19 Block
 OpDecorate %48 BuiltIn GlobalInvocationId
-OpDecorate %48 Flat
 %2 = OpTypeVoid
 %4 = OpTypeInt 32 0
 %3 = OpConstant  %4  1500

diff --git a/tests/out/spv/collatz.spvasm b/tests/out/spv/collatz.spvasm
@@ -24,7 +24,6 @@ OpDecorate %11 DescriptorSet 0
 OpDecorate %11 Binding 0
 OpDecorate %9 Block
 OpDecorate %46 BuiltIn GlobalInvocationId
-OpDecorate %46 Flat
 %2 = OpTypeVoid
 %4 = OpTypeInt 32 0
 %3 = OpConstant  %4  0

diff --git a/tests/out/spv/control-flow.spvasm b/tests/out/spv/control-flow.spvasm
@@ -8,7 +8,6 @@ OpMemoryModel Logical GLSL450
 OpEntryPoint GLCompute %44 "main" %41
 OpExecutionMode %44 LocalSize 1 1 1
 OpDecorate %41 BuiltIn GlobalInvocationId
-OpDecorate %41 Flat
 %2 = OpTypeVoid
 %4 = OpTypeInt 32 1
 %3 = OpConstant  %4  1

diff --git a/tests/out/spv/image.spvasm b/tests/out/spv/image.spvasm
@@ -95,9 +95,7 @@ OpDecorate %68 Binding 2
 OpDecorate %70 DescriptorSet 1
 OpDecorate %70 Binding 3
 OpDecorate %73 BuiltIn LocalInvocationId
-OpDecorate %73 Flat
 OpDecorate %119 BuiltIn LocalInvocationId
-OpDecorate %119 Flat
 OpDecorate %140 BuiltIn Position
 OpDecorate %193 BuiltIn Position
 OpDecorate %222 Location 0

diff --git a/tests/out/spv/interface.compute.spvasm b/tests/out/spv/interface.compute.spvasm
@@ -16,15 +16,10 @@ OpDecorate %16 ArrayStride 4
 OpMemberDecorate %18 0 Offset 0
 OpMemberDecorate %19 0 Offset 0
 OpDecorate %23 BuiltIn GlobalInvocationId
-OpDecorate %23 Flat
 OpDecorate %26 BuiltIn LocalInvocationId
-OpDecorate %26 Flat
 OpDecorate %28 BuiltIn LocalInvocationIndex
-OpDecorate %28 Flat
 OpDecorate %31 BuiltIn WorkgroupId
-OpDecorate %31 Flat
 OpDecorate %33 BuiltIn NumWorkgroups
-OpDecorate %33 Flat
 %2 = OpTypeVoid
 %4 = OpTypeFloat 32
 %3 = OpConstant  %4  1.0

diff --git a/tests/out/spv/interface.fragment.spvasm b/tests/out/spv/interface.fragment.spvasm
@@ -28,7 +28,6 @@ OpDecorate %34 BuiltIn SampleMask
 OpDecorate %34 Flat
 OpDecorate %36 BuiltIn FragDepth
 OpDecorate %38 BuiltIn SampleMask
-OpDecorate %38 Flat
 OpDecorate %40 Location 0
 %2 = OpTypeVoid
 %4 = OpTypeFloat 32

diff --git a/tests/out/spv/interface.vertex.spvasm b/tests/out/spv/interface.vertex.spvasm
@@ -15,9 +15,7 @@ OpDecorate %16 ArrayStride 4
 OpMemberDecorate %18 0 Offset 0
 OpMemberDecorate %19 0 Offset 0
 OpDecorate %21 BuiltIn VertexIndex
-OpDecorate %21 Flat
 OpDecorate %24 BuiltIn InstanceIndex
-OpDecorate %24 Flat
 OpDecorate %26 Location 10
 OpDecorate %28 Invariant
 OpDecorate %28 BuiltIn Position

diff --git a/tests/out/spv/interface.vertex_two_structs.spvasm b/tests/out/spv/interface.vertex_two_structs.spvasm
@@ -15,9 +15,7 @@ OpDecorate %16 ArrayStride 4
 OpMemberDecorate %18 0 Offset 0
 OpMemberDecorate %19 0 Offset 0
 OpDecorate %24 BuiltIn VertexIndex
-OpDecorate %24 Flat
 OpDecorate %28 BuiltIn InstanceIndex
-OpDecorate %28 Flat
 OpDecorate %30 Invariant
 OpDecorate %30 BuiltIn Position
 OpDecorate %32 BuiltIn PointSize

diff --git a/tests/out/spv/multiview.spvasm b/tests/out/spv/multiview.spvasm
@@ -9,7 +9,6 @@ OpExtension "SPV_KHR_multiview"
 OpMemoryModel Logical GLSL450
 OpEntryPoint Vertex %8 "main" %5
 OpDecorate %5 BuiltIn ViewIndex
-OpDecorate %5 Flat
 %2 = OpTypeVoid
 %3 = OpTypeInt 32 1
 %6 = OpTypePointer Input %3

diff --git a/tests/out/spv/skybox.spvasm b/tests/out/spv/skybox.spvasm
@@ -25,7 +25,6 @@ OpDecorate %22 Binding 1
 OpDecorate %24 DescriptorSet 0
 OpDecorate %24 Binding 2
 OpDecorate %32 BuiltIn VertexIndex
-OpDecorate %32 Flat
 OpDecorate %35 BuiltIn Position
 OpDecorate %37 Location 0
 OpDecorate %83 BuiltIn FragCoord