Merge "SF: explicitly mark surface frame without a composite as non janky" into main
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
index d0e2d7a..8369a1e 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp
@@ -681,6 +681,15 @@
}
}
+void SurfaceFrame::onCommitNotComposited(Fps refreshRate, Fps displayFrameRenderRate) {
+ std::scoped_lock lock(mMutex);
+
+ mDisplayFrameRenderRate = displayFrameRenderRate;
+ mActuals.presentTime = mPredictions.presentTime;
+ nsecs_t deadlineDelta = 0;
+ classifyJankLocked(JankType::None, refreshRate, displayFrameRenderRate, deadlineDelta);
+}
+
void SurfaceFrame::tracePredictions(int64_t displayFrameToken, nsecs_t monoBootOffset) const {
int64_t expectedTimelineCookie = mTraceCookieCounter.getCookieForTracing();
@@ -912,6 +921,15 @@
finalizeCurrentDisplayFrame();
}
+void FrameTimeline::onCommitNotComposited() {
+ ATRACE_CALL();
+ std::scoped_lock lock(mMutex);
+ mCurrentDisplayFrame->onCommitNotComposited();
+ mCurrentDisplayFrame.reset();
+ mCurrentDisplayFrame = std::make_shared<DisplayFrame>(mTimeStats, mJankClassificationThresholds,
+ &mTraceCookieCounter);
+}
+
void FrameTimeline::DisplayFrame::addSurfaceFrame(std::shared_ptr<SurfaceFrame> surfaceFrame) {
mSurfaceFrames.push_back(surfaceFrame);
}
@@ -1094,6 +1112,12 @@
}
}
+void FrameTimeline::DisplayFrame::onCommitNotComposited() {
+ for (auto& surfaceFrame : mSurfaceFrames) {
+ surfaceFrame->onCommitNotComposited(mRefreshRate, mRenderRate);
+ }
+}
+
void FrameTimeline::DisplayFrame::tracePredictions(pid_t surfaceFlingerPid,
nsecs_t monoBootOffset) const {
int64_t expectedTimelineCookie = mTraceCookieCounter.getCookieForTracing();
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
index a76f7d4..21bc95a 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
@@ -204,6 +204,8 @@
void onPresent(nsecs_t presentTime, int32_t displayFrameJankType, Fps refreshRate,
Fps displayFrameRenderRate, nsecs_t displayDeadlineDelta,
nsecs_t displayPresentDelta);
+ // Sets the frame as none janky as there was no real display frame.
+ void onCommitNotComposited(Fps refreshRate, Fps displayFrameRenderRate);
// All the timestamps are dumped relative to the baseTime
void dump(std::string& result, const std::string& indent, nsecs_t baseTime) const;
// Dumps only the layer, token, is buffer, jank metadata, prediction and present states.
@@ -318,6 +320,10 @@
virtual void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence,
const std::shared_ptr<FenceTime>& gpuFence) = 0;
+ // Tells FrameTimeline that a frame was committed but not composited. This is used to flush
+ // all the associated surface frames.
+ virtual void onCommitNotComposited() = 0;
+
// Args:
// -jank : Dumps only the Display Frames that are either janky themselves
// or contain janky Surface Frames.
@@ -390,6 +396,8 @@
std::optional<TimelineItem> predictions, nsecs_t wakeUpTime);
// Sets the appropriate metadata and classifies the jank.
void onPresent(nsecs_t signalTime, nsecs_t previousPresentTime);
+ // Flushes all the surface frames as those were not generating any actual display frames.
+ void onCommitNotComposited();
// Adds the provided SurfaceFrame to the current display frame.
void addSurfaceFrame(std::shared_ptr<SurfaceFrame> surfaceFrame);
@@ -475,6 +483,7 @@
void setSfWakeUp(int64_t token, nsecs_t wakeupTime, Fps refreshRate, Fps renderRate) override;
void setSfPresent(nsecs_t sfPresentTime, const std::shared_ptr<FenceTime>& presentFence,
const std::shared_ptr<FenceTime>& gpuFence = FenceTime::NO_FENCE) override;
+ void onCommitNotComposited() override;
void parseArgs(const Vector<String16>& args, std::string& result) override;
void setMaxDisplayFrames(uint32_t size) override;
float computeFps(const std::unordered_set<int32_t>& layerIds) override;
diff --git a/services/surfaceflinger/Scheduler/ISchedulerCallback.h b/services/surfaceflinger/Scheduler/ISchedulerCallback.h
index 9f4f5b6..43cdb5e 100644
--- a/services/surfaceflinger/Scheduler/ISchedulerCallback.h
+++ b/services/surfaceflinger/Scheduler/ISchedulerCallback.h
@@ -32,6 +32,7 @@
virtual void onChoreographerAttached() = 0;
virtual void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>,
Fps renderRate) = 0;
+ virtual void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) = 0;
protected:
~ISchedulerCallback() = default;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 005ec05..f72e7a0 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -221,6 +221,7 @@
if (FlagManager::getInstance().vrr_config()) {
compositor.sendNotifyExpectedPresentHint(pacesetterPtr->displayId);
}
+ mSchedulerCallback.onCommitNotComposited(pacesetterPtr->displayId);
return;
}
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5bb279e..33dbab0 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4405,6 +4405,12 @@
scheduleNotifyExpectedPresentHint(displayId);
}
+void SurfaceFlinger::onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) {
+ if (FlagManager::getInstance().commit_not_composited()) {
+ mFrameTimeline->onCommitNotComposited();
+ }
+}
+
void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) {
using namespace scheduler;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 8474515..edcc8d3 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -700,6 +700,8 @@
void onChoreographerAttached() override;
void onExpectedPresentTimePosted(TimePoint expectedPresentTime, ftl::NonNull<DisplayModePtr>,
Fps renderRate) override;
+ void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) override
+ REQUIRES(kMainThreadContext);
// ICEPowerCallback overrides:
void notifyCpuLoadUp() override;
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index 45b3290..e73cf5d 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -144,6 +144,7 @@
DUMP_READ_ONLY_FLAG(deprecate_vsync_sf);
DUMP_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter);
DUMP_READ_ONLY_FLAG(detached_mirror);
+ DUMP_READ_ONLY_FLAG(commit_not_composited);
#undef DUMP_READ_ONLY_FLAG
#undef DUMP_SERVER_FLAG
@@ -238,6 +239,7 @@
FLAG_MANAGER_READ_ONLY_FLAG(deprecate_vsync_sf, "");
FLAG_MANAGER_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter, "");
FLAG_MANAGER_READ_ONLY_FLAG(detached_mirror, "");
+FLAG_MANAGER_READ_ONLY_FLAG(commit_not_composited, "");
/// Trunk stable server flags ///
FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "")
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index 592e774..327cc4a 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -83,6 +83,7 @@
bool deprecate_vsync_sf() const;
bool allow_n_vsyncs_in_targeter() const;
bool detached_mirror() const;
+ bool commit_not_composited() const;
protected:
// overridden for unit tests
diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
index 4d3195d..f1ec3e1 100644
--- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
@@ -22,6 +22,17 @@
} # ce_fence_promise
flag {
+ name: "commit_not_composited"
+ namespace: "core_graphics"
+ description: "mark frames as non janky if the transaction resulted in no composition"
+ bug: "340633280"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+ } # commit_not_composited
+
+ flag {
name: "deprecate_vsync_sf"
namespace: "core_graphics"
description: "Depracate eVsyncSourceSurfaceFlinger and use vsync_app everywhere"
diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
index ddc3967..9fd687c 100644
--- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
@@ -341,6 +341,57 @@
EXPECT_NE(surfaceFrame2->getJankSeverityType(), std::nullopt);
}
+TEST_F(FrameTimelineTest, displayFrameSkippedComposition) {
+ // Layer specific increment
+ EXPECT_CALL(*mTimeStats, incrementJankyFrames(_)).Times(1);
+ auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE);
+ int64_t surfaceFrameToken1 = mTokenManager->generateTokenForPredictions({10, 20, 30});
+ int64_t sfToken1 = mTokenManager->generateTokenForPredictions({22, 26, 30});
+ FrameTimelineInfo ftInfo;
+ ftInfo.vsyncId = surfaceFrameToken1;
+ ftInfo.inputEventId = sInputEventId;
+ auto surfaceFrame1 =
+ mFrameTimeline->createSurfaceFrameForToken(ftInfo, sPidOne, sUidOne, sLayerIdOne,
+ sLayerNameOne, sLayerNameOne,
+ /*isBuffer*/ true, sGameMode);
+ auto surfaceFrame2 =
+ mFrameTimeline->createSurfaceFrameForToken(ftInfo, sPidOne, sUidOne, sLayerIdTwo,
+ sLayerNameTwo, sLayerNameTwo,
+ /*isBuffer*/ true, sGameMode);
+
+ mFrameTimeline->setSfWakeUp(sfToken1, 22, RR_11, RR_11);
+ surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented);
+ mFrameTimeline->addSurfaceFrame(surfaceFrame1);
+ mFrameTimeline->onCommitNotComposited();
+
+ EXPECT_EQ(surfaceFrame1->getActuals().presentTime, 30);
+ ASSERT_NE(surfaceFrame1->getJankType(), std::nullopt);
+ EXPECT_EQ(*surfaceFrame1->getJankType(), JankType::None);
+ ASSERT_NE(surfaceFrame1->getJankSeverityType(), std::nullopt);
+ EXPECT_EQ(*surfaceFrame1->getJankSeverityType(), JankSeverityType::None);
+
+ mFrameTimeline->setSfWakeUp(sfToken1, 22, RR_11, RR_11);
+ surfaceFrame2->setPresentState(SurfaceFrame::PresentState::Presented);
+ mFrameTimeline->addSurfaceFrame(surfaceFrame2);
+ mFrameTimeline->setSfPresent(26, presentFence1);
+
+ auto displayFrame = getDisplayFrame(0);
+ auto& presentedSurfaceFrame2 = getSurfaceFrame(0, 0);
+ presentFence1->signalForTest(42);
+
+ // Fences haven't been flushed yet, so it should be 0
+ EXPECT_EQ(displayFrame->getActuals().presentTime, 0);
+ EXPECT_EQ(presentedSurfaceFrame2.getActuals().presentTime, 0);
+
+ addEmptyDisplayFrame();
+
+ // Fences have flushed, so the present timestamps should be updated
+ EXPECT_EQ(displayFrame->getActuals().presentTime, 42);
+ EXPECT_EQ(presentedSurfaceFrame2.getActuals().presentTime, 42);
+ EXPECT_NE(surfaceFrame2->getJankType(), std::nullopt);
+ EXPECT_NE(surfaceFrame2->getJankSeverityType(), std::nullopt);
+}
+
TEST_F(FrameTimelineTest, displayFramesSlidingWindowMovesAfterLimit) {
// Insert kMaxDisplayFrames' count of DisplayFrames to fill the deque
int frameTimeFactor = 0;
diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
index 4ca0542..dec5fa5 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
@@ -30,6 +30,7 @@
MOCK_METHOD(void, onChoreographerAttached, (), (override));
MOCK_METHOD(void, onExpectedPresentTimePosted, (TimePoint, ftl::NonNull<DisplayModePtr>, Fps),
(override));
+ MOCK_METHOD(void, onCommitNotComposited, (PhysicalDisplayId), (override));
};
struct NoOpSchedulerCallback final : ISchedulerCallback {
@@ -39,6 +40,7 @@
void triggerOnFrameRateOverridesChanged() override {}
void onChoreographerAttached() override {}
void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>, Fps) override {}
+ void onCommitNotComposited(PhysicalDisplayId) override {}
};
} // namespace android::scheduler::mock