blast: transaction callbacks should come in order
Previously, if no SurfaceControls where marked to get callbacks,
a callback was sent directly from SurfaceComposerClient so we could
save a binder call into SurfaceFlinger and a binder call for the
callback.
Although this saved us 2 binder calls, it made the transactions
callbacks come out of order. The public api guarantees that all
callbacks must come in order.
This patch moves the callback from SurfaceComposerClient into
SurfaceFlinger so the callbacks are in order.
Bug: 128519264
Test: SurfaceFlinger_test
Change-Id: Ia1cadb81adb69b58a4d6d43ae453c96a1572f833
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index bc63d31..b74d675 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -69,7 +69,8 @@
const sp<IBinder>& applyToken,
const InputWindowCommands& commands,
int64_t desiredPresentTime,
- const cached_buffer_t& uncacheBuffer) {
+ const cached_buffer_t& uncacheBuffer,
+ const std::vector<ListenerCallbacks>& listenerCallbacks) {
Parcel data, reply;
data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
@@ -89,6 +90,14 @@
data.writeInt64(desiredPresentTime);
data.writeStrongBinder(uncacheBuffer.token);
data.writeUint64(uncacheBuffer.cacheId);
+
+ if (data.writeVectorSize(listenerCallbacks) == NO_ERROR) {
+ for (const auto& [listener, callbackIds] : listenerCallbacks) {
+ data.writeStrongBinder(IInterface::asBinder(listener));
+ data.writeInt64Vector(callbackIds);
+ }
+ }
+
remote()->transact(BnSurfaceComposer::SET_TRANSACTION_STATE, data, &reply);
}
@@ -978,8 +987,18 @@
uncachedBuffer.token = data.readStrongBinder();
uncachedBuffer.cacheId = data.readUint64();
+ std::vector<ListenerCallbacks> listenerCallbacks;
+ int32_t listenersSize = data.readInt32();
+ for (int32_t i = 0; i < listenersSize; i++) {
+ auto listener =
+ interface_cast<ITransactionCompletedListener>(data.readStrongBinder());
+ std::vector<CallbackId> callbackIds;
+ data.readInt64Vector(&callbackIds);
+ listenerCallbacks.emplace_back(listener, callbackIds);
+ }
+
setTransactionState(state, displays, stateFlags, applyToken, inputWindowCommands,
- desiredPresentTime, uncachedBuffer);
+ desiredPresentTime, uncachedBuffer, listenerCallbacks);
return NO_ERROR;
}
case BOOT_FINISHED: {
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index f6ca9e8..3077b21 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -86,14 +86,7 @@
memcpy(output.writeInplace(16 * sizeof(float)),
colorTransform.asArray(), 16 * sizeof(float));
output.writeFloat(cornerRadius);
-
- if (output.writeVectorSize(listenerCallbacks) == NO_ERROR) {
- for (const auto& [listener, callbackIds] : listenerCallbacks) {
- output.writeStrongBinder(IInterface::asBinder(listener));
- output.writeInt64Vector(callbackIds);
- }
- }
-
+ output.writeBool(hasListenerCallbacks);
output.writeStrongBinder(cachedBuffer.token);
output.writeUint64(cachedBuffer.cacheId);
output.writeParcelable(metadata);
@@ -163,15 +156,7 @@
colorTransform = mat4(static_cast<const float*>(input.readInplace(16 * sizeof(float))));
cornerRadius = input.readFloat();
-
- int32_t listenersSize = input.readInt32();
- for (int32_t i = 0; i < listenersSize; i++) {
- auto listener = interface_cast<ITransactionCompletedListener>(input.readStrongBinder());
- std::vector<CallbackId> callbackIds;
- input.readInt64Vector(&callbackIds);
- listenerCallbacks.emplace_back(listener, callbackIds);
- }
-
+ hasListenerCallbacks = input.readBool();
cachedBuffer.token = input.readStrongBinder();
cachedBuffer.cacheId = input.readUint64();
input.readParcelable(&metadata);
@@ -376,9 +361,9 @@
what |= eColorTransformChanged;
colorTransform = other.colorTransform;
}
- if (other.what & eListenerCallbacksChanged) {
- what |= eListenerCallbacksChanged;
- listenerCallbacks = other.listenerCallbacks;
+ if (other.what & eHasListenerCallbacksChanged) {
+ what |= eHasListenerCallbacksChanged;
+ hasListenerCallbacks = other.hasListenerCallbacks;
}
#ifndef NO_INPUT
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 39cd62f..84aed5f 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -381,7 +381,7 @@
s.state.parentHandleForChild = nullptr;
composerStates.add(s);
- sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1, {});
+ sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1, {}, {});
}
void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {
@@ -391,7 +391,7 @@
uncacheBuffer.token = BufferCache::getInstance().getToken();
uncacheBuffer.cacheId = cacheId;
- sf->setTransactionState({}, {}, 0, nullptr, {}, -1, uncacheBuffer);
+ sf->setTransactionState({}, {}, 0, nullptr, {}, -1, uncacheBuffer, {});
}
void SurfaceComposerClient::Transaction::cacheBuffers() {
@@ -434,6 +434,8 @@
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+ std::vector<ListenerCallbacks> listenerCallbacks;
+
// For every listener with registered callbacks
for (const auto& [listener, callbackInfo] : mListenerCallbacks) {
auto& [callbackIds, surfaceControls] = callbackInfo;
@@ -441,11 +443,7 @@
continue;
}
- // If the listener does not have any SurfaceControls set on this Transaction, send the
- // callback now
- if (surfaceControls.empty()) {
- listener->onTransactionCompleted(ListenerStats::createEmpty(listener, callbackIds));
- }
+ listenerCallbacks.emplace_back(listener, std::move(callbackIds));
// If the listener has any SurfaceControls set on this Transaction update the surface state
for (const auto& surfaceControl : surfaceControls) {
@@ -454,8 +452,8 @@
ALOGE("failed to get layer state");
continue;
}
- s->what |= layer_state_t::eListenerCallbacksChanged;
- s->listenerCallbacks.emplace_back(listener, std::move(callbackIds));
+ s->what |= layer_state_t::eHasListenerCallbacksChanged;
+ s->hasListenerCallbacks = true;
}
}
mListenerCallbacks.clear();
@@ -494,7 +492,8 @@
sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance());
sf->setTransactionState(composerStates, displayStates, flags, applyToken, mInputWindowCommands,
mDesiredPresentTime,
- {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/);
+ {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/,
+ listenerCallbacks);
mInputWindowCommands.clear();
mStatus = NO_ERROR;
return NO_ERROR;
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 0ef5b39..fe85fdf 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -20,13 +20,10 @@
#include <stdint.h>
#include <sys/types.h>
-#include <utils/RefBase.h>
-#include <utils/Errors.h>
-#include <utils/Timers.h>
-#include <utils/Vector.h>
-
#include <binder/IInterface.h>
+#include <gui/ITransactionCompletedListener.h>
+
#include <ui/ConfigStoreTypes.h>
#include <ui/DisplayedFrameStats.h>
#include <ui/FrameStats.h>
@@ -34,6 +31,11 @@
#include <ui/GraphicTypes.h>
#include <ui/PixelFormat.h>
+#include <utils/Errors.h>
+#include <utils/RefBase.h>
+#include <utils/Timers.h>
+#include <utils/Vector.h>
+
#include <optional>
#include <vector>
@@ -133,7 +135,8 @@
const sp<IBinder>& applyToken,
const InputWindowCommands& inputWindowCommands,
int64_t desiredPresentTime,
- const cached_buffer_t& uncacheBuffer) = 0;
+ const cached_buffer_t& uncacheBuffer,
+ const std::vector<ListenerCallbacks>& listenerCallbacks) = 0;
/* signal that we're done booting.
* Requires ACCESS_SURFACE_FLINGER permission
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 77bf8f1..2256497 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -23,7 +23,6 @@
#include <utils/Errors.h>
#include <gui/IGraphicBufferProducer.h>
-#include <gui/ITransactionCompletedListener.h>
#include <math/mat4.h>
#ifndef NO_INPUT
@@ -86,7 +85,7 @@
eApiChanged = 0x04000000,
eSidebandStreamChanged = 0x08000000,
eColorTransformChanged = 0x10000000,
- eListenerCallbacksChanged = 0x20000000,
+ eHasListenerCallbacksChanged = 0x20000000,
eInputInfoChanged = 0x40000000,
eCornerRadiusChanged = 0x80000000,
eFrameChanged = 0x1'00000000,
@@ -120,6 +119,7 @@
surfaceDamageRegion(),
api(-1),
colorTransform(mat4()),
+ hasListenerCallbacks(false),
bgColorAlpha(0),
bgColorDataspace(ui::Dataspace::UNKNOWN),
colorSpaceAgnostic(false) {
@@ -182,7 +182,7 @@
sp<NativeHandle> sidebandStream;
mat4 colorTransform;
- std::vector<ListenerCallbacks> listenerCallbacks;
+ bool hasListenerCallbacks;
#ifndef NO_INPUT
InputWindowInfo inputInfo;
#endif
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 94b669d..0ee9bff 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -561,7 +561,9 @@
const sp<IBinder>& /*applyToken*/,
const InputWindowCommands& /*inputWindowCommands*/,
int64_t /*desiredPresentTime*/,
- const cached_buffer_t& /*cachedBuffer*/) override {}
+ const cached_buffer_t& /*cachedBuffer*/,
+ const std::vector<ListenerCallbacks>& /*listenerCallbacks*/) override {
+ }
void bootFinished() override {}
bool authenticateSurfaceTexture(