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

Node transform in gltf2ozz.cc is not updated correctly #115

Closed infosia closed 3 years ago

infosia commented 3 years ago

Hi, I happened to find a potential issue in gltf2ozz.cc where node transform is not updated correctly.

https://github.com/guillaumeblanc/ozz-animation/blob/master/src/animation/offline/gltf/gltf2ozz.cc#L431-L435

ozz::math::SimdFloat4 translation, rotation, scale;
if (ToAffine(matrix, &translation, &rotation, &scale)) {
  ozz::math::Store3PtrU(translation, &_transform->translation.x);
  ozz::math::StorePtrU(translation, &_transform->rotation.x);
  ozz::math::Store3PtrU(translation, &_transform->scale.x);
  return true;
}

If I understand correctly rotation and scale is not updated there. It should be -

ozz::math::SimdFloat4 translation, rotation, scale;
if (ToAffine(matrix, &translation, &rotation, &scale)) {
  ozz::math::Store3PtrU(translation, &_transform->translation.x);
  ozz::math::StorePtrU(rotation, &_transform->rotation.x);
  ozz::math::Store3PtrU(scale, &_transform->scale.x);
  return true;
}

Sorry I just reviewed your code in the browser now and did not do any tests. Please close this if I said something stupid.

guillaumeblanc commented 3 years ago

Hi,

you're absolutely right! Seems that this case isn't covered by testing either.

Would be awesome if you could do a pull request to fix it. Can you?

Thanks a lot, Guillaume

infosia commented 3 years ago

Pushed PR #116 👍