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

Development support for future VK_KHR_portability_subset_metal extension. #2190

Conversation

billhollings
Copy link
Contributor

  • fetchDependencies script pulls VK_KHR_portability_subset_metal content from private Khronos repo.
  • Add VK_KHR_portability_subset_metal extension.
  • Include VkPhysicalDevicePortabilitySubsetMetalFeaturesKHR in pNext of VkPhysicalDeviceFeatures2.

…ion.

- fetchDependencies script pulls VK_KHR_portability_subset_metal
  content from private Khronos repo.
- Add VK_KHR_portability_subset_metal extension.
- Include VkPhysicalDevicePortabilitySubsetMetalFeaturesKHR
  in pNext of VkPhysicalDeviceFeatures2.
@billhollings
Copy link
Contributor Author

GitHub CI is failing because it doesn't have access to the private Khronos repo. This is expected.

Copy link
Collaborator

@aitor-lunarg aitor-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you!

@alyssarosenzweig
Copy link

alyssarosenzweig commented Mar 19, 2024

shaderImageGatherExtendedConstOffsets

This is straightforward to lower, as most native drivers do? https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/compiler/nir/nir_lower_tex.c?ref_type=heads#L1253

@cdavis5e
Copy link
Collaborator

shaderImageGatherExtendedConstOffsets

This is straightforward to lower, as most native drivers do? https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/compiler/nir/nir_lower_tex.c?ref_type=heads#L1253

Sadly, not as straightforward as one would like. It's simple enough to expand it to four texture::read() calls, but the problem is handling sampler coordinate wrapping. We can't introspect the sampler object for its wrap modes, so we'd have to pass this information alongside the sampler. This becomes particularly problematic when not using argument buffers, which we don't at all by default.

@alyssarosenzweig
Copy link

shaderImageGatherExtendedConstOffsets

This is straightforward to lower, as most native drivers do? https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/compiler/nir/nir_lower_tex.c?ref_type=heads#L1253

Sadly, not as straightforward as one would like. It's simple enough to expand it to four texture::read() calls, but the problem is handling sampler coordinate wrapping. We can't introspect the sampler object for its wrap modes, so we'd have to pass this information alongside the sampler. This becomes particularly problematic when not using argument buffers, which we don't at all by default.

See the linked lowering. You turn it into 4 gathers

@billhollings
Copy link
Contributor Author

See the linked lowering. You turn it into 4 gathers

Thanks for the suggestion, and example. I've added it to the full issue discussion.

@billhollings billhollings merged commit fbf45b6 into KhronosGroup:VK_KHR_portability_subset_metal Mar 20, 2024
2 of 6 checks passed
@billhollings billhollings deleted the VK_KHR_portability_subset_metal branch March 20, 2024 14:38
@billhollings
Copy link
Contributor Author

See the linked lowering. You turn it into 4 gathers

Thanks for the suggestion, and example. I've added it to the full issue discussion.

SPIRV-Cross PR #2325 adds support for image gather with ConstOffsets.

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.

4 participants