Bug 1319667 - Part 2. Move DrawResult from function parameter of PaintClipMask to the return value. draft
authorcku <cku@mozilla.com>
Thu, 24 Nov 2016 00:08:13 +0800
changeset 443252 09e0ae89fd388cca36a0a3d67cb26308101d4e76
parent 443251 ffb1272b5f3c3408ca950633b62f7903691d5924
child 443253 5ff057035104c234d8db1b92e0b5f47816474c95
child 443288 73727cfb8c09ccbbc2e09bb1217e9dc489e809a0
push id36940
push userbmo:cku@mozilla.com
push dateThu, 24 Nov 2016 04:14:24 +0000
bugs1319667, 1314536
milestone53.0a1
Bug 1319667 - Part 2. Move DrawResult from function parameter of PaintClipMask to the return value. Enlight by "Bug 1314536 - Part 1" MozReview-Commit-ID: Iogdh7LWsWa
layout/svg/nsSVGClipPathFrame.cpp
layout/svg/nsSVGClipPathFrame.h
layout/svg/nsSVGIntegrationUtils.cpp
layout/svg/nsSVGUtils.cpp
--- a/layout/svg/nsSVGClipPathFrame.cpp
+++ b/layout/svg/nsSVGClipPathFrame.cpp
@@ -160,23 +160,24 @@ nsSVGClipPathFrame::PaintClipMask(gfxCon
   nsSVGUtils::MaskUsage maskUsage;
   nsSVGUtils::DetermineMaskUsage(this, true, maskUsage);
 
   if (maskUsage.shouldApplyClipPath) {
     clipPathThatClipsClipPath->ApplyClipPath(aMaskContext, aClippedFrame,
                                              aMatrix);
   } else if (maskUsage.shouldGenerateClipMaskLayer) {
     Matrix maskTransform;
-    RefPtr<SourceSurface> mask =
+    RefPtr<SourceSurface> maskSurface;
+    Tie(result, maskSurface) =
       clipPathThatClipsClipPath->GetClipMask(aMaskContext, aClippedFrame,
                                              aMatrix, &maskTransform);
     aMaskContext.PushGroupForBlendBack(gfxContentType::ALPHA, 1.0,
-                                       mask, maskTransform);
+                                       maskSurface, maskTransform);
     // The corresponding PopGroupAndBlend call below will mask the
-    // blend using |mask|.
+    // blend using |maskSurface|.
   }
 
   // Paint our children into the mask:
   for (nsIFrame* kid = mFrames.FirstChild(); kid;
        kid = kid->GetNextSibling()) {
     result &= PaintFrameIntoMask(kid, aClippedFrame, aMaskContext, aMatrix);
   }
 
@@ -217,91 +218,84 @@ nsSVGClipPathFrame::PaintFrameIntoMask(n
   nsSVGClipPathFrame *clipPathThatClipsChild =
     nsSVGEffects::GetEffectProperties(aFrame).GetClipPathFrame(&isOK);
   if (!isOK) {
     return DrawResult::SUCCESS;
   }
 
   nsSVGUtils::MaskUsage maskUsage;
   nsSVGUtils::DetermineMaskUsage(aFrame, true, maskUsage);
+  DrawResult result = DrawResult::SUCCESS;
   if (maskUsage.shouldApplyClipPath) {
     clipPathThatClipsChild->ApplyClipPath(aTarget, aClippedFrame, aMatrix);
   } else if (maskUsage.shouldGenerateClipMaskLayer) {
     Matrix maskTransform;
-    RefPtr<SourceSurface> mask =
+    RefPtr<SourceSurface> maskSurface;
+    Tie(result, maskSurface) =
       clipPathThatClipsChild->GetClipMask(aTarget, aClippedFrame,
                                           aMatrix, &maskTransform);
     aTarget.PushGroupForBlendBack(gfxContentType::ALPHA, 1.0,
-                               mask, maskTransform);
+                                  maskSurface, maskTransform);
     // The corresponding PopGroupAndBlend call below will mask the
-    // blend using |mask|.
+    // blend using |maskSurface|.
   }
 
   gfxMatrix toChildsUserSpace = mMatrixForChildren;
   nsIFrame* child = do_QueryFrame(frame);
   nsIContent* childContent = child->GetContent();
   if (childContent->IsSVGElement()) {
     toChildsUserSpace =
       static_cast<const nsSVGElement*>(childContent)->
         PrependLocalTransformsTo(mMatrixForChildren, eUserSpaceToParent);
   }
 
   // Our children have NS_STATE_SVG_CLIPPATH_CHILD set on them, and
   // nsSVGPathGeometryFrame::Render checks for that state bit and paints
   // only the geometry (opaque black) if set.
-  DrawResult result = frame->PaintSVG(aTarget, toChildsUserSpace);
+  result &= frame->PaintSVG(aTarget, toChildsUserSpace);
 
   if (maskUsage.shouldGenerateClipMaskLayer) {
     aTarget.PopGroupAndBlend();
   } else if (maskUsage.shouldApplyClipPath) {
     aTarget.PopClip();
   }
 
   return result;
 }
 
-already_AddRefed<SourceSurface>
+mozilla::Pair<DrawResult, RefPtr<SourceSurface>>
 nsSVGClipPathFrame::GetClipMask(gfxContext& aReferenceContext,
                                 nsIFrame* aClippedFrame,
                                 const gfxMatrix& aMatrix,
                                 Matrix* aMaskTransform,
                                 SourceSurface* aExtraMask,
-                                const Matrix& aExtraMasksTransform,
-                                DrawResult* aResult)
+                                const Matrix& aExtraMasksTransform)
 {
   MOZ_ASSERT(!IsTrivial(), "Caller needs to use ApplyClipPath");
 
   IntPoint offset;
   RefPtr<DrawTarget> maskDT = CreateClipMask(aReferenceContext, offset);
   if (!maskDT) {
-    if (aResult) {
-      *aResult = DrawResult::SUCCESS;
-    }
-    return nullptr;
+    return MakePair(DrawResult::SUCCESS, RefPtr<SourceSurface>());
   }
 
   RefPtr<gfxContext> maskContext = gfxContext::CreateOrNull(maskDT);
   if (!maskContext) {
     gfxCriticalError() << "SVGClipPath context problem " << gfx::hexa(maskDT);
-    if (aResult) {
-      *aResult = DrawResult::TEMPORARY_ERROR;
-    }
-    return nullptr;
+    return MakePair(DrawResult::TEMPORARY_ERROR, RefPtr<SourceSurface>());
   }
   maskContext->SetMatrix(aReferenceContext.CurrentMatrix() *
                          gfxMatrix::Translation(-offset));
 
   DrawResult result = PaintClipMask(*maskContext, aClippedFrame, aMatrix,
                                     aMaskTransform, aExtraMask,
                                     aExtraMasksTransform);
-  if (aResult) {
-    *aResult = result;
-  }
 
-  return maskDT->Snapshot();
+  RefPtr<SourceSurface> surface = maskDT->Snapshot();
+  return MakePair(result, Move(surface));
 }
 
 bool
 nsSVGClipPathFrame::PointIsInsideClipPath(nsIFrame* aClippedFrame,
                                           const gfxPoint &aPoint)
 {
   // A clipPath can reference another clipPath.  We re-enter this method for
   // each clipPath in a reference chain, so here we limit chain length:
--- a/layout/svg/nsSVGClipPathFrame.h
+++ b/layout/svg/nsSVGClipPathFrame.h
@@ -72,24 +72,22 @@ public:
    * @param aMatrix The transform from aClippedFrame's user space to aContext's
    *   current transform.
    * @param [out] aMaskTransform The transform to use with the returned
    *   surface.
    * @param [in, optional] aExtraMask An extra surface that the returned
    *   surface should be masked with.
    * @param [in, optional] aExtraMasksTransform The transform to use with
    *   aExtraMask. Should be passed when aExtraMask is passed.
-   * @param [out, optional] aResult returns the result of drawing action.
    */
-  already_AddRefed<SourceSurface>
-    GetClipMask(gfxContext& aReferenceContext, nsIFrame* aClippedFrame,
-                const gfxMatrix& aMatrix, Matrix* aMaskTransform,
-                SourceSurface* aExtraMask = nullptr,
-                const Matrix& aExtraMasksTransform = Matrix(),
-                DrawResult* aResult = nullptr);
+  mozilla::Pair<DrawResult, RefPtr<SourceSurface>>
+  GetClipMask(gfxContext& aReferenceContext, nsIFrame* aClippedFrame,
+              const gfxMatrix& aMatrix, Matrix* aMaskTransform,
+              SourceSurface* aExtraMask = nullptr,
+              const Matrix& aExtraMasksTransform = Matrix());
 
   /**
    * Paint mask directly onto a given context(aMaskContext).
    *
    * @param aMaskContext The target of mask been painting on.
    * @param aClippedFrame The/an nsIFrame of the element that references this
    *   clipPath that is currently being processed.
    * @param aMatrix The transform from aClippedFrame's user space to
--- a/layout/svg/nsSVGIntegrationUtils.cpp
+++ b/layout/svg/nsSVGIntegrationUtils.cpp
@@ -908,30 +908,32 @@ nsSVGIntegrationUtils::PaintMaskAndClipP
 
     if (maskUsage.shouldGenerateClipMaskLayer) {
       matSR.Restore();
       matSR.SetContext(&context);
 
       SetupContextMatrix(firstFrame, aParams, offsetToBoundingBox,
                          offsetToUserSpace, false);
       Matrix clipMaskTransform;
-      RefPtr<SourceSurface> clipMaskSurface =
+      DrawResult clipMaskResult;
+      RefPtr<SourceSurface> clipMaskSurface;
+      Tie(clipMaskResult, clipMaskSurface) =
         clipPathFrame->GetClipMask(context, frame, cssPxToDevPxMatrix,
                                    &clipMaskTransform, maskSurface,
-                                   maskTransform, &result);
+                                   maskTransform);
 
       if (clipMaskSurface) {
         maskSurface = clipMaskSurface;
         maskTransform = clipMaskTransform;
       } else {
         // Either entire surface is clipped out, or gfx buffer allocation
         // failure in nsSVGClipPathFrame::GetClipMask.
-        return result;
+        return clipMaskResult;
       }
-
+      result &= clipMaskResult;
       shouldPushMask = true;
     }
 
     // opacity != 1.0f.
     if (!maskUsage.shouldGenerateClipMaskLayer &&
         !maskUsage.shouldGenerateMaskLayer) {
       MOZ_ASSERT(maskUsage.opacity != 1.0f);
 
--- a/layout/svg/nsSVGUtils.cpp
+++ b/layout/svg/nsSVGUtils.cpp
@@ -780,21 +780,23 @@ nsSVGUtils::PaintFrameWithEffects(nsIFra
         // Either entire surface is clipped out, or gfx buffer allocation
         // failure in nsSVGMaskFrame::GetMaskForMaskedFrame.
         return result;
       }
     }
 
     if (maskUsage.shouldGenerateClipMaskLayer) {
       Matrix clippedMaskTransform;
-      RefPtr<SourceSurface> clipMaskSurface =
+      DrawResult clipMaskResult;
+      RefPtr<SourceSurface> clipMaskSurface;
+      Tie(clipMaskResult, clipMaskSurface) =
         clipPathFrame->GetClipMask(aContext, aFrame, aTransform,
                                    &clippedMaskTransform, maskSurface,
-                                   maskTransform, &result);
-
+                                   maskTransform);
+      result &= clipMaskResult;
       if (clipMaskSurface) {
         maskSurface = clipMaskSurface;
         maskTransform = clippedMaskTransform;
       } else {
         // Either entire surface is clipped out, or gfx buffer allocation
         // failure in nsSVGClipPathFrame::GetClipMask.
         return result;
       }