Merge "SF: minor bug fix to flags::dont_skip_on_early" into main

2 files changed, 81 insertions, 7 deletions

diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index c4c9fa56ff..f4676706fd 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -40,16 +40,18 @@ using base::StringAppendF;
 
 namespace {
 
-nsecs_t getExpectedCallbackTime(nsecs_t nextVsyncTime,
+nsecs_t getExpectedCallbackTime(nsecs_t now, nsecs_t nextVsyncTime,
                                 const VSyncDispatch::ScheduleTiming& timing) {
-    return nextVsyncTime - timing.readyDuration - timing.workDuration;
+    const auto expectedCallbackTime = nextVsyncTime - timing.readyDuration - timing.workDuration;
+    const auto baseTime = flags::dont_skip_on_early() ? now : expectedCallbackTime;
+    return std::max(baseTime, expectedCallbackTime);
 }
 
 nsecs_t getExpectedCallbackTime(VSyncTracker& tracker, nsecs_t now,
                                 const VSyncDispatch::ScheduleTiming& timing) {
     const auto nextVsyncTime = tracker.nextAnticipatedVSyncTimeFrom(
             std::max(timing.earliestVsync, now + timing.workDuration + timing.readyDuration));
-    return getExpectedCallbackTime(nextVsyncTime, timing);
+    return getExpectedCallbackTime(now, nextVsyncTime, timing);
 }
 
 } // namespace
@@ -105,11 +107,11 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim
             mArmedInfo && ((nextWakeupTime > (mArmedInfo->mActualWakeupTime + mMinVsyncDistance)));
     if (flags::dont_skip_on_early()) {
         if (wouldSkipAVsyncTarget || wouldSkipAWakeup) {
-            return getExpectedCallbackTime(mArmedInfo->mActualVsyncTime, timing);
+            return getExpectedCallbackTime(now, mArmedInfo->mActualVsyncTime, timing);
         }
     } else {
         if (wouldSkipAVsyncTarget && wouldSkipAWakeup) {
-            return getExpectedCallbackTime(nextVsyncTime, timing);
+            return getExpectedCallbackTime(now, nextVsyncTime, timing);
         }
     }
 
@@ -119,7 +121,7 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim
     auto const nextReadyTime = nextVsyncTime - timing.readyDuration;
     mScheduleTiming = timing;
     mArmedInfo = {nextWakeupTime, nextVsyncTime, nextReadyTime};
-    return getExpectedCallbackTime(nextVsyncTime, timing);
+    return getExpectedCallbackTime(now, nextVsyncTime, timing);
 }
 
 void VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate(VSyncDispatch::ScheduleTiming timing) {
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index b8fdce1dce..1dc5498221 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -759,6 +759,8 @@ TEST_F(VSyncDispatchTimerQueueTest, canMoveCallbackBackwardsInTime) {
 
 // b/1450138150
 TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipAScheduledTargetVSync) {
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+
     EXPECT_CALL(mMockClock, alarmAt(_, 500));
     CountingCallback cb(mDispatch);
     auto result =
@@ -772,6 +774,29 @@ TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipASchedule
                                  {.workDuration = 800, .readyDuration = 0, .earliestVsync = 1000});
     EXPECT_TRUE(result.has_value());
     EXPECT_EQ(1200, *result);
+
+    advanceToNextCallback();
+    ASSERT_THAT(cb.mCalls.size(), Eq(1));
+}
+
+// b/1450138150
+TEST_F(VSyncDispatchTimerQueueTest, movesCallbackBackwardsAndSkipAScheduledTargetVSync) {
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+
+    EXPECT_CALL(mMockClock, alarmAt(_, 500));
+    CountingCallback cb(mDispatch);
+    auto result =
+            mDispatch->schedule(cb,
+                                {.workDuration = 500, .readyDuration = 0, .earliestVsync = 1000});
+    EXPECT_TRUE(result.has_value());
+    EXPECT_EQ(500, *result);
+    mMockClock.advanceBy(400);
+
+    result = mDispatch->schedule(cb,
+                                 {.workDuration = 800, .readyDuration = 0, .earliestVsync = 1000});
+    EXPECT_TRUE(result.has_value());
+    EXPECT_EQ(400, *result);
+
     advanceToNextCallback();
     ASSERT_THAT(cb.mCalls.size(), Eq(1));
 }
@@ -826,6 +851,8 @@ TEST_F(VSyncDispatchTimerQueueTest, canScheduleLargeNegativeOffset) {
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState) {
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+
     EXPECT_CALL(mMockClock, alarmAt(_, 600));
 
     CountingCallback cb(mDispatch);
@@ -843,6 +870,26 @@ TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState)
     advanceToNextCallback();
 }
 
+TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesAffectSchedulingState) {
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+
+    EXPECT_CALL(mMockClock, alarmAt(_, 600));
+
+    CountingCallback cb(mDispatch);
+    auto result =
+            mDispatch->schedule(cb,
+                                {.workDuration = 400, .readyDuration = 0, .earliestVsync = 1000});
+    EXPECT_TRUE(result.has_value());
+    EXPECT_EQ(600, *result);
+
+    result = mDispatch->schedule(cb,
+                                 {.workDuration = 1400, .readyDuration = 0, .earliestVsync = 1000});
+    EXPECT_TRUE(result.has_value());
+    EXPECT_EQ(0, *result);
+
+    advanceToNextCallback();
+}
+
 TEST_F(VSyncDispatchTimerQueueTest, helperMove) {
     EXPECT_CALL(mMockClock, alarmAt(_, 500)).Times(1);
     EXPECT_CALL(mMockClock, alarmCancel()).Times(1);
@@ -1045,6 +1092,8 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmSettingFutureWithReadyDuration) {
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, updatesVsyncTimeForCloseWakeupTime) {
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+
     Sequence seq;
     EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
 
@@ -1065,6 +1114,29 @@ TEST_F(VSyncDispatchTimerQueueTest, updatesVsyncTimeForCloseWakeupTime) {
     EXPECT_THAT(cb.mReadyTime[0], Eq(2000));
 }
 
+TEST_F(VSyncDispatchTimerQueueTest, doesNotUpdatesVsyncTimeForCloseWakeupTime) {
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+
+    Sequence seq;
+    EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
+
+    CountingCallback cb(mDispatch);
+
+    mDispatch->schedule(cb, {.workDuration = 400, .readyDuration = 0, .earliestVsync = 1000});
+    mDispatch->schedule(cb, {.workDuration = 1400, .readyDuration = 0, .earliestVsync = 1000});
+
+    advanceToNextCallback();
+
+    advanceToNextCallback();
+
+    ASSERT_THAT(cb.mCalls.size(), Eq(1));
+    EXPECT_THAT(cb.mCalls[0], Eq(1000));
+    ASSERT_THAT(cb.mWakeupTime.size(), Eq(1));
+    EXPECT_THAT(cb.mWakeupTime[0], Eq(600));
+    ASSERT_THAT(cb.mReadyTime.size(), Eq(1));
+    EXPECT_THAT(cb.mReadyTime[0], Eq(1000));
+}
+
 TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) {
     SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
 
@@ -1101,7 +1173,7 @@ TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) {
     result = mDispatch->schedule(cb,
                                  {.workDuration = 800, .readyDuration = 0, .earliestVsync = 1000});
     EXPECT_TRUE(result.has_value());
-    EXPECT_EQ(200, *result);
+    EXPECT_EQ(300, *result);
 
     advanceToNextCallback();
     ASSERT_THAT(cb.mCalls.size(), Eq(1));