Bug 967354 - Fix incorrect usage of UpdateWebGLErrorAndClearGLError(); r=jgilbert, a=1.2.x+
authorDan Glastonbury <dglastonbury@mozilla.com>
Thu, 29 May 2014 08:42:40 +1000
changeset 157143 8f952940d1c1
parent 157142 d2b20f54d7e5
child 157144 96a1ab166184
push id514
push userryanvm@gmail.com
push dateThu, 29 May 2014 00:42:44 +0000
reviewersjgilbert, 1
bugs967354
milestone26.0
Bug 967354 - Fix incorrect usage of UpdateWebGLErrorAndClearGLError(); r=jgilbert, a=1.2.x+
content/canvas/src/WebGLContext.cpp
content/canvas/src/WebGLContext.h
content/canvas/src/WebGLContextGL.cpp
--- a/content/canvas/src/WebGLContext.cpp
+++ b/content/canvas/src/WebGLContext.cpp
@@ -1153,24 +1153,26 @@ WebGLContext::PresentScreenBuffer()
         ClearScreen();
     }
 
     mShouldPresent = false;
 
     return true;
 }
 
-void
+bool
 WebGLContext::DummyFramebufferOperation(const char *info)
 {
     GLenum status = CheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER);
-    if (status == LOCAL_GL_FRAMEBUFFER_COMPLETE)
-        return;
-    else
-        return ErrorInvalidFramebufferOperation("%s: incomplete framebuffer", info);
+    if (status == LOCAL_GL_FRAMEBUFFER_COMPLETE) {
+        return true;
+    } else {
+        ErrorInvalidFramebufferOperation("%s: incomplete framebuffer", info);
+        return false;
+    }
 }
 
 // We use this timer for many things. Here are the things that it is activated for:
 // 1) If a script is using the MOZ_WEBGL_lose_context extension.
 // 2) If we are using EGL and _NOT ANGLE_, we query periodically to see if the
 //    CONTEXT_LOST_WEBGL error has been triggered.
 // 3) If we are using ANGLE, or anything that supports ARB_robustness, query the
 //    GPU periodically to see if the reset status bit has been set.
--- a/content/canvas/src/WebGLContext.h
+++ b/content/canvas/src/WebGLContext.h
@@ -191,17 +191,17 @@ public:
     void ErrorInvalidValue(const char *fmt = 0, ...);
     void ErrorInvalidFramebufferOperation(const char *fmt = 0, ...);
     void ErrorInvalidEnumInfo(const char *info, GLenum enumvalue);
     void ErrorOutOfMemory(const char *fmt = 0, ...);
 
     const char *ErrorName(GLenum error);
     bool IsTextureFormatCompressed(GLenum format);
 
