Bug 1430660 - Move the APZ update of scroll layer positions into a transaction. r=nical
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 17 Jan 2018 11:19:39 -0500
changeset 453976 96e818315d57980b93162a968cc74ae9ab68c01d
parent 453975 55b95957d30bddff2bf78f07fca2f6046141873d
child 453977 877584981d6bdacaa24b1860030c47e052ad1ea3
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1430660
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1430660 - Move the APZ update of scroll layer positions into a transaction. r=nical MozReview-Commit-ID: 45ijEtB1C0Z
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
gfx/layers/wr/WebRenderBridgeParent.cpp
gfx/layers/wr/WebRenderBridgeParent.h
gfx/webrender_bindings/RenderThread.cpp
gfx/webrender_bindings/WebRenderAPI.cpp
gfx/webrender_bindings/WebRenderAPI.h
gfx/webrender_bindings/src/bindings.rs
gfx/webrender_bindings/webrender_ffi_generated.h
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -445,23 +445,21 @@ APZCTreeManager::UpdateHitTestingTree(ui
                                       uint32_t aPaintSequenceNumber)
 {
   WebRenderScrollDataWrapper wrapper(&aScrollData);
   UpdateHitTestingTreeImpl(aRootLayerTreeId, wrapper, aIsFirstPaint,
                            aOriginatingLayersId, aPaintSequenceNumber);
 }
 
 bool
-APZCTreeManager::PushStateToWR(wr::WebRenderAPI* aWrApi,
+APZCTreeManager::PushStateToWR(wr::TransactionBuilder& aTxn,
                                const TimeStamp& aSampleTime,
                                nsTArray<wr::WrTransformProperty>& aTransformArray)
 {
   APZThreadUtils::AssertOnCompositorThread();
-  MOZ_ASSERT(aWrApi);
-  MOZ_ASSERT(aWrApi == RefPtr<wr::WebRenderAPI>(GetWebRenderAPI()).get());
 
   MutexAutoLock lock(mTreeLock);
 
   // During the first pass through the tree, we build a cache of guid->HTTN so
   // that we can find the relevant APZC instances quickly in subsequent passes,
   // such as the one below to generate scrollbar transforms. Without this, perf
   // could end up being O(n^2) instead of O(n log n) because we'd have to search
   // the tree to find the corresponding APZC every time we hit a thumb node.
@@ -508,17 +506,17 @@ APZCTreeManager::PushStateToWR(wr::WebRe
         ParentLayerPoint layerTranslation = apzc->GetCurrentAsyncTransform(
             AsyncPanZoomController::eForCompositing).mTranslation;
         // The positive translation means the painted content is supposed to
         // move down (or to the right), and that corresponds to a reduction in
         // the scroll offset. Since we are effectively giving WR the async
         // scroll delta here, we want to negate the translation.
         ParentLayerPoint asyncScrollDelta = -layerTranslation;
         // XXX figure out what zoom-related conversions need to happen here.
-        aWrApi->UpdateScrollPosition(lastPipelineId, apzc->GetGuid().mScrollId,
+        aTxn.UpdateScrollPosition(lastPipelineId, apzc->GetGuid().mScrollId,
             wr::ToLayoutPoint(LayoutDevicePoint::FromUnknownPoint(asyncScrollDelta.ToUnknownPoint())));
 
         apzc->ReportCheckerboard(aSampleTime);
         activeAnimations |= apzc->AdvanceAnimations(aSampleTime);
       });
 
   // Now we iterate over the nodes again, and generate the transforms needed
   // for scrollbar thumbs. Although we *could* do this as part of the previous
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -26,16 +26,17 @@
 #if defined(MOZ_WIDGET_ANDROID)
 #include "mozilla/layers/AndroidDynamicToolbarAnimator.h"
 #endif // defined(MOZ_WIDGET_ANDROID)
 
 namespace mozilla {
 class MultiTouchInput;
 
 namespace wr {
+class TransactionBuilder;
 class WebRenderAPI;
 struct WrTransformProperty;
 }
 
 namespace layers {
 
 class Layer;
 class AsyncPanZoomController;
@@ -174,17 +175,17 @@ public:
    * async scroll position. It also advances APZ animations to the specified
    * sample time. In effect it is the webrender equivalent of (part of) the
    * code in AsyncCompositionManager. If scrollbar transforms need updating
    * to reflect the async scroll position, the updated transforms are appended
    * to the provided aTransformArray.
    * Returns true if any APZ animations are in progress and we need to keep
    * compositing.
    */
-  bool PushStateToWR(wr::WebRenderAPI* aWrApi,
+  bool PushStateToWR(wr::TransactionBuilder& aTxn,
                      const TimeStamp& aSampleTime,
                      nsTArray<wr::WrTransformProperty>& aTransformArray);
 
   /**
    * Walk the tree of APZCs and flushes the repaint requests for all the APZCS
    * corresponding to the given layers id. Finally, sends a flush complete
    * notification to the GeckoContentController for the layers id.
    */
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -524,32 +524,33 @@ WebRenderBridgeParent::UpdateAPZ(bool aU
       apzc->UpdateHitTestingTree(rootLayersId, rootWrbp->GetScrollData(),
           mScrollData.IsFirstPaint(), GetLayersId(),
           mScrollData.GetPaintSequenceNumber());
     }
   }
 }
 
 bool
-WebRenderBridgeParent::PushAPZStateToWR(nsTArray<wr::WrTransformProperty>& aTransformArray)
+WebRenderBridgeParent::PushAPZStateToWR(wr::TransactionBuilder& aTxn,
+                                        nsTArray<wr::WrTransformProperty>& aTransformArray)
 {
   CompositorBridgeParent* cbp = GetRootCompositorBridgeParent();
   if (!cbp) {
     return false;
   }
   if (RefPtr<APZCTreeManager> apzc = cbp->GetAPZCTreeManager()) {
     TimeStamp animationTime = cbp->GetTestingTimeStamp().valueOr(
         mCompositorScheduler->GetLastComposeTime());
     TimeDuration frameInterval = cbp->GetVsyncInterval();
     // As with the non-webrender codepath in AsyncCompositionManager, we want to
     // use the timestamp for the next vsync when advancing animations.
     if (frameInterval != TimeDuration::Forever()) {
       animationTime += frameInterval;
     }
-    return apzc->PushStateToWR(mApi, animationTime, aTransformArray);
+    return apzc->PushStateToWR(aTxn, animationTime, aTransformArray);
   }
   return false;
 }
 
 const WebRenderScrollData&
 WebRenderBridgeParent::GetScrollData() const
 {
   MOZ_ASSERT(mozilla::layers::CompositorThreadHolder::IsInCompositorThread());
@@ -1224,29 +1225,29 @@ WebRenderBridgeParent::CompositeToTarget
   nsTArray<wr::WrOpacityProperty> opacityArray;
   nsTArray<wr::WrTransformProperty> transformArray;
 
   SampleAnimations(opacityArray, transformArray);
   if (!transformArray.IsEmpty() || !opacityArray.IsEmpty()) {
     ScheduleGenerateFrame();
   }
 
-  if (PushAPZStateToWR(transformArray)) {
+  wr::TransactionBuilder txn;
+
+  if (PushAPZStateToWR(txn, transformArray)) {
     ScheduleGenerateFrame();
   }
 
   wr::RenderThread::Get()->IncPendingFrameCount(mApi->GetId());
 
 #if defined(ENABLE_FRAME_LATENCY_LOG)
   auto startTime = TimeStamp::Now();
   mApi->SetFrameStartTime(startTime);
 #endif
 
-  wr::TransactionBuilder txn;
-
   if (!transformArray.IsEmpty() || !opacityArray.IsEmpty()) {
     txn.UpdateDynamicProperties(opacityArray, transformArray);
   }
 
   txn.GenerateFrame();
 
   mApi->SendTransaction(txn);
 }
--- a/gfx/layers/wr/WebRenderBridgeParent.h
+++ b/gfx/layers/wr/WebRenderBridgeParent.h
@@ -215,17 +215,18 @@ private:
                         nsTArray<wr::WrTransformProperty>& aTransformArray);
 
   CompositorBridgeParent* GetRootCompositorBridgeParent() const;
 
   // Have APZ push the async scroll state to WR. Returns true if an APZ
   // animation is in effect and we need to schedule another composition.
   // If scrollbars need their transforms updated, the provided aTransformArray
   // is populated with the property update details.
-  bool PushAPZStateToWR(nsTArray<wr::WrTransformProperty>& aTransformArray);
+  bool PushAPZStateToWR(wr::TransactionBuilder& aTxn,
+                        nsTArray<wr::WrTransformProperty>& aTransformArray);
 
   // Helper method to get an APZC reference from a scroll id. Uses the layers
   // id of this bridge, and may return null if the APZC wasn't found.
   already_AddRefed<AsyncPanZoomController> GetTargetAPZC(const FrameMetrics::ViewID& aId);
 
   uint32_t GetNextWrEpoch();
 
 private:
--- a/gfx/webrender_bindings/RenderThread.cpp
+++ b/gfx/webrender_bindings/RenderThread.cpp
@@ -454,28 +454,36 @@ WebRenderProgramCache::~WebRenderProgram
   wr_program_cache_delete(mProgramCache);
 }
 
 } // namespace wr
 } // namespace mozilla
 
 extern "C" {
 
-void wr_notifier_new_frame_ready(mozilla::wr::WrWindowId aWindowId)
+static void NewFrameReady(mozilla::wr::WrWindowId aWindowId)
 {
   mozilla::wr::RenderThread::Get()->IncRenderingFrameCount(aWindowId);
   mozilla::wr::RenderThread::Get()->NewFrameReady(mozilla::wr::WindowId(aWindowId));
 }
 
+void wr_notifier_new_frame_ready(mozilla::wr::WrWindowId aWindowId)
+{
+  NewFrameReady(aWindowId);
+}
+
 void wr_notifier_new_scroll_frame_ready(mozilla::wr::WrWindowId aWindowId, bool aCompositeNeeded)
 {
-  // It is not necessary to update rendering with new_scroll_frame_ready.
-  // WebRenderBridgeParent::CompositeToTarget() is implemented to call
-  // WebRenderAPI::GenerateFrame() if it is necessary to trigger UpdateAndRender().
-  // See Bug 1377688.
+  // If we sent a transaction that contained both scrolling updates and a
+  // GenerateFrame, we can get this function called with aCompositeNeeded=true
+  // instead of wr_notifier_new_frame_ready. In that case we want to update the
+  // rendering.
+  if (aCompositeNeeded) {
+    NewFrameReady(aWindowId);
+  }
 }
 
 void wr_notifier_external_event(mozilla::wr::WrWindowId aWindowId, size_t aRawEvent)
 {
   mozilla::UniquePtr<mozilla::wr::RendererEvent> evt(
     reinterpret_cast<mozilla::wr::RendererEvent*>(aRawEvent));
   mozilla::wr::RenderThread::Get()->RunEvent(mozilla::wr::WindowId(aWindowId),
                                              mozilla::Move(evt));
--- a/gfx/webrender_bindings/WebRenderAPI.cpp
+++ b/gfx/webrender_bindings/WebRenderAPI.cpp
@@ -217,16 +217,24 @@ TransactionBuilder::SetWindowParameters(
 }
 
 void
 TransactionBuilder::UpdateResources(ResourceUpdateQueue& aUpdates)
 {
   wr_transaction_update_resources(mTxn, aUpdates.Raw());
 }
 
+void
+TransactionBuilder::UpdateScrollPosition(const wr::WrPipelineId& aPipelineId,
+                                         const layers::FrameMetrics::ViewID& aScrollId,
+                                         const wr::LayoutPoint& aScrollPosition)
+{
+  wr_transaction_scroll_layer(mTxn, aPipelineId, aScrollId, aScrollPosition);
+}
+
 
 /*static*/ void
 WebRenderAPI::InitExternalLogHandler()
 {
   // Redirect the webrender's log to gecko's log system.
   // The current log level is "error".
   mozilla::wr::wr_init_external_log_handler(wr::LogLevelFilter::Error);
 }
@@ -303,24 +311,16 @@ WebRenderAPI::~WebRenderAPI()
 }
 
 void
 WebRenderAPI::SendTransaction(TransactionBuilder& aTxn)
 {
   wr_api_send_transaction(mDocHandle, aTxn.Raw());
 }
 
-void
-WebRenderAPI::UpdateScrollPosition(const wr::WrPipelineId& aPipelineId,
-                                   const layers::FrameMetrics::ViewID& aScrollId,
-                                   const wr::LayoutPoint& aScrollPosition)
-{
-  wr_scroll_layer_with_id(mDocHandle, aPipelineId, aScrollId, aScrollPosition);
-}
-
 bool
 WebRenderAPI::HitTest(const wr::WorldPoint& aPoint,
                       wr::WrPipelineId& aOutPipelineId,
                       layers::FrameMetrics::ViewID& aOutScrollId,
                       gfx::CompositorHitTestInfo& aOutHitInfo)
 {
   static_assert(sizeof(gfx::CompositorHitTestInfo) == sizeof(uint16_t),
                 "CompositorHitTestInfo should be u16-sized");
--- a/gfx/webrender_bindings/WebRenderAPI.h
+++ b/gfx/webrender_bindings/WebRenderAPI.h
@@ -149,16 +149,20 @@ public:
 
   void UpdateDynamicProperties(const nsTArray<wr::WrOpacityProperty>& aOpacityArray,
                                const nsTArray<wr::WrTransformProperty>& aTransformArray);
 
   void SetWindowParameters(LayoutDeviceIntSize size);
 
   void UpdateResources(ResourceUpdateQueue& aUpdates);
 
+  void UpdateScrollPosition(const wr::WrPipelineId& aPipelineId,
+                            const layers::FrameMetrics::ViewID& aScrollId,
+                            const wr::LayoutPoint& aScrollPosition);
+
   bool IsEmpty() const;
 
   Transaction* Raw() { return mTxn; }
 protected:
   Transaction* mTxn;
 };
 
 class WebRenderAPI
@@ -174,19 +178,16 @@ public:
   // Redirect the WR's log to gfxCriticalError/Note.
   static void InitExternalLogHandler();
   static void ShutdownExternalLogHandler();
 
   already_AddRefed<WebRenderAPI> Clone();
 
   wr::WindowId GetId() const { return mId; }
 
-  void UpdateScrollPosition(const wr::WrPipelineId& aPipelineId,
-                            const layers::FrameMetrics::ViewID& aScrollId,
-                            const wr::LayoutPoint& aScrollPosition);
   bool HitTest(const wr::WorldPoint& aPoint,
                wr::WrPipelineId& aOutPipelineId,
                layers::FrameMetrics::ViewID& aOutScrollId,
                gfx::CompositorHitTestInfo& aOutHitInfo);
 
   void SendTransaction(TransactionBuilder& aTxn);
 
   void SetFrameStartTime(const TimeStamp& aTime);
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -948,16 +948,28 @@ pub extern "C" fn wr_transaction_update_
             properties.floats.push(prop);
         }
     }
 
     txn.update_dynamic_properties(properties);
 }
 
 #[no_mangle]
