Bug 1326385 - Handle undefined images in BlitFramebuffer. - r=kvark
authorJeff Gilbert <jgilbert@mozilla.com>
Fri, 30 Dec 2016 03:02:14 -0800
changeset 327766 bd28250d3dcb3730aac1de2e918254e03e112d94
parent 327765 07a8f320d6a574a91c5da1bbe0baec4fd6a48fdc
child 327767 81a71a4ea34d61a51fdee2836a7ea4663bc19f06
push id85270
push userjgilbert@mozilla.com
push dateMon, 02 Jan 2017 10:29:48 +0000
treeherdermozilla-inbound@bd28250d3dcb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskvark
bugs1326385
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);
     };