SF: Avoid adjusting the frame time when last frame is presented early
BUG: 350738983
Test: atest VSyncPredictorTest
Flag: EXEMPT bugfix
Change-Id: I7206811bc83929a45bac66d9fc1ad0a5cfb70f11
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index 16799bd..e79eac0 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -486,13 +486,13 @@
return 0ns;
}
-void VSyncPredictor::onFrameBegin(TimePoint expectedPresentTime,
- TimePoint lastConfirmedPresentTime) {
+void VSyncPredictor::onFrameBegin(TimePoint expectedPresentTime, FrameTime lastSignaledFrameTime) {
SFTRACE_NAME("VSyncPredictor::onFrameBegin");
std::lock_guard lock(mMutex);
if (!mDisplayModePtr->getVrrConfig()) return;
+ const auto [lastConfirmedPresentTime, lastConfirmedExpectedPresentTime] = lastSignaledFrameTime;
if (CC_UNLIKELY(mTraceOn)) {
SFTRACE_FORMAT_INSTANT("vsync is %.2f past last signaled fence",
static_cast<float>(expectedPresentTime.ns() -
@@ -518,6 +518,11 @@
}
}
+ if (lastConfirmedExpectedPresentTime.ns() - lastConfirmedPresentTime.ns() > threshold) {
+ SFTRACE_FORMAT_INSTANT("lastFramePresentedEarly");
+ return;
+ }
+
const auto phase = ensureMinFrameDurationIsKept(expectedPresentTime, lastConfirmedPresentTime);
if (phase > 0ns) {
mMissedVsync = {expectedPresentTime, minFramePeriodLocked()};
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.h b/services/surfaceflinger/Scheduler/VSyncPredictor.h
index 66a7d71..db5f455 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.h
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.h
@@ -22,6 +22,7 @@
#include <vector>
#include <android-base/thread_annotations.h>
+#include <scheduler/FrameTime.h>
#include <scheduler/TimeKeeper.h>
#include <ui/DisplayId.h>
@@ -77,7 +78,7 @@
void setRenderRate(Fps, bool applyImmediately) final EXCLUDES(mMutex);
- void onFrameBegin(TimePoint expectedPresentTime, TimePoint lastConfirmedPresentTime) final
+ void onFrameBegin(TimePoint expectedPresentTime, FrameTime lastSignaledFrameTime) final
EXCLUDES(mMutex);
void onFrameMissed(TimePoint expectedPresentTime) final EXCLUDES(mMutex);
diff --git a/services/surfaceflinger/Scheduler/VSyncTracker.h b/services/surfaceflinger/Scheduler/VSyncTracker.h
index 134d28e..3376fad 100644
--- a/services/surfaceflinger/Scheduler/VSyncTracker.h
+++ b/services/surfaceflinger/Scheduler/VSyncTracker.h
@@ -21,6 +21,7 @@
#include <scheduler/Fps.h>
#include <scheduler/FrameRateMode.h>
+#include <scheduler/FrameTime.h>
#include "VSyncDispatch.h"
@@ -112,8 +113,7 @@
*/
virtual void setRenderRate(Fps, bool applyImmediately) = 0;
- virtual void onFrameBegin(TimePoint expectedPresentTime,
- TimePoint lastConfirmedPresentTime) = 0;
+ virtual void onFrameBegin(TimePoint expectedPresentTime, FrameTime lastSignaledFrameTime) = 0;
virtual void onFrameMissed(TimePoint expectedPresentTime) = 0;
diff --git a/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h b/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h
index d37d2dc..38cb446 100644
--- a/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h
+++ b/services/surfaceflinger/Scheduler/include/scheduler/FrameTargeter.h
@@ -26,6 +26,7 @@
#include <ui/FenceTime.h>
#include <scheduler/Features.h>
+#include <scheduler/FrameTime.h>
#include <scheduler/Time.h>
#include <scheduler/VsyncId.h>
#include <scheduler/interface/CompositeResult.h>
@@ -57,13 +58,6 @@
// The time of the VSYNC that preceded this frame. See `presentFenceForPastVsync` for details.
TimePoint pastVsyncTime(Period minFramePeriod) const;
- // The present fence for the frame that had targeted the most recent VSYNC before this frame.
- // If the target VSYNC for any given frame is more than `vsyncPeriod` in the future, then the
- // VSYNC of at least one previous frame has not yet passed. In other words, this is NOT the
- // `presentFenceForPreviousFrame` if running N VSYNCs ahead, but the one that should have been
- // signaled by now (unless that frame missed).
- FenceTimePtr presentFenceForPastVsync(Period minFramePeriod) const;
-
// Equivalent to `presentFenceForPastVsync` unless running N VSYNCs ahead.
const FenceTimePtr& presentFenceForPreviousFrame() const {
return mPresentFences.front().fenceTime;
@@ -72,7 +66,7 @@
bool isFramePending() const { return mFramePending; }
bool didMissFrame() const { return mFrameMissed; }
bool didMissHwcFrame() const { return mHwcFrameMissed && !mGpuFrameMissed; }
- TimePoint lastSignaledFrameTime() const { return mLastSignaledFrameTime; };
+ FrameTime lastSignaledFrameTime() const { return mLastSignaledFrameTime; }
protected:
explicit FrameTarget(const std::string& displayLabel);
@@ -106,10 +100,17 @@
FenceTimePtr fenceTime = FenceTime::NO_FENCE;
TimePoint expectedPresentTime = TimePoint();
};
+
+ // The present fence for the frame that had targeted the most recent VSYNC before this frame.
+ // If the target VSYNC for any given frame is more than `vsyncPeriod` in the future, then the
+ // VSYNC of at least one previous frame has not yet passed. In other words, this is NOT the
+ // `presentFenceForPreviousFrame` if running N VSYNCs ahead, but the one that should have been
+ // signaled by now (unless that frame missed).
+ FenceWithFenceTime presentFenceForPastVsync(Period minFramePeriod) const;
std::array<FenceWithFenceTime, 2> mPresentFences;
utils::RingBuffer<FenceWithFenceTime, 5> mFenceWithFenceTimes;
- TimePoint mLastSignaledFrameTime;
+ FrameTime mLastSignaledFrameTime;
private:
friend class FrameTargeterTestBase;
@@ -120,16 +121,17 @@
return expectedFrameDuration() > (N - 1) * minFramePeriod;
}
- const FenceTimePtr pastVsyncTimePtr() const {
- auto pastFenceTimePtr = FenceTime::NO_FENCE;
+ FenceWithFenceTime pastVsyncTimePtr() const {
+ FenceWithFenceTime pastFenceWithFenceTime;
for (size_t i = 0; i < mFenceWithFenceTimes.size(); i++) {
- const auto& [_, fenceTimePtr, expectedPresentTime] = mFenceWithFenceTimes[i];
- if (expectedPresentTime > mFrameBeginTime) {
- return pastFenceTimePtr;
+ const auto& fenceWithFenceTime = mFenceWithFenceTimes[i];
+ // TODO(b/354007767) Fix the below condition to avoid frame drop
+ if (fenceWithFenceTime.expectedPresentTime > mFrameBeginTime) {
+ return pastFenceWithFenceTime;
}
- pastFenceTimePtr = fenceTimePtr;
+ pastFenceWithFenceTime = fenceWithFenceTime;
}
- return pastFenceTimePtr;
+ return pastFenceWithFenceTime;
}
};
diff --git a/services/surfaceflinger/Scheduler/include/scheduler/FrameTime.h b/services/surfaceflinger/Scheduler/include/scheduler/FrameTime.h
new file mode 100644
index 0000000..ed5c899
--- /dev/null
+++ b/services/surfaceflinger/Scheduler/include/scheduler/FrameTime.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <scheduler/Time.h>
+
+namespace android::scheduler {
+struct FrameTime {
+ TimePoint signalTime;
+ TimePoint expectedPresentTime;
+};
+} // namespace android::scheduler
\ No newline at end of file
diff --git a/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp b/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp
index 60694b9..1d248fb 100644
--- a/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp
+++ b/services/surfaceflinger/Scheduler/src/FrameTargeter.cpp
@@ -33,12 +33,12 @@
return mExpectedPresentTime - Period::fromNs(minFramePeriod.ns() << shift);
}
-FenceTimePtr FrameTarget::presentFenceForPastVsync(Period minFramePeriod) const {
+FrameTarget::FenceWithFenceTime FrameTarget::presentFenceForPastVsync(Period minFramePeriod) const {
if (FlagManager::getInstance().allow_n_vsyncs_in_targeter()) {
return pastVsyncTimePtr();
}
const size_t i = static_cast<size_t>(targetsVsyncsAhead<2>(minFramePeriod));
- return mPresentFences[i].fenceTime;
+ return mPresentFences[i];
}
bool FrameTarget::wouldPresentEarly(Period minFramePeriod) const {
@@ -51,7 +51,7 @@
return true;
}
- const auto fence = presentFenceForPastVsync(minFramePeriod);
+ const auto fence = presentFenceForPastVsync(minFramePeriod).fenceTime;
return fence->isValid() && fence->getSignalTime() != Fence::SIGNAL_TIME_PENDING;
}
@@ -93,7 +93,7 @@
ticks<std::milli, float>(mExpectedPresentTime - TimePoint::now()),
mExpectedPresentTime == args.expectedVsyncTime ? "" : " (adjusted)");
- const FenceTimePtr& pastPresentFence = presentFenceForPastVsync(minFramePeriod);
+ FenceWithFenceTime pastPresentFence = presentFenceForPastVsync(minFramePeriod);
// In cases where the present fence is about to fire, give it a small grace period instead of
// giving up on the frame.
@@ -105,8 +105,8 @@
// Pending frames may trigger backpressure propagation.
const auto& isFencePending = *isFencePendingFuncPtr;
- mFramePending = pastPresentFence != FenceTime::NO_FENCE &&
- isFencePending(pastPresentFence, graceTimeForPresentFenceMs);
+ mFramePending = pastPresentFence.fenceTime != FenceTime::NO_FENCE &&
+ isFencePending(pastPresentFence.fenceTime, graceTimeForPresentFenceMs);
// A frame is missed if the prior frame is still pending. If no longer pending, then we still
// count the frame as missed if the predicted present time was further in the past than when the
@@ -114,9 +114,10 @@
// than a typical frame duration, but should not be so small that it reports reasonable drift as
// a missed frame.
mFrameMissed = mFramePending || [&] {
- const nsecs_t pastPresentTime = pastPresentFence->getSignalTime();
+ const nsecs_t pastPresentTime = pastPresentFence.fenceTime->getSignalTime();
if (pastPresentTime < 0) return false;
- mLastSignaledFrameTime = TimePoint::fromNs(pastPresentTime);
+ mLastSignaledFrameTime = {.signalTime = TimePoint::fromNs(pastPresentTime),
+ .expectedPresentTime = pastPresentFence.expectedPresentTime};
const nsecs_t frameMissedSlop = vsyncPeriod.ns() / 2;
return lastScheduledPresentTime.ns() < pastPresentTime - frameMissedSlop;
}();
diff --git a/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp b/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp
index 5448eec..190d062 100644
--- a/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp
+++ b/services/surfaceflinger/Scheduler/tests/FrameTargeterTest.cpp
@@ -57,6 +57,10 @@
return target().wouldPresentEarly(minFramePeriod);
}
+ FrameTarget::FenceWithFenceTime presentFenceForPastVsync(Period minFramePeriod) const {
+ return target().presentFenceForPastVsync(minFramePeriod);
+ }
+
struct Frame {
Frame(FrameTargeterTestBase* testPtr, VsyncId vsyncId, TimePoint& frameBeginTime,
Duration frameDuration, Fps refreshRate, Fps peakRefreshRate,
@@ -181,7 +185,7 @@
const auto fence = frame.end();
EXPECT_EQ(target().pastVsyncTime(kPeriod), frameBeginTime + kFrameDuration - kPeriod);
- EXPECT_EQ(target().presentFenceForPastVsync(kPeriod), fence);
+ EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, fence);
}
}
@@ -200,7 +204,7 @@
const auto fence = frame.end();
EXPECT_EQ(target().pastVsyncTime(kPeriod), frameBeginTime + kFrameDuration - 2 * kPeriod);
- EXPECT_EQ(target().presentFenceForPastVsync(kPeriod), previousFence);
+ EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, previousFence);
previousFence = fence;
}
@@ -222,7 +226,7 @@
const auto pastVsyncTime = frameBeginTime + kFrameDuration - 2 * kPeriod;
EXPECT_EQ(target().pastVsyncTime(kPeriod), pastVsyncTime);
- EXPECT_EQ(target().presentFenceForPastVsync(kFrameDuration), previousFence);
+ EXPECT_EQ(presentFenceForPastVsync(kFrameDuration).fenceTime, previousFence);
frameBeginTime += kPeriod;
previousFence = fence;
@@ -248,7 +252,7 @@
const auto fence = frame.end();
EXPECT_EQ(target().pastVsyncTime(kPeriod), frameBeginTime + kFrameDuration - 2 * kPeriod);
- EXPECT_EQ(target().presentFenceForPastVsync(kPeriod), previousFence);
+ EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, previousFence);
previousFence = fence;
}
@@ -274,7 +278,7 @@
const auto pastVsyncTime = frameBeginTime + kFrameDuration - 2 * kPeriod;
EXPECT_EQ(target().pastVsyncTime(kPeriod), pastVsyncTime);
- EXPECT_EQ(target().presentFenceForPastVsync(kFrameDuration), previousFence);
+ EXPECT_EQ(presentFenceForPastVsync(kFrameDuration).fenceTime, previousFence);
frameBeginTime += kPeriod;
previousFence = fence;
@@ -283,7 +287,7 @@
TEST_F(FrameTargeterTest, doesNotDetectEarlyPresentIfNoFence) {
constexpr Period kPeriod = (60_Hz).getPeriod();
- EXPECT_EQ(target().presentFenceForPastVsync(kPeriod), FenceTime::NO_FENCE);
+ EXPECT_EQ(presentFenceForPastVsync(kPeriod).fenceTime, FenceTime::NO_FENCE);
EXPECT_FALSE(wouldPresentEarly(kPeriod));
}