From fb53f83503659b4977742fac832dc2defd3bacb4 Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 12 Nov 2020 15:00:16 -0500 Subject: [PATCH] Avoid spuriously adding Geometry capability for vert, tesc, tese (#2462) Use of gl_Layer and gl_ViewportIndex in tessellation and vertex shaders should not trigger the addition of the Geometry capability. Fixes #2461 Added tests for use of gl_Layer and gl_ViewportIndex in a tessellation evaluation shader. Several tests for NVIDIA features for tessellation, vertex, or mesh shaders now lose the Geometry or MultiViewport capabilities. This is ok because the functionality is already covered by the ShaderViewportIndexLayerNV capability. The spv.meshShaderPerViewBuiltins.mesh test now fails validation because the validator does not know that PrimitiveId (and possibly other) builtins are enabled by the MeshShadingNV capability. I filed https://github.com/KhronosGroup/SPIRV-Headers/issues/179 to fix the grammar upstream. --- SPIRV/GlslangToSpv.cpp | 10 +++++-- Test/baseResults/spv.layer.tese.out | 30 +++++++++++++++++++ .../spv.meshShaderBuiltins.mesh.out | 1 - .../spv.meshShaderPerViewBuiltins.mesh.out | 2 +- .../spv.meshShaderRedeclBuiltins.mesh.out | 1 - .../spv.stereoViewRendering.tesc.out | 1 - .../spv.stereoViewRendering.vert.out | 1 - Test/baseResults/spv.viewportArray2.tesc.out | 2 -- Test/baseResults/spv.viewportArray2.vert.out | 2 -- Test/baseResults/spv.viewportindex.tese.out | 30 +++++++++++++++++++ Test/spv.layer.tese | 8 +++++ Test/spv.viewportindex.tese | 8 +++++ gtests/Spv.FromFile.cpp | 2 ++ 13 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 Test/baseResults/spv.layer.tese.out create mode 100644 Test/baseResults/spv.viewportindex.tese.out create mode 100644 Test/spv.layer.tese create mode 100644 Test/spv.viewportindex.tese diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index a24b52239..e0b5a19fe 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -725,7 +725,10 @@ spv::BuiltIn TGlslangToSpvTraverser::TranslateBuiltInDecoration(glslang::TBuiltI return spv::BuiltInCullDistance; case glslang::EbvViewportIndex: - builder.addCapability(spv::CapabilityMultiViewport); + if (glslangIntermediate->getStage() == EShLangGeometry || + glslangIntermediate->getStage() == EShLangFragment) { + builder.addCapability(spv::CapabilityMultiViewport); + } if (glslangIntermediate->getStage() == EShLangVertex || glslangIntermediate->getStage() == EShLangTessControl || glslangIntermediate->getStage() == EShLangTessEvaluation) { @@ -754,7 +757,10 @@ spv::BuiltIn TGlslangToSpvTraverser::TranslateBuiltInDecoration(glslang::TBuiltI if (glslangIntermediate->getStage() == EShLangMeshNV) { return spv::BuiltInLayer; } - builder.addCapability(spv::CapabilityGeometry); + if (glslangIntermediate->getStage() == EShLangGeometry || + glslangIntermediate->getStage() == EShLangFragment) { + builder.addCapability(spv::CapabilityGeometry); + } if (glslangIntermediate->getStage() == EShLangVertex || glslangIntermediate->getStage() == EShLangTessControl || glslangIntermediate->getStage() == EShLangTessEvaluation) { diff --git a/Test/baseResults/spv.layer.tese.out b/Test/baseResults/spv.layer.tese.out new file mode 100644 index 000000000..906340fe8 --- /dev/null +++ b/Test/baseResults/spv.layer.tese.out @@ -0,0 +1,30 @@ +spv.layer.tese +// Module Version 10000 +// Generated by (magic number): 8000a +// Id's are bound by 10 + + Capability Tessellation + Capability ShaderViewportIndexLayerNV + Extension "SPV_EXT_shader_viewport_index_layer" + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint TessellationEvaluation 4 "main" 8 + ExecutionMode 4 Triangles + ExecutionMode 4 SpacingEqual + ExecutionMode 4 VertexOrderCcw + Source GLSL 450 + SourceExtension "GL_ARB_shader_viewport_layer_array" + Name 4 "main" + Name 8 "gl_Layer" + Decorate 8(gl_Layer) BuiltIn Layer + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeInt 32 1 + 7: TypePointer Output 6(int) + 8(gl_Layer): 7(ptr) Variable Output + 9: 6(int) Constant 1 + 4(main): 2 Function None 3 + 5: Label + Store 8(gl_Layer) 9 + Return + FunctionEnd diff --git a/Test/baseResults/spv.meshShaderBuiltins.mesh.out b/Test/baseResults/spv.meshShaderBuiltins.mesh.out index f0c252a96..b26122ef7 100644 --- a/Test/baseResults/spv.meshShaderBuiltins.mesh.out +++ b/Test/baseResults/spv.meshShaderBuiltins.mesh.out @@ -5,7 +5,6 @@ spv.meshShaderBuiltins.mesh Capability ClipDistance Capability CullDistance - Capability MultiViewport Capability DrawParameters Capability ShaderViewportMaskNV Capability MeshShadingNV diff --git a/Test/baseResults/spv.meshShaderPerViewBuiltins.mesh.out b/Test/baseResults/spv.meshShaderPerViewBuiltins.mesh.out index 907ae287a..38a426cd9 100644 --- a/Test/baseResults/spv.meshShaderPerViewBuiltins.mesh.out +++ b/Test/baseResults/spv.meshShaderPerViewBuiltins.mesh.out @@ -1,9 +1,9 @@ spv.meshShaderPerViewBuiltins.mesh +Validation failed // Module Version 10000 // Generated by (magic number): 8000a // Id's are bound by 126 - Capability MultiViewport Capability PerViewAttributesNV Capability MeshShadingNV Extension "SPV_NVX_multiview_per_view_attributes" diff --git a/Test/baseResults/spv.meshShaderRedeclBuiltins.mesh.out b/Test/baseResults/spv.meshShaderRedeclBuiltins.mesh.out index 66c3a0d05..bfd2d85b9 100644 --- a/Test/baseResults/spv.meshShaderRedeclBuiltins.mesh.out +++ b/Test/baseResults/spv.meshShaderRedeclBuiltins.mesh.out @@ -5,7 +5,6 @@ spv.meshShaderRedeclBuiltins.mesh Capability ClipDistance Capability CullDistance - Capability MultiViewport Capability ShaderViewportMaskNV Capability MeshShadingNV Extension "SPV_NV_mesh_shader" diff --git a/Test/baseResults/spv.stereoViewRendering.tesc.out b/Test/baseResults/spv.stereoViewRendering.tesc.out index a357346dc..f01e53bf8 100644 --- a/Test/baseResults/spv.stereoViewRendering.tesc.out +++ b/Test/baseResults/spv.stereoViewRendering.tesc.out @@ -3,7 +3,6 @@ spv.stereoViewRendering.tesc // Generated by (magic number): 8000a // Id's are bound by 42 - Capability Geometry Capability Tessellation Capability ShaderViewportIndexLayerNV Capability ShaderViewportMaskNV diff --git a/Test/baseResults/spv.stereoViewRendering.vert.out b/Test/baseResults/spv.stereoViewRendering.vert.out index 0667cb8ca..e74921af9 100644 --- a/Test/baseResults/spv.stereoViewRendering.vert.out +++ b/Test/baseResults/spv.stereoViewRendering.vert.out @@ -4,7 +4,6 @@ spv.stereoViewRendering.vert // Id's are bound by 27 Capability Shader - Capability Geometry Capability ShaderViewportIndexLayerNV Capability ShaderViewportMaskNV Capability ShaderStereoViewNV diff --git a/Test/baseResults/spv.viewportArray2.tesc.out b/Test/baseResults/spv.viewportArray2.tesc.out index 6a9dccccf..74235a571 100644 --- a/Test/baseResults/spv.viewportArray2.tesc.out +++ b/Test/baseResults/spv.viewportArray2.tesc.out @@ -4,9 +4,7 @@ Validation failed // Generated by (magic number): 8000a // Id's are bound by 25 - Capability Geometry Capability Tessellation - Capability MultiViewport Capability ShaderViewportIndexLayerNV Capability ShaderViewportMaskNV Extension "SPV_EXT_shader_viewport_index_layer" diff --git a/Test/baseResults/spv.viewportArray2.vert.out b/Test/baseResults/spv.viewportArray2.vert.out index 96e5b0782..cf29cd79d 100644 --- a/Test/baseResults/spv.viewportArray2.vert.out +++ b/Test/baseResults/spv.viewportArray2.vert.out @@ -4,8 +4,6 @@ spv.viewportArray2.vert // Id's are bound by 19 Capability Shader - Capability Geometry - Capability MultiViewport Capability ShaderViewportIndexLayerNV Capability ShaderViewportMaskNV Extension "SPV_EXT_shader_viewport_index_layer" diff --git a/Test/baseResults/spv.viewportindex.tese.out b/Test/baseResults/spv.viewportindex.tese.out new file mode 100644 index 000000000..12a30cf4c --- /dev/null +++ b/Test/baseResults/spv.viewportindex.tese.out @@ -0,0 +1,30 @@ +spv.viewportindex.tese +// Module Version 10000 +// Generated by (magic number): 8000a +// Id's are bound by 10 + + Capability Tessellation + Capability ShaderViewportIndexLayerNV + Extension "SPV_EXT_shader_viewport_index_layer" + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint TessellationEvaluation 4 "main" 8 + ExecutionMode 4 Triangles + ExecutionMode 4 SpacingEqual + ExecutionMode 4 VertexOrderCcw + Source GLSL 450 + SourceExtension "GL_ARB_shader_viewport_layer_array" + Name 4 "main" + Name 8 "gl_ViewportIndex" + Decorate 8(gl_ViewportIndex) BuiltIn ViewportIndex + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeInt 32 1 + 7: TypePointer Output 6(int) +8(gl_ViewportIndex): 7(ptr) Variable Output + 9: 6(int) Constant 1 + 4(main): 2 Function None 3 + 5: Label + Store 8(gl_ViewportIndex) 9 + Return + FunctionEnd diff --git a/Test/spv.layer.tese b/Test/spv.layer.tese new file mode 100644 index 000000000..b0ce079da --- /dev/null +++ b/Test/spv.layer.tese @@ -0,0 +1,8 @@ +#version 450 + +#extension GL_ARB_shader_viewport_layer_array : require + +layout(triangles) in; +void main() { + gl_Layer = 1; +} diff --git a/Test/spv.viewportindex.tese b/Test/spv.viewportindex.tese new file mode 100644 index 000000000..0da4cc862 --- /dev/null +++ b/Test/spv.viewportindex.tese @@ -0,0 +1,8 @@ +#version 450 + +#extension GL_ARB_shader_viewport_layer_array : require + +layout(triangles) in; +void main() { + gl_ViewportIndex = 1; +} diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp index 9cb3dbab6..0f6e9f81b 100644 --- a/gtests/Spv.FromFile.cpp +++ b/gtests/Spv.FromFile.cpp @@ -352,6 +352,7 @@ INSTANTIATE_TEST_SUITE_P( "spv.int64.frag", "spv.intcoopmat.comp", "spv.intOps.vert", + "spv.layer.tese", "spv.layoutNested.vert", "spv.length.frag", "spv.localAggregates.frag", @@ -439,6 +440,7 @@ INSTANTIATE_TEST_SUITE_P( "spv.terminate.frag", "spv.precise.tese", "spv.precise.tesc", + "spv.viewportindex.tese", "spv.volatileAtomic.comp", "spv.vulkan100.subgroupArithmetic.comp", "spv.vulkan100.subgroupPartitioned.comp",