KhronosGroup / glTF-Sample-Models

glTF Sample Models
3.13k stars 1.31k forks source link

InterpolationTest translation animations loop badly #391

Closed mkulagowski closed 11 months ago

mkulagowski commented 1 year ago

I've noticed that both scale and rotation animations in InterpolationTest loop nicely. The translation animations on the other hand do not - animation starts in the middle but ends below.

Can we make sure that the last keyframe overlaps with the first one? Or at least explicitly note somewhere that the translation animation is not supposed to loop nicely, but instead jump a little bit?

javagl commented 1 year ago

I first wasn't sure what this referred to, but it is this:

Khronos Interpolation

There seem to be strange values in the corresponding accessor. And... looking more closely at the model, nearly all values seem to be ... slightly off. For example, the (initial) translations of the nodes for the translation animation are

  node6.setTranslation([0,6.665226459503174,0]);
  node7.setTranslation([3.3051798343658447,6.734194278717041,0]);
  node8.setTranslation([-3.2975807189941406,6.958913326263428,0]);

i.e. their initial y-value is 6.66, 6.73 and 6.95, respectively. This is probably caused by the model initially being created in blender by dragging the objects to the "right" position with the mouse.

I started "fixing" this locally (basically by throwing it into gltfTransformifier and hand-editing the values...). A first pass is attached here:

InterpolationTestFix0001.zip

There, the initial y-values are all 6.8. The animations start at 6.8 and and at 6.8. Aint that nice?

But ... here are the key frame times of the animations:

  accessor9.setArray(new Float32Array([
    0.0416666679084301, 
    0.4166666567325592, 
    0.875, 
    1.25, 
    1.6666666269302368
  ]));
  accessor12.setArray(new Float32Array([
    0, 
    0.4166666567325592, 
    0.875, 
    1.25, 
    1.7083333730697632
  ]));
  accessor15.setArray(new Float32Array([
    0, 
    0.4583333432674408, 
    0.8333333134651184, 
    1.2916666269302368, 
    1.7083333730697632
  ]));
  accessor17.setArray(new Float32Array([
    0, 
    0.4166666567325592, 
    0.8333333134651184, 
    1.25, 
    1.6666666269302368
  ]));

I'll probably do another pass over that and try to fix it a bit more.

In the meantime, for all other perfectionists out there, enjoy this image of a 89° angle....

89degree

javagl commented 1 year ago

An updated version, with "all nice" values:

InterpolationTestFix0003.zip

(it also contains the hand-tweaked gltfTransformifier output that was used for creating it)

Unless someone disagrees, I'd create a PR to update the sample model with this one.

mkulagowski commented 1 year ago

I've tested the 3rd version and it loops perfectly! This issue is actually result of my debug session, searching why my gltf anim implementation is jumping on looping...only to find out that the animation itself was baked poorly 🤣

javagl commented 1 year ago

An update for the model is in https://github.com/KhronosGroup/glTF-Sample-Assets/pull/26 (note that the glTF-Sample-Models repo will soon transition to the glTF-Sample-Assets repo, so it probably makes sense to create the PR there).

There is still some issue with either the model or the glTF-Sample-Viewer. That may have to be sorted out before merging it.

DRx3D commented 11 months ago

@javagl : Do you believe that all aspects of this issue have been resolved with the PR to Sample Assets?

If so, this issue will be closed by 27 Nov.

javagl commented 11 months ago

The issue itself is resolved with the linked PR, and when the linked PR is merged, then this issue can be closed.

I just added a comment at https://github.com/KhronosGroup/glTF-Sample-Assets/issues/77 : We could consider handling the "0-1-scaling" in the InterpolationTest as well. But one could also make a case for creating a new model (no strong opinion here)

DRx3D commented 11 months ago

PR #6 for Sample-Assets has been merged. This Issue is being closed without action because Sample-Assets supersedes Sample-Models.