Loaders: set .quaternion instead of .rotation #13488

WestLangley · 2018-03-03T19:30:45Z

Trying this to see what this change looks like... Not too bad, actually.

The idea is that at some point, Object3d.rotation may be removed, but the Euler class would be retained.

Feedback requested.

WestLangley · 2018-03-03T19:32:38Z

examples/js/loaders/ColladaLoader.js

@@ -3607,7 +3607,7 @@ THREE.ColladaLoader.prototype = {

 		if ( asset.upAxis === 'Z_UP' ) {

-			scene.rotation.x = - Math.PI / 2;
+			scene.quaternion.setFromEuler( new THREE.Euler( - Math.PI / 2, 0, 0 ) );


@Mugen87 I think new is OK here...

Looks good 👍

WestLangley · 2018-03-03T19:33:46Z

examples/js/loaders/FBXLoader.js

@@ -1989,7 +1989,7 @@

 			var rotation = modelNode.Lcl_Rotation.value.map( THREE.Math.degToRad );
 			rotation.push( 'ZYX' );
-			model.rotation.fromArray( rotation );
+			model.quaternion.setFromEuler( new THREE.Euler().fromArray( rotation ) );


@takahirox @looeee I think new is OK here, too.

Yep, that looks fine 👍

WestLangley · 2018-03-03T19:34:40Z

examples/js/loaders/FBXLoader.js

-			var currentRotation = new THREE.Quaternion().setFromEuler( model.rotation );
-			preRotations.multiply( currentRotation );
-			model.rotation.setFromQuaternion( preRotations, 'ZYX' );
+			model.quaternion.premultiply( preRotations );


@takahirox @looeee I think this is equivalent to what was intended...

Yep, this looks good too 🙂

WestLangley · 2018-03-03T19:36:45Z

examples/js/loaders/PlayCanvasLoader.js

@@ -140,7 +140,7 @@ THREE.PlayCanvasLoader.prototype = {
 			object.name = data.name;

 			object.position.fromArray( data.position );
-			object.rotation.fromArray( data.rotation );
+			object.quaternion.setFromEuler( new THREE.Euler().fromArray( data.rotation ) );


@Mugen87 Again, instantiating a new instance may be OK...

WestLangley · 2018-03-03T19:40:57Z

examples/js/loaders/sea3d/SEA3DLoader.js

@@ -1695,7 +1695,7 @@ THREE.SEA3D.prototype.updateTransform = function ( obj3d, sea ) {

 	// ignore rotation scale

-	obj3d.rotation.setFromRotationMatrix( THREE.SEA3D.identityMatrixScale( mtx ) );
+	obj3d.quaternion.setFromRotationMatrix( THREE.SEA3D.identityMatrixScale( mtx ) );


/ping @sunag Should be OK as long as the argument is a pure rotation matrix.

/ping @sunag opinion??

/ping @sunag Should be OK as long as the argument is a pure rotation matrix.

Yes, yes! Looks good

WestLangley · 2018-03-03T19:42:40Z

examples/js/loaders/sea3d/physics/SEA3DAmmoLoader.js

@@ -491,7 +491,7 @@ THREE.SEA3D.Domain.prototype.applyTransform = function ( matrix ) {
 		// ignore rotation scale

 		mtx.scale( vec.set( 1 / obj3d.scale.x, 1 / obj3d.scale.y, 1 / obj3d.scale.z ) );
-		obj3d.rotation.setFromRotationMatrix( mtx );
+		obj3d.quaternion.setFromRotationMatrix( mtx );


/ping @sunag Perhaps this block of code can be implemented more-simply with this, instead:

obj3d.applyMatrix( matrix );

Hi @WestLangley
I do not know now but I broke my head a little because quaternion is affected in decomposition matrix if scale is not (1,1,1)
https://github.com/WestLangley/three.js/blob/e865ad729f60fe919373a14073a9486c90a92d9c/examples/js/loaders/sea3d/physics/SEA3DAmmoLoader.js#L493

in AS3(Flash) version for example the decompose matrix scale not influence the quaternion.

looeee · 2018-03-07T03:25:04Z

Object3d.rotation may be removed, but the Euler class would be retained

How would a (beginner friendly) change of rotation look on an Object3D after this change?
Would you need to do do something like:

const euler = new THREE.Euler( 0, 1, 0 );
object.setRotationFromEuler ( euler );

euler.set( Math.PI, 1, 0 );
object.setRotationFromEuler ( euler );

etc. ?

WestLangley · 2018-03-07T03:50:50Z

Yes. Also

object.rotateX( 0.01 );
object.rotateAxisAngle( axis, angle );

Thing is, it is difficult to reproduce the tumbling effect one gets from

object.rotation.x += 0.01;
object.rotation.y += 0.01;

There would be some consequences to removing Object3D.rotation.

looeee · 2018-03-07T03:57:04Z

Thing is, it is difficult to reproduce the tumbling effect...

Yeah, this was my thought as well. That's used in lots of introductory tutorials. In fact this change would render basically every three.js tutorial and book obsolete, which is something we should definitely consider.

WestLangley · 2018-03-07T04:13:18Z

One could use this pattern in the render loop. It produces the same result.

var euler = new THREE.Euler();

euler.x += 0.01;
euler.y += 0.01;
object.setRotationFromEuler( euler );

@mrdoob That method probably should have been called setOrientationFromEuler(). Oh, well...

mrdoob · 2018-03-07T04:21:03Z

@mrdoob That method probably should have been called setOrientationFromEuler(). Oh, well...

Agreed. We can't do everything perfectly all the time... 😆

mrdoob · 2018-03-07T04:22:15Z

Is this good to go then?

WestLangley · 2018-03-07T04:32:25Z

Yes. This can be merged. :)

mrdoob · 2018-03-07T04:34:43Z

Sweet!

mrdoob · 2018-03-07T04:34:57Z

Thanks!

Set .quaternion instead of .rotation

e865ad7

WestLangley commented Mar 3, 2018

View reviewed changes

WestLangley added the Loaders label Mar 3, 2018

mrdoob added this to the r91 milestone Mar 7, 2018

mrdoob merged commit ac3b5be into mrdoob:dev Mar 7, 2018

WestLangley deleted the dev-loaders_quat branch March 7, 2018 04:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Loaders: set .quaternion instead of .rotation #13488

Loaders: set .quaternion instead of .rotation #13488

WestLangley commented Mar 3, 2018 •

edited

Loading

WestLangley Mar 3, 2018

Mugen87 Mar 3, 2018

WestLangley Mar 3, 2018

looeee Mar 4, 2018

WestLangley Mar 3, 2018

looeee Mar 4, 2018

WestLangley Mar 3, 2018

Mugen87 Mar 3, 2018

WestLangley Mar 3, 2018

WestLangley Mar 7, 2018

sunag Mar 7, 2018

WestLangley Mar 3, 2018 •

edited

Loading

sunag Mar 7, 2018

sunag Mar 7, 2018

looeee commented Mar 7, 2018 •

edited

Loading

WestLangley commented Mar 7, 2018

looeee commented Mar 7, 2018

WestLangley commented Mar 7, 2018

mrdoob commented Mar 7, 2018

mrdoob commented Mar 7, 2018

WestLangley commented Mar 7, 2018

mrdoob commented Mar 7, 2018

mrdoob commented Mar 7, 2018

Loaders: set .quaternion instead of .rotation #13488

Loaders: set .quaternion instead of .rotation #13488

Conversation

WestLangley commented Mar 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WestLangley Mar 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

looeee commented Mar 7, 2018 • edited Loading

WestLangley commented Mar 7, 2018

looeee commented Mar 7, 2018

WestLangley commented Mar 7, 2018

mrdoob commented Mar 7, 2018

mrdoob commented Mar 7, 2018

WestLangley commented Mar 7, 2018

mrdoob commented Mar 7, 2018

mrdoob commented Mar 7, 2018

WestLangley commented Mar 3, 2018 •

edited

Loading

WestLangley Mar 3, 2018 •

edited

Loading

looeee commented Mar 7, 2018 •

edited

Loading