Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TransformControls: fix some edge case transforms #27719

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

TransformControls: fix some edge case transforms #27719

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6aa02d4
TransformControls: fix rotated parented attached object snap
AlaricBaraou Feb 10, 2024
1c453e6
TransformControls: fix rotated scene influence on transforms
AlaricBaraou Feb 10, 2024
8bee424
Merge branch 'dev' into transformControls
Mugen87 Mar 21, 2024
4cd38c5
Merge branch 'mrdoob:dev' into transformControls
AlaricBaraou Mar 26, 2024
86ec938
fix transform controls in transformed scene
AlaricBaraou Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 12 additions & 14 deletions examples/jsm/controls/TransformControls.js
View file
Open in desktop
Expand Up @@ -215,7 +215,13 @@ class TransformControls extends Object3D {

}

super.updateMatrixWorld( force );
// TransformControls should ignore any parent matrix
this.matrixAutoUpdate = false;
this.updateMatrix();
this.matrixWorldNeedsUpdate = false;
this.matrixWorld.copy( this.matrix );

super.updateMatrixWorld( false );
Copy link
Collaborator

@Mugen87 Mugen87 Mar 21, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm your PR fixes the issue but the approach isn't ideal, tbh. We need to find a solution without influencing how the world transformation matrix is computed.

If not, this will cause issues when we rethink the update matrix process and lifecycle in three.js and try to find a more optimized implementation. Ideally, components just request a world matrix and do not modify their computation.

In other words, TransformControls has to work with an unmodified version of the scene's transformation hierarchy. Fixing the controls in combination with rotated ancestor nodes has to be done in a way that the rotation is correctly taken into account and not ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this match your requirements ?

If we replace my block

// if TransformControls has a parent, set its local position to the inverse of the parent world in order to have a worldMatrix to identity
if ( this.parent ) {

			this._transformMatrixWorldInv.copy( this.parent.matrixWorld ).invert();
			this._transformMatrixWorldInv.decompose( this.position, this.rotation, this.scale );

}

super.updateMatrixWorld( true );

That way we don't influence how the world transformation matrix is computed, we just set the TransformControls local position.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks indeed better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!
I'll push that commit later today

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated, it should be good now 馃憤


}

Expand Down Expand Up @@ -359,35 +365,27 @@ class TransformControls extends Object3D {

if ( space === 'world' ) {

if ( object.parent ) {

object.position.add( _tempVector.setFromMatrixPosition( object.parent.matrixWorld ) );

}
object.getWorldPosition( this.worldPosition );

if ( axis.search( 'X' ) !== - 1 ) {

object.position.x = Math.round( object.position.x / this.translationSnap ) * this.translationSnap;
this.worldPosition.x = Math.round( this.worldPosition.x / this.translationSnap ) * this.translationSnap;

}

if ( axis.search( 'Y' ) !== - 1 ) {

object.position.y = Math.round( object.position.y / this.translationSnap ) * this.translationSnap;
this.worldPosition.y = Math.round( this.worldPosition.y / this.translationSnap ) * this.translationSnap;

}

if ( axis.search( 'Z' ) !== - 1 ) {

object.position.z = Math.round( object.position.z / this.translationSnap ) * this.translationSnap;
this.worldPosition.z = Math.round( this.worldPosition.z / this.translationSnap ) * this.translationSnap;

}

if ( object.parent ) {

object.position.sub( _tempVector.setFromMatrixPosition( object.parent.matrixWorld ) );

}
object.position.copy( object.parent.worldToLocal( this.worldPosition ) );

}

Expand Down
2 changes: 1 addition & 1 deletion examples/misc_controls_transform.html
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@

<div id="info">
"W" translate | "E" rotate | "R" scale | "+/-" adjust size<br />
"Q" toggle world/local space | "Shift" snap to grid<br />
"Q" toggle world/local space | "Shift" hold to snap to grid<br />
"X" toggle X | "Y" toggle Y | "Z" toggle Z | "Spacebar" toggle enabled<br />
"Esc" reset current transform<br />
"C" toggle camera | "V" random zoom
Expand Down