Hello,
I just stumbled over the fact that BABYLON.Quaternion.RotationAxis(axis, ...)
normalizes the input axis in place, even though it is declared as a DeepImmutable<Vector3>
.
Example:
- The thin tube, which uses the original axis, is quite long.
- The thick tube, which uses the axis after a call to
RotationAxis(...)
only has length 1. The length should be unchanged.
The problem is apparently caused by this line and the fact that axis.normalize()
actually normalizes in place.
Since there is a lot of code relying on the fact that .normalize()
modifies the vector in place, we cannot fix the problem by changing that method. We rather have to change BABYLON.Quaternion.RotationAxis(...)
or actually BABYLON.Quaternion.RotationAxisToRef(...)
.
Here is my suggestion:
/**
* Creates a rotation around an axis and stores it into the given quaternion
* Example Playground https://playground.babylonjs.com/#L49EJ7#73
* @param axis defines the axis to use
* @param angle defines the angle to use
* @param result defines the target quaternion
* @returns the target quaternion
*/
public static RotationAxisToRef<T extends Quaternion>(axis: DeepImmutable<Vector3>, angle: number, result: T): T {
result._w = Math.cos(angle / 2);
const sinByLength = Math.sin(angle / 2) / axis.length();
result._x = axis._x * sinByLength;
result._y = axis._y * sinByLength;
result._z = axis._z * sinByLength;
result._isDirty = true;
return result;
}
I’ve had a look at the 16 occurrences of .normalize()
in the source file. Many of them are applied to MathTmp
vectors, which should be harmless. Other cases are apparently intended. But these cases look buggy as well:
- This line in
RotationQuaternionFromAxisToRef(...)
- This line in
Matrix.RotationAxisToRef(...)
- This line in
ReflectionToRef(...)
Regards,
Heribert