Closed zakorgy closed 5 years ago
Thanks for the review @kvark, it's good to have you back. I have answered the questions, which I think needed explanation. I can agree with the remaining ones, or we will discuss them on the next call.
The changes related to push constants (beec365a64dec910616c0490969a694413b759d2) are not working on Windows. This could be an Intel driver issue and related to https://software.intel.com/en-us/forums/graphics-driver-bug-reporting/topic/777459 .
@acohade Is Intel Developer Zone the right place to report driver issues to you? Seeing this bug not having a single reply in a year makes me worried.
FWIW, I ran Vulkan CTS "push_constant" section on Iris 550 / Windows 10 (driver version 23.20.16.4973) and got 17/17 passed tests. That is to say, I'm not seeing the issue reported in the developer zone either. Perhaps, it got accidentally fixed in the meantime?
Added a push_constants
feature as a fast workaround for the Intel issue in the last commit. I will merge this and the remaining work (better state tracking for the descriptors, immutable samplers, moving pipeline layout in the code) will go in a different PR
This depends on #280
The general idea behind this PR is described in https://github.com/szeged/webrender/issues/236, and here is the list of changes I made:
Locals
uniforms in a different descriptor sets. It was previously in the same group with the per draw changing descriptor sets. Separating them makes them more manageable in term of descriptor usage, because they depend on different kind of resources.DescPool
for these uniforms, (we don't need aDescriptorPools
for these because we only have only one kind of layout) and added mapping between theLocals
and descriptor sets indices from the pool. This way we don't need to create new set for a usedLocals
, just use the stored one.~ TheLocals
part uses push constants instead of using DescriptorSets.DescriptorSetResources
) are now the group of the shader (there are 3 of them, see:ShaderGroup
) and the bound textures (5 texture). Here I kept theDescriptorPools
for each frame, because we can't share a descriptor set between a submitted frame and the next one (the validation layer tells us not to do so). If a texture is removed, we invalidate all the mapping which contains it's id, and mark the descriptor set free to use again. This has the benefit that we don't need to track the lifetime of the mapped descriptor sets becauseRenderer
does it for us with the textures: if a texture is removed then it's related descriptors are returned to the pool. This approach greatly reduced the number of used descriptor sets for wrench tests: we only needed to create around 500 descriptor sets for around 9000 descriptor set requests.