Bug 1088345 - Improve glGetError handling. - r=kamidphish
authorJeff Gilbert <jgilbert@mozilla.com>
Fri, 24 Oct 2014 16:52:35 -0700
changeset 213041 5e5b3c89df165dae1ee53e682641ccd2599e1200
parent 213040 8ccb2bf14892d13c278580667b2a1670e5ec2b88
child 213042 931498bf7d4ff27c774a434ec5c55156d82a224a
push idunknown
push userunknown
push dateunknown
reviewerskamidphish
bugs1088345
milestone36.0a1
Bug 1088345 - Improve glGetError handling. - r=kamidphish
dom/canvas/WebGLContextUtils.cpp
dom/canvas/WebGLContextValidate.cpp
gfx/gl/GLContext.cpp
gfx/gl/GLContext.h
gfx/gl/GLScreenBuffer.cpp
gfx/gl/SharedSurfaceGL.cpp
gfx/layers/opengl/CompositorOGL.cpp
gfx/layers/opengl/TextureHostOGL.cpp
--- a/dom/canvas/WebGLContextUtils.cpp
+++ b/dom/canvas/WebGLContextUtils.cpp
@@ -669,17 +669,17 @@ WebGLContext::IsTextureFormatCompressed(
 {
     return IsCompressedTextureFormat(format.get());
 }
 
 GLenum
 WebGLContext::GetAndFlushUnderlyingGLErrors()
 {
     // Get and clear GL error in ALL cases.
-    GLenum error = gl->GetAndClearError();
+    GLenum error = gl->fGetError();
 
     // Only store in mUnderlyingGLError if is hasn't already recorded an
     // error.
     if (!mUnderlyingGLError)
         mUnderlyingGLError = error;
 
     return error;
 }
--- a/dom/canvas/WebGLContextValidate.cpp
+++ b/dom/canvas/WebGLContextValidate.cpp
@@ -1616,30 +1616,30 @@ WebGLContext::InitAndValidateGL()
             gl->fGetIntegerv(LOCAL_GL_MAX_VERTEX_UNIFORM_COMPONENTS, &mGLMaxVertexUniformVectors);
             mGLMaxVertexUniformVectors /= 4;
 
             // we are now going to try to read GL_MAX_VERTEX_OUTPUT_COMPONENTS and GL_MAX_FRAGMENT_INPUT_COMPONENTS,
             // however these constants only entered the OpenGL standard at OpenGL 3.2. So we will try reading,
             // and check OpenGL error for INVALID_ENUM.
 
             // before we start, we check that no error already occurred, to prevent hiding it in our subsequent error handling
-            error = gl->GetAndClearError();
+            error = gl->fGetError();
             if (error != LOCAL_GL_NO_ERROR) {
                 GenerateWarning("GL error 0x%x occurred during WebGL context initialization!", error);
                 return false;
             }
 
             // On the public_webgl list, "problematic GetParameter pnames" thread, the following formula was given:
             //   mGLMaxVaryingVectors = min (GL_MAX_VERTEX_OUTPUT_COMPONENTS, GL_MAX_FRAGMENT_INPUT_COMPONENTS) / 4
             GLint maxVertexOutputComponents,
                   minFragmentInputComponents;
             gl->fGetIntegerv(LOCAL_GL_MAX_VERTEX_OUTPUT_COMPONENTS, &maxVertexOutputComponents);
             gl->fGetIntegerv(LOCAL_GL_MAX_FRAGMENT_INPUT_COMPONENTS, &minFragmentInputComponents);
 
-            error = gl->GetAndClearError();
+            error = gl->fGetError();
             switch (error) {
                 case LOCAL_GL_NO_ERROR:
                     mGLMaxVaryingVectors = std::min(maxVertexOutputComponents, minFragmentInputComponents) / 4;
                     break;
                 case LOCAL_GL_INVALID_ENUM:
                     mGLMaxVaryingVectors = 16; // = 64/4, 64 is the min value for maxVertexOutputComponents in OpenGL 3.2 spec
                     break;
                 default:
@@ -1689,19 +1689,20 @@ WebGLContext::InitAndValidateGL()
             GenerateWarning("GLSL translator initialization failed!");
             return false;
         }
     }
 
     // Mesa can only be detected with the GL_VERSION string, of the form "2.1 Mesa 7.11.0"
     mIsMesa = strstr((const char *)(gl->fGetString(LOCAL_GL_VERSION)), "Mesa");
 
