Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Animated Cube is shown wrong in glTF viewers #74

Open
ToolTech opened this issue Dec 1, 2023 · 26 comments
Open

Animated Cube is shown wrong in glTF viewers #74

ToolTech opened this issue Dec 1, 2023 · 26 comments

Comments

@ToolTech
Copy link

ToolTech commented Dec 1, 2023

the animated cube has two keyframes for rotation of the small box.
First keyframe has quat {0,0,0,-1} wich has an argument of 180 degrees
Second keyfram has quat {1,0,0,+} (a very small w) wich has an argument of 90 degrees (180 degrees rotation around [1,0,0] )

All slerps between quat 1 and quat 2 will have arguments from 180 down to 90 around x axis wich is a backward rotation around the x axis. All viewers I have found shows a clockwise rotation around x axis and that I dont understand

@javagl javagl transferred this issue from KhronosGroup/glTF-Sample-Models Dec 2, 2023
@javagl
Copy link
Contributor

javagl commented Dec 2, 2023

(The glTF-Sample-Models repo is being archived and replaced by glTF-Sample-Assets, so new issues should be opened there. I just moved this one, just as a heads-up)

And just to make sure: You're talking about https://github.com/KhronosGroup/glTF-Sample-Assets/tree/main/Models/AnimatedCube ?

For me, this looks like a counter-clockwise rotation around the y-axis (also, there are three keyframes in ths model, so I'm not sure what this refers to....). (Beyond that, I'd have to re-read the slerp interpolation rules for quaternions, but I just want to make sure that this is the right model)

@ToolTech
Copy link
Author

ToolTech commented Dec 2, 2023 via email

@javagl
Copy link
Contributor

javagl commented Dec 2, 2023

So the model is https://github.com/KhronosGroup/glTF-Sample-Assets/tree/main/Models/BoxAnimated

The keyframes for the rotation of the smaller cube rectangular cuboid are

  // Accessor 7 of 10
  const accessor7 = document.createAccessor('');
  accessor7.setType('VEC4');
  accessor7.setBuffer(buffer0);
  accessor7.setArray(new Float32Array([
    0, 0, 0, -1, 
    1, 0, 0, 4.4896593387466766e-11
  ]));

The first one is (0,0,0,-1), which is "a rotation about 360° around the x-axis". This is indeed a bit odd. It should just be (0,0,0,1), and some tools/libraries might choke on that, but it should be a valid way of describing "no rotation".

The second one is (1,0,0,~0), which is a rotation about ~180° degrees around the x-axis. Why it has this epsilon-value and not just 0.0 is not clear, but ... that's the way it is, and it should be valid as well.

