summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--services/surfaceflinger/BufferStateLayer.cpp15
-rw-r--r--services/surfaceflinger/BufferStateLayer.h3
-rw-r--r--services/surfaceflinger/Layer.cpp11
-rw-r--r--services/surfaceflinger/Layer.h6
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp21
-rw-r--r--services/surfaceflinger/TransactionCallbackInvoker.cpp8
-rw-r--r--services/surfaceflinger/TransactionCallbackInvoker.h12
-rw-r--r--services/surfaceflinger/tests/LayerCallback_test.cpp45
8 files changed, 89 insertions, 32 deletions
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index f7f96ab1b1..b6bea97481 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -549,13 +549,19 @@ bool BufferStateLayer::setSidebandStream(const sp<NativeHandle>& sidebandStream)
}
bool BufferStateLayer::setTransactionCompletedListeners(
- const std::vector<sp<CallbackHandle>>& handles) {
+ const std::vector<ListenerCallbacks>& listenerCallbacks, const sp<IBinder>& layerHandle) {
// If there is no handle, we will not send a callback so reset mReleasePreviousBuffer and return
- if (handles.empty()) {
+ if (listenerCallbacks.empty()) {
mReleasePreviousBuffer = false;
return false;
}
+ std::vector<sp<CallbackHandle>> handles;
+ handles.reserve(listenerCallbacks.size());
+ for (auto& [listener, callbackIds] : listenerCallbacks) {
+ handles.emplace_back(new CallbackHandle(listener, callbackIds, layerHandle));
+ }
+
const bool willPresent = willPresentCurrentTransaction();
for (const auto& handle : handles) {
@@ -571,9 +577,10 @@ bool BufferStateLayer::setTransactionCompletedListeners(
// Store so latched time and release fence can be set
mDrawingState.callbackHandles.push_back(handle);
- } else { // If this layer will NOT need to be relatched and presented this frame
+ } else {
+ // If this layer will NOT need to be relatched and presented this frame
// Notify the transaction completed thread this handle is done
- mFlinger->getTransactionCallbackInvoker().registerUnpresentedCallbackHandle(handle);
+ mFlinger->getTransactionCallbackInvoker().addUnpresentedCallbackHandle(handle);
}
}
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index eea700cf7b..ceed188f0e 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -65,7 +65,8 @@ public:
bool setSurfaceDamageRegion(const Region& surfaceDamage) override;
bool setApi(int32_t api) override;
bool setSidebandStream(const sp<NativeHandle>& sidebandStream) override;
- bool setTransactionCompletedListeners(const std::vector<sp<CallbackHandle>>& handles) override;
+ bool setTransactionCompletedListeners(const std::vector<ListenerCallbacks>& handles,
+ const sp<IBinder>& layerHandle) override;
bool addFrameEvent(const sp<Fence>& acquireFence, nsecs_t postedTime,
nsecs_t requestedPresentTime) override;
bool setPosition(float /*x*/, float /*y*/) override;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index a27c487822..d85e843d83 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2640,6 +2640,17 @@ bool Layer::setDropInputMode(gui::DropInputMode mode) {
return true;
}
+bool Layer::setTransactionCompletedListeners(
+ const std::vector<ListenerCallbacks>& listenerCallbacks, const sp<IBinder>&) {
+ if (listenerCallbacks.empty()) {
+ return false;
+ }
+ for (auto& listener : listenerCallbacks) {
+ mFlinger->getTransactionCallbackInvoker().addEmptyCallback(listener);
+ }
+ return false;
+}
+
// ---------------------------------------------------------------------------
std::ostream& operator<<(std::ostream& stream, const Layer::FrameRate& rate) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index bf338c18ad..d36b816481 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -426,10 +426,8 @@ public:
virtual bool setSurfaceDamageRegion(const Region& /*surfaceDamage*/) { return false; };
virtual bool setApi(int32_t /*api*/) { return false; };
virtual bool setSidebandStream(const sp<NativeHandle>& /*sidebandStream*/) { return false; };
- virtual bool setTransactionCompletedListeners(
- const std::vector<sp<CallbackHandle>>& /*handles*/) {
- return false;
- };
+ virtual bool setTransactionCompletedListeners(const std::vector<ListenerCallbacks>& /*handles*/,
+ const sp<IBinder>& /* layerHandle */);
virtual bool addFrameEvent(const sp<Fence>& /*acquireFence*/, nsecs_t /*postedTime*/,
nsecs_t /*requestedPresentTime*/) {
return false;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 6ed9e5fb3e..c2dcd70166 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3803,11 +3803,10 @@ bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin
transactionFlags |= setDisplayStateLocked(display);
}
- // start and end registration for listeners w/ no surface so they can get their callback. Note
- // that listeners with SurfaceControls will start registration during setClientStateLocked
- // below.
+ // Add listeners w/ surfaces so they can get their callback. Note that listeners with
+ // SurfaceControls will start registration during setClientStateLocked below.
for (const auto& listener : listenerCallbacks) {
- mTransactionCallbackInvoker.addEmptyTransaction(listener);
+ mTransactionCallbackInvoker.addEmptyCallback(listener);
}
uint32_t clientStateFlags = 0;
@@ -3979,7 +3978,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime
}
if (layer == nullptr) {
for (auto& [listener, callbackIds] : s.listeners) {
- mTransactionCallbackInvoker.registerUnpresentedCallbackHandle(
+ mTransactionCallbackInvoker.addUnpresentedCallbackHandle(
new CallbackHandle(listener, callbackIds, s.surface));
}
return 0;
@@ -4250,12 +4249,6 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime
flags |= eTransactionNeeded | eTraversalNeeded;
}
}
- std::vector<sp<CallbackHandle>> callbackHandles;
- if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!filteredListeners.empty())) {
- for (auto& [listener, callbackIds] : filteredListeners) {
- callbackHandles.emplace_back(new CallbackHandle(listener, callbackIds, s.surface));
- }
- }
if (what & layer_state_t::eBufferChanged &&
layer->setBuffer(s.bufferData, postTime, desiredPresentTime, isAutoTimestamp,
@@ -4265,7 +4258,11 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime
layer->setFrameTimelineVsyncForBufferlessTransaction(frameTimelineInfo, postTime);
}
- if (layer->setTransactionCompletedListeners(callbackHandles)) flags |= eTraversalNeeded;
+ if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!filteredListeners.empty())) {
+ if (layer->setTransactionCompletedListeners(filteredListeners, s.surface)) {
+ flags |= eTraversalNeeded;
+ }
+ }
// Do not put anything that updates layer state or modifies flags after
// setTransactionCompletedListener
return flags;
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index f3d46ea061..418fbc5c44 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -74,10 +74,10 @@ TransactionCallbackInvoker::~TransactionCallbackInvoker() {
}
}
-void TransactionCallbackInvoker::addEmptyTransaction(const ListenerCallbacks& listenerCallbacks) {
+void TransactionCallbackInvoker::addEmptyCallback(const ListenerCallbacks& listenerCallbacks) {
auto& [listener, callbackIds] = listenerCallbacks;
- auto& transactionStatsDeque = mCompletedTransactions[listener];
- transactionStatsDeque.emplace_back(callbackIds);
+ TransactionStats* transactionStats;
+ findOrCreateTransactionStats(listener, callbackIds, &transactionStats);
}
status_t TransactionCallbackInvoker::addOnCommitCallbackHandles(
@@ -116,7 +116,7 @@ status_t TransactionCallbackInvoker::addCallbackHandles(
return NO_ERROR;
}
-status_t TransactionCallbackInvoker::registerUnpresentedCallbackHandle(
+status_t TransactionCallbackInvoker::addUnpresentedCallbackHandle(
const sp<CallbackHandle>& handle) {
return addCallbackHandle(handle, std::vector<JankData>());
}
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h
index e203d41bd9..6f67947081 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -71,8 +71,10 @@ public:
// Adds the Transaction CallbackHandle from a layer that does not need to be relatched and
// presented this frame.
- status_t registerUnpresentedCallbackHandle(const sp<CallbackHandle>& handle);
- void addEmptyTransaction(const ListenerCallbacks& listenerCallbacks);
+ status_t addUnpresentedCallbackHandle(const sp<CallbackHandle>& handle);
+ // Adds the callback handles for empty transactions or for non-buffer layer updates which do not
+ // include layer stats.
+ void addEmptyCallback(const ListenerCallbacks& listenerCallbacks);
void addPresentFence(const sp<Fence>& presentFence);
@@ -81,14 +83,12 @@ public:
mCompletedTransactions.clear();
}
- status_t addCallbackHandle(const sp<CallbackHandle>& handle,
- const std::vector<JankData>& jankData);
-
-
private:
status_t findOrCreateTransactionStats(const sp<IBinder>& listener,
const std::vector<CallbackId>& callbackIds,
TransactionStats** outTransactionStats);
+ status_t addCallbackHandle(const sp<CallbackHandle>& handle,
+ const std::vector<JankData>& jankData);
std::unordered_map<sp<IBinder>, std::deque<TransactionStats>, IListenerHash>
mCompletedTransactions;
diff --git a/services/surfaceflinger/tests/LayerCallback_test.cpp b/services/surfaceflinger/tests/LayerCallback_test.cpp
index 91a5b528f8..7beba15062 100644
--- a/services/surfaceflinger/tests/LayerCallback_test.cpp
+++ b/services/surfaceflinger/tests/LayerCallback_test.cpp
@@ -1067,7 +1067,7 @@ TEST_F(LayerCallbackTest, EmptyBufferStateChanges) {
}
// b202394221
-TEST_F(LayerCallbackTest, DISABLED_NonBufferLayerStateChanges) {
+TEST_F(LayerCallbackTest, NonBufferLayerStateChanges) {
sp<SurfaceControl> layer;
ASSERT_NO_FATAL_FAILURE(layer = createColorLayer("ColorLayer", Color::RED));
@@ -1085,4 +1085,47 @@ TEST_F(LayerCallbackTest, DISABLED_NonBufferLayerStateChanges) {
EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
}
+class TimedCallbackHelper {
+public:
+ static void function(void* callbackContext, nsecs_t, const sp<Fence>&,
+ const std::vector<SurfaceControlStats>&) {
+ if (!callbackContext) {
+ ALOGE("failed to get callback context");
+ }
+ TimedCallbackHelper* helper = static_cast<TimedCallbackHelper*>(callbackContext);
+ std::lock_guard lock(helper->mMutex);
+ helper->mInvokedTime = systemTime();
+ helper->mCv.notify_all();
+ }
+
+ void waitForCallback() {
+ std::unique_lock lock(mMutex);
+ ASSERT_TRUE(mCv.wait_for(lock, std::chrono::seconds(3), [&] { return mInvokedTime != -1; }))
+ << "did not receive callback";
+ }
+ void* getContext() { return static_cast<void*>(this); }
+
+ std::mutex mMutex;
+ std::condition_variable mCv;
+ nsecs_t mInvokedTime = -1;
+};
+
+TEST_F(LayerCallbackTest, EmptyTransactionCallbackOrder) {
+ TimedCallbackHelper onCommitCallback;
+ TimedCallbackHelper onCompleteCallback;
+
+ // Add transaction callback before on commit callback
+ Transaction()
+ .addTransactionCompletedCallback(onCompleteCallback.function,
+ onCompleteCallback.getContext())
+ .addTransactionCommittedCallback(onCommitCallback.function,
+ onCommitCallback.getContext())
+ .apply();
+
+ EXPECT_NO_FATAL_FAILURE(onCompleteCallback.waitForCallback());
+ EXPECT_NO_FATAL_FAILURE(onCommitCallback.waitForCallback());
+ // verify we get the oncomplete at the same time or after the oncommit callback.
+ EXPECT_GE(onCompleteCallback.mInvokedTime, onCommitCallback.mInvokedTime);
+}
+
} // namespace android
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-24 23:42:08 +0000