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.

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

Already on GitHub? Sign in to your account

Copy rotation.order when copying Object3D #19789

Merged
merged 1 commit into from Jul 4, 2020
Merged

Copy rotation.order when copying Object3D #19789

merged 1 commit into from Jul 4, 2020

Conversation

luisfonsivevo
Copy link
Contributor

Not sure if there is a technical reason behind why the euler rotation order isn't cloned by Object3D.copy(), but hey now it is.

Not sure if there is a technical reason behind why the euler rotation order isn't cloned by Object3D.copy(), but hey now it is.
@luisfonsivevo luisfonsivevo changed the title clone euler rotation order when cloning/copyng object3d clone euler rotation order when cloning/copying object3d Jul 3, 2020
Copy link
Contributor

@trusktr trusktr left a comment

Choose a reason for hiding this comment

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

There should instead be a line

this.rotation.copy(source.rotation)

that handles it.

@luisfonsivevo
Copy link
Contributor Author

If we copy the entire euler instead of only copying its order, it won't be necessary to copy the quaternion anymore. Because inside Euler.copy(), the onChangeCallback is called which automatically recalculates the quaternion. So really, it's only necessary to copy either the quaternion or the euler.

This (what I have currently)

this.position.copy( source.position );
this.rotation.order = source.rotation.order;
this.quaternion.copy( source.quaternion );
this.scale.copy( source.scale );

or this (a bit more concise)

this.position.copy( source.position );
this.rotation.copy( source.rotation );
this.scale.copy( source.scale );

will both get the job done.

@WestLangley WestLangley added this to the r119 milestone Jul 4, 2020
@WestLangley
Copy link
Collaborator

  1. three.js is quaternion-based; the Euler angles are provided for user-convenience.

  2. Quaternions are not unique -- and neither are Euler angles. Copying object.rotation will result in identical Euler angles, but possibly different quaternions.

That is why the quaternion is copied, and not the rotation.

Failing to copy rotation.order was an oversight.

This PR, as written, is correct.

@luisfonsivevo
Copy link
Contributor Author

When you say quaternions are not unique, do you mean that two quaternions with different xyzw values can represent the same orientation? I never knew that.

@mrdoob mrdoob merged commit d6fcf4a into mrdoob:dev Jul 4, 2020
@mrdoob
Copy link
Owner

mrdoob commented Jul 4, 2020

Thanks!

@WestLangley
Copy link
Collaborator

@luisfonsivevo Unit quaternions q( x, y, z, w ) and q( - x, - y, - z, - w ) represent the same orientations.

@WestLangley WestLangley changed the title clone euler rotation order when cloning/copying object3d Copy rotation.order when copying Object3D Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants