SF: do not latch unsignaled in early offsets

When we are in early offsets we are most likely in client composition
and/or WM animating windows. Avoid latching unsignaled buffers in this
case to avoid jank when an unsignaled buffers block newer transactions
from getting applied in the next frame.

Bug: 205153280
Test: modified TouchLatency app with long draws
Change-Id: Ice5456db1dec1feaf11f6e022a1eaaedfb26781f

6 files changed, 90 insertions, 12 deletions

diff --git a/services/surfaceflinger/Scheduler/VsyncModulator.cpp b/services/surfaceflinger/Scheduler/VsyncModulator.cpp
index 245db0f5c7..be57b2acd7 100644
--- a/services/surfaceflinger/Scheduler/VsyncModulator.cpp
+++ b/services/surfaceflinger/Scheduler/VsyncModulator.cpp
@@ -134,16 +134,27 @@ VsyncModulator::VsyncConfig VsyncModulator::getVsyncConfig() const {
     return mVsyncConfig;
 }
 
-const VsyncModulator::VsyncConfig& VsyncModulator::getNextVsyncConfig() const {
+auto VsyncModulator::getNextVsyncConfigType() const -> VsyncConfigType {
     // Early offsets are used if we're in the middle of a refresh rate
     // change, or if we recently begin a transaction.
     if (!mEarlyWakeupRequests.empty() || mTransactionSchedule == Schedule::EarlyEnd ||
         mEarlyTransactionFrames > 0 || mRefreshRateChangePending) {
-        return mVsyncConfigSet.early;
+        return VsyncConfigType::Early;
     } else if (mEarlyGpuFrames > 0) {
-        return mVsyncConfigSet.earlyGpu;
+        return VsyncConfigType::EarlyGpu;
     } else {
-        return mVsyncConfigSet.late;
+        return VsyncConfigType::Late;
+    }
+}
+
+const VsyncModulator::VsyncConfig& VsyncModulator::getNextVsyncConfig() const {
+    switch (getNextVsyncConfigType()) {
+        case VsyncConfigType::Early:
+            return mVsyncConfigSet.early;
+        case VsyncConfigType::EarlyGpu:
+            return mVsyncConfigSet.earlyGpu;
+        case VsyncConfigType::Late:
+            return mVsyncConfigSet.late;
     }
 }
 
@@ -176,4 +187,9 @@ void VsyncModulator::binderDied(const wp<IBinder>& who) {
     static_cast<void>(updateVsyncConfigLocked());
 }
 
+bool VsyncModulator::isVsyncConfigDefault() const {
+    std::lock_guard<std::mutex> lock(mMutex);
+    return getNextVsyncConfigType() == VsyncConfigType::Late;
+}
+
 } // namespace android::scheduler
diff --git a/services/surfaceflinger/Scheduler/VsyncModulator.h b/services/surfaceflinger/Scheduler/VsyncModulator.h
index 2000c546a0..537cae1d7c 100644
--- a/services/surfaceflinger/Scheduler/VsyncModulator.h
+++ b/services/surfaceflinger/Scheduler/VsyncModulator.h
@@ -109,11 +109,16 @@ public:
 
     [[nodiscard]] VsyncConfigOpt onDisplayRefresh(bool usedGpuComposition);
 
+    [[nodiscard]] bool isVsyncConfigDefault() const;
+
 protected:
     // Called from unit tests as well
     void binderDied(const wp<IBinder>&) override EXCLUDES(mMutex);
 
 private:
