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

[Matteo Bruni]

On Thu, Oct 7, 2021 at 6:34 PM Zebediah Figura (she/her)
 wrote:
> 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.

That sounds like a good plan.