Merge "SF: check previous present fence to avoid early presentation" into sc-dev
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index e15e11c..2970116 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -132,8 +132,7 @@
BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp<SurfaceControl>& surface,
int width, int height, int32_t format)
- : mName(name),
- mSurfaceControl(surface),
+ : mSurfaceControl(surface),
mSize(width, height),
mRequestedSize(mSize),
mFormat(format),
@@ -150,6 +149,7 @@
GraphicBuffer::USAGE_HW_TEXTURE,
1, false);
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++;
@@ -313,24 +313,24 @@
// BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client.
// So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer.
// Otherwise, this is a no-op.
-static void releaseBufferCallbackThunk(wp<BLASTBufferQueue> context, uint64_t graphicBufferId,
+static void releaseBufferCallbackThunk(wp<BLASTBufferQueue> context, const ReleaseCallbackId& id,
const sp<Fence>& releaseFence, uint32_t transformHint,
uint32_t currentMaxAcquiredBufferCount) {
sp<BLASTBufferQueue> blastBufferQueue = context.promote();
- ALOGV("releaseBufferCallbackThunk graphicBufferId=%" PRIu64 " blastBufferQueue=%s",
- graphicBufferId, blastBufferQueue ? "alive" : "dead");
if (blastBufferQueue) {
- blastBufferQueue->releaseBufferCallback(graphicBufferId, releaseFence, transformHint,
+ blastBufferQueue->releaseBufferCallback(id, releaseFence, transformHint,
currentMaxAcquiredBufferCount);
+ } else {
+ ALOGV("releaseBufferCallbackThunk %s blastBufferQueue is dead", id.to_string().c_str());
}
}
-void BLASTBufferQueue::releaseBufferCallback(uint64_t graphicBufferId,
+void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id,
const sp<Fence>& releaseFence, uint32_t transformHint,
uint32_t currentMaxAcquiredBufferCount) {
ATRACE_CALL();
std::unique_lock _lock{mMutex};
- BQA_LOGV("releaseBufferCallback graphicBufferId=%" PRIu64, graphicBufferId);
+ BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str());
if (mSurfaceControl != nullptr) {
mTransformHint = transformHint;
@@ -343,25 +343,26 @@
// on a lower refresh rate than the max supported. We only do that for EGL
// clients as others don't care about latency
const bool isEGL = [&] {
- const auto it = mSubmitted.find(graphicBufferId);
+ const auto it = mSubmitted.find(id);
return it != mSubmitted.end() && it->second.mApi == NATIVE_WINDOW_API_EGL;
}();
const auto numPendingBuffersToHold =
isEGL ? std::max(0u, mMaxAcquiredBuffers - currentMaxAcquiredBufferCount) : 0;
- mPendingRelease.emplace_back(ReleasedBuffer{graphicBufferId, releaseFence});
+ mPendingRelease.emplace_back(ReleasedBuffer{id, releaseFence});
// Release all buffers that are beyond the ones that we need to hold
while (mPendingRelease.size() > numPendingBuffersToHold) {
const auto releaseBuffer = mPendingRelease.front();
mPendingRelease.pop_front();
- auto it = mSubmitted.find(releaseBuffer.bufferId);
+ auto it = mSubmitted.find(releaseBuffer.callbackId);
if (it == mSubmitted.end()) {
- BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %" PRIu64,
- graphicBufferId);
+ BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s",
+ releaseBuffer.callbackId.to_string().c_str());
return;
}
mNumAcquired--;
+ BQA_LOGV("released %s", id.to_string().c_str());
mBufferItemConsumer->releaseBuffer(it->second, releaseBuffer.releaseFence);
mSubmitted.erase(it);
processNextBufferLocked(false /* useNextTransaction */);
@@ -428,7 +429,9 @@
}
mNumAcquired++;
- mSubmitted[buffer->getId()] = bufferItem;
+ mLastAcquiredFrameNumber = bufferItem.mFrameNumber;
+ ReleaseCallbackId releaseCallbackId(buffer->getId(), mLastAcquiredFrameNumber);
+ mSubmitted[releaseCallbackId] = bufferItem;
bool needsDisconnect = false;
mBufferItemConsumer->getConnectionEvents(bufferItem.mFrameNumber, &needsDisconnect);
@@ -442,7 +445,6 @@
incStrong((void*)transactionCallbackThunk);
Rect crop = computeCrop(bufferItem);
- mLastAcquiredFrameNumber = bufferItem.mFrameNumber;
mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(),
bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform,
bufferItem.mScalingMode, crop);
@@ -451,7 +453,7 @@
std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */,
std::placeholders::_1, std::placeholders::_2, std::placeholders::_3,
std::placeholders::_4);
- t->setBuffer(mSurfaceControl, buffer, releaseBufferCallback);
+ t->setBuffer(mSurfaceControl, buffer, releaseCallbackId, releaseBufferCallback);
t->setDataspace(mSurfaceControl, static_cast<ui::Dataspace>(bufferItem.mDataSpace));
t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata);
t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage);
@@ -510,11 +512,11 @@
BQA_LOGV("processNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64
" applyTransaction=%s mTimestamp=%" PRId64 "%s mPendingTransactions.size=%d"
- " graphicBufferId=%" PRIu64,
+ " graphicBufferId=%" PRIu64 "%s",
mSize.width, mSize.height, bufferItem.mFrameNumber, toString(applyTransaction),
bufferItem.mTimestamp, bufferItem.mIsAutoTimestamp ? "(auto)" : "",
- static_cast<uint32_t>(mPendingTransactions.size()),
- bufferItem.mGraphicBuffer->getId());
+ static_cast<uint32_t>(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(),
+ bufferItem.mAutoRefresh ? " mAutoRefresh" : "");
}
Rect BLASTBufferQueue::computeCrop(const BufferItem& item) {
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index 17499ec..98e8b54 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -125,7 +125,7 @@
for (const auto& data : jankData) {
SAFE_PARCEL(output->writeParcelable, data);
}
- SAFE_PARCEL(output->writeUint64, previousBufferId);
+ SAFE_PARCEL(output->writeParcelable, previousReleaseCallbackId);
return NO_ERROR;
}
@@ -149,7 +149,7 @@
SAFE_PARCEL(input->readParcelable, &data);
jankData.push_back(data);
}
- SAFE_PARCEL(input->readUint64, &previousBufferId);
+ SAFE_PARCEL(input->readParcelable, &previousReleaseCallbackId);
return NO_ERROR;
}
@@ -253,11 +253,11 @@
stats);
}
- void onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence, uint32_t transformHint,
- uint32_t currentMaxAcquiredBufferCount) override {
+ void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence,
+ uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) override {
callRemoteAsync<decltype(
&ITransactionCompletedListener::onReleaseBuffer)>(Tag::ON_RELEASE_BUFFER,
- graphicBufferId, releaseFence,
+ callbackId, releaseFence,
transformHint,
currentMaxAcquiredBufferCount);
}
@@ -308,4 +308,18 @@
return NO_ERROR;
}
+status_t ReleaseCallbackId::writeToParcel(Parcel* output) const {
+ SAFE_PARCEL(output->writeUint64, bufferId);
+ SAFE_PARCEL(output->writeUint64, framenumber);
+ return NO_ERROR;
+}
+
+status_t ReleaseCallbackId::readFromParcel(const Parcel* input) {
+ SAFE_PARCEL(input->readUint64, &bufferId);
+ SAFE_PARCEL(input->readUint64, &framenumber);
+ return NO_ERROR;
+}
+
+const ReleaseCallbackId ReleaseCallbackId::INVALID_ID = ReleaseCallbackId(0, 0);
+
}; // namespace android
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index fd78309..6ea070ea 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -197,15 +197,16 @@
}
}
-void TransactionCompletedListener::setReleaseBufferCallback(uint64_t graphicBufferId,
+void TransactionCompletedListener::setReleaseBufferCallback(const ReleaseCallbackId& callbackId,
ReleaseBufferCallback listener) {
std::scoped_lock<std::mutex> lock(mMutex);
- mReleaseBufferCallbacks[graphicBufferId] = listener;
+ mReleaseBufferCallbacks[callbackId] = listener;
}
-void TransactionCompletedListener::removeReleaseBufferCallback(uint64_t graphicBufferId) {
+void TransactionCompletedListener::removeReleaseBufferCallback(
+ const ReleaseCallbackId& callbackId) {
std::scoped_lock<std::mutex> lock(mMutex);
- mReleaseBufferCallbacks.erase(graphicBufferId);
+ mReleaseBufferCallbacks.erase(callbackId);
}
void TransactionCompletedListener::addSurfaceStatsListener(void* context, void* cookie,
@@ -319,14 +320,15 @@
// and call them. This is a performance optimization when we have a transaction
// callback and a release buffer callback happening at the same time to avoid an
// additional ipc call from the server.
- if (surfaceStats.previousBufferId) {
+ if (surfaceStats.previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) {
ReleaseBufferCallback callback;
{
std::scoped_lock<std::mutex> lock(mMutex);
- callback = popReleaseBufferCallbackLocked(surfaceStats.previousBufferId);
+ callback = popReleaseBufferCallbackLocked(
+ surfaceStats.previousReleaseCallbackId);
}
if (callback) {
- callback(surfaceStats.previousBufferId,
+ callback(surfaceStats.previousReleaseCallbackId,
surfaceStats.previousReleaseFence
? surfaceStats.previousReleaseFence
: Fence::NO_FENCE,
@@ -362,25 +364,26 @@
}
}
-void TransactionCompletedListener::onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence,
- uint32_t transformHint,
+void TransactionCompletedListener::onReleaseBuffer(ReleaseCallbackId callbackId,
+ sp<Fence> releaseFence, uint32_t transformHint,
uint32_t currentMaxAcquiredBufferCount) {
ReleaseBufferCallback callback;
{
std::scoped_lock<std::mutex> lock(mMutex);
- callback = popReleaseBufferCallbackLocked(graphicBufferId);
+ callback = popReleaseBufferCallbackLocked(callbackId);
}
if (!callback) {
- ALOGE("Could not call release buffer callback, buffer not found %" PRIu64, graphicBufferId);
+ ALOGE("Could not call release buffer callback, buffer not found %s",
+ callbackId.to_string().c_str());
return;
}
- callback(graphicBufferId, releaseFence, transformHint, currentMaxAcquiredBufferCount);
+ callback(callbackId, releaseFence, transformHint, currentMaxAcquiredBufferCount);
}
ReleaseBufferCallback TransactionCompletedListener::popReleaseBufferCallbackLocked(
- uint64_t graphicBufferId) {
+ const ReleaseCallbackId& callbackId) {
ReleaseBufferCallback callback;
- auto itr = mReleaseBufferCallbacks.find(graphicBufferId);
+ auto itr = mReleaseBufferCallbacks.find(callbackId);
if (itr == mReleaseBufferCallbacks.end()) {
return nullptr;
}
@@ -1258,7 +1261,7 @@
}
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer(
- const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
+ const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer, const ReleaseCallbackId& id,
ReleaseBufferCallback callback) {
layer_state_t* s = getLayerState(sc);
if (!s) {
@@ -1271,7 +1274,7 @@
if (mIsAutoTimestamp) {
mDesiredPresentTime = systemTime();
}
- setReleaseBufferCallback(s, callback);
+ setReleaseBufferCallback(s, id, callback);
registerSurfaceControlForCallback(sc);
@@ -1286,10 +1289,13 @@
s->what &= ~static_cast<uint64_t>(layer_state_t::eReleaseBufferListenerChanged);
s->releaseBufferListener = nullptr;
- TransactionCompletedListener::getInstance()->removeReleaseBufferCallback(s->buffer->getId());
+ auto listener = TransactionCompletedListener::getInstance();
+ listener->removeReleaseBufferCallback(s->releaseCallbackId);
+ s->releaseCallbackId = ReleaseCallbackId::INVALID_ID;
}
void SurfaceComposerClient::Transaction::setReleaseBufferCallback(layer_state_t* s,
+ const ReleaseCallbackId& id,
ReleaseBufferCallback callback) {
if (!callback) {
return;
@@ -1303,8 +1309,9 @@
s->what |= layer_state_t::eReleaseBufferListenerChanged;
s->releaseBufferListener = TransactionCompletedListener::getIInstance();
+ s->releaseCallbackId = id;
auto listener = TransactionCompletedListener::getInstance();
- listener->setReleaseBufferCallback(s->buffer->getId(), callback);
+ listener->setReleaseBufferCallback(id, callback);
}
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setAcquireFence(
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 26c7285..0d6a673 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -89,7 +89,7 @@
void transactionCallback(nsecs_t latchTime, const sp<Fence>& presentFence,
const std::vector<SurfaceControlStats>& stats);
- void releaseBufferCallback(uint64_t graphicBufferId, const sp<Fence>& releaseFence,
+ void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,
uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount);
void setNextTransaction(SurfaceComposerClient::Transaction *t);
void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber);
@@ -144,13 +144,14 @@
// 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<uint64_t /* bufferId */, BufferItem> mSubmitted GUARDED_BY(mMutex);
+ std::unordered_map<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mSubmitted
+ GUARDED_BY(mMutex);
// Keep a queue of the released buffers instead of immediately releasing
// the buffers back to the buffer queue. This would be controlled by SF
// setting the max acquired buffer count.
struct ReleasedBuffer {
- uint64_t bufferId;
+ ReleaseCallbackId callbackId;
sp<Fence> releaseFence;
};
std::deque<ReleasedBuffer> mPendingRelease GUARDED_BY(mMutex);
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index d286c34..937095c 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -53,6 +53,36 @@
std::size_t operator()(const CallbackId& key) const { return std::hash<int64_t>()(key.id); }
};
+class ReleaseCallbackId : public Parcelable {
+public:
+ static const ReleaseCallbackId INVALID_ID;
+
+ uint64_t bufferId;
+ uint64_t framenumber;
+ ReleaseCallbackId() {}
+ ReleaseCallbackId(uint64_t bufferId, uint64_t framenumber)
+ : bufferId(bufferId), framenumber(framenumber) {}
+ status_t writeToParcel(Parcel* output) const override;
+ status_t readFromParcel(const Parcel* input) override;
+
+ bool operator==(const ReleaseCallbackId& rhs) const {
+ return bufferId == rhs.bufferId && framenumber == rhs.framenumber;
+ }
+ bool operator!=(const ReleaseCallbackId& rhs) const { return !operator==(rhs); }
+ std::string to_string() const {
+ if (*this == INVALID_ID) return "INVALID_ID";
+
+ return "bufferId:" + std::to_string(bufferId) +
+ " framenumber:" + std::to_string(framenumber);
+ }
+};
+
+struct ReleaseBufferCallbackIdHash {
+ std::size_t operator()(const ReleaseCallbackId& key) const {
+ return std::hash<uint64_t>()(key.bufferId);
+ }
+};
+
class FrameEventHistoryStats : public Parcelable {
public:
status_t writeToParcel(Parcel* output) const override;
@@ -103,7 +133,7 @@
SurfaceStats(const sp<IBinder>& sc, nsecs_t time, const sp<Fence>& prevReleaseFence,
uint32_t hint, uint32_t currentMaxAcquiredBuffersCount,
FrameEventHistoryStats frameEventStats, std::vector<JankData> jankData,
- uint64_t previousBufferId)
+ ReleaseCallbackId previousReleaseCallbackId)
: surfaceControl(sc),
acquireTime(time),
previousReleaseFence(prevReleaseFence),
@@ -111,7 +141,7 @@
currentMaxAcquiredBufferCount(currentMaxAcquiredBuffersCount),
eventStats(frameEventStats),
jankData(std::move(jankData)),
- previousBufferId(previousBufferId) {}
+ previousReleaseCallbackId(previousReleaseCallbackId) {}
sp<IBinder> surfaceControl;
nsecs_t acquireTime = -1;
@@ -120,7 +150,7 @@
uint32_t currentMaxAcquiredBufferCount = 0;
FrameEventHistoryStats eventStats;
std::vector<JankData> jankData;
- uint64_t previousBufferId;
+ ReleaseCallbackId previousReleaseCallbackId;
};
class TransactionStats : public Parcelable {
@@ -161,7 +191,7 @@
virtual void onTransactionCompleted(ListenerStats stats) = 0;
- virtual void onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence,
+ virtual void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence,
uint32_t transformHint,
uint32_t currentMaxAcquiredBufferCount) = 0;
};
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 16430b3..8ac1e5d 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -234,6 +234,12 @@
// layers only. The callback includes a release fence as well as the graphic
// buffer id to identify the buffer.
sp<ITransactionCompletedListener> releaseBufferListener;
+
+ // Keeps track of the release callback id associated with the listener. This
+ // is not sent to the server since the id can be reconstructed there. This
+ // is used to remove the old callback from the client process map if it is
+ // overwritten by another setBuffer call.
+ ReleaseCallbackId releaseCallbackId;
};
struct ComposerState {
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 13994fd..c2963b5 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -82,7 +82,7 @@
std::function<void(nsecs_t /*latchTime*/, const sp<Fence>& /*presentFence*/,
const std::vector<SurfaceControlStats>& /*stats*/)>;
using ReleaseBufferCallback =
- std::function<void(uint64_t /* graphicsBufferId */, const sp<Fence>& /*releaseFence*/,
+ std::function<void(const ReleaseCallbackId&, const sp<Fence>& /*releaseFence*/,
uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount)>;
using SurfaceStatsCallback =
@@ -397,8 +397,9 @@
void cacheBuffers();
void registerSurfaceControlForCallback(const sp<SurfaceControl>& sc);
- void setReleaseBufferCallback(layer_state_t* state, ReleaseBufferCallback callback);
- void removeReleaseBufferCallback(layer_state_t* state);
+ void setReleaseBufferCallback(layer_state_t*, const ReleaseCallbackId&,
+ ReleaseBufferCallback);
+ void removeReleaseBufferCallback(layer_state_t*);
public:
Transaction();
@@ -470,6 +471,7 @@
Transaction& setTransformToDisplayInverse(const sp<SurfaceControl>& sc,
bool transformToDisplayInverse);
Transaction& setBuffer(const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
+ const ReleaseCallbackId& id = ReleaseCallbackId::INVALID_ID,
ReleaseBufferCallback callback = nullptr);
Transaction& setCachedBuffer(const sp<SurfaceControl>& sc, int32_t bufferId);
Transaction& setAcquireFence(const sp<SurfaceControl>& sc, const sp<Fence>& fence);
@@ -678,7 +680,7 @@
std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks
GUARDED_BY(mMutex);
std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex);
- std::unordered_map<uint64_t /* graphicsBufferId */, ReleaseBufferCallback>
+ std::unordered_map<ReleaseCallbackId, ReleaseBufferCallback, ReleaseBufferCallbackIdHash>
mReleaseBufferCallbacks GUARDED_BY(mMutex);
// This is protected by mSurfaceStatsListenerMutex, but GUARDED_BY isn't supported for
@@ -717,16 +719,16 @@
SurfaceStatsCallback listener);
void removeSurfaceStatsListener(void* context, void* cookie);
- void setReleaseBufferCallback(uint64_t /* graphicsBufferId */, ReleaseBufferCallback);
- void removeReleaseBufferCallback(uint64_t /* graphicsBufferId */);
+ void setReleaseBufferCallback(const ReleaseCallbackId&, ReleaseBufferCallback);
+ void removeReleaseBufferCallback(const ReleaseCallbackId&);
// BnTransactionCompletedListener overrides
void onTransactionCompleted(ListenerStats stats) override;
- void onReleaseBuffer(uint64_t /* graphicsBufferId */, sp<Fence> releaseFence,
- uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) override;
+ void onReleaseBuffer(ReleaseCallbackId, sp<Fence> releaseFence, uint32_t transformHint,
+ uint32_t currentMaxAcquiredBufferCount) override;
private:
- ReleaseBufferCallback popReleaseBufferCallbackLocked(uint64_t /* graphicsBufferId */);
+ ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&);
};
} // namespace android
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index cd3d80e..760c8b9 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -133,6 +133,7 @@
bool mTransformToDisplayInverse{false};
std::shared_ptr<renderengine::ExternalTexture> mBuffer;
+ uint64_t mFrameNumber;
int mBufferSlot{BufferQueue::INVALID_BUFFER_SLOT};
bool mFrameLatencyNeeded{false};
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 6b5cf04..2a49a0a 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -44,25 +44,18 @@
using PresentState = frametimeline::SurfaceFrame::PresentState;
namespace {
void callReleaseBufferCallback(const sp<ITransactionCompletedListener>& listener,
- const sp<GraphicBuffer>& buffer, const sp<Fence>& releaseFence,
- uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) {
+ const sp<GraphicBuffer>& buffer, uint64_t framenumber,
+ const sp<Fence>& releaseFence, uint32_t transformHint,
+ uint32_t currentMaxAcquiredBufferCount) {
if (!listener) {
return;
}
- listener->onReleaseBuffer(buffer->getId(), releaseFence ? releaseFence : Fence::NO_FENCE,
- transformHint, currentMaxAcquiredBufferCount);
+ listener->onReleaseBuffer({buffer->getId(), framenumber},
+ releaseFence ? releaseFence : Fence::NO_FENCE, transformHint,
+ currentMaxAcquiredBufferCount);
}
} // namespace
-// clang-format off
-const std::array<float, 16> BufferStateLayer::IDENTITY_MATRIX{
- 1, 0, 0, 0,
- 0, 1, 0, 0,
- 0, 0, 1, 0,
- 0, 0, 0, 1
-};
-// clang-format on
-
BufferStateLayer::BufferStateLayer(const LayerCreationArgs& args)
: BufferLayer(args), mHwcSlotGenerator(new HwcSlotGenerator()) {
mDrawingState.dataspace = ui::Dataspace::V0_SRGB;
@@ -75,8 +68,8 @@
// issue with the clone layer trying to use the texture.
if (mBufferInfo.mBuffer != nullptr && !isClone()) {
callReleaseBufferCallback(mDrawingState.releaseBufferListener,
- mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFence,
- mTransformHint,
+ mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFrameNumber,
+ mBufferInfo.mFence, mTransformHint,
mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
mOwnerUid));
}
@@ -87,7 +80,7 @@
if (ch == nullptr) {
return OK;
}
- ch->previousBufferId = mPreviousBufferId;
+ ch->previousReleaseCallbackId = mPreviousReleaseCallbackId;
if (!ch->previousReleaseFence.get()) {
ch->previousReleaseFence = fence;
return OK;
@@ -214,7 +207,7 @@
// see BufferStateLayer::onLayerDisplayed.
for (auto& handle : mDrawingState.callbackHandles) {
if (handle->releasePreviousBuffer) {
- handle->previousBufferId = mPreviousBufferId;
+ handle->previousReleaseCallbackId = mPreviousReleaseCallbackId;
break;
}
}
@@ -438,8 +431,8 @@
// dropped and we should decrement the pending buffer count and
// call any release buffer callbacks if set.
callReleaseBufferCallback(mDrawingState.releaseBufferListener,
- mDrawingState.buffer->getBuffer(), mDrawingState.acquireFence,
- mTransformHint,
+ mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber,
+ mDrawingState.acquireFence, mTransformHint,
mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
mOwnerUid));
decrementPendingBufferCount();
@@ -684,7 +677,7 @@
* DeferTransactionUntil -> frameNumber = 2
* Random other stuff
* }
- * Now imagine getHeadFrameNumber returned mDrawingState.mFrameNumber (or mCurrentFrameNumber).
+ * Now imagine mFrameNumber returned mDrawingState.frameNumber (or mCurrentFrameNumber).
* Prior to doTransaction SurfaceFlinger will call notifyAvailableFrames, but because we
* haven't swapped mDrawingState to mDrawingState yet we will think the sync point
* is not ready. So we will return false from applyPendingState and not swap
@@ -725,7 +718,7 @@
bool BufferStateLayer::hasFrameUpdate() const {
const State& c(getDrawingState());
- return mDrawingStateModified && (c.buffer != nullptr || c.bgColorLayer != nullptr);
+ return (mDrawingStateModified || mDrawingState.modified) && (c.buffer != nullptr || c.bgColorLayer != nullptr);
}
status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nsecs_t latchTime,
@@ -792,9 +785,10 @@
decrementPendingBufferCount();
}
- mPreviousBufferId = getCurrentBufferId();
+ mPreviousReleaseCallbackId = {getCurrentBufferId(), mBufferInfo.mFrameNumber};
mBufferInfo.mBuffer = s.buffer;
mBufferInfo.mFence = s.acquireFence;
+ mBufferInfo.mFrameNumber = s.frameNumber;
return NO_ERROR;
}
@@ -969,8 +963,8 @@
// then we will drop a buffer and should decrement the pending buffer count and
// call any release buffer callbacks if set.
callReleaseBufferCallback(mDrawingState.releaseBufferListener,
- mDrawingState.buffer->getBuffer(), mDrawingState.acquireFence,
- mTransformHint,
+ mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber,
+ mDrawingState.acquireFence, mTransformHint,
mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
mOwnerUid));
decrementPendingBufferCount();
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 2747018..e567478 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -143,15 +143,8 @@
bool bufferNeedsFiltering() const override;
- static const std::array<float, 16> IDENTITY_MATRIX;
-
- std::unique_ptr<renderengine::Image> mTextureImage;
-
- mutable uint64_t mFrameNumber{0};
- uint64_t mFrameCounter{0};
-
sp<Fence> mPreviousReleaseFence;
- uint64_t mPreviousBufferId = 0;
+ ReleaseCallbackId mPreviousReleaseCallbackId = ReleaseCallbackId::INVALID_ID;
uint64_t mPreviousReleasedFrameNumber = 0;
bool mReleasePreviousBuffer = false;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
index 7cb0f6b..2bf931c 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h
@@ -141,7 +141,7 @@
compositionengine::OutputLayer* getBlurLayer() const;
- bool hasHdrLayers() const;
+ bool hasUnsupportedDataspace() const;
bool hasProtectedLayers() const;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
index a20d7b3..bce438f 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h
@@ -242,13 +242,6 @@
ui::Dataspace getDataspace() const { return mOutputDataspace.get(); }
- bool isHdr() const {
- const ui::Dataspace transfer =
- static_cast<ui::Dataspace>(getDataspace() & ui::Dataspace::TRANSFER_MASK);
- return (transfer == ui::Dataspace::TRANSFER_ST2084 ||
- transfer == ui::Dataspace::TRANSFER_HLG);
- }
-
bool isProtected() const {
return getOutputLayer()->getLayerFE().getCompositionState()->hasProtectedContent;
}
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index e4e46a7..56e9d27 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -216,8 +216,18 @@
// reduce uses a FloatRect to provide more accuracy during the
// transformation. We then round upon constructing 'frame'.
- Rect frame{
- layerTransform.transform(reduce(layerState.geomLayerBounds, activeTransparentRegion))};
+ FloatRect geomLayerBounds = layerState.geomLayerBounds;
+
+ // Some HWCs may clip client composited input to its displayFrame. Make sure
+ // that this does not cut off the shadow.
+ if (layerState.forceClientComposition && layerState.shadowRadius > 0.0f) {
+ const auto outset = layerState.shadowRadius;
+ geomLayerBounds.left -= outset;
+ geomLayerBounds.top -= outset;
+ geomLayerBounds.right += outset;
+ geomLayerBounds.bottom += outset;
+ }
+ Rect frame{layerTransform.transform(reduce(geomLayerBounds, activeTransparentRegion))};
if (!frame.intersect(outputState.layerStackSpace.content, &frame)) {
frame.clear();
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
index acc7ed2..b24274e 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
@@ -295,9 +295,7 @@
return false;
}
- // Do not use a hole punch with an HDR layer; this should be done in client
- // composition to properly mix HDR with SDR.
- if (hasHdrLayers()) {
+ if (hasUnsupportedDataspace()) {
return false;
}
@@ -352,9 +350,22 @@
return mBlurLayer ? mBlurLayer->getOutputLayer() : nullptr;
}
-bool CachedSet::hasHdrLayers() const {
- return std::any_of(mLayers.cbegin(), mLayers.cend(),
- [](const Layer& layer) { return layer.getState()->isHdr(); });
+bool CachedSet::hasUnsupportedDataspace() const {
+ return std::any_of(mLayers.cbegin(), mLayers.cend(), [](const Layer& layer) {
+ auto dataspace = layer.getState()->getDataspace();
+ const auto transfer = static_cast<ui::Dataspace>(dataspace & ui::Dataspace::TRANSFER_MASK);
+ if (transfer == ui::Dataspace::TRANSFER_ST2084 || transfer == ui::Dataspace::TRANSFER_HLG) {
+ // Skip HDR.
+ return true;
+ }
+
+ if ((dataspace & HAL_DATASPACE_STANDARD_MASK) == HAL_DATASPACE_STANDARD_BT601_625) {
+ // RenderEngine does not match some DPUs, so skip
+ // to avoid flickering/color differences.
+ return true;
+ }
+ return false;
+ });
}
bool CachedSet::hasProtectedLayers() const {
@@ -378,10 +389,14 @@
if (mLayers.size() == 1) {
base::StringAppendF(&result, " Layer [%s]\n", mLayers[0].getName().c_str());
base::StringAppendF(&result, " Buffer %p", mLayers[0].getBuffer().get());
+ base::StringAppendF(&result, " Protected [%s]",
+ mLayers[0].getState()->isProtected() ? "true" : "false");
} else {
result.append(" Cached set of:");
for (const Layer& layer : mLayers) {
base::StringAppendF(&result, "\n Layer [%s]", layer.getName().c_str());
+ base::StringAppendF(&result, "\n Protected [%s]",
+ layer.getState()->isProtected() ? "true" : "false");
}
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
index 2bcaf60..f033279 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp
@@ -420,7 +420,7 @@
const bool layerIsInactive = now - currentSet->getLastUpdate() > mActiveLayerTimeout;
const bool layerHasBlur = currentSet->hasBlurBehind();
if (layerIsInactive && (firstLayer || runHasFirstLayer || !layerHasBlur) &&
- !currentSet->hasHdrLayers() && !currentSet->hasProtectedLayers()) {
+ !currentSet->hasUnsupportedDataspace()) {
if (isPartOfRun) {
builder.append(currentSet->getLayerCount());
} else {
@@ -491,6 +491,14 @@
return;
}
+ for (const CachedSet& layer : mLayers) {
+ // TODO (b/191997217): make it less aggressive, and sync with findCandidateRuns
+ if (layer.hasProtectedLayers()) {
+ ATRACE_NAME("layer->hasProtectedLayers()");
+ return;
+ }
+ }
+
std::vector<Run> runs = findCandidateRuns(now);
std::optional<Run> bestRun = findBestRun(runs);
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index e9ecf3e..c8c6012 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -324,6 +324,27 @@
EXPECT_THAT(calculateOutputDisplayFrame(), expected);
}
+TEST_F(OutputLayerDisplayFrameTest, shadowExpandsDisplayFrame) {
+ const int kShadowRadius = 5;
+ mLayerFEState.shadowRadius = kShadowRadius;
+ mLayerFEState.forceClientComposition = true;
+
+ mLayerFEState.geomLayerBounds = FloatRect{100.f, 100.f, 200.f, 200.f};
+ Rect expected{mLayerFEState.geomLayerBounds};
+ expected.inset(-kShadowRadius, -kShadowRadius, -kShadowRadius, -kShadowRadius);
+ EXPECT_THAT(calculateOutputDisplayFrame(), expected);
+}
+
+TEST_F(OutputLayerDisplayFrameTest, shadowExpandsDisplayFrame_onlyIfForcingClientComposition) {
+ const int kShadowRadius = 5;
+ mLayerFEState.shadowRadius = kShadowRadius;
+ mLayerFEState.forceClientComposition = false;
+
+ mLayerFEState.geomLayerBounds = FloatRect{100.f, 100.f, 200.f, 200.f};
+ Rect expected{mLayerFEState.geomLayerBounds};
+ EXPECT_THAT(calculateOutputDisplayFrame(), expected);
+}
+
/*
* OutputLayer::calculateOutputRelativeBufferTransform()
*/
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
index 0acc317..7f0e186 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp
@@ -457,6 +457,20 @@
EXPECT_FALSE(cachedSet.requiresHolePunch());
}
+TEST_F(CachedSetTest, holePunch_requiresNonBT601_625) {
+ mTestLayers[0]->outputLayerCompositionState.dataspace = ui::Dataspace::STANDARD_BT601_625;
+ mTestLayers[0]->layerState->update(&mTestLayers[0]->outputLayer);
+
+ CachedSet::Layer& layer = *mTestLayers[0]->cachedSetLayer.get();
+ mTestLayers[0]->layerFECompositionState.buffer = sp<GraphicBuffer>::make();
+ sp<mock::LayerFE> layerFE = mTestLayers[0]->layerFE;
+
+ CachedSet cachedSet(layer);
+ EXPECT_CALL(*layerFE, hasRoundedCorners()).WillRepeatedly(Return(true));
+
+ EXPECT_FALSE(cachedSet.requiresHolePunch());
+}
+
TEST_F(CachedSetTest, requiresHolePunch) {
CachedSet::Layer& layer = *mTestLayers[0]->cachedSetLayer.get();
mTestLayers[0]->layerFECompositionState.buffer = sp<GraphicBuffer>::make();
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
index 334b855..f5cfd2f 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp
@@ -937,5 +937,152 @@
(kCachedSetRenderDuration + 10ms));
}
+TEST_F(FlattenerTest, flattenLayers_skipsBT601_625) {
+ auto& layerState1 = mTestLayers[0]->layerState;
+ const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
+
+ auto& layerState2 = mTestLayers[1]->layerState;
+ const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer;
+
+ // The third layer uses a dataspace that will not be flattened due to
+ // possible mismatch with DPU rendering.
+ auto& layerState3 = mTestLayers[2]->layerState;
+ const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer;
+ mTestLayers[2]->outputLayerCompositionState.dataspace = ui::Dataspace::STANDARD_BT601_625;
+ mTestLayers[2]->layerState->update(&mTestLayers[2]->outputLayer);
+
+ const std::vector<const LayerState*> layers = {
+ layerState1.get(),
+ layerState2.get(),
+ layerState3.get(),
+ };
+
+ initializeFlattener(layers);
+
+ mTime += 200ms;
+ initializeOverrideBuffer(layers);
+ EXPECT_EQ(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+
+ // This will render a CachedSet.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Return(NO_ERROR));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ // We've rendered a CachedSet, but we haven't merged it in.
+ EXPECT_EQ(nullptr, overrideBuffer1);
+ EXPECT_EQ(nullptr, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+
+ // This time we merge the CachedSet in, so we have a new hash, and we should
+ // only have two sets.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).Times(0);
+ initializeOverrideBuffer(layers);
+ EXPECT_NE(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ EXPECT_NE(nullptr, overrideBuffer1);
+ EXPECT_EQ(overrideBuffer1, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+}
+
+TEST_F(FlattenerTest, flattenLayers_skipsHDR) {
+ auto& layerState1 = mTestLayers[0]->layerState;
+ const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
+
+ auto& layerState2 = mTestLayers[1]->layerState;
+ const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer;
+
+ // The third layer uses a dataspace that will not be flattened due to
+ // possible mismatch with DPU rendering.
+ auto& layerState3 = mTestLayers[2]->layerState;
+ const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer;
+ mTestLayers[2]->outputLayerCompositionState.dataspace = ui::Dataspace::BT2020_ITU_HLG;
+ mTestLayers[2]->layerState->update(&mTestLayers[2]->outputLayer);
+
+ const std::vector<const LayerState*> layers = {
+ layerState1.get(),
+ layerState2.get(),
+ layerState3.get(),
+ };
+
+ initializeFlattener(layers);
+
+ mTime += 200ms;
+ initializeOverrideBuffer(layers);
+ EXPECT_EQ(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+
+ // This will render a CachedSet.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Return(NO_ERROR));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ // We've rendered a CachedSet, but we haven't merged it in.
+ EXPECT_EQ(nullptr, overrideBuffer1);
+ EXPECT_EQ(nullptr, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+
+ // This time we merge the CachedSet in, so we have a new hash, and we should
+ // only have two sets.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).Times(0);
+ initializeOverrideBuffer(layers);
+ EXPECT_NE(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ EXPECT_NE(nullptr, overrideBuffer1);
+ EXPECT_EQ(overrideBuffer1, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+}
+
+TEST_F(FlattenerTest, flattenLayers_skipsHDR2) {
+ auto& layerState1 = mTestLayers[0]->layerState;
+ const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer;
+
+ auto& layerState2 = mTestLayers[1]->layerState;
+ const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer;
+
+ // The third layer uses a dataspace that will not be flattened due to
+ // possible mismatch with DPU rendering.
+ auto& layerState3 = mTestLayers[2]->layerState;
+ const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer;
+ mTestLayers[2]->outputLayerCompositionState.dataspace = ui::Dataspace::BT2020_PQ;
+ mTestLayers[2]->layerState->update(&mTestLayers[2]->outputLayer);
+
+ const std::vector<const LayerState*> layers = {
+ layerState1.get(),
+ layerState2.get(),
+ layerState3.get(),
+ };
+
+ initializeFlattener(layers);
+
+ mTime += 200ms;
+ initializeOverrideBuffer(layers);
+ EXPECT_EQ(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+
+ // This will render a CachedSet.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).WillOnce(Return(NO_ERROR));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ // We've rendered a CachedSet, but we haven't merged it in.
+ EXPECT_EQ(nullptr, overrideBuffer1);
+ EXPECT_EQ(nullptr, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+
+ // This time we merge the CachedSet in, so we have a new hash, and we should
+ // only have two sets.
+ EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _, _)).Times(0);
+ initializeOverrideBuffer(layers);
+ EXPECT_NE(getNonBufferHash(layers),
+ mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime));
+ mFlattener->renderCachedSets(mOutputState, std::nullopt);
+
+ EXPECT_NE(nullptr, overrideBuffer1);
+ EXPECT_EQ(overrideBuffer1, overrideBuffer2);
+ EXPECT_EQ(nullptr, overrideBuffer3);
+}
+
} // namespace
} // namespace android::compositionengine
diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
index 15ecf13..139f91f 100644
--- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h
+++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h
@@ -96,7 +96,7 @@
// The various thresholds for App and SF. If the actual timestamp falls within the threshold
// compared to prediction, we treat it as on time.
nsecs_t presentThreshold = std::chrono::duration_cast<std::chrono::nanoseconds>(2ms).count();
- nsecs_t deadlineThreshold = std::chrono::duration_cast<std::chrono::nanoseconds>(2ms).count();
+ nsecs_t deadlineThreshold = std::chrono::duration_cast<std::chrono::nanoseconds>(0ms).count();
nsecs_t startThreshold = std::chrono::duration_cast<std::chrono::nanoseconds>(2ms).count();
};
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index 4f47ed8..0334d70 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -889,6 +889,7 @@
const auto& deviceMin = *mMinSupportedRefreshRate;
const auto& minByPolicy = getMinRefreshRateByPolicyLocked();
const auto& maxByPolicy = getMaxRefreshRateByPolicyLocked();
+ const auto& currentPolicy = getCurrentPolicyLocked();
// Kernel idle timer will set the refresh rate to the device min. If DisplayManager says that
// the min allowed refresh rate is higher than the device min, we do not want to enable the
@@ -897,6 +898,10 @@
return RefreshRateConfigs::KernelIdleTimerAction::TurnOff;
}
if (minByPolicy == maxByPolicy) {
+ // when min primary range in display manager policy is below device min turn on the timer.
+ if (currentPolicy->primaryRange.min.lessThanWithMargin(deviceMin.getFps())) {
+ return RefreshRateConfigs::KernelIdleTimerAction::TurnOn;
+ }
return RefreshRateConfigs::KernelIdleTimerAction::TurnOff;
}
// Turn on the timer in all other cases.
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index 0358e8c..dfd1395 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -326,8 +326,7 @@
return mRefreshRates.size() > 1;
}
- // Class to enumerate options around toggling the kernel timer on and off. We have an option
- // for no change to avoid extra calls to kernel.
+ // Class to enumerate options around toggling the kernel timer on and off.
enum class KernelIdleTimerAction {
TurnOff, // Turn off the idle timer.
TurnOn // Turn on the idle timer.
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index fdf16a7..6af69f0 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -237,7 +237,8 @@
handle->previousReleaseFence,
handle->transformHint,
handle->currentMaxAcquiredBufferCount,
- eventStats, jankData, handle->previousBufferId);
+ eventStats, jankData,
+ handle->previousReleaseCallbackId);
}
return NO_ERROR;
}
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h
index 444bec6..6f4d812 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -51,7 +51,7 @@
nsecs_t refreshStartTime = 0;
nsecs_t dequeueReadyTime = 0;
uint64_t frameNumber = 0;
- uint64_t previousBufferId = 0;
+ ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID;
};
class TransactionCallbackInvoker {
diff --git a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
index 5aa809d..579a26e 100644
--- a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
+++ b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
@@ -29,7 +29,7 @@
// b/181132765 - disabled until cuttlefish failures are investigated
class ReleaseBufferCallbackHelper {
public:
- static void function(void* callbackContext, uint64_t graphicsBufferId,
+ static void function(void* callbackContext, ReleaseCallbackId callbackId,
const sp<Fence>& releaseFence,
uint32_t /*currentMaxAcquiredBufferCount*/) {
if (!callbackContext) {
@@ -38,11 +38,11 @@
ReleaseBufferCallbackHelper* helper =
static_cast<ReleaseBufferCallbackHelper*>(callbackContext);
std::lock_guard lock(helper->mMutex);
- helper->mCallbackDataQueue.emplace(graphicsBufferId, releaseFence);
+ helper->mCallbackDataQueue.emplace(callbackId, releaseFence);
helper->mConditionVariable.notify_all();
}
- void getCallbackData(uint64_t* bufferId) {
+ void getCallbackData(ReleaseCallbackId* callbackId) {
std::unique_lock lock(mMutex);
if (mCallbackDataQueue.empty()) {
if (!mConditionVariable.wait_for(lock, std::chrono::seconds(3),
@@ -53,7 +53,7 @@
auto callbackData = mCallbackDataQueue.front();
mCallbackDataQueue.pop();
- *bufferId = callbackData.first;
+ *callbackId = callbackData.first;
}
void verifyNoCallbacks() {
@@ -72,7 +72,7 @@
std::mutex mMutex;
std::condition_variable mConditionVariable;
- std::queue<std::pair<uint64_t, sp<Fence>>> mCallbackDataQueue;
+ std::queue<std::pair<ReleaseCallbackId, sp<Fence>>> mCallbackDataQueue;
};
class ReleaseBufferCallbackTest : public LayerTransactionTest {
@@ -82,10 +82,11 @@
}
static void submitBuffer(const sp<SurfaceControl>& layer, sp<GraphicBuffer> buffer,
- sp<Fence> fence, CallbackHelper& callback,
+ sp<Fence> fence, CallbackHelper& callback, const ReleaseCallbackId& id,
ReleaseBufferCallbackHelper& releaseCallback) {
Transaction t;
- t.setBuffer(layer, buffer, releaseCallback.getCallback());
+ t.setFrameNumber(layer, id.framenumber);
+ t.setBuffer(layer, buffer, id, releaseCallback.getCallback());
t.setAcquireFence(layer, fence);
t.addTransactionCompletedCallback(callback.function, callback.getContext());
t.apply();
@@ -98,10 +99,10 @@
}
static void waitForReleaseBufferCallback(ReleaseBufferCallbackHelper& releaseCallback,
- uint64_t expectedReleaseBufferId) {
- uint64_t actualReleaseBufferId;
+ const ReleaseCallbackId& expectedCallbackId) {
+ ReleaseCallbackId actualReleaseBufferId;
releaseCallback.getCallbackData(&actualReleaseBufferId);
- EXPECT_EQ(expectedReleaseBufferId, actualReleaseBufferId);
+ EXPECT_EQ(expectedCallbackId, actualReleaseBufferId);
releaseCallback.verifyNoCallbacks();
}
static ReleaseBufferCallbackHelper* getReleaseBufferCallbackHelper() {
@@ -116,6 +117,10 @@
BufferUsage::COMPOSER_OVERLAY,
"test");
}
+ static uint64_t generateFrameNumber() {
+ static uint64_t sFrameNumber = 0;
+ return ++sFrameNumber;
+ }
};
TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) {
@@ -125,7 +130,9 @@
// If a buffer is being presented, we should not emit a release callback.
sp<GraphicBuffer> firstBuffer = getBuffer();
- submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+ ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, firstBufferCallbackId,
+ *releaseCallback);
ExpectedResult expected;
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
ExpectedResult::Buffer::NOT_ACQUIRED);
@@ -143,13 +150,15 @@
// If a presented buffer is replaced, we should emit a release callback for the
// previously presented buffer.
sp<GraphicBuffer> secondBuffer = getBuffer();
- submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+ ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, secondBufferCallbackId,
+ *releaseCallback);
expected = ExpectedResult();
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
ExpectedResult::Buffer::NOT_ACQUIRED,
ExpectedResult::PreviousBuffer::RELEASED);
ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
- ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+ ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
}
TEST_F(ReleaseBufferCallbackTest, DISABLED_OffScreenLayer) {
@@ -160,7 +169,9 @@
// If a buffer is being presented, we should not emit a release callback.
sp<GraphicBuffer> firstBuffer = getBuffer();
- submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+ ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, firstBufferCallbackId,
+ *releaseCallback);
ExpectedResult expected;
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
ExpectedResult::Buffer::NOT_ACQUIRED);
@@ -184,23 +195,27 @@
// If a presented buffer is replaced, we should emit a release callback for the
// previously presented buffer.
sp<GraphicBuffer> secondBuffer = getBuffer();
- submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+ ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, secondBufferCallbackId,
+ *releaseCallback);
expected = ExpectedResult();
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
ExpectedResult::Buffer::NOT_ACQUIRED,
ExpectedResult::PreviousBuffer::NOT_RELEASED);
ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
- ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+ ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
// If continue to submit buffer we continue to get release callbacks
sp<GraphicBuffer> thirdBuffer = getBuffer();
- submitBuffer(layer, thirdBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+ ReleaseCallbackId thirdBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, thirdBuffer, Fence::NO_FENCE, transactionCallback, thirdBufferCallbackId,
+ *releaseCallback);
expected = ExpectedResult();
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
ExpectedResult::Buffer::NOT_ACQUIRED,
ExpectedResult::PreviousBuffer::NOT_RELEASED);
ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
- ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBuffer->getId()));
+ ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBufferCallbackId));
}
TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_layerdestroy) {
@@ -210,7 +225,9 @@
// If a buffer is being presented, we should not emit a release callback.
sp<GraphicBuffer> firstBuffer = getBuffer();
- submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback);
+ ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, firstBufferCallbackId,
+ *releaseCallback);
{
ExpectedResult expected;
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
@@ -225,7 +242,7 @@
t.apply();
layer = nullptr;
- ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+ ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
}
// Destroying a never presented layer emits a callback.
@@ -242,7 +259,9 @@
// Submitting a buffer does not emit a callback.
sp<GraphicBuffer> firstBuffer = getBuffer();
- submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback);
+ ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, firstBufferCallbackId,
+ *releaseCallback);
{
ExpectedResult expected;
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
@@ -253,19 +272,21 @@
// Submitting a second buffer will replace the drawing state buffer and emit a callback.
sp<GraphicBuffer> secondBuffer = getBuffer();
- submitBuffer(layer, secondBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback);
+ ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+ submitBuffer(layer, secondBuffer, Fence::NO_FENCE, *transactionCallback, secondBufferCallbackId,
+ *releaseCallback);
{
ExpectedResult expected;
expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
ExpectedResult::Buffer::NOT_ACQUIRED);
ASSERT_NO_FATAL_FAILURE(waitForCallback(*transactionCallback, expected));
ASSERT_NO_FATAL_FAILURE(
- waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+ waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
}
// Destroying the offscreen layer emits a callback.
layer = nullptr;
- ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBuffer->getId()));
+ ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBufferCallbackId));
}
TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) {
@@ -275,12 +296,13 @@
// If a buffer is being presented, we should not emit a release callback.
sp<GraphicBuffer> firstBuffer = getBuffer();
+ ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
// Try to present 100ms in the future
nsecs_t time = systemTime() + std::chrono::nanoseconds(100ms).count();
Transaction t;
- t.setBuffer(layer, firstBuffer, releaseCallback->getCallback());
+ t.setBuffer(layer, firstBuffer, firstBufferCallbackId, releaseCallback->getCallback());
t.setAcquireFence(layer, Fence::NO_FENCE);
t.addTransactionCompletedCallback(transactionCallback.function,
transactionCallback.getContext());
@@ -295,7 +317,8 @@
// Dropping frames in transaction queue emits a callback
sp<GraphicBuffer> secondBuffer = getBuffer();
- t.setBuffer(layer, secondBuffer, releaseCallback->getCallback());
+ ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+ t.setBuffer(layer, secondBuffer, secondBufferCallbackId, releaseCallback->getCallback());
t.setAcquireFence(layer, Fence::NO_FENCE);
t.addTransactionCompletedCallback(transactionCallback.function,
transactionCallback.getContext());
@@ -307,7 +330,7 @@
ExpectedResult::Buffer::NOT_ACQUIRED,
ExpectedResult::PreviousBuffer::RELEASED);
ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
- ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+ ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
}
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
index 0a8c748..97b60e0 100644
--- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp
@@ -160,7 +160,7 @@
size_t maxTokens;
static constexpr pid_t kSurfaceFlingerPid = 666;
static constexpr nsecs_t kPresentThreshold = std::chrono::nanoseconds(2ns).count();
- static constexpr nsecs_t kDeadlineThreshold = std::chrono::nanoseconds(2ns).count();
+ static constexpr nsecs_t kDeadlineThreshold = std::chrono::nanoseconds(0ns).count();
static constexpr nsecs_t kStartThreshold = std::chrono::nanoseconds(2ns).count();
static constexpr JankClassificationThresholds kTestThresholds{kPresentThreshold,
kDeadlineThreshold,
diff --git a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
index 9bb5ca1..3423bd5 100644
--- a/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/RefreshRateConfigsTest.cpp
@@ -2066,6 +2066,35 @@
EXPECT_EQ(KernelIdleTimerAction::TurnOff, refreshRateConfigs->getIdleTimerAction());
}
+TEST_F(RefreshRateConfigsTest, testKernelIdleTimerActionFor120Hz) {
+ using KernelIdleTimerAction = scheduler::RefreshRateConfigs::KernelIdleTimerAction;
+
+ // Tests with 120Hz
+ auto refreshRateConfigs =
+ std::make_unique<RefreshRateConfigs>(m60_120Device,
+ /*currentConfigId=*/HWC_CONFIG_ID_120);
+ // SetPolicy(0, 60), current 60Hz => TurnOn.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy({HWC_CONFIG_ID_60, {Fps(0), Fps(60)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOn, refreshRateConfigs->getIdleTimerAction());
+
+ // SetPolicy(60, 60), current 60Hz => TurnOff.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy({HWC_CONFIG_ID_60, {Fps(60), Fps(60)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOff, refreshRateConfigs->getIdleTimerAction());
+
+ // SetPolicy(60, 120), current 60Hz => TurnOn.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy({HWC_CONFIG_ID_60, {Fps(60), Fps(120)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOn, refreshRateConfigs->getIdleTimerAction());
+
+ // SetPolicy(120, 120), current 120Hz => TurnOff.
+ ASSERT_GE(refreshRateConfigs->setDisplayManagerPolicy(
+ {HWC_CONFIG_ID_120, {Fps(120), Fps(120)}}),
+ 0);
+ EXPECT_EQ(KernelIdleTimerAction::TurnOff, refreshRateConfigs->getIdleTimerAction());
+}
+
TEST_F(RefreshRateConfigsTest, getFrameRateDivider) {
auto refreshRateConfigs =
std::make_unique<RefreshRateConfigs>(m30_60_72_90_120Device,