Change math module to behave like GLSL (#738)

* Fix Mat3, Mat4 multiplication to assume column-major ordering

* Add test that Mat3 multiplication is associative

* examples: Fix matrix multiplication order

* math: Remove look_at_direction (broken and redundant)

* math: Document that behaviour matches GLSL

* math: Clarify projection docs

* Revert "Update Matrix reprs to show columns"

This reverts (part of) commit 9a19a74.
The use of pipes doesn't really communicate columns; they could still be
transposed rows. To be most clear, we should show matrices in the same way they
are input, as before.

examples/3dmodel/model.py

@@ -31,13 +31,13 @@ def animate(dt):
     rot_y = Mat4.from_rotation(time/2, Vec3(0, 1, 0))
     rot_z = Mat4.from_rotation(time/3, Vec3(0, 0, 1))
     trans = Mat4.from_translation(Vec3(1.25, 0, 2))
-    model_logo.matrix = rot_x @ rot_y @ rot_z @ trans
+    model_logo.matrix = trans @ rot_x @ rot_y @ rot_z
 
     rot_x = Mat4.from_rotation(time, Vec3(1, 0, 0))
     rot_y = Mat4.from_rotation(time/3, Vec3(0, 1, 0))
     rot_z = Mat4.from_rotation(time/2, Vec3(0, 0, 1))
     trans = Mat4.from_translation(Vec3(-1.75, 0, 0))
-    model_box.matrix = rot_x @ rot_y @ rot_z @ trans
+    model_box.matrix = trans @ rot_x @ rot_y @ rot_z
 
 
 if __name__ == "__main__":

examples/opengl/opengl.py

@@ -82,7 +82,7 @@ def update(dt):
     rot_y = Mat4.from_rotation(time/2, Vec3(0, 1, 0))
     rot_z = Mat4.from_rotation(time/4, Vec3(0, 0, 1))
     trans = Mat4.from_translation((0, 0, -3.0))
-    torus_model.matrix = rot_x @ rot_y @ rot_z @ trans
+    torus_model.matrix = trans @ rot_x @ rot_y @ rot_z
 
 def setup():
     # One-time GL setup

pyglet/math.py

@@ -41,6 +41,9 @@
 transforming. The :py:class:`~pyglet.matrix.Mat4` includes class methods
 for creating orthographic and perspective projection matrixes.
 
+Matrices behave just like they do in GLSL: they are specified in column-major
+order and multiply on the left of vectors, which are treated as columns.
+
 :note: For performance, Matrixes subclass the `tuple` type. They
     are therefore immutable - all operations return a new object;
     the object is not updated in-place.
@@ -696,36 +699,33 @@ def __matmul__(self, other: Mat3) -> Mat3:
 
     def __matmul__(self, other):
         if isinstance(other, Vec3):
-            # Columns:
-            c0 = self[0::3]
-            c1 = self[1::3]
-            c2 = self[2::3]
-            return Vec3(sum(map(_mul, c0, other)),
-                        sum(map(_mul, c1, other)),
-                        sum(map(_mul, c2, other)))
+            # Rows:
+            r0 = self[0::3]
+            r1 = self[1::3]
+            r2 = self[2::3]
+            return Vec3(sum(map(_mul, r0, other)),
+                        sum(map(_mul, r1, other)),
+                        sum(map(_mul, r2, other)))
 
         if not isinstance(other, Mat3):
             raise TypeError("Can only multiply with Mat3 or Vec3 types")
 
         # Rows:
-        r0 = self[0:3]
-        r1 = self[3:6]
-        r2 = self[6:9]
+        r0 = self[0::3]
+        r1 = self[1::3]
+        r2 = self[2::3]
         # Columns:
-        c0 = other[0::3]
-        c1 = other[1::3]
-        c2 = other[2::3]
+        c0 = other[0:3]
+        c1 = other[3:6]
+        c2 = other[6:9]
 
-        # Multiply and sum rows * colums:
-        return Mat3((sum(map(_mul, r0, c0)), sum(map(_mul, r0, c1)), sum(map(_mul, r0, c2)),
-                     sum(map(_mul, r1, c0)), sum(map(_mul, r1, c1)), sum(map(_mul, r1, c2)),
-                     sum(map(_mul, r2, c0)), sum(map(_mul, r2, c1)), sum(map(_mul, r2, c2))))
+        # Multiply and sum rows * columns:
+        return Mat3((sum(map(_mul, c0, r0)), sum(map(_mul, c0, r1)), sum(map(_mul, c0, r2)),
+                     sum(map(_mul, c1, r0)), sum(map(_mul, c1, r1)), sum(map(_mul, c1, r2)),
+                     sum(map(_mul, c2, r0)), sum(map(_mul, c2, r1)), sum(map(_mul, c2, r2))))
 
     def __repr__(self) -> str:
-        array = [str(num)[:12] for num in self]
-        return (f"| {array[0]:>12s} | {array[3]:>12s} | {array[6]:>12s} |\n"
-                f"| {array[1]:>12s} | {array[4]:>12s} | {array[7]:>12s} |\n"
-                f"| {array[2]:>12s} | {array[5]:>12s} | {array[8]:>12s} |\n")
+        return f"{self.__class__.__name__}{self[0:3]}\n    {self[3:6]}\n    {self[6:9]}"
 
 
 class Mat4(tuple):
@@ -768,7 +768,11 @@ def orthogonal_projection(
         z_near: float,
         z_far: float
     ) -> Mat4T:
-        """Create a Mat4 orthographic projection matrix."""
+        """Create a Mat4 orthographic projection matrix for use with OpenGL.
+
+        This matrix doesn't actually perform the projection; it transforms the
+        space so that OpenGL's vertex processing performs it.
+        """
         width = right - left
         height = top - bottom
         depth = z_far - z_near
@@ -795,7 +799,10 @@ def perspective_projection(
         fov: float = 60
     ) -> Mat4T:
         """
-        Create a Mat4 perspective projection matrix.
+        Create a Mat4 perspective projection matrix for use with OpenGL.
+
+        This matrix doesn't actually perform the projection; it transforms the
+        space so that OpenGL's vertex processing performs it.
 
         :Parameters:
             `aspect` : The aspect ratio as a `float`
@@ -860,17 +867,6 @@ def from_translation(cls: type[Mat4T], vector: Vec3) -> Mat4T:
                     0.0, 0.0, 1.0, 0.0,
                     vector[0], vector[1], vector[2], 1.0))
 
-    @classmethod
-    def look_at_direction(cls: type[Mat4T], direction: Vec3, up: Vec3) -> Mat4T:
-        vec_z = direction.normalize()
-        vec_x = direction.cross(up).normalize()
-        vec_y = direction.cross(vec_z).normalize()
-
-        return cls((vec_x.x, vec_y.x, vec_z.x, 0.0,
-                    vec_x.y, vec_y.y, vec_z.y, 0.0,
-                    vec_x.z, vec_z.z, vec_z.z, 0.0,
-                    0.0, 0.0, 0.0, 1.0))
-
     @classmethod
     def look_at(cls: type[Mat4T], position: Vec3, target: Vec3, up: Vec3):
         f = (target - position).normalize()
@@ -1015,42 +1011,38 @@ def __matmul__(self, other: Mat4) -> Mat4:
 
     def __matmul__(self, other):
         if isinstance(other, Vec4):
-            # Columns:
-            c0 = self[0::4]
-            c1 = self[1::4]
-            c2 = self[2::4]
-            c3 = self[3::4]
-            return Vec4(sum(map(_mul, c0, other)),
-                        sum(map(_mul, c1, other)),
-                        sum(map(_mul, c2, other)),
-                        sum(map(_mul, c3, other)))
+            # Rows:
+            r0 = self[0::4]
+            r1 = self[1::4]
+            r2 = self[2::4]
+            r3 = self[3::4]
+            return Vec4(sum(map(_mul, r0, other)),
+                        sum(map(_mul, r1, other)),
+                        sum(map(_mul, r2, other)),
+                        sum(map(_mul, r3, other)))
 
         if not isinstance(other, Mat4):
             raise TypeError("Can only multiply with Mat4 or Vec4 types")
         # Rows:
-        r0 = self[0:4]
-        r1 = self[4:8]
-        r2 = self[8:12]
-        r3 = self[12:16]
+        r0 = self[0::4]
+        r1 = self[1::4]
+        r2 = self[2::4]
+        r3 = self[3::4]
         # Columns:
-        c0 = other[0::4]
-        c1 = other[1::4]
-        c2 = other[2::4]
-        c3 = other[3::4]
+        c0 = other[0:4]
+        c1 = other[4:8]
+        c2 = other[8:12]
+        c3 = other[12:16]
 
         # Multiply and sum rows * columns:
-        return Mat4((sum(map(_mul, r0, c0)), sum(map(_mul, r0, c1)), sum(map(_mul, r0, c2)), sum(map(_mul, r0, c3)),
-                     sum(map(_mul, r1, c0)), sum(map(_mul, r1, c1)), sum(map(_mul, r1, c2)), sum(map(_mul, r1, c3)),
-                     sum(map(_mul, r2, c0)), sum(map(_mul, r2, c1)), sum(map(_mul, r2, c2)), sum(map(_mul, r2, c3)),
-                     sum(map(_mul, r3, c0)), sum(map(_mul, r3, c1)), sum(map(_mul, r3, c2)), sum(map(_mul, r3, c3))))
+        return Mat4((sum(map(_mul, c0, r0)), sum(map(_mul, c0, r1)), sum(map(_mul, c0, r2)), sum(map(_mul, c0, r3)),
+                     sum(map(_mul, c1, r0)), sum(map(_mul, c1, r1)), sum(map(_mul, c1, r2)), sum(map(_mul, c1, r3)),
+                     sum(map(_mul, c2, r0)), sum(map(_mul, c2, r1)), sum(map(_mul, c2, r2)), sum(map(_mul, c2, r3)),
+                     sum(map(_mul, c3, r0)), sum(map(_mul, c3, r1)), sum(map(_mul, c3, r2)), sum(map(_mul, c3, r3))))
 
     # def __getitem__(self, item):
     #     row = [slice(0, 4), slice(4, 8), slice(8, 12), slice(12, 16)][item]
     #     return super().__getitem__(row)
 
     def __repr__(self) -> str:
-        array = [str(num)[:12] for num in self]
-        return (f"| {array[0]:>12s} | {array[4]:>12s} | {array[8] :>12s} | {array[12]:>12s} |\n"
-                f"| {array[1]:>12s} | {array[5]:>12s} | {array[9] :>12s} | {array[13]:>12s} |\n"
-                f"| {array[2]:>12s} | {array[6]:>12s} | {array[10]:>12s} | {array[14]:>12s} |\n"
-                f"| {array[3]:>12s} | {array[7]:>12s} | {array[11]:>12s} | {array[15]:>12s} |\n")
+        return f"{self.__class__.__name__}{self[0:4]}\n    {self[4:8]}\n    {self[8:12]}\n    {self[12:16]}"

tests/unit/test_math.py

@@ -1,6 +1,6 @@
 import pytest
 
-from pyglet.math import Mat3, Mat4
+from pyglet.math import Mat3, Mat4, Vec3
 
 
 @pytest.fixture()
@@ -132,3 +132,10 @@ def test_mat4_inversion(mat4):
     assert round(mat4 @ inverse_1, 9) == Mat4()
     assert round(mat4 @ inverse_2, 9) == Mat4()
 
+
+def test_mat3_associative_mul():
+    swap_xy = Mat3((0,1,0, 1,0,0, 0,0,1))
+    scale_x = Mat3((2,0,0, 0,1,0, 0,0,1))
+    v1 = (swap_xy @ scale_x) @ Vec3(0,1,0)
+    v2 = swap_xy @ (scale_x @ Vec3(0,1,0))
+    assert v1 == v2 and abs(v1) != 0