You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The Quaternion.computeAxis function seems to be questionable in one case, and wrong in another case. When executing the following
const q0 = new Cesium.Quaternion(0,0,0,1);
console.log(Cesium.Quaternion.computeAngle(q0));
console.log(Cesium.Quaternion.computeAxis(q0, new Cesium.Cartesian3()));
const q1 = new Cesium.Quaternion(0,0,0,-1);
console.log(Cesium.Quaternion.computeAngle(q1));
console.log(Cesium.Quaternion.computeAxis(q1, new Cesium.Cartesian3()));
then the output will be
0
(0, 0, 0)
6.283185307179586
(NaN, NaN, NaN)
One could make a case for the first part: The unit quaternion 0,0,0,1 describes "no rotation" - so what is the axis of "no rotation"? It's arbitrary. But I'd be strongly in favor to return a "valid" (unit-length!!!) axis there as well. So it should probably be (1,0,0).
(I wonder how many "normalized result is not a number" errors have been caused by this under the hood...)
But the second output cannot be justified: These NaNs should not be there. One could argue whether it is a rotation about 0° or about 360°, of course, but computeAngle returns 360°, so the rotation axis could just be (1,0,0) as well.
The
Quaternion.computeAxis
function seems to be questionable in one case, and wrong in another case. When executing the followingthen the output will be
One could make a case for the first part: The unit quaternion
0,0,0,1
describes "no rotation" - so what is the axis of "no rotation"? It's arbitrary. But I'd be strongly in favor to return a "valid" (unit-length!!!) axis there as well. So it should probably be(1,0,0)
.(I wonder how many "normalized result is not a number" errors have been caused by this under the hood...)
But the second output cannot be justified: These
NaN
s should not be there. One could argue whether it is a rotation about 0° or about 360°, of course, butcomputeAngle
returns 360°, so the rotation axis could just be(1,0,0)
as well.Both cases boil down to this part of the code, which currently says
If everybody agrees, I could create a PR to change that to
(And update/add the unit tests, which probably do not cover that right now)
Then, the output will be
The following is ... somewhat unrelated, but I noticed this while debugging a bit:
EDIT: Moved to #11685
The text was updated successfully, but these errors were encountered: