Repository: pygfx/pygfx
Pull request #565 (2023-07-13 12:02:22)

Propagate scaling signs and handle manually set transforms
I realized we didn't support manually set transform matrices properly, and I figured out how to propagate scaling signs. :)


Patch:
diff --git a/pygfx/utils/transform.py b/pygfx/utils/transform.py
index e6c605a..d79aebf 100644
--- a/pygfx/utils/transform.py
+++ b/pygfx/utils/transform.py
@@ -150,9 +150,19 @@ class AffineBase:
     def _inverse_matrix(self) -> np.ndarray:
         return np.linalg.inv(self.matrix)
 
+    @property
+    def scaling_signs(self):
+        return self._scaling_signs
+
     @cached
     def _decomposed(self) -> Tuple[np.ndarray, np.ndarray, np.ndarray]:
-        return la.mat_decompose(self.matrix, scaling_signs=self._scaling_signs)
+        try:
+            return la.mat_decompose(self.matrix, scaling_signs=self.scaling_signs)
+        except ValueError:
+            # the matrix has been set manually
+            # and so there is no scaling component to preserve
+            # any decomposed scaling is acceptable
+            return la.mat_decompose(self.matrix)
 
     @cached
     def _directions(self):
@@ -663,5 +673,5 @@ class RecursiveTransform(AffineBase):
             return np.asarray(self) @ other
 
     @cached
-    def _decomposed(self) -> Tuple[np.ndarray, np.ndarray, np.ndarray]:
-        return la.mat_decompose(self.matrix)
+    def scaling_signs(self):
+        return self._parent.scaling_signs * self.own.scaling_signs


Test patch:
diff --git a/tests/objects/test_world_object.py b/tests/objects/test_world_object.py
index 606e668..435c451 100644
--- a/tests/objects/test_world_object.py
+++ b/tests/objects/test_world_object.py
@@ -4,6 +4,7 @@ from weakref import ref
 import pylinalg as la
 import numpy as np
 import numpy.testing as npt
+import pytest
 
 from pygfx import WorldObject
 import pygfx as gfx
@@ -394,16 +395,49 @@ def test_bounding_box():
 
 
 def test_scale_preservation():
-    # Add a point that is transformed, to make sure that is taken into account
+    """Test that the original scaling component is preserved through
+    matrix composition roundtrips"""
     ob = gfx.WorldObject()
     s = (1, -2, 3)
     ob.local.scale = s
     # without scale preservation in matrix compose -> decompose roundtrip
     # ob.local.scale becomes (-1, 2, 3)
-    npt.assert_array_equal(ob.local.scale, s)
+    npt.assert_array_almost_equal(ob.local.scale, s)
+
+    child = gfx.WorldObject()
+    ob.add(child)
+    npt.assert_array_almost_equal(child.local.scale, [1, 1, 1])
+    npt.assert_array_almost_equal(child.world.scale, s)
+
+    s2 = (-4, -4, 4)
+    child.local.scale = s2
+    npt.assert_array_almost_equal(child.local.scale, s2)
+    npt.assert_array_almost_equal(child.world.scale, [-4, 8, 12])
+
+
+def test_scaling_signs_manual_matrix():
+    """Test that if the matrix is set directly, everything still works
+    and we do not manage to reconstruct the original signs."""
+    ob = gfx.WorldObject()
+    s = (1, -2, 3)
+    expected = (-1, 2, 3)
+    t = la.mat_compose(
+        (10, 0, 0),
+        la.quat_from_axis_angle((0, 0, 1), np.pi / 2),
+        s,
+    )
+    ob.local.matrix = t
+
+    with pytest.raises(AssertionError):
+        npt.assert_array_almost_equal(ob.local.scale, s)
+    npt.assert_array_almost_equal(ob.local.scale, expected)
 
     child = gfx.WorldObject()
     ob.add(child)
-    npt.assert_array_equal(child.local.scale, [1, 1, 1])
-    # we don't preserve the WORLD scale component
-    npt.assert_array_equal(child.world.scale, [-1, 2, 3])
+    npt.assert_array_almost_equal(child.local.scale, [1, 1, 1])
+    npt.assert_array_almost_equal(child.world.scale, expected)
+
+    s2 = (-4, -4, 4)
+    child.local.scale = s2
+    npt.assert_array_almost_equal(child.local.scale, s2)
+    npt.assert_array_almost_equal(child.world.scale, [-4, 8, 12])