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

Math: Added Vector3.setFromEuler(); removed Euler.toVector3() #23494

Merged
merged 1 commit into from
Feb 15, 2022

Conversation

WestLangley
Copy link
Collaborator

@WestLangley WestLangley commented Feb 14, 2022

This new method parallels the existing Vector3.setFrom*() methods.

Similarly, Euler.setFromVector3().

We also avoid "optional targets" in core. This should have been fixed previously; it was just missed.

link.rotation
.toVector3( _vector )
.max( rotationMin ) );
link.rotation.setFromVector3( _vector.setFromEuler( link.rotation ).max( rotationMin ) );
Copy link
Collaborator Author

@WestLangley WestLangley Feb 14, 2022

Choose a reason for hiding this comment

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

/ping @takahirox

  1. There does not appear to be a test case for use of rotationMin/Max. Personally, I do not think it makes sense to clamp Euler angles anyway -- except in very special cases.

  2. Also unrelated to this PR, I am not sure you want to call updateMatrixWorld() a few lines later. Maybe use updateWorldMatrix(), where you can control recursion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for pointing out. Let me take a look closer later...

@@ -403,7 +403,7 @@ function SidebarObject( editor ) {
}

const newRotation = new THREE.Euler( objectRotationX.getValue() * THREE.MathUtils.DEG2RAD, objectRotationY.getValue() * THREE.MathUtils.DEG2RAD, objectRotationZ.getValue() * THREE.MathUtils.DEG2RAD );
if ( object.rotation.toVector3().distanceTo( newRotation.toVector3() ) >= 0.01 ) {
if ( new THREE.Vector3().setFromEuler( object.rotation ).distanceTo( new THREE.Vector3().setFromEuler( newRotation ) ) >= 0.01 ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unrelated to this PR, this code block instantiates new object frequently. At least it is now more honest about it, and is no worse than before.

@WestLangley WestLangley added this to the r138 milestone Feb 15, 2022
@mrdoob mrdoob merged commit 02cf0df into mrdoob:dev Feb 15, 2022
@mrdoob
Copy link
Owner

mrdoob commented Feb 15, 2022

Thanks!

@WestLangley WestLangley deleted the dev-setFromEuler branch February 15, 2022 21:49
@Mugen87 Mugen87 mentioned this pull request Feb 17, 2022
19 tasks
@0b5vr 0b5vr mentioned this pull request Mar 1, 2022
16 tasks
0b5vr added a commit to 0b5vr/three-ts-types that referenced this pull request Mar 1, 2022
donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
@jynxio jynxio mentioned this pull request Oct 18, 2022
Mugen87 pushed a commit that referenced this pull request Oct 18, 2022
## Description
Remove the description of toVector3 from Euler's Chinese document.

## Reason
Euler's toVector3 method has been removed (#23494), but the Chinese document still retains the description of the API.
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.

4 participants