-    // notice that the point of calling GetAndClearError here is not only to check for error,
-    // it is also to reset the error flags so that a subsequent WebGL getError call will give the correct result.
-    error = gl->GetAndClearError();
+    // Notice that the point of calling fGetError here is not only to check for
+    // errors, but also to reset the error flags so that a subsequent WebGL
+    // getError call will give the correct result.
+    error = gl->fGetError();
     if (error != LOCAL_GL_NO_ERROR) {
         GenerateWarning("GL error 0x%x occurred during WebGL context initialization!", error);
         return false;
     }
 
     if (IsWebGL2() &&
         !InitWebGL2())
     {
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -287,19 +287,18 @@ GLContext::GLContext(const SurfaceCaps& 
   : mInitialized(false),
     mIsOffscreen(isOffscreen),
     mContextLost(false),
     mVersion(0),
     mProfile(ContextProfile::Unknown),
     mVendor(GLVendor::Other),
     mRenderer(GLRenderer::Other),
     mHasRobustness(false),
-#ifdef MOZ_GL_DEBUG
-    mIsInLocalErrorCheck(false),
-#endif
+    mTopError(LOCAL_GL_NO_ERROR),
+    mLocalErrorScope(nullptr),
     mSharedContext(sharedContext),
     mCaps(caps),
     mScreen(nullptr),
     mLockedSurface(nullptr),
     mMaxTextureSize(0),
     mMaxCubeMapTextureSize(0),
     mMaxTextureImageSize(0),
     mMaxRenderbufferSize(0),
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -575,120 +575,93 @@ 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() {
-        // the first error is what we want to return
-        GLenum error = fGetError();
-
-        if (error) {
-            // clear all pending errors
-            while(fGetError()) {}
-        }
-
-        return error;
-    }
-
 private:
-    GLenum raw_fGetError() {
+    GLenum mTopError;
+
+    GLenum RawGetError() {
         return mSymbols.fGetError();
     }
 
-    std::queue<GLenum> mGLErrorQueue;
+    GLenum RawGetErrorAndClear() {
+        GLenum err = RawGetError();
+
+        if (err)
+            while (RawGetError()) {}
+
+        return err;
+    }
 
 public:
+    GLenum FlushErrors() {
+        GLenum err = RawGetErrorAndClear();
+        if (!mTopError)
+            mTopError = err;
+        return err;
+    }
+
+    // We smash all errors together, so you never have to loop on this. We
+    // guarantee that immediately after this call, there are no errors left.
     GLenum fGetError() {
-        if (!mGLErrorQueue.empty()) {
-            GLenum err = mGLErrorQueue.front();
-            mGLErrorQueue.pop();
-            return err;
-        }
-
-        return GetUnpushedError();
-    }
-
-private:
-    GLenum GetUnpushedError() {
-        return raw_fGetError();
-    }
-
-    void ClearUnpushedErrors() {
-        while (GetUnpushedError()) {
-            // Discard errors.
-        }
-    }
-
-    GLenum GetAndClearUnpushedErrors() {
-        GLenum err = GetUnpushedError();
-        if (err) {
-            ClearUnpushedErrors();
-        }
+        FlushErrors();
+
+        GLenum err = mTopError;
+        mTopError = LOCAL_GL_NO_ERROR;
         return err;
     }
 
-    void PushError(GLenum err) {
-        mGLErrorQueue.push(err);
-    }
-
-    void GetAndPushAllErrors() {
-        while (true) {
-            GLenum err = GetUnpushedError();
-            if (!err)
-                break;
-
-            PushError(err);
-        }
-    }
-
     ////////////////////////////////////
     // Use this safer option.
+    class LocalErrorScope;
+
 private:
-#ifdef MOZ_GL_DEBUG
-    bool mIsInLocalErrorCheck;
-#endif
+    LocalErrorScope* mLocalErrorScope;
 
 public:
-    class ScopedLocalErrorCheck {
-        GLContext* const mGL;
+    class LocalErrorScope {
+        GLContext& mGL;
+        GLenum mOldTop;
         bool mHasBeenChecked;
 
     public:
-        explicit ScopedLocalErrorCheck(GLContext* gl)
+        explicit LocalErrorScope(GLContext& gl)
             : mGL(gl)
             , mHasBeenChecked(false)
         {
-#ifdef MOZ_GL_DEBUG
-            MOZ_ASSERT(!mGL->mIsInLocalErrorCheck);
-            mGL->mIsInLocalErrorCheck = true;
-#endif
-            mGL->GetAndPushAllErrors();
+            MOZ_ASSERT(!mGL.mLocalErrorScope);
+            mGL.mLocalErrorScope = this;
+
+            mGL.FlushErrors();
+
+            mOldTop = mGL.mTopError;
+            mGL.mTopError = LOCAL_GL_NO_ERROR;
         }
 
-        GLenum GetLocalError() {
-#ifdef MOZ_GL_DEBUG
-            MOZ_ASSERT(mGL->mIsInLocalErrorCheck);
-            mGL->mIsInLocalErrorCheck = false;
-#endif
-
+        GLenum GetError() {
             MOZ_ASSERT(!mHasBeenChecked);
             mHasBeenChecked = true;
 
-            return mGL->GetAndClearUnpushedErrors();
+            return mGL.fGetError();
         }
 
-        ~ScopedLocalErrorCheck() {
+        ~LocalErrorScope() {
             MOZ_ASSERT(mHasBeenChecked);
+
+            MOZ_ASSERT(mGL.fGetError() == LOCAL_GL_NO_ERROR);
+
+            mGL.mTopError = mOldTop;
+
+            MOZ_ASSERT(mGL.mLocalErrorScope == this);
+            mGL.mLocalErrorScope = nullptr;
         }
     };
 
 private:
     static void GLAPIENTRY StaticDebugCallback(GLenum source,
                                                GLenum type,
                                                GLuint id,
                                                GLenum severity,
@@ -717,53 +690,57 @@ 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* funcName) {
         MOZ_ASSERT(IsCurrent());
+
         if (DebugMode()) {
-            GLContext *currentGLContext = nullptr;
-
-            currentGLContext = (GLContext*)PR_GetThreadPrivate(sCurrentGLContextTLS);
+            FlushErrors();
 
             if (DebugMode() & DebugTrace)
-                printf_stderr("[gl:%p] > %s\n", this, glFunction);
-            if (this != currentGLContext) {
-                printf_stderr("Fatal: %s called on non-current context %p. "
-                              "The current context for this thread is %p.\n",
-                              glFunction, this, currentGLContext);
-                NS_ABORT();
+                printf_stderr("[gl:%p] > %s\n", this, funcName);
+
+            GLContext* tlsContext = (GLContext*)PR_GetThreadPrivate(sCurrentGLContextTLS);
+            if (this != tlsContext) {
+                printf_stderr("Fatal: %s called on non-current context %p. The"
+                              " current context for this thread is %p.\n",
+                              funcName, this, tlsContext);
+                MOZ_CRASH("GLContext is not current.");
             }
         }
     }
 
-    void AfterGLCall(const char* glFunction) {
+    void AfterGLCall(const char* funcName) {
         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();
-            GLenum err = GetUnpushedError();
-            PushError(err);
-
-            if (DebugMode() & DebugTrace)
-                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(err),
-                              err);
+            GLenum err = FlushErrors();
+
+            if (DebugMode() & DebugTrace) {
+                printf_stderr("[gl:%p] < %s [%s (0x%04x)]\n", this, funcName,
+                              GLErrorToString(err), err);
+            }
+
+            if (err != LOCAL_GL_NO_ERROR &&
+                !mLocalErrorScope)
+            {
+                printf_stderr("[gl:%p] %s: Generated unexpected %s error."
+                              " (0x%04x)\n", this, funcName,
+                              GLErrorToString(err), err);
+
                 if (DebugMode() & DebugAbortOnError)
-                    NS_ABORT();
+                    MOZ_CRASH("MOZ_GL_DEBUG_ABORT_ON_ERROR");
             }
         }
     }
 
     GLContext *TrackingContext()
     {
         GLContext *tip = this;
         while (tip->mSharedContext)
--- a/gfx/gl/GLScreenBuffer.cpp
+++ b/gfx/gl/GLScreenBuffer.cpp
@@ -595,29 +595,30 @@ DrawBuffer::Create(GLContext* const gl,
     } else {
         if (!formats.depth)
             pDepthRB = nullptr;
 
         if (!formats.stencil)
             pStencilRB = nullptr;
     }
 
-    GLContext::ScopedLocalErrorCheck localError(gl);
+    GLContext::LocalErrorScope 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) );
 
-    GLenum err = localError.GetLocalError();
+    GLenum err = localError.GetError();
+    MOZ_ASSERT_IF(err != LOCAL_GL_NO_ERROR, err == LOCAL_GL_OUT_OF_MEMORY);
     if (err || !gl->IsFramebufferComplete(fb))
         return false;
 
     *out_buffer = Move(ret);
     return true;
 }
 
 DrawBuffer::~DrawBuffer()
@@ -654,17 +655,17 @@ 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);
+    GLContext::LocalErrorScope localError(*gl);
 
     CreateRenderbuffersForOffscreen(gl, formats, surf->mSize, caps.antialias,
                                     nullptr, pDepthRB, pStencilRB);
 
     GLuint colorTex = 0;
     GLuint colorRB = 0;
     GLenum target = 0;
 
@@ -684,17 +685,18 @@ 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) );
 
-    GLenum err = localError.GetLocalError();
+    GLenum err = localError.GetError();
+    MOZ_ASSERT_IF(err != LOCAL_GL_NO_ERROR, err == LOCAL_GL_OUT_OF_MEMORY);
     if (err || !gl->IsFramebufferComplete(fb)) {
         ret = nullptr;
     }
 
     return Move(ret);
 }
 
 ReadBuffer::~ReadBuffer()
