Bug 1326385 - Handle undefined images in BlitFramebuffer. - r=kvark draft
authorJeff Gilbert <jgilbert@mozilla.com>
Fri, 30 Dec 2016 03:02:14 -0800
changeset 454992 5a0d2a7fdacb8cd94f9278ab1a3f475cdd063bce
parent 454747 6f63f95e28ffc05c0d2f5ef6cd6e05905fe8ea5a
child 540864 8072ebda89aeacb7d582458e50b65e626e6ea7aa
push id40102
push userbmo:jgilbert@mozilla.com
push dateSat, 31 Dec 2016 02:21:32 +0000
reviewerskvark
bugs1326385
milestone53.0a1
Bug 1326385 - Handle undefined images in BlitFramebuffer. - r=kvark MozReview-Commit-ID: 3FjzETVL0AZ
dom/canvas/WebGLContextFramebufferOperations.cpp
dom/canvas/WebGLFramebuffer.cpp
dom/canvas/WebGLFramebuffer.h
--- a/dom/canvas/WebGLContextFramebufferOperations.cpp
+++ b/dom/canvas/WebGLContextFramebufferOperations.cpp
@@ -32,18 +32,20 @@ WebGLContext::Clear(GLbitfield mask)
         GenerateWarning("Calling gl.clear() with RASTERIZER_DISCARD enabled has no effects.");
     }
 
     if (mBoundDrawFramebuffer) {
         if (!mBoundDrawFramebuffer->ValidateAndInitAttachments(funcName))
             return;
 
         if (mask & LOCAL_GL_COLOR_BUFFER_BIT) {
-            const auto& resolvedData = mBoundDrawFramebuffer->ResolvedCompleteData();
-            for (const auto& cur : resolvedData->colorDrawBuffers) {
+            for (const auto& cur : mBoundDrawFramebuffer->ColorDrawBuffers()) {
+                if (!cur->IsDefined())
+                    continue;
+
                 switch (cur->Format()->format->componentType) {
                 case webgl::ComponentType::Float:
                 case webgl::ComponentType::NormInt:
                 case webgl::ComponentType::NormUInt:
                     break;
 
                 default:
                     ErrorInvalidOperation("%s: Color draw buffers must be floating-point"
--- a/dom/canvas/WebGLFramebuffer.cpp
+++ b/dom/canvas/WebGLFramebuffer.cpp
@@ -874,22 +874,21 @@ WebGLFramebuffer::ValidateAndInitAttachm
 bool
 WebGLFramebuffer::ValidateClearBufferType(const char* funcName, GLenum buffer,
                                           uint32_t drawBuffer, GLenum funcType) const
 {
     if (buffer != LOCAL_GL_COLOR)
         return true;
 
     const auto& attach = mColorAttachments[drawBuffer];
-    if (!count(mResolvedCompleteData->colorDrawBuffers.begin(),
-               mResolvedCompleteData->colorDrawBuffers.end(),
-               &attach))
-    {
+    if (!attach.IsDefined())
+        return true;
+
+    if (!count(mColorDrawBuffers.begin(), mColorDrawBuffers.end(), &attach))
         return true; // DRAW_BUFFERi set to NONE.
-    }
 
     GLenum attachType;
     switch (attach.Format()->format->componentType) {
     case webgl::ComponentType::Int:
         attachType = LOCAL_GL_INT;
         break;
     case webgl::ComponentType::UInt:
         attachType = LOCAL_GL_UNSIGNED_INT;
@@ -1076,42 +1075,24 @@ WebGLFramebuffer::ResolveAttachmentData(
             cur->SetImageDataStatus(WebGLImageDataStatus::InitializedImageData);
         }
     }
 
     return true;
 }
 
 WebGLFramebuffer::ResolvedData::ResolvedData(const WebGLFramebuffer& parent)
-    : hasSampleBuffers(false)
-    , depthBuffer(nullptr)
-    , stencilBuffer(nullptr)
 {
-    if (parent.mDepthAttachment.IsDefined()) {
-        depthBuffer = &parent.mDepthAttachment;
-    }
-    if (parent.mStencilAttachment.IsDefined()) {
-        stencilBuffer = &parent.mStencilAttachment;
-    }
-    if (parent.mDepthStencilAttachment.IsDefined()) {
-        depthBuffer = &parent.mDepthStencilAttachment;
-        stencilBuffer = &parent.mDepthStencilAttachment;
-    }
 
-    ////
-
-    colorDrawBuffers.reserve(parent.mColorDrawBuffers.size());
     texDrawBuffers.reserve(parent.mColorDrawBuffers.size() + 2); // +2 for depth+stencil.
 
     const auto fnCommon = [&](const WebGLFBAttachPoint& attach) {
         if (!attach.IsDefined())
             return false;
 
-        hasSampleBuffers |= bool(attach.Samples());
-
         if (attach.Texture()) {
             texDrawBuffers.push_back(&attach);
         }
         return true;
     };
 
     ////
 
@@ -1130,17 +1111,16 @@ WebGLFramebuffer::ResolvedData::Resolved
     ////
 
     for (const auto& pAttach : parent.mColorDrawBuffers) {
         const auto& attach = *pAttach;
         if (!fnCommon(attach))
             return;
 
         drawSet.insert(WebGLFBAttachPoint::Ordered(attach));
-        colorDrawBuffers.push_back(&attach);
     }
 
     if (parent.mColorReadBuffer) {
         const auto& attach = *parent.mColorReadBuffer;
         if (!fnCommon(attach))
             return;
 
         readSet.insert(WebGLFBAttachPoint::Ordered(attach));
@@ -1623,87 +1603,125 @@ GetBackbufferFormats(const WebGLContext*
 WebGLFramebuffer::BlitFramebuffer(WebGLContext* webgl,
                                   const WebGLFramebuffer* srcFB, GLint srcX0, GLint srcY0,
                                   GLint srcX1, GLint srcY1,
                                   const WebGLFramebuffer* dstFB, GLint dstX0, GLint dstY0,
                                   GLint dstX1, GLint dstY1,
                                   GLbitfield mask, GLenum filter)
 {
     const char funcName[] = "blitFramebuffer";
-    auto& gl = webgl->gl;
-
+    const auto& gl = webgl->gl;
 
     ////
     // Collect data
 
-    const auto fnGetFormat = [](const WebGLFBAttachPoint* cur) -> const webgl::FormatInfo*
+    const auto fnGetDepthAndStencilAttach = [](const WebGLFramebuffer* fb,
+                                               const WebGLFBAttachPoint** const out_depth,
+                                               const WebGLFBAttachPoint** const out_stencil)
     {
-        if (!cur)
-            return nullptr;
+        *out_depth = nullptr;
+        *out_stencil = nullptr;
+
+        if (!fb)
+            return;
 
-        MOZ_ASSERT(cur->IsDefined());
-        return cur->Format()->format;
+        if (fb->mDepthStencilAttachment.IsDefined()) {
+            *out_depth = *out_stencil = &fb->mDepthStencilAttachment;
+            return;
+        }
+        if (fb->mDepthAttachment.IsDefined()) {
+            *out_depth = &fb->mDepthAttachment;
+        }
+        if (fb->mStencilAttachment.IsDefined()) {
+            *out_stencil = &fb->mStencilAttachment;
+        }
     };
 
-    bool srcSampleBuffers;
-    const webgl::FormatInfo* srcColorFormat;
-    const webgl::FormatInfo* srcDepthFormat;
-    const webgl::FormatInfo* srcStencilFormat;
-
-    if (srcFB) {
-        srcSampleBuffers = srcFB->mResolvedCompleteData->hasSampleBuffers;
-
-        srcColorFormat = fnGetFormat(srcFB->mColorReadBuffer);
-        srcDepthFormat = fnGetFormat(srcFB->mResolvedCompleteData->depthBuffer);
-        srcStencilFormat = fnGetFormat(srcFB->mResolvedCompleteData->stencilBuffer);
-    } else {
-        srcSampleBuffers = false; // Always false.
-
-        GetBackbufferFormats(webgl, &srcColorFormat, &srcDepthFormat, &srcStencilFormat);
-    }
+    const WebGLFBAttachPoint* srcDepthAttach;
+    const WebGLFBAttachPoint* srcStencilAttach;
+    fnGetDepthAndStencilAttach(srcFB, &srcDepthAttach, &srcStencilAttach);
+    const WebGLFBAttachPoint* dstDepthAttach;
+    const WebGLFBAttachPoint* dstStencilAttach;
+    fnGetDepthAndStencilAttach(dstFB, &dstDepthAttach, &dstStencilAttach);
 
     ////
 
-    bool dstSampleBuffers;
-    const webgl::FormatInfo* dstDepthFormat;
-    const webgl::FormatInfo* dstStencilFormat;
-    bool dstHasColor = false;
-    bool colorFormatsMatch = true;
-    bool colorTypesMatch = true;
+    const auto fnGetFormat = [](const WebGLFBAttachPoint* cur,
+                                bool* const out_hasSamples) -> const webgl::FormatInfo*
+    {
+        if (!cur || !cur->IsDefined())
+            return nullptr;
+
+        *out_hasSamples |= bool(cur->Samples());
+        return cur->Format()->format;
+    };
 
     const auto fnNarrowComponentType = [&](const webgl::FormatInfo* format) {
         switch (format->componentType) {
         case webgl::ComponentType::NormInt:
         case webgl::ComponentType::NormUInt:
             return webgl::ComponentType::Float;
 
         default:
             return format->componentType;
         }
     };
 
+    bool srcHasSamples;
+    const webgl::FormatInfo* srcColorFormat;
+    webgl::ComponentType srcColorType = webgl::ComponentType::None;
+    const webgl::FormatInfo* srcDepthFormat;
+    const webgl::FormatInfo* srcStencilFormat;
+
+    if (srcFB) {
+        srcHasSamples = false;
+        srcColorFormat = fnGetFormat(srcFB->mColorReadBuffer, &srcHasSamples);
+        srcDepthFormat = fnGetFormat(srcDepthAttach, &srcHasSamples);
+        srcStencilFormat = fnGetFormat(srcStencilAttach, &srcHasSamples);
+    } else {
+        srcHasSamples = false; // Always false.
+
+        GetBackbufferFormats(webgl, &srcColorFormat, &srcDepthFormat, &srcStencilFormat);
+    }
+
+    if (srcColorFormat) {
+        srcColorType = fnNarrowComponentType(srcColorFormat);
+    }
+
+    ////
+
+    bool dstHasSamples;
+    const webgl::FormatInfo* dstDepthFormat;
+    const webgl::FormatInfo* dstStencilFormat;
+    bool dstHasColor = false;
+    bool colorFormatsMatch = true;
+    bool colorTypesMatch = true;
+
     const auto fnCheckColorFormat = [&](const webgl::FormatInfo* dstFormat) {
         MOZ_ASSERT(dstFormat->r || dstFormat->g || dstFormat->b || dstFormat->a);
         dstHasColor = true;
         colorFormatsMatch &= (dstFormat == srcColorFormat);
-        colorTypesMatch &= ( fnNarrowComponentType(dstFormat) ==
-                             fnNarrowComponentType(srcColorFormat) );
+        colorTypesMatch &= ( fnNarrowComponentType(dstFormat) == srcColorType );
     };
 
     if (dstFB) {
-        dstSampleBuffers = dstFB->mResolvedCompleteData->hasSampleBuffers;
+        dstHasSamples = false;
 
-        dstDepthFormat = fnGetFormat(dstFB->mResolvedCompleteData->depthBuffer);
-        dstStencilFormat = fnGetFormat(dstFB->mResolvedCompleteData->stencilBuffer);
+        for (const auto& cur : dstFB->mColorDrawBuffers) {
+            const auto& format = fnGetFormat(cur, &dstHasSamples);
+            if (!format)
+                continue;
 
-        for (const auto& cur : dstFB->mResolvedCompleteData->colorDrawBuffers) {
-            fnCheckColorFormat(cur->Format()->format);
+            fnCheckColorFormat(format);
         }
+
+        dstDepthFormat = fnGetFormat(dstDepthAttach, &dstHasSamples);
+        dstStencilFormat = fnGetFormat(dstStencilAttach, &dstHasSamples);
     } else {
-        dstSampleBuffers = bool(gl->Screen()->Samples());
+        dstHasSamples = bool(gl->Screen()->Samples());
 
         const webgl::FormatInfo* dstColorFormat;
         GetBackbufferFormats(webgl, &dstColorFormat, &dstDepthFormat, &dstStencilFormat);
 
         fnCheckColorFormat(dstColorFormat);
     }
 
     ////
@@ -1767,43 +1785,43 @@ WebGLFramebuffer::BlitFramebuffer(WebGLC
      *   mask includes DEPTH_BUFFER_BIT or STENCIL_BUFFER_BIT, and the source
      *   and destination depth and stencil buffer formats do not match.
      *
      * jgilbert: The wording is such that if only DEPTH_BUFFER_BIT is specified,
      * the stencil formats must match. This seems wrong. It could be a spec bug,
      * or I could be missing an interaction in one of the earlier paragraphs.
      */
     if (mask & LOCAL_GL_DEPTH_BUFFER_BIT &&
-        dstDepthFormat != srcDepthFormat)
+        dstDepthFormat && dstDepthFormat != srcDepthFormat)
     {
         webgl->ErrorInvalidOperation("%s: Depth buffer formats must match if selected.",
                                      funcName);
         return;
     }
 
     if (mask & LOCAL_GL_STENCIL_BUFFER_BIT &&
-        dstStencilFormat != srcStencilFormat)
+        dstStencilFormat && dstStencilFormat != srcStencilFormat)
     {
         webgl->ErrorInvalidOperation("%s: Stencil buffer formats must match if selected.",
                                      funcName);
         return;
     }
 
     ////
 
-    if (dstSampleBuffers) {
+    if (dstHasSamples) {
         webgl->ErrorInvalidOperation("%s: DRAW_FRAMEBUFFER may not have multiple"
                                      " samples.",
                                      funcName);
         return;
     }
 
-    if (srcSampleBuffers) {
+    if (srcHasSamples) {
         if (mask & LOCAL_GL_COLOR_BUFFER_BIT &&
-            !colorFormatsMatch)
+            dstHasColor && !colorFormatsMatch)
         {
             webgl->ErrorInvalidOperation("%s: Color buffer formats must match if"
                                          " selected, when reading from a multisampled"
                                          " source.",
                                          funcName);
             return;
         }
 
@@ -1821,37 +1839,35 @@ WebGLFramebuffer::BlitFramebuffer(WebGLC
 
     ////
     // Check for feedback
 
     if (srcFB && dstFB) {
         const WebGLFBAttachPoint* feedback = nullptr;
 
         if (mask & LOCAL_GL_COLOR_BUFFER_BIT) {
-            for (const auto& cur : dstFB->mResolvedCompleteData->colorDrawBuffers) {
-                if (srcFB->mColorReadBuffer->IsEquivalent(*cur)) {
+            MOZ_ASSERT(srcFB->mColorReadBuffer->IsDefined());
+            for (const auto& cur : dstFB->mColorDrawBuffers) {
+                if (srcFB->mColorReadBuffer->IsEquivalentForFeedback(*cur)) {
                     feedback = cur;
+                    break;
                 }
             }
         }
 
-        const auto& srcDepthBuffer = srcFB->mResolvedCompleteData->depthBuffer;
-        const auto& dstDepthBuffer = dstFB->mResolvedCompleteData->depthBuffer;
         if (mask & LOCAL_GL_DEPTH_BUFFER_BIT &&
-            srcDepthBuffer->IsEquivalent(*dstDepthBuffer))
+            srcDepthAttach->IsEquivalentForFeedback(*dstDepthAttach))
         {
-            feedback = dstDepthBuffer;
+            feedback = dstDepthAttach;
         }
 
-        const auto& srcStencilBuffer = srcFB->mResolvedCompleteData->stencilBuffer;
-        const auto& dstStencilBuffer = dstFB->mResolvedCompleteData->stencilBuffer;
         if (mask & LOCAL_GL_STENCIL_BUFFER_BIT &&
-            srcStencilBuffer->IsEquivalent(*dstStencilBuffer))
+            srcStencilAttach->IsEquivalentForFeedback(*dstStencilAttach))
         {
-            feedback = dstStencilBuffer;
+            feedback = dstStencilAttach;
         }
 
         if (feedback) {
             webgl->ErrorInvalidOperation("%s: Feedback detected into DRAW_FRAMEBUFFER's"
                                          " 0x%04x attachment.",
                                          funcName, feedback->mAttachmentPoint);
             return;
         }
--- a/dom/canvas/WebGLFramebuffer.h
+++ b/dom/canvas/WebGLFramebuffer.h
@@ -97,18 +97,19 @@ public:
     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;
 
-    bool IsEquivalent(const WebGLFBAttachPoint& other) const {
-        MOZ_ASSERT(IsDefined() && other.IsDefined());
+    bool IsEquivalentForFeedback(const WebGLFBAttachPoint& other) const {
+        if (!IsDefined() || !other.IsDefined())
+            return false;
 
 #define _(X) X == other.X
         return ( _(mRenderbufferPtr) &&
                  _(mTexturePtr) &&
                  _(mTexImageTarget.get()) &&
                  _(mTexImageLevel) &&
                  _(mTexImageLayer) );
 #undef _
@@ -178,22 +179,16 @@ protected:
     ////
 
     std::vector<const WebGLFBAttachPoint*> mColorDrawBuffers; // Non-null
     const WebGLFBAttachPoint* mColorReadBuffer; // Null if NONE
 
     ////
 
     struct ResolvedData {
-        // BlitFramebuffer
-        bool hasSampleBuffers;
-        std::vector<const WebGLFBAttachPoint*> colorDrawBuffers; // Non-null, defined
-        const WebGLFBAttachPoint* depthBuffer; // null if not defined
-        const WebGLFBAttachPoint* stencilBuffer; // null if not defined
-
         // IsFeedback
         std::vector<const WebGLFBAttachPoint*> texDrawBuffers; // Non-null
         std::set<WebGLFBAttachPoint::Ordered> drawSet;
         std::set<WebGLFBAttachPoint::Ordered> readSet;
 
         explicit ResolvedData(const WebGLFramebuffer& parent);
     };