Bug 1552344 - Fix image layer diffing. r=jwatt
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 17 May 2019 04:37:24 +0200
changeset 474360 18b02f666766b281cb400d3f551ce26931c61713
parent 474359 b02147ae4804fa1a434dca44e1020c2e7361f5f7
child 474361 418d8f9bd4b14313481ceeb41462144aa9c61100
push id36031
push userrgurzau@mozilla.com
push dateFri, 17 May 2019 21:43:13 +0000
treeherdermozilla-central@1ae707852b60 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1552344
milestone68.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 1552344 - Fix image layer diffing. r=jwatt It was missing the cases where you changed values, but not count, and the image was not visible, like: mask-image: none; mask-mode: match-source, match-source; Then change mask-mode to `match-source, alpha`, for example. Differential Revision: https://phabricator.services.mozilla.com//D31568
layout/style/nsStyleStruct.cpp
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -2419,54 +2419,90 @@ nsStyleImageLayers::nsStyleImageLayers(c
     mImageCount = std::max(mImageCount, count);
     mSizeCount = std::max(mSizeCount, count);
     mMaskModeCount = std::max(mMaskModeCount, count);
     mBlendModeCount = std::max(mBlendModeCount, count);
     mCompositeCount = std::max(mCompositeCount, count);
   }
 }
 
+static bool IsElementImage(const nsStyleImageLayers::Layer& aLayer) {
+  return aLayer.mImage.GetType() == eStyleImageType_Element;
+}
+
+static bool AnyLayerIsElementImage(const nsStyleImageLayers& aLayers) {
+  NS_FOR_VISIBLE_IMAGE_LAYERS_BACK_TO_FRONT(i, aLayers) {
+    if (IsElementImage(aLayers.mLayers[i])) {
+      return true;
+    }
+  }
+  return false;
+}
+
 nsChangeHint nsStyleImageLayers::CalcDifference(
-    const nsStyleImageLayers& aNewLayers,
-    nsStyleImageLayers::LayerType aType) const {
+    const nsStyleImageLayers& aNewLayers, LayerType aType) const {
+
   nsChangeHint hint = nsChangeHint(0);
 
+  // If the number of visible images changes, then it's easy-peasy.
+  if (mImageCount != aNewLayers.mImageCount) {
+    hint |= nsChangeHint_RepaintFrame;
+    if (aType == nsStyleImageLayers::LayerType::Mask ||
+        AnyLayerIsElementImage(*this) || AnyLayerIsElementImage(aNewLayers)) {
+      hint |= nsChangeHint_UpdateEffects;
+    }
+    return hint;
+  }
+
   const nsStyleImageLayers& moreLayers =
-      mImageCount > aNewLayers.mImageCount ? *this : aNewLayers;
+      mLayers.Length() > aNewLayers.mLayers.Length() ? *this : aNewLayers;
   const nsStyleImageLayers& lessLayers =
-      mImageCount > aNewLayers.mImageCount ? aNewLayers : *this;
-
-  NS_FOR_VISIBLE_IMAGE_LAYERS_BACK_TO_FRONT(i, moreLayers) {
-    if (i < lessLayers.mImageCount) {
+      mLayers.Length() > aNewLayers.mLayers.Length() ? aNewLayers : *this;
+
+  for (size_t i = 0; i < moreLayers.mLayers.Length(); ++i) {
+    const Layer& moreLayersLayer = moreLayers.mLayers[i];
+    if (i < moreLayers.mImageCount) {
+      // This is a visible image we're diffing, we may need to repaint.
+      const Layer& lessLayersLayer = lessLayers.mLayers[i];
       nsChangeHint layerDifference =
-          moreLayers.mLayers[i].CalcDifference(lessLayers.mLayers[i]);
+          moreLayersLayer.CalcDifference(lessLayersLayer);
+      if (layerDifference && (IsElementImage(moreLayersLayer) ||
+                              IsElementImage(lessLayersLayer))) {
+        layerDifference |= nsChangeHint_UpdateEffects |
+                           nsChangeHint_RepaintFrame;
+      }
       hint |= layerDifference;
-      if (layerDifference && ((moreLayers.mLayers[i].mImage.GetType() ==
-                               eStyleImageType_Element) ||
-                              (lessLayers.mLayers[i].mImage.GetType() ==
-                               eStyleImageType_Element))) {
-        hint |= nsChangeHint_UpdateEffects | nsChangeHint_RepaintFrame;
-      }
-    } else {
-      hint |= nsChangeHint_RepaintFrame;
-      if (moreLayers.mLayers[i].mImage.GetType() == eStyleImageType_Element) {
-        hint |= nsChangeHint_UpdateEffects | nsChangeHint_RepaintFrame;
-      }
+      continue;
+    }
+    if (hint) {
+      // If they're different by now, we're done.
+      return hint;
     }
-  }
-
-  if (aType == nsStyleImageLayers::LayerType::Mask &&
-      mImageCount != aNewLayers.mImageCount) {
-    hint |= nsChangeHint_UpdateEffects;
+    if (i >= lessLayers.mLayers.Length()) {
+      // The layer count differs, we know some property has changed, but if we
+      // got here we know it won't affect rendering.
+      return nsChangeHint_NeutralChange;
+    }
+
+    const Layer& lessLayersLayer = lessLayers.mLayers[i];
+    MOZ_ASSERT(moreLayersLayer.mImage.GetType() == eStyleImageType_Null);
+    MOZ_ASSERT(lessLayersLayer.mImage.GetType() == eStyleImageType_Null);
+    if (moreLayersLayer.CalcDifference(lessLayersLayer)) {
+      // We don't care about the difference returned, we know it's not visible,
+      // but if something changed, then we need to return the neutral change.
+      return nsChangeHint_NeutralChange;
+    }
   }
 
   if (hint) {
+    // If they're different by now, we're done.
     return hint;
   }
 
+  // We could have same layers and values, but different count still.
   if (mAttachmentCount != aNewLayers.mAttachmentCount ||
       mBlendModeCount != aNewLayers.mBlendModeCount ||
       mClipCount != aNewLayers.mClipCount ||
       mCompositeCount != aNewLayers.mCompositeCount ||
       mMaskModeCount != aNewLayers.mMaskModeCount ||
       mOriginCount != aNewLayers.mOriginCount ||
       mRepeatCount != aNewLayers.mRepeatCount ||
       mPositionXCount != aNewLayers.mPositionXCount ||