Invoke commit callback after latch buffers
If commit callback is invoked before the buffers can be latched, they
won't be included in the commit callback and would only be invoked in
the complete callback. This defeats the purpose of the commit callback
as an early indicator that the buffer has been committed.
Move the commit callback invocation so it's after latchBuffers
This also fixes a bug where offscreen layers could be latched in
flushAndCommitTransactions, so only offscreen layers would get the
commit callback. This would cause issues if the offscreen layer
transaction was merged with a layer that was on screen, preventing the
on screen layer from getting a commit callback.
Test: LayerCallbackTest#CommitCallbackOffscreenLayer
Test: BLASTBufferQueueTest
Test: BLASTBufferQueueTransformTest
Test: BLASTFrameEventHistoryTest
Fixes: 205563588
Change-Id: I5692211dd7717258829be8eb8a68b2dcb4a5498d
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d0f2174..98f2107 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1971,9 +1971,34 @@
mFrameTimeline->setSfWakeUp(vsyncId, frameTime, Fps::fromPeriodNsecs(stats.vsyncPeriod));
- mustComposite |= flushAndCommitTransactions();
+ bool needsTraversal = false;
+ if (clearTransactionFlags(eTransactionFlushNeeded)) {
+ needsTraversal = flushTransactionQueues();
+ }
+
+ const bool shouldCommit =
+ (getTransactionFlags() & ~eTransactionFlushNeeded) || needsTraversal;
+ if (shouldCommit) {
+ commitTransactions();
+ }
+
+ if (transactionFlushNeeded()) {
+ setTransactionFlags(eTransactionFlushNeeded);
+ }
+
+ mustComposite |= shouldCommit;
mustComposite |= latchBuffers();
+ // This has to be called after latchBuffers because we want to include the layers that have
+ // been latched in the commit callback
+ if (!needsTraversal) {
+ // Invoke empty transaction callbacks early.
+ mTransactionCallbackInvoker.sendCallbacks(false /* onCommitOnly */);
+ } else {
+ // Invoke OnCommit callbacks.
+ mTransactionCallbackInvoker.sendCallbacks(true /* onCommitOnly */);
+ }
+
updateLayerGeometry();
if (tracePreComposition) {
@@ -2000,34 +2025,6 @@
return mustComposite && CC_LIKELY(mBootStage != BootStage::BOOTLOADER);
}
-bool SurfaceFlinger::flushAndCommitTransactions() {
- ATRACE_CALL();
-
- bool needsTraversal = false;
- if (clearTransactionFlags(eTransactionFlushNeeded)) {
- needsTraversal = flushTransactionQueues();
- }
-
- const bool shouldCommit = (getTransactionFlags() & ~eTransactionFlushNeeded) || needsTraversal;
- if (shouldCommit) {
- commitTransactions();
- }
-
- if (!needsTraversal) {
- // Invoke empty transaction callbacks early.
- mTransactionCallbackInvoker.sendCallbacks(false /* onCommitOnly */);
- } else {
- // Invoke OnCommit callbacks.
- mTransactionCallbackInvoker.sendCallbacks(true /* onCommitOnly */);
- }
-
- if (transactionFlushNeeded()) {
- setTransactionFlags(eTransactionFlushNeeded);
- }
-
- return shouldCommit;
-}
-
void SurfaceFlinger::composite(nsecs_t frameTime) {
ATRACE_CALL();