From edb6bdfac1965da4a6895d4110ec4a451fa62b8c Mon Sep 17 00:00:00 2001
From: Alec Mouri <alecmouri@google.com>
Date: Tue, 14 Mar 2023 20:43:12 +0000
Subject: [PATCH] Drive SurfaceView visibility on renderthread

In some scenarios, a SurfaceView:

1. May be moved offscreen by an app, such as through a ViewPager2
2. Have its RenderNode removed from the draw tree
3. The SurfaceView is remade visible on the UI thread

In this case, the view position is offscreen, but the SurfaceControl
position is at (0, 0). If there happens to be another SurfaceView at (0,
0), then this causes an accidental overlap.

To avoid this, visibility should be fully driven by RenderNode position
callbacks.

Bug: 269113414
Test: Test app with ViewPager2
Change-Id: Ia0abd56fe4be8b669d39864c5fc82e941fb3be8e
---
 core/java/android/view/SurfaceView.java | 32 +++++++++----------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index b46a68c1d5fde..d8b6b7b25a24f 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -851,10 +851,14 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
             }
             mParentSurfaceSequenceId = viewRoot.getSurfaceSequenceId();
 
-            if (mViewVisibility) {
-                surfaceUpdateTransaction.show(mSurfaceControl);
-            } else {
-                surfaceUpdateTransaction.hide(mSurfaceControl);
+            // Only control visibility if we're not hardware-accelerated. Otherwise we'll
+            // let renderthread drive since offscreen SurfaceControls should not be visible.
+            if (!isHardwareAccelerated()) {
+                if (mViewVisibility) {
+                    surfaceUpdateTransaction.show(mSurfaceControl);
+                } else {
+                    surfaceUpdateTransaction.hide(mSurfaceControl);
+                }
             }
 
             updateBackgroundVisibility(surfaceUpdateTransaction);
@@ -1417,12 +1421,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
     }
 
     private final Rect mRTLastReportedPosition = new Rect();
-    private final Point mRTLastReportedSurfaceSize = new Point();
 
     private class SurfaceViewPositionUpdateListener implements RenderNode.PositionUpdateListener {
         private final int mRtSurfaceWidth;
         private final int mRtSurfaceHeight;
-        private boolean mRtFirst = true;
         private final SurfaceControl.Transaction mPositionChangedTransaction =
                 new SurfaceControl.Transaction();
 
@@ -1433,15 +1435,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
 
         @Override
         public void positionChanged(long frameNumber, int left, int top, int right, int bottom) {
-            if (!mRtFirst && (mRTLastReportedPosition.left == left
-                    && mRTLastReportedPosition.top == top
-                    && mRTLastReportedPosition.right == right
-                    && mRTLastReportedPosition.bottom == bottom
-                    && mRTLastReportedSurfaceSize.x == mRtSurfaceWidth
-                    && mRTLastReportedSurfaceSize.y == mRtSurfaceHeight)) {
-                return;
-            }
-            mRtFirst = false;
             try {
                 if (DEBUG_POSITION) {
                     Log.d(TAG, String.format(
@@ -1452,8 +1445,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                 }
                 synchronized (mSurfaceControlLock) {
                     if (mSurfaceControl == null) return;
+
                     mRTLastReportedPosition.set(left, top, right, bottom);
-                    mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight);
                     onSetSurfacePositionAndScale(mPositionChangedTransaction, mSurfaceControl,
                             mRTLastReportedPosition.left /*positionLeft*/,
                             mRTLastReportedPosition.top /*positionTop*/,
@@ -1461,10 +1454,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                                     / (float) mRtSurfaceWidth /*postScaleX*/,
                             mRTLastReportedPosition.height()
                                     / (float) mRtSurfaceHeight /*postScaleY*/);
-                    if (mViewVisibility) {
-                        // b/131239825
-                        mPositionChangedTransaction.show(mSurfaceControl);
-                    }
+
+                    mPositionChangedTransaction.show(mSurfaceControl);
                 }
                 applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);
             } catch (Exception ex) {
@@ -1490,7 +1481,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                         System.identityHashCode(this), frameNumber));
             }
             mRTLastReportedPosition.setEmpty();
-            mRTLastReportedSurfaceSize.set(-1, -1);
 
             // positionLost can be called while UI thread is un-paused.
             synchronized (mSurfaceControlLock) {