-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
WebGLState: Added caching for blend equation #14059
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
Conversation
|
Can you please setup your PR on the latest |
|
|
||
| if ( blending !== CustomBlending ) { | ||
|
|
||
| if ( blending !== currentBlending || premultipliedAlpha !== currentPremultipledAlpha ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused. This line should already ensure no unnecessary updates, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was confused as well but it turns out that the currentBlending variable is shared between different materials so it did not ensure the blending equation caching (that's why I use the currentMaterial variable actually). Maybe it was supposed to serve another purpose that I'm not aware of, so I kept it just in case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the change is correct. Caching WebGL State information is something different than caching uniforms. The WebGL state is always related to a draw call, it can't retain information that is related to a material (or shader program). In other words: You always have to set the blending equation or function if the current ones do not match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I believe you are right about that. I believe that the reason of the redundant calls was that the blending equation function is acually the same (gl.FUNC_ADD) for different blending modes and that probably was the reason why this PR seemed to work. Maybe there should be different caches for blending equations and functions to prevent the redundant calls. Any thoughts?
For example:
if (NOT the last blending equation was gl.FUNC_ADD){
gl.blendEquationSeparate( gl.FUNC_ADD, gl.FUNC_ADD );
}
The line if ( blending !== currentBlending || premultipliedAlpha !== currentPremultipledAlpha) is not preventing gl.BlendEquation calls. I believe it was designed for gl.blendFuncSeparate calls.
I'm closing this PR as this adresses the problem in a wrong way :) I'll try non material based caching for blend equations as I mentionned above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for investigating this! 👍
In this PR a cache mechanism is implemented for Blend equations inside the WebGLState. I found this to be necessary since three.js was making redundant WebGL calls for such functions:

After the implementation I observed that such WebGL calls are eliminated thus performance increase is expected for scenes with many materials.