Repository: pygfx/pygfx (python)

Issue:
Pass `scale` vector to `mat_decompose` when a `scale_XXX` setter is called on a transform
This will help preserve the `scale` vector in a roundtrip from components -> matrix -> components.

This should resolve the problem observed in #553

This requires a fix in pylinalg, see https://github.com/pygfx/pylinalg/issues/81

Pull request description:
Preserve scaling factors on transforms
Closes #556

Patch:
diff --git a/pygfx/__init__.py b/pygfx/__init__.py
index bd9ba36..3055b83 100644
--- a/pygfx/__init__.py
+++ b/pygfx/__init__.py
@@ -28,7 +28,7 @@ __version__ = "0.1.13"
 version_info = tuple(map(int, __version__.split(".")))
 
 __wgpu_version_range__ = "0.9.4", "0.10.0"
-__pylinalg_version_range__ = "0.4.0", "0.5.0"
+__pylinalg_version_range__ = "0.4.1", "0.5.0"
 
 
 def _check_lib_version(libname, pipname, version_range):
diff --git a/pygfx/utils/transform.py b/pygfx/utils/transform.py
index 698ff50..e6c605a 100644
--- a/pygfx/utils/transform.py
+++ b/pygfx/utils/transform.py
@@ -135,6 +135,7 @@ class AffineBase:
         self.update_callbacks = {}
         self.is_camera_space = int(is_camera_space)
         self._reference_up = np.asarray(reference_up, dtype=float)
+        self._scaling_signs = np.asarray([1, 1, 1], dtype=float)
 
     @property
     def matrix(self):
@@ -151,7 +152,7 @@ class AffineBase:
 
     @cached
     def _decomposed(self) -> Tuple[np.ndarray, np.ndarray, np.ndarray]:
-        return la.mat_decompose(self.matrix)
+        return la.mat_decompose(self.matrix, scaling_signs=self._scaling_signs)
 
     @cached
     def _directions(self):
@@ -297,7 +298,9 @@ class AffineBase:
 
     @scale.setter
     def scale(self, value):
-        self.matrix = la.mat_compose(self.position, self.rotation, value)
+        m = la.mat_compose(self.position, self.rotation, value)
+        self._scaling_signs = np.sign(value)
+        self.matrix = m
 
     @reference_up.setter
     def reference_up(self, value):
@@ -658,3 +661,7 @@ class RecursiveTransform(AffineBase):
             return RecursiveTransform(other, parent=self)
         else:
             return np.asarray(self) @ other
+
+    @cached
+    def _decomposed(self) -> Tuple[np.ndarray, np.ndarray, np.ndarray]:
+        return la.mat_decompose(self.matrix)


Test patch:
diff --git a/tests/objects/test_world_object.py b/tests/objects/test_world_object.py
index 51527f5..606e668 100644
--- a/tests/objects/test_world_object.py
+++ b/tests/objects/test_world_object.py
@@ -3,6 +3,7 @@ from unittest.mock import Mock, call
 from weakref import ref
 import pylinalg as la
 import numpy as np
+import numpy.testing as npt
 
 from pygfx import WorldObject
 import pygfx as gfx
@@ -390,3 +391,19 @@ def test_bounding_box():
     )
     point_with_children.add(scene)
     assert point_with_children.get_bounding_box().tolist() == [[-5, 0, 0], [9, 3, 6]]
+
+
+def test_scale_preservation():
+    # Add a point that is transformed, to make sure that is taken into account
+    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)
+
+    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])