Bug 1411627 - Send the applied offset for sticky frames to WR. r?mstange draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 07 Nov 2017 10:16:48 -0500
changeset 694247 7267fe056e173a3730ee270e172aa83beaf58de6
parent 694246 74abebf13326c2ae5104e17f7bd34dae3054b5a6
child 694248 146bed0eaab07a01fdf2a90986f405ac874ec252
push id88084
push userkgupta@mozilla.com
push dateTue, 07 Nov 2017 15:18:14 +0000
reviewersmstange
bugs1411627
milestone58.0a1
Bug 1411627 - Send the applied offset for sticky frames to WR. r?mstange There are cases where we do a main-thread paint at a scroll position where sticky offsets have been applied in order to keep sticky items visually unmoving. From that paint, it's possible to do an async-scroll in the direction that reduces the sticky offset. In order for WR to handle this case properly we need to tell WR how much of a sticky offset was already applied on the main thread. MozReview-Commit-ID: 79DsfPpsIfA
gfx/layers/wr/WebRenderLayersLogging.cpp
gfx/layers/wr/WebRenderLayersLogging.h
gfx/webrender_bindings/WebRenderAPI.cpp
gfx/webrender_bindings/WebRenderAPI.h
gfx/webrender_bindings/src/bindings.rs
gfx/webrender_bindings/webrender_ffi_generated.h
layout/painting/nsDisplayList.cpp
--- a/gfx/layers/wr/WebRenderLayersLogging.cpp
+++ b/gfx/layers/wr/WebRenderLayersLogging.cpp
@@ -70,10 +70,19 @@ AppendToString(std::stringstream& aStrea
     aStream << "ImageRendering::Pixelated"; break;
   case wr::ImageRendering::Sentinel:
     NS_ERROR("unknown texture filter");
     aStream << "???";
   }
   aStream << sfx;
 }
 
