Skip to content
Snippets Groups Projects

tests/shader-runner: Query for SM6 support instead of checking macro.

Merged Francisco Casas requested to merge fcasas/vkd3d:skip_without_unsupported_defs2 into master
2 unresolved threads

2/2 avoids skipping the SM6 tests on a Windows build, where the support is present but the VKD3D_SHADER_UNSUPPORTED_DXIL macro is not defined; a mistake I introduced with !1109 (merged).

Thanks @giomasce for pointing this out and suggesting querying the backend instead, this is also done for the GLSL backend in 1/2.

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
  • Giovanni Mascellani approved this merge request

    approved this merge request

  • added 1 commit

    • 60abe8d4 - tests/shader-runner: Query for SM6 support instead of checking macro.

    Compare with previous version

    • Why do we still have VKD3D_SHADER_UNSUPPORTED_DXIL at this point? What's missing for us to consider it supported? Do we need to have every opcode implemented?

    • IIRC the idea was to at least support the whole of SM 6.0, which is mostly there but a couple of MRs are still outstanding. Then eventually we should support higher SMs, but that's another thing.

      On top of that, my understanding is that the point of UNSUPPORTED macros isn't really saying that a feature is not complete as much as saying that we're not yet committed to preserve backward compatibility for that feature. I'm not aware of any specific issue why we consider that commitment problematic for SM6.

    • Please register or sign in to reply
  • added 1 commit

    • 0cf52370 - tests/shader-runner: Query for SM6 support instead of checking macro.

    Compare with previous version

    • Well, the change could be as simple as this:

      -#ifndef VKD3D_SHADER_UNSUPPORTED_DXIL
      +#if !defined(VKD3D_SHADER_UNSUPPORTED_DXIL) && !defined(VKD3D_CROSSTEST)

      It seems a bit questionable to run the shader runner tests against different versions of libvkd3d-shader, compiled with different configure options. If you're using the .shader_test files from the version used to compile the shader runner, todo's and the like generally aren't going to line up with the different libvkd3d-shader version. If instead you're using the .shader_test files from the version used to compile libvkd3d-shader, the format of the .shader_test files may not line up with what the shader runner accepts. We do make changes to the format from time to time, after all. And sure, you can probably make it work for a limited set of cases if you're careful, but if you're being careful, you might as well be careful about the options you pass to configure as well.

      That said, for the GLSL change, I'd suggest a helper function like this:

      static bool glsl_supported(void)
      {
          const enum vkd3d_shader_target_type *target_types;
          unsigned int count, i;
      
          target_types = vkd3d_shader_get_supported_target_types(VKD3D_SHADER_SOURCE_DXBC_TPF, &count);
          for (i = 0; i < count; ++i)
          {
              if (target_types[i] == VKD3D_SHADER_TARGET_GLSL)
                  return true;
          }
      
          return false;
      }

      Why do we still have VKD3D_SHADER_UNSUPPORTED_DXIL at this point? What's missing for us to consider it supported? Do we need to have every opcode implemented?

      I'm fairly sure I mentioned this sometime before the 1.13 release, but one thing I'd like to see is something along the lines of test_scan_signatures() for DXIL shaders. Mostly to make sure we don't e.g. return VKD3D_SHADER_COMPONENT_BOOL or VKD3D_SHADER_COMPONENT_DOUBLE for 16-bit integers, and then have to add a VKD3D_SHADER_API_VERSION check to fix it.

    • Author Developer

      I added the suggested change for the GLSL change.

    • Please register or sign in to reply
  • Francisco Casas added 2 commits

    added 2 commits

    • 5709f5be - tests/shader-runner: Query for GLSL target support instead of checking macro.
    • dcd47687 - tests/shader-runner: Query for SM6 support instead of checking macro.

    Compare with previous version

  • Francisco Casas added 2 commits

    added 2 commits

    • 2e0a24a3 - tests/shader-runner: Query for GLSL target support instead of checking macro.
    • 0985f106 - tests/shader-runner: Query for SM6 support instead of checking macro.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Henri Verbeet added 20 commits

    added 20 commits

    • 0985f106...828e5f12 - 18 commits from branch wine:master
    • 42f910e5 - tests/shader_runner: Query for GLSL target support instead of checking...
    • 95bd06be - tests/shader-runner: Query for SM6 support instead of checking VKD3D_SHADER_UNSUPPORTED_DXIL.

    Compare with previous version

  • Please register or sign in to reply
    Loading