+    enum class VsyncConfigType { Early, EarlyGpu, Late };
+
+    VsyncConfigType getNextVsyncConfigType() const REQUIRES(mMutex);
     const VsyncConfig& getNextVsyncConfig() const REQUIRES(mMutex);
     [[nodiscard]] VsyncConfig updateVsyncConfig() EXCLUDES(mMutex);
     [[nodiscard]] VsyncConfig updateVsyncConfigLocked() REQUIRES(mMutex);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5991b86d5d..80b7ce7b4d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3806,7 +3806,7 @@ bool SurfaceFlinger::frameIsEarly(nsecs_t expectedPresentTime, int64_t vsyncId)
             prediction->presentTime - expectedPresentTime >= earlyLatchVsyncThreshold;
 }
 bool SurfaceFlinger::shouldLatchUnsignaled(const sp<Layer>& layer, const layer_state_t& state,
-                                           size_t numStates, size_t totalTXapplied) {
+                                           size_t numStates, size_t totalTXapplied) const {
     if (enableLatchUnsignaledConfig == LatchUnsignaledConfig::Disabled) {
         ALOGV("%s: false (LatchUnsignaledConfig::Disabled)", __func__);
         return false;
@@ -3824,11 +3824,22 @@ bool SurfaceFlinger::shouldLatchUnsignaled(const sp<Layer>& layer, const layer_s
         return false;
     }
 
-    if (enableLatchUnsignaledConfig == LatchUnsignaledConfig::AutoSingleLayer &&
-        totalTXapplied > 0) {
-        ALOGV("%s: false (LatchUnsignaledConfig::AutoSingleLayer; totalTXapplied=%zu)", __func__,
-              totalTXapplied);
-        return false;
+    if (enableLatchUnsignaledConfig == LatchUnsignaledConfig::AutoSingleLayer) {
+        if (totalTXapplied > 0) {
+            ALOGV("%s: false (LatchUnsignaledConfig::AutoSingleLayer; totalTXapplied=%zu)",
+                  __func__, totalTXapplied);
+            return false;
+        }
+
+        // We don't want to latch unsignaled if are in early / client composition
+        // as it leads to jank due to RenderEngine waiting for unsignaled buffer
+        // or window animations being slow.
+        const auto isDefaultVsyncConfig = mVsyncModulator->isVsyncConfigDefault();
+        if (!isDefaultVsyncConfig) {
+            ALOGV("%s: false (LatchUnsignaledConfig::AutoSingleLayer; !isDefaultVsyncConfig)",
+                  __func__);
+            return false;
+        }
     }
 
     if (!layer->simpleBufferUpdate(state)) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 02d5f1e49e..9674a77e6a 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -782,8 +782,8 @@ private:
             const std::unordered_set<sp<IBinder>, SpHash<IBinder>>& bufferLayersReadyToPresent,
             size_t totalTXapplied) const REQUIRES(mStateLock);
     static LatchUnsignaledConfig getLatchUnsignaledConfig();
-    static bool shouldLatchUnsignaled(const sp<Layer>& layer, const layer_state_t&,
-                                      size_t numStates, size_t totalTXapplied);
+    bool shouldLatchUnsignaled(const sp<Layer>& layer, const layer_state_t&, size_t numStates,
+                               size_t totalTXapplied) const;
     bool stopTransactionProcessing(const std::unordered_set<sp<IBinder>, SpHash<IBinder>>&
                                            applyTokensWithUnsignaledTransactions) const;
     bool applyTransactions(std::vector<TransactionState>& transactions, int64_t vsyncId)
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 8cadb3175a..67e47e7b37 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -270,6 +270,8 @@ public:
     scheduler::TestableScheduler& mutableScheduler() { return *mScheduler; }
     scheduler::mock::SchedulerCallback& mockSchedulerCallback() { return mSchedulerCallback; }
 
+    auto& mutableVsyncModulator() { return mFlinger->mVsyncModulator; }
+
     using CreateBufferQueueFunction = surfaceflinger::test::Factory::CreateBufferQueueFunction;
     void setCreateBufferQueueFunction(CreateBufferQueueFunction f) {
         mFactory.mCreateBufferQueue = f;
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index 4683c5132f..eefa11f1aa 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -631,6 +631,28 @@ TEST_F(LatchUnsignaledAutoSingleLayerTest, Flush_KeepsTransactionInTheQueue) {
                          kExpectedTransactionsApplied, kExpectedTransactionsPending);
 }
 
+TEST_F(LatchUnsignaledAutoSingleLayerTest, DontLatchUnsignaledWhenEarlyOffset) {
+    const sp<IBinder> kApplyToken =
+            IInterface::asBinder(TransactionCompletedListener::getIInstance());
+    const auto kLayerId = 1;
+    const auto kExpectedTransactionsApplied = 0u;
+    const auto kExpectedTransactionsPending = 1u;
+
+    const auto unsignaledTransaction =
+            createTransactionInfo(kApplyToken,
+                                  {
+                                          createComposerState(kLayerId,
+                                                              fence(Fence::Status::Unsignaled),
+                                                              layer_state_t::eBufferChanged),
+                                  });
+
+    // Get VsyncModulator out of the default config
+    static_cast<void>(mFlinger.mutableVsyncModulator()->onRefreshRateChangeInitiated());
+
+    setTransactionStates({unsignaledTransaction}, kExpectedTransactionsApplied,
+                         kExpectedTransactionsPending);
+}
+
 class LatchUnsignaledDisabledTest : public LatchUnsignaledTest {
 public:
     void SetUp() override {
@@ -999,4 +1021,26 @@ TEST_F(LatchUnsignaledAlwaysTest, Flush_RemovesUnsignaledFromTheQueue) {
                          kExpectedTransactionsApplied, kExpectedTransactionsPending);
 }
 
+TEST_F(LatchUnsignaledAlwaysTest, LatchUnsignaledWhenEarlyOffset) {
+    const sp<IBinder> kApplyToken =
+            IInterface::asBinder(TransactionCompletedListener::getIInstance());
+    const auto kLayerId = 1;
+    const auto kExpectedTransactionsApplied = 1u;
+    const auto kExpectedTransactionsPending = 0u;
+
+    const auto unsignaledTransaction =
+            createTransactionInfo(kApplyToken,
+                                  {
+                                          createComposerState(kLayerId,
+                                                              fence(Fence::Status::Unsignaled),
+                                                              layer_state_t::eBufferChanged),
+                                  });
+
+    // Get VsyncModulator out of the default config
+    static_cast<void>(mFlinger.mutableVsyncModulator()->onRefreshRateChangeInitiated());
+
+    setTransactionStates({unsignaledTransaction}, kExpectedTransactionsApplied,
+                         kExpectedTransactionsPending);
+}
+
 } // namespace android