From 2ac3327b4cce9581d35d4bd06ac8cf7e39a117eb Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 10 May 2023 00:19:32 +0200 Subject: [PATCH 1/4] Fix ups --- examples/feature_demo/multi_slice1.py | 2 +- pygfx/objects/_base.py | 2 +- pygfx/utils/cube_camera.py | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/examples/feature_demo/multi_slice1.py b/examples/feature_demo/multi_slice1.py index 971dc6961..f035722b2 100644 --- a/examples/feature_demo/multi_slice1.py +++ b/examples/feature_demo/multi_slice1.py @@ -60,7 +60,7 @@ # camera = gfx.PerspectiveCamera(70, 16 / 9) camera = gfx.PerspectiveCamera(0) -camera.up = 0, 0, 1 +camera.local.reference_up = 0, 0, 1 camera.local.position = (200, 200, 200) camera.show_pos((0, 0, 0)) diff --git a/pygfx/objects/_base.py b/pygfx/objects/_base.py index 74dd572a0..99ab31f9d 100644 --- a/pygfx/objects/_base.py +++ b/pygfx/objects/_base.py @@ -196,7 +196,7 @@ def __del__(self): def up(self): """ Relic of old WorldObjects that aliases with the new ``transform.up`` - direction. Prefer (minus) `obj.world.reference_up.` instead + direction. Prefer `obj.world.reference_up` instead. """ diff --git a/pygfx/utils/cube_camera.py b/pygfx/utils/cube_camera.py index 5a936e7c0..6441551d6 100644 --- a/pygfx/utils/cube_camera.py +++ b/pygfx/utils/cube_camera.py @@ -51,32 +51,32 @@ def __init__(self, target, near=0.1, far=1000, blend_mode="default"): # so camrea_px is actually looking at the neg-x direction, and camera_nx is looking at the pos-x direction. camera_px = PerspectiveCamera(fov, aspect, depth_range=depth_range) - camera_px.up = (0, 1, 0) + camera_px.world.reference_up = (0, 1, 0) camera_px.look_at((-1, 0, 0)) self.add(camera_px) camera_nx = PerspectiveCamera(fov, aspect, depth_range=depth_range) - camera_nx.up = (0, 1, 0) + camera_nx.world.reference_up = (0, 1, 0) camera_nx.look_at((1, 0, 0)) self.add(camera_nx) camera_py = PerspectiveCamera(fov, aspect, depth_range=depth_range) - camera_py.up = (0, 0, -1) + camera_py.world.reference_up = (0, 0, -1) camera_py.look_at((0, 1, 0)) self.add(camera_py) camera_ny = PerspectiveCamera(fov, aspect, depth_range=depth_range) - camera_ny.up = (0, 0, 1) + camera_ny.world.reference_up = (0, 0, 1) camera_ny.look_at((0, -1, 0)) self.add(camera_ny) camera_pz = PerspectiveCamera(fov, aspect, depth_range=depth_range) - camera_pz.up = (0, 1, 0) + camera_pz.world.reference_up = (0, 1, 0) camera_pz.look_at((0, 0, 1)) self.add(camera_pz) camera_nz = PerspectiveCamera(fov, aspect, depth_range=depth_range) - camera_nz.up = (0, 1, 0) + camera_nz.world.reference_up = (0, 1, 0) camera_nz.look_at((0, 0, -1)) self.add(camera_nz) From 722168973c0edad1e608c03845ada0e63803968a Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 10 May 2023 21:58:33 +0200 Subject: [PATCH 2/4] Fix that setting ob.world.reference_up is not maintained. Plus some minor fixes --- examples/feature_demo/multi_slice1.py | 2 +- pygfx/utils/transform.py | 44 ++++++++++++++++----------- tests/cameras/test_cameras.py | 29 ++++++++++++++++++ 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/examples/feature_demo/multi_slice1.py b/examples/feature_demo/multi_slice1.py index f035722b2..868d75fa8 100644 --- a/examples/feature_demo/multi_slice1.py +++ b/examples/feature_demo/multi_slice1.py @@ -60,7 +60,7 @@ # camera = gfx.PerspectiveCamera(70, 16 / 9) camera = gfx.PerspectiveCamera(0) -camera.local.reference_up = 0, 0, 1 +camera.world.reference_up = 0, 0, 1 camera.local.position = (200, 200, 200) camera.show_pos((0, 0, 0)) diff --git a/pygfx/utils/transform.py b/pygfx/utils/transform.py index 578847cf1..b1c97d754 100644 --- a/pygfx/utils/transform.py +++ b/pygfx/utils/transform.py @@ -503,7 +503,7 @@ class RecursiveTransform(AffineBase): This transform behaves semantically identical to an ordinary ``AffineTransform`` (same properties), except that users may define a - ``parent`` transform which proceeds the ``matrix`` used by the ordinary + ``parent`` transform which preceeds the ``matrix`` used by the ordinary ``AffineTransform``. The resulting ``RecursiveTransform`` then controls the total transform that results from combinign the two transforms via:: @@ -539,11 +539,11 @@ class RecursiveTransform(AffineBase): parent : AffineBase, optional The parent transform that preceeds the base transform. reference_up : ndarray, [3] - The direction of the reference_up vector - expressed in the target frame. It is the inverse of ``WorldObject.up`` - and used by the axis properties (right, up, forward) to maintain a - common level of rotation around an axis when it is updated by it's - setter. By default, it points along the negative Y-axis. + The direction of the reference_up vector expressed in the target + frame. It is used by the axis properties (right, up, forward) + to maintain a common level of rotation around an axis when it + is updated by it's setter. By default, it points along the + positive Y-axis. is_camera_space : bool If True, the transform represents a camera space which means that it's ``forward`` and ``right`` directions are inverted. @@ -590,30 +590,38 @@ def __init__( self._reference_up = reference_up else: # use given reference_up (and sync own) - self._reference_up = reference_up + self._reference_up = np.asarray(reference_up, dtype=float) own_ref = la.vec_transform(reference_up, self.parent.inverse_matrix) self.own._reference_up = own_ref - self.parent.on_update(self.parent_updated) - self.own.on_update(self.child_updated) + self.parent.on_update(self._parent_updated) + self.own.on_update(self._own_updated) def flag_update(self): self.last_modified = perf_counter_ns() super().flag_update() @callback - def parent_updated(self, parent: AffineBase): - own_ref = la.vec_transform(self.reference_up, parent.inverse_matrix) - parent_origin = la.vec_transform((0, 0, 0), parent.inverse_matrix) - self.own._reference_up = own_ref - parent_origin - + def _parent_updated(self, parent: AffineBase): + self._propagate_reference_up() self.flag_update() + def _propagate_reference_up(self): + new_ref = la.vec_transform(self.reference_up, self.parent.inverse_matrix) + origin = la.vec_transform((0, 0, 0), self.parent.inverse_matrix) + self.own._reference_up = new_ref - origin + @callback - def child_updated(self, own: AffineBase): + def _own_updated(self, own: AffineBase): new_ref = la.vec_transform(own.reference_up, self.parent.matrix) - self._reference_up = new_ref - self.parent.position + origin = self.parent.position + self._reference_up = new_ref - origin + self.flag_update() + @AffineBase.reference_up.setter + def reference_up(self, value): + self._reference_up = np.asarray(value) + self._propagate_reference_up() self.flag_update() @property @@ -623,7 +631,7 @@ def parent(self) -> AffineBase: @parent.setter def parent(self, value): - self.parent.remove_callback(self.parent_updated) + self.parent.remove_callback(self._parent_updated) if value is None: self._parent = AffineTransform() @@ -634,7 +642,7 @@ def parent(self, value): parent_origin = la.vec_transform((0, 0, 0), self._parent.inverse_matrix) self.own._reference_up = own_ref - parent_origin - self.parent.on_update(self.parent_updated) + self.parent.on_update(self._parent_updated) self.flag_update() @cached diff --git a/tests/cameras/test_cameras.py b/tests/cameras/test_cameras.py index d284196ac..b11f4e749 100644 --- a/tests/cameras/test_cameras.py +++ b/tests/cameras/test_cameras.py @@ -79,6 +79,35 @@ def test_camera_show_methods(): assert np.allclose(camera.local.position, [250, 300, 550]) +def test_camera_reference_up(): + # Avoid regressions, see #527 + + camera = gfx.PerspectiveCamera() + + # Check default up + assert tuple(camera.world.reference_up) == (0, 1, 0) + + # Using current up + camera.show_pos((1, 0, 0)) + assert tuple(camera.world.reference_up) == (0, 1, 0) + + # Using an up +Z + camera.show_pos((1, 0, 0), up=(0, 0, 1)) + assert tuple(camera.world.reference_up) == (0, 0, 1) + + # Using an up +X + camera.show_pos((0, 1, 0), up=(1, 0, 0)) + assert tuple(camera.world.reference_up) == (1, 0, 0) + + # Not specifying up, keeps the current up (no revert to default) + camera.show_pos((0, 1, 1)) + assert tuple(camera.world.reference_up) == (1, 0, 0) + + # Back to +Y + camera.show_pos((1, 0, 0), up=(0, 1, 0)) + assert tuple(camera.world.reference_up) == (0, 1, 0) + + def _run_for_camera(camera, near, far, check_halfway): # Some notes: # From 7ca6d91b3994cd529d25f91eb74d74e37737a1d9 Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 10 May 2023 23:37:21 +0200 Subject: [PATCH 3/4] normalize reference_up --- pygfx/utils/transform.py | 24 ++++++++++-------------- tests/objects/test_world_object.py | 16 +++++++++++++--- tests/renderers/test_reactivity.py | 4 ++-- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/pygfx/utils/transform.py b/pygfx/utils/transform.py index b1c97d754..698ff504c 100644 --- a/pygfx/utils/transform.py +++ b/pygfx/utils/transform.py @@ -301,7 +301,7 @@ def scale(self, value): @reference_up.setter def reference_up(self, value): - self._reference_up = np.asarray(value) + self._reference_up = la.vec_normalize(value) self.flag_update() @x.setter @@ -587,12 +587,11 @@ def __init__( if reference_up is None: # use own's reference_up reference_up = la.vec_transform(self.own.reference_up, self.parent.matrix) - self._reference_up = reference_up + self._reference_up = la.vec_normalize(reference_up) else: # use given reference_up (and sync own) - self._reference_up = np.asarray(reference_up, dtype=float) - own_ref = la.vec_transform(reference_up, self.parent.inverse_matrix) - self.own._reference_up = own_ref + self._reference_up = la.vec_normalize(reference_up) + self._propagate_reference_up() self.parent.on_update(self._parent_updated) self.own.on_update(self._own_updated) @@ -607,20 +606,20 @@ def _parent_updated(self, parent: AffineBase): self.flag_update() def _propagate_reference_up(self): - new_ref = la.vec_transform(self.reference_up, self.parent.inverse_matrix) - origin = la.vec_transform((0, 0, 0), self.parent.inverse_matrix) - self.own._reference_up = new_ref - origin + new_ref = la.vec_transform(self._reference_up, self._parent.inverse_matrix) + origin = la.vec_transform((0, 0, 0), self._parent.inverse_matrix) + self.own._reference_up = la.vec_normalize(new_ref - origin) @callback def _own_updated(self, own: AffineBase): new_ref = la.vec_transform(own.reference_up, self.parent.matrix) origin = self.parent.position - self._reference_up = new_ref - origin + self._reference_up = la.vec_normalize(new_ref - origin) self.flag_update() @AffineBase.reference_up.setter def reference_up(self, value): - self._reference_up = np.asarray(value) + self._reference_up = la.vec_normalize(value) self._propagate_reference_up() self.flag_update() @@ -638,10 +637,7 @@ def parent(self, value): else: self._parent = value - own_ref = la.vec_transform(self.reference_up, self._parent.inverse_matrix) - parent_origin = la.vec_transform((0, 0, 0), self._parent.inverse_matrix) - self.own._reference_up = own_ref - parent_origin - + self._propagate_reference_up() self.parent.on_update(self._parent_updated) self.flag_update() diff --git a/tests/objects/test_world_object.py b/tests/objects/test_world_object.py index bb56de45f..135a40c6d 100644 --- a/tests/objects/test_world_object.py +++ b/tests/objects/test_world_object.py @@ -277,6 +277,11 @@ def test_axis_setters(): assert np.allclose(obj.world.up, (0, 1, 0)) assert np.allclose(obj.world.forward, (0, 0, -1)) + # these vectors are unit + + obj.world.forward = (2, 0, 0) + assert np.allclose(obj.world.forward, (1, 0, 0)) + obj.world.up = (1, 1, 1) assert np.allclose(obj.world.up, (1 / np.sqrt(3), 1 / np.sqrt(3), 1 / np.sqrt(3))) @@ -309,14 +314,19 @@ def test_reference_up(): assert np.allclose(group.world.reference_up, (0, 1, 0)) assert np.allclose(obj1.world.reference_up, (0, 1, 0)) - # (world) up_reference is independent between objects + # (world) reference_up is independent between objects obj2 = gfx.WorldObject() obj2.world.rotation = (0, 0, 1, 0) group.add(obj1, keep_world_matrix=True) + obj3 = gfx.WorldObject() + obj1.world.reference_up = (1, 2, 3) obj2.world.reference_up = (1, 0, 1) + obj3.world.reference_up = (0, 42, 0) + isqrt2 = 1 / np.sqrt(2) assert np.allclose(group.local.reference_up, (0, 1, 0)) - assert np.allclose(obj1.world.reference_up, (1, 2, 3)) - assert np.allclose(obj2.world.reference_up, (1, 0, 1)) + assert np.allclose(obj1.world.reference_up, la.vec_normalize((1, 2, 3))) + assert np.allclose(obj2.world.reference_up, (isqrt2, 0, isqrt2)) + assert np.allclose(obj3.world.reference_up, (0, 1, 0)) diff --git a/tests/renderers/test_reactivity.py b/tests/renderers/test_reactivity.py index 559a6c6e5..9a0b750de 100644 --- a/tests/renderers/test_reactivity.py +++ b/tests/renderers/test_reactivity.py @@ -141,7 +141,7 @@ def test_reactivity_mesh3(): tex1 = gfx.cm.cividis tex2 = gfx.cm.inferno cmap3 = np.array([(1,), (0,), (0,), (1,)], np.int32) - tex3 = gfx.Texture(cmap3, dim=1).get_view(filter="linear") + tex3 = gfx.Texture(cmap3, dim=1) # only float32 color map is supported in MeshPhongMaterial for now obj = gfx.Mesh(geometry, gfx.MeshBasicMaterial(map=tex1)) @@ -157,7 +157,7 @@ def test_reactivity_mesh3(): # Change to colormap of different format, need rebuild! obj.material.map = tex3 print("uv", geometry.texcoords.data.shape) - print("map", obj.material.map.view_dim) + print("map", obj.material.map.dim) changed = render(obj) assert changed == {"bindings", "compile_shader", "compose_pipeline"} From f5e425306edb8bea37535a0120802bd7f1739140 Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 10 May 2023 23:40:16 +0200 Subject: [PATCH 4/4] more tests --- tests/cameras/test_cameras.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/cameras/test_cameras.py b/tests/cameras/test_cameras.py index b11f4e749..23321b894 100644 --- a/tests/cameras/test_cameras.py +++ b/tests/cameras/test_cameras.py @@ -107,6 +107,20 @@ def test_camera_reference_up(): camera.show_pos((1, 0, 0), up=(0, 1, 0)) assert tuple(camera.world.reference_up) == (0, 1, 0) + # --- + + # It normalizes too + camera.show_pos((1, 0, 0), up=(0, 0, 2)) + assert tuple(camera.world.reference_up) == (0, 0, 1) + + # Can also set up directly + camera.world.reference_up = (2, 0, 0) + assert tuple(camera.world.reference_up) == (1, 0, 0) + + # Stays that way + camera.show_pos((1, 1, 0)) + assert tuple(camera.world.reference_up) == (1, 0, 0) + def _run_for_camera(camera, near, far, check_halfway): # Some notes: