Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VK_EXT_mesh_shader: Further plans #2375

Open
zmarlon opened this issue Oct 13, 2024 · 8 comments
Open

VK_EXT_mesh_shader: Further plans #2375

zmarlon opened this issue Oct 13, 2024 · 8 comments
Labels
Answered A question was answered Question

Comments

@zmarlon
Copy link

zmarlon commented Oct 13, 2024

Hello, I have a suggestion regarding the implementation of the extension. At the moment it is the case that emitMeshTasksEXT terminates the execution of the task shader. However, this is not the case in Metal. The possibilities that exist now are the following:

  1. Not to support Task Shader at all
  2. Support a subset of the task shaders, which would still cover a huge number of use cases.

I don't like this solution, as mesh shaders offer an enormous benefit for writing a high-performance modern game engine. Therefore, I would suggest the second solution.

@billhollings What is your opinion on this? It would be good if we could come to an agreement on this, because if the SPIRV Cross PR was merged we could finally close the PR on the MoltenVK side. Future changes regarding the terminating could still be made on the Spirv Cross side, but it would be a good start.

@zmarlon
Copy link
Author

zmarlon commented Oct 13, 2024

Tagging a few people that might be Interested @BeastLe9enD @spnda @Try @squidbus

@BeastLe9enD
Copy link
Contributor

BeastLe9enD commented Oct 13, 2024

I think it makes sense to add initial support for task shaders even if only a subset is supported, because only supporting mesh shaders would be against the spec too and so it makes more sense to add the existing task shader subset. The only question is how to expose this to the user to get a feedback that the extension is only half-supported.

We could add a VkPhysicalDeviceMeshShaderPortabilityFeaturesMVK like structure that needs to be passed along the VkPhysicalDeviceFeatures2 when VkPhysicalDeviceMeshShaderFeaturesEXT is inside pNext in order to succeed device creation.

When task shaders are fully supported, the only thing we need to do is remove this check that ensures device creation only succeeds when our portability struct is passed a long. All other changes would only need to happen on the spirv cross side, as the current limitation happens due to incomplete spirv -> msl translation

Im open for further discussion, but this is the way I would take.

@Try
Copy link

Try commented Oct 30, 2024

Mesh shader for spirv-cross:
KhronosGroup/SPIRV-Cross#2400, and KhronosGroup/SPIRV-Cross#2401 are merged.

Task shader on review:
KhronosGroup/SPIRV-Cross#2402

@billhollings
Copy link
Contributor

Mesh shader for spirv-cross: KhronosGroup/SPIRV-Cross#2400, and KhronosGroup/SPIRV-Cross#2401 are merged.

Task shader on review: KhronosGroup/SPIRV-Cross#2402

Thanks for the excellent work. I look forward to a MoltenVK PR that pull this all in!


At the moment it is the case that emitMeshTasksEXT terminates the execution of the task shader. However, this is not the case in Metal. The possibilities that exist now are the following:

  1. Not to support Task Shader at all
  2. Support a subset of the task shaders, which would still cover a huge number of use cases.

I agree that we should support as much as we possibly can. Once a PR is ready for inclusion into MoltenVK, we can run a full suite of CTS tests to determine what is not passing, and figure out if we can work around it, or whether we need to publish what is not working, so developers are aware, when using it.

@billhollings billhollings added the Answered A question was answered label Oct 31, 2024
@squidbus
Copy link
Contributor

squidbus commented Nov 1, 2024

Looks like both the mesh and task shader PRs are now merged on the SPIRV-Cross side

@zeux
Copy link

zeux commented Nov 19, 2024

only supporting mesh shaders would be against the spec too

My understanding is that it is valid for an implementation to expose EXT_mesh_shader without task shaders; this is indicated via VkPhysicalDeviceMeshShaderFeaturesEXT::taskShader bit. Mesh shaders are definitely useful in isolation (especially given issues with task shader performance on AMD hardware).

@squidbus
Copy link
Contributor

squidbus commented Jan 8, 2025

Has there been any movement on the MoltenVK side of this since SPIRV-Cross got mesh shader support?

@BeastLe9enD
Copy link
Contributor

@squidbus The last state was a working mesh shader version In ProjectKML@88e5f00. Task shader is untested, but should also work. I'll come back to this once I find the time, but I cannot promise when yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Answered A question was answered Question
Projects
None yet
Development

No branches or pull requests

6 participants