(1) This patch has the user pass the signature from the pixel shader when compiling the vertex shader, and looks up register indices already arbitrarily allocated by the pixel shader. This is problematic when trying to use this signature as a cache key, by virtue of it not being clear (or even defined) which fields are key elements and which aren't. It's also not particularly kind to lookup, on account of not being directly comparable with memcmp(). There are a few options I see:
(a) Provide an internal function to compare key elements. This feels... odd, like a very special-purpose function, but perhaps workable. (b) Just make the user deal with it, and assert that all fields are key elements. (c) Use some alternative, perhaps shortened structure as a field of vkd3d_shader_next_stage_info. This has the disadvantage that it is not as simple for a hypothetical user to retrieve from the pixel shader, but we would presumably provide a function to generate one from a shader signature. This would probably also be kinder to cache lookup if it's shorter. (d) Make caching vkd3d's responsibility, to some degree. This seems daunting, but the more we optimize, the more difficult it may be to design API that allows for nice caching.
Yeah, the interstage interface is probably one of the harder parts to get right.
So it seems like what we're ultimately going to do with NEXT_STAGE_INFO is to construct an output map for the vertex shader. One (perhaps obvious) alternative then would be to pass that output map to vkd3d-shader directly, instead of NEXT_STAGE_INFO. The main advantage would be that such an output map could be stored in a fairly compact way if needed, and could easily be compared with memcmp().
There may also be variants we could do based on that. E.g., keep passing NEXT_STAGE_INFO to vkd3d_shader_compile(), but introduce a function that returns an output map consistent with what vkd3d_shader_compile() would use for a given signature pair. Or we could return the output map that would be used from vkd3d_shader_scan().
(2) Assuming we use signatures, there is a memory management problem that 5cfb9d930f11e spells out. This is probably a matter of "just fix it", but I suppose another option is to take the GLSL or ARB architecture.
We could, and it might not even be that hard, but it would of course introduce some extra allocations and copying that would be superfluous for more typical usage. In particular, the expectation is that we have essentially two kinds of uses of the API/structure:
- Users that keep neither the signature nor the shader around. E.g. because the user of the API is just printing some information about the shader, or e.g. because the user of the API is using its own structures to store whatever information it actually cares about.
- Users that keep both the signature and the shader around. E.g. because the API user isn't particularly concerned about just keeping the shader around, or because it needs to keep the shader around anyway because it will be needed again later.
I.e., the expectation is that it should be fairly uncommon to have a need to hold on to a struct vkd3d_shader_signature, but not the corresponding shader.
Note that "just fix it" doesn't make the issue in the commit in question that much easier to fix either; we'd still need to keep track of when it's safe to release the signature. We could wrap the signature in some structure with a refcount in wined3d, but in that case we could also just keep a reference to the shader the signature belongs to, and that might not end up being much worse in practice.