Bug 978418 - Split GetError flags for WebGL. - r=kamidphish

content/canvas/src/WebGLContext.cpp

@@ -116,7 +116,6 @@ WebGLContext::WebGLContext()
     mOptionsFrozen = false;
 
     mActiveTexture = 0;
-    mWebGLError = LOCAL_GL_NO_ERROR;
     mPixelStoreFlipY = false;
     mPixelStorePremultiplyAlpha = false;
     mPixelStoreColorspaceConversion = BROWSER_DEFAULT_WEBGL;
@@ -1289,7 +1288,7 @@ WebGLContext::RobustnessTimerCallback(nsITimer* timer)
                                              true);
         // Set all flags back to the state they were in before the context was
         // lost.
-        mContextLostErrorSet = false;
+        mEmitContextLostErrorOnce = true;
         mAllowRestore = true;
     }
 

content/canvas/src/WebGLContext.h

@@ -244,10 +244,6 @@ public:
     // 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 (if not nullptr),
-    // and copies it into mWebGLError if it doesn't already have an error set
-    void UpdateWebGLErrorAndClearGLError(GLenum *currentGLError = nullptr);
-
     bool MinCapabilityMode() const { return mMinCapability; }
 
     void RobustnessTimerCallback(nsITimer* timer);
@@ -852,7 +848,12 @@ protected:
     void DeleteWebGLObjectsArray(nsTArray<WebGLObjectType>& array);
 
     GLuint mActiveTexture;
+
+    // glGetError sources:
+    bool mEmitContextLostErrorOnce;
     GLenum mWebGLError;
+    GLenum mUnderlyingGLError;
+    GLenum GetAndFlushUnderlyingGLErrors();
 
     // whether shader validation is supported
     bool mShaderValidation;

content/canvas/src/WebGLContextBuffers.cpp

