Fix out of bounds access to rotation array in assimp loader

[luca-della-vedova]

🦟 Bug fix

Alternative to #478, it fixes the root cause of the issue that is undefined behavior due to access past the end of the array.

Summary

When the parsing function was written, the assumption was that the position keys array and the rotation keys array would be the same length, this proved to not be the case since they can have different lengths.
The root cause of the issue addressed through #478 was that the rotation key array only had one element while the position key array was much longer, hence we were accessing values past the end of the array which is undefined behavior and can cause all sorts of trouble to downstream users.
Other cases with other animations can cause NaN exceptions in OGRE2 that crash the whole application.

With this PR we iterate over the longest of the arrays (to make sure no data is lost) and set the access to the position / rotation keys to be the minimum between the current index and the length of the array, to avoid undefined behavior.
This means that the last key will be duplicated in case the array is shorter, but that is acceptable behavior. It seems assimp returns a rotation array of length 1 to denote that rotation does not change throughout the animation.

The first one-line commit fixes the issue we are personally observing but I added f2cb2ce to fix the symmetrical case

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov]

Codecov Report

Merging #479 (f2cb2ce) into gz-common5 (22b0340) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           gz-common5     #479   +/-   ##
===========================================
  Coverage       80.61%   80.62%           
===========================================
  Files              90       90           
  Lines           10241    10244    +3     
===========================================
+ Hits             8256     8259    +3     
  Misses           1985     1985           

Impacted Files	Coverage Δ
graphics/src/AssimpLoader.cc	89.13% <100.00%> (+0.12%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mxgrey]

I'm concerned that the approach of truncating the key frames to the array with a smaller number will throw away important animation information.

Instead I'd suggest fixing this by interpolating the smaller array to have the same number of key frames as the larger one, and then combining them in the poses used by the skeletal animation.

I misread the changes. This PR is duplicating the last key of the smaller array, not truncating the additional keys of the larger one. That seems like a reasonable way to fix this issue.