Bug 1373349 - Correct the origin of the content rect when creating scroll layers in WR. r?botond,jrmuizel
Also tweaks the documentation in FrameMetrics.h to be more detailed and
less misleading.
MozReview-Commit-ID: 183V2Q9kY6C
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -543,20 +543,24 @@ private:
// The same restrictions for mDisplayPort apply here.
CSSRect mCriticalDisplayPort;
// The scrollable bounds of a frame. This is determined by reflow.
// Ordinarily the x and y will be 0 and the width and height will be the
// size of the element being scrolled. However for RTL pages or elements
// the x value may be negative.
//
- // This is relative to the document. It is in the same coordinate space as
- // |mScrollOffset|, but a different coordinate space than |mViewport| and
- // |mDisplayPort|. Note also that this coordinate system is understood by
- // window.scrollTo().
+ // For scrollable frames that are overflow:hidden the x and y are usually
+ // set to the value of the current scroll offset, and the width and height
+ // will match the composition bounds width and height. In effect this reduces
+ // the scrollable range to 0.
+ //
+ // This is in the same coordinate space as |mScrollOffset|, but a different
+ // coordinate space than |mViewport| and |mDisplayPort|. Note also that this
+ // coordinate system is understood by window.scrollTo().
//
// This is valid on any layer unless it has no content.
CSSRect mScrollableRect;
// The cumulative resolution that the current frame has been painted at.
// This is the product of the pres-shell resolutions of the document
// containing this scroll frame and its ancestors, and any css-driven
// resolution. This information is provided by Gecko at layout/paint time.
--- a/gfx/layers/wr/ScrollingLayersHelper.cpp
+++ b/gfx/layers/wr/ScrollingLayersHelper.cpp
@@ -58,21 +58,23 @@ ScrollingLayersHelper::ScrollingLayersHe
PixelCastJustification::WebRenderHasUnitResolution);
// TODO: check coordinate systems are sane here
LayerRect clipBounds = ViewAs<LayerPixel>(
fm.GetCompositionBounds(),
PixelCastJustification::MovingDownToChildren);
// The content rect that we hand to PushScrollLayer should be relative to
// the same origin as the clipBounds that we hand to PushScrollLayer - that
// is, both of them should be relative to the stacking context `aStackingContext`.
- // However, when we get the scrollable rect from the FrameMetrics, it has
- // a nominal top-left of 0,0 (maybe different for RTL pages?) and so to
- // get it in the same coordinate space we're going to shift it by the
- // composition bounds top-left.
- contentRect.MoveBy(clipBounds.TopLeft());
+ // However, when we get the scrollable rect from the FrameMetrics, the origin
+ // has nothing to do with the position of the frame but instead represents
+ // the minimum allowed scroll offset of the scrollable content. While APZ
+ // uses this to clamp the scroll position, we don't need to send this to
+ // WebRender at all. Instead, we take the position from the composition
+ // bounds.
+ contentRect.MoveTo(clipBounds.TopLeft());
mBuilder->PushScrollLayer(fm.GetScrollId(),
aStackingContext.ToRelativeWrRect(contentRect),
aStackingContext.ToRelativeWrRect(clipBounds));
}
// The scrolled clip on the layer is "inside" all of the scrollable metadatas
// on that layer. That is, the clip scrolls along with the content in
// child layers. So we need to apply this after pushing all the scroll layers,