Bug 1259696 - Check read buffer mode when doing CopyTexImage. r=jgilbert
authorEthan Lin <ethlin@mozilla.com>
Thu, 31 Mar 2016 23:06:33 -0700
changeset 291189 53400e993cb8a295a72dc247436b862844686588
parent 291188 538d248fa252a4100082fd9bc3fdc08d322cda22
child 291190 9d39b91bba9c192fe3cca3c772e3e83e96b9a298
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgilbert
bugs1259696
milestone48.0a1
Bug 1259696 - Check read buffer mode when doing CopyTexImage. r=jgilbert MozReview-Commit-ID: FYMiMaiRhii
dom/canvas/WebGL2ContextFramebuffers.cpp
dom/canvas/WebGLContext.cpp
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextGL.cpp
dom/canvas/WebGLFramebuffer.cpp
dom/canvas/WebGLFramebuffer.h
dom/canvas/WebGLTextureUpload.cpp
gfx/gl/GLScreenBuffer.h
--- a/dom/canvas/WebGL2ContextFramebuffers.cpp
+++ b/dom/canvas/WebGL2ContextFramebuffers.cpp
@@ -551,16 +551,17 @@ WebGL2Context::ReadBuffer(GLenum mode)
         {
             ErrorInvalidOperation("readBuffer: If READ_FRAMEBUFFER is non-null, `mode` "
                                   "must be COLOR_ATTACHMENTi or NONE. Was %s",
                                   EnumName(mode));
             return;
         }
 
         MakeContextCurrent();
+        mBoundReadFramebuffer->SetReadBufferMode(mode);
         gl->fReadBuffer(mode);
         return;
     }
 
     // Operating on the default framebuffer.
     if (mode != LOCAL_GL_NONE &&
         mode != LOCAL_GL_BACK)
     {
--- a/dom/canvas/WebGLContext.cpp
+++ b/dom/canvas/WebGLContext.cpp
@@ -1798,37 +1798,39 @@ WebGLContext::DidRefresh()
     if (gl) {
         gl->FlushIfHeavyGLCallsSinceLastFlush();
     }
 }
 
 bool
 WebGLContext::ValidateCurFBForRead(const char* funcName,
                                    const webgl::FormatUsageInfo** const out_format,
-                                   uint32_t* const out_width, uint32_t* const out_height)
+                                   uint32_t* const out_width, uint32_t* const out_height,
+                                   GLenum* const out_mode)
 {
     if (!mBoundReadFramebuffer) {
         ClearBackbufferIfNeeded();
 
         // FIXME - here we're assuming that the default framebuffer is backed by
         // UNSIGNED_BYTE that might not always be true, say if we had a 16bpp default
         // framebuffer.
         auto effFormat = mOptions.alpha ? webgl::EffectiveFormat::RGBA8
                                         : webgl::EffectiveFormat::RGB8;
 
         *out_format = mFormatUsage->GetUsage(effFormat);
         MOZ_ASSERT(*out_format);
 
         *out_width = mWidth;
         *out_height = mHeight;
+        *out_mode = gl->Screen()->GetReadBufferMode();
         return true;
     }
 
     return mBoundReadFramebuffer->ValidateForRead(funcName, out_format, out_width,
-                                                  out_height);
+                                                  out_height, out_mode);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 WebGLContext::ScopedMaskWorkaround::ScopedMaskWorkaround(WebGLContext& webgl)
     : mWebGL(webgl)
     , mFakeNoAlpha(ShouldFakeNoAlpha(webgl))
     , mFakeNoDepth(ShouldFakeNoDepth(webgl))
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -1282,17 +1282,18 @@ protected:
                                       WebGLTexDimensions dims);
 
     bool ValidateUniformLocationForProgram(WebGLUniformLocation* location,
                                            WebGLProgram* program,
                                            const char* funcName);
 
     bool ValidateCurFBForRead(const char* funcName,
                               const webgl::FormatUsageInfo** const out_format,
-                              uint32_t* const out_width, uint32_t* const out_height);
+                              uint32_t* const out_width, uint32_t* const out_height,
+                              GLenum* const out_mode);
 
     void Invalidate();
     void DestroyResourcesAndContext();
 
     void MakeContextCurrent() const;
 
     // helpers
 
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -1566,17 +1566,18 @@ WebGLContext::ReadPixels(GLint x, GLint 
         return;
     }
 
     MakeContextCurrent();
 
     const webgl::FormatUsageInfo* srcFormat;
     uint32_t srcWidth;
     uint32_t srcHeight;
-    if (!ValidateCurFBForRead("readPixels", &srcFormat, &srcWidth, &srcHeight))
+    GLenum srcMode;
+    if (!ValidateCurFBForRead("readPixels", &srcFormat, &srcWidth, &srcHeight, &srcMode))
         return;
 
     // Check the format and type params to assure they are an acceptable pair (as per spec)
     auto srcType = srcFormat->format->componentType;
     GLenum mainReadFormat;
     GLenum mainReadType;
     switch (srcType) {
         case webgl::ComponentType::Float:
--- a/dom/canvas/WebGLFramebuffer.cpp
+++ b/dom/canvas/WebGLFramebuffer.cpp
@@ -1141,17 +1141,18 @@ WebGLFramebuffer::FinalizeAttachments() 
     }
 
     FinalizeDrawAndReadBuffers(gl, mColorAttachment0.IsDefined());
 }
 
 bool
 WebGLFramebuffer::ValidateForRead(const char* funcName,
                                   const webgl::FormatUsageInfo** const out_format,
-                                  uint32_t* const out_width, uint32_t* const out_height)
+                                  uint32_t* const out_width, uint32_t* const out_height,
+                                  GLenum* const out_mode)
 {
     if (!ValidateAndInitAttachments(funcName))
         return false;
 
     if (mReadBufferMode == LOCAL_GL_NONE) {
         mContext->ErrorInvalidOperation("%s: Read buffer mode must not be"
                                         " NONE.", funcName);
         return false;
@@ -1159,16 +1160,17 @@ WebGLFramebuffer::ValidateForRead(const 
 
     const auto attachPoint = GetAttachPoint(mReadBufferMode);
     if (!attachPoint || !attachPoint->IsDefined()) {
         mContext->ErrorInvalidOperation("%s: The attachment specified for reading is"
                                         " null.", funcName);
         return false;
     }
 
+    *out_mode = mReadBufferMode;
     *out_format = attachPoint->Format();
     attachPoint->Size(out_width, out_height);
     return true;
 }
 
 static bool
 AttachmentsDontMatch(const WebGLFBAttachPoint& a, const WebGLFBAttachPoint& b)
 {
--- a/dom/canvas/WebGLFramebuffer.h
+++ b/dom/canvas/WebGLFramebuffer.h
@@ -248,16 +248,20 @@ public:
     const WebGLFBAttachPoint& StencilAttachment() const {
         return mStencilAttachment;
     }
 
     const WebGLFBAttachPoint& DepthStencilAttachment() const {
         return mDepthStencilAttachment;
     }
 
+    void SetReadBufferMode(GLenum readBufferMode) {
+        mReadBufferMode = readBufferMode;
+    }
+
 protected:
     WebGLFBAttachPoint* GetAttachPoint(GLenum attachment); // Fallible
 
 public:
     void DetachTexture(const WebGLTexture* tex);
 
     void DetachRenderbuffer(const WebGLRenderbuffer* rb);
 
@@ -275,17 +279,18 @@ public:
     bool ValidateAndInitAttachments(const char* funcName);
 
     void InvalidateFramebufferStatus() const {
         mIsKnownFBComplete = false;
     }
 
     bool ValidateForRead(const char* info,
                          const webgl::FormatUsageInfo** const out_format,
-                         uint32_t* const out_width, uint32_t* const out_height);
+                         uint32_t* const out_width, uint32_t* const out_height,
+                         GLenum* const out_mode);
 
     JS::Value GetAttachmentParameter(const char* funcName, JSContext* cx, GLenum target,
                                      GLenum attachment, GLenum pname,
                                      ErrorResult* const out_error);
 };
 
 } // namespace mozilla
 
--- a/dom/canvas/WebGLTextureUpload.cpp
+++ b/dom/canvas/WebGLTextureUpload.cpp
@@ -1714,20 +1714,31 @@ WebGLTexture::CopyTexImage2D(TexImageTar
     MOZ_ASSERT(imageInfo);
 
     ////////////////////////////////////
     // Get source info
 
     const webgl::FormatUsageInfo* srcUsage;
     uint32_t srcWidth;
     uint32_t srcHeight;
-    if (!mContext->ValidateCurFBForRead(funcName, &srcUsage, &srcWidth, &srcHeight))
+    GLenum srcMode;
+    if (!mContext->ValidateCurFBForRead(funcName, &srcUsage, &srcWidth, &srcHeight,
+                                        &srcMode))
         return;
     auto srcFormat = srcUsage->format;
 
+    // GLES 3.0.4 p145:
+    // "Calling CopyTexSubImage3D, CopyTexImage2D, or CopyTexSubImage2D will result in an
+    //  INVALID_OPERATION error if any of the following conditions is true: READ_BUFFER
+    //  is NONE"
+    if (srcMode == LOCAL_GL_NONE) {
+        mContext->ErrorInvalidOperation("%s: READ_BUFFER is NONE. ", funcName);
+        return;
+    }
+
     ////////////////////////////////////
     // Check that source and dest info are compatible
 
     const auto& fua = mContext->mFormatUsage;
 
     auto dstUsage = fua->GetSizedTexUsage(internalFormat);
     if (!dstUsage) {
         // It must be an unsized format then...
@@ -1864,20 +1875,31 @@ WebGLTexture::CopyTexSubImage(const char
     }
 
     ////////////////////////////////////
     // Get source info
 
     const webgl::FormatUsageInfo* srcUsage;
     uint32_t srcWidth;
     uint32_t srcHeight;
-    if (!mContext->ValidateCurFBForRead(funcName, &srcUsage, &srcWidth, &srcHeight))
+    GLenum srcMode;
+    if (!mContext->ValidateCurFBForRead(funcName, &srcUsage, &srcWidth, &srcHeight,
+                                        &srcMode))
         return;
     auto srcFormat = srcUsage->format;
 
+    // GLES 3.0.4 p145:
+    // "Calling CopyTexSubImage3D, CopyTexImage2D, or CopyTexSubImage2D will result in an
+    //  INVALID_OPERATION error if any of the following conditions is true: READ_BUFFER
+    //  is NONE"
+    if (srcMode == LOCAL_GL_NONE) {
+        mContext->ErrorInvalidOperation("%s: READ_BUFFER is NONE. ", funcName);
+        return;
+    }
+
     ////////////////////////////////////
     // Check that source and dest info are compatible
 
     if (!ValidateCopyTexImageFormats(mContext, funcName, srcFormat, dstFormat))
         return;
 
     ////////////////////////////////////
     // Do the thing!
--- a/gfx/gl/GLScreenBuffer.h
+++ b/gfx/gl/GLScreenBuffer.h
@@ -227,16 +227,20 @@ public:
     void BeforeReadCall();
 
     bool CopyTexImage2D(GLenum target, GLint level, GLenum internalformat, GLint x,
                         GLint y, GLsizei width, GLsizei height, GLint border);
 
     void SetReadBuffer(GLenum userMode);
     void SetDrawBuffer(GLenum userMode);
 
+    GLenum GetReadBufferMode() const {
+        return mUserReadBufferMode;
+    }
+
     /**
      * Attempts to read pixels from the current bound framebuffer, if
      * it is backed by a SharedSurface.
      *
      * Returns true if the pixel data has been read back, false
      * otherwise.
      */
     bool ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height,