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

Emscripten WebGL inefficiency with glEnable/DisableVertexAttribArray #1960

Open
juj opened this issue Dec 3, 2019 · 6 comments
Open

Emscripten WebGL inefficiency with glEnable/DisableVertexAttribArray #1960

juj opened this issue Dec 3, 2019 · 6 comments

Comments

@juj
Copy link
Contributor

juj commented Dec 3, 2019

Looking at a bgfx demo in Spector.js shows the two following profiles:

spector_js

spector_js_2

The renderer first disables all attrib arrays, and then re-enables them. These API calls are quite inefficient in web browsers. It would be better to use a bitfield to track which are currently set, and which want to be set, and only do the minimal delta of enable/disable calls to switch from old state to new state. This can improve CPU performance on the order of +5%.

@bkaradzic
Copy link
Owner

This can improve CPU performance on the order of +5%.

Cool. Send PR.

@juj
Copy link
Contributor Author

juj commented Dec 8, 2019

A quick test with removing the redundant calls to glEnableVertexAttribArray and glDisableVertexAttribArray from the generated JS build output code shows a -9.9% reduction in used CPU time (or +11% performance increase).

Before:
Screen Shot 2019-12-08 at 10 06 37 AM

After:
Screen Shot 2019-12-08 at 10 07 31 AM

(glDisableVertexAttribArray removed to be never called, glEnableVertexAttribArray only called to enable array indices if they haven't been enabled yet)

@bkaradzic
Copy link
Owner

You still have to call glDisableVertexAttribArray for attributes not used by vertex shader. This type of optimization affects other platforms. Could you check on some other platform other than desktop?

@juj
Copy link
Contributor Author

juj commented Dec 9, 2019

You still have to call glDisableVertexAttribArray for attributes not used by vertex shader.

Actually, why? A honest question, I was thinking the same, but cannot find a reason in the GL spec to see why any attrib arrays need to be disabled. The GL spec does not seem to give a reason, technical or behavioral, that I could find. Disabling an attrib array means "I want to use a constant vector value for this attribute instead of sourcing the attribute from a VBO" (i.e. source that attribute via glVertexAttrib*f instead of glVertexAttribPointerfv), it does not mean that that attribute index would be disabled (there is no function to disable attribute indices). Also disabling an attrib array does not unbind the VBO from that attribute index so calling glDisableVertexAttribArray does not contribute to releasing memory either. Extra attributes not consumed by a VS are just ignored.

This type of optimization affects other platforms. Could you check on some other platform other than desktop?

I did not actually modify the engine code, but just the build output of a page from Emscripten that I was given to consult. I don't have the source project to test other platforms.

@bkaradzic
Copy link
Owner

See: #1515

@juj
Copy link
Contributor Author

juj commented Dec 9, 2019

Thanks, very interesting. In such case, using the bitfield approach to keep the enabled set of attribute array indices to a minimum can be used. In the profile numbers above where I removed the glDisableVertexAttribArray calls altogether there was no effect to rendering, so the profiles show an upper limit as to what the performance effect of these api calls is.

juj added a commit to juj/bgfx that referenced this issue Mar 17, 2020
bkaradzic pushed a commit that referenced this issue Mar 30, 2020
pigpigyyy added a commit to pigpigyyy/bgfx that referenced this issue Mar 31, 2020
* master:
  Move GL_CHECK to GL callsites.
  make sure depth textures are resolved as well when blitting frameBuffer
  Cleanup.
  Adjust integer texture format enums for desktop OpenGL < 4.0
  Revert back to using GL_HALF_FLOAT (that is GL_HALF_FLOAT_OES)
  Fix OpenGL ES texture formats and remove runtime texture probing on WebGL.
  Cleanup.
  Create WebGL 2 context if available. Work around Chrome performance bug https://bugs.chromium.org/p/chromium/issues/detail?id=1045643
  Optimize glUniform1iv and glUniform4fv to use faster glUniform1i and glUniform4f on WebGL.
  Avoid redundant GL calls to glEnableVertexAttribArray() and glDisableVertexAttribArray(). bkaradzic#1960
rozgo pushed a commit to VertexStudio/bgfx that referenced this issue Aug 14, 2020
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

No branches or pull requests

2 participants