[SF] Use LayerHierarchy traverse() for FpsReporter
The legacy layer state traverse function which FpsReporter uses
is not working currently as it will not properly get to the root
layer where the taskId resides and thus not getting a matching
taskId with the callbacks from game dashboard.
This patch updates FpsReporter to use LaterHierarchy traverse
function which works as expected to return the correct taskId
in the root layer.
Bug: 316111174
Test: libsurfaceflinger_unittest
Test: libsurfaceflinger_unittest:FpsReporterTest
Change-Id: Icec40488289a563967b282a078515b52322a55db
diff --git a/services/surfaceflinger/FpsReporter.cpp b/services/surfaceflinger/FpsReporter.cpp
index 155cf4d..a47348f 100644
--- a/services/surfaceflinger/FpsReporter.cpp
+++ b/services/surfaceflinger/FpsReporter.cpp
@@ -26,13 +26,12 @@
namespace android {
-FpsReporter::FpsReporter(frametimeline::FrameTimeline& frameTimeline, SurfaceFlinger& flinger,
- std::unique_ptr<Clock> clock)
- : mFrameTimeline(frameTimeline), mFlinger(flinger), mClock(std::move(clock)) {
+FpsReporter::FpsReporter(frametimeline::FrameTimeline& frameTimeline, std::unique_ptr<Clock> clock)
+ : mFrameTimeline(frameTimeline), mClock(std::move(clock)) {
LOG_ALWAYS_FATAL_IF(mClock == nullptr, "Passed in null clock when constructing FpsReporter!");
}
-void FpsReporter::dispatchLayerFps() {
+void FpsReporter::dispatchLayerFps(const frontend::LayerHierarchy& layerHierarchy) {
const auto now = mClock->now();
if (now - mLastDispatch < kMinDispatchDuration) {
return;
@@ -52,31 +51,42 @@
}
std::unordered_set<int32_t> seenTasks;
- std::vector<std::pair<TrackedListener, sp<Layer>>> listenersAndLayersToReport;
+ std::vector<std::pair<TrackedListener, const frontend::LayerHierarchy*>>
+ listenersAndLayersToReport;
- mFlinger.mCurrentState.traverse([&](Layer* layer) {
- auto& currentState = layer->getDrawingState();
- if (currentState.metadata.has(gui::METADATA_TASK_ID)) {
- int32_t taskId = currentState.metadata.getInt32(gui::METADATA_TASK_ID, 0);
+ layerHierarchy.traverse([&](const frontend::LayerHierarchy& hierarchy,
+ const frontend::LayerHierarchy::TraversalPath& traversalPath) {
+ if (traversalPath.variant == frontend::LayerHierarchy::Variant::Detached) {
+ return false;
+ }
+ const auto& metadata = hierarchy.getLayer()->metadata;
+ if (metadata.has(gui::METADATA_TASK_ID)) {
+ int32_t taskId = metadata.getInt32(gui::METADATA_TASK_ID, 0);
if (seenTasks.count(taskId) == 0) {
// localListeners is expected to be tiny
for (TrackedListener& listener : localListeners) {
if (listener.taskId == taskId) {
seenTasks.insert(taskId);
- listenersAndLayersToReport.push_back(
- {listener, sp<Layer>::fromExisting(layer)});
+ listenersAndLayersToReport.push_back({listener, &hierarchy});
break;
}
}
}
}
+ return true;
});
- for (const auto& [listener, layer] : listenersAndLayersToReport) {
+ for (const auto& [listener, hierarchy] : listenersAndLayersToReport) {
std::unordered_set<int32_t> layerIds;
- layer->traverse(LayerVector::StateSet::Current,
- [&](Layer* layer) { layerIds.insert(layer->getSequence()); });
+ hierarchy->traverse([&](const frontend::LayerHierarchy& hierarchy,
+ const frontend::LayerHierarchy::TraversalPath& traversalPath) {
+ if (traversalPath.variant == frontend::LayerHierarchy::Variant::Detached) {
+ return false;
+ }
+ layerIds.insert(static_cast<int32_t>(hierarchy.getLayer()->id));
+ return true;
+ });
listener.listener->onFpsReported(mFrameTimeline.computeFps(layerIds));
}
diff --git a/services/surfaceflinger/FpsReporter.h b/services/surfaceflinger/FpsReporter.h
index 438b1aa..01f1e07 100644
--- a/services/surfaceflinger/FpsReporter.h
+++ b/services/surfaceflinger/FpsReporter.h
@@ -24,6 +24,7 @@
#include "Clock.h"
#include "FrameTimeline/FrameTimeline.h"
+#include "FrontEnd/LayerHierarchy.h"
#include "WpHash.h"
namespace android {
@@ -33,13 +34,13 @@
class FpsReporter : public IBinder::DeathRecipient {
public:
- FpsReporter(frametimeline::FrameTimeline& frameTimeline, SurfaceFlinger& flinger,
+ FpsReporter(frametimeline::FrameTimeline& frameTimeline,
std::unique_ptr<Clock> clock = std::make_unique<SteadyClock>());
// Dispatches updated layer fps values for the registered listeners
// This method promotes Layer weak pointers and performs layer stack traversals, so mStateLock
// must be held when calling this method.
- void dispatchLayerFps() EXCLUDES(mMutex);
+ void dispatchLayerFps(const frontend::LayerHierarchy&) EXCLUDES(mMutex);
// Override for IBinder::DeathRecipient
void binderDied(const wp<IBinder>&) override;
@@ -58,7 +59,6 @@
};
frametimeline::FrameTimeline& mFrameTimeline;
- SurfaceFlinger& mFlinger;
static const constexpr std::chrono::steady_clock::duration kMinDispatchDuration =
std::chrono::milliseconds(500);
std::unique_ptr<Clock> mClock;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index c3bfb58..be0dd55 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3096,7 +3096,7 @@
{
Mutex::Autolock lock(mStateLock);
if (mFpsReporter) {
- mFpsReporter->dispatchLayerFps();
+ mFpsReporter->dispatchLayerFps(mLayerHierarchyBuilder.getHierarchy());
}
if (mTunnelModeEnabledReporter) {
@@ -4240,7 +4240,7 @@
mRegionSamplingThread =
sp<RegionSamplingThread>::make(*this,
RegionSamplingThread::EnvironmentTimingTunables());
- mFpsReporter = sp<FpsReporter>::make(*mFrameTimeline, *this);
+ mFpsReporter = sp<FpsReporter>::make(*mFrameTimeline);
}
void SurfaceFlinger::doCommitTransactions() {
diff --git a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
index f695b09..9e8e306 100644
--- a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
@@ -24,7 +24,11 @@
#include <gtest/gtest.h>
#include <gui/LayerMetadata.h>
+#include "Client.h" // temporarily needed for LayerCreationArgs
#include "FpsReporter.h"
+#include "FrontEnd/LayerCreationArgs.h"
+#include "FrontEnd/LayerHierarchy.h"
+#include "FrontEnd/LayerLifecycleManager.h"
#include "Layer.h"
#include "TestableSurfaceFlinger.h"
#include "fake/FakeClock.h"
@@ -76,7 +80,15 @@
sp<Layer> createBufferStateLayer(LayerMetadata metadata);
- TestableSurfaceFlinger mFlinger;
+ LayerCreationArgs createArgs(uint32_t id, bool canBeRoot, uint32_t parentId,
+ LayerMetadata metadata);
+
+ void createRootLayer(uint32_t id, LayerMetadata metadata);
+
+ void createLayer(uint32_t id, uint32_t parentId, LayerMetadata metadata);
+
+ frontend::LayerLifecycleManager mLifecycleManager;
+
mock::FrameTimeline mFrameTimeline =
mock::FrameTimeline(std::make_shared<impl::TimeStats>(), 0);
@@ -89,8 +101,8 @@
sp<TestableFpsListener> mFpsListener;
fake::FakeClock* mClock = new fake::FakeClock();
- sp<FpsReporter> mFpsReporter = sp<FpsReporter>::make(mFrameTimeline, *(mFlinger.flinger()),
- std::unique_ptr<Clock>(mClock));
+ sp<FpsReporter> mFpsReporter =
+ sp<FpsReporter>::make(mFrameTimeline, std::unique_ptr<Clock>(mClock));
};
FpsReporterTest::FpsReporterTest() {
@@ -98,9 +110,6 @@
::testing::UnitTest::GetInstance()->current_test_info();
ALOGD("**** Setting up for %s.%s\n", test_info->test_case_name(), test_info->name());
- mFlinger.setupMockScheduler();
- mFlinger.setupComposer(std::make_unique<Hwc2::mock::Composer>());
-
mFpsListener = sp<TestableFpsListener>::make();
}
@@ -110,76 +119,94 @@
ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name());
}
-sp<Layer> FpsReporterTest::createBufferStateLayer(LayerMetadata metadata = {}) {
+LayerCreationArgs FpsReporterTest::createArgs(uint32_t id, bool canBeRoot, uint32_t parentId,
+ LayerMetadata metadata) {
sp<Client> client;
- LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", LAYER_FLAGS, metadata);
- return sp<Layer>::make(args);
+ LayerCreationArgs args(std::make_optional(id));
+ args.name = "testlayer";
+ args.addToRoot = canBeRoot;
+ args.flags = LAYER_FLAGS;
+ args.metadata = metadata;
+ args.parentId = parentId;
+ return args;
+}
+
+void FpsReporterTest::createRootLayer(uint32_t id, LayerMetadata metadata = LayerMetadata()) {
+ std::vector<std::unique_ptr<frontend::RequestedLayerState>> layers;
+ layers.emplace_back(std::make_unique<frontend::RequestedLayerState>(
+ createArgs(/*id=*/id, /*canBeRoot=*/true, /*parent=*/UNASSIGNED_LAYER_ID,
+ /*metadata=*/metadata)));
+ mLifecycleManager.addLayers(std::move(layers));
+}
+
+void FpsReporterTest::createLayer(uint32_t id, uint32_t parentId,
+ LayerMetadata metadata = LayerMetadata()) {
+ std::vector<std::unique_ptr<frontend::RequestedLayerState>> layers;
+ layers.emplace_back(std::make_unique<frontend::RequestedLayerState>(
+ createArgs(/*id=*/id, /*canBeRoot=*/false, /*parent=*/parentId,
+ /*mirror=*/metadata)));
+ mLifecycleManager.addLayers(std::move(layers));
}
namespace {
TEST_F(FpsReporterTest, callsListeners) {
- mParent = createBufferStateLayer();
constexpr int32_t kTaskId = 12;
LayerMetadata targetMetadata;
targetMetadata.setInt32(gui::METADATA_TASK_ID, kTaskId);
- mTarget = createBufferStateLayer(targetMetadata);
- mChild = createBufferStateLayer();
- mGrandChild = createBufferStateLayer();
- mUnrelated = createBufferStateLayer();
- mParent->addChild(mTarget);
- mTarget->addChild(mChild);
- mChild->addChild(mGrandChild);
- mParent->commitChildList();
- mFlinger.mutableCurrentState().layersSortedByZ.add(mParent);
- mFlinger.mutableCurrentState().layersSortedByZ.add(mTarget);
- mFlinger.mutableCurrentState().layersSortedByZ.add(mChild);
- mFlinger.mutableCurrentState().layersSortedByZ.add(mGrandChild);
+
+ createRootLayer(1, targetMetadata);
+ createLayer(11, 1);
+ createLayer(111, 11);
+
+ frontend::LayerHierarchyBuilder hierarchyBuilder;
+ hierarchyBuilder.update(mLifecycleManager);
float expectedFps = 44.0;
- EXPECT_CALL(mFrameTimeline,
- computeFps(UnorderedElementsAre(mTarget->getSequence(), mChild->getSequence(),
- mGrandChild->getSequence())))
+ EXPECT_CALL(mFrameTimeline, computeFps(UnorderedElementsAre(1, 11, 111)))
.WillOnce(Return(expectedFps));
mFpsReporter->addListener(mFpsListener, kTaskId);
mClock->advanceTime(600ms);
- mFpsReporter->dispatchLayerFps();
+ mFpsReporter->dispatchLayerFps(hierarchyBuilder.getHierarchy());
EXPECT_EQ(expectedFps, mFpsListener->lastReportedFps);
mFpsReporter->removeListener(mFpsListener);
Mock::VerifyAndClearExpectations(&mFrameTimeline);
EXPECT_CALL(mFrameTimeline, computeFps(_)).Times(0);
- mFpsReporter->dispatchLayerFps();
+ mFpsReporter->dispatchLayerFps(hierarchyBuilder.getHierarchy());
}
TEST_F(FpsReporterTest, rateLimits) {
const constexpr int32_t kTaskId = 12;
LayerMetadata targetMetadata;
targetMetadata.setInt32(gui::METADATA_TASK_ID, kTaskId);
- mTarget = createBufferStateLayer(targetMetadata);
- mFlinger.mutableCurrentState().layersSortedByZ.add(mTarget);
+ createRootLayer(1);
+ createLayer(11, 1, targetMetadata);
+
+ frontend::LayerHierarchyBuilder hierarchyBuilder;
+ hierarchyBuilder.update(mLifecycleManager);
float firstFps = 44.0;
float secondFps = 53.0;
- EXPECT_CALL(mFrameTimeline, computeFps(UnorderedElementsAre(mTarget->getSequence())))
+ EXPECT_CALL(mFrameTimeline, computeFps(UnorderedElementsAre(11)))
.WillOnce(Return(firstFps))
.WillOnce(Return(secondFps));
mFpsReporter->addListener(mFpsListener, kTaskId);
mClock->advanceTime(600ms);
- mFpsReporter->dispatchLayerFps();
+ mFpsReporter->dispatchLayerFps(hierarchyBuilder.getHierarchy());
EXPECT_EQ(firstFps, mFpsListener->lastReportedFps);
mClock->advanceTime(200ms);
- mFpsReporter->dispatchLayerFps();
+ mFpsReporter->dispatchLayerFps(hierarchyBuilder.getHierarchy());
EXPECT_EQ(firstFps, mFpsListener->lastReportedFps);
mClock->advanceTime(200ms);
- mFpsReporter->dispatchLayerFps();
+ mFpsReporter->dispatchLayerFps(hierarchyBuilder.getHierarchy());
EXPECT_EQ(firstFps, mFpsListener->lastReportedFps);
mClock->advanceTime(200ms);
- mFpsReporter->dispatchLayerFps();
+ mFpsReporter->dispatchLayerFps(hierarchyBuilder.getHierarchy());
EXPECT_EQ(secondFps, mFpsListener->lastReportedFps);
}