+void
+AppendToString(std::stringstream& aStream, wr::LayoutVector2D aVector,
+               const char* pfx, const char* sfx)
+{
+  aStream << pfx;
+  aStream << nsPrintfCString("(x=%f, y=%f)", aVector.x, aVector.y).get();
+  aStream << sfx;
+}
+
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/wr/WebRenderLayersLogging.h
+++ b/gfx/layers/wr/WebRenderLayersLogging.h
@@ -16,15 +16,19 @@ namespace layers {
 void
 AppendToString(std::stringstream& aStream, wr::MixBlendMode aMixBlendMode,
                const char* pfx="", const char* sfx="");
 
 void
 AppendToString(std::stringstream& aStream, wr::ImageRendering aTextureFilter,
                const char* pfx="", const char* sfx="");
 
+void
+AppendToString(std::stringstream& aStream, wr::LayoutVector2D aVector,
+               const char* pfx="", const char* sfx="");
+
 } // namespace layers
 } // namespace mozilla
 
 // this ensures that the WebRender AppendToString's are in scope for Stringify
 #include "LayersLogging.h"
 
 #endif // GFX_WEBRENDERLAYERSLOGGING_H
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -806,29 +806,32 @@ DisplayListBuilder::GetCacheOverride(con
 
 wr::WrStickyId
 DisplayListBuilder::DefineStickyFrame(const wr::LayoutRect& aContentRect,
                                       const float* aTopMargin,
                                       const float* aRightMargin,
                                       const float* aBottomMargin,
                                       const float* aLeftMargin,
                                       const StickyOffsetBounds& aVerticalBounds,
-                                      const StickyOffsetBounds& aHorizontalBounds)
+                                      const StickyOffsetBounds& aHorizontalBounds,
+                                      const wr::LayoutVector2D& aAppliedOffset)
 {
   uint64_t id = wr_dp_define_sticky_frame(mWrState, aContentRect, aTopMargin,
-      aRightMargin, aBottomMargin, aLeftMargin, aVerticalBounds, aHorizontalBounds);
-  WRDL_LOG("DefineSticky id=%" PRIu64 " c=%s t=%s r=%s b=%s l=%s v=%s h=%s\n",
+      aRightMargin, aBottomMargin, aLeftMargin, aVerticalBounds, aHorizontalBounds,
+      aAppliedOffset);
+  WRDL_LOG("DefineSticky id=%" PRIu64 " c=%s t=%s r=%s b=%s l=%s v=%s h=%s a=%s\n",
       mWrState, id,
       Stringify(aContentRect).c_str(),
       aTopMargin ? Stringify(*aTopMargin).c_str() : "none",
       aRightMargin ? Stringify(*aRightMargin).c_str() : "none",
       aBottomMargin ? Stringify(*aBottomMargin).c_str() : "none",
       aLeftMargin ? Stringify(*aLeftMargin).c_str() : "none",
       Stringify(aVerticalBounds).c_str(),
-      Stringify(aHorizontalBounds).c_str());
+      Stringify(aHorizontalBounds).c_str(),
+      Stringify(aAppliedOffset).c_str());
   return wr::WrStickyId { id };
 }
 
 void
 DisplayListBuilder::PushStickyFrame(const wr::WrStickyId& aStickyId,
                                     const DisplayItemClipChain* aParent)
 {
   wr_dp_push_clip(mWrState, aStickyId.id);
--- a/gfx/webrender_bindings/WebRenderAPI.h
+++ b/gfx/webrender_bindings/WebRenderAPI.h
@@ -242,17 +242,18 @@ public:
   Maybe<wr::WrClipId> GetCacheOverride(const DisplayItemClipChain* aParent);
 
   wr::WrStickyId DefineStickyFrame(const wr::LayoutRect& aContentRect,
                                    const float* aTopMargin,
                                    const float* aRightMargin,
                                    const float* aBottomMargin,
                                    const float* aLeftMargin,
                                    const StickyOffsetBounds& aVerticalBounds,
-                                   const StickyOffsetBounds& aHorizontalBounds);
+                                   const StickyOffsetBounds& aHorizontalBounds,
+                                   const wr::LayoutVector2D& aAppliedOffset);
   void PushStickyFrame(const wr::WrStickyId& aStickyId,
                        const DisplayItemClipChain* aParent);
   void PopStickyFrame(const DisplayItemClipChain* aParent);
 
   bool IsScrollLayerDefined(layers::FrameMetrics::ViewID aScrollId) const;
   void DefineScrollLayer(const layers::FrameMetrics::ViewID& aScrollId,
                          const Maybe<layers::FrameMetrics::ViewID>& aAncestorScrollId,
                          const Maybe<wr::WrClipId>& aAncestorClipId,
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -1326,28 +1326,28 @@ pub extern "C" fn wr_dp_pop_clip(state: 
 #[no_mangle]
 pub extern "C" fn wr_dp_define_sticky_frame(state: &mut WrState,
                                             content_rect: LayoutRect,
                                             top_margin: *const f32,
                                             right_margin: *const f32,
                                             bottom_margin: *const f32,
                                             left_margin: *const f32,
                                             vertical_bounds: StickyOffsetBounds,
-                                            horizontal_bounds: StickyOffsetBounds)
+                                            horizontal_bounds: StickyOffsetBounds,
+                                            applied_offset: LayoutVector2D)
                                             -> u64 {
     assert!(unsafe { is_in_main_thread() });
     let clip_id = state.frame_builder.dl_builder.define_sticky_frame(
         None, content_rect, SideOffsets2D::new(
             unsafe { top_margin.as_ref() }.cloned(),
             unsafe { right_margin.as_ref() }.cloned(),
             unsafe { bottom_margin.as_ref() }.cloned(),
             unsafe { left_margin.as_ref() }.cloned()
         ),
-        vertical_bounds, horizontal_bounds,
-        LayoutVector2D::new(0.0, 0.0));
+        vertical_bounds, horizontal_bounds, applied_offset);
     match clip_id {
         ClipId::Clip(id, pipeline_id) => {
             assert!(pipeline_id == state.pipeline_id);
             id
         },
         _ => panic!("Got unexpected clip id type"),
     }
 }
