SF: minor bug fix to flags::dont_skip_on_early
Bug: 306686616
Test: presubmit
Change-Id: I352ad2f6ad5971e598441972066cdcab71382b4d
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index c4c9fa5..f467670 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -40,16 +40,18 @@
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 @@
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 @@
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 b8fdce1..1dc5498 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -759,6 +759,8 @@
// 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 @@
{.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, scheduleUpdatesDoesNotAffectSchedulingState) {
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+
EXPECT_CALL(mMockClock, alarmAt(_, 600));
CountingCallback cb(mDispatch);
@@ -843,6 +870,26 @@
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, updatesVsyncTimeForCloseWakeupTime) {
+ SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+
Sequence seq;
EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
@@ -1065,6 +1114,29 @@
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 @@
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));