Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 797aa006 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

SurfaceView: Synchronize all surface view changes with VRI draw 

Re landing with the following changes:
We initially cloned the SurfaceControl handle to avoid locking when
accessing the SurfaceControl from the position listener callbacks
running from RT workers. But this meant the last reference to the
layer handle would only be released by GC. If SurfaceViews are
created and destroyed rapidly, we would be at the mercy of GC to
release buffers.

Original change:

There are three transaction queues that can submit SurfaceView
changes.

1. Buffer updates via BBQ apply token
2. SCC apply token
3. ViewRootImpl BBQ apply token

It makes sense for most SurfaceView changes to be synchronized with
ViewRootImpl draws since the caller can optionally synchronize the
change with main window content.

This change eliminates the tmp transaction that is applied directly
via the SCC apply token and instead applies them with the ViewRootImpl
draw transaction.

Also take the opportunity to scope down mSurfaceControlLock usage.

Test: atest SurfaceViewSyncTest
Test: go/wm-smoke
Test: run mem tests via forrest
Bug: b/217973491, b/221631942
Change-Id: Idba712d146e62d7346920dc4f060cba92d47fada
parent 455035b4

core/java/android/view/SurfaceView.java

+75 −107
Original line number Diff line number Diff line
@@ -133,9 +133,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    private boolean mDisableBackgroundLayer = false;



    /**

     * We use this lock to protect access to mSurfaceControl and

     * SurfaceViewPositionUpdateListener#mPositionChangedTransaction. Both are accessed on the UI

     * thread and the render thread.

     * We use this lock to protect access to mSurfaceControl. Both are accessed on the UI

     * thread and the render thread via RenderNode.PositionUpdateListener#positionLost.

     */

    final Object mSurfaceControlLock = new Object();

    final Rect mTmpRect = new Rect();
@@ -224,12 +223,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    private final SurfaceControl.Transaction mFrameCallbackTransaction =

            new SurfaceControl.Transaction();



    /**

     * A temporary transaction holder that should only be used when applying right away. There

     * should be no assumption about thread safety for this transaction.

     */

    private final SurfaceControl.Transaction mTmpTransaction = new SurfaceControl.Transaction();



    private int mParentSurfaceSequenceId;



    private RemoteAccessibilityController mRemoteAccessibilityController =
