-
Notifications
You must be signed in to change notification settings - Fork 402
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
Improvements to bindless resources and descriptor indexing #2260
Merged
billhollings
merged 7 commits into
KhronosGroup:main
from
billhollings:mtl3-arg-buff-support
Jul 3, 2024
Merged
Improvements to bindless resources and descriptor indexing #2260
billhollings
merged 7 commits into
KhronosGroup:main
from
billhollings:mtl3-arg-buff-support
Jul 3, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The pipeline option was structurally permitted and designed for, but was never implemented. With the availability of Metal 3, which provides significant additional bindless argument buffer support, this option is being removed, in favor of future code enhancements that build on Metal 3. - Base all argument buffer support around descriptor sets. - Rename internal functions that test for support for argument buffers. - Update code documentation accordingly.
- Add MVKPhysicalDeviceMetalFeatures::needsArgumentBufferEncoders to indicate if Metal argument buffer encoders are needed to populate argument buffer content. - Update MVK_PRIVATE_API_VERSION to 38. - Add MVKArgumentBufferEncoder to handle populating Metal argument buffers with or without Metal argument encoders, and pass this to descriptors. - Indicate support for argument buffers on iOS, when Metal 3 is available. - MVKConfiguration::useMetalArgumentBuffers set to MVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS_ALWAYS by default, which means Metal argument buffers are now used by default. - runcts script enables argument buffers by default.
…rSets(). - Combine MVKMTLArgumentEncoder and MVKArgumentBufferEncoder into MVKMetalArgumentBuffer, which holds an arg buffer and its encoder, removing need to lock the encoder. - MVKDescriptorSet tracks argument buffer, and calls to vkUpdateDescriptorSets() directly insert resources into the argument buffer. - Add MVKDescriptor encodeResourceUsage() to encode indirect resource usage when using argument buffers.
- Include descriptor element count when calculating Metal3 argument buffer encoded size. - Support MVKPhysicalDeviceMetalFeatures::nativeTextureAtomics in argument buffer operations. - Add ability to log descriptor set layout. - Support debug logging alongside other logging levels.
- Update SPIRV-Cross version to include latest argument buffer fixes. - Disable CompilerMSL::Options::force_active_argument_buffer_resources. - Track OpArrayLength buffer-sizes buffer as an auxiliary buffer in each descriptor set argument buffer, as this is how SPIRV-Cross expects it. - Revert MVKConfiguration::useMetalArgumentBuffers and env var MVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS to boolean value, because descriptor indexing value is not longer required. - Update max number of buffer and textures per stage to 1M. - Refactor and simplify calculating argument buffer resource indexes - Remove MVKShaderStageResourceBinding::resourceIndex, and track resource index incrementing locally. - Update MVKDescriptorSetLayout::_descriptorCount & _mtlResourceCount from within MVKDescriptorSetLayoutBinding. - Refactor and simplify tracking argument buffer usage - Rename MVKDeviceTrackingMixin::isUsingDescriptorSetMetalArgumentBuffers() to isUsingMetalArgumentBuffers(), and allow it to be overridden in MVKDescriptorSetLayout. - Move and rename MVKDevice::_isUsingDescriptorSetMetalArgumentBuffers to MVKPhysicalDevice::_isUsingMetalArgumentBuffers. - Remove MVKPhysicalDevice::supportsDescriptorSetMetalArgumentBuffers(); - Remove MVKDescriptorSetLayout::isUsingMetalArgumentBuffer() and replace with overridden isUsingMetalArgumentBuffers() function. - Rename MVKDescriptorSet::isUsingMetalArgumentBuffer to hasMetalArgumentBuffer(). - Remove MVKDescriptorSetLayoutBinding::isUsingMetalArgumentBuffer(). - Remove MVKDescriptorSetLayout::isPushDescriptorLayout(). - Update debugging logging - Refactor and enhance ability to log descriptor set descriptions for debugging. - MVKPipelineLayout support debug logging and remove a few unused member functions. - Adjust whitespace in logging of instances and devices. - Update MVK_PRIVATE_API_VERSION to version 43.
a546911
to
a3908bf
Compare
cdavis5e
requested changes
Jul 2, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about the wait. This was, as you noted, a large change.
Thanks for reviewing this large PR so quickly! I made one suggested change, responded to the others, and asked for your thoughts on a couple of discussion items. |
cdavis5e
approved these changes
Jul 3, 2024
This was referenced Jul 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a large PR that adds significant improvements to the handling of bindless resources and descriptor indexing functionality.
MVKConfiguration::useMetalArgumentBuffers
and env varMVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS
to a boolean value, and enable it by default.Further detailed outlines of the additions and changes to functionality are contained in the notes for each commit.
The new use of argument buffers by default, plus the use of either Metal3 argument buffers or argument buffer encoding, all pass the same CTS tests that are passed using discrete resource indexes, and that were being passed prior to these changes.