There's currently a negative performance impact if WEBGL_draw_buffers is enabled. For example, we do additional work in Clear(). This is unexpected and can cause frameworks like emscripten to unintentionally lower the performance of applications.

I've filed https://github.com/kripken/emscripten/issues/2373 about trying to deal with this.

What is the perf impact? Glancing through the code, I don't see anything obvious. I assume we're over-clearing somehow?

The drawBuffers call in clear() causes a flush

... what drawBuffers call in clear()?

Ooh, ok. That's not in webgl.clear, that's in *ClearScreen*. This is an easy fix, we just need to cache calls to DrawBuffers.

It may also be that DrawBuffers() is just causing work that the Clear() would have caused anyways...

Comment on attachment 8428010 [details] [diff] [review] Something like this could work Review of attachment 8428010 [details] [diff] [review]: ----------------------------------------------------------------- We probably shouldn't be doing the state introspection that we do here, but if it's "ok" for now, we can leave it. ::: content/canvas/src/WebGLContext.cpp @@ +994,5 @@ > bool initializeColorBuffer = 0 != (mask & LOCAL_GL_COLOR_BUFFER_BIT); > bool initializeDepthBuffer = 0 != (mask & LOCAL_GL_DEPTH_BUFFER_BIT); > bool initializeStencilBuffer = 0 != (mask & LOCAL_GL_STENCIL_BUFFER_BIT); > bool drawBuffersIsEnabled = IsExtensionEnabled(WebGLExtensionID::WEBGL_draw_buffers); > + bool saveDrawBuffers = false; It's less that we need to save it, and more that we need to override it temporarily. `shouldOverrideDrawBuffers`?

(In reply to Jeff Gilbert [:jgilbert] from comment #8) > Comment on attachment 8428010 [details] [diff] [review] > Something like this could work > > Review of attachment 8428010 [details] [diff] [review]: > ----------------------------------------------------------------- > > We probably shouldn't be doing the state introspection that we do here, but > if it's "ok" for now, we can leave it. What would your preferred implementation look like?

I confirmed this fixes the bug I was looking at

(In reply to Jeff Muizelaar [:jrmuizel] from comment #9) > (In reply to Jeff Gilbert [:jgilbert] from comment #8) > > Comment on attachment 8428010 [details] [diff] [review] > > Something like this could work > > > > Review of attachment 8428010 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > We probably shouldn't be doing the state introspection that we do here, but > > if it's "ok" for now, we can leave it. > > What would your preferred implementation look like? We should just cache this stuff, instead of ever risking calls to glGet* functions. We don't need to do that here, though.

We are hoping this to be uplifted to FFOS 1.4, since we are targeting to improve performance on that release. Is this change possible to be applied there?

1.4+ blocks a blocker