SceneUtils: Added up axis conversion support

[Mugen87]

This PR introduces SceneUtils.convertFromZUp() that converts the up axis of a hierarchy of objects and corresponding animations to the three.js standard (Y up). I've developed this in a way, so it is easy to create more conversion methods. You only have to define a conversion matrix and create a new instance of UpAxisConverter. Just have a look at the implementation of SceneUtils. convertFromZUp().

[Mugen87]

@WestLangley What do you think about this approach? I've tried to decouple things by using a matrix that determines the actual type of the conversion (e.g. Z->Y up).

[WestLangley]

Yes, you seem to be on the right track.

This is just a change of basis, right? So you are given a scene and an animation expressed in one basis, and you want to convert the scene and animation to another basis -- the three.js basis.

If it is convenient to assume that conversion is a pure rotation plus, perhaps, a reflection -- and if it simplifies the math -- that is fine, but that assumption is not necessary. I see you are making assumptions when you transform the vertex normals and do not use a normal matrix for that purpose.

It is not sufficient to refer to a basis as "z-up"; the basis may be left- or right-handed. I suggest you use different terminology. Your conversionMatrix is a change-of-basis matrix. convertMatrix4 needs to be renamed, IMO, as does UpAxisConverter. Maybe BasisConverter, ColladaConversionMatrix, etc.

I do not follow your implementation of .convert() and .convertMatrix4() at all. (That does not mean they are wrong.) I do not understand the necessity for calling decompose(). I do see, however, the methods appear to work in your example. Maybe adding comments explaining each step would be helpful.

Related: link.

[Mugen87]

It is not sufficient to refer to a basis as "z-up"; the basis may be left- or right-handed.

Yeah, that's right. The current used matrix ensures that the handedness stays untouched. So it converts a right-handed Z-UP to right-handed Y-UP. To be precise, the matrix swaps z and y of a vector and then inverts the sign of z. This transform can be applied to single vectors or to matrices (via convertMatrix4).

This is just a change of basis, right?

convertMatrix4 changes not only the basis vectors but also the translation part of a 4x4 matrix. I'm not sure about the correct terminology, but i think the conversion is therefore not limited on the basis.

I see you are making assumptions when you transform the vertex normals and do not use a normal matrix for that purpose.

What kind of assumptions do i make here? Do you mean i assume that the translation part of a conversion matrix is always zero?

But yeah, using a 3x3 normal matrix (without translation) is of course more correct.

I do not understand the necessity for calling decompose().

I thought this is necessary so .position, .quaternion and .scale are correct after the conversion. When the model matrix changes, they need an update in order to reflect the new values.

BTW: Thanks for your feedback so far! 😊

[Mugen87]

Maybe adding comments explaining each step would be helpful.

I'll improve this.

[Mugen87]

I'd like to move forward with this PR because i need this logic in ColladaLoader2. The implementation should be correct although i'm not able to explain the math of all steps in detail. I know that the calculations in convertMatrix4() are necessary in order to produce correct results. But i've worked primarily with references like the old collada loader code or this post here.

If the implementation or the terminology of methods and variables is not 100% satisfying, then i have no problem to outsource the code to ColladaLoader2. I agree we should only enhance the core with consistent solutions.

I initially thought that the code might be interesting not only for ColladaLoader2 but also for other loaders that load models or animations with a different coordinate system.

[WestLangley]

I'd like to move forward with this PR because i need this logic in ColladaLoader2.

That's fine

The implementation should be correct although i'm not able to explain the math of all steps in detail.

See my comments -- and the video I linked to above.

[WestLangley]

@mrdoob As an aside, this is a handy utility to convert a scene from one basis to another, but it needs to be called every time the scene is loaded. I expect it would be better to implement the same logic in the exporter, itself.

[Mugen87]

@mrdoob I think this PR is ready to merge now.

@WestLangley Any objections?

[WestLangley]

Any objections?

My only concern is my prior comment about implementing this in the exporter, instead.

[mrdoob]

@Mugen87 How about adding this in the examples folder for now?

Actually, I would vote for adding this to SceneUtils and move SceneUtils to the examples/js folder.

[WestLangley]

Actually, I would vote for adding this to SceneUtils and move SceneUtils to the examples/js folder.

... and remove usage of THREE.SceneUtils.createMultiMaterialObject() from the examples, as it is not needed in most examples.

If there is an example that requires a mesh and its wireframe, use this pattern:

mesh.add( wireframe );