SF: Avoid updating clients with stale or incorrect transform hints
When the layer is removed from a display or the display the layer is on
is turned off, the client will continue to receive transform hint
updates via the transaction complete callback using the default/active
displays install orientation. Once the layer is back on the display and
it does not submit a new frame, a buffer with a suboptimal transform
may remain on display.
Fix this by not reporting stale/incorrect values via the callback. Once
the layer is reparent back to the display and the display state is not
OFF, it will continue to get hints via the callback.
For special cases where we want the app to draw its first frame before
the display is available, we rely on WMS and DMS to provide the right
information so the client can calculate the hint.
Bug: 251360251
Test: move app between displays, rotate, check final buffer transforms
Change-Id: I0a9abac7e9cf4ade1c49ec400e73b634c8269b4b
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 8b9a878..7fcb8e8 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -334,9 +334,11 @@
std::optional<SurfaceControlStats> statsOptional = findMatchingStat(stats, pendingSC);
if (statsOptional) {
SurfaceControlStats stat = *statsOptional;
- mTransformHint = stat.transformHint;
- mBufferItemConsumer->setTransformHint(mTransformHint);
- BQA_LOGV("updated mTransformHint=%d", mTransformHint);
+ if (stat.transformHint) {
+ mTransformHint = *stat.transformHint;
+ mBufferItemConsumer->setTransformHint(mTransformHint);
+ BQA_LOGV("updated mTransformHint=%d", mTransformHint);
+ }
// Update frametime stamps if the frame was latched and presented, indicated by a
// valid latch time.
if (stat.latchTime > 0) {
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index e4b8bad..712aefd 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -17,6 +17,9 @@
#define LOG_TAG "ITransactionCompletedListener"
//#define LOG_NDEBUG 0
+#include <cstdint>
+#include <optional>
+
#include <gui/ISurfaceComposer.h>
#include <gui/ITransactionCompletedListener.h>
#include <gui/LayerState.h>
@@ -126,7 +129,12 @@
} else {
SAFE_PARCEL(output->writeBool, false);
}
- SAFE_PARCEL(output->writeUint32, transformHint);
+
+ SAFE_PARCEL(output->writeBool, transformHint.has_value());
+ if (transformHint.has_value()) {
+ output->writeUint32(transformHint.value());
+ }
+
SAFE_PARCEL(output->writeUint32, currentMaxAcquiredBufferCount);
SAFE_PARCEL(output->writeParcelable, eventStats);
SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(jankData.size()));
@@ -156,7 +164,16 @@
previousReleaseFence = new Fence();
SAFE_PARCEL(input->read, *previousReleaseFence);
}
- SAFE_PARCEL(input->readUint32, &transformHint);
+ bool hasTransformHint = false;
+ SAFE_PARCEL(input->readBool, &hasTransformHint);
+ if (hasTransformHint) {
+ uint32_t tempTransformHint;
+ SAFE_PARCEL(input->readUint32, &tempTransformHint);
+ transformHint = std::make_optional(tempTransformHint);
+ } else {
+ transformHint = std::nullopt;
+ }
+
SAFE_PARCEL(input->readUint32, ¤tMaxAcquiredBufferCount);
SAFE_PARCEL(input->readParcelable, &eventStats);
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 81b4d3d..a957059 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -385,10 +385,11 @@
surfaceStats.previousReleaseFence, surfaceStats.transformHint,
surfaceStats.eventStats,
surfaceStats.currentMaxAcquiredBufferCount);
- if (callbacksMap[callbackId].surfaceControls[surfaceStats.surfaceControl]) {
+ if (callbacksMap[callbackId].surfaceControls[surfaceStats.surfaceControl] &&
+ surfaceStats.transformHint.has_value()) {
callbacksMap[callbackId]
.surfaceControls[surfaceStats.surfaceControl]
- ->setTransformHint(surfaceStats.transformHint);
+ ->setTransformHint(*surfaceStats.transformHint);
}
// If there is buffer id set, we look up any pending client release buffer callbacks
// and call them. This is a performance optimization when we have a transaction
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index cc136bb..162d3d3 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -132,7 +132,7 @@
SurfaceStats() = default;
SurfaceStats(const sp<IBinder>& sc, std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence,
- const sp<Fence>& prevReleaseFence, uint32_t hint,
+ const sp<Fence>& prevReleaseFence, std::optional<uint32_t> hint,
uint32_t currentMaxAcquiredBuffersCount, FrameEventHistoryStats frameEventStats,
std::vector<JankData> jankData, ReleaseCallbackId previousReleaseCallbackId)
: surfaceControl(sc),
@@ -147,7 +147,7 @@
sp<IBinder> surfaceControl;
std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence = -1;
sp<Fence> previousReleaseFence;
- uint32_t transformHint = 0;
+ std::optional<uint32_t> transformHint = 0;
uint32_t currentMaxAcquiredBufferCount = 0;
FrameEventHistoryStats eventStats;
std::vector<JankData> jankData;
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index d138d68..36dc3b7 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -69,7 +69,7 @@
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,
+ std::optional<uint32_t> hint, FrameEventHistoryStats eventStats,
uint32_t currentMaxAcquiredBufferCount)
: surfaceControl(sc),
latchTime(latchTime),
@@ -85,7 +85,7 @@
std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence = -1;
sp<Fence> presentFence;
sp<Fence> previousReleaseFence;
- uint32_t transformHint = 0;
+ std::optional<uint32_t> transformHint = 0;
FrameEventHistoryStats frameEventStats;
uint32_t currentMaxAcquiredBufferCount = 0;
};