Bug 1422950 - Move the area calculation to the display item constructor to hit the ToReferenceFrame fastpath. r?mstange draft
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 04 Dec 2017 15:47:29 -0500
changeset 707165 5c226ec0852871df5f4ce96220012dd48aef0be7
parent 707157 7d191882de19faa537753b2deaea9444277a6533
child 742855 e386c4b442ea6265bd9cb18d4b85ad98c0d60369
push id92020
push userkgupta@mozilla.com
push dateMon, 04 Dec 2017 20:47:51 +0000
reviewersmstange
bugs1422950
milestone59.0a1
Bug 1422950 - Move the area calculation to the display item constructor to hit the ToReferenceFrame fastpath. r?mstange MozReview-Commit-ID: 6ROpprO26JV
layout/generic/nsGfxScrollFrame.cpp
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -3645,18 +3645,18 @@ ScrollFrameHelper::BuildDisplayList(nsDi
     // a displayport for this frame. We'll add the item later on.
     if (!mWillBuildScrollableLayer) {
       int32_t zIndex =
         MaxZIndexInListOfItemsContainedInFrame(scrolledContent.PositionedDescendants(), mOuter);
       if (aBuilder->BuildCompositorHitTestInfo()) {
         CompositorHitTestInfo info = CompositorHitTestInfo::eVisibleToHitTest
                                    | CompositorHitTestInfo::eDispatchToContent;
         nsDisplayCompositorHitTestInfo* hitInfo =
-            new (aBuilder) nsDisplayCompositorHitTestInfo(aBuilder, mScrolledFrame, info, 1);
-        hitInfo->SetArea(mScrollPort + aBuilder->ToReferenceFrame(mOuter));
+            new (aBuilder) nsDisplayCompositorHitTestInfo(aBuilder, mScrolledFrame, info, 1,
+                Some(mScrollPort + aBuilder->ToReferenceFrame(mOuter)));
         AppendInternalItemToTop(scrolledContent, hitInfo, zIndex);
       }
       if (aBuilder->IsBuildingLayerEventRegions()) {
         nsDisplayLayerEventRegions* inactiveRegionItem =
             new (aBuilder) nsDisplayLayerEventRegions(aBuilder, mScrolledFrame, 1);
         inactiveRegionItem->AddInactiveScrollPort(mScrolledFrame, mScrollPort + aBuilder->ToReferenceFrame(mOuter));
         AppendInternalItemToTop(scrolledContent, inactiveRegionItem, zIndex);
       }
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -4939,17 +4939,18 @@ nsDisplayEventReceiver::CreateWebRenderC
   // list for WebRender consumption, so this function should never get called.
   MOZ_ASSERT(false);
   return true;
 }
 
 nsDisplayCompositorHitTestInfo::nsDisplayCompositorHitTestInfo(nsDisplayListBuilder* aBuilder,
                                                                nsIFrame* aFrame,
                                                                mozilla::gfx::CompositorHitTestInfo aHitTestInfo,
-                                                               uint32_t aIndex)
+                                                               uint32_t aIndex,
+                                                               const mozilla::Maybe<nsRect>& aArea)
   : nsDisplayEventReceiver(aBuilder, aFrame)
   , mHitTestInfo(aHitTestInfo)
   , mIndex(aIndex)
 {
   MOZ_COUNT_CTOR(nsDisplayCompositorHitTestInfo);
   // We should never even create this display item if we're not building
   // compositor hit-test info or if the computed hit info indicated the
   // frame is invisible to hit-testing
@@ -4957,56 +4958,55 @@ nsDisplayCompositorHitTestInfo::nsDispla
   MOZ_ASSERT(mHitTestInfo != mozilla::gfx::CompositorHitTestInfo::eInvisibleToHitTest);
 
   if (aBuilder->GetCurrentScrollbarFlags() != nsDisplayOwnLayerFlags::eNone) {
     // In the case of scrollbar frames, we use the scrollbar's target scrollframe
     // instead of the scrollframe with which the scrollbar actually moves.
     MOZ_ASSERT(mHitTestInfo & CompositorHitTestInfo::eScrollbar);
     mScrollTarget = Some(aBuilder->GetCurrentScrollbarTarget());
   }
-}
-
-void
-nsDisplayCompositorHitTestInfo::SetArea(const nsRect& aArea)
-{
-  mArea = Some(aArea);
+
+  if (aArea.isSome()) {
+    mArea = *aArea;
+  } else {
+    nsIScrollableFrame* scrollFrame = nsLayoutUtils::GetScrollableFrameFor(mFrame);
+    if (scrollFrame) {
+      // If the frame is content of a scrollframe, then we need to pick up the
+      // area corresponding to the overflow rect as well. Otherwise the parts of
+      // the overflow that are not occupied by descendants get skipped and the
+      // APZ code sends touch events to the content underneath instead.
+      // See https://bugzilla.mozilla.org/show_bug.cgi?id=1127773#c15.
+      mArea = mFrame->GetScrollableOverflowRect();
+    } else {
+      mArea = nsRect(nsPoint(0, 0), mFrame->GetSize());
+    }
+
+    // Note that it's important to do this call to ToReferenceFrame here in the
+    // nsDisplayCompositorHitTestInfo constructor, because then we'll hit the good
+    // fast path (because aBuilder will already have the info we want cached).
+    // This is as opposed to, say, calling it in CreateWebRenderCommands where
+    // we would not hit the fast path.
+    mArea += aBuilder->ToReferenceFrame(mFrame);
+  }
 }
 
 bool
 nsDisplayCompositorHitTestInfo::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
                                                         mozilla::wr::IpcResourceUpdateQueue& aResources,
                                                         const StackingContextHelper& aSc,
                                                         mozilla::layers::WebRenderLayerManager* aManager,
                                                         nsDisplayListBuilder* aDisplayListBuilder)
 {
-  if (mArea.isNothing()) {
-    nsRect borderBox;
-    nsIScrollableFrame* scrollFrame = nsLayoutUtils::GetScrollableFrameFor(mFrame);
-    if (scrollFrame) {
-      // If the frame is content of a scrollframe, then we need to pick up the
-      // area corresponding to the overflow rect as well. Otherwise the parts of
-      // the overflow that are not occupied by descendants get skipped and the
-      // APZ code sends touch events to the content underneath instead.
-      // See https://bugzilla.mozilla.org/show_bug.cgi?id=1127773#c15.
-      borderBox = mFrame->GetScrollableOverflowRect();
-    } else {
-      borderBox = nsRect(nsPoint(0, 0), mFrame->GetSize());
-    }
-
-    if (borderBox.IsEmpty()) {
-      return true;
-    }
-
-    mArea = Some(borderBox + aDisplayListBuilder->ToReferenceFrame(mFrame));
-  }
-
-  MOZ_ASSERT(mArea.isSome());
+  if (mArea.IsEmpty()) {
+    return true;
+  }
+
   wr::LayoutRect rect = aSc.ToRelativeLayoutRect(
       LayoutDeviceRect::FromAppUnits(
-          *mArea,
+          mArea,
           mFrame->PresContext()->AppUnitsPerDevPixel()));
 
   // XXX: eventually this scrollId computation and the SetHitTestInfo
   // call will get moved out into the WR display item iteration code so that
   // we don't need to do it as often, and so that we can do it for other
   // display item types as well (reducing the need for as many instances of
   // this display item).
   FrameMetrics::ViewID scrollId = mScrollTarget.valueOrFrom(
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -4353,44 +4353,44 @@ public:
  * this gets built when we're doing widget painting and we need to send the
  * compositor some hit-test info for a frame. This is effectively a dummy item
  * whose sole purpose is to carry the hit-test info to the compositor.
  */
 class nsDisplayCompositorHitTestInfo : public nsDisplayEventReceiver {
 public:
   nsDisplayCompositorHitTestInfo(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                                  mozilla::gfx::CompositorHitTestInfo aHitTestInfo,
-                                 uint32_t aIndex = 0);
+                                 uint32_t aIndex = 0,
+                                 const mozilla::Maybe<nsRect>& aArea = mozilla::Nothing());
 
 #ifdef NS_BUILD_REFCNT_LOGGING
   virtual ~nsDisplayCompositorHitTestInfo()
   {
     MOZ_COUNT_DTOR(nsDisplayCompositorHitTestInfo);
   }
 #endif
 
   mozilla::gfx::CompositorHitTestInfo HitTestInfo() const { return mHitTestInfo; }
-  void SetArea(const nsRect& aArea);
 
   bool CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
                                mozilla::wr::IpcResourceUpdateQueue& aResources,
                                const StackingContextHelper& aSc,
                                mozilla::layers::WebRenderLayerManager* aManager,
                                nsDisplayListBuilder* aDisplayListBuilder) override;
   void WriteDebugInfo(std::stringstream& aStream) override;
   uint32_t GetPerFrameKey() const override;
   int32_t ZIndex() const override;
   void SetOverrideZIndex(int32_t aZIndex);
 
   NS_DISPLAY_DECL_NAME("CompositorHitTestInfo", TYPE_COMPOSITOR_HITTEST_INFO)
 
 private:
   mozilla::gfx::CompositorHitTestInfo mHitTestInfo;
   mozilla::Maybe<mozilla::layers::FrameMetrics::ViewID> mScrollTarget;
-  mozilla::Maybe<nsRect> mArea;
+  nsRect mArea;
   uint32_t mIndex;
   mozilla::Maybe<int32_t> mOverrideZIndex;
 };
 
 /**
  * A display item that tracks event-sensitive regions which will be set
  * on the ContainerLayer that eventually contains this item.
  *