Combining that should yield a clockwise rotation around the x-axis (considering that the positive x-axis points right from the viewer perslective, but left from the perspective of the model, and that's what is shown in the screenshot and all viewers that I've seen so far.

@ToolTech
Copy link
Author

ToolTech commented Dec 2, 2023 via email

@javagl
Copy link
Contributor

javagl commented Dec 2, 2023

I don't really understand what the question is. If the confusion is about the direction of the rotation: The left one uses (0,0,0,1) as the first keyframe, and the right one uses (0,0,0,-1) as the first keyframe:

Khronos Box Rotation

Maybe someone wants to go though the math at https://en.wikipedia.org/wiki/Slerp#Quaternion_slerp and https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#interpolation-slerp to explain the details...

@ToolTech
Copy link
Author

ToolTech commented Dec 3, 2023 via email

@javagl
Copy link
Contributor

javagl commented Dec 3, 2023

Considering the right side of the animated GIF:
It uses (0,0,0,-1) as the first quaternion. This is a rotation about 360 degrees.
It uses (1,0,0,0) as the second quaternion. This is a rotation about 180 degrees.
The animation interpolates between the two. The interpolated values should be something like

[0.0, 0.0, 0.0, -1.0] with 360.0 degrees
[0.1736482, 0.0, 0.0, -0.9848078] with 340.0 degrees
[0.34202015, 0.0, 0.0, -0.9396926] with 320.0 degrees
[0.5, 0.0, 0.0, -0.8660254] with 300.0 degrees
[0.64278764, 0.0, 0.0, -0.76604444] with 280.0 degrees
[0.7660445, 0.0, 0.0, -0.6427876] with 260.0 degrees
[0.86602545, 0.0, 0.0, -0.5] with 240.0 degrees
[0.9396927, 0.0, 0.0, -0.34202012] with 220.0 degrees
[0.9848078, 0.0, 0.0, -0.17364818] with 200.0 degrees
[1.0, 0.0, 0.0, 0.0] with 180.0 degrees

(just printed with my local implementation)

This is a counterclockwise rotation. And the animation in the image looks like a proper counterclockwise rotation for me.
(Note that the x-axis points right in the image (left from the perspective of the model))

However: The model itself is (valid, and therefore) not "right" or "wrong". If you think that viewers are displaying it wrong, then ... you might consider filing issue reports in the respective glTF viewer repositories.

@ToolTech
Copy link
Author

ToolTech commented Dec 4, 2023

Ok. Thanx. Then we agree that this sample should perform like your animated gifs shows. I will go to the glTF sample viewer repo and file the bug there. Thanx a lot !

@javagl
Copy link
Contributor

javagl commented Dec 5, 2023

If you refer to the https://github.khronos.org/glTF-Sample-Viewer-Release/, then this seems to already behave as it is shown in the image (and so do all other viewers that I've looked at via https://github.com/cx20/gltf-test ... )

@ToolTech
Copy link
Author

ToolTech commented Dec 5, 2023 via email

@javagl
Copy link
Contributor

javagl commented Dec 5, 2023

I might need some support/confirmation here as well.

  1. Referring to the specification

@lexaknyazev The formula at https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#interpolation-slerp talks about the sign of the dot product, which is not well defined when the dot product is zero.

In implementations, this usually boils down to something like if (dot < 0) someResult = -someResult, which more or less graciously handles the 0-case, but still leaves another point open:

  1. Referring to the model:

As I said above, the model uses the quaternions

  accessor7.setArray(new Float32Array([
    0, 0, 0, -1, 
    1, 0, 0, 4.4896593387466766e-11
  ]));

where the second one is... a bit unfortunate. Whatever that last component is: It is prone to slipping through some if (someValue < epsilon) check here and there, and therefore, is either completely useless (and distracting), or (depending on whether the implementation sets epsilon=1e-11 or epsilon=1e-12) exposes differences on the implementation level that (in a perfect world) shouldn't be there.

How should we deal with that?


(As such, this is related to #26 - I think that such a sample model should not have values like the given quaternion here, or keyframe time values like [0, 1.25, 2.5, 3.708329916000366 ]. The last one should be 3.70 or better 3.75, just for "regularity", so that one could confidently mention these values in the README if necessary....)

@emackey
Copy link
Member

emackey commented May 29, 2024

The problem is the interpretation of the first quat.  In my world its a 360 deg rotation and the second quat is a 180 deg rotation.  All rotations between them goes from 360 down to 180 which is a counter clockwise rotation.  But in ref viewers like gltf viever its shown as clockwise from 0 to 180But whats is right according to spec ?

I think this is a misunderstanding of how Slerp works for quaternions. Slerp always takes the path that it finds shortest. For example if you have a quaternion that starts at "360 degrees" and ends at "85 degrees", you won't pass through "270 degrees" along the way at all. Slerp would choose the +85 movement over the -275 movement easily. So instead you'll find it goes 0 to 85, counter-clockwise, which might not be what you expect if you were thinking about the original angles involved. You can't think about quaternions as if they were Euclidian angles with degrees like this, because that's not what's going on here. Slerp is a particular algorithm for interpolating between quaterions without regard for honoring particular numbers of degrees or Euclidean angle consistency.

I think this issue can be closed.

@ToolTech
Copy link
Author

ToolTech commented May 29, 2024 via email

@javagl
Copy link
Contributor

javagl commented May 30, 2024

@emackey This thread was inactive for a while, but if I remember the context here correctly, then this was indeed related to the "shortest path" to some extent. But it was a corner case: As mentioned above, the rotations in this sample are

    0, 0, 0, -1, 
    1, 0, 0, 4.4896593387466766e-11

If the second one was 1, 0, 0, 0, then it would raise the question about what the "shortest path" should be. And the specification does not answer this, because this case leads into a 'division by zero' here (which is an issue on its own, IMHO)

But now, there's this 4.4896593387466766e-11, which is prone to be cut off by some epislon-check. So eventually, the rotation direction (i.e. the "shortest path") may depend on 1. some 'epsilon' that is used somewhere, and 2. the 'default' direction that is assumed for the 'shortest path' when rotating from 0 to 180...

@javagl
Copy link
Contributor

javagl commented May 30, 2024

One could make a case for closing this issue and moving the specific point of

The formula at https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#interpolation-slerp talks about the sign of the dot product, which is not well defined when the dot product is zero.

to the glTF repo. This should also cover the "default rotation direction" for the case of a 180° rotation.

@ToolTech
Copy link
Author

Ok. Perhaps i should drop this then but my point is that there is a math formula for quaternion slerp using quaternion math and not some special code for glTF. https://en.wikipedia.org/wiki/Slerp

That formula is exact and give no strange behaviour using our values. The two quats are identified as one rotation 360 degrees and one rotation 180 degrees. And the math defined slerp of slerp value 0.5 is a rotation of 270 degrees. So basically you are not able to define a rotation between two quats with 0 and 360 degrees rotation in glTF. That is glTF has a own special interpretation of quaternions and quaternion slerp

@emackey
Copy link
Member

emackey commented May 30, 2024

Indeed, there is no special slerp for glTF. Have you tried running the numbers?

Here's a version in Cesium, because I was able to hack it up quickly in their Sandcastle tool: Live demo

Code:

// Settings
var startQ = new Cesium.Quaternion(0, 0, 0, -1);
var endQ = new Cesium.Quaternion(1, 0, 0, 4.4896593387466766e-11);
var vector = Cesium.Cartesian3.UNIT_Y;
var numStepsMinusOne = 8;

// Scratch variables
var resultQ = new Cesium.Quaternion();
var resultMat3 = new Cesium.Matrix3();
var resultVec3 = new Cesium.Cartesian3();

for (var i = 0; i <= numStepsMinusOne; ++i) {
  // Slerp from startQ to endQ
  Cesium.Quaternion.slerp(startQ, endQ, i / numStepsMinusOne, resultQ);

  // Convert result to mat3
  Cesium.Matrix3.fromQuaternion(resultQ, resultMat3);

  // Apply to a vector (+Y)
  Cesium.Matrix3.multiplyByVector(resultMat3, vector, resultVec3);
  
  var msg = "Step " + i + " (" + resultVec3.x + ", " + resultVec3.y +
      ", " + resultVec3.z + ")";
  
  console.log(msg);
  document.getElementById('cesiumContainer').innerHTML += msg + "<br/>";
}

Output:

Step 0 (0, 1, 0)
Step 1 (0, 0.9238795325155821, 0.38268343235472)
Step 2 (0, 0.7071067812024208, 0.7071067811706742)
Step 3 (0, 0.38268343239619895, 0.9238795324984007)
Step 4 (0, 4.489658644857286e-11, 1)
Step 5 (0, -0.382683432313241, 0.9238795325327631)
Step 6 (0, -0.7071067811389274, 0.7071067812341675)
Step 7 (0, -0.9238795324812198, 0.3826834324376781)
Step 8 (0, -1, 8.979318677493353e-11)

You can see a +Y vector rotate through +Z (not -Z) on its way to -Y. This is counter-clockwise rotation about +X, using a standard slerp implementation. There's no glTF magic here, it's how the rotation normally goes.

Note that in the endQ variable, the sign of the epsilon is critical! Having endQ.w be positive makes the rotation through +Z be the shorter path, and if you switch endQ.w to be a negative epsilon, the rotation will go clockwise through -Z instead, as that becomes the shorter path in that case. That epsilon is playing a vital role here, selecting the rotational direction.

In general, one should never make a complete 180 rotation between adjacent quaternion keyframes, as one can get into a situation where there's no clear "shorter" path (clockwise vs counter) to get from start to end. This sample model narrowly (extremely narrowly) avoids the problem with the epsilon in endQ.w there.

@ToolTech
Copy link
Author

ToolTech commented May 30, 2024 via email

@javagl
Copy link
Contributor

javagl commented May 30, 2024

Note that in the endQ variable, the sign of the epsilon is critical! [...] That epsilon is playing a vital role here, selecting the rotational direction.

In general, one should never make a complete 180 rotation between adjacent quaternion keyframes, as one can get into a situation where there's no clear "shorter" path (clockwise vs counter) to get from start to end. This sample model narrowly (extremely narrowly) avoids the problem with the epsilon in endQ.w there.

I see two issues here:

  1. This ambiguity (basically by the dot product being 0) is not handled by the spec
  2. The fact that the sample model crucially relies on this epsilon is not ideal

I mean, maybe I wouldn't even nitpick about the second point if that value was something like 1e-8. But that 4.4896593387466766e-11 looks like noise - something that could easily (accidentally and unintentionally) have fallen out of some "AxisAngle-to-Quat" conversion or so.


An aside: The fact that you are using Cesium to show the interpolation is interesting. And *nervously looks left and right* the date and contents of CesiumGS/cesium#11684 are totally unrelated to this issue and a pure coincidence ;-) But seriously: The underlying implementation shows the difficulty of these "epsilons": If this was EPSILON12 instead of EPSILON6, then the result might be messed up in one way or another...


A suggestion to resolve this:

Could we agree that it would make more sense to insert some additional interpolation keyframes with clear 90 degree rotations in this model? This way, the "shorter path" would be unambiguously clear, and we could get rid of this ugly "necessary epsilon value" ...

@javagl
Copy link
Contributor

javagl commented May 30, 2024

A short addendum: Another sandcastle with the code

const startQ = new Cesium.Quaternion(0, 0, 0, -1);
const endQ = new Cesium.Quaternion(1, 0, 0, 4.4896593387466766e-11);
const n = 9;

for (let i = 0; i < n; ++i) {
  const q = Cesium.Quaternion.slerp(startQ, endQ, i / (n - 1), new Cesium.Quaternion());
  const axis = Cesium.Quaternion.computeAxis(q, new Cesium.Cartesian3());  
  const angleRad = Cesium.Quaternion.computeAngle(q);
  const angleDeg = Cesium.Math.toDegrees(angleRad);
  console.log("Step "+i+" angleDeg "+angleDeg+" axis "+axis);
}

and the output

Step 0 angleDeg 360 axis (1, 0, 0)
Step 1 angleDeg 337.5000000006431 axis (-1.000000000000001, 0, 0)
Step 2 angleDeg 315.00000000128614 axis (-0.9999999999999993, 0, 0)
Step 3 angleDeg 292.50000000192927 axis (-0.9999999999999997, 0, 0)
Step 4 angleDeg 270.0000000025724 axis (-1, 0, 0)
Step 5 angleDeg 247.5000000032155 axis (-0.9999999999999999, 0, 0)
Step 6 angleDeg 225.0000000038586 axis (-0.9999999999999999, 0, 0)
Step 7 angleDeg 202.5000000045017 axis (-1, 0, 0)
Step 8 angleDeg 180.00000000514478 axis (-1, 0, 0)

There is a certain degree of arbitrariness in the initial rotation (i.e. the first quat), because it is a rotation about 360 degrees, and therefore, the axis could be any axis. But using the x-axis seems to be a common default for this case.

@ToolTech
Copy link
Author

ToolTech commented May 30, 2024 via email

@ToolTech
Copy link
Author

ToolTech commented May 30, 2024 via email

@emackey
Copy link
Member

emackey commented May 30, 2024

Thanks @javagl

This ambiguity (basically by the dot product being 0) is not handled by the spec

I'm sure some folks would advocate adding it to the spec for completeness. But in practice, various implementations I've worked on over the years (including Cesium, STK, and Sample Viewer) generally use basic slerp from some lower-level math library or some official reference on quaternions, not on glTF. I don't know any implementation that has a custom glTF-specific slerp that is separate from the general-purpose slerp.

So I'm not saying we shouldn't include it in the spec. I'm saying it will be another area where all tools effectively ignore what we've written in the spec and continue to use their built-in or third-party math libraries that they've already been using.

could we agree that it would make more sense to insert some additional interpolation keyframes

Yes. I'm usually hesitant to edit such a classic sample that's been around since before glTF itself has been around, but this does seem a case where adding a 90-degree intermediate step might make sense. The counter-argument would be that since this model is mostly featureless, a casual viewer can't even tell what's front or back, and therefore can't tell at a glance what's clockwise or counter-clockwise. If you don't like the direction it's rotating, just spin the model around in your viewer and look at it from the other side. This model doesn't care which way it rotates, it's there to test the timing of the separate animation targets. We have plenty other models where it matters much more obviously which way the rotation goes.

We could also add a warning in the validator if it finds adjacent 180-degree-apart rotational keyframes. In general that's a bad situation for slerp, susceptible to differences between implementations, and could warrant a warning.

The fact that you are using Cesium to show the interpolation is interesting

Ah, I actually wasn't aware of your recent changes to Quaternions there. I chose Cesium because I'm an early contributor, the original author of the Sandcastle tool and I'm most comfortable rapid-prototyping code in that environment. Any time I need to demonstrate the effects of spherical linear interpolation without warping an unsuspecting citizen's brain with thoughts of how a point slides along the surface of a four-dimensional unit sphere that controls the orientation of a 3D object, then I tend to just dash off some Cesium code to print the numbers and show the answer. Then I can say "See? It works!"

@ToolTech
Copy link
Author

ToolTech commented May 30, 2024 via email

@emackey
Copy link
Member

emackey commented May 30, 2024

@ToolTech You might also want to try InterpolationTest. That one should be easier to visually inspect and compare to the animation in its README.

@javagl
Copy link
Contributor

javagl commented May 30, 2024

I'm sure some folks would advocate adding it to the spec for completeness.
[...]
I'm saying it will be another area where all tools effectively ignore what we've written in the spec and continue to use their built-in or third-party math libraries that they've already been using.

When there is a math formula with a division, where the divisor can be zero, then this always makes my eye twitch. But I agree with the latter, so it may not be utterly important.

Yes. I'm usually hesitant to edit such a classic sample that's been around since before glTF itself has been around, but this does seem a case where adding a 90-degree intermediate step might make sense. The counter-argument would be that since this model is mostly featureless, a casual viewer can't even tell what's front or back, and therefore can't tell at a glance what's clockwise or counter-clockwise.

I agree that one main point of this model is the handling of animation targets, which is explicitly pointed out in https://github.com/KhronosGroup/glTF-Sample-Assets/tree/main/Models/BoxAnimated#common-problems . But ... in their struggle to get this right, people shouldn't additionally be stressed out by some epsilon-based quaternion glitches. There should be a clear "right" and "wrong" here, and the model should allow testing the implementation, ...
"without warping an unsuspecting citizen's brain with thoughts of how a point slides along the surface of a four-dimensional unit sphere that controls the orientation of a 3D object"
(Exactly that 🙂 )

I'll try to allocate some time to update the model 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants