Bug 996266 - Support testing for errors at the GLContext level. - r=kamidphish
☠☠ backed out by 365cdbcaaf6a ☠ ☠
authorJeff Gilbert <jgilbert@mozilla.com>
Wed, 27 Aug 2014 01:31:14 -0700
changeset 223556 c97ad94c5220533ebea774b5c67b9187527f3a6c
parent 223555 6bcda9f223b3e832c374c67f43168e863eeafc30
child 223557 2984228b8fb120f57d91ef79fe5a39f4e70f4531
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskamidphish
bugs996266
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 996266 - Support testing for errors at the GLContext level. - r=kamidphish
gfx/gl/GLBlitHelper.cpp
gfx/gl/GLContext.cpp
gfx/gl/GLContext.h
gfx/gl/GLScreenBuffer.cpp
gfx/gl/SharedSurfaceANGLE.cpp
gfx/gl/SharedSurfaceGL.cpp
--- a/gfx/gl/GLBlitHelper.cpp
+++ b/gfx/gl/GLBlitHelper.cpp
@@ -33,29 +33,34 @@ RenderbufferStorageBySamples(GLContext* 
                                              aSize.width, aSize.height);
     } else {
         aGL->fRenderbufferStorage(LOCAL_GL_RENDERBUFFER,
                                   aInternalFormat,
                                   aSize.width, aSize.height);
     }
 }
 
-
 GLuint
 CreateTexture(GLContext* aGL, GLenum aInternalFormat, GLenum aFormat,
               GLenum aType, const gfx::IntSize& aSize, bool linear)
 {
     GLuint tex = 0;
     aGL->fGenTextures(1, &tex);
     ScopedBindTexture autoTex(aGL, tex);
 
-    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MIN_FILTER, linear ? LOCAL_GL_LINEAR : LOCAL_GL_NEAREST);
-    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MAG_FILTER, linear ? LOCAL_GL_LINEAR : LOCAL_GL_NEAREST);
-    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_WRAP_S, LOCAL_GL_CLAMP_TO_EDGE);
-    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_WRAP_T, LOCAL_GL_CLAMP_TO_EDGE);
+    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D,
+                        LOCAL_GL_TEXTURE_MIN_FILTER, linear ? LOCAL_GL_LINEAR
+                                                            : LOCAL_GL_NEAREST);
+    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D,
+                        LOCAL_GL_TEXTURE_MAG_FILTER, linear ? LOCAL_GL_LINEAR
+                                                            : LOCAL_GL_NEAREST);
+    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_WRAP_S,
+                        LOCAL_GL_CLAMP_TO_EDGE);
+    aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_WRAP_T,
+                        LOCAL_GL_CLAMP_TO_EDGE);
 
     aGL->fTexImage2D(LOCAL_GL_TEXTURE_2D,
                      0,
                      aInternalFormat,
                      aSize.width, aSize.height,
                      0,
                      aFormat,
                      aType,
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -272,17 +272,17 @@ GLContext::GLContext(const SurfaceCaps& 
     mIsOffscreen(isOffscreen),
     mContextLost(false),
     mVersion(0),
     mProfile(ContextProfile::Unknown),
     mVendor(GLVendor::Other),
     mRenderer(GLRenderer::Other),
     mHasRobustness(false),
 #ifdef DEBUG
-    mGLError(LOCAL_GL_NO_ERROR),
+    mIsInLocalErrorCheck(false),
 #endif
     mSharedContext(sharedContext),
     mCaps(caps),
     mScreen(nullptr),
     mLockedSurface(nullptr),
     mMaxTextureSize(0),
     mMaxCubeMapTextureSize(0),
     mMaxTextureImageSize(0),
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -7,16 +7,17 @@
 #ifndef GLCONTEXT_H_
 #define GLCONTEXT_H_
 
 #include <stdio.h>
 #include <stdint.h>
 #include <ctype.h>
 #include <map>
 #include <bitset>
+#include <queue>
 
 #ifdef DEBUG
 #include <string.h>
 #endif
 
 #ifdef WIN32
 #include <windows.h>
 #endif
@@ -500,38 +501,33 @@ private:
     /**
      * Is this feature supported using the core (unsuffixed) symbols?
      */
     bool IsFeatureProvidedByCoreSymbols(GLFeature feature);
 
 // -----------------------------------------------------------------------------
 // Robustness handling
 public:
-
     bool HasRobustness() const {
         return mHasRobustness;
     }
 
     /**
      * The derived class is expected to provide information on whether or not it
      * supports robustness.
      */
     virtual bool SupportsRobustness() const = 0;
 
-
 private:
     bool mHasRobustness;
 
-
 // -----------------------------------------------------------------------------
 // Error handling
 public:
-
-    static const char* GLErrorToString(GLenum aError)
-    {
+    static const char* GLErrorToString(GLenum aError) {
         switch (aError) {
             case LOCAL_GL_INVALID_ENUM:
                 return "GL_INVALID_ENUM";
             case LOCAL_GL_INVALID_VALUE:
                 return "GL_INVALID_VALUE";
             case LOCAL_GL_INVALID_OPERATION:
                 return "GL_INVALID_OPERATION";
             case LOCAL_GL_STACK_OVERFLOW:
@@ -544,57 +540,124 @@ public:
                 return "GL_TABLE_TOO_LARGE";
             case LOCAL_GL_INVALID_FRAMEBUFFER_OPERATION:
                 return "GL_INVALID_FRAMEBUFFER_OPERATION";
             default:
                 return "";
         }
     }
 
-
     /** \returns the first GL error, and guarantees that all GL error flags are cleared,
      * i.e. that a subsequent GetError call will return NO_ERROR
      */
-    GLenum GetAndClearError()
-    {
+    GLenum GetAndClearError() {
         // the first error is what we want to return
         GLenum error = fGetError();
 
         if (error) {
             // clear all pending errors
             while(fGetError()) {}
         }
 
         return error;
     }
 
-
-    /*** In GL debug mode, we completely override glGetError ***/
-
-    GLenum fGetError()
-    {
-#ifdef DEBUG
-        // debug mode ends up eating the error in AFTER_GL_CALL
-        if (DebugMode()) {
-            GLenum err = mGLError;
-            mGLError = LOCAL_GL_NO_ERROR;
+private:
+    GLenum raw_fGetError() {
+        return mSymbols.fGetError();
+    }
+
+    std::queue<GLenum> mGLErrorQueue;
+
+public:
+    GLenum fGetError() {
+        if (!mGLErrorQueue.empty()) {
+            GLenum err = mGLErrorQueue.front();
+            mGLErrorQueue.pop();
             return err;
         }
-#endif // DEBUG
-
-        return mSymbols.fGetError();
+
+        return GetUnpushedError();
+    }
+
+private:
+    GLenum GetUnpushedError() {
+        return raw_fGetError();
+    }
+
+    void ClearUnpushedErrors() {
+        while (GetUnpushedError()) {
+            // Discard errors.
+        }
+    }
+
+    GLenum GetAndClearUnpushedErrors() {
+        GLenum err = GetUnpushedError();
+        if (err) {
+            ClearUnpushedErrors();
+        }
+        return err;
+    }
+
+    void PushError(GLenum err) {
+        mGLErrorQueue.push(err);
+    }
+
+    void GetAndPushAllErrors() {
+        while (true) {
+            GLenum err = GetUnpushedError();
+            if (!err)
+                break;
+
+            PushError(err);
+        }
     }
 
-
-#ifdef DEBUG
+    ////////////////////////////////////
+    // Use this safer option.
 private:
-
-    GLenum mGLError;
-#endif // DEBUG
-
+#ifdef DEBUG
+    bool mIsInLocalErrorCheck;
+#endif
+
+public:
+    class ScopedLocalErrorCheck {
+        GLContext* const mGL;
+        bool mHasBeenChecked;
+
+    public:
+        ScopedLocalErrorCheck(GLContext* gl)
+            : mGL(gl)
+            , mHasBeenChecked(false)
+        {
+#ifdef DEBUG
+            MOZ_ASSERT(!mGL->mIsInLocalErrorCheck);
+            mGL->mIsInLocalErrorCheck = true;
+#endif
+            mGL->GetAndPushAllErrors();
+        }
+
+        GLenum GetLocalError() {
+#ifdef DEBUG
+            MOZ_ASSERT(mGL->mIsInLocalErrorCheck);
+            mGL->mIsInLocalErrorCheck = false;
+#endif
+
+            MOZ_ASSERT(!mHasBeenChecked);
+            mHasBeenChecked = true;
+
+            return mGL->GetAndClearUnpushedErrors();
+        }
+
+        ~ScopedLocalErrorCheck() {
+            MOZ_ASSERT(mHasBeenChecked);
+        }
+    };
+
+private:
     static void GLAPIENTRY StaticDebugCallback(GLenum source,
                                                GLenum type,
                                                GLuint id,
                                                GLenum severity,
                                                GLsizei length,
                                                const GLchar* message,
                                                const GLvoid* userParam);
     void DebugCallback(GLenum source,
@@ -619,18 +682,17 @@ private:
 #  define MOZ_FUNCTION_NAME __PRETTY_FUNCTION__
 # elif defined(_MSC_VER)
 #  define MOZ_FUNCTION_NAME __FUNCTION__
 # else
 #  define MOZ_FUNCTION_NAME __func__  // defined in C99, supported in various C++ compilers. Just raw function name.
 # endif
 #endif
 
-    void BeforeGLCall(const char* glFunction)
-    {
+    void BeforeGLCall(const char* glFunction) {
         MOZ_ASSERT(IsCurrent());
         if (DebugMode()) {
             GLContext *currentGLContext = nullptr;
 
             currentGLContext = (GLContext*)PR_GetThreadPrivate(sCurrentGLContextTLS);
 
             if (DebugMode() & DebugTrace)
                 printf_stderr("[gl:%p] > %s\n", this, glFunction);
@@ -638,31 +700,33 @@ private:
                 printf_stderr("Fatal: %s called on non-current context %p. "
                               "The current context for this thread is %p.\n",
                               glFunction, this, currentGLContext);
                 NS_ABORT();
             }
         }
     }
 
-    void AfterGLCall(const char* glFunction)
-    {
+    void AfterGLCall(const char* glFunction) {
         if (DebugMode()) {
             // calling fFinish() immediately after every GL call makes sure that if this GL command crashes,
             // the stack trace will actually point to it. Otherwise, OpenGL being an asynchronous API, stack traces
             // tend to be meaningless
             mSymbols.fFinish();
-            mGLError = mSymbols.fGetError();
+            GLenum err = GetUnpushedError();
+            PushError(err);
+
             if (DebugMode() & DebugTrace)
-                printf_stderr("[gl:%p] < %s [0x%04x]\n", this, glFunction, mGLError);
-            if (mGLError != LOCAL_GL_NO_ERROR) {
+                printf_stderr("[gl:%p] < %s [0x%04x]\n", this, glFunction, err);
+
+            if (err != LOCAL_GL_NO_ERROR) {
                 printf_stderr("GL ERROR: %s generated GL error %s(0x%04x)\n",
                               glFunction,
-                              GLErrorToString(mGLError),
-                              mGLError);
+                              GLErrorToString(err),
+                              err);
                 if (DebugMode() & DebugAbortOnError)
                     NS_ABORT();
             }
         }
     }
 
     GLContext *TrackingContext()
     {
--- a/gfx/gl/GLScreenBuffer.cpp
+++ b/gfx/gl/GLScreenBuffer.cpp
@@ -563,27 +563,30 @@ DrawBuffer::Create(GLContext* const gl,
     } else {
         if (!formats.depth)
             pDepthRB = nullptr;
 
         if (!formats.stencil)
             pStencilRB = nullptr;
     }
 
+    GLContext::ScopedLocalErrorCheck localError(gl);
+
     CreateRenderbuffersForOffscreen(gl, formats, size, caps.antialias,
                                     pColorMSRB, pDepthRB, pStencilRB);
 
     GLuint fb = 0;
     gl->fGenFramebuffers(1, &fb);
     gl->AttachBuffersToFB(0, colorMSRB, depthRB, stencilRB, fb);
 
     UniquePtr<DrawBuffer> ret( new DrawBuffer(gl, size, fb, colorMSRB,
                                               depthRB, stencilRB) );
 
-    if (!gl->IsFramebufferComplete(fb))
+    GLenum err = localError.GetLocalError();
+    if (err || !gl->IsFramebufferComplete(fb))
         return false;
 
     *out_buffer = Move(ret);
     return true;
 }
 
 DrawBuffer::~DrawBuffer()
 {
@@ -619,16 +622,18 @@ ReadBuffer::Create(GLContext* gl,
     }
 
     GLuint depthRB = 0;
     GLuint stencilRB = 0;
 
     GLuint* pDepthRB   = caps.depth   ? &depthRB   : nullptr;
     GLuint* pStencilRB = caps.stencil ? &stencilRB : nullptr;
 
+    GLContext::ScopedLocalErrorCheck localError(gl);
+
     CreateRenderbuffersForOffscreen(gl, formats, surf->mSize, caps.antialias,
                                     nullptr, pDepthRB, pStencilRB);
 
     GLuint colorTex = 0;
     GLuint colorRB = 0;
     GLenum target = 0;
 
     switch (surf->mAttachType) {
@@ -646,17 +651,19 @@ ReadBuffer::Create(GLContext* gl,
 
     GLuint fb = 0;
     gl->fGenFramebuffers(1, &fb);
     gl->AttachBuffersToFB(colorTex, colorRB, depthRB, stencilRB, fb, target);
     gl->mFBOMapping[fb] = surf;
 
     UniquePtr<ReadBuffer> ret( new ReadBuffer(gl, fb, depthRB,
                                               stencilRB, surf) );
-    if (!gl->IsFramebufferComplete(fb)) {
+
+    GLenum err = localError.GetLocalError();
+    if (err || !gl->IsFramebufferComplete(fb)) {
         ret = nullptr;
     }
 
     return Move(ret);
 }
 
 ReadBuffer::~ReadBuffer()
 {
--- a/gfx/gl/SharedSurfaceANGLE.cpp
+++ b/gfx/gl/SharedSurfaceANGLE.cpp
@@ -139,30 +139,38 @@ ChooseConfig(GLContext* gl,
 
     if (gl->DebugMode()) {
         egl->DumpEGLConfig(config);
     }
 
     return config;
 }
 
-// Returns EGL_NO_SURFACE on error.
+// Returns `EGL_NO_SURFACE` (`0`) on error.
 static EGLSurface
 CreatePBufferSurface(GLLibraryEGL* egl,
                      EGLDisplay display,
                      EGLConfig config,
                      const gfx::IntSize& size)
 {
+    auto width = size.width;
+    auto height = size.height;
+
     EGLint attribs[] = {
-        LOCAL_EGL_WIDTH, size.width,
-        LOCAL_EGL_HEIGHT, size.height,
+        LOCAL_EGL_WIDTH, width,
+        LOCAL_EGL_HEIGHT, height,
         LOCAL_EGL_NONE
     };
 
+    DebugOnly<EGLint> preCallErr = egl->fGetError();
+    MOZ_ASSERT(preCallErr == LOCAL_EGL_SUCCESS);
     EGLSurface surface = egl->fCreatePbufferSurface(display, config, attribs);
+    EGLint err = egl->fGetError();
+    if (err != LOCAL_EGL_SUCCESS)
+        return 0;
 
     return surface;
 }
 
 /*static*/ UniquePtr<SharedSurface_ANGLEShareHandle>
 SharedSurface_ANGLEShareHandle::Create(GLContext* gl,
                                        EGLContext context, EGLConfig config,
                                        const gfx::IntSize& size, bool hasAlpha)
--- a/gfx/gl/SharedSurfaceGL.cpp
+++ b/gfx/gl/SharedSurfaceGL.cpp
@@ -19,20 +19,27 @@ using gfx::SurfaceFormat;
 
 /*static*/ UniquePtr<SharedSurface_Basic>
 SharedSurface_Basic::Create(GLContext* gl,
                             const GLFormats& formats,
                             const IntSize& size,
                             bool hasAlpha)
 {
     gl->MakeCurrent();
+
+    GLContext::ScopedLocalErrorCheck localError(gl);
     GLuint tex = CreateTexture(gl, formats.color_texInternalFormat,
                                formats.color_texFormat,
                                formats.color_texType,
                                size);
+    GLenum err = localError.GetLocalError();
+    if (err) {
+        gl->fDeleteTextures(1, &tex);
+        return nullptr;
+    }
 
     SurfaceFormat format = SurfaceFormat::B8G8R8X8;
     switch (formats.color_texInternalFormat) {
     case LOCAL_GL_RGB:
     case LOCAL_GL_RGB8:
         if (formats.color_texType == LOCAL_GL_UNSIGNED_SHORT_5_6_5)
             format = SurfaceFormat::R5G6B5;
         else
@@ -69,21 +76,18 @@ SharedSurface_Basic::SharedSurface_Basic
 
     ScopedBindFramebuffer autoFB(mGL, mFB);
     mGL->fFramebufferTexture2D(LOCAL_GL_FRAMEBUFFER,
                               LOCAL_GL_COLOR_ATTACHMENT0,
                               LOCAL_GL_TEXTURE_2D,
                               mTex,
                               0);
 
-    GLenum status = mGL->fCheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER);
-    if (status != LOCAL_GL_FRAMEBUFFER_COMPLETE) {
-        mGL->fDeleteFramebuffers(1, &mFB);
-        mFB = 0;
-    }
+    DebugOnly<GLenum> status = mGL->fCheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER);
+    MOZ_ASSERT(status == LOCAL_GL_FRAMEBUFFER_COMPLETE);
 
     int32_t stride = gfx::GetAlignedStride<4>(size.width * BytesPerPixel(format));
     mData = gfx::Factory::CreateDataSourceSurfaceWithStride(size, format, stride);
 }
 
 SharedSurface_Basic::~SharedSurface_Basic()
 {
     if (!mGL->MakeCurrent())
@@ -118,24 +122,34 @@ SharedSurface_GLTexture::Create(GLContex
     MOZ_ASSERT(!consGL || prodGL->SharesWith(consGL));
 
     prodGL->MakeCurrent();
 
     GLuint tex = texture;
 
     bool ownsTex = false;
 
+    UniquePtr<SharedSurface_GLTexture> ret;
+
     if (!tex) {
+      GLContext::ScopedLocalErrorCheck localError(prodGL);
+
       tex = CreateTextureForOffscreen(prodGL, formats, size);
+
+      GLenum err = localError.GetLocalError();
+      if (err) {
+          prodGL->fDeleteTextures(1, &tex);
+          return Move(ret);
+      }
+
       ownsTex = true;
     }
 
-    typedef SharedSurface_GLTexture ptrT;
-    UniquePtr<ptrT> ret( new ptrT(prodGL, consGL, size, hasAlpha, tex,
-                                  ownsTex) );
+    ret.reset( new SharedSurface_GLTexture(prodGL, consGL, size,
+                                           hasAlpha, tex, ownsTex) );
     return Move(ret);
 }
 
 SharedSurface_GLTexture::~SharedSurface_GLTexture()
 {
     if (!mGL->MakeCurrent())
         return;