bug 843667 - update WebGL EXT_draw_buffers to fix a potential crash on mobile - r=bjacob
☠☠ backed out by 4c67eebe3e20 ☠ ☠
authorGuillaume Abadie <gabadie@mozilla.com>
Tue, 02 Jul 2013 16:50:31 -0400
changeset 137248 43e688b70d84e681ff0baf677d49c31c2933f2c5
parent 137247 0f4163dd261def59476623746fbad8d533557cde
child 137249 796773a10db13eec7601a23c73fb30c98c9adee5
push id1824
push userryanvm@gmail.com
push dateWed, 03 Jul 2013 18:16:56 +0000
treeherderfx-team@dcbbfcdf7bb4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbjacob
bugs843667
milestone25.0a1
bug 843667 - update WebGL EXT_draw_buffers to fix a potential crash on mobile - r=bjacob
content/canvas/src/WebGLContext.cpp
content/canvas/src/WebGLContext.h
content/canvas/src/WebGLExtensionDrawBuffers.cpp
content/canvas/src/WebGLFramebuffer.cpp
gfx/gl/GLContext.cpp
--- a/content/canvas/src/WebGLContext.cpp
+++ b/content/canvas/src/WebGLContext.cpp
@@ -1180,17 +1180,17 @@ WebGLContext::GetExtension(JSContext *cx
     }
 
     return WebGLObjectAsJSObject(cx, mExtensions[ext].get(), rv);
 }
 
 void
 WebGLContext::ClearScreen()
 {
-    bool colorAttachmentsMask[WebGLContext::sMaxColorAttachments] = {false};
+    bool colorAttachmentsMask[WebGLContext::kMaxColorAttachments] = {false};
 
     MakeContextCurrent();
     ScopedBindFramebuffer autoFB(gl, 0);
 
     GLbitfield clearMask = LOCAL_GL_COLOR_BUFFER_BIT;
     if (mOptions.depth)
         clearMask |= LOCAL_GL_DEPTH_BUFFER_BIT;
     if (mOptions.stencil)
@@ -1205,26 +1205,26 @@ WebGLContext::ClearScreen()
 #ifdef DEBUG
 // For NaNs, etc.
 static bool IsSameFloat(float a, float b) {
     return (a == b) || (IsNaN(a) && IsNaN(b));
 }
 #endif
 
 void
-WebGLContext::ForceClearFramebufferWithDefaultValues(GLbitfield mask, const bool colorAttachmentsMask[sMaxColorAttachments])
+WebGLContext::ForceClearFramebufferWithDefaultValues(GLbitfield mask, const bool colorAttachmentsMask[kMaxColorAttachments])
 {
     MakeContextCurrent();
 
     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(WEBGL_draw_buffers);
 
-    GLenum currentDrawBuffers[WebGLContext::sMaxColorAttachments];
+    GLenum currentDrawBuffers[WebGLContext::kMaxColorAttachments];
 
     // Fun GL fact: No need to worry about the viewport here, glViewport is just
     // setting up a coordinates transformation, it doesn't affect glClear at all.
 
 #ifdef DEBUG
     // Scope to hide our variables.
     {
         // Sanity-check that all our state is set properly. Otherwise, when we
@@ -1282,17 +1282,19 @@ WebGLContext::ForceClearFramebufferWithD
 
     // Prepare GL state for clearing.
     gl->fDisable(LOCAL_GL_SCISSOR_TEST);
 
     if (initializeColorBuffer) {
 
         if (drawBuffersIsEnabled) {
 
-            GLenum drawBuffersCommand[WebGLContext::sMaxColorAttachments] = { LOCAL_GL_NONE };
+            MOZ_ASSERT(size_t(mGLMaxDrawBuffers) <= WebGLContext::kMaxColorAttachments);
+
+            GLenum drawBuffersCommand[WebGLContext::kMaxColorAttachments] = { LOCAL_GL_NONE };
 
             for(int32_t i = 0; i < mGLMaxDrawBuffers; i++) {
                 GLint temp;
                 gl->fGetIntegerv(LOCAL_GL_DRAW_BUFFER0 + i, &temp);
                 currentDrawBuffers[i] = temp;
 
                 if (colorAttachmentsMask[i]) {
                     drawBuffersCommand[i] = LOCAL_GL_COLOR_ATTACHMENT0 + i;
--- a/content/canvas/src/WebGLContext.h
+++ b/content/canvas/src/WebGLContext.h
@@ -228,23 +228,23 @@ public:
     bool PresentScreenBuffer();
 
     // a number that increments every time we have an event that causes
     // all context resources to be lost.
     uint32_t Generation() { return mGeneration.value(); }
 
     const WebGLRectangleObject *FramebufferRectangleObject() const;
 
-    static const size_t sMaxColorAttachments = 16;
+    static const size_t kMaxColorAttachments = 16;
 
     // This is similar to GLContext::ClearSafely, but tries to minimize the
     // amount of work it does.
     // It only clears the buffers we specify, and can reset its state without
     // first having to query anything, as WebGL knows its state at all times.
-    void ForceClearFramebufferWithDefaultValues(GLbitfield mask, const bool colorAttachmentsMask[sMaxColorAttachments]);
+    void ForceClearFramebufferWithDefaultValues(GLbitfield mask, const bool colorAttachmentsMask[kMaxColorAttachments]);
 
     // Calls ForceClearFramebufferWithDefaultValues() for the Context's 'screen'.
     void ClearScreen();
 
     // checks for GL errors, clears any pending GL error, stores the current GL error in currentGLError,
     // and copies it into mWebGLError if it doesn't already have an error set
     void UpdateWebGLErrorAndClearGLError(GLenum *currentGLError) {
         // get and clear GL error in ALL cases
--- a/content/canvas/src/WebGLExtensionDrawBuffers.cpp
+++ b/content/canvas/src/WebGLExtensionDrawBuffers.cpp
@@ -15,25 +15,27 @@ using namespace mozilla;
 using namespace gl;
 
 WebGLExtensionDrawBuffers::WebGLExtensionDrawBuffers(WebGLContext* context)
     : WebGLExtensionBase(context)
 {
     GLint maxColorAttachments = 0;
     GLint maxDrawBuffers = 0;
 
-    gl::GLContext* gl = context->GL();
+    MOZ_ASSERT(IsSupported(context), "should not construct WebGLExtensionDrawBuffers : EXT_draw_buffers is not supported");
+
+    GLContext* gl = context->GL();
 
     context->MakeContextCurrent();
 
     gl->fGetIntegerv(LOCAL_GL_MAX_COLOR_ATTACHMENTS, &maxColorAttachments);
     gl->fGetIntegerv(LOCAL_GL_MAX_DRAW_BUFFERS, &maxDrawBuffers);
 
     // WEBGL_draw_buffers specifications don't give a maximal value reachable by MAX_COLOR_ATTACHMENTS.
-    maxColorAttachments = std::min(maxColorAttachments, GLint(WebGLContext::sMaxColorAttachments));
+    maxColorAttachments = std::min(maxColorAttachments, GLint(WebGLContext::kMaxColorAttachments));
 
     if (context->MinCapabilityMode())
     {
         maxColorAttachments = std::min(maxColorAttachments, GLint(sMinColorAttachments));
     }
 
     // WEBGL_draw_buffers specifications request MAX_COLOR_ATTACHMENTS >= MAX_DRAW_BUFFERS.
     maxDrawBuffers = std::min(maxDrawBuffers, GLint(maxColorAttachments));
@@ -49,17 +51,17 @@ WebGLExtensionDrawBuffers::~WebGLExtensi
 void WebGLExtensionDrawBuffers::DrawBuffersWEBGL(const dom::Sequence<GLenum>& buffers)
 {
     const size_t buffersLength = buffers.Length();
 
     if (buffersLength == 0) {
         return mContext->ErrorInvalidValue("drawBuffersWEBGL: invalid <buffers> (buffers must not be empty)");
     }
 
-    if (mContext->mBoundFramebuffer == 0)
+    if (!mContext->mBoundFramebuffer)
     {
         // OK: we are rendering in the default framebuffer
 
         /* EXT_draw_buffers :
          If the GL is bound to the default framebuffer, then <buffersLength> must be 1
          and the constant must be BACK or NONE. When draw buffer zero is
          BACK, color values are written into the sole buffer for single-
          buffered contexts, or into the back buffer for double-buffered
@@ -68,23 +70,23 @@ void WebGLExtensionDrawBuffers::DrawBuff
          */
         if (buffersLength != 1) {
             return mContext->ErrorInvalidValue("drawBuffersWEBGL: invalid <buffers> (main framebuffer: buffers.length must be 1)");
         }
 
         mContext->MakeContextCurrent();
 
         if (buffers[0] == LOCAL_GL_NONE) {
-            const GLenum drawBufffersCommand = LOCAL_GL_NONE;
-            mContext->GL()->fDrawBuffers(1, &drawBufffersCommand);
+            const GLenum drawBuffersCommand = LOCAL_GL_NONE;
+            mContext->GL()->fDrawBuffers(1, &drawBuffersCommand);
             return;
         }
         else if (buffers[0] == LOCAL_GL_BACK) {
-            const GLenum drawBufffersCommand = LOCAL_GL_COLOR_ATTACHMENT0;
-            mContext->GL()->fDrawBuffers(1, &drawBufffersCommand);
+            const GLenum drawBuffersCommand = LOCAL_GL_COLOR_ATTACHMENT0;
+            mContext->GL()->fDrawBuffers(1, &drawBuffersCommand);
             return;
         }
         return mContext->ErrorInvalidOperation("drawBuffersWEBGL: invalid operation (main framebuffer: buffers[0] must be GL_NONE or GL_BACK)");
     }
 
     // OK: we are rendering in a framebuffer object
 
     if (buffersLength > size_t(mContext->mGLMaxDrawBuffers)) {
--- a/content/canvas/src/WebGLFramebuffer.cpp
+++ b/content/canvas/src/WebGLFramebuffer.cpp
@@ -106,17 +106,17 @@ WebGLFramebuffer::Attachment::IsComplete
 
         if (mAttachmentPoint == LOCAL_GL_DEPTH_ATTACHMENT) {
             return format == LOCAL_GL_DEPTH_COMPONENT;
         }
         else if (mAttachmentPoint == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) {
             return format == LOCAL_GL_DEPTH_STENCIL;
         }
         else if (mAttachmentPoint >= LOCAL_GL_COLOR_ATTACHMENT0 &&
-                 mAttachmentPoint < WebGLenum(LOCAL_GL_COLOR_ATTACHMENT0 + WebGLContext::sMaxColorAttachments)) {
+                 mAttachmentPoint < WebGLenum(LOCAL_GL_COLOR_ATTACHMENT0 + WebGLContext::kMaxColorAttachments)) {
             return (format == LOCAL_GL_ALPHA ||
                     format == LOCAL_GL_LUMINANCE ||
                     format == LOCAL_GL_LUMINANCE_ALPHA ||
                     format == LOCAL_GL_RGB ||
                     format == LOCAL_GL_RGBA);
         }
         MOZ_CRASH("Invalid WebGL attachment poin?");
     }
@@ -324,20 +324,17 @@ const WebGLFramebuffer::Attachment&
 WebGLFramebuffer::GetAttachment(WebGLenum attachment) const {
     if (attachment == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT)
         return mDepthStencilAttachment;
     if (attachment == LOCAL_GL_DEPTH_ATTACHMENT)
         return mDepthAttachment;
     if (attachment == LOCAL_GL_STENCIL_ATTACHMENT)
         return mStencilAttachment;
 
-    if (!CheckColorAttachementNumber(attachment, "getAttachment")) {
-        NS_ABORT();
-        return mColorAttachments[0];
-    }
+    MOZ_ASSERT(CheckColorAttachementNumber(attachment, "getAttachment"));
 
     uint32_t colorAttachmentId = uint32_t(attachment - LOCAL_GL_COLOR_ATTACHMENT0);
 
     if (colorAttachmentId >= mColorAttachments.Length()) {
         NS_ABORT();
         return mColorAttachments[0];
     }
 
@@ -421,19 +418,19 @@ WebGLFramebuffer::CheckAndInitializeRend
 
     mContext->MakeContextCurrent();
 
     WebGLenum status = mContext->CheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER);
     if (status != LOCAL_GL_FRAMEBUFFER_COMPLETE)
         return false;
 
     uint32_t mask = 0;
-    bool colorAttachmentsMask[WebGLContext::sMaxColorAttachments] = { false };
+    bool colorAttachmentsMask[WebGLContext::kMaxColorAttachments] = { false };
 
-    MOZ_ASSERT( colorAttachmentCount <= WebGLContext::sMaxColorAttachments );
+    MOZ_ASSERT( colorAttachmentCount <= WebGLContext::kMaxColorAttachments );
 
     for (size_t i = 0; i < colorAttachmentCount; i++)
     {
         colorAttachmentsMask[i] = mColorAttachments[i].HasUninitializedRenderbuffer();
 
         if (colorAttachmentsMask[i]) {
             mask |= LOCAL_GL_COLOR_BUFFER_BIT;
         }
@@ -506,17 +503,17 @@ bool WebGLFramebuffer::CheckColorAttache
 
 void WebGLFramebuffer::EnsureColorAttachments(size_t colorAttachmentId) {
     size_t currentAttachmentCount = mColorAttachments.Length();
 
     if (mColorAttachments.Length() > colorAttachmentId) {
         return;
     }
 
-    MOZ_ASSERT( colorAttachmentId < WebGLContext::sMaxColorAttachments );
+    MOZ_ASSERT( colorAttachmentId < WebGLContext::kMaxColorAttachments );
 
     mColorAttachments.SetLength(colorAttachmentId + 1);
 
     for (size_t i = colorAttachmentId; i >= currentAttachmentCount; i--) {
         mColorAttachments[i].mAttachmentPoint = LOCAL_GL_COLOR_ATTACHMENT0 + i;
     }
 }
 
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -310,17 +310,17 @@ GLContext::InitWithPrefix(const char *pr
                 { (PRFuncPtr*) &mSymbols.fBeginQuery, { "BeginQuery", NULL } },
                 { (PRFuncPtr*) &mSymbols.fGetQueryObjectuiv, { "GetQueryObjectuiv", NULL } },
                 { (PRFuncPtr*) &mSymbols.fGenQueries, { "GenQueries", NULL } },
                 { (PRFuncPtr*) &mSymbols.fDeleteQueries, { "DeleteQueries", NULL } },
                 { (PRFuncPtr*) &mSymbols.fGetQueryiv, { "GetQueryiv", NULL } },
                 { (PRFuncPtr*) &mSymbols.fGetQueryObjectiv, { "GetQueryObjectiv", NULL } },
                 { (PRFuncPtr*) &mSymbols.fEndQuery, { "EndQuery", NULL } },
                 { (PRFuncPtr*) &mSymbols.fDrawBuffer, { "DrawBuffer", NULL } },
-                { (PRFuncPtr*) &mSymbols.fDrawBuffers, { "DrawBuffers", NULL } },
+                { (PRFuncPtr*) &mSymbols.fDrawBuffers, { "DrawBuffers", "DrawBuffersARB", NULL } },
                 { NULL, { NULL } },
             };
 
             if (!LoadSymbols(&symbols_desktop[0], trygl, prefix)) {
                 NS_ERROR("Desktop symbols failed to load.");
                 mInitialized = false;
             }
         }
@@ -594,16 +594,30 @@ GLContext::InitWithPrefix(const char *pr
                 MarkExtensionUnsupported(APPLE_vertex_array_object);
                 mSymbols.fIsVertexArray = nullptr;
                 mSymbols.fGenVertexArrays = nullptr;
                 mSymbols.fBindVertexArray = nullptr;
                 mSymbols.fDeleteVertexArrays = nullptr;
             }
         }
 
+        if (mIsGLES2 && IsExtensionSupported(EXT_draw_buffers)) {
+            SymLoadStruct vaoSymbols[] = {
+                { (PRFuncPtr*) &mSymbols.fDrawBuffers, { "DrawBuffersEXT", nullptr } },
+                { nullptr, { nullptr } },
+            };
+
+            if (!LoadSymbols(vaoSymbols, trygl, prefix)) {
+                NS_ERROR("GL ES supports EXT_draw_buffers without supplying its function.");
+
+                MarkExtensionUnsupported(EXT_draw_buffers);
+                mSymbols.fDrawBuffers = nullptr;
+            }
+        }
+
         // Load developer symbols, don't fail if we can't find them.
         SymLoadStruct auxSymbols[] = {
                 { (PRFuncPtr*) &mSymbols.fGetTexImage, { "GetTexImage", nullptr } },
                 { (PRFuncPtr*) &mSymbols.fGetTexLevelParameteriv, { "GetTexLevelParameteriv", nullptr } },
                 { nullptr, { nullptr } },
         };
         bool warnOnFailures = DebugMode();
         LoadSymbols(&auxSymbols[0], trygl, prefix, warnOnFailures);