Calculate I3dm instance rotations by converting basis to a matrix

[timoore]

The previous approach tried to create rotations directly from the up and right vectors that encode the instance rotation, but it naïvely didn't handle rotations of 180 degrees of the up and right vector. It is reliable to create the rotation matrix, filling in the Z column by doing a cross-product, and then let the algorithm for converting a rotation matrix to a quaternion do its robust thing.

Fixes #1046

Note: in Cesium for Unreal, the instances in the sample tileset supplied by the user in CesiumGS/cesium#11477 still have missing geometry. This is most likely due to negative scale factors in the instance transformations; it's not clear if this can be addressed in a practical way.

[kring]

@timoore there's code for turning a 3x3 rotation+scale matrix into a quaternion in Transforms::computeTranslationRotationScaleFromMatrix, and it handles the possibility of a negative scale. The trick would be constructing that rotation+scale matrix, which would require deciding which is applied first, rotation or non-uniform scale? The I3DM spec doesn't seem clear on this to me, but maybe I missed it somewhere.

But also, CesiumJS doesn't have the missing geometry problem you mentioned, right? It just has incorrect normals? So we can at least match CesiumJS behavior, which seems like it would be an improvement.

[timoore]

@timoore there's code for turning a 3x3 rotation+scale matrix into a quaternion in Transforms::computeTranslationRotationScaleFromMatrix, and it handles the possibility of a negative scale. The trick would be constructing that rotation+scale matrix, which would require deciding which is applied first, rotation or non-uniform scale? The I3DM spec doesn't seem clear on this to me, but maybe I missed it somewhere.

We get translation, rotation, and scale separately from I3dm for each instance, and it they are applied in that order. The bug addressed here is specifically in the process of turning orthonormal right, up vectors into a rotation and is not affected by negative scale factors. The new code here does what Transforms::computeTranslationRotationScaleFromMatrix ultimately does too: call glm::quat_cast.

But also, CesiumJS doesn't have the missing geometry problem you mentioned, right? It just has incorrect normals? So we can at least match CesiumJS behavior, which seems like it would be an improvement.

Well, vsgCs doesn't have the bug either when using this PR. I agree that the problem shouldn't be hand-waved away saying that "Unreal doesn't like negative scaling factors," but we already go to a lot of trouble to avoid right-hand coordinate systems in vertex geometry for Unreal, and at first glance it doesn't seem super practical to deal with instance geometry on an instance-by-instance basis in order to handle negative scale factors.

[kring]

Ok, thanks Tim. I don't quite understand what's going wrong in Unreal, though. Unreal doesn't inherently have a problem with negative scale. Even the shenanigans we added to avoid it were added to make physics work correctly; rendering was happy even with the negative scale (see CesiumGS/cesium-unreal#1126). I spent a little bit of time looking at this, though, and it's not obvious to me what the problem is. Presumably our math is wrong somewhere, but I don't know where. It's certainly more complicated in Unreal than elsewhere because of the inversion of the Y coordinate in vertex positions, and the paired matrix that un-does that and effectively cancels out the negative scale in the right-handed to left-handed transformation.

[kring]

I'm going to merge this, because it's a clear improvement even if it doesn't handle negative scales. Thanks @timoore!