SF: dont shift vsync timeline unless missed a frame
Bug: 329036771
Test: presubmit
Test: com.android.uibench.microbenchmark.UiBenchInflatingHanListViewFlingMicrobenchmark
Change-Id: Id0acfd4aff9b002df09fe79f175e07814f46c5a8
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index acb3760..b3c1f6b 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -635,33 +635,30 @@
const auto threshold = model.slope / 2;
const auto lastFrameMissed =
lastVsyncOpt && std::abs(*lastVsyncOpt - missedVsync.vsync.ns()) < threshold;
- nsecs_t vsyncFixupTime = 0;
if (FlagManager::getInstance().vrr_config() && lastFrameMissed) {
// If the last frame missed is the last vsync, we already shifted the timeline. Depends on
// whether we skipped the frame (onFrameMissed) or not (onFrameBegin) we apply a different
// fixup. There is no need to to shift the vsync timeline again.
vsyncTime += missedVsync.fixup.ns();
ATRACE_FORMAT_INSTANT("lastFrameMissed");
- } else if (minFramePeriodOpt) {
- if (FlagManager::getInstance().vrr_config() && lastVsyncOpt) {
- // lastVsyncOpt is based on the old timeline before we shifted it. we should correct it
- // first before trying to use it.
- if (mLastVsyncSequence->seq > 0) {
- lastVsyncOpt = snapToVsyncAlignedWithRenderRate(model, *lastVsyncOpt);
- }
- const auto vsyncDiff = vsyncTime - *lastVsyncOpt;
- if (vsyncDiff <= minFramePeriodOpt->ns() - threshold) {
- vsyncFixupTime = *lastVsyncOpt + minFramePeriodOpt->ns() - vsyncTime;
- ATRACE_FORMAT_INSTANT("minFramePeriod violation. next in %.2f which is %.2f "
- "from "
- "prev. "
- "adjust by %.2f",
- static_cast<float>(vsyncTime - TimePoint::now().ns()) / 1e6f,
- static_cast<float>(vsyncTime - *lastVsyncOpt) / 1e6f,
- static_cast<float>(vsyncFixupTime) / 1e6f);
- }
+ } else if (FlagManager::getInstance().vrr_config() && minFramePeriodOpt && mRenderRateOpt &&
+ lastVsyncOpt) {
+ // lastVsyncOpt is based on the old timeline before we shifted it. we should correct it
+ // first before trying to use it.
+ lastVsyncOpt = snapToVsyncAlignedWithRenderRate(model, *lastVsyncOpt);
+ const auto vsyncDiff = vsyncTime - *lastVsyncOpt;
+ if (vsyncDiff <= minFramePeriodOpt->ns() - threshold) {
+ // avoid a duplicate vsync
+ ATRACE_FORMAT_INSTANT("skipping a vsync to avoid duplicate frame. next in %.2f which "
+ "is %.2f "
+ "from "
+ "prev. "
+ "adjust by %.2f",
+ static_cast<float>(vsyncTime - TimePoint::now().ns()) / 1e6f,
+ static_cast<float>(vsyncDiff) / 1e6f,
+ static_cast<float>(mRenderRateOpt->getPeriodNsecs()) / 1e6f);
+ vsyncTime += mRenderRateOpt->getPeriodNsecs();
}
- vsyncTime += vsyncFixupTime;
}
ATRACE_FORMAT_INSTANT("vsync in %.2fms", float(vsyncTime - TimePoint::now().ns()) / 1e6f);
@@ -671,12 +668,6 @@
return std::nullopt;
}
- // If we needed a fixup, it means that we changed the render rate and the chosen vsync would
- // cross minFramePeriod. In that case we need to shift the entire vsync timeline.
- if (vsyncFixupTime > 0) {
- shiftVsyncSequence(Duration::fromNs(vsyncFixupTime));
- }
-
return TimePoint::fromNs(vsyncTime);
}
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index d4735c7..e6bae90 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -607,10 +607,10 @@
EXPECT_EQ(Fps::fromPeriodNsecs(2000),
scheduler.getNextFrameInterval(kMode->getPhysicalDisplayId(),
- TimePoint::fromNs(4500)));
+ TimePoint::fromNs(5500)));
EXPECT_EQ(Fps::fromPeriodNsecs(2000),
scheduler.getNextFrameInterval(kMode->getPhysicalDisplayId(),
- TimePoint::fromNs(6500)));
+ TimePoint::fromNs(7500)));
}
TEST_F(SchedulerTest, resyncAllToHardwareVsync) FTL_FAKE_GUARD(kMainThreadContext) {
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index aac1cac..eafba0a 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -704,14 +704,17 @@
EXPECT_EQ(21500, vrrTracker.nextAnticipatedVSyncTimeFrom(19000, 19000));
vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000), /*applyImmediately*/ false);
- EXPECT_EQ(5000, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
- EXPECT_EQ(6000, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
- EXPECT_EQ(7000, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6000));
- EXPECT_EQ(9000, vrrTracker.nextAnticipatedVSyncTimeFrom(8000, 8000));
- EXPECT_EQ(11000, vrrTracker.nextAnticipatedVSyncTimeFrom(10000, 10000));
- EXPECT_EQ(13000, vrrTracker.nextAnticipatedVSyncTimeFrom(12000, 12000));
- EXPECT_EQ(17000, vrrTracker.nextAnticipatedVSyncTimeFrom(15500, 15500));
- EXPECT_EQ(20000, vrrTracker.nextAnticipatedVSyncTimeFrom(19000, 19000));
+ EXPECT_EQ(5500, vrrTracker.nextAnticipatedVSyncTimeFrom(4000, 4000));
+ EXPECT_EQ(6500, vrrTracker.nextAnticipatedVSyncTimeFrom(5000, 5000));
+ EXPECT_EQ(7500, vrrTracker.nextAnticipatedVSyncTimeFrom(6000, 6000));
+ EXPECT_EQ(9500, vrrTracker.nextAnticipatedVSyncTimeFrom(8000, 8000));
+ EXPECT_EQ(11500, vrrTracker.nextAnticipatedVSyncTimeFrom(10000, 10000));
+ EXPECT_EQ(13500, vrrTracker.nextAnticipatedVSyncTimeFrom(12000, 12000));
+ EXPECT_EQ(16500, vrrTracker.nextAnticipatedVSyncTimeFrom(15500, 15500));
+ EXPECT_EQ(20500, vrrTracker.nextAnticipatedVSyncTimeFrom(19000, 19000));
+
+ // matches the previous cadence
+ EXPECT_EQ(21500, vrrTracker.nextAnticipatedVSyncTimeFrom(20500, 20500));
}
TEST_F(VSyncPredictorTest, minFramePeriodDoesntApplyWhenSameWithRefreshRate) {
@@ -820,7 +823,7 @@
vrrTracker.setRenderRate(Fps::fromPeriodNsecs(1000), /*applyImmediately*/ false);
vrrTracker.addVsyncTimestamp(0);
- EXPECT_EQ(2000, vrrTracker.nextAnticipatedVSyncTimeFrom(1234, 1234));
+ EXPECT_EQ(2500, vrrTracker.nextAnticipatedVSyncTimeFrom(1234, 1234));
}
TEST_F(VSyncPredictorTest, adjustsVrrTimeline) {