@@ -485,10 +485,9 @@ WebGLContext::CheckedBufferData(GLenum target,
 
     bool sizeChanges = uint32_t(size) != boundBuffer->ByteLength();
     if (sizeChanges) {
-        UpdateWebGLErrorAndClearGLError();
+        GetAndFlushUnderlyingGLErrors();
         gl->fBufferData(target, size, data, usage);
-        GLenum error = LOCAL_GL_NO_ERROR;
-        UpdateWebGLErrorAndClearGLError(&error);
+        GLenum error = GetAndFlushUnderlyingGLErrors();
         return error;
     } else {
         gl->fBufferData(target, size, data, usage);

content/canvas/src/WebGLContextGL.cpp

@@ -538,13 +538,12 @@ WebGLContext::CopyTexImage2D(GLenum target,
     }
 
     if (sizeMayChange)
-        UpdateWebGLErrorAndClearGLError();
+        GetAndFlushUnderlyingGLErrors();
 
     CopyTexSubImage2D_base(target, level, internalformat, 0, 0, x, y, width, height, false);
 
     if (sizeMayChange) {
-        GLenum error = LOCAL_GL_NO_ERROR;
-        UpdateWebGLErrorAndClearGLError(&error);
+        GLenum error = GetAndFlushUnderlyingGLErrors();
         if (error) {
             GenerateWarning("copyTexImage2D generated error %s", ErrorName(error));
             return;
@@ -902,8 +901,7 @@ WebGLContext::DoFakeVertexAttrib0(GLuint vertexCount)
 
         gl->fBindBuffer(LOCAL_GL_ARRAY_BUFFER, mFakeVertexAttrib0BufferObject);
 
-        GLenum error = LOCAL_GL_NO_ERROR;
-        UpdateWebGLErrorAndClearGLError();
+        GetAndFlushUnderlyingGLErrors();
 
         if (mFakeVertexAttrib0BufferStatus == WebGLVertexAttrib0Status::EmulatedInitializedArray) {
             nsAutoArrayPtr<GLfloat> array(new GLfloat[4 * vertexCount]);
@@ -917,7 +915,7 @@ WebGLContext::DoFakeVertexAttrib0(GLuint vertexCount)
         } else {
             gl->fBufferData(LOCAL_GL_ARRAY_BUFFER, dataSize, nullptr, LOCAL_GL_DYNAMIC_DRAW);
         }
-        UpdateWebGLErrorAndClearGLError(&error);
+        GLenum error = GetAndFlushUnderlyingGLErrors();
 
         gl->fBindBuffer(LOCAL_GL_ARRAY_BUFFER, mBoundArrayBuffer ? mBoundArrayBuffer->GLName() : 0);
 
@@ -1499,19 +1497,53 @@ WebGLContext::CreateTexture()
     return globj.forget();
 }
 
+static GLenum
+GetAndClearError(GLenum* errorVar)
+{
+    MOZ_ASSERT(errorVar);
+    GLenum ret = *errorVar;
+    *errorVar = LOCAL_GL_NO_ERROR;
+    return ret;
+}
+
 GLenum
 WebGLContext::GetError()
 {
-    if (mContextStatus == ContextNotLost) {
-        MakeContextCurrent();
-        UpdateWebGLErrorAndClearGLError();
-    } else if (!mContextLostErrorSet) {
-        mWebGLError = LOCAL_GL_CONTEXT_LOST;
-        mContextLostErrorSet = true;
+    /* WebGL 1.0: Section 5.14.3: Setting and getting state:
+     *   If the context's webgl context lost flag is set, returns
+     *   CONTEXT_LOST_WEBGL the first time this method is called.
+     *   Afterward, returns NO_ERROR until the context has been
+     *   restored.
+     *
+     * WEBGL_lose_context:
+     *   [When this extension is enabled: ] loseContext and
+     *   restoreContext are allowed to generate INVALID_OPERATION errors
+     *   even when the context is lost.
+     */
+
+    if (IsContextLost()) {
+        if (mEmitContextLostErrorOnce) {
+            mEmitContextLostErrorOnce = false;
+            return LOCAL_GL_CONTEXT_LOST;
+        }
+        // Don't return yet, since WEBGL_lose_contexts contradicts the
+        // original spec, and allows error generation while lost.
     }
 
-    GLenum err = mWebGLError;
-    mWebGLError = LOCAL_GL_NO_ERROR;
+    GLenum err = GetAndClearError(&mWebGLError);
+    if (err != LOCAL_GL_NO_ERROR)
+        return err;
+
+    if (IsContextLost())
+        return LOCAL_GL_NO_ERROR;
+
+    // Either no WebGL-side error, or it's already been cleared.
+    // UnderlyingGL-side errors, now.
+
+    MakeContextCurrent();
+    GetAndFlushUnderlyingGLErrors();
+
+    err = GetAndClearError(&mUnderlyingGLError);
     return err;
 }
 
@@ -2496,10 +2528,9 @@ WebGLContext::RenderbufferStorage(GLenum target, GLenum internalformat, GLsizei
                        height != mBoundRenderbuffer->Height() ||
                        internalformat != mBoundRenderbuffer->InternalFormat();
     if (sizeChanges) {
-        UpdateWebGLErrorAndClearGLError();
+        GetAndFlushUnderlyingGLErrors();
         mBoundRenderbuffer->RenderbufferStorage(internalformatForGL, width, height);
-        GLenum error = LOCAL_GL_NO_ERROR;
-        UpdateWebGLErrorAndClearGLError(&error);
+        GLenum error = GetAndFlushUnderlyingGLErrors();
         if (error) {
             GenerateWarning("renderbufferStorage generated error %s", ErrorName(error));
             return;
@@ -3632,12 +3663,11 @@ GLenum WebGLContext::CheckedTexImage2D(GLenum target,
     }
 
     if (sizeMayChange) {
-        UpdateWebGLErrorAndClearGLError();
+        GetAndFlushUnderlyingGLErrors();
 
         gl->fTexImage2D(target, level, internalFormat, width, height, border, format, realType, data);
 
-        GLenum error = LOCAL_GL_NO_ERROR;
-        UpdateWebGLErrorAndClearGLError(&error);
+        GLenum error = GetAndFlushUnderlyingGLErrors();
         return error;
     }
 

content/canvas/src/WebGLContextUtils.cpp

@@ -106,15 +106,12 @@ WebGLContext::GetImageSize(GLsizei height,
 void
 WebGLContext::SynthesizeGLError(GLenum err)
 {
-    // If there is already a pending error, don't overwrite it;
-    // but if there isn't, then we need to check for a gl error
-    // that may have occurred before this one and use that code
-    // instead.
-
-    MakeContextCurrent();
-
-    UpdateWebGLErrorAndClearGLError();
-
+    /* ES2 section 2.5 "GL Errors" states that implementations can have
+     * multiple 'flags', as errors might be caught in different parts of
+     * a distributed implementation.
+     * We're signing up as a distributed implementation here, with
+     * separate flags for WebGL and the underlying GLContext.
+     */
     if (!mWebGLError)
         mWebGLError = err;
 }
@@ -234,14 +231,16 @@ WebGLContext::IsTextureFormatCompressed(GLenum format)
     }
 }
 
-void
-WebGLContext::UpdateWebGLErrorAndClearGLError(GLenum *currentGLError)
+GLenum
+WebGLContext::GetAndFlushUnderlyingGLErrors()
 {
-    // get and clear GL error in ALL cases
+    // Get and clear GL error in ALL cases.
     GLenum error = gl->GetAndClearError();
-    if (currentGLError)
-        *currentGLError = error;
-    // only store in mWebGLError if is hasn't already recorded an error
-    if (!mWebGLError)
-        mWebGLError = error;
+
+    // Only store in mUnderlyingGLError if is hasn't already recorded an
+    // error.
+    if (!mUnderlyingGLError)
+        mUnderlyingGLError = error;
+
+    return error;
 }

content/canvas/src/WebGLContextValidate.cpp

@@ -1586,7 +1586,9 @@ WebGLContext::InitAndValidateGL()
     }
 
     mActiveTexture = 0;
+    mEmitContextLostErrorOnce = true;
     mWebGLError = LOCAL_GL_NO_ERROR;
+    mUnderlyingGLError = LOCAL_GL_NO_ERROR;
 
     mBound2DTextures.Clear();
     mBoundCubeMapTextures.Clear();

content/canvas/src/WebGLTexture.cpp

@@ -451,13 +451,12 @@ WebGLTexture::DoDeferredImageInitialization(GLenum imageTarget, GLint level)
     void *zeros = calloc(1, checked_byteLength.value());
 
     GLenum format = WebGLTexelConversions::GLFormatForTexelFormat(texelformat);
-    mContext->UpdateWebGLErrorAndClearGLError();
+    mContext->GetAndFlushUnderlyingGLErrors();
     mContext->gl->fTexImage2D(imageTarget, level, imageInfo.mInternalFormat,
                               imageInfo.mWidth, imageInfo.mHeight,
                               0, format, imageInfo.mType,
                               zeros);
-    GLenum error = LOCAL_GL_NO_ERROR;
-    mContext->UpdateWebGLErrorAndClearGLError(&error);
+    GLenum error = mContext->GetAndFlushUnderlyingGLErrors();
 
     free(zeros);
     SetImageDataStatus(imageTarget, level, WebGLImageDataStatus::InitializedImageData);