Bug 1362000 - Implement nsDisplayMask::HasInvalidSVGMaskOrClipPath to validate svg mask & clip-path. draft
authorcku <cku@mozilla.com>
Sat, 13 May 2017 15:36:35 +0800
changeset 577328 68d7a4bdf5d0e64ed4d62f5d1ae3a126ef0014dc
parent 576745 8a7d0b15595f9916123848ca906f29c62d4914c9
child 577466 83e505d7365315590c4d1495a0806599063e24c0
child 577489 7421ca3f334145cc32cd7f6a0c2681322df915f7
child 577490 1a9b311ef1b84704f1dadd755424a9045d3202d0
child 577538 a7d6885417cc6d7070ecf1c97d720768d7e8299f
push id58659
push usercku@mozilla.com
push dateSat, 13 May 2017 08:46:39 +0000
bugs1362000
milestone55.0a1
Bug 1362000 - Implement nsDisplayMask::HasInvalidSVGMaskOrClipPath to validate svg mask & clip-path. At [1], we used "aMaskItem->IsInvalid(dirtyRect)" to check the validation of mask/clip-path, which is not correct. nsDisplayMask::IsInvalid checks validation of masked & clipped frames, instead of SVGMaskFrame & SVGClipPathFrame. So I implemented HasInvalidSVGMaskOrClipPath to do the validation checking on the right frames. In this bug, we find glitch during OMTA. Mostly, if you try to change the position of a masked frame, aMaskItem->IsInvalid will return true, so even we detect validation on wrong frame, luckyly, we can still regenerate a new mask layer. But in the case of OMTA, IsInvalid of a moving masked frame might return *false* during the animation, so we see glitch. Since glitch is noticable only during animation, so it's not easy to create a reftest for it. I manually test this path on the following pages: https://codepen.io/tadeuszwojcik/pen/ZKJwRE https://codepen.io/tadeuszwojcik/pen/oWexWW http://codepen.io/iiroullin/pen/oWEOOM [1] https://hg.mozilla.org/mozilla-central/file/a2832968581c/layout/painting/FrameLayerBuilder.cpp#l3884 MozReview-Commit-ID: AxugrJc5nTW
layout/painting/FrameLayerBuilder.cpp
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -3875,18 +3875,37 @@ ContainerState::SetupMaskLayerForCSSMask
   // We do not repaint mask for mask position change, so update base transform
   // each time is required.
   Matrix4x4 matrix;
   matrix.PreTranslate(itemRect.x, itemRect.y, 0);
   matrix.PreTranslate(mParameters.mOffset.x, mParameters.mOffset.y, 0);
   maskLayer->SetBaseTransform(matrix);
 
   CSSMaskLayerUserData newUserData(aMaskItem->Frame(), itemRect.Size());
-  nsRect dirtyRect;
-  if (!aMaskItem->IsInvalid(dirtyRect) && *oldUserData == newUserData) {
+  // Now, we are going to determine whether reuse cache mask layer or not.
+  //
+  // 1. For CSS image mask(SVG mask is excluded)
+  //    We care about if any mask-longhand property was changed, which is
+  //    compared in 'CSSMaskLayerUserData::operator=='. We also care about
+  //    the size of masked frame keep still, which was compared in
+  //    'CSSMaskLayerUserData::operator==' too.
+  // 2. For SVG mask & SVG clipPath
+  //    We have to check the validation of nsSVGMaskFrame & nsClipPathFrame,
+  //    which is done in HasInvalidSVGMaskOrClipPath. We don't really care
+  //    about mask longhand property(except mask-type). So the comparison in
+  //    'CSSMaskLayerUserData::operator==' is kind of too much for SVG
+  //    mask/clipPath. For example, modify mask-repeat will not change
+  //    anything of SVG mask. Keep it as it is unless we have proof it impact
+  //    performance.
+  //    Meanwhile, we also care about both the size and position of
+  //    masked/clipped frame, since maskUnits/maskContentUnits/clipPathUnits
+  //    may base on the bounding box of the masked/clipped frame. Luckily, any
+  //    geometry change of masked/clipped frame will invalid mask/clipPath
+  //    frame, so we can count on HasInvalidSVGMaskOrClipPath.
+  if (!aMaskItem->HasInvalidSVGMaskOrClipPath() && *oldUserData == newUserData) {
     aLayer->SetMaskLayer(maskLayer);
     return;
   }
 
   int32_t maxSize = mManager->GetMaxTextureSize();
   IntSize surfaceSize(std::min(itemRect.width, maxSize),
                       std::min(itemRect.height, maxSize));
 
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -8402,16 +8402,43 @@ nsDisplayMask::BuildLayer(nsDisplayListB
   RefPtr<ContainerLayer> container = aManager->GetLayerBuilder()->
     BuildContainerLayerFor(aBuilder, aManager, mFrame, this, &mList,
                            aContainerParameters, nullptr);
 
   return container.forget();
 }
 
 bool
+nsDisplayMask::HasInvalidSVGMaskOrClipPath() const
+{
+  nsIFrame* firstFrame =
+    nsLayoutUtils::FirstContinuationOrIBSplitSibling(mFrame);
+  nsSVGEffects::EffectProperties effectProperties =
+    nsSVGEffects::GetEffectProperties(firstFrame);
+
+  nsRect dummy;
+  // Check validation of SVG mask.
+  nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
+  for (uint32_t i = 0; i < maskFrames.Length(); i++) {
+    if (maskFrames[i] && !maskFrames[i]->IsInvalid(dummy)) {
+      return true;
+    }
+  }
+
+  // Check validation of SVG clip-path.
+  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame();
+  if (clipPathFrame && !clipPathFrame->IsInvalid(dummy)) {
+    return true;
+  }
+
+  // Either we don't have SVG mask and clip-path, or all of them are valid.
+  return false;
+}
+
+bool
 nsDisplayMask::PaintMask(nsDisplayListBuilder* aBuilder,
                          gfxContext* aMaskContext)
 {
   MOZ_ASSERT(aMaskContext->GetDrawTarget()->GetFormat() == SurfaceFormat::A8);
 
   uint32_t flags = aBuilder->ShouldSyncDecodeImages()
                   ? imgIContainer::FLAG_SYNC_DECODE
                   : imgIContainer::FLAG_SYNC_DECODE_IF_FAST;
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -4425,16 +4425,18 @@ public:
                     nsRenderingContext* aCtx,
                     LayerManager* aManager);
 
   /*
    * Paint mask onto aMaskContext in mFrame's coordinate space.
    */
   bool PaintMask(nsDisplayListBuilder* aBuilder, gfxContext* aMaskContext);
 
+  bool HasInvalidSVGMaskOrClipPath() const;
+
   const nsTArray<nsRect>& GetDestRects()
   {
     return mDestRects;
   }
 private:
   // According to mask property and the capability of aManager, determine
   // whether paint mask onto a dedicate mask layer.
   bool ShouldPaintOnMaskLayer(LayerManager* aManager);