[PATCH v2 0/1] MR546: vkd3d-shader/spirv: Enable capability ShaderViewportIndexLayerEXT when needed.

[Giovanni Mascellani (＠giomasce)]

Capability Geometry allows to use the Layer builtin in geometry shaders.
For vertex shaders ShaderLayer should be used, but it's only available
starting from SPIR-V 1.5. ShaderViewportIndexLayerEXT can be used instead
with extension SPV_EXT_shader_viewport_index_layer.

--
  v2: vkd3d-shader/spirv: Enable capability ShaderViewportIndexLayerEXT when needed.
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/546

[PATCH v2 1/1] vkd3d-shader/spirv: Enable capability ShaderViewportIndexLayerEXT when needed.

[Giovanni Mascellani]

From: Giovanni Mascellani <gmascellani(a)codeweavers.com>

Capability Geometry allows to use the Layer builtin in geometry and pixel
shaders. For vertex shaders ShaderLayer should be used, but it's only
available starting from SPIR-V 1.5. ShaderViewportIndexLayerEXT can be used
instead with extension SPV_EXT_shader_viewport_index_layer.
---
 libs/vkd3d-shader/spirv.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c
index 8a4704e56..3bb3afd43 100644
--- a/libs/vkd3d-shader/spirv.c
+++ b/libs/vkd3d-shader/spirv.c
@@ -1949,6 +1949,8 @@ static bool vkd3d_spirv_compile_module(struct vkd3d_spirv_builder *builder,
         vkd3d_spirv_build_op_extension(&stream, "SPV_EXT_descriptor_indexing");
     if (vkd3d_spirv_capability_is_enabled(builder, SpvCapabilityStencilExportEXT))
         vkd3d_spirv_build_op_extension(&stream, "SPV_EXT_shader_stencil_export");
+    if (vkd3d_spirv_capability_is_enabled(builder, SpvCapabilityShaderViewportIndexLayerEXT))
+        vkd3d_spirv_build_op_extension(&stream, "SPV_EXT_shader_viewport_index_layer");
 
     if (builder->ext_instr_set_glsl_450)
         vkd3d_spirv_build_op_ext_inst_import(&stream, builder->ext_instr_set_glsl_450, "GLSL.std.450");
@@ -4284,7 +4286,21 @@ static void spirv_compiler_decorate_builtin(struct spirv_compiler *compiler,
             spirv_compiler_emit_execution_mode(compiler, SpvExecutionModeDepthReplacing, NULL, 0);
             break;
         case SpvBuiltInLayer:
-            vkd3d_spirv_enable_capability(builder, SpvCapabilityGeometry);
+            switch (compiler->shader_type)
+            {
+                case VKD3D_SHADER_TYPE_PIXEL:
+                case VKD3D_SHADER_TYPE_GEOMETRY:
+                    vkd3d_spirv_enable_capability(builder, SpvCapabilityGeometry);
+                    break;
+
+                case VKD3D_SHADER_TYPE_VERTEX:
+                    vkd3d_spirv_enable_capability(builder, SpvCapabilityShaderViewportIndexLayerEXT);
+                    break;
+
+                default:
+                    FIXME("Unhandled use of a Layer builtin in a shader of type %u.\n", compiler->shader_type);
+                    break;
+            }
             break;
         case SpvBuiltInViewportIndex:
             vkd3d_spirv_enable_capability(builder, SpvCapabilityMultiViewport);
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/546

[Giovanni Mascellani (＠giomasce)]

I find the SPIR-V specs a bit harder to read than Vulkan, especially when dealing with extensions, so I'm not completely sure I'm getting this right, but hopefully now it's better. It might be that `ShaderViewportIndexLayerEXT` is appropriate for domain shaders too, but I haven't encountered that in a shader yet.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/546#note_56986

[PATCH v2 0/1] MR546: vkd3d-shader/spirv: Enable capability ShaderViewportIndexLayerEXT when needed. - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.
-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/546

[Zebediah Figura (＠zfigura)]

Don't we need to feed this through vkd3d_shader_spirv_extension?

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/546#note_57013

[Henri Verbeet (＠hverbeet)]

> Don't we need to feed this through vkd3d_shader_spirv_extension?

I think we'll need an extension check and corresponding vkd3d and wined3d (see "viewport_array_index_any_shader") code, yes.
> ```diff
> @@ -4284,7 +4286,21 @@ static void spirv_compiler_decorate_builtin(struct spirv_compiler *compiler,
>              spirv_compiler_emit_execution_mode(compiler, SpvExecutionModeDepthReplacing, NULL, 0);
>              break;
>          case SpvBuiltInLayer:
> -            vkd3d_spirv_enable_capability(builder, SpvCapabilityGeometry);
> +            switch (compiler->shader_type)
> +            {
> +                case VKD3D_SHADER_TYPE_PIXEL:
> +                case VKD3D_SHADER_TYPE_GEOMETRY:
> +                    vkd3d_spirv_enable_capability(builder, SpvCapabilityGeometry);
> +                    break;
> +
> +                case VKD3D_SHADER_TYPE_VERTEX:
> +                    vkd3d_spirv_enable_capability(builder, SpvCapabilityShaderViewportIndexLayerEXT);
> +                    break;
> +
> +                default:
> +                    FIXME("Unhandled use of a Layer builtin in a shader of type %u.\n", compiler->shader_type);
> +                    break;
> ```

We should use spirv_compiler_error() there instead of FIXME.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/546#note_57022