Support task ID for fps listener rather than SurfaceControl.
Bug: 174956756
Test: e2e test with dashboard cls
Change-Id: I841af53ac820a91d270a75c5cc0ca258df0a3945
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index 05e1935..989abd9 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -755,11 +755,10 @@
return error;
}
- virtual status_t addFpsListener(const sp<IBinder>& layerHandle,
- const sp<gui::IFpsListener>& listener) {
+ virtual status_t addFpsListener(int32_t taskId, const sp<gui::IFpsListener>& listener) {
Parcel data, reply;
SAFE_PARCEL(data.writeInterfaceToken, ISurfaceComposer::getInterfaceDescriptor());
- SAFE_PARCEL(data.writeStrongBinder, layerHandle);
+ SAFE_PARCEL(data.writeInt32, taskId);
SAFE_PARCEL(data.writeStrongBinder, IInterface::asBinder(listener));
const status_t error =
remote()->transact(BnSurfaceComposer::ADD_FPS_LISTENER, data, &reply);
@@ -1669,8 +1668,8 @@
}
case ADD_FPS_LISTENER: {
CHECK_INTERFACE(ISurfaceComposer, data, reply);
- sp<IBinder> layerHandle;
- status_t result = data.readNullableStrongBinder(&layerHandle);
+ int32_t taskId;
+ status_t result = data.readInt32(&taskId);
if (result != NO_ERROR) {
ALOGE("addFpsListener: Failed to read layer handle");
return result;
@@ -1681,7 +1680,7 @@
ALOGE("addFpsListener: Failed to read listener");
return result;
}
- return addFpsListener(layerHandle, listener);
+ return addFpsListener(taskId, listener);
}
case REMOVE_FPS_LISTENER: {
CHECK_INTERFACE(ISurfaceComposer, data, reply);
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index f5cde59..5e8ab92 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1982,9 +1982,9 @@
return ComposerService::getComposerService()->removeRegionSamplingListener(listener);
}
-status_t SurfaceComposerClient::addFpsListener(const sp<IBinder>& layerHandle,
+status_t SurfaceComposerClient::addFpsListener(int32_t taskId,
const sp<gui::IFpsListener>& listener) {
- return ComposerService::getComposerService()->addFpsListener(layerHandle, listener);
+ return ComposerService::getComposerService()->addFpsListener(taskId, listener);
}
status_t SurfaceComposerClient::removeFpsListener(const sp<gui::IFpsListener>& listener) {
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 8b64bcf..88cfe4b 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -351,16 +351,15 @@
/* Registers a listener that streams fps updates from SurfaceFlinger.
*
- * The listener will stream fps updates for the layer tree rooted at layerHandle. Usually, this
- * should be tied to a task. Layers that are not descendants of that task are out of scope for
- * FPS computations.
+ * The listener will stream fps updates for the layer tree rooted at the layer denoted by the
+ * task ID, i.e., the layer must have the task ID as part of its layer metadata with key
+ * METADATA_TASK_ID. If there is no such layer, then no fps is expected to be reported.
*
* Multiple listeners may be supported.
*
- * Requires the ACCESS_SURFACE_FLINGER permission.
+ * Requires the READ_FRAME_BUFFER permission.
*/
- virtual status_t addFpsListener(const sp<IBinder>& layerHandle,
- const sp<gui::IFpsListener>& listener) = 0;
+ virtual status_t addFpsListener(int32_t taskId, const sp<gui::IFpsListener>& listener) = 0;
/*
* Removes a listener that was streaming fps updates from SurfaceFlinger.
*/
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index ab0347c..de88943 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -589,8 +589,7 @@
const sp<IBinder>& stopLayerHandle,
const sp<IRegionSamplingListener>& listener);
static status_t removeRegionSamplingListener(const sp<IRegionSamplingListener>& listener);
- static status_t addFpsListener(const sp<IBinder>& layerHandle,
- const sp<gui::IFpsListener>& listener);
+ static status_t addFpsListener(int32_t taskId, const sp<gui::IFpsListener>& listener);
static status_t removeFpsListener(const sp<gui::IFpsListener>& listener);
private:
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 91b2aff..e8fb71d 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -824,8 +824,7 @@
const sp<IRegionSamplingListener>& /*listener*/) override {
return NO_ERROR;
}
- status_t addFpsListener(const sp<IBinder>& /*layerHandle*/,
- const sp<gui::IFpsListener>& /*listener*/) {
+ status_t addFpsListener(int32_t /*taskId*/, const sp<gui::IFpsListener>& /*listener*/) {
return NO_ERROR;
}
status_t removeFpsListener(const sp<gui::IFpsListener>& /*listener*/) { return NO_ERROR; }
diff --git a/services/surfaceflinger/FpsReporter.cpp b/services/surfaceflinger/FpsReporter.cpp
index c7dbf88..0bc2d3e 100644
--- a/services/surfaceflinger/FpsReporter.cpp
+++ b/services/surfaceflinger/FpsReporter.cpp
@@ -18,14 +18,16 @@
#define LOG_TAG "FpsReporter"
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
-#include "FpsReporter.h"
+#include <algorithm>
+#include "FpsReporter.h"
#include "Layer.h"
+#include "SurfaceFlinger.h"
namespace android {
-FpsReporter::FpsReporter(frametimeline::FrameTimeline& frameTimeline)
- : mFrameTimeline(frameTimeline) {}
+FpsReporter::FpsReporter(frametimeline::FrameTimeline& frameTimeline, SurfaceFlinger& flinger)
+ : mFrameTimeline(frameTimeline), mFlinger(flinger) {}
void FpsReporter::dispatchLayerFps() const {
std::vector<TrackedListener> localListeners;
@@ -41,16 +43,33 @@
});
}
- for (const auto& listener : localListeners) {
- sp<Layer> promotedLayer = listener.layer.promote();
- if (promotedLayer != nullptr) {
- std::unordered_set<int32_t> layerIds;
+ std::unordered_set<int32_t> seenTasks;
+ std::vector<std::pair<TrackedListener, sp<Layer>>> listenersAndLayersToReport;
- promotedLayer->traverse(LayerVector::StateSet::Drawing,
- [&](Layer* layer) { layerIds.insert(layer->getSequence()); });
-
- listener.listener->onFpsReported(mFrameTimeline.computeFps(layerIds));
+ mFlinger.mCurrentState.traverse([&](Layer* layer) {
+ auto& currentState = layer->getCurrentState();
+ if (currentState.metadata.has(METADATA_TASK_ID)) {
+ int32_t taskId = currentState.metadata.getInt32(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>(layer)});
+ break;
+ }
+ }
+ }
}
+ });
+
+ for (const auto& [listener, layer] : listenersAndLayersToReport) {
+ std::unordered_set<int32_t> layerIds;
+
+ layer->traverse(LayerVector::StateSet::Current,
+ [&](Layer* layer) { layerIds.insert(layer->getSequence()); });
+
+ listener.listener->onFpsReported(mFrameTimeline.computeFps(layerIds));
}
}
@@ -59,11 +78,11 @@
mListeners.erase(who);
}
-void FpsReporter::addListener(const sp<gui::IFpsListener>& listener, const wp<Layer>& layer) {
+void FpsReporter::addListener(const sp<gui::IFpsListener>& listener, int32_t taskId) {
sp<IBinder> asBinder = IInterface::asBinder(listener);
asBinder->linkToDeath(this);
std::lock_guard lock(mMutex);
- mListeners.emplace(wp<IBinder>(asBinder), TrackedListener{listener, layer});
+ mListeners.emplace(wp<IBinder>(asBinder), TrackedListener{listener, taskId});
}
void FpsReporter::removeListener(const sp<gui::IFpsListener>& listener) {
diff --git a/services/surfaceflinger/FpsReporter.h b/services/surfaceflinger/FpsReporter.h
index d64b3dc..1cec295 100644
--- a/services/surfaceflinger/FpsReporter.h
+++ b/services/surfaceflinger/FpsReporter.h
@@ -27,10 +27,11 @@
namespace android {
class Layer;
+class SurfaceFlinger;
class FpsReporter : public IBinder::DeathRecipient {
public:
- FpsReporter(frametimeline::FrameTimeline& frameTimeline);
+ FpsReporter(frametimeline::FrameTimeline& frameTimeline, SurfaceFlinger& flinger);
// Dispatches updated layer fps values for the registered listeners
// This method promotes Layer weak pointers and performs layer stack traversals, so mStateLock
@@ -41,7 +42,7 @@
void binderDied(const wp<IBinder>&) override;
// Registers an Fps listener that listens to fps updates for the provided layer
- void addListener(const sp<gui::IFpsListener>& listener, const wp<Layer>& layer);
+ void addListener(const sp<gui::IFpsListener>& listener, int32_t taskId);
// Deregisters an Fps listener
void removeListener(const sp<gui::IFpsListener>& listener);
@@ -55,10 +56,11 @@
struct TrackedListener {
sp<gui::IFpsListener> listener;
- wp<Layer> layer;
+ int32_t taskId;
};
frametimeline::FrameTimeline& mFrameTimeline;
+ SurfaceFlinger& mFlinger;
std::unordered_map<wp<IBinder>, TrackedListener, WpHash> mListeners GUARDED_BY(mMutex);
};
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index ad91183..727386c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1433,14 +1433,12 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::addFpsListener(const sp<IBinder>& layerHandle,
- const sp<gui::IFpsListener>& listener) {
+status_t SurfaceFlinger::addFpsListener(int32_t taskId, const sp<gui::IFpsListener>& listener) {
if (!listener) {
return BAD_VALUE;
}
- const wp<Layer> layer = fromHandle(layerHandle);
- mFpsReporter->addListener(listener, layer);
+ mFpsReporter->addListener(listener, taskId);
return NO_ERROR;
}
@@ -3006,7 +3004,7 @@
mRegionSamplingThread =
new RegionSamplingThread(*this, *mScheduler,
RegionSamplingThread::EnvironmentTimingTunables());
- mFpsReporter = new FpsReporter(*mFrameTimeline);
+ mFpsReporter = new FpsReporter(*mFrameTimeline, *this);
// Dispatch a mode change request for the primary display on scheduler
// initialization, so that the EventThreads always contain a reference to a
// prior configuration.
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 6434ca2..8e69ead 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -363,6 +363,7 @@
friend class BufferQueueLayer;
friend class BufferStateLayer;
friend class Client;
+ friend class FpsReporter;
friend class Layer;
friend class MonitoredProducer;
friend class RefreshRateOverlay;
@@ -627,8 +628,7 @@
status_t addRegionSamplingListener(const Rect& samplingArea, const sp<IBinder>& stopLayerHandle,
const sp<IRegionSamplingListener>& listener) override;
status_t removeRegionSamplingListener(const sp<IRegionSamplingListener>& listener) override;
- status_t addFpsListener(const sp<IBinder>& layerHandle,
- const sp<gui::IFpsListener>& listener) override;
+ status_t addFpsListener(int32_t taskId, const sp<gui::IFpsListener>& listener) override;
status_t removeFpsListener(const sp<gui::IFpsListener>& listener) override;
status_t setDesiredDisplayModeSpecs(const sp<IBinder>& displayToken,
ui::DisplayModeId displayModeId, bool allowGroupSwitching,
diff --git a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
index a9e5df3..a2291b2 100644
--- a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp
@@ -76,7 +76,7 @@
void setupScheduler();
void setupComposer(uint32_t virtualDisplayCount);
- sp<BufferStateLayer> createBufferStateLayer();
+ sp<BufferStateLayer> createBufferStateLayer(LayerMetadata metadata);
TestableSurfaceFlinger mFlinger;
Hwc2::mock::Composer* mComposer = nullptr;
@@ -91,7 +91,7 @@
sp<Layer> mUnrelated;
sp<TestableFpsListener> mFpsListener;
- sp<FpsReporter> mFpsReporter = new FpsReporter(mFrameTimeline);
+ sp<FpsReporter> mFpsReporter = new FpsReporter(mFrameTimeline, *(mFlinger.flinger()));
};
FpsReporterTest::FpsReporterTest() {
@@ -110,10 +110,10 @@
ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name());
}
-sp<BufferStateLayer> FpsReporterTest::createBufferStateLayer() {
+sp<BufferStateLayer> FpsReporterTest::createBufferStateLayer(LayerMetadata metadata = {}) {
sp<Client> client;
LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", WIDTH, HEIGHT,
- LAYER_FLAGS, LayerMetadata());
+ LAYER_FLAGS, metadata);
return new BufferStateLayer(args);
}
@@ -154,7 +154,10 @@
TEST_F(FpsReporterTest, callsListeners) {
mParent = createBufferStateLayer();
- mTarget = createBufferStateLayer();
+ const constexpr int32_t kTaskId = 12;
+ LayerMetadata targetMetadata;
+ targetMetadata.setInt32(METADATA_TASK_ID, kTaskId);
+ mTarget = createBufferStateLayer(targetMetadata);
mChild = createBufferStateLayer();
mGrandChild = createBufferStateLayer();
mUnrelated = createBufferStateLayer();
@@ -162,6 +165,10 @@
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);
float expectedFps = 44.0;
@@ -170,7 +177,7 @@
mGrandChild->getSequence())))
.WillOnce(Return(expectedFps));
- mFpsReporter->addListener(mFpsListener, mTarget);
+ mFpsReporter->addListener(mFpsListener, kTaskId);
mFpsReporter->dispatchLayerFps();
EXPECT_EQ(expectedFps, mFpsListener->lastReportedFps);
mFpsReporter->removeListener(mFpsListener);