Use Hermite instead of Bezier for glTF spline interpolation

[Gurvan]

The cubic spline interpolation implementation in glTF animations wrongly uses Bezier splines instead of Hermite splines (gltf 2.0 spec, Hermite Spline). This results in incorrect interpolation, see related issue.

Thankfully, there is an easy conversion from Hermite coefficients to Bezier coefficients that fixes the issue with minimal change.

• Bugsquad edit, closes: https://github.com/godotengine/godot/issues/93596

[fire]

We’ll look into this.

[Gurvan]

Yes, it is the other option. I went for the minimal change in the code, but changing the bezier function to properly compute the interpolation the way it's explained in the glTF spec would probably be clearer. In that case it would also be better to rename the function from bezier to hermite, and also rename the Quaternion one (that doesn't do Bezier nor Hermite interpolation by the way).

[AThousandShips]

You merged your branch incorrectly or rebased incorrectly, taking over other commits, please fix by:

git reset --hard b653ec8
git rebase -i master

Then add your fixes again and

git push --force

[fire]

Is the test case in https://github.com/godotengine/godot/issues/93596 ok to validate this or is there other cases?

[Gurvan]

It does test the spline interpolation for translation and scaling (rotation works differently) for arbitrary values and tangents. A case that combine translation and scale could be made, but it wouldn't test anything more than the test in #93596. The glTF samples repository has infact an InterpolationTest.glb, but all the tangents in the file are 0, so it can't be used as a actual test case.

Personally, I would be confident to use it as it is.

[fire]

I will try to do a review in a few days and then approve.

[Gurvan]

There are so many different types of cubic splines, I would say it's probably better to keep their names explicit when possible. I would be happy to follow up with another PR for moving the interpolation functions into Math.

[akien-mga]

Looks good! Could you squash the commits? See PR workflow for instructions.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution :tada:

[Gurvan]

Thanks to the Godot team for making the whole process feel great!

[TokageItLab]

@Gurvan If you have time, give it a try the PR to implement hermite_interpolate() in the Math class and call them in the glTF importer :) Although the merge will probably be 4.4.dev (after than 4.3.stable) at the earliest since it is enhancement.