SF: pass acquire fence on BLAST callbacks
When latching unsignaled buffers, the acquire fence is
not signaled by the time BLAST callback is invoked. In
that case pass a fence instead. For latch signaled, we still
pass the acquire time itself to avoid sending file descriptors
over binder.
Bug: 198190384
Test: SF unit tests
Change-Id: Ic7ad9b603b60dbf46a62eaf6b76bfbdeeebf6cec
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index aa7ebc9..f7392d4 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -111,7 +111,14 @@
status_t SurfaceStats::writeToParcel(Parcel* output) const {
SAFE_PARCEL(output->writeStrongBinder, surfaceControl);
- SAFE_PARCEL(output->writeInt64, acquireTime);
+ if (const auto* acquireFence = std::get_if<sp<Fence>>(&acquireTimeOrFence)) {
+ SAFE_PARCEL(output->writeBool, true);
+ SAFE_PARCEL(output->write, **acquireFence);
+ } else {
+ SAFE_PARCEL(output->writeBool, false);
+ SAFE_PARCEL(output->writeInt64, std::get<nsecs_t>(acquireTimeOrFence));
+ }
+
if (previousReleaseFence) {
SAFE_PARCEL(output->writeBool, true);
SAFE_PARCEL(output->write, *previousReleaseFence);
@@ -131,10 +138,20 @@
status_t SurfaceStats::readFromParcel(const Parcel* input) {
SAFE_PARCEL(input->readStrongBinder, &surfaceControl);
- SAFE_PARCEL(input->readInt64, &acquireTime);
+
bool hasFence = false;
SAFE_PARCEL(input->readBool, &hasFence);
if (hasFence) {
+ acquireTimeOrFence = sp<Fence>::make();
+ SAFE_PARCEL(input->read, *std::get<sp<Fence>>(acquireTimeOrFence));
+ } else {
+ nsecs_t acquireTime;
+ SAFE_PARCEL(input->readInt64, &acquireTime);
+ acquireTimeOrFence = acquireTime;
+ }
+
+ SAFE_PARCEL(input->readBool, &hasFence);
+ if (hasFence) {
previousReleaseFence = new Fence();
SAFE_PARCEL(input->read, *previousReleaseFence);
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 91b2fb1..f78726c 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -296,7 +296,7 @@
surfaceControlStats
.emplace_back(callbacksMap[callbackId]
.surfaceControls[surfaceStats.surfaceControl],
- transactionStats.latchTime, surfaceStats.acquireTime,
+ transactionStats.latchTime, surfaceStats.acquireTimeOrFence,
transactionStats.presentFence,
surfaceStats.previousReleaseFence, surfaceStats.transformHint,
surfaceStats.eventStats);
@@ -321,7 +321,7 @@
surfaceControlStats
.emplace_back(callbacksMap[callbackId]
.surfaceControls[surfaceStats.surfaceControl],
- transactionStats.latchTime, surfaceStats.acquireTime,
+ transactionStats.latchTime, surfaceStats.acquireTimeOrFence,
transactionStats.presentFence,
surfaceStats.previousReleaseFence, surfaceStats.transformHint,
surfaceStats.eventStats);
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index 0df5822..a791c66 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -30,6 +30,7 @@
#include <cstdint>
#include <unordered_map>
#include <unordered_set>
+#include <variant>
namespace android {
@@ -130,12 +131,12 @@
status_t readFromParcel(const Parcel* input) override;
SurfaceStats() = default;
- SurfaceStats(const sp<IBinder>& sc, nsecs_t time, const sp<Fence>& prevReleaseFence,
- uint32_t hint, uint32_t currentMaxAcquiredBuffersCount,
- FrameEventHistoryStats frameEventStats, std::vector<JankData> jankData,
- ReleaseCallbackId previousReleaseCallbackId)
+ SurfaceStats(const sp<IBinder>& sc, std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence,
+ const sp<Fence>& prevReleaseFence, uint32_t hint,
+ uint32_t currentMaxAcquiredBuffersCount, FrameEventHistoryStats frameEventStats,
+ std::vector<JankData> jankData, ReleaseCallbackId previousReleaseCallbackId)
: surfaceControl(sc),
- acquireTime(time),
+ acquireTimeOrFence(std::move(acquireTimeOrFence)),
previousReleaseFence(prevReleaseFence),
transformHint(hint),
currentMaxAcquiredBufferCount(currentMaxAcquiredBuffersCount),
@@ -144,7 +145,7 @@
previousReleaseCallbackId(previousReleaseCallbackId) {}
sp<IBinder> surfaceControl;
- nsecs_t acquireTime = -1;
+ std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence = -1;
sp<Fence> previousReleaseFence;
uint32_t transformHint = 0;
uint32_t currentMaxAcquiredBufferCount = 0;
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 61eeab3..731ac65 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -58,12 +58,13 @@
using gui::IRegionSamplingListener;
struct SurfaceControlStats {
- SurfaceControlStats(const sp<SurfaceControl>& sc, nsecs_t latchTime, nsecs_t acquireTime,
+ SurfaceControlStats(const sp<SurfaceControl>& sc, nsecs_t latchTime,
+ std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence,
const sp<Fence>& presentFence, const sp<Fence>& prevReleaseFence,
uint32_t hint, FrameEventHistoryStats eventStats)
: surfaceControl(sc),
latchTime(latchTime),
- acquireTime(acquireTime),
+ acquireTimeOrFence(std::move(acquireTimeOrFence)),
presentFence(presentFence),
previousReleaseFence(prevReleaseFence),
transformHint(hint),
@@ -71,7 +72,7 @@
sp<SurfaceControl> surfaceControl;
nsecs_t latchTime = -1;
- nsecs_t acquireTime = -1;
+ std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence = -1;
sp<Fence> presentFence;
sp<Fence> previousReleaseFence;
uint32_t transformHint = 0;
diff --git a/libs/gui/include/gui/test/CallbackUtils.h b/libs/gui/include/gui/test/CallbackUtils.h
index 6403208..62d1496 100644
--- a/libs/gui/include/gui/test/CallbackUtils.h
+++ b/libs/gui/include/gui/test/CallbackUtils.h
@@ -134,12 +134,15 @@
void verifySurfaceControlStats(const SurfaceControlStats& surfaceControlStats,
nsecs_t latchTime) const {
- const auto& [surfaceControl, latch, acquireTime, presentFence, previousReleaseFence,
- transformHint, frameEvents] = surfaceControlStats;
+ const auto& [surfaceControl, latch, acquireTimeOrFence, presentFence,
+ previousReleaseFence, transformHint, frameEvents] = surfaceControlStats;
- ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED)
+ ASSERT_TRUE(std::holds_alternative<nsecs_t>(acquireTimeOrFence));
+ ASSERT_EQ(std::get<nsecs_t>(acquireTimeOrFence) > 0,
+ mBufferResult == ExpectedResult::Buffer::ACQUIRED)
<< "bad acquire time";
- ASSERT_LE(acquireTime, latchTime) << "acquire time should be <= latch time";
+ ASSERT_LE(std::get<nsecs_t>(acquireTimeOrFence), latchTime)
+ << "acquire time should be <= latch time";
if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::RELEASED) {
ASSERT_NE(previousReleaseFence, nullptr)
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index fccd8f1..02e444d 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -416,8 +416,14 @@
? bufferData.acquireFence
: Fence::NO_FENCE;
mDrawingState.acquireFenceTime = std::make_unique<FenceTime>(mDrawingState.acquireFence);
- // The acquire fences of BufferStateLayers have already signaled before they are set
- mCallbackHandleAcquireTime = mDrawingState.acquireFenceTime->getSignalTime();
+ if (mDrawingState.acquireFenceTime->getSignalTime() == Fence::SIGNAL_TIME_PENDING) {
+ // We latched this buffer unsiganled, so we need to pass the acquire fence
+ // on the callback instead of just the acquire time, since it's unknown at
+ // this point.
+ mCallbackHandleAcquireTimeOrFence = mDrawingState.acquireFence;
+ } else {
+ mCallbackHandleAcquireTimeOrFence = mDrawingState.acquireFenceTime->getSignalTime();
+ }
mDrawingState.modified = true;
setTransactionFlags(eTransactionNeeded);
@@ -527,7 +533,7 @@
// If this layer will be presented in this frame
if (willPresent) {
// If this transaction set an acquire fence on this layer, set its acquire time
- handle->acquireTime = mCallbackHandleAcquireTime;
+ handle->acquireTimeOrFence = mCallbackHandleAcquireTimeOrFence;
handle->frameNumber = mDrawingState.frameNumber;
// Store so latched time and release fence can be set
@@ -540,7 +546,7 @@
}
mReleasePreviousBuffer = false;
- mCallbackHandleAcquireTime = -1;
+ mCallbackHandleAcquireTimeOrFence = -1;
return willPresent;
}
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 4256212..669eaad 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -145,7 +145,7 @@
// Stores the last set acquire fence signal time used to populate the callback handle's acquire
// time.
- nsecs_t mCallbackHandleAcquireTime = -1;
+ std::variant<nsecs_t, sp<Fence>> mCallbackHandleAcquireTimeOrFence = -1;
std::deque<std::shared_ptr<android::frametimeline::SurfaceFrame>> mPendingJankClassifications;
// An upper bound on the number of SurfaceFrames in the pending classifications deque.
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index b705d9c..e1f348f 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -167,7 +167,7 @@
handle->gpuCompositionDoneFence->getSnapshot().fence,
handle->compositorTiming, handle->refreshStartTime,
handle->dequeueReadyTime);
- transactionStats->surfaceStats.emplace_back(surfaceControl, handle->acquireTime,
+ transactionStats->surfaceStats.emplace_back(surfaceControl, handle->acquireTimeOrFence,
handle->previousReleaseFence,
handle->transformHint,
handle->currentMaxAcquiredBufferCount,
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h
index 5ef5475..a68cd87 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -47,7 +47,7 @@
std::string name;
sp<Fence> previousReleaseFence;
std::vector<std::shared_future<renderengine::RenderEngineResult>> previousReleaseFences;
- nsecs_t acquireTime = -1;
+ std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence = -1;
nsecs_t latchTime = -1;
uint32_t transformHint = 0;
uint32_t currentMaxAcquiredBufferCount = 0;