Combine Buffer Properties when calling setBuffer
There are several properties, like releaseCallback, acquireFence,
frameNumber, that are tied to the buffer being set. If a new buffer is
set, all those properties should also be overwritten, rather than
through a separate call.
1. Remove Transaction.setAcquireFence and Transaction.setFrameNumber and
added them to the setBuffer as optional
2. Combine all the buffer info into a struct called BufferData that's
sent to SF as one. This will also help with merging or overwriting
buffer data since the callback, frameNumber, and acquireFence should
also be updated.
3. Combine the functions in SF so there's no longer a separate call to
BSL to set fence. Instead, send all buffer related data to BSL.setBuffer
Test: SurfaceFlinger_test
Fixes: 200065015
Change-Id: I53ad12dd105cd4cac6c3a7ecd48279d1b3cd2b8f
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index bbd3cca..aca59b6 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -805,7 +805,7 @@
layer_state_t* s = &(mComposerStates[handle].state);
if (!(s->what & layer_state_t::eBufferChanged)) {
continue;
- } else if (s->what & layer_state_t::eCachedBufferChanged) {
+ } else if (s->bufferData.flags.test(BufferData::BufferDataChange::cachedBufferChanged)) {
// If eBufferChanged and eCachedBufferChanged are both trued then that means
// we already cached the buffer in a previous call to cacheBuffers, perhaps
// from writeToParcel on a Transaction that was merged in to this one.
@@ -814,23 +814,22 @@
// Don't try to cache a null buffer. Sending null buffers is cheap so we shouldn't waste
// time trying to cache them.
- if (!s->buffer) {
+ if (!s->bufferData.buffer) {
continue;
}
uint64_t cacheId = 0;
- status_t ret = BufferCache::getInstance().getCacheId(s->buffer, &cacheId);
+ status_t ret = BufferCache::getInstance().getCacheId(s->bufferData.buffer, &cacheId);
if (ret == NO_ERROR) {
// Cache-hit. Strip the buffer and send only the id.
- s->what &= ~static_cast<uint64_t>(layer_state_t::eBufferChanged);
- s->buffer = nullptr;
+ s->bufferData.buffer = nullptr;
} else {
// Cache-miss. Include the buffer and send the new cacheId.
- cacheId = BufferCache::getInstance().cache(s->buffer);
+ cacheId = BufferCache::getInstance().cache(s->bufferData.buffer);
}
- s->what |= layer_state_t::eCachedBufferChanged;
- s->cachedBuffer.token = BufferCache::getInstance().getToken();
- s->cachedBuffer.id = cacheId;
+ s->bufferData.flags |= BufferData::BufferDataChange::cachedBufferChanged;
+ s->bufferData.cachedBuffer.token = BufferCache::getInstance().getToken();
+ s->bufferData.cachedBuffer.id = cacheId;
// If we have more buffers than the size of the cache, we should stop caching so we don't
// evict other buffers in this transaction
@@ -1289,22 +1288,33 @@
}
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer(
- const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer, const ReleaseCallbackId& id,
- ReleaseBufferCallback callback) {
+ const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
+ const std::optional<sp<Fence>>& fence, const std::optional<uint64_t>& frameNumber,
+ const ReleaseCallbackId& id, ReleaseBufferCallback callback) {
layer_state_t* s = getLayerState(sc);
if (!s) {
mStatus = BAD_INDEX;
return *this;
}
removeReleaseBufferCallback(s);
- s->what |= layer_state_t::eBufferChanged;
- s->buffer = buffer;
- s->releaseBufferEndpoint = IInterface::asBinder(TransactionCompletedListener::getIInstance());
+ BufferData bufferData;
+ bufferData.buffer = buffer;
+ if (frameNumber) {
+ bufferData.frameNumber = *frameNumber;
+ bufferData.flags |= BufferData::BufferDataChange::frameNumberChanged;
+ }
+ if (fence) {
+ bufferData.acquireFence = *fence;
+ bufferData.flags |= BufferData::BufferDataChange::fenceChanged;
+ }
+ bufferData.releaseBufferEndpoint =
+ IInterface::asBinder(TransactionCompletedListener::getIInstance());
if (mIsAutoTimestamp) {
mDesiredPresentTime = systemTime();
}
- setReleaseBufferCallback(s, id, callback);
-
+ setReleaseBufferCallback(&bufferData, id, callback);
+ s->what |= layer_state_t::eBufferChanged;
+ s->bufferData = bufferData;
registerSurfaceControlForCallback(sc);
mContainsBuffer = true;
@@ -1312,51 +1322,33 @@
}
void SurfaceComposerClient::Transaction::removeReleaseBufferCallback(layer_state_t* s) {
- if (!s->releaseBufferListener) {
+ if (!(s->what & layer_state_t::eBufferChanged)) {
return;
}
- s->what &= ~static_cast<uint64_t>(layer_state_t::eReleaseBufferListenerChanged);
- s->releaseBufferListener = nullptr;
auto listener = TransactionCompletedListener::getInstance();
- listener->removeReleaseBufferCallback(s->releaseCallbackId);
- s->releaseCallbackId = ReleaseCallbackId::INVALID_ID;
+ listener->removeReleaseBufferCallback(s->bufferData.releaseCallbackId);
}
-void SurfaceComposerClient::Transaction::setReleaseBufferCallback(layer_state_t* s,
+void SurfaceComposerClient::Transaction::setReleaseBufferCallback(BufferData* bufferData,
const ReleaseCallbackId& id,
ReleaseBufferCallback callback) {
if (!callback) {
return;
}
- if (!s->buffer) {
+ if (!bufferData->buffer) {
ALOGW("Transaction::setReleaseBufferCallback"
"ignored trying to set a callback on a null buffer.");
return;
}
- s->what |= layer_state_t::eReleaseBufferListenerChanged;
- s->releaseBufferListener = TransactionCompletedListener::getIInstance();
- s->releaseCallbackId = id;
+ bufferData->releaseBufferListener = TransactionCompletedListener::getIInstance();
+ bufferData->releaseCallbackId = id;
auto listener = TransactionCompletedListener::getInstance();
listener->setReleaseBufferCallback(id, callback);
}
-SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setAcquireFence(
- const sp<SurfaceControl>& sc, const sp<Fence>& fence) {
- layer_state_t* s = getLayerState(sc);
- if (!s) {
- mStatus = BAD_INDEX;
- return *this;
- }
- s->what |= layer_state_t::eAcquireFenceChanged;
- s->acquireFence = fence;
-
- registerSurfaceControlForCallback(sc);
- return *this;
-}
-
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setDataspace(
const sp<SurfaceControl>& sc, ui::Dataspace dataspace) {
layer_state_t* s = getLayerState(sc);
@@ -1506,20 +1498,6 @@
return *this;
}
-SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setFrameNumber(
- const sp<SurfaceControl>& sc, uint64_t frameNumber) {
- layer_state_t* s = getLayerState(sc);
- if (!s) {
- mStatus = BAD_INDEX;
- return *this;
- }
-
- s->what |= layer_state_t::eFrameNumberChanged;
- s->frameNumber = frameNumber;
-
- return *this;
-}
-
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setInputWindowInfo(
const sp<SurfaceControl>& sc, const WindowInfo& info) {
layer_state_t* s = getLayerState(sc);