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

Fix array removal for task/mesh in interface validation #5497

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Rua
Copy link

@Rua Rua commented Dec 3, 2023

The array removal in the interface validation wasn't correct for task and mesh shaders in a few ways:

  • The SPV_NV_mesh_shader extension states that PerTaskNV is only permitted on task output and mesh input. The current code tests for it on mesh output instead, which is pointless.
  • TaskNV, TaskEXT and MeshEXT shaders were not included. TaskEXT output and MeshEXT input do not have a PerTaskNV decoration, so they are always arrayed. MeshEXT output is also always arrayed.

Note that the Vulkan spec does not mention the special cases for task output and mesh input, because the arraying must always be matched on both sides of the interface, and therefore is irrelevant for merely matching the interface. But for the code here, which tries to retrieve the type that underlies the array, it still needs to be taken into account.

@CLAassistant
Copy link

CLAassistant commented Dec 3, 2023

CLA assistant check
All committers have signed the CLA.

@alan-baker
Copy link
Contributor

Apologies I haven't had time to review this yet, but I would like to see some unit tests added to cover the changes. I admit my familiarity with mesh and task shaders is fairly low so I'd appreciate having some simple examples added there. Separately has this patch been tested against Vulkan CTS?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants