On Tue Mar 26 11:26:16 2024 +0000, Jan Sikorski wrote:
I've got a couple questions here. Isn't VK_DYNAMIC_STATE_PRIMITIVE_TOPOLOGY_EXT provided by extended dynamic state 1? Why put it under 2? (I may be forgetting why it wasn't included previously?) Why are we setting `key->ia_desc.topology` and `update` here, was that not supposed to be dynamic? It looks like this patch is only for dynamic primitive restart.
So in answer to the second part, why we're setting key->ia_desc.topology: VK_DYNAMIC_STATE_PRIMITIVE_TOPOLOGY_EXT is awkward and doesn't actually get us all the way there. The pipeline state needs to contain the "class", so we can dynamically set the state as long as we don't change class. Note we're using vk_topology_class_from_wined3d() here not vk_topology_from_wined3d().
There is a dynamicPrimitiveTopologyUnrestricted property, part of EDS3, and we could just require that if we're going to offer dynamic topology at all.
The reason we need EDS2 is actually a similar problem, but I solved it the other way. VUID 00428 requires (annoyingly) that we disable primitive restart if we're outputting a list, so that adds more cases where we may need to invalidate the pipeline state. We could indeed handle primitive topology dynamically and just invalidate the pipeline state desc anyway if we don't have EDS2, but I decided to just roll dynamic topology and dynamic primitive restart together.
The difference is a bit inconsistent, though it's worth noting that my radv machine does expose EDS2 but not dynamicPrimitiveTopologyUnrestricted. I don't know if we get anything out of dynamic primitive topology, but as mentioned before I haven't seen clear signs that we get anything out of any part of dynamic state.