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

Commit befbb69c authored by Vishnu Nair's avatar Vishnu Nair
Browse files

SurfaceView: Synchronize position updates with fixed size changes 

This cl solves a couple of issues:
1. Render thread workers and UI thread share the surface size. They work
on different frames (UI thread sets up the next frame while render
thread is working on the current frame. Accessing the surface size is
unsafe and can result in flickers.

2. UI thread is changing the geometry on size change which might
conflict with render thread causing flickers. This is because of
unsafe accesses as the one mentioned above and the UI thread may
not have the up-to-date position of the view.

This cl fixes the issues by only applying geometry changes in the
UI thread when creating the surface, otherwise the render thread
workers are responsible for updating the geometry. The RenderNode
position update listeners are updated whenever the surface size
changes in order to capture the new size and accompanying changes
which must be applied with the new geometry changes.

Note: updating the position update listeners will trigger a
position update callback so we are guaranteed to apply the
changes in scenarios where the fixed size changes but the view
size does not change.

Test: atest SurfaceViewSyncTest#testSurfaceViewChangeFixedSize
Test: atest SurfaceViewSyncTest#testSurfaceViewChangeFixedSizeWithViewSizeChanges
Test: go/wm-smoke
Test: repro steps from b/190449942
Fixes: 190449942
Change-Id: I076321c853f9a0f6cbf169637e3f3ede60361d60
parent f13eac9f

core/java/android/view/SurfaceView.java

+95 −56
Original line number Original line Diff line number Diff line
@@ -33,6 +33,7 @@ import android.graphics.Color; 
import android.graphics.Matrix;

import android.graphics.Matrix;

import android.graphics.Paint;

import android.graphics.Paint;

import android.graphics.PixelFormat;

import android.graphics.PixelFormat;

import android.graphics.Point;

import android.graphics.Rect;

import android.graphics.Rect;

import android.graphics.Region;

import android.graphics.Region;

import android.graphics.RenderNode;

import android.graphics.RenderNode;
@@ -236,15 +237,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    private final SurfaceControl.Transaction mFrameCallbackTransaction =

    private final SurfaceControl.Transaction mFrameCallbackTransaction =

            new SurfaceControl.Transaction();

            new SurfaceControl.Transaction();





    /**

     * Transaction that should be used for

     * {@link RenderNode.PositionUpdateListener#positionChanged(long, int, int, int, int)}

     * The callback is invoked from a thread pool so it's not thread safe with other render thread

     * transactions. Keep the transactions for position changed callbacks on its own transaction.

     */

    private final SurfaceControl.Transaction mPositionChangedTransaction =

            new SurfaceControl.Transaction();



    /**

    /**

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

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

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

     * should be no assumption about thread safety for this transaction.
@@ -295,7 +287,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
            int defStyleRes, boolean disableBackgroundLayer) {

            int defStyleRes, boolean disableBackgroundLayer) {

        super(context, attrs, defStyleAttr, defStyleRes);

        super(context, attrs, defStyleAttr, defStyleRes);

        mUseBlastAdapter = useBlastAdapter(context);

        mUseBlastAdapter = useBlastAdapter(context);

        mRenderNode.addPositionUpdateListener(mPositionListener);





        setWillNotDraw(true);

        setWillNotDraw(true);

        mDisableBackgroundLayer = disableBackgroundLayer;

        mDisableBackgroundLayer = disableBackgroundLayer;
@@ -943,6 +934,24 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        }

        }

    }

    }







    // The position update listener is used to safely share the surface size between render thread

    // workers and the UI thread. Both threads need to know the surface size to determine the scale.

    // The parent layer scales the surface size to view size. The child (BBQ) layer scales

    // the buffer to the surface size. Both scales along with the window crop must be applied

    // 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,

            @Nullable Transaction geometryTransaction) {

        if (mPositionListener != null) {

            mRenderNode.removePositionUpdateListener(mPositionListener);

        }

        mPositionListener = new SurfaceViewPositionUpdateListener(surfaceWidth, surfaceHeight,

                geometryTransaction);

        mRenderNode.addPositionUpdateListener(mPositionListener);

    }



    private boolean performSurfaceTransaction(ViewRootImpl viewRoot, Translator translator,

    private boolean performSurfaceTransaction(ViewRootImpl viewRoot, Translator translator,

            boolean creating, boolean sizeChanged, boolean hintChanged) {

            boolean creating, boolean sizeChanged, boolean hintChanged) {

        boolean realSizeChanged = false;

        boolean realSizeChanged = false;
@@ -985,13 +994,13 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
            // While creating the surface, we will set it's initial

            // While creating the surface, we will set it's initial

            // geometry. Outside of that though, we should generally

            // geometry. Outside of that though, we should generally

            // leave it to the RenderThread.

            // leave it to the RenderThread.

            //

            Transaction geometryTransaction = new Transaction();

            // There is one more case when the buffer size changes we aren't yet

            geometryTransaction.setCornerRadius(mSurfaceControl, mCornerRadius);

            // prepared to sync (as even following the transaction applying

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

            // we still need to latch a buffer).

                setBufferSize(geometryTransaction);

            // b/28866173

            }

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

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

                onSetSurfacePositionAndScaleRT(mTmpTransaction, mSurfaceControl,

                onSetSurfacePositionAndScaleRT(geometryTransaction, mSurfaceControl,

                        mScreenRect.left, /*positionLeft*/

                        mScreenRect.left, /*positionLeft*/

                        mScreenRect.top /*positionTop*/ ,

                        mScreenRect.top /*positionTop*/ ,

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

                        mScreenRect.width() / (float) mSurfaceWidth /*postScaleX*/,
@@ -1002,17 +1011,31 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                // use SCALING_MODE_SCALE and submit a larger size than the surface

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

                // size.

                // size.

                if (mClipSurfaceToBounds && mClipBounds != null) {

                if (mClipSurfaceToBounds && mClipBounds != null) {

                    mTmpTransaction.setWindowCrop(mSurfaceControl, mClipBounds);

                    geometryTransaction.setWindowCrop(mSurfaceControl, mClipBounds);

                } else {

                } else {

                    mTmpTransaction.setWindowCrop(mSurfaceControl, mSurfaceWidth,

                    geometryTransaction.setWindowCrop(mSurfaceControl, mSurfaceWidth,

                            mSurfaceHeight);

                            mSurfaceHeight);

                }

                }



                boolean applyChangesOnRenderThread =

                        sizeChanged && !creating && isHardwareAccelerated();

                if (isHardwareAccelerated()) {

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

                    // applied on a render worker thread.

                    replacePositionUpdateListener(mSurfaceWidth, mSurfaceHeight,

                            applyChangesOnRenderThread ? geometryTransaction : null);

                }

                }

            mTmpTransaction.setCornerRadius(mSurfaceControl, mCornerRadius);

                if (DEBUG_POSITION) {

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

                    Log.d(TAG, String.format(

                setBufferSize(mTmpTransaction);

                            "%d updateSurfacePosition %s"

                                + "position = [%d, %d, %d, %d] surfaceSize = %dx%d",

                            System.identityHashCode(this),

                            applyChangesOnRenderThread ? "RenderWorker" : "UiThread",

                            mScreenRect.left, mScreenRect.top, mScreenRect.right,

                            mScreenRect.bottom, mSurfaceWidth, mSurfaceHeight));

                }

                }



            }

            mTmpTransaction.merge(geometryTransaction);

            mTmpTransaction.apply();

            mTmpTransaction.apply();

            updateEmbeddedAccessibilityMatrix();

            updateEmbeddedAccessibilityMatrix();
