[PATCH vkd3d v2 3/5] vkd3d-shader/hlsl: Parse explicitly typed texture types.

[Zebediah Figura (she/her)]

On 10/7/21 11:19, Zebediah Figura (she/her) wrote:
> On 10/7/21 06:58, Giovanni Mascellani wrote:
>> Hi,
>>
>> Il 06/10/21 16:45, Zebediah Figura ha scritto:
>>> @@ -344,9 +345,14 @@ bool hlsl_types_are_equal(const struct hlsl_type 
>>> *t1, const struct hlsl_type *t2
>>>           return false;
>>>       if (t1->base_type != t2->base_type)
>>>           return false;
>>> -    if ((t1->base_type == HLSL_TYPE_SAMPLER || t1->base_type == 
>>> HLSL_TYPE_TEXTURE)
>>> -            && t1->sampler_dim != t2->sampler_dim)
>>> -        return false;
>>> +    if (t1->base_type == HLSL_TYPE_SAMPLER || t1->base_type == 
>>> HLSL_TYPE_TEXTURE)
>>> +    {
>>> +        if (t1->sampler_dim != t2->sampler_dim)
>>> +            return false;
>>> +        if (t1->base_type == HLSL_TYPE_TEXTURE && t1->sampler_dim != 
>>> HLSL_SAMPLER_DIM_GENERIC
>>> +                && !hlsl_types_are_equal(t1->e.resource_format, 
>>> t2->e.resource_format))
>>> +            return false;
>>> +    }
>>>       if ((t1->modifiers & HLSL_MODIFIER_ROW_MAJOR)
>>>               != (t2->modifiers & HLSL_MODIFIER_ROW_MAJOR))
>>>           return false;
>>
>> Shouldn't compare_param_hlsl_types be changed in a similar way?
> 
> Indeed; thanks for catching that.
> 
>>
>> More in general, do we really need both hlsl_types_are_equal and 
>> compare_param_hlsl_types? Couldn't hlsl_types_are_equal just cast to 
>> bool the result on compare_param_hlsl_types?
> 
> They don't behave exactly the same, though. In particular, 
> compare_param_hlsl_types() will count "float" as equal to "float1", and 
> "row_major float4x4" as equal to "column_major float4x4".
> 
> Some brief testing suggests this is incorrect, though. Native will let 
> you do this:
> 
> void func(float a) {}
> 
> void func(float1 a) {}
> 
> It will also accept "row_major float4x4" alongside "column_major 4x4". 
> Even worse, though, it will accept "float4x4" alongside "column_major 
> 4x4", "float" alongside "const float" alongside "volatile float", and 
> even the following:
> 
> void func(float a) {}
> 
> typedef float myfloat;
> void func(myfloat a) {}
> 
> It won't accept "float" alongside "in float", though. That's too far. 
> But it will accept "float" alongside "inout float" alongside "out float".
> 
> Of course, attempting to actually *call* func in any of these cases 
> results in an "ambiguous function call" error. So it's not completely 
> insane.
> 

Anyway, what I was going to say is that we probably don't want that 
function (and if we do really need differences we may want to implement 
them with flags instead; cf. expr_compatible_data_types().) But it 
deserves some actual unit tests.