Use matrix for ISceneNode rotation

You discovered a bug in the engine, and you are sure that it is not a problem of your code? Just post it in here. Please read the bug posting guidelines first.
Post Reply
pgimeno
Posts: 3
Joined: Thu Dec 20, 2018 11:11 am

Use matrix for ISceneNode rotation

Post by pgimeno »

This is half way between bug report and feature request.

The current way to store a rotation in an ISceneNode is through extrinsic XYZ Euler angles. Euler angles are a horrible way of storing rotations, due to (a) gimbal lock issues, (b) precision loss, and (c) forcing a specific axis convention where Z must be vertical and X must be forward, in order to be able to use the Z angle as yaw, the Y angle as pitch, and the X angle as roll.

Conversions between extrinsic XYZ and any other axis convention are possible, by first converting to matrix form, but they lose precision and the gimbal lock problem becomes more important, as it may appear at rotations that produce gimbal lock in either of the conventions, not just one.

It would be much better if the rotation is stored in either matrix or quaternion form, leaving up to the user rotating them in any desired order if they really want to handle rotations as Euler.

Here's a post by CuteAlien saying that the idea "has been floating around a few times already": http://irrlicht.sourceforge.net/forum// ... 47#p293347

Here's an example of the havoc that has caused in a project that is using Irrlicht with a different axis convention to the one mentioned above: https://github.com/minetest/minetest/issues/7927

A workaround mentioned by CuteAlien is to use IDummyTransformationSceneNode as a parent of the node, which provides a matrix. This involves creating special cases for certain nodes that don't need it, at least in the Minetest sources (see workaround patch here for Minetest as an example: https://gitlab.com/pgimeno/minetest/mer ... 57cd581daa ) and has the additional problem that the scale must be applied to the child node, while the position and rotation must be applied to the IDummyTransformationSceneNode, which, additionally, lacks setPosition/getPosition methods rather than using them to alter the matrix. The reason for applying the scale to the child, is to prevent losing the rotation information when at least one of the scale components is zero, since IDummyTransformationSceneNode does not apply the scale separately. The reason for needing to keep the position in the dummy node instead of the child, is that otherwise the position would rotate as the dummy matrix does.

One of the undesirable consequences of using an intermediate IDummyTransformationSceneNode is that the number of matrix multiplications is raised, as it involves at least one extra 4x4 matrix multiply per node, which could be saved.

This proposal is about changing ISceneNode to make it more flexible, but IDummyTransformationSceneNode also needs some fixes to make it more useful, as outlined above.

There are two independent decisions to make here. One is: matrix or quaternion? Both have pros and cons.

A quaternion stores a rotation and a global scale, therefore protecting against skew. A normalized quaternion only stores a rotation. It's even possible to store three of the four components of a normalized quaternion, and rebuild the fourth when required, by solving for w in w²+x²+y²+z² = 1. It's therefore more compact, but it has a cost in number of calculations. A re-normalization step will be necessary when its norm drifts too much from 1. There's also the issue that the ISceneNode method currently used to perform the rotation returns a matrix4, therefore it should be converted to matrix4 form in getRelativeTransformation() in order to keep compatibility and avoid putting everything upside down.

A matrix uses 16 floats, which is not ideal compared to the 6 required for position and rotation. Scale should be kept separate, for the reasons mentioned earlier. As with quaternions, some precautions should be taken to re-normalize the matrix in case of numeric drifting causing loss of orthogonality or changes in scale. Re-normalization involves two cross products, six multiplications, four sums and possibly two square roots and two divisions. It's more complicated than that of quaternions (no cross products, 4 multiplications, 3 sums, possibly 1 square root and 1 division). One advantage is that it's better known and it's easier to work with for many.

The other decision is whether to still keep the Eulers of setRotation/getRotation and the positions of setPosition/getPosition.

The idea is to maximize compatibility and avoid slowing down existing programs that perform incremental Euler calculations. Currently you have a guarantee that whatever you set with setRotation(), you get back with getRotation(), due to the numbers you pass being the primary source of information. But fully transferring the storage responsibility to either a quaternion or a matrix, would imply that setRotation() and getRotation() would instead perform calculations to transform the matrix to/from Euler form every time they are invoked. This would mean that code that is currently performing incremental rotations by using getRotation(), changing one of the numbers and doing setRotation() again, would be penalized.

One solution is to implement an invalidation strategy, where there would be a field indicating whether the rotation is "dirty" (needs to be recalculated on the next read).
  • When calling setRotation(), the Matrix/Quaternion info would be declared dirty (needs recalculation).
  • When calling setMatrix() (or setQuaternion() depending on the decision above) the Eulers info would be declared dirty.
  • When calling getRotation(), if the Eulers info is dirty it would be recalculated from the matrix.
  • When calling getMatrix() or getQuaternion(), if the matrix/quaternion is dirty it would be recalculated from the Eulers.
One disadvantage is that if the application is holding a pointer to the matrix, Eulers can't be marked dirty when modified. Therefore, it will still involve some manual handling.

Sorry for the long post. The topic is complex and deserved elaborated discussion. I can work on a patch once the decisions above are taken.

(Edited to make the links clickable)
Last edited by pgimeno on Wed Jan 23, 2019 11:23 pm, edited 1 time in total.
pgimeno
Posts: 3
Joined: Thu Dec 20, 2018 11:11 am

Re: Use matrix for ISceneNode rotation

Post by pgimeno »

Thanks. So no need for me to provide a patch? What would be the choices? Matrix or quaternion? Make getRotation always calculate the result from the matrix, or use a Dirty flag to maximize forward compatibility?
devsh
Competition winner
Posts: 2057
Joined: Tue Dec 09, 2008 6:00 pm
Location: UK
Contact:

Re: Use matrix for ISceneNode rotation

Post by devsh »

In my own private fork I will replace all setRotation/getRotation calls on nodes to take quaternions (normalized, no scale) instead of Euler XYZ angles.

In the fork you can already provide whole pre-calculated matrices instead of translation,rotation and scale separately. There's also an atomic-counter based versioning system so you know when and what you need to recompute (matrix or TRS).


Quaternion class should have a `fromEuler` static function or a constructor that would allow you to quickly Ctrl+R port the old stuff to new.
Post Reply