Animation support.

[mrehayden1]

Just opening a PR to help track what's left to do.

I've started testing this with some real animated models exported from Blender. So far so good, but we're missing support for accessors with normalized integer component types which I might add, but that hasn't been a problem so far.

Regression tests are also needed too I suppose.

[mrehayden1]

In the spirit of the higher-level nature of the library, I also thought it best to remove AnimationSamplers from the adapted Animations and inlined the loaded buffer data for each of the AnimationChannels' transformations. It's a one to one relationship (I think) between AnimationChannels and AnimationSamplers, so it felt okay.

[sgillespie]

It's a one to one relationship (I think) between AnimationChannels and AnimationSamplers, so it felt okay.

I'm not sure I agree with this. There are some counterexamples here: https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#animations. However,

I also thought it best to remove AnimationSamplers from the adapted Animations and inlined the loaded buffer data for each of the AnimationChannels' transformations.

I still think this is fine. The worst case is we'll have some duplicated data

[mrehayden1]

This implementation is still missing support for sampler outputs that use normalized integers instead of just floats, namely rotations and weights.

See the last two tables in the section linked for more details.
https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#animations

[sgillespie]

Tests are failing with:

Failures:

  test/Text/GLTF/Loader/Internal/AdapterSpec.hs:315:3: 
  1) Text.GLTF.Loader.Internal.Adapter.runAdapter Runs a basic GlTF adapter
       uncaught exception: RecConError
       test/Text/GLTF/Loader/Internal/AdapterSpec.hs:(315,3)-(324,5): Missing field in record construction gltfAnimations

  To rerun use: --match "/Text.GLTF.Loader.Internal.Adapter/runAdapter/Runs a basic GlTF adapter/"

  test/Text/GLTF/Loader/Internal/AdapterSpec.hs:315:3: 
  2) Text.GLTF.Loader.Internal.Adapter.adaptGltf Adapts a basic GlTF
       uncaught exception: RecConError
       test/Text/GLTF/Loader/Internal/AdapterSpec.hs:(315,3)-(324,5): Missing field in record construction gltfAnimations

  To rerun use: --match "/Text.GLTF.Loader.Internal.Adapter/adaptGltf/Adapts a basic GlTF/"

Randomized with seed 1077071137

EDIT: Otherwise, LGTM

[sgillespie]

@mrehayden1 How are you feeling about this? Are you ready to merge this?

[mrehayden1]

@sgillespie I've got a branch that adds skinning support too to merge in also now the PR to matrices is merged. Can we do it here or shall I open another PR?

Update: Reading the code and from what I remember, skeletal animations is independent of skins (but not the other way round), so I would be happy to merge this.

[mrehayden1]

A preview of how animations and skinning are working.

5g9w8r_2.mp4

[sgillespie]

@sgillespie I've got a branch that adds skinning support too to merge in also now the PR to matrices is merged. Can we do it here or shall I open another PR?

I'll leave that up to you--whatever is easier for you

[mrehayden1]

Okay. I've just opened a new PR for skinning.

[sgillespie]

Great! If you resolve the conflicts here, I'll merge

[mrehayden1]

I think i pushed a merge commit before. Did that not resolve any conflicts?

[sgillespie]

I think i pushed a merge commit before. Did that not resolve any conflicts?

Ah, there's a merge commit, that's why I can't rebase. No worries, I'll do a regular merge

[mrehayden1]

I think i pushed a merge commit before. Did that not resolve any conflicts?

Ah, there's a merge commit, that's why I can't rebase. No worries, I'll do a regular merge

Thanks!

I'll make sure to rebase next time.