Bug 1410527 - Update how we tell WR about position:sticky elements. r?mstange
My original understanding of the API was flawed, and so while I had
position:sticky working in some cases it didn't work properly in a lot
of other cases. This patch corrects the usage of the API to match what
WR is expecting and fixes a lot of test cases.
MozReview-Commit-ID: AdMux19Fk9U
--- a/gfx/layers/wr/StackingContextHelper.cpp
+++ b/gfx/layers/wr/StackingContextHelper.cpp
@@ -55,22 +55,16 @@ StackingContextHelper::StackingContextHe
StackingContextHelper::~StackingContextHelper()
{
if (mBuilder) {
mBuilder->PopStackingContext();
}
}
-void
-StackingContextHelper::AdjustOrigin(const LayoutDevicePoint& aDelta)
-{
- mOrigin += aDelta;
-}
-
wr::LayoutRect
StackingContextHelper::ToRelativeLayoutRect(const LayoutDeviceRect& aRect) const
{
return wr::ToLayoutRect(RoundedToInt(aRect - mOrigin));
}
} // namespace layers
} // namespace mozilla
--- a/gfx/layers/wr/StackingContextHelper.h
+++ b/gfx/layers/wr/StackingContextHelper.h
@@ -41,18 +41,16 @@ public:
// of the tree, so that we have a StackingContextHelper to pass down into
// the RenderLayer traversal, but don't actually want it to push a stacking
// context on the display list builder.
StackingContextHelper();
// Pops the stacking context, if one was pushed during the constructor.
~StackingContextHelper();
- void AdjustOrigin(const LayoutDevicePoint& aDelta);
-
// When this StackingContextHelper is in scope, this function can be used
// to convert a rect from the layer system's coordinate space to a LayoutRect
// that is relative to the stacking context. This is useful because most
// things that are pushed inside the stacking context need to be relative
// to the stacking context.
// We allow passing in a LayoutDeviceRect for convenience because in a lot of
// cases with WebRender display item generate the layout device space is the
// same as the layer space. (TODO: try to make this more explicit somehow).
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7271,107 +7271,128 @@ nsDisplayStickyPosition::BuildLayer(nsDi
aContainerParameters.mXScale,
NSAppUnitsToFloatPixels(inner.height, factor) *
aContainerParameters.mYScale);
layer->SetStickyPositionData(scrollId, stickyOuter, stickyInner);
return layer.forget();
}
+// Returns the smallest distance from "0" to the range [min, max] where
+// min <= max.
+static nscoord DistanceToRange(nscoord min, nscoord max) {
+ MOZ_ASSERT(min <= max);
+ if (max < 0) {
+ return max;
+ }
+ if (min > 0) {
+ return min;
+ }
+ MOZ_ASSERT(min <= 0 && max >= 0);
+ return 0;
+}
+
bool
nsDisplayStickyPosition::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
mozilla::wr::IpcResourceUpdateQueue& aResources,
const StackingContextHelper& aSc,
WebRenderLayerManager* aManager,
nsDisplayListBuilder* aDisplayListBuilder)
{
- LayoutDevicePoint scTranslation;
StickyScrollContainer* stickyScrollContainer = StickyScrollContainer::GetStickyScrollContainerForFrame(mFrame);
if (stickyScrollContainer) {
float auPerDevPixel = mFrame->PresContext()->AppUnitsPerDevPixel();
bool snap;
nsRect itemBounds = GetBounds(aDisplayListBuilder, &snap);
- // The itemBounds here already take into account the main-thread
- // position:sticky implementation, so we need to unapply that.
- nsIFrame* firstCont = nsLayoutUtils::FirstContinuationOrIBSplitSibling(mFrame);
- nsPoint translation = stickyScrollContainer->ComputePosition(firstCont) - firstCont->GetNormalPosition();
- itemBounds.MoveBy(-translation);
- scTranslation = LayoutDevicePoint::FromAppUnits(translation, auPerDevPixel);
-
- LayoutDeviceRect bounds = LayoutDeviceRect::FromAppUnits(itemBounds, auPerDevPixel);
-
Maybe<wr::StickySideConstraint> top;
Maybe<wr::StickySideConstraint> right;
Maybe<wr::StickySideConstraint> bottom;
Maybe<wr::StickySideConstraint> left;
nsRect outer;
nsRect inner;
stickyScrollContainer->GetScrollRanges(mFrame, &outer, &inner);
+ nsIFrame* scrollFrame = do_QueryFrame(stickyScrollContainer->ScrollFrame());
+ nsPoint offset = scrollFrame->GetOffsetTo(ReferenceFrame());
+
+ // Adjust the scrollPort coordinates to be relative to the reference frame,
+ // so that it is in the same space as everything else.
nsRect scrollPort = stickyScrollContainer->ScrollFrame()->GetScrollPortRect();
+ scrollPort += offset;
// The following computations make more sense upon understanding the
// semantics of "inner" and "outer", which is explained in the comment on
// SetStickyPositionData in Layers.h.
if (outer.YMost() != inner.YMost()) {
// Question: How far will itemBounds.y be from the top of the scrollport
- // when we have scrolled down from the current scroll position of "0" to a
- // scroll position of "inner.YMost()" (which is >= 0 since we are
- // scrolling down)?
- // Answer: (itemBounds.y - 0) - (inner.YMost() - 0)
- // == itemBounds.y - inner.YMost()
- float margin = NSAppUnitsToFloatPixels(itemBounds.y - inner.YMost(), auPerDevPixel);
- // The scroll distance during which the item should remain "stuck"
- float maxOffset = NSAppUnitsToFloatPixels(outer.YMost() - inner.YMost(), auPerDevPixel);
+ // when we have scrolled from the current scroll position of "0" to
+ // reach the range [inner.YMost(), outer.YMost()] where the item gets
+ // stuck?
+ // Answer: the current distance is "itemBounds.y - scrollPort.y". That
+ // needs to be adjusted by the distance to the range. If the distance is
+ // negative (i.e. inner.YMost() <= outer.YMost() < 0) then we would be
+ // scrolling upwards (decreasing scroll offset) to reach that range,
+ // which would increase itemBounds.y and make it farther away from the
+ // top of the scrollport. So in that case the adjustment is -distance.
+ // If the distance is positive (0 < inner.YMost() <= outer.YMost()) then
+ // we would be scrolling downwards, itemBounds.y would decrease, and we
+ // again need to adjust by -distance. If we are already in the range
+ // then no adjustment is needed and distance is 0 so again using
+ // -distance works.
+ nscoord distance = DistanceToRange(inner.YMost(), outer.YMost());
+ float margin = NSAppUnitsToFloatPixels(itemBounds.y - scrollPort.y - distance, auPerDevPixel);
+ // Question: Given the current state, what is the range during which
+ // WR will have to apply an adjustment to the item (in order to prevent
+ // the item from visually moving) as a result of async scrolling?
+ // Answer: [inner.YMost(), outer.YMost()]. But right now the WR API
+ // doesn't allow us to provide the whole range; it just takes one side
+ // of the range and assumes it has a particular sign. Bug 1411627 will
+ // fix this more completely but for now we do the best we can. Note that
+ // this value also needs to be converted from being relative to the
+ // scrollframe to being relative to the reference frame, so we have to
+ // adjust it by |offset|.
+ float maxOffset = NSAppUnitsToFloatPixels(std::max(0, outer.YMost() + offset.y), auPerDevPixel);
top = Some(wr::StickySideConstraint { margin, maxOffset });
}
if (outer.y != inner.y) {
- // Question: How far will itemBounds.YMost() be from the bottom of the
- // scrollport when we have scrolled up from the current scroll position of
- // "0" to a scroll position of "inner.y" (which is <= 0 since we are
- // scrolling up)?
- // Answer: (scrollPort.height - itemBounds.YMost()) - (0 - inner.y)
- // == scrollPort.height - itemBounds.YMost() + inner.y
- float margin = NSAppUnitsToFloatPixels(scrollPort.height - itemBounds.YMost() + inner.y, auPerDevPixel);
- // The scroll distance during which the item should remain "stuck"
- float maxOffset = NSAppUnitsToFloatPixels(outer.y - inner.y, auPerDevPixel);
+ // Similar logic as in the previous section, but this time we care about
+ // the distance from itemBounds.YMost() to scrollPort.YMost().
+ nscoord distance = DistanceToRange(outer.y, inner.y);
+ float margin = NSAppUnitsToFloatPixels(scrollPort.YMost() - itemBounds.YMost() + distance, auPerDevPixel);
+ // And here WR will be moving the item upwards rather than downwards so
+ // again things are inverted from the previous block.
+ float maxOffset = NSAppUnitsToFloatPixels(std::min(0, outer.y + offset.y), auPerDevPixel);
bottom = Some(wr::StickySideConstraint { margin, maxOffset });
}
// Same as above, but for the x-axis
if (outer.XMost() != inner.XMost()) {
- float margin = NSAppUnitsToFloatPixels(itemBounds.x - inner.XMost(), auPerDevPixel);
- float maxOffset = NSAppUnitsToFloatPixels(outer.XMost() - inner.XMost(), auPerDevPixel);
+ nscoord distance = DistanceToRange(inner.XMost(), outer.XMost());
+ float margin = NSAppUnitsToFloatPixels(itemBounds.x - scrollPort.x - distance, auPerDevPixel);
+ float maxOffset = NSAppUnitsToFloatPixels(std::max(0, outer.XMost() + offset.x), auPerDevPixel);
left = Some(wr::StickySideConstraint { margin, maxOffset });
}
if (outer.x != inner.x) {
- float margin = NSAppUnitsToFloatPixels(scrollPort.width - itemBounds.XMost() + inner.x, auPerDevPixel);
- float maxOffset = NSAppUnitsToFloatPixels(outer.x - inner.x, auPerDevPixel);
+ nscoord distance = DistanceToRange(outer.x, inner.x);
+ float margin = NSAppUnitsToFloatPixels(scrollPort.XMost() - itemBounds.XMost() + distance, auPerDevPixel);
+ float maxOffset = NSAppUnitsToFloatPixels(std::min(0, outer.x + offset.x), auPerDevPixel);
right = Some(wr::StickySideConstraint { margin, maxOffset });
}
+ LayoutDeviceRect bounds = LayoutDeviceRect::FromAppUnits(itemBounds, auPerDevPixel);
wr::WrStickyId id = aBuilder.DefineStickyFrame(aSc.ToRelativeLayoutRect(bounds),
top.ptrOr(nullptr), right.ptrOr(nullptr), bottom.ptrOr(nullptr), left.ptrOr(nullptr));
aBuilder.PushStickyFrame(id, GetClipChain());
}
- // All the things inside this position:sticky item also have the main-thread
- // translation already applied, so we need to make sure that gets unapplied.
- // The easiest way to do it is to just create a new stacking context with an
- // adjusted origin and use that for the nested items. This way all the
- // ToRelativeLayoutRect calls on this StackingContextHelper object will
- // include the necessary adjustment.
- StackingContextHelper sc(aSc, aBuilder);
- sc.AdjustOrigin(scTranslation);
-
- nsDisplayOwnLayer::CreateWebRenderCommands(aBuilder, aResources, sc,
+ nsDisplayOwnLayer::CreateWebRenderCommands(aBuilder, aResources, aSc,
aManager, aDisplayListBuilder);
if (stickyScrollContainer) {
aBuilder.PopStickyFrame(GetClipChain());
}
return true;
}
--- a/layout/reftests/position-sticky/reftest.list
+++ b/layout/reftests/position-sticky/reftest.list
@@ -8,44 +8,44 @@ fuzzy-if(Android,2,1568) == top-6.html t
== bottom-2a.html bottom-2-ref.html
== bottom-2b.html bottom-2-ref.html
== bottom-2c.html bottom-2-ref.html
== bottom-3.html bottom-3-ref.html
== bottom-4.html bottom-4-ref.html
fuzzy-if(Android,2,4) == left-1.html left-1-ref.html
fuzzy-if(Android,2,4) == left-2.html left-2-ref.html
== left-3.html left-3-ref.html
-fails-if(webrender) == right-1.html right-1-ref.html
-fuzzy-if(Android,2,4) fails-if(webrender) == right-2.html right-2-ref.html
+== right-1.html right-1-ref.html
+fuzzy-if(Android,2,4) == right-2.html right-2-ref.html
fuzzy-if(Android,2,4) == right-3.html right-3-ref.html
== margin-1.html margin-1-ref.html
== padding-1.html padding-1-ref.html
== padding-2.html padding-2-ref.html
-fails-if(webrender) == padding-3.html padding-3-ref.html
+== padding-3.html padding-3-ref.html
== overcontain-1.html overcontain-1-ref.html
== initial-1.html initial-1-ref.html
== initial-scroll-1.html initial-scroll-1-ref.html
== scrollframe-reflow-1.html scrollframe-reflow-1-ref.html
== scrollframe-reflow-2.html scrollframe-reflow-2-ref.html
== scrollframe-auto-1.html scrollframe-auto-1-ref.html
fuzzy-if(Android,2,3) == stacking-context-1.html stacking-context-1-ref.html
== top-bottom-1.html top-bottom-1-ref.html
== top-bottom-2.html top-bottom-2-ref.html
== top-bottom-3.html top-bottom-3-ref.html
== left-right-1.html left-right-1-ref.html
== left-right-2.html left-right-2-ref.html
== left-right-3.html left-right-3-ref.html
-fuzzy-if(Android,4,810) fails-if(webrender) == containing-block-1.html containing-block-1-ref.html
+fuzzy-if(Android,4,810) == containing-block-1.html containing-block-1-ref.html
== overconstrained-1.html overconstrained-1-ref.html
== overconstrained-2.html overconstrained-2-ref.html
== overconstrained-3.html overconstrained-3-ref.html
== inline-1.html inline-1-ref.html
== inline-2.html inline-2-ref.html
fuzzy-if(OSX,99,210) == inline-3.html inline-3-ref.html
-fails-if(webrender) skip-if(!asyncPan) == inline-4.html inline-4-ref.html # bug 1366295 for webrender
+skip-if(!asyncPan) == inline-4.html inline-4-ref.html
fails == column-contain-1a.html column-contain-1-ref.html
== column-contain-1b.html column-contain-1-ref.html
== column-contain-2.html column-contain-2-ref.html
== block-in-inline-1.html block-in-inline-1-ref.html
fuzzy-if(skiaContent,1,22) fuzzy-if(winWidget&&!layersGPUAccelerated,116,1320) fuzzy-if(Android,8,1533) == block-in-inline-2.html block-in-inline-2-ref.html
fuzzy-if(Android,8,630) fuzzy-if(OSX,1,11) fuzzy-if(skiaContent,1,220) fuzzy-if(winWidget&&!layersGPUAccelerated,116,1320) == block-in-inline-3.html block-in-inline-3-ref.html
== block-in-inline-continuations.html block-in-inline-continuations-ref.html
fuzzy-if(winWidget&&!layersGPUAccelerated,140,140) == inner-table-1.html inner-table-1-ref.html