commit 45ed7a82f535f1f627088a23f71f39f7aabbea85
author Ady Abraham <adyabr@google.com> Thu Mar 14 17:36:22 2024 -0700
committer Ady Abraham <adyabr@google.com> Thu Mar 14 17:45:37 2024 -0700
tree 04edf9d384ea3954c069391508db60f32661a06a
parent fc2deb96cee3ba1695c14e1955745458b4fdb050

SF: VsyncTimeline was added twice

This CL fixes the issue where VsyncTimeline was added twice.

Fixes: 329310308
Test: presubmit
Change-Id: I8cc574f3f2fe6f7e68149108cc3bcf507e798a37

services/surfaceflinger/Scheduler/VSyncPredictor.cpp

diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index db1930d..acb3760 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -377,15 +377,20 @@
     mRenderRateOpt = renderRate;
     const auto renderPeriodDelta =
             prevRenderRate ? prevRenderRate->getPeriodNsecs() - renderRate.getPeriodNsecs() : 0;
-    const bool newRenderRateIsHigher = renderPeriodDelta > renderRate.getPeriodNsecs() &&
-            mLastCommittedVsync.ns() - mClock->now() > 2 * renderRate.getPeriodNsecs();
     if (applyImmediately) {
+        ATRACE_FORMAT_INSTANT("applyImmediately");
         while (mTimelines.size() > 1) {
             mTimelines.pop_front();
         }
 
         mTimelines.front().setRenderRate(renderRate);
-    } else if (newRenderRateIsHigher) {
+        return;
+    }
+
+    const bool newRenderRateIsHigher = renderPeriodDelta > renderRate.getPeriodNsecs() &&
+            mLastCommittedVsync.ns() - mClock->now() > 2 * renderRate.getPeriodNsecs();
+    if (newRenderRateIsHigher) {
+        ATRACE_FORMAT_INSTANT("newRenderRateIsHigher");
         mTimelines.clear();
         mLastCommittedVsync = TimePoint::fromNs(0);
 

services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp

diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index 48707cb..aac1cac 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -944,6 +944,27 @@
     EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(8000));
 }
 
+// b/329310308
+TEST_F(VSyncPredictorTest, renderRateChangeAfterAppliedImmediately) {
+    tracker.addVsyncTimestamp(1000);
+
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1001), Eq(2000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(2001), Eq(3000));
+
+    tracker.setRenderRate(Fps::fromPeriodNsecs(2000), /*applyImmediately*/ true);
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1001), Eq(3000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(3001), Eq(5000));
+
+    tracker.setRenderRate(Fps::fromPeriodNsecs(4000), /*applyImmediately*/ false);
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1001), Eq(3000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(3001), Eq(5000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(9000));
+    EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(9001), Eq(13000));
+}
+
 } // namespace android::scheduler
 
 // TODO(b/129481165): remove the #pragma below and fix conversion issues