Fix glTF / glb root node transform

[iche033]

🦟 Bug fix

Summary

There is a long standing issue in assimp on models having incorrect rotations: assimp/assimp#849.

Currently, our assimp loader drops the root node transform (setting it to identity) to work around the issue. However, this caused other assets's rotation to be appear incorrect and inconsistent with other rendering / modeling tools. This PR adapts a another workaround posted in assimp/assimp#849 to fix the root node transform for glTF / glb meshes.

Update as of 40c7bbb - Changes should how only affect glTF / glb and kept the same behavior for other file formats like dae.

Here are tests with a couple of glb meshes in gazebo and other tools. The camera in Gazebo is set up to have the same orientation as the these tools (Y up, facing -X)

WaterBottle

Gazebo Before (ignore the black seam at the front, it's clipped by camera's near clip plane):

Gazebo After (pay attention to logo on the bottle):

three.js editor:

godot:

BarramundiFish:

Gazebo Before:

Gazebo After:

three.js editor:

godot:

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.

[iche033]

@luca-della-vedova do you still remember the meshes that you were having the rotation issue with? I should probably also test them to make sure they still work

[codecov]

Codecov Report

Merging #543 (183fd39) into gz-common5 (d9d1722) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 183fd39 differs from pull request most recent head 9b533c8. Consider uploading reports for the commit 9b533c8 to get more accurate results

@@              Coverage Diff               @@
##           gz-common5     #543      +/-   ##
==============================================
+ Coverage       83.64%   83.66%   +0.01%     
==============================================
  Files              90       90              
  Lines           10243    10252       +9     
==============================================
+ Hits             8568     8577       +9     
  Misses           1675     1675              

Files	Coverage Δ
graphics/src/AssimpLoader.cc	89.33% <100.00%> (+0.36%)	⬆️

[ahcorde]

can we add a test to UpdatedRootNodeTransform ?

[luca-della-vedova]

@luca-della-vedova do you still remember the meshes that you were having the rotation issue with? I should probably also test them to make sure they still work

Very interesting, thanks for the fix!
I believe the issue was actually not found on glb models but on other extensions, I can't remember the details but I believe one of the meshes that gave us the most trouble was the walking actor (collada).
All the glb meshes we used had identity transforms in the root node.

[iche033]

can we add a test to UpdatedRootNodeTransform ?

After testing with different glb meshes, I found that most of the code from the workaround is actually not needed so I simplified the fix quite a bit in 8b0d752. The current AssimpLoader tests has code to load glTF / glb meshes and should now cover all of the the new code (coverage).

[iche033]

I believe the issue was actually not found on glb models but on other extensions

ah ok I see! The changes in this PR should only affect glTF / glb meshes and not daes so I think we should be fine here.

[iche033]

added one more test to verify that the glb mesh's vertices are correct. 9b533c8