diff --git a/CHANGELOG.md b/CHANGELOG.md index 70f87d5..29dac86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed the spinner getting stuck after a WebSocket reconnect: spinner messages are now persisted (under a stable `obj_id="spinner"`) like other viewer state, so a reconnecting client sees the current spinner state instead of missing a "stop" it never received. - Fixed large model uploads silently dropping the WebSocket connection: the server's `ws_max_size` is raised from uvicorn's 16 MiB default to 256 MiB. +- Fixed scale-mode `object_transform` on `Shape` geometry (Box, Sphere, ...) moving the object's pivot away from where the gizmo showed it: the full delta is now applied to `frame.point`, only the rotation to the frame axes, and the resize goes through `Shape.scale()`. ### Removed diff --git a/src/compas_threejs/viewer/inbox.py b/src/compas_threejs/viewer/inbox.py index 4502971..a051fbd 100644 --- a/src/compas_threejs/viewer/inbox.py +++ b/src/compas_threejs/viewer/inbox.py @@ -13,6 +13,65 @@ console = Console() +_SCALE_TOLERANCE = 1e-9 + + +def _apply_transform_with_scale(geometry, transformation) -> None: + """Applies `transformation` to `geometry`, resizing it too if the transformation + carries a scale component (e.g. a TransformControls "scale" gizmo drag). + + `Shape.transform` (the base class for Box/Sphere/...) only ever applies to the + shape's frame, and explicitly does not support scale - see its own docstring: + "only (combinations of) translations and rotations are supported. To scale a + shape, use the Shape.scale method." A scale-mode gizmo drag produces exactly the + kind of delta transform that docstring warns about, so passing it straight to + `.transform()` silently drops the resize entirely (confirmed empirically: + `Box.xsize` never changes). + """ + scale, _shear, rotation, translation, _projection = transformation.decomposed() + sx, sy, sz = scale.matrix[0][0], scale.matrix[1][1], scale.matrix[2][2] + has_scale = not (abs(sx - 1.0) < _SCALE_TOLERANCE and abs(sy - 1.0) < _SCALE_TOLERANCE and abs(sz - 1.0) < _SCALE_TOLERANCE) + + if not has_scale: + # No resize at all - Shape.transform (== frame.transform) already does + # exactly the right thing here: moves the point, rotates the axes. + geometry.transform(translation * rotation) + return + + if not hasattr(geometry, "frame"): + # Not a Shape (e.g. a Point/Mesh/Polyline, transformed via its own raw + # coordinates) - unlike Shape, these support a general affine `.transform()` + # including scale directly (Shape is the one deliberate exception - see its + # own docstring above), so the full delta can just be applied as-is. + geometry.transform(transformation) + return + + # A Shape (Box, Sphere, ...) being resized: frame.point needs the *full* delta + # (translation AND scale) applied directly - not `Shape.transform()` (drops scale + # entirely, see above), and not just the rigid translation*rotation part either. + # A scale-mode gizmo drag keeps the object's own pivot fixed and only changes its + # scale factor, so relative to the object's own drag-start frame this delta is a + # "scale about that original point" transform (in 1D: T(pos*(1-k)) * S(k)) - + # applying only the rigid part to the point recovers pos*(2-k), not pos: a visible + # jump to the wrong location, not the pivot-preserving resize the gizmo showed. + # Applying the full delta to the point is the correct way to relocate a point + # under a pivot-relative scale; the frame's own axes only ever need the rotation + # part (a Vector's `.transformed` already ignores translation, and has no "size" + # for scale to distort). + geometry.frame.point = geometry.frame.point.transformed(transformation) + geometry.frame.xaxis = geometry.frame.xaxis.transformed(rotation) + geometry.frame.yaxis = geometry.frame.yaxis.transformed(rotation) + + try: + geometry.scale(sx, sy, sz) + except TypeError: + # Uniform-scale-only geometry (e.g. Sphere.scale(factor)) - average the three + # factors rather than silently dropping a non-uniform resize on the floor. + geometry.scale((sx + sy + sz) / 3.0) + except NotImplementedError: + console.log(f"[yellow]{type(geometry).__name__} does not support scaling - resize from this drag was dropped[/yellow]") + + # Maps a frontend-creatable type name to its COMPAS constructor and the numeric # parameter names a "create_geometry" message is allowed to set on it. _CREATABLE_TYPES = { @@ -193,7 +252,7 @@ def _handle_object_transform(self, message, outbox, workspace_id): transformation = Transformation.from_matrix(matrix) with self.lock: - geometry.transform(transformation) + _apply_transform_with_scale(geometry, transformation) if self.app is not None: self.app.get_workspace(workspace_id).update_geometry(geometry) diff --git a/tests/test_object_transform_scale.py b/tests/test_object_transform_scale.py new file mode 100644 index 0000000..6fe53a5 --- /dev/null +++ b/tests/test_object_transform_scale.py @@ -0,0 +1,97 @@ +import unittest + +from compas.geometry import Box +from compas.geometry import Frame +from compas.geometry import Point +from compas.geometry import Rotation +from compas.geometry import Scale +from compas.geometry import Sphere +from compas.geometry import Translation + +from compas_threejs.viewer.inbox import _apply_transform_with_scale + + +class ObjectTransformScaleTest(unittest.TestCase): + def test_box_translate_and_rotate_only_matches_plain_transform(self): + # No scale component - should behave exactly like geometry.transform(delta), + # the pre-fix (and still correct, for this case) behavior. + box = Box(xsize=1, ysize=2, zsize=3, frame=Frame(Point(0, 0, 0), [1, 0, 0], [0, 1, 0])) + delta = Translation.from_vector([1, 2, 3]) * Rotation.from_axis_and_angle([0, 0, 1], 0.4) + + reference = box.copy() + reference.transform(delta) + + _apply_transform_with_scale(box, delta) + + self.assertAlmostEqual(box.frame.point.x, reference.frame.point.x) + self.assertAlmostEqual(box.frame.point.y, reference.frame.point.y) + self.assertAlmostEqual(box.frame.point.z, reference.frame.point.z) + self.assertEqual((box.xsize, box.ysize, box.zsize), (1, 2, 3)) + + def test_box_scale_resizes_and_recenters(self): + # Box.transform() alone silently drops any scale component (it only applies + # to the frame - see its own docstring) - this is the bug being fixed. + box = Box(xsize=1, ysize=1, zsize=1, frame=Frame(Point(0, 0, 0.5), [1, 0, 0], [0, 1, 0])) + delta = Translation.from_vector([1, 0, 0]) * Scale.from_factors([2, 1, 1]) + + _apply_transform_with_scale(box, delta) + + self.assertEqual((box.xsize, box.ysize, box.zsize), (2.0, 1.0, 1.0)) + self.assertAlmostEqual(box.frame.point.x, 1.0) + self.assertAlmostEqual(box.frame.point.z, 0.5) + + def test_scale_drag_keeps_the_gizmo_pivot_fixed_not_shifted(self): + # Reproduces the reported "box jumps to another location after scaling" bug. + # A TransformControls scale-mode drag keeps the object's OWN pivot fixed and + # only changes its scale factor (confirmed against three.js's own source) - + # relative to the object's drag-start frame, that delta is a "scale about the + # object's original center" transform, which for a center at x=pos and factor + # k is T(pos*(1-k)) * S(k) in 1D. Applying only the rigid (translation * + # rotation) part of that to frame.point - the pre-fix behavior - recovers + # pos*(2-k), not pos: for pos=0.36, k=1.6667, that's 0.12, a full grid unit + # (0.24) away from the correct, gizmo-matching 0.36. The earlier version of + # this test suite never caught this because it only ever used a starting + # frame.point at x=0, where the wrong formula and the right one coincide. + pos = 0.36 + k = 1.6667 + delta = Translation.from_vector([pos * (1 - k), 0, 0]) * Scale.from_factors([k, 1, 1]) + box = Box(xsize=0.72, ysize=1, zsize=1, frame=Frame(Point(pos, 0, 0), [1, 0, 0], [0, 1, 0])) + + _apply_transform_with_scale(box, delta) + + self.assertAlmostEqual(box.frame.point.x, pos) + self.assertAlmostEqual(box.xsize, 0.72 * k) + + def test_non_uniform_scale_on_uniform_only_shape_falls_back_to_average(self): + # Sphere.scale(factor) only takes one factor - a non-uniform delta (e.g. a + # per-axis TransformControls scale drag) can't be applied exactly, so this + # averages the three factors rather than crashing or dropping the resize. + sphere = Sphere(radius=1, frame=Frame(Point(0, 0, 0))) + + _apply_transform_with_scale(sphere, Scale.from_factors([2, 3, 4])) + + self.assertAlmostEqual(sphere.radius, 3.0) + + def test_scale_on_a_frameless_geometry_uses_the_full_delta_directly(self): + # A Point (or Mesh/Polyline) has no .frame at all - unlike Shape, its own + # .transform() isn't scale-blind, so the full delta can go straight through. + point = Point(1, 2, 3) + + _apply_transform_with_scale(point, Scale.from_factors([2, 3, 4])) + + self.assertAlmostEqual(point.x, 2.0) + self.assertAlmostEqual(point.y, 6.0) + self.assertAlmostEqual(point.z, 12.0) + + def test_identity_scale_is_a_no_op_and_does_not_call_scale(self): + box = Box(xsize=1, ysize=1, zsize=1, frame=Frame(Point(0, 0, 0), [1, 0, 0], [0, 1, 0])) + called = [] + box.scale = lambda *a: called.append(a) # would fail loudly if invoked wrongly + + _apply_transform_with_scale(box, Translation.from_vector([1, 0, 0])) + + self.assertEqual(called, []) + + +if __name__ == "__main__": + unittest.main()