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().
Yeah, that was broadly what I was trying to propose with (c).
Not sure if I should take this as an endorsement or preference of that approach, but I at least can't say I like anything else better, so I may just try to start writing an implementation thereof.
I'm mostly just proposing ideas at this stage. Something like this seems the most workable to me, but I haven't entirely thought it through, and it's entirely possible that better alternatives exist. To the extent that I have a preference, I'm leaning towards the option of keeping NEXT_STAGE_INFO and then returning the resulting output map from vkd3d_shader_scan(), but it's only a weak preference at this point.
Side note: maybe this should be a vkd3d_shader_parameter instead? I think it'd probably be good to lean in that direction, rather than adding more chained structures, for new compilation options like this. Mostly because it's easier to design and, I think, easier to use. It'll require rerolling vkd3d_shader_parameter which unfortunately can only have uint32_t in it, but we needed that anyway.
What would that look like? Shader parameters were roughly intended as the vkd3d-shader equivalent of SPIR-V specialisation constants; it would probably make sense to use them for e.g. spirv_compiler_emit_sample_position(), but it's less clear to me whether the interstage interface would be a great fit.
As a side note—when I wrote vkd3d_shader_next_stage_info I had in mind that it could be extended with other aspects of the next shader. The only one of these that I can currently actually think of is the shader type, which I think GLSL cares about sometimes.
Yeah, I'd suggest adding the shader type if we're going with NEXT_STAGE_INFO; I haven't commented much on the specifics so far, mostly just in order to focus on the broader questions first. I seem to recall that there may also be some cases where we'd need to initialise vertex shader outputs that aren't otherwise written by the vertex shader, but I'd need to check that.
Or, perhaps more saliently, I could see someone assuming (despite the documentation) or forgetting about that implicit dependency, and causing subtle UAF problems as a result.
Or I could see an argument for someone wanting to, say, compile a shader into SPIRV, keep the compiled shader and the signature around, and throw away the original.
Which is not me trying to belabour the argument in favor of "fixing" the "bug"—I'm not even fully convinced that it needs fixing either—but it does make me nervous.
Yeah, I think those were concerns at the time as well. Note that if we do decide to change this there may be broader implications as well; we do this kind of thing in a number of other places as well.
There is a performance question here—do we care about avoiding shader recompilation when multiple different PS's with identical signatures are used? Wine seems to care, at least enough to avoid recompiling the VS into GLSL.
Generally, yes. I think the driver part of shader compilation is probably more expensive than anything we do, but we can at least attempt to not make it worse or to mitigate it somewhat. And while much of the world seems to have thrown up their hands and said "well, shader caches" at the problem of shader/pipeline compilation stutter, we do in theory have VK_EXT_shader_object now.