[PATCH vkd3d 3/4] vkd3d-shader: Add support for retrieving the shader version through vkd3d_shader_scan().

[Zebediah Figura (she/her)]

On 1/6/22 04:15, Henri Verbeet wrote:
> On Tue, 28 Dec 2021 at 23:20, Zebediah Figura <zfigura at codeweavers.com> wrote:
>> Besides being the first patch implementing reflection, this patch introduces an
>> API problem with regard to D3DReflect() that will affect many different fields.
>> Namely, if the SM4 version token contains a nonsensical version D3DReflect()
>> will happily return it verbatim. As I see it our options are:
>>
>> (1) return the token verbatim anyway instead of using struct
>>      vkd3d_shader_version,
>>
>> (2) don't use vkd3d_shader_scan() for wine's D3DReflect() [and either diverge
>>      from native in vkd3d-utils' D3DReflect(), or don't use vkd3d_shader_scan()
>>      there either]
>>
>> (3) return the token verbatim in a separate field (and in a separate chained
>>      structure?)
>>
>> (4) punt, and use one of the above solutions if we ever do come across a shader
>>      that contains insane data.
>>
> I'd be inclined to go with option 3 here. Somewhat analogous to e.g.
> vkd3d_shader_spirv_target_info or vkd3d_shader_hlsl_source_info, we
> could perhaps do something like the following:
> 
>      struct vkd3d_shader_tpf_scan_info
>      {
>          ...
>          uint32_t version_token;
>          const char *creator;
>          uint32_t flags;
>          ...
>      };
> 
> We could perhaps slightly generalise that to something like
> "vkd3d_shader_d3d_scan_info", since the documentation claims the
> ID3D12ShaderReflection should be supported for the d3dbc, tpf and dxil
> targets, although in case of the dxil target that would normally be
> provided by the dxcompiler DLL instead of d3dcompiler.

I think that makes sense to me.

> 
>> There are some other, similar problems:
>>
>> * HLSL compile flags would be especially annoying to translate to e.g.
>>    libvkd3d-shader compile arguments and then back again. (Frankly, it's already
>>    annoying that we have to do that in the HLSL compiler.) This is basically the
>>    same problem as the above, but it forms an argument for returning tokens
>>    verbatim, or at least that one.
>>
> Sure.
> 
>> * Instruction counts (and I think everything else in the STAT chunk) are always
>>    taken from the STAT chunk; if the STAT chunk is missing then D3DReflect() will
>>    return zero, even though it could just count them. Should we preserve that
>>    behaviour in libvkd3d-shader? in libvkd3d-utils?
>>
> If we're going with something like
> vkd3d_shader_tpf_scan_info/vkd3d_shader_d3d_scan_info, probably yes.

Okay, I think that also makes sense. It then becomes an open question 
whether we should report more accurate information in struct 
vkd3d_shader_scan_function_info (or whatever the generic structure is) 
but that can be deferred until later.