--- a/gfx/webrender_bindings/webrender_ffi_generated.h
+++ b/gfx/webrender_bindings/webrender_ffi_generated.h
@@ -541,16 +541,31 @@ struct StickyOffsetBounds {
   float max;
 
   bool operator==(const StickyOffsetBounds& aOther) const {
     return min == aOther.min &&
            max == aOther.max;
   }
 };
 
+// A 2d Vector tagged with a unit.
+struct TypedVector2D_f32__LayerPixel {
+  float x;
+  float y;
+
+  bool operator==(const TypedVector2D_f32__LayerPixel& aOther) const {
+    return x == aOther.x &&
+           y == aOther.y;
+  }
+};
+
+typedef TypedVector2D_f32__LayerPixel LayerVector2D;
+
+typedef LayerVector2D LayoutVector2D;
+
 struct BorderWidths {
   float left;
   float top;
   float right;
   float bottom;
 
   bool operator==(const BorderWidths& aOther) const {
     return left == aOther.left &&
@@ -621,31 +636,16 @@ struct NinePatchDescriptor {
 
   bool operator==(const NinePatchDescriptor& aOther) const {
     return width == aOther.width &&
            height == aOther.height &&
            slice == aOther.slice;
   }
 };
 
-// A 2d Vector tagged with a unit.
-struct TypedVector2D_f32__LayerPixel {
-  float x;
-  float y;
-
-  bool operator==(const TypedVector2D_f32__LayerPixel& aOther) const {
-    return x == aOther.x &&
-           y == aOther.y;
-  }
-};
-
-typedef TypedVector2D_f32__LayerPixel LayerVector2D;
-
-typedef LayerVector2D LayoutVector2D;
-
 struct Shadow {
   LayoutVector2D offset;
   ColorF color;
   float blur_radius;
 
   bool operator==(const Shadow& aOther) const {
     return offset == aOther.offset &&
            color == aOther.color &&
@@ -1073,17 +1073,18 @@ WR_FUNC;
 WR_INLINE
 uint64_t wr_dp_define_sticky_frame(WrState *aState,
                                    LayoutRect aContentRect,
                                    const float *aTopMargin,
                                    const float *aRightMargin,
                                    const float *aBottomMargin,
                                    const float *aLeftMargin,
                                    StickyOffsetBounds aVerticalBounds,
-                                   StickyOffsetBounds aHorizontalBounds)
+                                   StickyOffsetBounds aHorizontalBounds,
+                                   LayoutVector2D aAppliedOffset)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_pop_all_shadows(WrState *aState)
 WR_FUNC;
 
 WR_INLINE
 void wr_dp_pop_clip(WrState *aState)
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7341,16 +7341,17 @@ nsDisplayStickyPosition::CreateWebRender
     nsRect itemBounds = GetBounds(aDisplayListBuilder, &snap);
 
     Maybe<float> topMargin;
     Maybe<float> rightMargin;
     Maybe<float> bottomMargin;
     Maybe<float> leftMargin;
     wr::StickyOffsetBounds vBounds = { 0.0, 0.0 };
     wr::StickyOffsetBounds hBounds = { 0.0, 0.0 };
+    nsPoint appliedOffset;
 
     nsRect outer;
     nsRect inner;
     stickyScrollContainer->GetScrollRanges(mFrame, &outer, &inner);
 
     nsIFrame* scrollFrame = do_QueryFrame(stickyScrollContainer->ScrollFrame());
     nsPoint offset = scrollFrame->GetOffsetToCrossDoc(ReferenceFrame());
 
@@ -7376,54 +7377,80 @@ nsDisplayStickyPosition::CreateWebRender
       // 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());
       topMargin = Some(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|.
-      vBounds.max = NSAppUnitsToFloatPixels(std::max(0, outer.YMost() + offset.y), auPerDevPixel);
+      // Question: What is the maximum positive ("downward") offset that WR
+      // will have to apply to this item in order to prevent the item from
+      // visually moving?
+      // Answer: Since the item is "sticky" in the range [inner.YMost(), outer.YMost()],
+      // the maximum offset will be the size of the range, which is
+      // outer.YMost() - inner.YMost().
+      vBounds.max = NSAppUnitsToFloatPixels(outer.YMost() - inner.YMost(), auPerDevPixel);
+      // Question: how much of an offset has layout already applied to the item?
+      // Answer: if we are
+      // (a) inside the sticky range (inner.YMost() < 0 <= outer.YMost()), or
+      // (b) past the sticky range (inner.YMost() < outer.YMost() < 0)
+      // then layout has already applied some offset to the position of the
+      // item. The amount of the adjustment is |0 - inner.YMost()| in case (a)
+      // and |outer.YMost() - inner.YMost()| in case (b).
+      if (inner.YMost() < 0) {
+        appliedOffset.y = std::min(0, outer.YMost()) - inner.YMost();
+        MOZ_ASSERT(appliedOffset.y > 0);
+      }
     }
     if (outer.y != inner.y) {
       // 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);
       bottomMargin = Some(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.
-      vBounds.min = NSAppUnitsToFloatPixels(std::min(0, outer.y + offset.y), auPerDevPixel);
+      vBounds.min = NSAppUnitsToFloatPixels(outer.y - inner.y, auPerDevPixel);
+      // We can't have appliedOffset be both positive and negative, and the top
+      // adjustment takes priority. So here we only update appliedOffset.y if
+      // it wasn't set by the top-sticky case above.
+      if (appliedOffset.y == 0 && inner.y > 0) {
+        appliedOffset.y = std::max(0, outer.y) - inner.y;
+        MOZ_ASSERT(appliedOffset.y < 0);
+      }
     }
     // Same as above, but for the x-axis
     if (outer.XMost() != inner.XMost()) {
       nscoord distance = DistanceToRange(inner.XMost(), outer.XMost());
       leftMargin = Some(NSAppUnitsToFloatPixels(itemBounds.x - scrollPort.x - distance, auPerDevPixel));
-      hBounds.max = NSAppUnitsToFloatPixels(std::max(0, outer.XMost() + offset.x), auPerDevPixel);
+      hBounds.max = NSAppUnitsToFloatPixels(outer.XMost() - inner.XMost(), auPerDevPixel);
+      if (inner.XMost() < 0) {
+        appliedOffset.x = std::min(0, outer.XMost()) - inner.XMost();
+        MOZ_ASSERT(appliedOffset.x > 0);
+      }
     }
     if (outer.x != inner.x) {
       nscoord distance = DistanceToRange(outer.x, inner.x);
       rightMargin = Some(NSAppUnitsToFloatPixels(scrollPort.XMost() - itemBounds.XMost() + distance, auPerDevPixel));
-      hBounds.min = NSAppUnitsToFloatPixels(std::min(0, outer.x + offset.x), auPerDevPixel);
+      hBounds.min = NSAppUnitsToFloatPixels(outer.x - inner.x, auPerDevPixel);
+      if (appliedOffset.x == 0 && inner.x > 0) {
+        appliedOffset.x = std::max(0, outer.x) - inner.x;
+        MOZ_ASSERT(appliedOffset.x < 0);
+      }
     }
 
     LayoutDeviceRect bounds = LayoutDeviceRect::FromAppUnits(itemBounds, auPerDevPixel);
+    wr::LayoutVector2D applied = {
+      NSAppUnitsToFloatPixels(appliedOffset.x, auPerDevPixel),
+      NSAppUnitsToFloatPixels(appliedOffset.y, auPerDevPixel)
+    };
     wr::WrStickyId id = aBuilder.DefineStickyFrame(aSc.ToRelativeLayoutRect(bounds),
         topMargin.ptrOr(nullptr), rightMargin.ptrOr(nullptr),
         bottomMargin.ptrOr(nullptr), leftMargin.ptrOr(nullptr),
-        vBounds, hBounds);
+        vBounds, hBounds, applied);
 
     aBuilder.PushStickyFrame(id, GetClipChain());
   }
 
   nsDisplayOwnLayer::CreateWebRenderCommands(aBuilder, aResources, aSc,
       aManager, aDisplayListBuilder);
 
   if (stickyScrollContainer) {