Merge "Add producerId so we know when the BBQ producer has been changed."
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 66c0041..9d82c14 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -20,6 +20,7 @@
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
//#define LOG_NDEBUG 0
+#include <cutils/atomic.h>
#include <gui/BLASTBufferQueue.h>
#include <gui/BufferItemConsumer.h>
#include <gui/BufferQueueConsumer.h>
@@ -157,11 +158,11 @@
GraphicBuffer::USAGE_HW_COMPOSER |
GraphicBuffer::USAGE_HW_TEXTURE,
1, false, this);
- static int32_t id = 0;
- mName = name + "#" + std::to_string(id);
- auto consumerName = mName + "(BLAST Consumer)" + std::to_string(id);
- mQueuedBufferTrace = "QueuedBuffer - " + mName + "BLAST#" + std::to_string(id);
- id++;
+ static std::atomic<uint32_t> nextId = 0;
+ mProducerId = nextId++;
+ mName = name + "#" + std::to_string(mProducerId);
+ auto consumerName = mName + "(BLAST Consumer)" + std::to_string(mProducerId);
+ mQueuedBufferTrace = "QueuedBuffer - " + mName + "BLAST#" + std::to_string(mProducerId);
mBufferItemConsumer->setName(String8(consumerName.c_str()));
mBufferItemConsumer->setFrameAvailableListener(this);
@@ -572,7 +573,8 @@
std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */,
std::placeholders::_1, std::placeholders::_2, std::placeholders::_3);
sp<Fence> fence = bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE;
- t->setBuffer(mSurfaceControl, buffer, fence, bufferItem.mFrameNumber, releaseBufferCallback);
+ t->setBuffer(mSurfaceControl, buffer, fence, bufferItem.mFrameNumber, mProducerId,
+ releaseBufferCallback);
t->setDataspace(mSurfaceControl, static_cast<ui::Dataspace>(bufferItem.mDataSpace));
t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata);
t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage);
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 7772a65..a6276e5 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -984,6 +984,7 @@
SAFE_PARCEL(output->writeUint64, cachedBuffer.id);
SAFE_PARCEL(output->writeBool, hasBarrier);
SAFE_PARCEL(output->writeUint64, barrierFrameNumber);
+ SAFE_PARCEL(output->writeUint32, producerId);
return NO_ERROR;
}
@@ -1022,6 +1023,7 @@
SAFE_PARCEL(input->readBool, &hasBarrier);
SAFE_PARCEL(input->readUint64, &barrierFrameNumber);
+ SAFE_PARCEL(input->readUint32, &producerId);
return NO_ERROR;
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index a345938..5088604 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1632,7 +1632,7 @@
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer(
const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
const std::optional<sp<Fence>>& fence, const std::optional<uint64_t>& optFrameNumber,
- ReleaseBufferCallback callback) {
+ uint32_t producerId, ReleaseBufferCallback callback) {
layer_state_t* s = getLayerState(sc);
if (!s) {
mStatus = BAD_INDEX;
@@ -1651,6 +1651,7 @@
bufferData->buffer = buffer;
uint64_t frameNumber = sc->resolveFrameNumber(optFrameNumber);
bufferData->frameNumber = frameNumber;
+ bufferData->producerId = producerId;
bufferData->flags |= BufferData::BufferDataChange::frameNumberChanged;
if (fence) {
bufferData->acquireFence = *fence;
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 8d07162..b9e0647 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -162,6 +162,11 @@
int32_t mNumFrameAvailable GUARDED_BY(mMutex) = 0;
int32_t mNumAcquired GUARDED_BY(mMutex) = 0;
+ // A value used to identify if a producer has been changed for the same SurfaceControl.
+ // This is needed to know when the frame number has been reset to make sure we don't
+ // latch stale buffers and that we don't wait on barriers from an old producer.
+ uint32_t mProducerId = 0;
+
// Keep a reference to the submitted buffers so we can release when surfaceflinger drops the
// buffer or the buffer has been presented and a new buffer is ready to be presented.
std::unordered_map<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mSubmitted
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 03a2582..ddaf473 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -111,6 +111,7 @@
uint64_t frameNumber = 0;
bool hasBarrier = false;
uint64_t barrierFrameNumber = 0;
+ uint32_t producerId = 0;
// Listens to when the buffer is safe to be released. This is used for blast
// layers only. The callback includes a release fence as well as the graphic
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 809ea5a..ffd5af1 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -536,7 +536,7 @@
Transaction& setBuffer(const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
const std::optional<sp<Fence>>& fence = std::nullopt,
const std::optional<uint64_t>& frameNumber = std::nullopt,
- ReleaseBufferCallback callback = nullptr);
+ uint32_t producerId = 0, ReleaseBufferCallback callback = nullptr);
std::shared_ptr<BufferData> getAndClearBuffer(const sp<SurfaceControl>& sc);
/**
diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.h b/services/surfaceflinger/FrontEnd/TransactionHandler.h
index a06b870..7fc825e 100644
--- a/services/surfaceflinger/FrontEnd/TransactionHandler.h
+++ b/services/surfaceflinger/FrontEnd/TransactionHandler.h
@@ -34,7 +34,7 @@
class TransactionHandler {
public:
struct TransactionFlushState {
- const TransactionState* transaction;
+ TransactionState* transaction;
bool firstTransaction = true;
nsecs_t queueProcessTime = 0;
// Layer handles that have transactions with buffers that are ready to be applied.
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 62e31b9..d1b5fcc 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -228,9 +228,7 @@
if (mBufferInfo.mBuffer != nullptr) {
callReleaseBufferCallback(mDrawingState.releaseBufferListener,
mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFrameNumber,
- mBufferInfo.mFence,
- mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
- mOwnerUid));
+ mBufferInfo.mFence);
}
if (!isClone()) {
// The original layer and the clone layer share the same texture. Therefore, only one of
@@ -2732,12 +2730,13 @@
void Layer::callReleaseBufferCallback(const sp<ITransactionCompletedListener>& listener,
const sp<GraphicBuffer>& buffer, uint64_t framenumber,
- const sp<Fence>& releaseFence,
- uint32_t currentMaxAcquiredBufferCount) {
+ const sp<Fence>& releaseFence) {
if (!listener) {
return;
}
ATRACE_FORMAT_INSTANT("callReleaseBufferCallback %s - %" PRIu64, getDebugName(), framenumber);
+ uint32_t currentMaxAcquiredBufferCount =
+ mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid);
listener->onReleaseBuffer({buffer->getId(), framenumber},
releaseFence ? releaseFence : Fence::NO_FENCE,
currentMaxAcquiredBufferCount);
@@ -2988,9 +2987,7 @@
// call any release buffer callbacks if set.
callReleaseBufferCallback(mDrawingState.releaseBufferListener,
mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber,
- mDrawingState.acquireFence,
- mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
- mOwnerUid));
+ mDrawingState.acquireFence);
decrementPendingBufferCount();
if (mDrawingState.bufferSurfaceFrameTX != nullptr &&
mDrawingState.bufferSurfaceFrameTX->getPresentState() != PresentState::Presented) {
@@ -3000,13 +2997,12 @@
} else if (EARLY_RELEASE_ENABLED && mLastClientCompositionFence != nullptr) {
callReleaseBufferCallback(mDrawingState.releaseBufferListener,
mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber,
- mLastClientCompositionFence,
- mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
- mOwnerUid));
+ mLastClientCompositionFence);
mLastClientCompositionFence = nullptr;
}
}
+ mDrawingState.producerId = bufferData.producerId;
mDrawingState.frameNumber = frameNumber;
mDrawingState.releaseBufferListener = bufferData.releaseBufferListener;
mDrawingState.buffer = std::move(buffer);
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 429dfb0..e2bae23 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -141,6 +141,8 @@
uint64_t frameNumber;
ui::Transform transform;
+
+ uint32_t producerId = 0;
uint32_t bufferTransform;
bool transformToDisplayInverse;
Region transparentRegionHint;
@@ -833,6 +835,10 @@
void updateRelativeMetadataSnapshot(const LayerMetadata& relativeLayerMetadata,
std::unordered_set<Layer*>& visited);
+ void callReleaseBufferCallback(const sp<ITransactionCompletedListener>& listener,
+ const sp<GraphicBuffer>& buffer, uint64_t framenumber,
+ const sp<Fence>& releaseFence);
+
protected:
// For unit tests
friend class TestableSurfaceFlinger;
@@ -1044,6 +1050,10 @@
const sp<Fence>& releaseFence,
uint32_t currentMaxAcquiredBufferCount);
+ // Returns true if the transformed buffer size does not match the layer size and we need
+ // to apply filtering.
+ bool bufferNeedsFiltering() const;
+
// Returns true if there is a valid color to fill.
bool fillsColor() const;
// Returns true if this layer has a blur value.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index e3649ec..c7c91ce 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3971,16 +3971,30 @@
sp<Layer> layer = LayerHandle::getLayer(s.surface);
const auto& transaction = *flushState.transaction;
// check for barrier frames
- if (s.bufferData->hasBarrier &&
- ((layer->getDrawingState().frameNumber) < s.bufferData->barrierFrameNumber)) {
- const bool willApplyBarrierFrame =
- flushState.bufferLayersReadyToPresent.contains(s.surface.get()) &&
- (flushState.bufferLayersReadyToPresent.get(s.surface.get()) >=
- s.bufferData->barrierFrameNumber);
- if (!willApplyBarrierFrame) {
- ATRACE_NAME("NotReadyBarrier");
- ready = TransactionReadiness::NotReadyBarrier;
- return false;
+ if (s.bufferData->hasBarrier) {
+ // The current producerId is already a newer producer than the buffer that has a
+ // barrier. This means the incoming buffer is older and we can release it here. We
+ // don't wait on the barrier since we know that's stale information.
+ if (layer->getDrawingState().producerId > s.bufferData->producerId) {
+ layer->callReleaseBufferCallback(s.bufferData->releaseBufferListener,
+ s.bufferData->buffer, s.bufferData->frameNumber,
+ s.bufferData->acquireFence);
+ // Delete the entire state at this point and not just release the buffer because
+ // everything associated with the Layer in this Transaction is now out of date.
+ ATRACE_NAME("DeleteStaleBuffer");
+ return TraverseBuffersReturnValues::DELETE_AND_CONTINUE_TRAVERSAL;
+ }
+
+ if (layer->getDrawingState().frameNumber < s.bufferData->barrierFrameNumber) {
+ const bool willApplyBarrierFrame =
+ flushState.bufferLayersReadyToPresent.contains(s.surface.get()) &&
+ ((flushState.bufferLayersReadyToPresent.get(s.surface.get()) >=
+ s.bufferData->barrierFrameNumber));
+ if (!willApplyBarrierFrame) {
+ ATRACE_NAME("NotReadyBarrier");
+ ready = TransactionReadiness::NotReadyBarrier;
+ return TraverseBuffersReturnValues::STOP_TRAVERSAL;
+ }
}
}
@@ -3991,7 +4005,7 @@
if (layer->backpressureEnabled() && hasPendingBuffer && transaction.isAutoTimestamp) {
ATRACE_NAME("hasPendingBuffer");
ready = TransactionReadiness::NotReady;
- return false;
+ return TraverseBuffersReturnValues::STOP_TRAVERSAL;
}
// check fence status
@@ -4018,14 +4032,14 @@
"Buffer processing hung up due to stuck "
"fence. Indicates GPU hang");
}
- return false;
+ return TraverseBuffersReturnValues::STOP_TRAVERSAL;
}
ready = enableLatchUnsignaledConfig == LatchUnsignaledConfig::AutoSingleLayer
? TransactionReadiness::ReadyUnsignaledSingle
: TransactionReadiness::ReadyUnsignaled;
}
- return true;
+ return TraverseBuffersReturnValues::CONTINUE_TRAVERSAL;
});
ATRACE_INT("TransactionReadiness", static_cast<int>(ready));
return ready;
diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h
index 5025c49..6c5a8b2 100644
--- a/services/surfaceflinger/TransactionState.h
+++ b/services/surfaceflinger/TransactionState.h
@@ -27,6 +27,12 @@
namespace android {
+enum TraverseBuffersReturnValues {
+ CONTINUE_TRAVERSAL,
+ STOP_TRAVERSAL,
+ DELETE_AND_CONTINUE_TRAVERSAL,
+};
+
// Extends the client side composer state by resolving buffer.
class ResolvedComposerState : public ComposerState {
public:
@@ -75,12 +81,18 @@
}
template <typename Visitor>
- void traverseStatesWithBuffersWhileTrue(Visitor&& visitor) const {
- for (const auto& state : states) {
- if (state.state.hasBufferChanges() && state.state.hasValidBuffer() &&
- state.state.surface) {
- if (!visitor(state.state)) return;
+ void traverseStatesWithBuffersWhileTrue(Visitor&& visitor) {
+ for (auto state = states.begin(); state != states.end();) {
+ if (state->state.hasBufferChanges() && state->state.hasValidBuffer() &&
+ state->state.surface) {
+ int result = visitor(state->state);
+ if (result == STOP_TRAVERSAL) return;
+ if (result == DELETE_AND_CONTINUE_TRAVERSAL) {
+ state = states.erase(state);
+ continue;
+ }
}
+ state++;
}
}
diff --git a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
index 16076ea..c23fb9b 100644
--- a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
+++ b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
@@ -85,7 +85,8 @@
sp<Fence> fence, CallbackHelper& callback, const ReleaseCallbackId& id,
ReleaseBufferCallbackHelper& releaseCallback) {
Transaction t;
- t.setBuffer(layer, buffer, fence, id.framenumber, releaseCallback.getCallback());
+ t.setBuffer(layer, buffer, fence, id.framenumber, 0 /* producerId */,
+ releaseCallback.getCallback());
t.addTransactionCompletedCallback(callback.function, callback.getContext());
t.apply();
}
@@ -301,7 +302,7 @@
Transaction t;
t.setBuffer(layer, firstBuffer, std::nullopt, firstBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
t.addTransactionCompletedCallback(transactionCallback.function,
transactionCallback.getContext());
t.setDesiredPresentTime(time);
@@ -317,7 +318,7 @@
sp<GraphicBuffer> secondBuffer = getBuffer();
ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
t.setBuffer(layer, secondBuffer, std::nullopt, secondBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
t.addTransactionCompletedCallback(transactionCallback.function,
transactionCallback.getContext());
t.setDesiredPresentTime(time);
@@ -362,7 +363,7 @@
Transaction transaction1;
transaction1.setBuffer(layer, secondBuffer, std::nullopt, secondBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
transaction1.addTransactionCompletedCallback(callback1.function, callback1.getContext());
// Set a different TransactionCompletedListener to mimic a second process
@@ -397,14 +398,14 @@
// Create transaction with a buffer.
Transaction transaction;
transaction.setBuffer(layer, firstBuffer, std::nullopt, firstBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
sp<GraphicBuffer> secondBuffer = getBuffer();
ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
// Call setBuffer on the same transaction with a different buffer.
transaction.setBuffer(layer, secondBuffer, std::nullopt, secondBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
}
@@ -419,7 +420,7 @@
// Create transaction with a buffer.
Transaction transaction1;
transaction1.setBuffer(layer, firstBuffer, std::nullopt, firstBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
sp<GraphicBuffer> secondBuffer = getBuffer();
ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
@@ -427,7 +428,7 @@
// Create a second transaction with a new buffer for the same layer.
Transaction transaction2;
transaction2.setBuffer(layer, secondBuffer, std::nullopt, secondBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
// merge transaction1 into transaction2 so ensure we get a proper buffer release callback.
transaction1.merge(std::move(transaction2));
@@ -450,7 +451,7 @@
Transaction transaction1;
transaction1.setBuffer(layer, firstBuffer, std::nullopt, firstBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
// Sent a second buffer to allow the first buffer to get released.
sp<GraphicBuffer> secondBuffer = getBuffer();
@@ -458,7 +459,7 @@
Transaction transaction2;
transaction2.setBuffer(layer, secondBuffer, std::nullopt, secondBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
// Set a different TransactionCompletedListener to mimic a second process
TransactionCompletedListener::setInstance(secondCompletedListener);
@@ -479,10 +480,11 @@
// Create transaction with a buffer.
Transaction transaction;
transaction.setBuffer(layer, firstBuffer, std::nullopt, firstBufferCallbackId.framenumber,
- releaseCallback->getCallback());
+ 0 /* producerId */, releaseCallback->getCallback());
// Call setBuffer on the same transaction with a null buffer.
- transaction.setBuffer(layer, nullptr, std::nullopt, 0, releaseCallback->getCallback());
+ transaction.setBuffer(layer, nullptr, std::nullopt, 0, 0 /* producerId */,
+ releaseCallback->getCallback());
ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
}