Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
61 changes: 60 additions & 1 deletion src/compas_threejs/viewer/inbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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)
Expand Down
97 changes: 97 additions & 0 deletions tests/test_object_transform_scale.py
Original file line number Diff line number Diff line change
@@ -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()
Loading