search

guillaumeblanc / ozz-animation

Open source c++ skeletal animation library and toolset
http://guillaumeblanc.github.io/ozz-animation/
Other
2.46k stars 302 forks source link

Fixes incorrect root joint detection bug in glTF importer. #124

Closed msiedlarek closed 3 years ago

msiedlarek commented 3 years ago

glTF 2.0 specification requires that all joints referenced by a skin have a common root, but does not require that this root is referenced by the skin directly. glTF importer assumed that, which caused invalid joint to be chosen as the root of the skeleton.

To fix this entire joint forest for the scene must be reversed, so FindSkinRootJointIndices method is also slightly refactored to this only once.

guillaumeblanc commented 3 years ago

Hi, thanks for submitting this PR!

I assume this is the specification you're referring to ?

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#joint-hierarchy

Joint Hierarchy The joint hierarchy used for controlling skinned mesh pose is simply the glTF node hierarchy, with each node designated as a joint. Each skin's joints must have a common root, which may or may not be a joint node itself. When a skin is referenced by a node within a scene, the common root must belong to the same scene.

The change looks good. I ran it on ozz gltf data and it works fine.

Can you see a way to include this use case in the unit tests ?

Thanks, Guillaume

guillaumeblanc commented 3 years ago

Any update on how can this be tested?

guillaumeblanc commented 3 years ago

Hi, FYI I added your name to the authors list as you contributed with a PR. Hope it's ok for you. Cheers, Guillaume