Bug 1628988. Don't apply ImgDrawResult::NOT_READY in PaintMaskSurface. r=mstange
authorTimothy Nikkel <tnikkel@gmail.com>
Mon, 13 Apr 2020 20:08:05 +0000
changeset 523719 37cd04f0b502e033bb790a8911bbecb48e2b841b
parent 523718 9b655bd7bcb1a4f3b9b19985428c6c7c3306ea5e
child 523720 59b1434154dc9d2c65df871af1fe65e0f1cd25bd
push id112761
push usertnikkel@mozilla.com
push dateMon, 13 Apr 2020 20:13:47 +0000
treeherderautoland@37cd04f0b502 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1628988, 1624532
milestone77.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 1628988. Don't apply ImgDrawResult::NOT_READY in PaintMaskSurface. r=mstange PaintMaskSurface shouldn't be applying ImgDrawResult::NOT_READY when we don't have a frame and the mask image hasn't been resolved. ImgDrawResult is only about drawing images, not about waiting for external resources to resolve or frames to get constructed. The only purpose of tracking ImgDrawResult's in painting code is to know which frames we need to invalidate because their rendering might change if we sync decode images during a Draw call. Applying NOT_READY here means we invalidate for every paint with the sync decode images flag (ie reftest paints), and it never changes from NOT_READY. This bites the reftest for this bug 1624532. To fix it, instead of "overloading" the ImgDrawResult we return a bool to indicate the mask is missing or incomplete. Differential Revision: https://phabricator.services.mozilla.com/D70595
gfx/layers/wr/WebRenderCommandBuilder.cpp
layout/painting/nsDisplayList.cpp
layout/svg/nsSVGIntegrationUtils.cpp
layout/svg/nsSVGIntegrationUtils.h
testing/web-platform/meta/css/vendor-imports/mozilla/mozilla-central-reftests/masking/mask-image-1d.html.ini
--- a/gfx/layers/wr/WebRenderCommandBuilder.cpp
+++ b/gfx/layers/wr/WebRenderCommandBuilder.cpp
@@ -2509,17 +2509,17 @@ Maybe<wr::ImageMask> WebRenderCommandBui
     RefPtr<gfxContext> context = gfxContext::CreateOrNull(dt);
     MOZ_ASSERT(context);
 
     context->SetMatrix(context->CurrentMatrix()
                            .PreTranslate(-itemRect.x, -itemRect.y)
                            .PreScale(scale.width, scale.height));
 
     bool maskPainted = false;
-    bool paintFinished =
+    bool maskIsComplete =
         aMaskItem->PaintMask(aDisplayListBuilder, context, &maskPainted);
     if (!maskPainted) {
       return Nothing();
     }
 
     recorder->FlushItem(IntRect(0, 0, size.width, size.height));
     recorder->Finish();
 
@@ -2540,17 +2540,17 @@ Maybe<wr::ImageMask> WebRenderCommandBui
     }
     maskData->ClearImageKey();
     maskData->mBlobKey = Some(key);
     maskData->mFonts = fonts;
     TakeExternalSurfaces(
         recorder, maskData->mExternalSurfaces,
         mManager->GetRenderRootStateManager(aBuilder.GetRenderRoot()),
         aResources);
-    if (paintFinished) {
+    if (maskIsComplete) {
       maskData->mItemRect = itemRect;
       maskData->mMaskOffset = maskOffset;
       maskData->mScale = scale;
       maskData->mMaskStyle = aMaskItem->Frame()->StyleSVGReset()->mMask;
     }
   }
 
   wr::ImageMask imageMask;
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -9810,25 +9810,27 @@ bool nsDisplayMasksAndClipPaths::PaintMa
   MOZ_ASSERT(aMaskContext->GetDrawTarget()->GetFormat() == SurfaceFormat::A8);
 
   imgDrawingParams imgParams(aBuilder->GetImageDecodeFlags());
   nsRect borderArea = nsRect(ToReferenceFrame(), mFrame->GetSize());
   nsSVGIntegrationUtils::PaintFramesParams params(
       *aMaskContext, mFrame, mBounds, borderArea, aBuilder, nullptr,
       mHandleOpacity, imgParams);
   ComputeMaskGeometry(params);
-  bool painted = nsSVGIntegrationUtils::PaintMask(params);
+  bool maskIsComplete = false;
+  bool painted = nsSVGIntegrationUtils::PaintMask(params, maskIsComplete);
   if (aMaskPainted) {
     *aMaskPainted = painted;
   }
 
   nsDisplayMasksAndClipPathsGeometry::UpdateDrawResult(this, imgParams.result);
 
-  return imgParams.result == ImgDrawResult::SUCCESS ||
-         imgParams.result == ImgDrawResult::SUCCESS_NOT_COMPLETE;
+  return maskIsComplete &&
+         (imgParams.result == ImgDrawResult::SUCCESS ||
+          imgParams.result == ImgDrawResult::SUCCESS_NOT_COMPLETE);
 }
 
 LayerState nsDisplayMasksAndClipPaths::GetLayerState(
     nsDisplayListBuilder* aBuilder, LayerManager* aManager,
     const ContainerLayerParameters& aParameters) {
   if (CanPaintOnMaskLayer(aManager)) {
     LayerState result = RequiredLayerStateForChildren(
         aBuilder, aManager, aParameters, mList, GetAnimatedGeometryRoot());
--- a/layout/svg/nsSVGIntegrationUtils.cpp
+++ b/layout/svg/nsSVGIntegrationUtils.cpp
@@ -438,18 +438,19 @@ class RegularFramePaintCallback : public
   LayerManager* mLayerManager;
   gfxPoint mUserSpaceToFrameSpaceOffset;
 };
 
 typedef nsSVGIntegrationUtils::PaintFramesParams PaintFramesParams;
 
 /**
  * Paint css-positioned-mask onto a given target(aMaskDT).
+ * Return value indicates if mask is complete.
  */
-static void PaintMaskSurface(const PaintFramesParams& aParams,
+static bool PaintMaskSurface(const PaintFramesParams& aParams,
                              DrawTarget* aMaskDT, float aOpacity,
                              ComputedStyle* aSC,
                              const nsTArray<nsSVGMaskFrame*>& aMaskFrames,
                              const Matrix& aMaskSurfaceMatrix,
                              const nsPoint& aOffsetToUserSpace) {
   MOZ_ASSERT(aMaskFrames.Length() > 0);
   MOZ_ASSERT(aMaskDT->GetFormat() == SurfaceFormat::A8);
   MOZ_ASSERT(aOpacity == 1.0 || aMaskFrames.Length() == 1);
@@ -461,16 +462,18 @@ static void PaintMaskSurface(const Paint
   nsPresContext* presContext = aParams.frame->PresContext();
   gfxPoint devPixelOffsetToUserSpace = nsLayoutUtils::PointToGfxPoint(
       aOffsetToUserSpace, presContext->AppUnitsPerDevPixel());
 
   RefPtr<gfxContext> maskContext =
       gfxContext::CreatePreservingTransformOrNull(aMaskDT);
   MOZ_ASSERT(maskContext);
 
+  bool isMaskComplete = true;
+
   // Multiple SVG masks interleave with image mask. Paint each layer onto
   // aMaskDT one at a time.
   for (int i = aMaskFrames.Length() - 1; i >= 0; i--) {
     nsSVGMaskFrame* maskFrame = aMaskFrames[i];
     CompositionOp compositionOp =
         (i == int(aMaskFrames.Length() - 1))
             ? CompositionOp::OP_OVER
             : nsCSSRendering::GetGFXCompositeMode(
@@ -501,19 +504,21 @@ static void PaintMaskSurface(const Paint
               aParams.frame,
               aParams.builder->GetBackgroundPaintFlags() |
                   nsCSSRendering::PAINTBG_MASK_IMAGE,
               i, compositionOp, aOpacity);
 
       aParams.imgParams.result &= nsCSSRendering::PaintStyleImageLayerWithSC(
           params, *maskContext, aSC, *aParams.frame->StyleBorder());
     } else {
-      aParams.imgParams.result &= ImgDrawResult::NOT_READY;
+      isMaskComplete = false;
     }
   }
+
+  return isMaskComplete;
 }
 
 struct MaskPaintResult {
   RefPtr<SourceSurface> maskSurface;
   Matrix maskTransform;
   bool transparentBlackMask;
   bool opacityApplied;
 
@@ -566,21 +571,23 @@ static MaskPaintResult CreateAndPaintMas
   // 2. No overlap among masks.
   // Collision detect in #2 is not that trivial, we only accept #1 here.
   paintResult.opacityApplied = (aMaskFrames.Length() == 1);
 
   // Set context's matrix on maskContext, offset by the maskSurfaceRect's
   // position. This makes sure that we combine the masks in device space.
   Matrix maskSurfaceMatrix = ctx.CurrentMatrix();
 
-  PaintMaskSurface(aParams, maskDT, paintResult.opacityApplied ? aOpacity : 1.0,
-                   aSC, aMaskFrames, maskSurfaceMatrix, aOffsetToUserSpace);
+  bool isMaskComplete = PaintMaskSurface(
+      aParams, maskDT, paintResult.opacityApplied ? aOpacity : 1.0, aSC,
+      aMaskFrames, maskSurfaceMatrix, aOffsetToUserSpace);
 
-  if (aParams.imgParams.result != ImgDrawResult::SUCCESS &&
-      aParams.imgParams.result != ImgDrawResult::SUCCESS_NOT_COMPLETE) {
+  if (!isMaskComplete ||
+      (aParams.imgParams.result != ImgDrawResult::SUCCESS &&
+       aParams.imgParams.result != ImgDrawResult::SUCCESS_NOT_COMPLETE)) {
     // Now we know the status of mask resource since we used it while painting.
     // According to the return value of PaintMaskSurface, we know whether mask
     // resource is resolvable or not.
     //
     // For a HTML doc:
     //   According to css-masking spec, always create a mask surface when
     //   we have any item in maskFrame even if all of those items are
     //   non-resolvable <mask-sources> or <images>.
@@ -744,17 +751,20 @@ class AutoPopGroup {
   }
 
   void SetContext(gfxContext* aContext) { mContext = aContext; }
 
  private:
   gfxContext* mContext;
 };
 
-bool nsSVGIntegrationUtils::PaintMask(const PaintFramesParams& aParams) {
+bool nsSVGIntegrationUtils::PaintMask(const PaintFramesParams& aParams,
+                                      bool& aOutIsMaskComplete) {
+  aOutIsMaskComplete = true;
+
   nsSVGUtils::MaskUsage maskUsage;
   nsSVGUtils::DetermineMaskUsage(aParams.frame, aParams.handleOpacity,
                                  maskUsage);
   if (!maskUsage.shouldDoSomething()) {
     return false;
   }
 
   nsIFrame* frame = aParams.frame;
@@ -815,20 +825,20 @@ bool nsSVGIntegrationUtils::PaintMask(co
   }
 
   // Paint mask onto ctx.
   if (maskUsage.shouldGenerateMaskLayer) {
     matSR.Restore();
     matSR.SetContext(&ctx);
 
     EffectOffsets offsets = MoveContextOriginToUserSpace(frame, aParams);
-    PaintMaskSurface(aParams, maskTarget,
-                     shouldPushOpacity ? 1.0 : maskUsage.opacity,
-                     firstFrame->Style(), maskFrames, ctx.CurrentMatrix(),
-                     offsets.offsetToUserSpace);
+    aOutIsMaskComplete = PaintMaskSurface(
+        aParams, maskTarget, shouldPushOpacity ? 1.0 : maskUsage.opacity,
+        firstFrame->Style(), maskFrames, ctx.CurrentMatrix(),
+        offsets.offsetToUserSpace);
   }
 
   // Paint clip-path onto ctx.
   if (maskUsage.shouldGenerateClipMaskLayer || maskUsage.shouldApplyClipPath) {
     matSR.Restore();
     matSR.SetContext(&ctx);
 
     MoveContextOriginToUserSpace(firstFrame, aParams);
--- a/layout/svg/nsSVGIntegrationUtils.h
+++ b/layout/svg/nsSVGIntegrationUtils.h
@@ -184,21 +184,25 @@ class nsSVGIntegrationUtils final {
   static void PaintMaskAndClipPath(const PaintFramesParams& aParams);
 
   // This should use FunctionRef instead of std::function because we don't need
   // to take ownership of the function. See bug 1490781.
   static void PaintMaskAndClipPath(const PaintFramesParams& aParams,
                                    const std::function<void()>& aPaintChild);
 
   /**
-   * Paint mask of non-SVG frame onto a given context, aParams.ctx.
+   * Paint mask of frame onto a given context, aParams.ctx.
    * aParams.ctx must contain an A8 surface. Returns false if the mask
    * didn't get painted and should be ignored at the call site.
+   * isMaskComplete is an outparameter returning whether the mask is complete.
+   * Incomplete masks should not be drawn and the proper fallback behaviour
+   * depends on if the masked element is html or svg.
    */
-  static bool PaintMask(const PaintFramesParams& aParams);
+  static bool PaintMask(const PaintFramesParams& aParams,
+                        bool& aOutIsMaskComplete);
 
   /**
    * Return true if all the mask resource of aFrame are ready.
    */
   static bool IsMaskResourceReady(nsIFrame* aFrame);
 
   /**
    * Paint non-SVG frame with filter and opacity effect.
deleted file mode 100644
--- a/testing/web-platform/meta/css/vendor-imports/mozilla/mozilla-central-reftests/masking/mask-image-1d.html.ini
+++ /dev/null
@@ -1,6 +0,0 @@
-[mask-image-1d.html]
-  expected:
-    if not webrender and (os == "android") and debug: ["PASS", "TIMEOUT"]
-    if not webrender and (os == "android") and not debug: ["PASS", "TIMEOUT"]
-    if webrender: PASS
-    TIMEOUT