Bug 1194480: Only update overflow region (& trigger DLBI) for changes to CSS 'box-shadow' or 'text-shadow', instead of triggering a reflow. r=heycam
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 01 Oct 2015 20:05:28 -0700
changeset 298311 009f9286101b1331fc9652743d7b702adc80b86d
parent 298310 24ded1d0386af4fb23a2b4331be5316e519b077a
child 298312 d55fde51f4b09e143d2b902410cba9647ee7fb03
push id6068
push userkcambridge@mozilla.com
push dateFri, 02 Oct 2015 20:37:20 +0000
reviewersheycam
bugs1194480
milestone44.0a1
Bug 1194480: Only update overflow region (& trigger DLBI) for changes to CSS 'box-shadow' or 'text-shadow', instead of triggering a reflow. r=heycam
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
layout/style/test/test_dynamic_change_causing_reflow.html
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -85,18 +85,18 @@ static bool EqualImages(imgIRequest *aIm
 static int safe_strcmp(const char16_t* a, const char16_t* b)
 {
   if (!a || !b) {
     return (int)(a - b);
   }
   return NS_strcmp(a, b);
 }
 
-static nsChangeHint CalcShadowDifference(nsCSSShadowArray* lhs,
-                                         nsCSSShadowArray* rhs);
+static bool AreShadowArraysEqual(nsCSSShadowArray* lhs,
+                                 nsCSSShadowArray* rhs);
 
 // --------------------
 // nsStyleFont
 //
 nsStyleFont::nsStyleFont(const nsFont& aFont, nsPresContext *aPresContext)
   : mFont(aFont)
   , mGenericID(kGenericFont_NONE)
   , mExplicitLanguage(false)
@@ -514,45 +514,57 @@ nsStyleBorder::Destroy(nsPresContext* aC
   UntrackImage(aContext);
   this->~nsStyleBorder();
   aContext->PresShell()->
     FreeByObjectID(eArenaObjectID_nsStyleBorder, this);
 }
 
 nsChangeHint nsStyleBorder::CalcDifference(const nsStyleBorder& aOther) const
 {
-  nsChangeHint shadowDifference =
-    CalcShadowDifference(mBoxShadow, aOther.mBoxShadow);
-  MOZ_ASSERT(shadowDifference == unsigned(NS_STYLE_HINT_REFLOW) ||
-             shadowDifference == unsigned(NS_STYLE_HINT_VISUAL) ||
-             shadowDifference == unsigned(NS_STYLE_HINT_NONE),
-             "should do more with shadowDifference");
-
   // XXXbz we should be able to return a more specific change hint for
   // at least GetComputedBorder() differences...
   if (mTwipsPerPixel != aOther.mTwipsPerPixel ||
       GetComputedBorder() != aOther.GetComputedBorder() ||
       mFloatEdge != aOther.mFloatEdge ||
       mBorderImageOutset != aOther.mBorderImageOutset ||
-      (shadowDifference & nsChangeHint_NeedReflow) ||
       mBoxDecorationBreak != aOther.mBoxDecorationBreak)
     return NS_STYLE_HINT_REFLOW;
 
+  nsChangeHint boxShadowHint = nsChangeHint(0);
+  if (!AreShadowArraysEqual(mBoxShadow, aOther.mBoxShadow)) {
+    // Update overflow regions & trigger DLBI to be sure it's noticed:
+    NS_UpdateHint(boxShadowHint, nsChangeHint_UpdateOverflow);
+    NS_UpdateHint(boxShadowHint, nsChangeHint_SchedulePaint);
+    // Also request a repaint, since it's possible that only the color
+    // of the shadow is changing (and UpdateOverflow/SchedulePaint won't
+    // repaint for that, since they won't know what needs invalidating.)
+    NS_UpdateHint(boxShadowHint, nsChangeHint_RepaintFrame);
+    // Don't return yet; we may also need nsChangeHint_BorderStyleNoneChange.
+  }
+
   NS_FOR_CSS_SIDES(ix) {
     // See the explanation in nsChangeHint.h of
     // nsChangeHint_BorderStyleNoneChange .
     // Furthermore, even though we know *this* side is 0 width, just
     // assume a repaint hint for some other change rather than bother
     // tracking this result through the rest of the function.
     if (HasVisibleStyle(ix) != aOther.HasVisibleStyle(ix)) {
-      return NS_CombineHint(nsChangeHint_RepaintFrame,
+      return NS_CombineHint(boxShadowHint,
+                            nsChangeHint_RepaintFrame |
                             nsChangeHint_BorderStyleNoneChange);
     }
   }
 
+  if (boxShadowHint) {
+    // NOTE: This hint (UpdateOverflow + SchedulePaint + RepaintFrame) is
+    // expected to subsume all hints returned after this point. (Hence, we're
+    // OK to return early.)
+    return boxShadowHint;
+  }
+
   // Note that mBorderStyle stores not only the border style but also
   // color-related flags.  Given that we've already done an mComputedBorder
   // comparison, border-style differences can only lead to a repaint hint.  So
   // it's OK to just compare the values directly -- if either the actual
   // style or the color flags differ we want to repaint.
   NS_FOR_CSS_SIDES(ix) {
     if (mBorderStyle[ix] != aOther.mBorderStyle[ix] || 
         mBorderColor[ix] != aOther.mBorderColor[ix])
@@ -579,20 +591,16 @@ nsChangeHint nsStyleBorder::CalcDifferen
   if (mBorderColors) {
     NS_FOR_CSS_SIDES(ix) {
       if (!nsBorderColors::Equal(mBorderColors[ix],
                                  aOther.mBorderColors[ix]))
         return nsChangeHint_RepaintFrame;
     }
   }
 
-  if (shadowDifference) {
-    return shadowDifference;
-  }
-
   // mBorder is the specified border value.  Changes to this don't
   // need any change processing, since we operate on the computed
   // border values instead.
   if (mBorder != aOther.mBorder) {
     return nsChangeHint_NeutralChange;
   }
 
   return NS_STYLE_HINT_NONE;
@@ -3376,35 +3384,32 @@ nsChangeHint nsStyleTextReset::CalcDiffe
     if (mTextOverflow != aOther.mTextOverflow) {
       return nsChangeHint_RepaintFrame;
     }
     return NS_STYLE_HINT_NONE;
   }
   return NS_STYLE_HINT_REFLOW;
 }
 
-// Allowed to return one of NS_STYLE_HINT_NONE, NS_STYLE_HINT_REFLOW
-// or NS_STYLE_HINT_VISUAL. Currently we just return NONE or REFLOW, though.
-// XXXbz can this not return a more specific hint?  If that's ever
-// changed, nsStyleBorder::CalcDifference will need changing too.
-static nsChangeHint
-CalcShadowDifference(nsCSSShadowArray* lhs,
+// Returns true if the given shadow-arrays are equal.
+static bool
+AreShadowArraysEqual(nsCSSShadowArray* lhs,
                      nsCSSShadowArray* rhs)
 {
   if (lhs == rhs)
-    return NS_STYLE_HINT_NONE;
+    return true;
 
   if (!lhs || !rhs || lhs->Length() != rhs->Length())
-    return NS_STYLE_HINT_REFLOW;
+    return false;
 
   for (uint32_t i = 0; i < lhs->Length(); ++i) {
     if (*lhs->ShadowAt(i) != *rhs->ShadowAt(i))
-      return NS_STYLE_HINT_REFLOW;
+      return false;
   }
-  return NS_STYLE_HINT_NONE;
+  return true;
 }
 
 // --------------------
 // nsStyleText
 //
 
 nsStyleText::nsStyleText(void)
 { 
@@ -3490,17 +3495,22 @@ nsChangeHint nsStyleText::CalcDifference
       (mTextSizeAdjust != aOther.mTextSizeAdjust) ||
       (mLetterSpacing != aOther.mLetterSpacing) ||
       (mLineHeight != aOther.mLineHeight) ||
       (mTextIndent != aOther.mTextIndent) ||
       (mWordSpacing != aOther.mWordSpacing) ||
       (mTabSize != aOther.mTabSize))
     return NS_STYLE_HINT_REFLOW;
 
-  return CalcShadowDifference(mTextShadow, aOther.mTextShadow);
+  if (!AreShadowArraysEqual(mTextShadow, aOther.mTextShadow)) {
+    return nsChangeHint_UpdateSubtreeOverflow |
+           nsChangeHint_SchedulePaint |
+           nsChangeHint_RepaintFrame;
+  }
+  return NS_STYLE_HINT_NONE;
 }
 
 //-----------------------
 // nsStyleUserInterface
 //
 
 nsCursorImage::nsCursorImage()
   : mHaveHotspot(false)
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -859,19 +859,20 @@ struct nsStyleBorder {
   void* operator new(size_t sz, nsPresContext* aContext) CPP_THROW_NEW {
     return aContext->PresShell()->
       AllocateByObjectID(mozilla::eArenaObjectID_nsStyleBorder, sz);
   }
   void Destroy(nsPresContext* aContext);
 
   nsChangeHint CalcDifference(const nsStyleBorder& aOther) const;
   static nsChangeHint MaxDifference() {
-    return NS_CombineHint(NS_STYLE_HINT_REFLOW,
-                          NS_CombineHint(nsChangeHint_BorderStyleNoneChange,
-                                         nsChangeHint_NeutralChange));
+    return NS_STYLE_HINT_REFLOW |
+           nsChangeHint_UpdateOverflow |
+           nsChangeHint_BorderStyleNoneChange |
+           nsChangeHint_NeutralChange;
   }
   static nsChangeHint DifferenceAlwaysHandledForDescendants() {
     // CalcDifference never returns the reflow hints that are sometimes
     // handled for descendants as hints not handled for descendants.
     return nsChangeHint_NeedReflow |
            nsChangeHint_ReflowChangesSizeOrPosition |
            nsChangeHint_ClearAncestorIntrinsics;
   }
@@ -1647,17 +1648,18 @@ struct nsStyleText {
   void Destroy(nsPresContext* aContext) {
     this->~nsStyleText();
     aContext->PresShell()->
       FreeByObjectID(mozilla::eArenaObjectID_nsStyleText, this);
   }
 
   nsChangeHint CalcDifference(const nsStyleText& aOther) const;
   static nsChangeHint MaxDifference() {
-    return NS_STYLE_HINT_FRAMECHANGE;
+    return NS_STYLE_HINT_FRAMECHANGE |
+           nsChangeHint_UpdateSubtreeOverflow;
   }
   static nsChangeHint DifferenceAlwaysHandledForDescendants() {
     // CalcDifference never returns the reflow hints that are sometimes
     // handled for descendants as hints not handled for descendants.
     return nsChangeHint_NeedReflow |
            nsChangeHint_ReflowChangesSizeOrPosition |
            nsChangeHint_ClearAncestorIntrinsics;
   }
--- a/layout/style/test/test_dynamic_change_causing_reflow.html
+++ b/layout/style/test/test_dynamic_change_causing_reflow.html
@@ -43,22 +43,52 @@ https://bugzilla.mozilla.org/show_bug.cg
 const gTestcases = [
   // Things that shouldn't cause reflow:
   // -----------------------------------
   // * Adding an outline (e.g. for focus ring).
   {
     afterStyle:  "outline: 1px dotted black",
   },
 
-  // * Changing between completely-different outlines.
+  // * Changing between completely different outlines.
   {
     beforeStyle: "outline: 2px solid black",
     afterStyle:  "outline: 6px dashed yellow",
   },
 
+  // * Adding a box-shadow.
+  {
+    afterStyle: "box-shadow: inset 3px 3px gray",
+  },
+  {
+    afterStyle: "box-shadow: 0px 0px 10px 30px blue"
+  },
+
+  // * Changing between completely different box-shadow values,
+  // e.g. from an upper-left shadow to a bottom-right shadow:
+  {
+    beforeStyle: "box-shadow: -15px -20px teal",
+    afterStyle:  "box-shadow:  30px  40px yellow",
+  },
+
+  // * Adding a text-shadow.
+  {
+    afterStyle: "text-shadow: 3px 3px gray",
+  },
+  {
+    afterStyle: "text-shadow: 0px 0px 10px blue"
+  },
+
+  // * Changing between completely different text-shadow values,
+  // e.g. from an upper-left shadow to a bottom-right shadow:
+  {
+    beforeStyle: "text-shadow: -15px -20px teal",
+    afterStyle:  "text-shadow:  30px  40px yellow",
+  },
+
   // Things that *should* cause reflow:
   // ----------------------------------
   // (e.g. to make sure our counts are actually measuring something)
 
   // * Changing 'height' should cause reflow, but not frame construction.
   {
     beforeStyle: "height: 10px",
     afterStyle:  "height: 15px",