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

Fix AssimpLoader collada texture coordinates #634

Merged
merged 1 commit into from
Sep 5, 2024
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Sep 5, 2024

🦟 Bug fix

Fixes #633

Summary

Fixes UNIT_AssimpLoader_TEST with assimp 5.4.3. This should fix homebrew CI.

This PR tweaks the way we check for texture coordinates from assimp. Empty texture coordinate slots are now allowed as of assimp/assimp#5636. The new logic should be compatible with assimp 5.4.3 and prior versions

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: Ian Chen <[email protected]>
@iche033 iche033 marked this pull request as draft September 5, 2024 20:34
@iche033 iche033 changed the title Fix AssimpLoader Texcoords [DRAFT - Testing CI] Fix AssimpLoader loading texture coordinates Sep 5, 2024
@iche033 iche033 marked this pull request as ready for review September 5, 2024 21:32
@iche033
Copy link
Contributor Author

iche033 commented Sep 5, 2024

Homebrew CI is fixed. This PR is ready for review.

@iche033 iche033 changed the title Fix AssimpLoader loading texture coordinates Fix AssimpLoader texture coordinates Sep 5, 2024
@iche033 iche033 changed the title Fix AssimpLoader texture coordinates Fix AssimpLoader collada texture coordinates Sep 5, 2024
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@iche033 iche033 merged commit 3f129c3 into gz-common6 Sep 5, 2024
12 checks passed
@iche033 iche033 deleted the assimp_dae_uvset branch September 5, 2024 22:13
Crola1702 pushed a commit that referenced this pull request Sep 16, 2024
Fixes UNIT_AssimpLoader_TEST with assimp 5.4.3. This should fix homebrew CI.

This PR tweaks the way we checks for texture coordinates from assimp. Empty texture coordinate slots are now allowed as of assimp/assimp#5636. The new logic should be compatible with assimp 5.4.3 and prior versions

Signed-off-by: Ian Chen <[email protected]>
@Crola1702 Crola1702 mentioned this pull request Sep 16, 2024
8 tasks
Crola1702 pushed a commit that referenced this pull request Sep 23, 2024
Fixes UNIT_AssimpLoader_TEST with assimp 5.4.3. This should fix homebrew CI.

This PR tweaks the way we checks for texture coordinates from assimp. Empty texture coordinate slots are now allowed as of assimp/assimp#5636. The new logic should be compatible with assimp 5.4.3 and prior versions

Signed-off-by: Ian Chen <[email protected]>
scpeters pushed a commit that referenced this pull request Sep 23, 2024
Fixes UNIT_AssimpLoader_TEST with assimp 5.4.3. This should fix homebrew CI.

This PR tweaks the way we checks for texture coordinates from assimp. Empty texture coordinate slots are now allowed as of assimp/assimp#5636. The new logic should be compatible with assimp 5.4.3 and prior versions

Signed-off-by: Ian Chen <[email protected]>
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.

UNIT_AssimpLoader_TEST tests failing on homebrew
2 participants