Bug 1461122 - Clean up the composition flow for empty transactions. r?sotaro draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 15 May 2018 08:49:34 -0400
changeset 795282 de7429880c98078fd5c7aa86f2e326ae2653d7e4
parent 795281 185266b047fb2ce8a0c4d6b76454e0bdcf2d4358
child 795283 ee4f91b5f872e2effabd94ccf691270408277400
push id109913
push userkgupta@mozilla.com
push dateTue, 15 May 2018 12:50:00 +0000
reviewerssotaro
bugs1461122
milestone62.0a1
Bug 1461122 - Clean up the composition flow for empty transactions. r?sotaro This is mostly just refactoring to make the code more understandable. However, there are a couple of functional changes: - If we have scroll offset updates we now schedule a composite instead of sending the DidComposite right away. This is needed because we want to actually composite the scroll offset change. - If there are WebRenderParentCommands provided, we process them and update the epoch in a single transaction, instead of splitting it across two transactions for no good reason. MozReview-Commit-ID: 2HCa19EGxUD
gfx/layers/wr/WebRenderBridgeParent.cpp
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -719,52 +719,57 @@ WebRenderBridgeParent::RecvEmptyTransact
 
   AUTO_PROFILER_TRACING("Paint", "EmptyTransaction");
   UpdateFwdTransactionId(aFwdTransactionId);
 
   // This ensures that destroy operations are always processed. It is not safe
   // to early-return without doing so.
   AutoWebRenderBridgeParentAsyncMessageSender autoAsyncMessageSender(this, &aToDestroy);
 
-  if (!aCommands.IsEmpty()) {
-    mAsyncImageManager->SetCompositionTime(TimeStamp::Now());
-    wr::TransactionBuilder txn;
-    ProcessWebRenderParentCommands(aCommands, txn);
-    mApi->SendTransaction(txn);
-    ScheduleGenerateFrame();
-  }
+  bool scheduleComposite = false;
 
   UpdateAPZFocusState(aFocusTarget);
   if (!aUpdates.empty()) {
     // aUpdates is moved into this function but that is not reflected by the
     // function signature due to the way the IPDL generator works. We remove the
     // const so that we can move this structure all the way to the desired
     // destination.
     UpdateAPZScrollOffsets(Move(const_cast<ScrollUpdatesMap&>(aUpdates)), aPaintSequenceNumber);
+    scheduleComposite = true;
   }
 
   if (!aCommands.IsEmpty()) {
+    mAsyncImageManager->SetCompositionTime(TimeStamp::Now());
     wr::TransactionBuilder txn;
     wr::Epoch wrEpoch = GetNextWrEpoch();
     txn.UpdateEpoch(mPipelineId, wrEpoch);
+    ProcessWebRenderParentCommands(aCommands, txn);
     mApi->SendTransaction(txn);
+    scheduleComposite = true;
+  }
 
-    HoldPendingTransactionId(wrEpoch, aTransactionId, aTxnStartTime, aFwdTime);
-  } else {
-    bool sendDidComposite = false;
-    if (mPendingTransactionIds.empty()) {
-      sendDidComposite = true;
-    }
-    HoldPendingTransactionId(WrEpoch(), aTransactionId, aTxnStartTime, aFwdTime);
-    // If WebRenderBridgeParent does not have pending DidComposites,
-    // send DidComposite now.
-    if (sendDidComposite) {
-      TimeStamp now = TimeStamp::Now();
-      mCompositorBridge->DidComposite(GetLayersId(), now, now);
-    }
+  bool sendDidComposite = true;
+  if (scheduleComposite || !mPendingTransactionIds.empty()) {
+    // If we are going to kick off a new composite as a result of this
+    // transaction, or if there are already composite-triggering pending
+    // transactions inflight, then set sendDidComposite to false because we will
+    // send the DidComposite message after the composite occurs.
+    // If there are no pending transactions and we're not going to do a
+    // composite, then we leave sendDidComposite as true so we just send
+    // the DidComposite notification now.
+    sendDidComposite = false;
+  }
+
+  HoldPendingTransactionId(WrEpoch(), aTransactionId, aTxnStartTime, aFwdTime);
+
+  if (scheduleComposite) {
+    ScheduleGenerateFrame();
+  } else if (sendDidComposite) {
+    TimeStamp now = TimeStamp::Now();
+    mCompositorBridge->DidComposite(GetLayersId(), now, now);
   }
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 WebRenderBridgeParent::RecvSetFocusTarget(const FocusTarget& aFocusTarget)
 {