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
authorMarkus Stange <mstange@themasta.com>
Tue, 23 Feb 2016 18:22:21 +0100
changeset 287365 84a877cbf631931dd3b6e6dc9444517a10b5c473
parent 287364 66c5a85656b61a93bf18e8cfd351a4ea7d639937
child 287366 cbfd284f87917880578044190815c4a1a028e9da
push id30068
push usercbook@mozilla.com
push dateWed, 09 Mar 2016 10:46:58 +0000
treeherdermozilla-central@af7c0cb0798f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1238564
milestone47.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 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));