Correctly pass screenshot fences to transaction callbacks

In some instances, a screenshot may be captured before a layer has a
release callback registered. This can happen when a new buffer has
not yet been transacted before a screenshot is captured. This causes the
screenshot fence to be dropped and the possibility of tearing when
capturing a screenshot while continuously rendering.

To resolve this, buffer screenshot fences into a list of future fences
when there is no callback registered, and merge those fences when
dispatching the release callback.

Bug: 302703346
Test: SurfaceViewTests#testMovingWhiteSurfaceView 100 times
Change-Id: I91aec3cdb0973092d48cd77e59dd3999e9d9e847

1 files changed, 2 insertions, 26 deletions

diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index 6a155c17df..7b5298c82e 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -25,6 +25,7 @@
 
 #include "TransactionCallbackInvoker.h"
 #include "BackgroundExecutor.h"
+#include "Utils/FenceUtils.h"
 
 #include <cinttypes>
 
@@ -127,33 +128,8 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>&
     sp<IBinder> surfaceControl = handle->surfaceControl.promote();
     if (surfaceControl) {
         sp<Fence> prevFence = nullptr;
-
         for (const auto& future : handle->previousReleaseFences) {
-            sp<Fence> currentFence = future.get().value_or(Fence::NO_FENCE);
-            if (prevFence == nullptr && currentFence->getStatus() != Fence::Status::Invalid) {
-                prevFence = std::move(currentFence);
-            } else if (prevFence != nullptr) {
-                // If both fences are signaled or both are unsignaled, we need to merge
-                // them to get an accurate timestamp.
-                if (prevFence->getStatus() != Fence::Status::Invalid &&
-                    prevFence->getStatus() == currentFence->getStatus()) {
-                    char fenceName[32] = {};
-                    snprintf(fenceName, 32, "%.28s", handle->name.c_str());
-                    sp<Fence> mergedFence = Fence::merge(fenceName, prevFence, currentFence);
-                    if (mergedFence->isValid()) {
-                        prevFence = std::move(mergedFence);
-                    }
-                } else if (currentFence->getStatus() == Fence::Status::Unsignaled) {
-                    // If one fence has signaled and the other hasn't, the unsignaled
-                    // fence will approximately correspond with the correct timestamp.
-                    // There's a small race if both fences signal at about the same time
-                    // and their statuses are retrieved with unfortunate timing. However,
-                    // by this point, they will have both signaled and only the timestamp
-                    // will be slightly off; any dependencies after this point will
-                    // already have been met.
-                    prevFence = std::move(currentFence);
-                }
-            }
+            mergeFence(handle->name.c_str(), future.get().value_or(Fence::NO_FENCE), prevFence);
         }
         handle->previousReleaseFence = prevFence;
         handle->previousReleaseFences.clear();