From 396e84d33afd3bbfdb70730352fec17b77eda2a0 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Thu, 1 Sep 2022 13:44:36 -0700 Subject: [PATCH] [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. --- src/back/spv/writer.rs | 49 +++++++++---------- tests/out/spv/access.spvasm | 1 - tests/out/spv/boids.spvasm | 1 - tests/out/spv/collatz.spvasm | 1 - tests/out/spv/control-flow.spvasm | 1 - tests/out/spv/image.spvasm | 2 - tests/out/spv/interface.compute.spvasm | 5 -- tests/out/spv/interface.fragment.spvasm | 1 - tests/out/spv/interface.vertex.spvasm | 2 - .../spv/interface.vertex_two_structs.spvasm | 2 - tests/out/spv/multiview.spvasm | 1 - tests/out/spv/skybox.spvasm | 1 - 12 files changed, 23 insertions(+), 44 deletions(-) diff --git a/src/back/spv/writer.rs b/src/back/spv/writer.rs index 0bb937c2e7..59fe739f2c 100644 --- 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, binding: &crate::Binding, - interpolation_decoration: bool, ) -> Result { 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 index a710a39b9b..fe7e75d4a4 100644 --- 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 index 69a7433b33..887cdb6adb 100644 --- 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 index c3630f9164..6e6483da10 100644 --- 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 index 467393b01d..dfd1b145a2 100644 --- 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 index 35f9734420..7ad2b35603 100644 --- 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 index 748b445b6e..6c1dac372d 100644 --- 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 index 5033458258..424208def3 100644 --- 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 index 83a6aae827..62e4acb1b6 100644 --- 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 index dc681b5b47..190f391c39 100644 --- 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 index 343396ea02..bb67d7eba8 100644 --- 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 index 227c7786e5..ca51a06dd8 100644 --- 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