Add new sample for VK_EXT_descriptor_heap#1493
Conversation
Both not required, as the sample uses dynamic rendering
Core since 1.1
|
Please read:
There is nothing misleading about this example. This PR is also a DRAFT. |
# Conflicts: # framework/vulkan_type_mapping.h
|
This PR is now ready to review. I also added documentation/tutorial. A variation using untyped pointers will follow independently at a later pointer. |
asuessenbach
left a comment
asuessenbach
left a comment
There was a problem hiding this comment.
Looks great!
Just a couple of mostly minor issues:
asuessenbach
left a comment
asuessenbach
left a comment
There was a problem hiding this comment.
I think, we should not use const_cast in our samples. It's dangerous and not a good coding style.
But if nobody else complains, I'm ok with it.
As discussed, I added a non-const function in the allocated class and removed all const casts from the sample. |
| * @return The pointer to the host visible memory. | ||
| * @note Non-const. This performs no checking that the memory is actually mapped, so it's possible to get a nullptr | ||
| */ | ||
| uint8_t *get_mapped_data() const; |
There was a problem hiding this comment.
A function returning a non-const pointer should not be const.
Moreover, it's hard to see the difference to get_data(), just by looking at the name.
Maybe you could change get_data() to uint8_t const * get_const_data() const; and change the new function to uint8_t * get_data();?
There was a problem hiding this comment.
If okay, I'd do that after this PR has been merged. Don't want to defer this any further.
iagoCL
left a comment
iagoCL
left a comment
There was a problem hiding this comment.
I think the PR is good.
I could only test this on Nvidia hardware, but the code looks good and is a nice overview of the basic parts of how to use descriptor heap.
Happy for this to be merged.
|
|
||
| === Descriptor set and binding mappings | ||
|
|
||
| link:https://docs.vulkan.org/refpages/latest/refpages/source/VkDescriptorSetAndBindingMappingEXT.html[VkDescriptorSetAndBindingMappingEXT] can be used to simplify porting existing codebases to descriptor heaps. By using this, existing shaders can be used without having to modify them. The syntax for binding descriptors, regardless of shading language, stays the same. |
There was a problem hiding this comment.
I would add a small note:
We are only showing VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_CONSTANT_OFFSET_EXT but there are more binding-mapping options. Developers should check https://docs.vulkan.org/guide/latest/descriptor_heap.html#_mapping_the_heap_to_exisiting_shaders and consider the best strategy for their use case.
I do not want to delay this so I am fine with merging it as it is.
3 participants
Description
This PR adds a new sample for the recently released VK_EXT_descriptor_heap extension. It shows off different heap types (resources, samplers). It only does the descriptor set and binding mapping way of using heaps. Untyped pointers might follow at a later point of certain things have been resolved (see our internal discussions).
Also comes with documentation/tutorial material.
Tested on Win11 with an Nvidia RTX 4070.
Fixes #1486
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batchcommand line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: