Bug 1303879 - Refresh DrawBuffers and ReadBuffer to handle GL4.0 and below. - r=mtseng draft
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 08 Sep 2016 15:56:58 -0700
changeset 415746 5ceff567db55eee03fdf7330ff8280e2d96c94e5
parent 415745 8b43f584f629e8227788c909f7c5b00d5c28e19e
child 415747 22bb230b948bbc65c47bdafb26434c4c6be28ed2
push id29953
push userbmo:jgilbert@mozilla.com
push dateTue, 20 Sep 2016 23:29:17 +0000
reviewersmtseng
bugs1303879
milestone51.0a1
Bug 1303879 - Refresh DrawBuffers and ReadBuffer to handle GL4.0 and below. - r=mtseng GL4.0 and below requires that framebuffers are incomplete if any DrawBuffers/ReadBuffer-selected buffers have no image attached. MozReview-Commit-ID: 8SFiy2tvgPT
dom/canvas/WebGLContext.cpp
dom/canvas/WebGLContext.h
dom/canvas/WebGLFramebuffer.cpp
dom/canvas/WebGLFramebuffer.h
--- a/dom/canvas/WebGLContext.cpp
+++ b/dom/canvas/WebGLContext.cpp
@@ -2114,16 +2114,34 @@ ScopedUnpackReset::UnwrapImpl()
         if (mWebGL->mBoundPixelUnpackBuffer) {
             pbo = mWebGL->mBoundPixelUnpackBuffer->mGLName;
         }
 
         mGL->fBindBuffer(LOCAL_GL_PIXEL_UNPACK_BUFFER, pbo);
     }
 }
 
+////////////////////
+
+void
+ScopedFBRebinder::UnwrapImpl()
+{
+    const auto fnName = [&](WebGLFramebuffer* fb) {
+        return fb ? fb->mGLName : 0;
+    };
+
+    if (mWebGL->IsWebGL2()) {
+        mGL->fBindFramebuffer(LOCAL_GL_DRAW_FRAMEBUFFER, fnName(mWebGL->mBoundDrawFramebuffer));
+        mGL->fBindFramebuffer(LOCAL_GL_READ_FRAMEBUFFER, fnName(mWebGL->mBoundReadFramebuffer));
+    } else {
+        MOZ_ASSERT(mWebGL->mBoundDrawFramebuffer == mWebGL->mBoundReadFramebuffer);
+        mGL->fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, fnName(mWebGL->mBoundDrawFramebuffer));
+    }
+}
+
 ////////////////////////////////////////
 
 void
 Intersect(uint32_t srcSize, int32_t dstStartInSrc, uint32_t dstSize,
           uint32_t* const out_intStartInSrc, uint32_t* const out_intStartInDst,
           uint32_t* const out_intSize)
 {
     // Only >0 if dstStartInSrc is >0:
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -184,16 +184,17 @@ public:
 class WebGLContext
     : public nsIDOMWebGLRenderingContext
     , public nsICanvasRenderingContextInternal
     , public nsSupportsWeakReference
     , public WebGLContextUnchecked
     , public WebGLRectangleObject
     , public nsWrapperCache
 {
+    friend class ScopedFBRebinder;
     friend class WebGL2Context;
     friend class WebGLContextUserData;
     friend class WebGLExtensionCompressedTextureATC;
     friend class WebGLExtensionCompressedTextureES3;
     friend class WebGLExtensionCompressedTextureETC1;
     friend class WebGLExtensionCompressedTexturePVRTC;
     friend class WebGLExtensionCompressedTextureS3TC;
     friend class WebGLExtensionDepthTexture;
@@ -1316,16 +1317,21 @@ protected:
     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);
 
+    bool HasDrawBuffers() const {
+        return IsWebGL2() ||
+               IsExtensionEnabled(WebGLExtensionID::WEBGL_draw_buffers);
+    }
+
     void Invalidate();
     void DestroyResourcesAndContext();
 
     void MakeContextCurrent() const;
 
     // helpers
 
     bool ConvertImage(size_t width, size_t height, size_t srcStride,
@@ -1777,28 +1783,46 @@ public:
     explicit operator bool() const { return bool(mBuffer); }
 
     void* get() const { return mBuffer; }
 
     UniqueBuffer(const UniqueBuffer& other) = delete; // construct using Move()!
     void operator =(const UniqueBuffer& other) = delete; // assign using Move()!
 };
 
-class ScopedUnpackReset
+class ScopedUnpackReset final
     : public gl::ScopedGLWrapper<ScopedUnpackReset>
 {
     friend struct gl::ScopedGLWrapper<ScopedUnpackReset>;
 
-protected:
+private:
     WebGLContext* const mWebGL;
 
 public:
     explicit ScopedUnpackReset(WebGLContext* webgl);
 
-protected:
+private:
+    void UnwrapImpl();
+};
+
+class ScopedFBRebinder final
+    : public gl::ScopedGLWrapper<ScopedFBRebinder>
+{
+    friend struct gl::ScopedGLWrapper<ScopedFBRebinder>;
+
+private:
+    WebGLContext* const mWebGL;
+
+public:
+    explicit ScopedFBRebinder(WebGLContext* webgl)
+        : ScopedGLWrapper<ScopedFBRebinder>(webgl->gl)
+        , mWebGL(webgl)
+    { }
+
+private:
     void UnwrapImpl();
 };
 
 void
 ComputeLengthAndData(const dom::ArrayBufferViewOrSharedArrayBufferView& view,
                      void** const out_data, size_t* const out_length,
                      js::Scalar::Type* const out_type);
 
--- a/dom/canvas/WebGLFramebuffer.cpp
+++ b/dom/canvas/WebGLFramebuffer.cpp
@@ -337,36 +337,36 @@ WebGLFBAttachPoint::IsComplete(WebGLCont
             return false;
         }
     }
 
     return true;
 }
 
 void
-WebGLFBAttachPoint::Resolve(gl::GLContext* gl, FBTarget target) const
+WebGLFBAttachPoint::Resolve(gl::GLContext* gl) const
 {
     if (!HasImage())
         return;
 
     if (Renderbuffer()) {
-        Renderbuffer()->DoFramebufferRenderbuffer(target, mAttachmentPoint);
+        Renderbuffer()->DoFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, mAttachmentPoint);
         return;
     }
     MOZ_ASSERT(Texture());
 
     MOZ_ASSERT(gl == Texture()->mContext->GL());
 
     const auto& texName = Texture()->mGLName;
 
     ////
 
     const auto fnAttach2D = [&](GLenum attachmentPoint) {
-        gl->fFramebufferTexture2D(target.get(), attachmentPoint, mTexImageTarget.get(),
-                                  texName, mTexImageLevel);
+        gl->fFramebufferTexture2D(LOCAL_GL_FRAMEBUFFER, attachmentPoint,
+                                  mTexImageTarget.get(), texName, mTexImageLevel);
     };
 
     ////
 
     switch (mTexImageTarget.get()) {
     case LOCAL_GL_TEXTURE_2D:
     case LOCAL_GL_TEXTURE_CUBE_MAP_POSITIVE_X:
     case LOCAL_GL_TEXTURE_CUBE_MAP_NEGATIVE_X:
@@ -381,18 +381,18 @@ WebGLFBAttachPoint::Resolve(gl::GLContex
             fnAttach2D(mAttachmentPoint);
         }
         break;
 
     case LOCAL_GL_TEXTURE_2D_ARRAY:
     case LOCAL_GL_TEXTURE_3D:
         // If we have fFramebufferTextureLayer, we can rely on having
         // DEPTH_STENCIL_ATTACHMENT.
-        gl->fFramebufferTextureLayer(target.get(), mAttachmentPoint,
-                                     texName, mTexImageLevel, mTexImageLayer);
+        gl->fFramebufferTextureLayer(LOCAL_GL_FRAMEBUFFER, mAttachmentPoint, texName,
+                                     mTexImageLevel, mTexImageLayer);
         break;
     }
 }
 
 JS::Value
 WebGLFBAttachPoint::GetParameter(const char* funcName, WebGLContext* webgl, JSContext* cx,
                                  GLenum target, GLenum attachment, GLenum pname,
                                  ErrorResult* const out_error) const
@@ -889,42 +889,43 @@ WebGLFramebuffer::ValidateForRead(const 
     mColorReadBuffer->Size(out_width, out_height);
     return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Resolution and caching
 
 void
-WebGLFramebuffer::ResolveAttachments(FBTarget target) const
+WebGLFramebuffer::ResolveAttachments() const
 {
     const auto& gl = mContext->gl;
 
     ////
     // Nuke attachment points.
 
     for (uint32_t i = 0; i < mContext->mImplMaxColorAttachments; i++) {
         const GLenum attachEnum = LOCAL_GL_COLOR_ATTACHMENT0 + i;
-        gl->fFramebufferRenderbuffer(target.get(), attachEnum, LOCAL_GL_RENDERBUFFER, 0);
+        gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, attachEnum,
+                                     LOCAL_GL_RENDERBUFFER, 0);
     }
 
-    gl->fFramebufferRenderbuffer(target.get(), LOCAL_GL_DEPTH_ATTACHMENT,
+    gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_DEPTH_ATTACHMENT,
                                  LOCAL_GL_RENDERBUFFER, 0);
-    gl->fFramebufferRenderbuffer(target.get(), LOCAL_GL_STENCIL_ATTACHMENT,
+    gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_STENCIL_ATTACHMENT,
                                  LOCAL_GL_RENDERBUFFER, 0);
 
     ////
 
     for (const auto& attach : mColorAttachments) {
-        attach.Resolve(gl, target);
+        attach.Resolve(gl);
     }
 
-    mDepthAttachment.Resolve(gl, target);
-    mStencilAttachment.Resolve(gl, target);
-    mDepthStencilAttachment.Resolve(gl, target);
+    mDepthAttachment.Resolve(gl);
+    mStencilAttachment.Resolve(gl);
+    mDepthStencilAttachment.Resolve(gl);
 }
 
 bool
 WebGLFramebuffer::ResolveAttachmentData(const char* funcName) const
 {
     //////
     // Check if we need to initialize anything
 
@@ -979,25 +980,18 @@ WebGLFramebuffer::ResolveAttachmentData(
         if (!tex->InitializeImageData(funcName, attach->ImageTarget(),
                                       attach->MipLevel()))
         {
             return false;
         }
     }
 
     if (clearBits) {
-        const auto drawBufferExt = WebGLExtensionID::WEBGL_draw_buffers;
-        const bool hasDrawBuffers = (mContext->IsWebGL2() ||
-                                     mContext->IsExtensionEnabled(drawBufferExt));
-
         const auto fnDrawBuffers = [&](const std::vector<const WebGLFBAttachPoint*>& src)
         {
-            if (!hasDrawBuffers)
-                return;
-
             std::vector<GLenum> enumList;
 
             for (const auto& cur : src) {
                 const auto& attachEnum = cur->mAttachmentPoint;
                 const GLenum attachId = attachEnum - LOCAL_GL_COLOR_ATTACHMENT0;
 
                 while (enumList.size() < attachId) {
                     enumList.push_back(LOCAL_GL_NONE);
@@ -1008,25 +1002,30 @@ WebGLFramebuffer::ResolveAttachmentData(
             mContext->gl->fDrawBuffers(enumList.size(), enumList.data());
         };
 
         ////
         // Clear
 
         mContext->MakeContextCurrent();
 
-        fnDrawBuffers(colorAttachmentsToClear);
+        const bool hasDrawBuffers = mContext->HasDrawBuffers();
+        if (hasDrawBuffers) {
+            fnDrawBuffers(colorAttachmentsToClear);
+        }
 
         {
             gl::ScopedBindFramebuffer autoBind(mContext->gl, mGLName);
 
             mContext->ForceClearFramebufferWithDefaultValues(clearBits, false);
         }
 
-        fnDrawBuffers(mColorDrawBuffers);
+        if (hasDrawBuffers) {
+            RefreshDrawBuffers();
+        }
 
         // Mark initialized.
         for (const auto& cur : attachmentsToClear) {
             cur->SetImageDataStatus(WebGLImageDataStatus::InitializedImageData);
         }
     }
 
     return true;
@@ -1094,17 +1093,17 @@ WebGLFramebuffer::ResolvedData::Resolved
     }
 
     if (parent.mColorReadBuffer) {
         fnColor(*(parent.mColorReadBuffer), &readSet);
     }
 }
 
 void
-WebGLFramebuffer::RecacheResolvedData()
+WebGLFramebuffer::RefreshResolvedData()
 {
     if (mResolvedCompleteData) {
         mResolvedCompleteData.reset(new ResolvedData(*this));
     }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Entrypoints
@@ -1122,37 +1121,33 @@ WebGLFramebuffer::CheckFramebufferStatus
     do {
         if (ret != LOCAL_GL_FRAMEBUFFER_COMPLETE)
             break;
 
         // Looks good on our end. Let's ask the driver.
         gl::GLContext* const gl = mContext->gl;
         gl->MakeCurrent();
 
+        const ScopedFBRebinder autoFB(mContext);
+        gl->fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mGLName);
+
         ////
 
-        const FBTarget fbTarget = (mContext->IsWebGL2() ? LOCAL_GL_DRAW_FRAMEBUFFER
-                                                        : LOCAL_GL_FRAMEBUFFER);
-        const bool needsFBRebind = (mContext->mBoundDrawFramebuffer != this);
-        if (needsFBRebind) {
-            gl->fBindFramebuffer(fbTarget.get(), mGLName);
-        }
+        ResolveAttachments(); // OK, attach everything properly!
+        RefreshDrawBuffers();
+        RefreshReadBuffer();
 
-        ResolveAttachments(fbTarget); // OK, attach everything properly!
-        ret = gl->fCheckFramebufferStatus(fbTarget.get());
-
-        if (needsFBRebind) {
-            gl->fBindFramebuffer(fbTarget.get(),
-                                 mContext->mBoundDrawFramebuffer->mGLName);
-        }
+        ret = gl->fCheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER);
 
         ////
 
         if (ret != LOCAL_GL_FRAMEBUFFER_COMPLETE) {
-            statusInfo.AssignLiteral("Bad status according to the driver:");
+            const nsPrintfCString text("Bad status according to the driver: 0x%04x",
+                                       ret.get());
+            statusInfo = text;
             break;
         }
 
         if (!ResolveAttachmentData(funcName)) {
             ret = LOCAL_GL_FRAMEBUFFER_UNSUPPORTED;
             statusInfo.AssignLiteral("Failed to lazily-initialize attachment data.");
             break;
         }
@@ -1165,90 +1160,126 @@ WebGLFramebuffer::CheckFramebufferStatus
     mContext->GenerateWarning("%s: Framebuffer not complete. (status: 0x%04x) %s",
                               funcName, ret.get(), statusInfo.BeginReading());
     return ret;
 }
 
 ////
 
 void
+WebGLFramebuffer::RefreshDrawBuffers() const
+{
+    const auto& gl = mContext->gl;
+    if (!gl->IsSupported(gl::GLFeature::draw_buffers))
+        return;
+
+    // Prior to GL4.1, having a no-image FB attachment that's selected by DrawBuffers
+    // yields a framebuffer status of FRAMEBUFFER_INCOMPLETE_DRAW_BUFFER.
+    // We could workaround this only on affected versions, but it's easier be
+    // unconditional.
+    std::vector<GLenum> driverBuffers(mContext->mImplMaxDrawBuffers, LOCAL_GL_NONE);
+    for (const auto& attach : mColorDrawBuffers) {
+        if (attach->HasImage()) {
+            const uint32_t index = attach->mAttachmentPoint - LOCAL_GL_COLOR_ATTACHMENT0;
+            driverBuffers[index] = attach->mAttachmentPoint;
+        }
+    }
+
+    gl->fDrawBuffers(driverBuffers.size(), driverBuffers.data());
+}
+
+void
+WebGLFramebuffer::RefreshReadBuffer() const
+{
+    const auto& gl = mContext->gl;
+    if (!gl->IsSupported(gl::GLFeature::read_buffer))
+        return;
+
+    // Prior to GL4.1, having a no-image FB attachment that's selected by ReadBuffer
+    // yields a framebuffer status of FRAMEBUFFER_INCOMPLETE_READ_BUFFER.
+    // We could workaround this only on affected versions, but it's easier be
+    // unconditional.
+    GLenum driverBuffer = LOCAL_GL_NONE;
+    if (mColorReadBuffer && mColorReadBuffer->HasImage()) {
+        driverBuffer = mColorReadBuffer->mAttachmentPoint;
+    }
+
+    gl->fReadBuffer(driverBuffer);
+}
+
+////
+
+void
 WebGLFramebuffer::DrawBuffers(const char* funcName, const dom::Sequence<GLenum>& buffers)
 {
     if (buffers.Length() > mContext->mImplMaxDrawBuffers) {
         // "An INVALID_VALUE error is generated if `n` is greater than MAX_DRAW_BUFFERS."
         mContext->ErrorInvalidValue("%s: `buffers` must have a length <="
                                     " MAX_DRAW_BUFFERS.", funcName);
         return;
     }
 
+    std::vector<const WebGLFBAttachPoint*> newColorDrawBuffers;
+    newColorDrawBuffers.reserve(buffers.Length());
+
     for (size_t i = 0; i < buffers.Length(); i++) {
         // "If the GL is bound to a draw framebuffer object, the `i`th buffer listed in
         //  bufs must be COLOR_ATTACHMENTi or NONE. Specifying a buffer out of order,
         //  BACK, or COLOR_ATTACHMENTm where `m` is greater than or equal to the value of
         // MAX_COLOR_ATTACHMENTS, will generate the error INVALID_OPERATION.
 
         // WEBGL_draw_buffers:
         // "The value of the MAX_COLOR_ATTACHMENTS_WEBGL parameter must be greater than or
         //  equal to that of the MAX_DRAW_BUFFERS_WEBGL parameter."
         // This means that if buffers.Length() isn't larger than MaxDrawBuffers, it won't
         // be larger than MaxColorAttachments.
-        if (buffers[i] != LOCAL_GL_NONE &&
-            buffers[i] != LOCAL_GL_COLOR_ATTACHMENT0 + i)
-        {
+        const auto& cur = buffers[i];
+        if (cur == LOCAL_GL_COLOR_ATTACHMENT0 + i) {
+            const auto& attach = mColorAttachments[i];
+            newColorDrawBuffers.push_back(&attach);
+        } else if (cur != LOCAL_GL_NONE) {
             mContext->ErrorInvalidOperation("%s: `buffers[i]` must be NONE or"
                                             " COLOR_ATTACHMENTi.",
                                             funcName);
             return;
         }
     }
 
     ////
-    // Record it.
-
-    mColorDrawBuffers.clear();
-    for (size_t i = 0; i < buffers.Length(); i++) {
-        const auto& attachEnum = buffers[i];
-        if (attachEnum == LOCAL_GL_NONE)
-            continue;
-
-        const auto& attach = mColorAttachments[i];
-        MOZ_ASSERT(attach.mAttachmentPoint == attachEnum);
-
-        mColorDrawBuffers.push_back(&attach);
-    }
-    RecacheResolvedData();
-
-    ////
 
     mContext->MakeContextCurrent();
-    mContext->gl->fDrawBuffers(buffers.Length(), buffers.Elements());
+
+    mColorDrawBuffers.swap(newColorDrawBuffers);
+    RefreshDrawBuffers(); // Calls glDrawBuffers.
+    RefreshResolvedData();
 }
 
 void
 WebGLFramebuffer::ReadBuffer(const char* funcName, GLenum attachPoint)
 {
     const auto& maybeAttach = GetColorAttachPoint(attachPoint);
     if (!maybeAttach) {
         const char text[] = "%s: `mode` must be a COLOR_ATTACHMENTi, for 0 <= i <"
                             " MAX_DRAW_BUFFERS.";
         if (attachPoint == LOCAL_GL_BACK) {
             mContext->ErrorInvalidOperation(text, funcName);
         } else {
             mContext->ErrorInvalidEnum(text, funcName);
         }
         return;
     }
-    const auto& attach = maybeAttach.value();
+    const auto& attach = maybeAttach.value(); // Might be nullptr.
 
-    // Record it.
-    mColorReadBuffer = attach;
-    RecacheResolvedData();
+    ////
 
     mContext->MakeContextCurrent();
-    mContext->gl->fReadBuffer(attachPoint);
+
+    mColorReadBuffer = attach;
+    RefreshReadBuffers(); // Calls glReadBuffer.
+    RefreshResolvedData();
 }
 
 ////
 
 void
 WebGLFramebuffer::FramebufferRenderbuffer(const char* funcName, GLenum attachEnum,
                                           GLenum rbtarget, WebGLRenderbuffer* rb)
 {
--- a/dom/canvas/WebGLFramebuffer.h
+++ b/dom/canvas/WebGLFramebuffer.h
@@ -89,17 +89,17 @@ public:
     bool HasUninitializedImageData() const;
     void SetImageDataStatus(WebGLImageDataStatus x) const;
 
     void Size(uint32_t* const out_width, uint32_t* const out_height) const;
 
     bool HasImage() const;
     bool IsComplete(WebGLContext* webgl, nsCString* const out_info) const;
 
-    void Resolve(gl::GLContext* gl, FBTarget target) const;
+    void Resolve(gl::GLContext* gl) const;
 
     JS::Value GetParameter(const char* funcName, WebGLContext* webgl, JSContext* cx,
                            GLenum target, GLenum attachment, GLenum pname,
                            ErrorResult* const out_error) const;
 
     void OnBackingStoreRespecified() const;
 
     ////
@@ -212,17 +212,19 @@ public:
     bool HasIncompleteAttachments(nsCString* const out_info) const;
     bool AllImageRectsMatch() const;
     bool AllImageSamplesMatch() const;
     FBStatus PrecheckFramebufferStatus(nsCString* const out_info) const;
 
 protected:
     Maybe<WebGLFBAttachPoint*> GetAttachPoint(GLenum attachment); // Fallible
     Maybe<WebGLFBAttachPoint*> GetColorAttachPoint(GLenum attachment); // Fallible
-    void ResolveAttachments(FBTarget target) const;
+    void ResolveAttachments() const;
+    void RefreshDrawBuffers() const;
+    void RefreshReadBuffer() const;
     bool ResolveAttachmentData(const char* funcName) const;
 
 public:
     void DetachTexture(const WebGLTexture* tex);
     void DetachRenderbuffer(const WebGLRenderbuffer* rb);
     bool ValidateAndInitAttachments(const char* funcName);
 
     bool ValidateForRead(const char* info,
@@ -247,17 +249,17 @@ public:
     // Invalidation
 
     bool IsResolvedComplete() const { return bool(mResolvedCompleteData); }
 
     void InvalidateFramebufferStatus() {
         mResolvedCompleteData = nullptr;
     }
 
-    void RecacheResolvedData();
+    void RefreshResolvedData();
 
     ////////////////
     // WebGL funcs
 
     FBStatus CheckFramebufferStatus(const char* funcName);
     void FramebufferRenderbuffer(const char* funcName, GLenum attachment, GLenum rbtarget,
                                  WebGLRenderbuffer* rb);
     void FramebufferTexture2D(const char* funcName, GLenum attachment,