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

WebGLRenderer: Ensure program properties are booleans. #22244

Merged
merged 1 commit into from
Aug 3, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Aug 2, 2021

Related issue: Fixed #22242

Description

Unfortunately certain checks in WebGLRenderer and WebGLPrograms added in r131 due to the removal of certain material properties introduce a performance regression since getProgram() is called too often. That happens because flags in WebGLPrograms and WebGLRenderer did not have the same type (boolean) so comparison always failed.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Aug 2, 2021

@CLVNSwan Would appreciate a second pair of eyes that can validate this fix 👍 .

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Aug 2, 2021

@mrdoob I guess this hotfix should be cherry-picked to master (sorry for the inconvenience 😞 ).

@CLVNSwan
Copy link

CLVNSwan commented Aug 2, 2021

@Mugen87 The fix worked for me 👍 Thank you! 😊

@titansoftime
Copy link
Contributor

This solved my performance issue as well. Thank you!

@mrdoob
Copy link
Owner

mrdoob commented Aug 3, 2021

@mrdoob I guess this hotfix should be cherry-picked to master (sorry for the inconvenience 😞 ).

No worries! 👍

@mrdoob mrdoob added this to the r132 milestone Aug 3, 2021
@mrdoob mrdoob merged commit 15ebdbd into mrdoob:dev Aug 3, 2021
@mrdoob
Copy link
Owner

mrdoob commented Aug 3, 2021

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Aug 3, 2021

Cherry picked to master, published 0.131.2 and updated gh-pages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance loss revision upgrade r130 -> r131
4 participants