+pub extern "C" fn wr_transaction_scroll_layer(
+    txn: &mut Transaction,
+    pipeline_id: WrPipelineId,
+    scroll_id: u64,
+    new_scroll_origin: LayoutPoint
+) {
+    assert!(unsafe { is_in_compositor_thread() });
+    let clip_id = ClipId::new(scroll_id, pipeline_id);
+    txn.scroll_node_with_id(new_scroll_origin, clip_id, ScrollClamping::NoClamping);
+}
+
+#[no_mangle]
 pub extern "C" fn wr_resource_updates_add_image(
     resources: &mut ResourceUpdates,
     image_key: WrImageKey,
     descriptor: &WrImageDescriptor,
     bytes: &mut WrVecU8,
 ) {
     resources.add_image(
         image_key,
@@ -1521,26 +1533,16 @@ pub extern "C" fn wr_dp_push_scroll_laye
 
 #[no_mangle]
 pub extern "C" fn wr_dp_pop_scroll_layer(state: &mut WrState) {
     debug_assert!(unsafe { is_in_main_thread() });
     state.frame_builder.dl_builder.pop_clip_id();
 }
 
 #[no_mangle]
-pub extern "C" fn wr_scroll_layer_with_id(dh: &mut DocumentHandle,
-                                          pipeline_id: WrPipelineId,
-                                          scroll_id: u64,
-                                          new_scroll_origin: LayoutPoint) {
-    assert!(unsafe { is_in_compositor_thread() });
-    let clip_id = ClipId::new(scroll_id, pipeline_id);
-    dh.api.scroll_node_with_id(dh.document_id, new_scroll_origin, clip_id, ScrollClamping::NoClamping);
-}
-
-#[no_mangle]
 pub extern "C" fn wr_dp_push_clip_and_scroll_info(state: &mut WrState,
                                                   scroll_id: u64,
                                                   clip_id: *const u64) {
     debug_assert!(unsafe { is_in_main_thread() });
     let info = make_scroll_info(state, Some(&scroll_id), unsafe { clip_id.as_ref() });
     debug_assert!(info.is_some());
     state.frame_builder.dl_builder.push_clip_and_scroll_info(info.unwrap());
 }
--- a/gfx/webrender_bindings/webrender_ffi_generated.h
+++ b/gfx/webrender_bindings/webrender_ffi_generated.h
@@ -1498,23 +1498,16 @@ WR_FUNC;
 WR_INLINE
 void wr_resource_updates_update_image(ResourceUpdates *aResources,
                                       WrImageKey aKey,
                                       const WrImageDescriptor *aDescriptor,
                                       WrVecU8 *aBytes)
 WR_FUNC;
 
 WR_INLINE
-void wr_scroll_layer_with_id(DocumentHandle *aDh,
-                             WrPipelineId aPipelineId,
-                             uint64_t aScrollId,
-                             LayoutPoint aNewScrollOrigin)
-WR_FUNC;
-
-WR_INLINE
 void wr_set_item_tag(WrState *aState,
                      uint64_t aScrollId,
                      uint16_t aHitInfo)
 WR_FUNC;
 
 WR_INLINE
 void wr_shutdown_external_log_handler()
 WR_FUNC;
@@ -1560,16 +1553,23 @@ Transaction *wr_transaction_new()
 WR_FUNC;
 
 WR_INLINE
 void wr_transaction_remove_pipeline(Transaction *aTxn,
                                     WrPipelineId aPipelineId)
 WR_FUNC;
 
 WR_INLINE
+void wr_transaction_scroll_layer(Transaction *aTxn,
+                                 WrPipelineId aPipelineId,
+                                 uint64_t aScrollId,
+                                 LayoutPoint aNewScrollOrigin)
+WR_FUNC;
+
+WR_INLINE
 void wr_transaction_set_display_list(Transaction *aTxn,
                                      WrEpoch aEpoch,
                                      ColorF aBackground,
                                      float aViewportWidth,
                                      float aViewportHeight,
                                      WrPipelineId aPipelineId,
                                      LayoutSize aContentSize,
                                      BuiltDisplayListDescriptor aDlDescriptor,