SF: pass a render rate to VsyncTracker instead of a divisor
passing a divisor to VsyncTracker might result in the wrong frame
rate when the vsync period is changing. The Divisor was calculated in
VsyncReactor which calculates it based on the new period, and passed
to VsyncTracker which might still be learning the new period, hence
using the old period.
Bug: 267780202
Test: SF unit tests
Change-Id: Ibb632d4ae6621a6ac0c0123e78f4c4d75699bd9e
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index 47c2dee..2908834 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -54,7 +54,7 @@
void resetModel() final {}
bool needsMoreSamples() const final { return false; }
bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
- void setDivisor(unsigned) final {}
+ void setRenderRate(Fps) final {}
void dump(std::string&) const final {}
private:
@@ -92,7 +92,7 @@
void resetModel() final {}
bool needsMoreSamples() const final { return false; }
bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
- void setDivisor(unsigned) final {}
+ void setRenderRate(Fps) final {}
void dump(std::string&) const final {}
private:
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 14a2860..f143b49 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -55,7 +55,7 @@
MOCK_METHOD0(resetModel, void());
MOCK_CONST_METHOD0(needsMoreSamples, bool());
MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, Fps));
- MOCK_METHOD(void, setDivisor, (unsigned), (override));
+ MOCK_METHOD(void, setRenderRate, (Fps), (override));
MOCK_CONST_METHOD1(dump, void(std::string&));
nsecs_t nextVSyncTime(nsecs_t timePoint) const {
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index 48d39cf..db531bf 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -554,7 +554,7 @@
EXPECT_THAT(intercept, IsCloseTo(expectedIntercept, mMaxRoundingError));
}
-TEST_F(VSyncPredictorTest, setDivisorIsRespected) {
+TEST_F(VSyncPredictorTest, setRenderRateIsRespected) {
auto last = mNow;
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(last + mPeriod));
@@ -563,7 +563,7 @@
tracker.addVsyncTimestamp(mNow);
}
- tracker.setDivisor(3);
+ tracker.setRenderRate(Fps::fromPeriodNsecs(3 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(mNow + mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 100), Eq(mNow + mPeriod));
@@ -574,7 +574,7 @@
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 5100), Eq(mNow + 7 * mPeriod));
}
-TEST_F(VSyncPredictorTest, setDivisorOfDivosorIsInPhase) {
+TEST_F(VSyncPredictorTest, setRenderRateOfDivisorIsInPhase) {
auto last = mNow;
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(last + mPeriod));
@@ -583,18 +583,44 @@
tracker.addVsyncTimestamp(mNow);
}
- tracker.setDivisor(4);
+ const auto refreshRate = Fps::fromPeriodNsecs(mPeriod);
+
+ tracker.setRenderRate(refreshRate / 4);
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(mNow + 3 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 3 * mPeriod), Eq(mNow + 7 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 7 * mPeriod), Eq(mNow + 11 * mPeriod));
- tracker.setDivisor(2);
+ tracker.setRenderRate(refreshRate / 2);
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(mNow + 1 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 1 * mPeriod), Eq(mNow + 3 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 3 * mPeriod), Eq(mNow + 5 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 5 * mPeriod), Eq(mNow + 7 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 7 * mPeriod), Eq(mNow + 9 * mPeriod));
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 9 * mPeriod), Eq(mNow + 11 * mPeriod));
+
+ tracker.setRenderRate(refreshRate / 6);
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(mNow + 1 * mPeriod));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 1 * mPeriod), Eq(mNow + 7 * mPeriod));
+}
+
+TEST_F(VSyncPredictorTest, setRenderRateIsIgnoredIfNotDivisor) {
+ auto last = mNow;
+ for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(last + mPeriod));
+ mNow += mPeriod;
+ last = mNow;
+ tracker.addVsyncTimestamp(mNow);
+ }
+
+ tracker.setRenderRate(Fps::fromPeriodNsecs(3.5f * mPeriod));
+
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(mNow + mPeriod));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 100), Eq(mNow + mPeriod));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 1100), Eq(mNow + 2 * mPeriod));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 2100), Eq(mNow + 3 * mPeriod));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 3100), Eq(mNow + 4 * mPeriod));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 4100), Eq(mNow + 5 * mPeriod));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 5100), Eq(mNow + 6 * mPeriod));
}
} // namespace android::scheduler
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index 1fb2709..fb8d989 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -50,7 +50,7 @@
MOCK_METHOD0(resetModel, void());
MOCK_CONST_METHOD0(needsMoreSamples, bool());
MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, Fps));
- MOCK_METHOD(void, setDivisor, (unsigned), (override));
+ MOCK_METHOD(void, setRenderRate, (Fps), (override));
MOCK_CONST_METHOD1(dump, void(std::string&));
};
diff --git a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
index 6893154..dcf25e1 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
@@ -34,7 +34,7 @@
MOCK_METHOD0(resetModel, void());
MOCK_CONST_METHOD0(needsMoreSamples, bool());
MOCK_CONST_METHOD2(isVSyncInPhase, bool(nsecs_t, Fps));
- MOCK_METHOD(void, setDivisor, (unsigned), (override));
+ MOCK_METHOD(void, setRenderRate, (Fps), (override));
MOCK_CONST_METHOD1(dump, void(std::string&));
};