Get world scale

[simonThiele]

Maybe I'm getting it wrong, but why is the hole matrix being decomposed if only the scale part is needed?
If it's not necessary, this PR just uses setFromMatrixScale to extract the scale from worldMatrix. Please check unit test if I missed any situation where decomposing and setFromMatrixScale may differ and please ignore first two commits (the second one reverts the first one) I just forgot to create a branch. Thx.

[simonThiele]

commented code is removed

[arose]

Looks good.

Another thing, looking at the Matrix4.decompose function I wonder if scale is correct when the determinant is below zero or if sx needs to be inverted again before assigning to scale.x?!

// if determine is negative, we need to invert one scale
var det = this.determinant();
if ( det < 0 ) {
    sx = - sx;
}
// ...
scale.x = sx;
scale.y = sy;
scale.z = sz;

[arose]

Maybe you could re-phrase these comments, removing the reference to old and new ways, which are quite specific to this PR.

[simonThiele]

Will do

[simonThiele]

Yeah I was wondering about the same but it should also be implemented in setFromMatrixScale but it's not. Don't know why.

[WestLangley]

Why zero scale?

[simonThiele]

To test all ways of how an object can be scaled. It does not really matter in production code but should be checked what happens if a matrix is empty (other than the 1 on 3,3)

[WestLangley]

three.js does not support zero-determinant transforms. Most of your tests have a zero scale in at least one dimension. We only need to test what is supported.

However, if you would like to improve the library so zero-determinant transforms work, that might be useful. I am not sure if there is a compelling reason to do so, however...

[WestLangley]

why is the [whole] matrix being decomposed if only the scale part is needed?

Your approach can get a different answer in cases like the following:

mesh.scale.set( - 1, 1, 1 );
mesh.updateMatrix();
var vec1 = mesh.getWorldScale();
var vec2 = new THREE.Vector3().setFromMatrixScale( mesh.matrix );

three.js does not officially support reflections in the object matrix, but we have done our best to do something reasonable in the math library.

If anyone wants to try to break the library, that would be helpful, and we can figure out what to do about it.

[WestLangley]

I would not make this change at this point. It can produce a different answer from the current method.

[simonThiele]

It's because the decompose generates a -sx at negative determinant. But why isn't that necessary on setFromMatrixScale? @mrdoob

[simonThiele]

@mrdoob Do you know why vector.setFromMatrixScale doesn't contain the xScale switching ?

var det = this.determinant();
if ( det < 0 ) {
   sx = - sx;
}

[WestLangley]

@simonThiele See #4272 for a discussion of the issues involved.

Also, experiment with this example.

mesh.position.set( 0, 0, 0 );
mesh.rotation.set( 0, 0, 0 );
mesh.scale.set( 1, 1, - 1 );
mesh.updateMatrix();

var p = new THREE.Vector3();
var q = new THREE.Quaternion();
var s = new THREE.Vector3();
mesh.matrix.decompose( p, q, s );
console.log( p );
console.log( q );
console.log( s );

[mrdoob]

I'll wait for @WestLangley approval on this one 😉

[threejsworker]

The examples of this pullrequest are now built and visible in threejsworker. To view them, go to the following link:

http://threejsworker.com/viewpullrequest.html#7116