diff options
11 files changed, 110 insertions, 0 deletions
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index d0e2d7a451..8369a1ec64 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -681,6 +681,15 @@ void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, } } +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 @@ void FrameTimeline::setSfPresent(nsecs_t sfPresentTime, 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::onPresent(nsecs_t signalTime, nsecs_t previous } } +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 a76f7d477a..21bc95a4c5 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -204,6 +204,8 @@ public: 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 @@ public: 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 @@ public: 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 @@ public: 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 9f4f5b6d39..43cdb5ec41 100644 --- a/services/surfaceflinger/Scheduler/ISchedulerCallback.h +++ b/services/surfaceflinger/Scheduler/ISchedulerCallback.h @@ -32,6 +32,7 @@ struct ISchedulerCallback { 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 005ec05aab..f72e7a05d5 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -221,6 +221,7 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId, 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 5bb279e2fc..33dbab01df 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4405,6 +4405,12 @@ void SurfaceFlinger::sendNotifyExpectedPresentHint(PhysicalDisplayId displayId) 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 84745150d0..edcc8d3206 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -700,6 +700,8 @@ private: 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 45b3290b2b..e73cf5d785 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -144,6 +144,7 @@ void FlagManager::dump(std::string& result) const { 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(latch_unsignaled_with_auto_refresh_changed, ""); 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 592e7745c9..327cc4ae41 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -83,6 +83,7 @@ public: 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 4d3195db7f..f1ec3e1ec2 100644 --- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig +++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig @@ -22,6 +22,17 @@ flag { } # 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 ddc3967c40..9fd687c6e7 100644 --- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp +++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp @@ -341,6 +341,57 @@ TEST_F(FrameTimelineTest, presentFenceSignaled_presentedFramesUpdated) { 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 4ca05423d7..dec5fa56ea 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h +++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h @@ -30,6 +30,7 @@ struct SchedulerCallback final : ISchedulerCallback { 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 @@ struct NoOpSchedulerCallback final : ISchedulerCallback { void triggerOnFrameRateOverridesChanged() override {} void onChoreographerAttached() override {} void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>, Fps) override {} + void onCommitNotComposited(PhysicalDisplayId) override {} }; } // namespace android::scheduler::mock |