Bug 1372912 - Place the layer local clip in the right spot for fixed-pos layers. r?jrmuizel,mrobinson draft
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 15 Jun 2017 17:02:22 -0400
changeset 594986 d8059a6da2900162a9f9a99c6ab54eac4572ee0c
parent 594985 e4ecb253636daec99b0574d42d099ef883d84670
child 633589 2666a638a8be2f49869d7c534f7cf155f9e41777
push id64210
push userkgupta@mozilla.com
push dateThu, 15 Jun 2017 21:03:35 +0000
reviewersjrmuizel, mrobinson
bugs1372912
milestone56.0a1
Bug 1372912 - Place the layer local clip in the right spot for fixed-pos layers. r?jrmuizel,mrobinson This patch makes the bg-fixed-child-mask.html reftest pass by ensuring the clips for the test case are all in the correct spots. See the comments in the patch for a more detailed explanation. MozReview-Commit-ID: 21MAHr0PJ9B
gfx/layers/wr/ScrollingLayersHelper.cpp
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -37,16 +37,24 @@ ScrollingLayersHelper::ScrollingLayersHe
     // itself. Therefore we need to push this clip before we push the
     // corresponding scroll layer, so that when we set an async scroll position
     // on the scroll layer, the clip isn't affected by it.
     if (const Maybe<LayerClip>& clip = metadata.GetScrollClip()) {
       PushLayerClip(clip.ref(), aStackingContext);
     }
 
     const FrameMetrics& fm = layer->GetFrameMetrics(i - 1);
+    if (layer->GetIsFixedPosition() &&
+        layer->GetFixedPositionScrollContainerId() == fm.GetScrollId()) {
+      // If the layer contents are fixed for this metadata onwards, we need
+      // to insert the layer's local clip at this point in the clip tree,
+      // as a child of whatever's on the stack.
+      PushLayerLocalClip(aStackingContext);
+    }
+
     if (!fm.IsScrollable()) {
       continue;
     }
     LayerRect contentRect = ViewAs<LayerPixel>(
         fm.GetExpandedScrollableRect() * fm.GetDevPixelsPerCSSPixel(),
         PixelCastJustification::WebRenderHasUnitResolution);
     // TODO: check coordinate systems are sane here
     LayerRect clipBounds = ViewAs<LayerPixel>(
@@ -73,28 +81,33 @@ ScrollingLayersHelper::ScrollingLayersHe
     PushLayerClip(scrolledClip.ref(), aStackingContext);
   }
 
   // If the layer is marked as fixed-position, it is fixed relative to something
   // (the scroll layer referred to by GetFixedPositionScrollContainerId, hereafter
   // referred to as the "scroll container"). What this really means is that we
   // don't want this content to scroll with any scroll layer on the stack up to
   // and including the scroll container, but we do want it to scroll with any
-  // ancestor scroll layers. So we do a PushClipAndScrollInfo that maintains
+  // ancestor scroll layers.
+  // Also, the local clip on the layer (defined by layer->GetClipRect() and
+  // layer->GetMaskLayer()) also need to be fixed relative to the scroll
+  // container. This is why we inserted it into the clip tree during the
+  // loop above when we encountered the scroll container.
+  // At this point we do a PushClipAndScrollInfo that maintains
   // the current non-scrolling clip stack, but resets the scrolling clip stack
   // to the ancestor of the scroll container.
   if (layer->GetIsFixedPosition()) {
     FrameMetrics::ViewID fixedFor = layer->GetFixedPositionScrollContainerId();
     Maybe<FrameMetrics::ViewID> scrollsWith = mBuilder->ParentScrollIdFor(fixedFor);
     Maybe<wr::WrClipId> clipId = mBuilder->TopmostClipId();
     // Default to 0 if there is no ancestor, because 0 refers to the root scrollframe.
     mBuilder->PushClipAndScrollInfo(scrollsWith.valueOr(0), clipId.ptrOr(nullptr));
+  } else {
+    PushLayerLocalClip(aStackingContext);
   }
-
-  PushLayerLocalClip(aStackingContext);
 }
 
 void
 ScrollingLayersHelper::PushLayerLocalClip(const StackingContextHelper& aStackingContext)
 {
   Layer* layer = mLayer->GetLayer();
   Maybe<ParentLayerRect> clip;
   if (const Maybe<ParentLayerIntRect>& rect = layer->GetClipRect()) {
@@ -127,33 +140,41 @@ ScrollingLayersHelper::PushLayerClip(con
     mask = maskWrLayer->RenderMaskLayer(aSc, maskLayer->GetTransform());
   }
   mBuilder->PushClip(aSc.ToRelativeWrRect(clipRect), mask.ptrOr(nullptr));
 }
 
 ScrollingLayersHelper::~ScrollingLayersHelper()
 {
   Layer* layer = mLayer->GetLayer();
-  if (mPushedLayerLocalClip) {
-    mBuilder->PopClip();
-  }
   if (!mLayer->WrManager()->AsyncPanZoomEnabled()) {
+    if (mPushedLayerLocalClip) {
+      mBuilder->PopClip();
+    }
     return;
   }
+
   if (layer->GetIsFixedPosition()) {
     mBuilder->PopClipAndScrollInfo();
+  } else if (mPushedLayerLocalClip) {
+    mBuilder->PopClip();
   }
   if (layer->GetScrolledClip()) {
     mBuilder->PopClip();
   }
   for (uint32_t i = 0; i < layer->GetScrollMetadataCount(); i++) {
     const FrameMetrics& fm = layer->GetFrameMetrics(i);
     if (fm.IsScrollable()) {
       mBuilder->PopScrollLayer();
     }
+    if (layer->GetIsFixedPosition() &&
+        layer->GetFixedPositionScrollContainerId() == fm.GetScrollId() &&
+        mPushedLayerLocalClip) {
+      mBuilder->PopClip();
+    }
     const ScrollMetadata& metadata = layer->GetScrollMetadata(i);
     if (metadata.GetScrollClip()) {
       mBuilder->PopClip();
     }
   }
 }
 
 } // namespace layers