@@ -1399,19 +1422,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        mTmpTransaction.apply();

        mTmpTransaction.apply();

    }

    }





    private void applySurfaceTransforms(SurfaceControl surface, SurfaceControl.Transaction t,

            Rect position) {

        onSetSurfacePositionAndScaleRT(t, surface,

                position.left /*positionLeft*/,

                position.top /*positionTop*/,

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

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



        if (mViewVisibility) {

            t.show(surface);

        }

    }



    /**

    /**

     * @return The last render position of the backing surface or an empty rect.

     * @return The last render position of the backing surface or an empty rect.

     *

     *
@@ -1421,13 +1431,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        return mRTLastReportedPosition;

        return mRTLastReportedPosition;

    }

    }





    private void setParentSpaceRectangle(Rect position, long frameNumber, Transaction t) {

        final ViewRootImpl viewRoot = getViewRootImpl();

        applySurfaceTransforms(mSurfaceControl, t, position);

        applyChildSurfaceTransaction_renderWorker(t, viewRoot.mSurface, frameNumber);

        applyOrMergeTransaction(t, frameNumber);

    }



    private void applyOrMergeTransaction(Transaction t, long frameNumber) {

    private void applyOrMergeTransaction(Transaction t, long frameNumber) {

        final ViewRootImpl viewRoot = getViewRootImpl();

        final ViewRootImpl viewRoot = getViewRootImpl();

        boolean useBLAST = viewRoot != null && useBLASTSync(viewRoot);

        boolean useBLAST = viewRoot != null && useBLASTSync(viewRoot);
@@ -1440,9 +1443,24 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    }

    }





    private Rect mRTLastReportedPosition = new Rect();

    private Rect mRTLastReportedPosition = new Rect();

    private Point mRTLastReportedSurfaceSize = new Point();





    private RenderNode.PositionUpdateListener mPositionListener =

    private class SurfaceViewPositionUpdateListener implements RenderNode.PositionUpdateListener {

            new RenderNode.PositionUpdateListener() {

        int mRtSurfaceWidth = -1;

        int mRtSurfaceHeight = -1;

        private final SurfaceControl.Transaction mPositionChangedTransaction =

                new SurfaceControl.Transaction();

        boolean mPendingTransaction = false;



        SurfaceViewPositionUpdateListener(int surfaceWidth, int surfaceHeight,

                @Nullable Transaction t) {

            mRtSurfaceWidth = surfaceWidth;

            mRtSurfaceHeight = surfaceHeight;

            if (t != null) {

                mPositionChangedTransaction.merge(t);

                mPendingTransaction = true;

            }

        }





        @Override

        @Override

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

        public void positionChanged(long frameNumber, int left, int top, int right, int bottom) {
@@ -1464,21 +1482,34 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
            if (mRTLastReportedPosition.left == left

            if (mRTLastReportedPosition.left == left

                    && mRTLastReportedPosition.top == top

                    && mRTLastReportedPosition.top == top

                    && mRTLastReportedPosition.right == right

                    && mRTLastReportedPosition.right == right

                    && mRTLastReportedPosition.bottom == bottom) {

                    && mRTLastReportedPosition.bottom == bottom

                    && mRTLastReportedSurfaceSize.x == mRtSurfaceWidth

                    && mRTLastReportedSurfaceSize.y == mRtSurfaceHeight

                    && !mPendingTransaction) {

                return;

                return;

            }

            }

            try {

            try {

                if (DEBUG_POSITION) {

                if (DEBUG_POSITION) {

                    Log.d(TAG, String.format(

                    Log.d(TAG, String.format(

                            "%d updateSurfacePosition RenderWorker, frameNr = %d, "

                            "%d updateSurfacePosition RenderWorker, frameNr = %d, "

                                    + "position = [%d, %d, %d, %d]",

                                    + "position = [%d, %d, %d, %d] surfaceSize = %dx%d",

                            System.identityHashCode(this), frameNumber,

                            System.identityHashCode(SurfaceView.this), frameNumber,

                            left, top, right, bottom));

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

                }

                }

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

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

                setParentSpaceRectangle(mRTLastReportedPosition, frameNumber,

                mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight);

                        mPositionChangedTransaction);

                onSetSurfacePositionAndScaleRT(mPositionChangedTransaction, mSurfaceControl,

                // Now overwrite mRTLastReportedPosition with our values

                        mRTLastReportedPosition.left /*positionLeft*/,

                        mRTLastReportedPosition.top /*positionTop*/,

                        mRTLastReportedPosition.width() / (float) mRtSurfaceWidth /*postScaleX*/,

                        mRTLastReportedPosition.height() / (float) mRtSurfaceHeight /*postScaleY*/);

                if (mViewVisibility) {

                    mPositionChangedTransaction.show(mSurfaceControl);

                }

                applyChildSurfaceTransaction_renderWorker(mPositionChangedTransaction,

                        getViewRootImpl().mSurface, frameNumber);

                applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);

                mPendingTransaction = false;

            } catch (Exception ex) {

            } catch (Exception ex) {

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

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

            }

            }
@@ -1502,7 +1533,13 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                        System.identityHashCode(this), frameNumber));

                        System.identityHashCode(this), frameNumber));

            }

            }

            mRTLastReportedPosition.setEmpty();

            mRTLastReportedPosition.setEmpty();



            mRTLastReportedSurfaceSize.set(-1, -1);

            if (mPendingTransaction) {

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

                        + "Pending transaction cleared.");

                mPositionChangedTransaction.clear();

                mPendingTransaction = false;

            }

            if (mSurfaceControl == null) {

            if (mSurfaceControl == null) {

                return;

                return;

            }

            }
@@ -1521,7 +1558,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                mRtHandlingPositionUpdates = false;

                mRtHandlingPositionUpdates = false;

            }

            }

        }

        }

    };

    }



    private SurfaceViewPositionUpdateListener mPositionListener = null;





    private SurfaceHolder.Callback[] getSurfaceCallbacks() {

    private SurfaceHolder.Callback[] getSurfaceCallbacks() {

        SurfaceHolder.Callback[] callbacks;

        SurfaceHolder.Callback[] callbacks;