commit f792cfe81a5d6db6ba0536954bb64e2b7acf6b80
author Alec Mouri <alecmouri@google.com> Fri Jun 05 13:11:23 2020 -0700
committer Alec Mouri <alecmouri@google.com> Wed Jun 10 19:44:55 2020 -0700
tree a38240509446515fe7b2c089ce96fb5e4162407f
parent 983e568b3c370659f11fcf76cbd45c61dfd8dc33

Only apply early offsets when transaction has been applied

This avoids a race where we decrement the early offset count in
postComposition() before the transaction applying early offsets has
actually been applied, so we're in early offsets for too few frames.

Bug: 158127834
Test: photos launch
Test: systrace

Change-Id: If7a7d12fa466ee4d163210263b7f02f7a74b3280

services/surfaceflinger/Scheduler/VSyncModulator.cpp

diff --git a/services/surfaceflinger/Scheduler/VSyncModulator.cpp b/services/surfaceflinger/Scheduler/VSyncModulator.cpp
index 40a992c..510dc2d 100644
--- a/services/surfaceflinger/Scheduler/VSyncModulator.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncModulator.cpp
@@ -25,6 +25,7 @@
 #include <cutils/properties.h>
 #include <utils/Trace.h>
 
+#include <chrono>
 #include <cinttypes>
 #include <mutex>
 
@@ -52,6 +53,7 @@
 void VSyncModulator::setTransactionStart(Scheduler::TransactionStart transactionStart) {
     if (transactionStart == Scheduler::TransactionStart::EARLY) {
         mRemainingEarlyFrameCount = MIN_EARLY_FRAME_COUNT_TRANSACTION;
+        mEarlyTxnStartTime = std::chrono::steady_clock::now();
     }
 
     // An early transaction stays an early transaction.
@@ -64,6 +66,7 @@
 }
 
 void VSyncModulator::onTransactionHandled() {
+    mTxnAppliedTime = std::chrono::steady_clock::now();
     if (mTransactionStart == Scheduler::TransactionStart::NORMAL) return;
     mTransactionStart = Scheduler::TransactionStart::NORMAL;
     updateOffsets();
@@ -87,9 +90,16 @@
 
 void VSyncModulator::onRefreshed(bool usedRenderEngine) {
     bool updateOffsetsNeeded = false;
-    if (mRemainingEarlyFrameCount > 0) {
-        mRemainingEarlyFrameCount--;
-        updateOffsetsNeeded = true;
+
+    // Apply a 1ms margin to account for potential data races
+    // This might make us stay in early offsets for one
+    // additional frame but it's better to be conservative here.
+    static const constexpr std::chrono::nanoseconds kMargin = 1ms;
+    if ((mEarlyTxnStartTime.load() + kMargin) < mTxnAppliedTime.load()) {
+        if (mRemainingEarlyFrameCount > 0) {
+            mRemainingEarlyFrameCount--;
+            updateOffsetsNeeded = true;
+        }
     }
     if (usedRenderEngine) {
         mRemainingRenderEngineUsageCount = MIN_EARLY_GL_FRAME_COUNT_TRANSACTION;