[PATCH vkd3d] vkd3d-shader/hlsl: Fix type comparison for column-major matrices.

[Giovanni Mascellani]

Previously hlsl_types_are_equal considered two matrices of different
type if one of those had an explicit column-major modifier while the
other didn't have any (in which case the matrix is still considered
column-major by default).

Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 8a2286c4..30098ea3 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -327,8 +327,8 @@ bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2
         return false;
     if (t1->base_type == HLSL_TYPE_SAMPLER && t1->sampler_dim != t2->sampler_dim)
         return false;
-    if ((t1->modifiers & HLSL_MODIFIERS_MAJORITY_MASK)
-            != (t2->modifiers & HLSL_MODIFIERS_MAJORITY_MASK))
+    if ((t1->modifiers & HLSL_MODIFIER_ROW_MAJOR)
+            != (t2->modifiers & HLSL_MODIFIER_ROW_MAJOR))
         return false;
     if (t1->dimx != t2->dimx)
         return false;
-- 
2.33.0

[Zebediah Figura (she/her)]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>

(I don't like the situation with matrix majority, or the amount of 
thought I had to put into this to make sure this was correct... but I 
suppose I'm at least partly to blame for making it this way.)

[Giovanni Mascellani]

Hi,

Il 28/09/21 04:23, Zebediah Figura (she/her) ha scritto:
> (I don't like the situation with matrix majority, or the amount of 
> thought I had to put into this to make sure this was correct... but I 
> suppose I'm at least partly to blame for making it this way.)

Yeah, it's a bit strange, because we try to represent with two bits what 
clearly has precisely two alternative possibilities (column- or row-major).

I think this is an artifact of how parsing works: each keyword simply 
sets the corresponding bit, and then there is some logic that checks 
that it is not the case that both bits are set. It is not really wrong 
to keep this behavior, provided that, once the check is done, the 
row-major bit is always used and the column-major is ignored.

In order to reduce the probability of mistakes, I would drop the 
constant HLSL_MODIFIERS_MAJORITY_MASK (unless it turns out that there 
are cases where it is useful) and always query majority using a 
dedicated macro like

#define HLSL_IS_ROW_MAJOR(modifiers) ((modifiers) & HLSL_MODIFIER_ROW_MAJOR)

Thanks, Giovanni.

[Henri Verbeet]

Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>