Add HAL methods to update descriptor set #179
Merged
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
This is WIP because only the Metal implementation is added. Part of the work for #175
dfrg
approved these changes
Jul 10, 2022
There was a problem hiding this comment.
This all looks simple, clean and correct to me.
One thought on related work: we will need some support for (non-storage) image and sampler descriptors for image rendering but my inclination is to special case and encapsulate the whole render pipeline state in HAL for now and expose the detailed bits if/when it becomes necessary.
Good to merge!
|
Yes, image and sampler descriptors will require more code; especially for the DX12 implementation where there is a lot of granularity about which heap the descriptor is allocated into. Hopefully that will be only somewhat tedious, rather than requiring any particular trickiness. |
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 patch adds the capability to the HAL to update descriptor sets.
Part of the work for #175 but might be useful for other purposes (such as window resize) and is fairly self-contained.
Note that there are rustfmt changes in unrelated areas, due to rustfmt not being reliably applied through macros. I'd like to figure out a better solution to that in CI.