Revert^2 "SF: Introduce VsyncTimeline to VsyncPredictor"
Add the concept of timeline freezing when switching render rate.
This allow us to change render rates in sync with the app and remain
jank free across render rate changes.
Bug: 326599221
Test: Run TouchLatency, change render rate and examine Perfetto trace
Change-Id: Ic91d482593d6b978c28e3c6c0d19e2c055d5f149
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 10e2220..049b092 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -25,6 +25,7 @@
#include "Scheduler/EventThread.h"
#include "Scheduler/RefreshRateSelector.h"
#include "Scheduler/VSyncPredictor.h"
+#include "Scheduler/VSyncReactor.h"
#include "TestableScheduler.h"
#include "TestableSurfaceFlinger.h"
#include "mock/DisplayHardware/MockDisplayMode.h"
@@ -563,7 +564,8 @@
hal::VrrConfig{.minFrameIntervalNs = static_cast<int32_t>(
frameRate.getPeriodNsecs())}));
std::shared_ptr<VSyncPredictor> vrrTracker =
- std::make_shared<VSyncPredictor>(kMode, kHistorySize, kMinimumSamplesForPrediction,
+ std::make_shared<VSyncPredictor>(std::make_unique<SystemClock>(), kMode, kHistorySize,
+ kMinimumSamplesForPrediction,
kOutlierTolerancePercent);
std::shared_ptr<RefreshRateSelector> vrrSelectorPtr =
std::make_shared<RefreshRateSelector>(makeModes(kMode), kMode->getId());
@@ -578,6 +580,8 @@
vrrSelectorPtr->setActiveMode(kMode->getId(), frameRate);
scheduler.setRenderRate(kMode->getPhysicalDisplayId(), frameRate);
vrrTracker->addVsyncTimestamp(0);
+ // Set 1000 as vsync seq #0
+ vrrTracker->nextAnticipatedVSyncTimeFrom(700);
EXPECT_EQ(Fps::fromPeriodNsecs(1000),
scheduler.getNextFrameInterval(kMode->getPhysicalDisplayId(),
@@ -587,7 +591,7 @@
TimePoint::fromNs(2000)));
// Not crossing the min frame period
- EXPECT_EQ(Fps::fromPeriodNsecs(1500),
+ EXPECT_EQ(Fps::fromPeriodNsecs(1000),
scheduler.getNextFrameInterval(kMode->getPhysicalDisplayId(),
TimePoint::fromNs(2500)));
// Change render rate
@@ -595,6 +599,9 @@
vrrSelectorPtr->setActiveMode(kMode->getId(), frameRate);
scheduler.setRenderRate(kMode->getPhysicalDisplayId(), frameRate);
+ // Set 2000 as vsync seq #0
+ vrrTracker->nextAnticipatedVSyncTimeFrom(1700);
+
EXPECT_EQ(Fps::fromPeriodNsecs(2000),
scheduler.getNextFrameInterval(kMode->getPhysicalDisplayId(),
TimePoint::fromNs(2000)));
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index d891008..c22deab 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -48,7 +48,7 @@
Period minFramePeriod() const final { return Period::fromNs(currentPeriod()); }
void resetModel() final {}
bool needsMoreSamples() const final { return false; }
- bool isVSyncInPhase(nsecs_t, Fps) const final { return false; }
+ bool isVSyncInPhase(nsecs_t, Fps) final { return false; }
void setDisplayModePtr(ftl::NonNull<DisplayModePtr>) final {}
void setRenderRate(Fps) final {}
void onFrameBegin(TimePoint, TimePoint) final {}
@@ -64,7 +64,7 @@
public:
FixedRateIdealStubTracker() : StubTracker{toNs(3ms)} {}
- nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t timePoint, std::optional<nsecs_t>) const final {
+ nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t timePoint, std::optional<nsecs_t>) final {
auto const floor = timePoint % mPeriod;
if (floor == 0) {
return timePoint;
@@ -77,7 +77,7 @@
public:
VRRStubTracker(nsecs_t period) : StubTracker(period) {}
- nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t time_point, std::optional<nsecs_t>) const final {
+ nsecs_t nextAnticipatedVSyncTimeFrom(nsecs_t time_point, std::optional<nsecs_t>) final {
std::lock_guard lock(mMutex);
auto const normalized_to_base = time_point - mBase;
auto const floor = (normalized_to_base) % mPeriod;
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index b9f3d70..6b9ea56 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -75,6 +75,28 @@
return ftl::as_non_null(createDisplayMode(DisplayModeId(0), refreshRate, kGroup, kResolution,
DEFAULT_DISPLAY_ID));
}
+
+class TestClock : public Clock {
+public:
+ TestClock() = default;
+
+ nsecs_t now() const override { return mNow; }
+ void setNow(nsecs_t now) { mNow = now; }
+
+private:
+ nsecs_t mNow = 0;
+};
+
+class ClockWrapper : public Clock {
+public:
+ ClockWrapper(std::shared_ptr<Clock> const& clock) : mClock(clock) {}
+
+ nsecs_t now() const { return mClock->now(); }
+
+private:
+ std::shared_ptr<Clock> const mClock;
+};
+
} // namespace
struct VSyncPredictorTest : testing::Test {
@@ -86,8 +108,10 @@
static constexpr size_t kOutlierTolerancePercent = 25;
static constexpr nsecs_t mMaxRoundingError = 100;
- VSyncPredictor tracker{mMode, kHistorySize, kMinimumSamplesForPrediction,
- kOutlierTolerancePercent};
+ std::shared_ptr<TestClock> mClock{std::make_shared<TestClock>()};
+
+ VSyncPredictor tracker{std::make_unique<ClockWrapper>(mClock), mMode, kHistorySize,
+ kMinimumSamplesForPrediction, kOutlierTolerancePercent};
};
TEST_F(VSyncPredictorTest, reportsAnticipatedPeriod) {
@@ -408,7 +432,8 @@
// See b/151146131
TEST_F(VSyncPredictorTest, hasEnoughPrecision) {
const auto mode = displayMode(mPeriod);
- VSyncPredictor tracker{mode, 20, kMinimumSamplesForPrediction, kOutlierTolerancePercent};
+ VSyncPredictor tracker{std::make_unique<ClockWrapper>(mClock), mode, 20,
+ kMinimumSamplesForPrediction, kOutlierTolerancePercent};
std::vector<nsecs_t> const simulatedVsyncs{840873348817, 840890049444, 840906762675,
840923581635, 840940161584, 840956868096,
840973702473, 840990256277, 841007116851,
@@ -606,35 +631,6 @@
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow + 5100), Eq(mNow + 7 * mPeriod));
}
-TEST_F(VSyncPredictorTest, setRenderRateOfDivisorIsInPhase) {
- 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);
- }
-
- 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.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++) {
@@ -670,8 +666,8 @@
.setVrrConfig(std::move(vrrConfig))
.build());
- VSyncPredictor vrrTracker{kMode, kHistorySize, kMinimumSamplesForPrediction,
- kOutlierTolerancePercent};
+ VSyncPredictor vrrTracker{std::make_unique<ClockWrapper>(mClock), kMode, kHistorySize,
+ kMinimumSamplesForPrediction, kOutlierTolerancePercent};
vrrTracker.setRenderRate(minFrameRate);
vrrTracker.addVsyncTimestamp(0);
@@ -687,7 +683,44 @@
vrrTracker.onFrameMissed(TimePoint::fromNs(4500));
EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4500, 4500));
EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
+
+ vrrTracker.onFrameBegin(TimePoint::fromNs(7000), TimePoint::fromNs(6500));
+ EXPECT_EQ(10500, vrrTracker.nextAnticipatedVSyncTimeFrom(9000, 7000));
}
+
+TEST_F(VSyncPredictorTest, renderRateIsPreservedForCommittedVsyncs) {
+ tracker.addVsyncTimestamp(1000);
+
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(6000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(7000));
+
+ tracker.setRenderRate(Fps::fromPeriodNsecs(2000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(6000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(7000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(7001), Eq(8000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(8001), Eq(10000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(9001), Eq(10000));
+
+ tracker.setRenderRate(Fps::fromPeriodNsecs(3000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(1000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(6000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(7000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(7001), Eq(8000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(8001), Eq(10000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(9001), Eq(10000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(10001), Eq(11000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(11001), Eq(14000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(12001), Eq(14000));
+
+ // Check the purge logic works
+ mClock->setNow(20000);
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(1), Eq(2000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(5001), Eq(8000));
+ EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(6001), Eq(8000));
+}
+
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues
diff --git a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
index 3870983..6d10a5c 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockVSyncTracker.h
@@ -29,12 +29,12 @@
MOCK_METHOD(bool, addVsyncTimestamp, (nsecs_t), (override));
MOCK_METHOD(nsecs_t, nextAnticipatedVSyncTimeFrom, (nsecs_t, std::optional<nsecs_t>),
- (const, override));
+ (override));
MOCK_METHOD(nsecs_t, currentPeriod, (), (const, override));
MOCK_METHOD(Period, minFramePeriod, (), (const, override));
MOCK_METHOD(void, resetModel, (), (override));
MOCK_METHOD(bool, needsMoreSamples, (), (const, override));
- MOCK_METHOD(bool, isVSyncInPhase, (nsecs_t, Fps), (const, override));
+ MOCK_METHOD(bool, isVSyncInPhase, (nsecs_t, Fps), (override));
MOCK_METHOD(void, setDisplayModePtr, (ftl::NonNull<DisplayModePtr>), (override));
MOCK_METHOD(void, setRenderRate, (Fps), (override));
MOCK_METHOD(void, onFrameBegin, (TimePoint, TimePoint), (override));