-    void DummyFramebufferOperation(const char *info);
+    bool DummyFramebufferOperation(const char *info);
 
     WebGLTexture *activeBoundTextureForTarget(GLenum target) const {
         return target == LOCAL_GL_TEXTURE_2D ? mBound2DTextures[mActiveTexture]
                                              : mBoundCubeMapTextures[mActiveTexture];
     }
 
     already_AddRefed<CanvasLayer> GetCanvasLayer(nsDisplayListBuilder* aBuilder,
                                                  CanvasLayer *aOldLayer,
@@ -994,17 +994,17 @@ protected:
     {
       return SurfaceFromElement(&aElement);
     }
 
     nsresult SurfaceFromElementResultToImageSurface(nsLayoutUtils::SurfaceFromElementResult& res,
                                                     gfxImageSurface **imageOut,
                                                     WebGLTexelFormat *format);
 
-    void CopyTexSubImage2D_base(GLenum target,
+    bool CopyTexSubImage2D_base(GLenum target,
                                 GLint level,
                                 GLenum internalformat,
                                 GLint xoffset,
                                 GLint yoffset,
                                 GLint x,
                                 GLint y,
                                 GLsizei width,
                                 GLsizei height,
--- a/content/canvas/src/WebGLContextGL.cpp
+++ b/content/canvas/src/WebGLContextGL.cpp
@@ -323,17 +323,17 @@ WebGLContext::CheckFramebufferStatus(GLe
 
     if(mBoundFramebuffer->HasIncompleteAttachment())
         return LOCAL_GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT;
     if(mBoundFramebuffer->HasAttachmentsOfMismatchedDimensions())
         return LOCAL_GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS;
     return gl->fCheckFramebufferStatus(target);
 }
 
-void
+bool
 WebGLContext::CopyTexSubImage2D_base(GLenum target,
                                      GLint level,
                                      GLenum internalformat,
                                      GLint xoffset,
                                      GLint yoffset,
                                      GLint x,
                                      GLint y,
                                      GLsizei width,
@@ -342,59 +342,65 @@ WebGLContext::CopyTexSubImage2D_base(GLe
 {
     const WebGLRectangleObject *framebufferRect = FramebufferRectangleObject();
     GLsizei framebufferWidth = framebufferRect ? framebufferRect->Width() : 0;
     GLsizei framebufferHeight = framebufferRect ? framebufferRect->Height() : 0;
 
     const char *info = sub ? "copyTexSubImage2D" : "copyTexImage2D";
 
     if (!ValidateLevelWidthHeightForTarget(target, level, width, height, info)) {
-        return;
+        return false;
     }
 
     MakeContextCurrent();
 
     WebGLTexture *tex = activeBoundTextureForTarget(target);
 
-    if (!tex)
-        return ErrorInvalidOperation("%s: no texture is bound to this target");
+    if (!tex) {
+        ErrorInvalidOperation("%s: no texture is bound to this target");
+        return false;
+    }
 
     if (CanvasUtils::CheckSaneSubrectSize(x, y, width, height, framebufferWidth, framebufferHeight)) {
         if (sub)
             gl->fCopyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
         else
             gl->fCopyTexImage2D(target, level, internalformat, x, y, width, height, 0);
     } else {
 
         // the rect doesn't fit in the framebuffer
 
         /*** first, we initialize the texture as black ***/
 
         // first, compute the size of the buffer we should allocate to initialize the texture as black
 
         uint32_t texelSize = 0;
         if (!ValidateTexFormatAndType(internalformat, LOCAL_GL_UNSIGNED_BYTE, -1, &texelSize, info))
-            return;
+            return false;
 
         CheckedUint32 checked_neededByteLength = 
             GetImageSize(height, width, texelSize, mPixelStoreUnpackAlignment);
 
-        if (!checked_neededByteLength.isValid())
-            return ErrorInvalidOperation("%s: integer overflow computing the needed buffer size", info);
+        if (!checked_neededByteLength.isValid()) {
+            ErrorInvalidOperation("%s: integer overflow computing the needed buffer size", info);
+            return false;
+        }
 
         uint32_t bytesNeeded = checked_neededByteLength.value();
 
         // now that the size is known, create the buffer
 
         // We need some zero pages, because GL doesn't guarantee the
         // contents of a texture allocated with nullptr data.
         // Hopefully calloc will just mmap zero pages here.
         void *tempZeroData = calloc(1, bytesNeeded);
-        if (!tempZeroData)
-            return ErrorOutOfMemory("%s: could not allocate %d bytes (for zero fill)", info, bytesNeeded);
+        if (!tempZeroData) {
+            ErrorOutOfMemory("%s: could not allocate %d bytes (for zero fill)", info, bytesNeeded);
+            return false;
+        }
 
         // now initialize the texture as black
 
         if (sub)
             gl->fTexSubImage2D(target, level, 0, 0, width, height,
                                internalformat, LOCAL_GL_UNSIGNED_BYTE, tempZeroData);
         else
             gl->fTexImage2D(target, level, internalformat, width, height,
@@ -421,16 +427,18 @@ WebGLContext::CopyTexSubImage2D_base(GLe
         GLsizei actual_height  = actual_y_plus_height - actual_y;
         GLint   actual_yoffset = yoffset + actual_y - y;
 
         gl->fCopyTexSubImage2D(target, level, actual_xoffset, actual_yoffset, actual_x, actual_y, actual_width, actual_height);
     }
 
     if (!sub)
         ReattachTextureToAnyFramebufferToWorkAroundBugs(tex, level);
+
+    return true;
 }
 
 void
 WebGLContext::CopyTexImage2D(GLenum target,
                              GLint level,
                              GLenum internalformat,
                              GLint x,
                              GLint y,
@@ -518,25 +526,29 @@ WebGLContext::CopyTexImage2D(GLenum targ
         sizeMayChange = width != imageInfo.Width() ||
                         height != imageInfo.Height() ||
                         internalformat != imageInfo.Format() ||
                         type != imageInfo.Type();
     }
 
     if (sizeMayChange) {
         UpdateWebGLErrorAndClearGLError();
-        CopyTexSubImage2D_base(target, level, internalformat, 0, 0, x, y, width, height, false);
+    }
+
+    bool ok = CopyTexSubImage2D_base(target, level, internalformat, 0, 0, x, y, width, height, false);
+    if (!ok)
+        return;
+
+    if (sizeMayChange) {
         GLenum error = LOCAL_GL_NO_ERROR;
         UpdateWebGLErrorAndClearGLError(&error);
         if (error) {
             GenerateWarning("copyTexImage2D generated error %s", ErrorName(error));
             return;
         }          
-    } else {
-        CopyTexSubImage2D_base(target, level, internalformat, 0, 0, x, y, width, height, false);
     }
     
     tex->SetImageInfo(target, level, width, height, internalformat, type);
 }
 
 void
 WebGLContext::CopyTexSubImage2D(GLenum target,
                                 GLint level,
@@ -608,17 +620,17 @@ WebGLContext::CopyTexSubImage2D(GLenum t
     if (format == LOCAL_GL_DEPTH_COMPONENT ||
         format == LOCAL_GL_DEPTH_STENCIL)
         return ErrorInvalidOperation("copyTexSubImage2D: a base internal format of DEPTH_COMPONENT or DEPTH_STENCIL isn't supported");
 
     if (mBoundFramebuffer)
         if (!mBoundFramebuffer->CheckAndInitializeRenderbuffers())
             return ErrorInvalidFramebufferOperation("copyTexSubImage2D: incomplete framebuffer");
 
-    return CopyTexSubImage2D_base(target, level, format, xoffset, yoffset, x, y, width, height, true);
+    CopyTexSubImage2D_base(target, level, format, xoffset, yoffset, x, y, width, height, true);
 }
 
 
 already_AddRefed<WebGLProgram>
 WebGLContext::CreateProgram()
 {
     if (IsContextLost())
         return nullptr;
@@ -2244,18 +2256,20 @@ WebGLContext::ReadPixels(GLint x, GLint 
     if (mBoundFramebuffer) {
         // prevent readback of arbitrary video memory through uninitialized renderbuffers!
         if (!mBoundFramebuffer->CheckAndInitializeRenderbuffers())
             return ErrorInvalidFramebufferOperation("readPixels: incomplete framebuffer");
     }
     // Now that the errors are out of the way, on to actually reading
 
     // If we won't be reading any pixels anyways, just skip the actual reading
-    if (width == 0 || height == 0)
-        return DummyFramebufferOperation("readPixels");
+    if (width == 0 || height == 0) {
+        DummyFramebufferOperation("readPixels");
+        return;
+    }
 
     if (CanvasUtils::CheckSaneSubrectSize(x, y, width, height, framebufferWidth, framebufferHeight)) {
         // the easy case: we're not reading out-of-range pixels
         gl->fReadPixels(x, y, width, height, format, type, data);
     } else {
         // the rectangle doesn't fit entirely in the bound buffer. We then have to set to zero the part
         // of the buffer that correspond to out-of-range pixels. We don't want to rely on system OpenGL
         // to do that for us, because passing out of range parameters to a buggy OpenGL implementation
@@ -2267,17 +2281,18 @@ WebGLContext::ReadPixels(GLint x, GLint 
         memset(data, 0, checked_neededByteLength.value());
 
         if (   x >= framebufferWidth
             || x+width <= 0
             || y >= framebufferHeight
             || y+height <= 0)
         {
             // we are completely outside of range, can exit now with buffer filled with zeros
-            return DummyFramebufferOperation("readPixels");
+            DummyFramebufferOperation("readPixels");
+            return;
         }
 
         // compute the parameters of the subrect we're actually going to call glReadPixels on
         GLint   subrect_x      = std::max(x, 0);
         GLint   subrect_end_x  = std::min(x+width, framebufferWidth);
         GLsizei subrect_width  = subrect_end_x - subrect_x;
 
         GLint   subrect_y      = std::max(y, 0);