--- a/gfx/gl/SharedSurfaceGL.cpp
+++ b/gfx/gl/SharedSurfaceGL.cpp
@@ -21,22 +21,24 @@ using gfx::SurfaceFormat;
 SharedSurface_Basic::Create(GLContext* gl,
                             const GLFormats& formats,
                             const IntSize& size,
                             bool hasAlpha)
 {
     UniquePtr<SharedSurface_Basic> ret;
     gl->MakeCurrent();
 
-    GLContext::ScopedLocalErrorCheck localError(gl);
+    GLContext::LocalErrorScope localError(*gl);
     GLuint tex = CreateTexture(gl, formats.color_texInternalFormat,
                                formats.color_texFormat,
                                formats.color_texType,
                                size);
-    GLenum err = localError.GetLocalError();
+
+    GLenum err = localError.GetError();
+    MOZ_ASSERT_IF(err != LOCAL_GL_NO_ERROR, err == LOCAL_GL_OUT_OF_MEMORY);
     if (err) {
         gl->fDeleteTextures(1, &tex);
         return Move(ret);
     }
 
     SurfaceFormat format = SurfaceFormat::B8G8R8X8;
     switch (formats.color_texInternalFormat) {
     case LOCAL_GL_RGB:
@@ -116,27 +118,28 @@ SharedSurface_GLTexture::Create(GLContex
 
     GLuint tex = texture;
 
     bool ownsTex = false;
 
     UniquePtr<SharedSurface_GLTexture> ret;
 
     if (!tex) {
-      GLContext::ScopedLocalErrorCheck localError(prodGL);
+        GLContext::LocalErrorScope localError(*prodGL);
 
-      tex = CreateTextureForOffscreen(prodGL, formats, size);
+        tex = CreateTextureForOffscreen(prodGL, formats, size);
 
-      GLenum err = localError.GetLocalError();
-      if (err) {
-          prodGL->fDeleteTextures(1, &tex);
-          return Move(ret);
-      }
+        GLenum err = localError.GetError();
+        MOZ_ASSERT_IF(err != LOCAL_GL_NO_ERROR, err == LOCAL_GL_OUT_OF_MEMORY);
+        if (err) {
+            prodGL->fDeleteTextures(1, &tex);
+            return Move(ret);
+        }
 
-      ownsTex = true;
+        ownsTex = true;
     }
 
     ret.reset( new SharedSurface_GLTexture(prodGL, consGL, size,
                                            hasAlpha, tex, ownsTex) );
     return Move(ret);
 }
 
 SharedSurface_GLTexture::~SharedSurface_GLTexture()
--- a/gfx/layers/opengl/CompositorOGL.cpp
+++ b/gfx/layers/opengl/CompositorOGL.cpp
@@ -697,17 +697,17 @@ CompositorOGL::CreateFBOWithTexture(cons
                               0,
                               LOCAL_GL_RGBA,
                               clampedRect.width, clampedRect.height,
                               0,
                               LOCAL_GL_RGBA,
                               LOCAL_GL_UNSIGNED_BYTE,
                               buf);
     }
-    GLenum error = mGLContext->GetAndClearError();
+    GLenum error = mGLContext->fGetError();
     if (error != LOCAL_GL_NO_ERROR) {
       nsAutoCString msg;
       msg.AppendPrintf("Texture initialization failed! -- error 0x%x, Source %d, Source format %d,  RGBA Compat %d",
                        error, aSourceFrameBuffer, format, isFormatCompatibleWithRGBA);
       NS_ERROR(msg.get());
     }
   } else {
     mGLContext->fTexImage2D(mFBOTextureTarget,
--- a/gfx/layers/opengl/TextureHostOGL.cpp
+++ b/gfx/layers/opengl/TextureHostOGL.cpp
@@ -447,21 +447,21 @@ void
 SurfaceTextureSource::BindTexture(GLenum aTextureUnit, gfx::Filter aFilter)
 {
   if (!gl()) {
     NS_WARNING("Trying to bind a texture without a GLContext");
     return;
   }
 
   gl()->fActiveTexture(aTextureUnit);
-#ifndef DEBUG
+
   // SurfaceTexture spams us if there are any existing GL errors, so
   // we'll clear them here in order to avoid that.
-  gl()->GetAndClearError();
-#endif
+  gl()->FlushErrors();
+
   mSurfTex->UpdateTexImage();
 
   ApplyFilterToBoundTexture(gl(), aFilter, mTextureTarget);
 }
 
 void
 SurfaceTextureSource::SetCompositor(Compositor* aCompositor)
 {