Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use scene name for material name #564

Conversation

luca-della-vedova
Copy link
Member

@luca-della-vedova luca-della-vedova commented Dec 13, 2023

🦟 Bug fix

Summary

#563 rebased to main.

When importing assimp scenes, the name of the generated texture used the name of the root node as a prefix.
This worked well in simple GLB assets where there is only one node and the name would be its name but the moment a user was to import a scene with more than one node assimp would automatically create a new node and hardcode its name to ROOT as shown here.

This creates issues when running gz with ogre2 since we use the texture name to share textures between different assets, so different files would result in textures called ROOT_[...] and would cause the wrong texture to be displayed.

The fix is to use the scene name instead of the root node name (which was probably the right implementation in the first place).

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.

Signed-off-by: Luca Della Vedova <[email protected]>
Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8816a65) 83.64% compared to head (4e5bba5) 83.64%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #564   +/-   ##
=======================================
  Coverage   83.64%   83.64%           
=======================================
  Files          79       79           
  Lines       10214    10214           
=======================================
  Hits         8543     8543           
  Misses       1671     1671           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@luca-della-vedova luca-della-vedova marked this pull request as ready for review December 13, 2023 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants