Bug 1362000 - (follow-up) Simplify match logic in CSSMaskLayerUserData::operator==. draft
authorcku <cku@mozilla.com>
Sat, 27 May 2017 01:38:54 +0800
changeset 585232 6b08b414648b2c1d8d35ef66552e1f3940adafb7
parent 585062 8c4e8d94ed0bb34912931eb334799beae1843198
child 585244 760763d30524cb5e4e7d373c7e0d5765ed1a2f14
push id61066
push userbmo:cku@mozilla.com
push dateFri, 26 May 2017 18:04:23 +0000
bugs1362000
milestone55.0a1
Bug 1362000 - (follow-up) Simplify match logic in CSSMaskLayerUserData::operator==. Remove HasUserSpaceOnUseUnitsMaskOrClipPath since it's not necessary. MozReview-Commit-ID: 2sen23m5GjE
layout/painting/FrameLayerBuilder.cpp
layout/svg/nsSVGEffects.cpp
layout/svg/nsSVGEffects.h
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -1627,40 +1627,19 @@ struct CSSMaskLayerUserData : public Lay
 
   void operator=(CSSMaskLayerUserData&& aOther)
   {
     mMaskBounds = aOther.mMaskBounds;
     mMaskStyle = Move(aOther.mMaskStyle);
   }
 
   bool
-  IsEqual(const CSSMaskLayerUserData& aOther, nsIFrame* aMaskedFrame) const
+  operator==(const CSSMaskLayerUserData& aOther) const
   {
-    // Even if the frame is valid, check the size of the display item's
-    // boundary is still necessary. For example, if we scale the masked frame
-    // by adding a transform property on it, the masked frame is valid itself
-    // but we have to regenerate mask according to the new size in device
-    // space.
-    if (mMaskBounds.Size() != aOther.mMaskBounds.Size()) {
-      return false;
-    }
-
-    // For SVG mask(or clipPath):
-    // The coordinate space of the mask layer that we created is in
-    // objectBoundingBox. If any of maskUnits or
-    // maskContentUnits is userSpaceOnUse, then mask/maskContent and the mask
-    // layer, where the mask was drawn, stay in different coordinate space.
-    // Since they are in different coordinate space, change the position of any
-    // one of them makes mask layer not reusable, and modifying the position of
-    // the masked frame does change the position of mask layer.
-    //
-    // CSS image mask's coordinate space is always objectBoudingBox, we do not
-    // have to worry about it.
-    if (mMaskBounds.TopLeft() != aOther.mMaskBounds.TopLeft() &&
-        nsSVGEffects::HasUserSpaceOnUseUnitsMaskOrClipPath(aMaskedFrame)) {
+    if (!mMaskBounds.IsEqualInterior(aOther.mMaskBounds)) {
       return false;
     }
 
     return mMaskStyle == aOther.mMaskStyle;
   }
 
 private:
   nsIntRect mMaskBounds;
@@ -3900,18 +3879,17 @@ ContainerState::SetupMaskLayerForCSSMask
   // 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);
   nsRect dirtyRect;
-  if (!aMaskItem->IsInvalid(dirtyRect) &&
-      oldUserData->IsEqual(newUserData, aMaskItem->Frame())) {
+  if (!aMaskItem->IsInvalid(dirtyRect) && *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/svg/nsSVGEffects.cpp
+++ b/layout/svg/nsSVGEffects.cpp
@@ -1032,48 +1032,8 @@ nsSVGEffects::GetMaskURI(nsIFrame* aFram
 {
   const nsStyleSVGReset* svgReset = aFrame->StyleSVGReset();
   MOZ_ASSERT(svgReset->mMask.mLayers.Length() > aIndex);
 
   mozilla::css::URLValueData* data =
     svgReset->mMask.mLayers[aIndex].mImage.GetURLValue();
   return ResolveURLUsingLocalRef(aFrame, data);
 }
-
-bool
-nsSVGEffects::HasUserSpaceOnUseUnitsMaskOrClipPath(nsIFrame* aFrame)
-{
-  nsIFrame* firstFrame =
-    nsLayoutUtils::FirstContinuationOrIBSplitSibling(aFrame);
-  nsSVGEffects::EffectProperties effectProperties =
-    nsSVGEffects::GetEffectProperties(firstFrame);
-
-  nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
-  for (uint32_t i =  0; i < maskFrames.Length() ; i++) {
-    if (maskFrames[i]) {
-      SVGMaskElement *element =
-        static_cast<SVGMaskElement*>(maskFrames[i]->GetContent());
-
-      RefPtr<SVGAnimatedEnumeration> maskUnits = element->MaskUnits();
-      RefPtr<SVGAnimatedEnumeration> contentUnits = element->MaskContentUnits();
-      if (maskUnits->AnimVal() == dom::SVG_UNIT_TYPE_USERSPACEONUSE ||
-          contentUnits->AnimVal() == dom::SVG_UNIT_TYPE_USERSPACEONUSE){
-        return true;
-      }
-    }
-  }
-
-  nsSVGClipPathFrame* clipPath = effectProperties.GetClipPathFrame();
-  if (clipPath) {
-    const nsStyleSVGReset* style = firstFrame->StyleSVGReset();
-    if (style->mClipPath.GetType() == StyleShapeSourceType::URL) {
-      SVGClipPathElement *element =
-        static_cast<SVGClipPathElement*>(clipPath->GetContent());
-
-      RefPtr<SVGAnimatedEnumeration> clipPathUnits = element->ClipPathUnits();
-      if (clipPathUnits->AnimVal()  == dom::SVG_UNIT_TYPE_USERSPACEONUSE) {
-        return true;
-      }
-    }
-  }
-
-  return false;
-}
--- a/layout/svg/nsSVGEffects.h
+++ b/layout/svg/nsSVGEffects.h
@@ -678,18 +678,11 @@ public:
    * @param aContent an element which uses a local-ref property. Here are some
    *                 examples:
    *                   <rect fill=url(#foo)>
    *                   <circle clip-path=url(#foo)>
    *                   <use xlink:href="#foo">
    */
   static already_AddRefed<nsIURI>
   GetBaseURLForLocalRef(nsIContent* aContent, nsIURI* aDocURI);
-
-  /**
-   * Return true if any of the value of
-   * maskUnits/maskContentUnits/clipPathUnits is userSpaceOnUse.
-   **/
-  static bool
-  HasUserSpaceOnUseUnitsMaskOrClipPath(nsIFrame* aFrame);
 };
 
 #endif /*NSSVGEFFECTS_H_*/