Bug 1238564 - When building a fixed/sticky display item, don't restore the clip until we're ready to build that item so that inner items aren't unnecessarily clipped. r?roc draft
authorMarkus Stange <mstange@themasta.com>
Tue, 23 Feb 2016 18:22:21 +0100
changeset 337030 ecf4774a47978fb0aa5ebde9346cf464ebc60ab6
parent 337029 e570f16ecedd80cba16051f0e1ac66764bc95815
child 337031 a23156202612ba4331f167f209eaa5c61d7dd402
push id12253
push usermstange@themasta.com
push dateFri, 04 Mar 2016 20:13:14 +0000
reviewersroc
bugs1238564
milestone47.0a1
Bug 1238564 - When building a fixed/sticky display item, don't restore the clip until we're ready to build that item so that inner items aren't unnecessarily clipped. r?roc This isn't really about regular clips, it's about scroll clips. If the inner item has an unnecessary scroll clip (one that's already on the parent), this can cause the inner item's bounds to be larger than necessary because, after the first patch in bug 1238564, it will include the whole bounds of async-scrollable scroll frames. Also, if e.g. the inner item is an opacity item, and it has a scroll clip that's not just the innermost ancestor scroll clip of all of its children, then FrameLayerBuilder's mContainerBounds == mAccumulatedChildBounds assertion can fail if the opacity item gets flattened away, because the child bounds won't have been enlarged for the scroll clip. There must be a better way to do the clip resetting in nsFrame::BuildDisplayListForStackingContext - the code is not pretty at all. But I'd rather get the tests passing first before we figure out how to clean it up. MozReview-Commit-ID: E1DdpN546va
layout/generic/nsFrame.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2191,17 +2191,17 @@ nsIFrame::BuildDisplayListForStackingCon
   if (aBuilder->ContainsBlendMode()) {
     DisplayListClipState::AutoSaveRestore blendContainerClipState(aBuilder);
     blendContainerClipState.Clear();
     resultList.AppendNewToTop(
       new (aBuilder) nsDisplayBlendContainer(aBuilder, this, &resultList,
                                              containerItemScrollClip));
   }
 
-  if (!isTransformed) {
+  if (!isTransformed && !useFixedPosition && !useStickyPosition) {
     // Restore saved clip state now so that any display items we create below
     // are clipped properly.
     clipState.Restore();
     if (didResetClip) {
       containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
     }
   }
 
@@ -2273,17 +2273,17 @@ nsIFrame::BuildDisplayListForStackingCon
         }
       }
       WrapSeparatorTransform(aBuilder, this, dirtyRect,
                              &nonparticipants, &participants, index++);
       resultList.AppendToTop(&participants);
     }
 
     // Restore clip state now so nsDisplayTransform is clipped properly.
-    if (!HasPerspective()) {
+    if (!HasPerspective() && !useFixedPosition && !useStickyPosition) {
       clipState.Restore();
       if (didResetClip) {
         containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
       }
     }
     // Revert to the dirtyrect coming in from the parent, without our transform
     // taken into account.
     buildingDisplayList.SetDirtyRect(dirtyRectOutsideTransform);
@@ -2298,35 +2298,44 @@ nsIFrame::BuildDisplayListForStackingCon
     buildingDisplayList.SetReferenceFrameAndCurrentOffset(outerReferenceFrame,
       GetOffsetToCrossDoc(outerReferenceFrame));
 
     nsDisplayTransform *transformItem =
       new (aBuilder) nsDisplayTransform(aBuilder, this, &resultList, dirtyRect);
     resultList.AppendNewToTop(transformItem);
 
     if (HasPerspective()) {
-      clipState.Restore();
-      if (didResetClip) {
-        containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
+      if (!useFixedPosition && !useStickyPosition) {
+        clipState.Restore();
+        if (didResetClip) {
+          containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
+        }
       }
       resultList.AppendNewToTop(
         new (aBuilder) nsDisplayPerspective(
           aBuilder, this,
           GetContainingBlock()->GetContent()->GetPrimaryFrame(), &resultList));
     }
 
     /* If we need an opacity item, but didn't do it earlier, add it now on the
      * outside of the transform.
      */
     if (useOpacity && !usingSVGEffects) {
       CreateOpacityItem(aBuilder, this, resultList, opacityItemForEventsOnly,
                         containerItemScrollClip);
     }
   }
 
+  if (useFixedPosition || useStickyPosition) {
+    clipState.Restore();
+    if (didResetClip) {
+      containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
+    }
+  }
+
   /* If we have sticky positioning, wrap it in a sticky position item.
    */
   if (useFixedPosition) {
     resultList.AppendNewToTop(
         new (aBuilder) nsDisplayFixedPosition(aBuilder, this, &resultList));
   } else if (useStickyPosition) {
     resultList.AppendNewToTop(
         new (aBuilder) nsDisplayStickyPosition(aBuilder, this, &resultList));