Bug 1430660 - Move the APZ update of scroll layer positions into a transaction. r=nical
☠☠ backed out by 1e723381da62 ☠ ☠
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 16 Jan 2018 15:33:34 -0500
changeset 399627 21f6b322e9a29a5938193369a890415742bf4081
parent 399626 80e57c896fd711a84fe3ce9a068c65699caa85b6
child 399628 76d1a0fa9eb7c5929f6bef2e68385b78260cafc0
push id33271
push usertoros@mozilla.com
push dateWed, 17 Jan 2018 21:46:52 +0000
treeherdermozilla-central@3fb310e17608 [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: 4fd3FM1K9T3
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
@@ -462,20 +462,23 @@ extern "C" {
 void wr_notifier_new_frame_ready(mozilla::wr::WrWindowId aWindowId)
 {
   mozilla::wr::RenderThread::Get()->IncRenderingFrameCount(aWindowId);
   mozilla::wr::RenderThread::Get()->NewFrameReady(mozilla::wr::WindowId(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) {
+    wr_notifier_new_frame_ready(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,