From 481ccbaedf364822f5ba4cc7ab299717d4796cb6 Mon Sep 17 00:00:00 2001
From: chaviw <chaviw@google.com>
Date: Thu, 24 Mar 2022 16:15:38 -0500
Subject: [PATCH] Clean up buffer sync logic in SurfaceView.

The current code is a bit confuisng to read so cleaned up the code that
handles syncing a buffer when a VRI relayout request creates a sync.

Test: YT, Camera, Chrome
Bug: 200284684
Change-Id: I243ee5ebcb59c478c48461544813cab43a2678d4
---
 core/java/android/view/SurfaceView.java | 64 ++++++++++++++-----------
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index c04b0964a7a43..ed57136b1d35e 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -966,13 +966,16 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
 
                 final boolean redrawNeeded = sizeChanged || creating || hintChanged
                         || (mVisible && !mDrawFinished);
-                final TransactionCallback transactionCallback =
-                        redrawNeeded ? new TransactionCallback() : null;
-                if (redrawNeeded && viewRoot.wasRelayoutRequested() && viewRoot.isInSync()) {
+                boolean shouldSyncBuffer =
+                        redrawNeeded && viewRoot.wasRelayoutRequested() && viewRoot.isInSync();
+                SyncBufferTransactionCallback syncBufferTransactionCallback = null;
+                if (shouldSyncBuffer) {
+                    syncBufferTransactionCallback = new SyncBufferTransactionCallback();
                     mBlastBufferQueue.syncNextTransaction(
                             false /* acquireSingleBuffer */,
-                            transactionCallback::onTransactionReady);
+                            syncBufferTransactionCallback::onTransactionReady);
                 }
+
                 final boolean realSizeChanged = performSurfaceTransaction(viewRoot,
                         translator, creating, sizeChanged, hintChanged, surfaceUpdateTransaction);
 
@@ -1011,7 +1014,18 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                             }
                         }
                         if (redrawNeeded) {
-                            redrawNeeded(callbacks, transactionCallback);
+                            if (DEBUG) {
+                                Log.i(TAG, System.identityHashCode(this) + " surfaceRedrawNeeded");
+                            }
+                            if (callbacks == null) {
+                                callbacks = getSurfaceCallbacks();
+                            }
+
+                            if (shouldSyncBuffer) {
+                                handleSyncBufferCallback(callbacks, syncBufferTransactionCallback);
+                            } else {
+                                redrawNeededAsync(callbacks, this::onDrawFinished);
+                            }
                         }
                     }
                 } finally {
@@ -1030,38 +1044,30 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
         }
     }
 
-    private void redrawNeeded(SurfaceHolder.Callback[] callbacks,
-            @Nullable TransactionCallback transactionCallback) {
-        if (DEBUG) {
-            Log.i(TAG, System.identityHashCode(this) + " surfaceRedrawNeeded");
-        }
-        final SurfaceHolder.Callback[] capturedCallbacks =
-                callbacks == null ? getSurfaceCallbacks() : callbacks;
+    /**
+     * If SV is trying to be part of the VRI sync, we need to add SV to the VRI sync before
+     * invoking the redrawNeeded call to the owner. This is to ensure we can set up the SV in
+     * the sync before the SV owner knows it needs to draw a new frame.
+     * Once the redrawNeeded callback is invoked, we can stop the continuous sync transaction
+     * call which will invoke the syncTransaction callback that contains the buffer. The
+     * code waits until we can retrieve the transaction that contains the buffer before
+     * notifying the syncer that the buffer is ready.
+     */
+    private void handleSyncBufferCallback(SurfaceHolder.Callback[] callbacks,
+            SyncBufferTransactionCallback syncBufferTransactionCallback) {
 
-        ViewRootImpl viewRoot = getViewRootImpl();
-        boolean isVriSync = viewRoot.addToSync(syncBufferCallback ->
-                redrawNeededAsync(capturedCallbacks, () -> {
+        getViewRootImpl().addToSync(syncBufferCallback ->
+                redrawNeededAsync(callbacks, () -> {
+                    Transaction t = null;
                     if (mBlastBufferQueue != null) {
                         mBlastBufferQueue.stopContinuousSyncTransaction();
+                        t = syncBufferTransactionCallback.waitForTransaction();
                     }
 
-                    Transaction t = null;
-                    if (transactionCallback != null && mBlastBufferQueue != null) {
-                        t = transactionCallback.waitForTransaction();
-                    }
-                    // If relayout was requested, then a callback from BBQ will
-                    // be invoked with the sync transaction. onDrawFinished will be
-                    // called in there
                     syncBufferCallback.onBufferReady(t);
                     onDrawFinished();
                 }));
 
-        // If isVriSync, then everything was setup in the addToSync.
-        if (isVriSync) {
-            return;
-        }
-
-        redrawNeededAsync(capturedCallbacks, this::onDrawFinished);
     }
 
     private void redrawNeededAsync(SurfaceHolder.Callback[] callbacks,
@@ -1070,7 +1076,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
         sch.dispatchSurfaceRedrawNeededAsync(mSurfaceHolder, callbacks);
     }
 
-    private static class TransactionCallback {
+    private static class SyncBufferTransactionCallback {
         private final CountDownLatch mCountDownLatch = new CountDownLatch(1);
         private Transaction mTransaction;