@@ -760,7 +753,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                mBlastBufferQueue = null;

            }



            Transaction transaction = new Transaction();

            final Transaction transaction = new Transaction();

            if (mSurfaceControl != null) {

                transaction.remove(mSurfaceControl);

                mSurfaceControl = null;
@@ -790,22 +783,17 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    // synchronously otherwise we may see flickers.

    // When the listener is updated, we will get at least a single position update call so we can

    // guarantee any changes we post will be applied.

    private void replacePositionUpdateListener(int surfaceWidth, int surfaceHeight,

            Transaction geometryTransaction) {

    private void replacePositionUpdateListener(int surfaceWidth, int surfaceHeight) {

        if (mPositionListener != null) {

            mRenderNode.removePositionUpdateListener(mPositionListener);

            synchronized (mSurfaceControlLock) {

                geometryTransaction = mPositionListener.getTransaction().merge(geometryTransaction);

            }

        }

        mPositionListener = new SurfaceViewPositionUpdateListener(surfaceWidth, surfaceHeight,

                geometryTransaction);

        mPositionListener = new SurfaceViewPositionUpdateListener(surfaceWidth, surfaceHeight);

        mRenderNode.addPositionUpdateListener(mPositionListener);

    }



    private boolean performSurfaceTransaction(ViewRootImpl viewRoot, Translator translator,

            boolean creating, boolean sizeChanged, boolean hintChanged,

            Transaction geometryTransaction) {

            Transaction surfaceUpdateTransaction) {

        boolean realSizeChanged = false;



        mSurfaceLock.lock();
@@ -820,59 +808,60 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
            // SurfaceChangedCallback to update the relative z. This is needed so that

            // we do not change the relative z before the server is ready to swap the

            // parent surface.

            if (creating || (mParentSurfaceSequenceId == viewRoot.getSurfaceSequenceId())) {

                updateRelativeZ(mTmpTransaction);

            if (creating) {

                updateRelativeZ(surfaceUpdateTransaction);

                if (mSurfacePackage != null) {

                    reparentSurfacePackage(surfaceUpdateTransaction, mSurfacePackage);

                }

            }

            mParentSurfaceSequenceId = viewRoot.getSurfaceSequenceId();



            if (mViewVisibility) {

                geometryTransaction.show(mSurfaceControl);

                surfaceUpdateTransaction.show(mSurfaceControl);

            } else {

                geometryTransaction.hide(mSurfaceControl);

                surfaceUpdateTransaction.hide(mSurfaceControl);

            }



            if (mSurfacePackage != null) {

                reparentSurfacePackage(mTmpTransaction, mSurfacePackage);

            }



            updateBackgroundVisibility(mTmpTransaction);

            updateBackgroundColor(mTmpTransaction);



            updateBackgroundVisibility(surfaceUpdateTransaction);

            updateBackgroundColor(surfaceUpdateTransaction);

            if (mUseAlpha) {

                float alpha = getFixedAlpha();

                mTmpTransaction.setAlpha(mSurfaceControl, alpha);

                surfaceUpdateTransaction.setAlpha(mSurfaceControl, alpha);

                mSurfaceAlpha = alpha;

            }



            geometryTransaction.setCornerRadius(mSurfaceControl, mCornerRadius);

            surfaceUpdateTransaction.setCornerRadius(mSurfaceControl, mCornerRadius);

            if ((sizeChanged || hintChanged) && !creating) {

                setBufferSize(geometryTransaction);

                setBufferSize(surfaceUpdateTransaction);

            }

            if (sizeChanged || creating || !isHardwareAccelerated()) {

                onSetSurfacePositionAndScaleRT(geometryTransaction, mSurfaceControl,

                        mScreenRect.left, /*positionLeft*/

                        mScreenRect.top /*positionTop*/ ,

                        mScreenRect.width() / (float) mSurfaceWidth /*postScaleX*/,

                        mScreenRect.height() / (float) mSurfaceHeight /*postScaleY*/);



                // Set a window crop when creating the surface or changing its size to

                // crop the buffer to the surface size since the buffer producer may

                // use SCALING_MODE_SCALE and submit a larger size than the surface

                // size.

                if (mClipSurfaceToBounds && mClipBounds != null) {

                    geometryTransaction.setWindowCrop(mSurfaceControl, mClipBounds);

                    surfaceUpdateTransaction.setWindowCrop(mSurfaceControl, mClipBounds);

                } else {

                    geometryTransaction.setWindowCrop(mSurfaceControl, mSurfaceWidth,

                    surfaceUpdateTransaction.setWindowCrop(mSurfaceControl, mSurfaceWidth,

                            mSurfaceHeight);

                }



                geometryTransaction.setDesintationFrame(mBlastSurfaceControl, mSurfaceWidth,

                surfaceUpdateTransaction.setDesintationFrame(mBlastSurfaceControl, mSurfaceWidth,

                            mSurfaceHeight);



                if (isHardwareAccelerated()) {

                    // This will consume the passed in transaction and the transaction will be

                    // applied on a render worker thread.

                    replacePositionUpdateListener(mSurfaceWidth, mSurfaceHeight,

                            geometryTransaction);

                    replacePositionUpdateListener(mSurfaceWidth, mSurfaceHeight);

                } else {

                    onSetSurfacePositionAndScale(surfaceUpdateTransaction, mSurfaceControl,

                            mScreenRect.left /*positionLeft*/,

                            mScreenRect.top /*positionTop*/,

                            mScreenRect.width() / (float) mSurfaceWidth /*postScaleX*/,

                            mScreenRect.height() / (float) mSurfaceHeight /*postScaleY*/);

                }

                if (DEBUG_POSITION) {

                    Log.d(TAG, String.format(
@@ -884,8 +873,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                            mScreenRect.bottom, mSurfaceWidth, mSurfaceHeight));

                }

            }

            mTmpTransaction.merge(geometryTransaction);

            mTmpTransaction.apply();

            applyTransactionOnVriDraw(surfaceUpdateTransaction);

            updateEmbeddedAccessibilityMatrix();



            mSurfaceFrame.left = 0;
@@ -993,17 +981,17 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                mScreenRect.offset(surfaceInsets.left, surfaceInsets.top);

                // Collect all geometry changes and apply these changes on the RenderThread worker

                // via the RenderNode.PositionUpdateListener.

                final Transaction geometryTransaction = new Transaction();

                final Transaction surfaceUpdateTransaction = new Transaction();

                if (creating) {

                    updateOpaqueFlag();

                    final String name = "SurfaceView[" + viewRoot.getTitle().toString() + "]";

                    createBlastSurfaceControls(viewRoot, name, geometryTransaction);

                    createBlastSurfaceControls(viewRoot, name, surfaceUpdateTransaction);

                } else if (mSurfaceControl == null) {

                    return;

                }



                final boolean realSizeChanged = performSurfaceTransaction(viewRoot,

                        translator, creating, sizeChanged, hintChanged, geometryTransaction);

                        translator, creating, sizeChanged, hintChanged, surfaceUpdateTransaction);

                final boolean redrawNeeded = sizeChanged || creating || hintChanged

                        || (mVisible && !mDrawFinished);
@@ -1139,7 +1127,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
     *

     */

    private void createBlastSurfaceControls(ViewRootImpl viewRoot, String name,

            Transaction geometryTransaction) {

            Transaction surfaceUpdateTransaction) {

        if (mSurfaceControl == null) {

            mSurfaceControl = new SurfaceControl.Builder(mSurfaceSession)

                    .setName(name)
@@ -1162,11 +1150,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                    .build();

        } else {

            // update blast layer

            mTmpTransaction

            surfaceUpdateTransaction

                    .setOpaque(mBlastSurfaceControl, (mSurfaceFlags & SurfaceControl.OPAQUE) != 0)

                    .setSecure(mBlastSurfaceControl, (mSurfaceFlags & SurfaceControl.SECURE) != 0)

                    .show(mBlastSurfaceControl)

                    .apply();

                    .show(mBlastSurfaceControl);

        }



        if (mBackgroundControl == null) {
@@ -1213,7 +1200,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
     *

     * @hide

     */

    protected void onSetSurfacePositionAndScaleRT(@NonNull Transaction transaction,

    protected void onSetSurfacePositionAndScale(@NonNull Transaction transaction,

            @NonNull SurfaceControl surface, int positionLeft, int positionTop,

            float postScaleX, float postScaleY) {

        transaction.setPosition(surface, positionLeft, positionTop);
@@ -1226,12 +1213,14 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        if (mSurfaceControl == null) {

            return;

        }

        onSetSurfacePositionAndScaleRT(mTmpTransaction, mSurfaceControl,

        final Transaction transaction = new Transaction();

        onSetSurfacePositionAndScale(transaction, mSurfaceControl,

                mScreenRect.left, /*positionLeft*/

                mScreenRect.top/*positionTop*/ ,

                mScreenRect.width() / (float) mSurfaceWidth /*postScaleX*/,

                mScreenRect.height() / (float) mSurfaceHeight /*postScaleY*/);

        mTmpTransaction.apply();

        applyTransactionOnVriDraw(transaction);

        invalidate();

    }



    /**
@@ -1253,39 +1242,28 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        }

    }



    private Rect mRTLastReportedPosition = new Rect();

    private Point mRTLastReportedSurfaceSize = new Point();

    private final Rect mRTLastReportedPosition = new Rect();

    private final Point mRTLastReportedSurfaceSize = new Point();



    private class SurfaceViewPositionUpdateListener implements RenderNode.PositionUpdateListener {

        int mRtSurfaceWidth = -1;

        int mRtSurfaceHeight = -1;

        private final int mRtSurfaceWidth;

        private final int mRtSurfaceHeight;

        private final SurfaceControl.Transaction mPositionChangedTransaction =

                new SurfaceControl.Transaction();

        boolean mPendingTransaction = false;



        SurfaceViewPositionUpdateListener(int surfaceWidth, int surfaceHeight,

                @Nullable Transaction t) {

        SurfaceViewPositionUpdateListener(int surfaceWidth, int surfaceHeight) {

            mRtSurfaceWidth = surfaceWidth;

            mRtSurfaceHeight = surfaceHeight;

            if (t != null) {

                mPositionChangedTransaction.merge(t);

                mPendingTransaction = true;

            }

        }



        @Override

        public void positionChanged(long frameNumber, int left, int top, int right, int bottom) {

            synchronized(mSurfaceControlLock) {

                if (mSurfaceControl == null) {

                    return;

                }

            if (mRTLastReportedPosition.left == left

                    && mRTLastReportedPosition.top == top

                    && mRTLastReportedPosition.right == right

                    && mRTLastReportedPosition.bottom == bottom

                    && mRTLastReportedSurfaceSize.x == mRtSurfaceWidth

                        && mRTLastReportedSurfaceSize.y == mRtSurfaceHeight

                        && !mPendingTransaction) {

                    && mRTLastReportedSurfaceSize.y == mRtSurfaceHeight) {

                return;

            }

            try {
@@ -1296,9 +1274,11 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                            System.identityHashCode(SurfaceView.this), frameNumber,

                            left, top, right, bottom, mRtSurfaceWidth, mRtSurfaceHeight));

                }

                synchronized (mSurfaceControlLock) {

                    if (mSurfaceControl == null) return;

                    mRTLastReportedPosition.set(left, top, right, bottom);

                    mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight);

                    onSetSurfacePositionAndScaleRT(mPositionChangedTransaction, mSurfaceControl,

                    onSetSurfacePositionAndScale(mPositionChangedTransaction, mSurfaceControl,

                            mRTLastReportedPosition.left /*positionLeft*/,

                            mRTLastReportedPosition.top /*positionTop*/,

                            mRTLastReportedPosition.width()
@@ -1306,15 +1286,15 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                            mRTLastReportedPosition.height()

                                    / (float) mRtSurfaceHeight /*postScaleY*/);

                    if (mViewVisibility) {

                        // b/131239825

                        mPositionChangedTransaction.show(mSurfaceControl);

                    }

                }

                applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);

                    mPendingTransaction = false;

            } catch (Exception ex) {

                Log.e(TAG, "Exception from repositionChild", ex);

            }

        }

        }



        @Override

        public void applyStretch(long frameNumber, float width, float height,
@@ -1336,28 +1316,14 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
            mRTLastReportedPosition.setEmpty();

            mRTLastReportedSurfaceSize.set(-1, -1);



            /**

             * positionLost can be called while UI thread is un-paused so we

             * need to hold the lock here.

             */

            // positionLost can be called while UI thread is un-paused.

            synchronized (mSurfaceControlLock) {

                if (mPendingTransaction) {

                    Log.w(TAG, System.identityHashCode(SurfaceView.this)

                            + "Pending transaction cleared.");

                    mPositionChangedTransaction.clear();

                    mPendingTransaction = false;

                }

                if (mSurfaceControl == null) {

                    return;

                }

                if (mSurfaceControl == null) return;

                // b/131239825

                mRtTransaction.hide(mSurfaceControl);

                applyOrMergeTransaction(mRtTransaction, frameNumber);

            }

        }



        public Transaction getTransaction() {

            return mPositionChangedTransaction;

        }

    }



    private SurfaceViewPositionUpdateListener mPositionListener = null;
@@ -1404,8 +1370,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
     * @hide

     */

    public void setResizeBackgroundColor(int bgColor) {

        setResizeBackgroundColor(mTmpTransaction, bgColor);

        mTmpTransaction.apply();

        final SurfaceControl.Transaction transaction = new SurfaceControl.Transaction();

        setResizeBackgroundColor(transaction, bgColor);

        applyTransactionOnVriDraw(transaction);

        invalidate();

    }



    /**

core/java/android/widget/inline/InlineContentView.java

+3 −2
Original line number Diff line number Diff line
@@ -230,8 +230,9 @@ public class InlineContentView extends ViewGroup { 
            int defStyleAttr, int defStyleRes) {

        super(context, attrs, defStyleAttr, defStyleRes);

        mSurfaceView = new SurfaceView(context, attrs, defStyleAttr, defStyleRes) {

            // b/219807628

            @Override

            protected void onSetSurfacePositionAndScaleRT(

            protected void onSetSurfacePositionAndScale(

                    @NonNull SurfaceControl.Transaction transaction,

                    @NonNull SurfaceControl surface, int positionLeft, int positionTop,

                    float postScaleX, float postScaleY) {
@@ -248,7 +249,7 @@ public class InlineContentView extends ViewGroup { 
                postScaleX = InlineContentView.this.getScaleX();

                postScaleY = InlineContentView.this.getScaleY();



                super.onSetSurfacePositionAndScaleRT(transaction, surface, positionLeft,

                super.onSetSurfacePositionAndScale(transaction, surface, positionLeft,

                        positionTop, postScaleX, postScaleY);

            }

        };