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 GLTF importer forcing vertex colors on all materials #82272

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

darksylinc
Copy link
Contributor

@darksylinc darksylinc commented Sep 24, 2023

The importer already checks if a mesh has vertex colors and enables vertex colors on the material using it.

Before this fix, GLTF importer would force shader generation to use vertex colors even if the scene did not have vertex colors at all, or did not need them for some of its meshes; causing inefficient shader and PSO generation.

The importer already checks if a mesh has vertex colors and enables
vertex colors on the material using it.

Before this fix, GLTF importer would force shader generation to use
vertex colors even if the scene did not have vertex colors at all, or
did not need them; causing inefficient shader and PSO generation.
@darksylinc darksylinc requested a review from a team as a code owner September 24, 2023 22:39
@darksylinc
Copy link
Contributor Author

This fix does not fix projects with already-imported GLTF meshes.

Meshes will have to be reimported to take advantage of the fix.

Visually, this bug is harmless. It only affects performance (and mostly low end, since high end will probably not bottlenecked by this).

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

According to the description, we are enabling vertex color on all cases rather than conditionally. I think conditionally apply vertex colour is fine, but don't have a large collection of user projects to test.

@akien-mga akien-mga merged commit 34de6c6 into godotengine:master Sep 25, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga changed the title Fix gltf importer forcing vertex colors on all materials Fix GLTF importer forcing vertex colors on all materials Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants