infapi00
commented
5 years ago Perhaps one way to fix this in the script format could be to extend the binding name to have an optional array index as the third argument. For example, it could look something like this:
I think that that makes sense, and in fact was easy to add. Having said so ...
In Vulkan, the buffers bound to a uniform block declared as an array are all bound to a single binding point. The VkRunner script format doesn’t currently provide any way to specify the buffers in a binding array and descriptorCount in VkDescriptorSetLayoutBinding is always set to 1.
Setting properly the array of ubo/ssbo goes beyond setting the descriptorCount number, as you also need to group the descriptors properly, and also the need to decide what to do with the buffers associated to each array of ubos.
Here I found two alternatives for how to use the buffers for each arrays of UBO. Let's take a look to allocate_ubo_buffers. There for the VkWriteDescriptorSet, we would need to set there the descriptorCount to the array size. But for the array of DescriptorBufferInfo, we could reuse the same VkBuffer, toying with the offsets. But as that needs to be aligned, would make computing that offset complex. So on that side, having one VkBuffer per BufferInfo, so per each binding::array_index combination is the easier.
But now it cames the tricky part: initially I just tried to keep things simple, and add an array_index parameter on vr_script_buffer. But that breaks the assumption at several places of the code that we would have a ubo per each buffer. Let¡s take for example create_vk_descriptor_set_layout at vr-pipeline.c. It iterates the buffers, and counts ubos/ssbos depending on the type, assumming a one-to-one relation. But there we would need to group the buffers with the same binding, in order to compute the descriptorCount for each VkDescritorSetLayoutBinding, and adjust the pool_sizes below. This grouping also applies to other part of the code.
So in the end I think that "keeping things simple" would not be enough. Thinking out loud, I think that the solution would be replace the vr_script_buffer list at vr_script for a vr_script_binding (or even vr_script_binding_layout) list. vr_script_binding would maintain the binding info plus a list of vr_script_buffers. And I bet that this would apply to other parts of the code where you would need to group the buffers.
Additionally, we need to take into account how all this would affect the descriptor set management at vr-pipeline. But again, I guess that one solution would be maintaining a more hierarchical data structure, instead of basically a list of buffers that we try to group at the relevant places. Somehow, I think that the data to be maintained by vr-script would need to be more similar to the high-level Vulkan structures (like VkDescriptorSetLayoutBinding). Its true that that would make filling the data somewhat more complex, but I think (hope) that would make creating the vulkan structures easier.
Finally, this is a long way to say that I have been looking to it a little to check if getting this done was just adding the parse method, and use that info there and there, and my conclusion is that is not, and several changes are needed. And I fear that I don't have the time to do it myself right now. I hope that at least my analysis is understandable enough to be useful to the person tackling this task.
samuelig
In Vulkan, the buffers bound to a uniform block declared as an array are all bound to a single binding point. The VkRunner script format doesn’t currently provide any way to specify the buffers in a binding array and
descriptorCountinVkDescriptorSetLayoutBindingis always set to 1.Perhaps one way to fix this in the script format could be to extend the binding name to have an optional array index as the third argument. For example, it could look something like this: