Skip to content
Snippets Groups Projects

vkd3d-shader/spirv: Enable capability ShaderViewportIndexLayerEXT when needed.

Merged Giovanni Mascellani requested to merge giomasce/vkd3d:yellowstone into master
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added 1 commit

    • 5365a102 - vkd3d-shader/spirv: Enable capability ShaderViewportIndexLayerEXT when needed.

    Compare with previous version

  • 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.

  • Giovanni Mascellani approved this merge request

    approved this merge request

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

  • 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.

    @@ -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.

  • added 2 commits

    • 1c51208f - vkd3d-shader/spirv: Use capability ShaderViewportIndexLayerEXT for decoration Layer.
    • 658781c7 - vkd3d-shader/spirv: Use capability ShaderViewportIndexLayerEXT for decoration ViewportIndex.

    Compare with previous version

  • Ok, I wrote the extension glue code. I also added a patch for decoration ViewportIndex, given that it's very similar. I was asked to write tests, though, so I'll do that at some point.

  • Giovanni Mascellani unapproved this merge request

    unapproved this merge request

  • Giovanni Mascellani marked this merge request as draft

    marked this merge request as draft

  • Giovanni Mascellani changed the description

    changed the description

  • Giovanni Mascellani marked this merge request as ready

    marked this merge request as ready

  • added 9 commits

    • 8f1fe96b - vkd3d-shader/spirv: Use capability ShaderViewportIndexLayerEXT for decoration Layer.
    • 85257942 - vkd3d-shader/spirv: Use capability ShaderViewportIndexLayerEXT for decoration ViewportIndex.
    • a8281d88 - vkd3d-shader/tpf: Support SV_RenderTargetArrayIndex in pixel and vertex shaders.
    • cfd30673 - vkd3d-shader/tpf: Support SV_InstanceID in vertex shaders.
    • 1b1c0934 - vkd3d-shader/tpf: Support SV_ViewportArrayIndex in pixel and vertex shaders.
    • c2dd5a1c - tests: Compile HLSL shaders at runtime in test_ps_layer().
    • 8bf6b352 - tests: Test using SV_RenderTargetArrayIndex in the vertex shader.
    • c2c1c96d - tests: Use test utils to create the pipeline state in test_ps_layer().
    • 949c390c - tests: Add a test for SV_ViewportArrayIndex.

    Compare with previous version

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • This should now be appropriately tested.

  • Henri Verbeet approved this merge request

    approved this merge request

    • Why both a FIXME and a spirv_compiler_error()?

    • I copied that from a similar instance. We're also using that scheme a lot in the DXIL parser. The reason I see is that the error is meant to be sent to the API client, so they know why their shader is failing compilation. But that information might not necessarily reach the Wine developer who is trying to make an application work with vkd3d, hence the FIXME.

    • Isn't the solution there for Wine to be printing vkd3d-shader errors as ERR? I thought I remembered a discussion specifically about this before.

    • It might be an alternative solution I guess. Right now, though, compilation errors are not even stored if logging is not enabled, while even if the application doesn't want logging (and, for example, vkd3d doesn't request logging when compiling to SPIR-V IIRC) the Wine developer could still be interested in seeing those failures. Also ERRs are shown even vkd3d-shader crashes before finishing the compilation. Of course both of these objections could be handled in a different way, but the current solution doesn't look terrible either to me, so I never cared too much.

    • Please register or sign in to reply
  • Alexandre Julliard added 123 commits

    added 123 commits

    • 949c390c...dd00e209 - 114 commits from branch wine:master
    • 6ac525d6 - vkd3d-shader/spirv: Use capability ShaderViewportIndexLayerEXT for decoration Layer.
    • 45495f54 - vkd3d-shader/spirv: Use capability ShaderViewportIndexLayerEXT for decoration ViewportIndex.
    • b500381b - vkd3d-shader/tpf: Support SV_RenderTargetArrayIndex in pixel and vertex shaders.
    • 3a90f3e2 - vkd3d-shader/tpf: Support SV_InstanceID in vertex shaders.
    • dc56320b - vkd3d-shader/tpf: Support SV_ViewportArrayIndex in pixel and vertex shaders.
    • 5ddf745d - tests: Compile HLSL shaders at runtime in test_ps_layer().
    • 245cce02 - tests: Test using SV_RenderTargetArrayIndex in the vertex shader.
    • bd84b96e - tests: Use test utils to create the pipeline state in test_ps_layer().
    • 72e2eeaf - tests: Add a test for SV_ViewportArrayIndex.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Isn't the solution there for Wine to be printing vkd3d-shader errors as ERR? I thought I remembered a discussion specifically about this before.

    Yes. My plan is essentially to wait for DXIL development to settle down a bit, and then do some cleanup to do just that, although I'd use WARN instead of ERR/FIXME. (I.e., errors in the input shader are generally not errors in vkd3d-shader.)

Please register or sign in to reply
Loading