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

Commit 422dd22c authored by chaviw's avatar chaviw
Browse files

Ensure overlapping draws won't break BLAST sync

Previously, the code would pause the renderer and then call
setNextTransaction. This was to ensure the transaction would
wait for the upcoming frame, not the previous one that may about
to get processed. This was bad becuase it slows down the renderer.

Instead, add a frameCallback listener to the renderer and call
setNextTransaction when the frame callback is invoked. This will
ensure that we only call setNextTransaction when the renderer is
ready to draw, ensuring the next frame in BLASTBufferQueue is the
correct frame to sync with the transaction.

SurfaceView doesn't need to check if VRI is in a blast sync transaction
since any call to PositionListener can be assumed to be a in a blast
sync. This is because SV requests to use blast sync when position, size,
or visibility changes.

Test: Youtube with BLAST enabled
      Contains a SurfaceView that will force blast sync transaction
Test: SurfaceViewSyncTest
Fixes: 149747443
Change-Id: I3e42f87aa8473ee0ee65f23cc00db95f112b4f63
parent b222d485
Loading
Loading
Loading
Loading
+57 −43
Original line number Diff line number Diff line
@@ -420,8 +420,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                final boolean useBLAST = viewRoot.useBLAST();
                viewRoot.registerRtFrameCallback(frame -> {
                    try {
                        final SurfaceControl.Transaction t = useBLAST ?
                            viewRoot.getBLASTSyncTransaction() : new SurfaceControl.Transaction();
                        synchronized (mSurfaceControlLock) {
                            if (!parent.isValid()) {
                                if (DEBUG) {
@@ -443,16 +441,22 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                                Log.d(TAG, System.identityHashCode(this)
                                        + " updateSurfaceAlpha RT: set alpha=" + alpha);
                            }
                            if (useBLAST) {
                                synchronized (viewRoot.getBlastTransactionLock()) {
                                    viewRoot.getBLASTSyncTransaction()
                                            .setAlpha(mSurfaceControl, alpha);
                                }
                            } else {
                                Transaction t = new SurfaceControl.Transaction();
                                t.setAlpha(mSurfaceControl, alpha);
                            if (!useBLAST) {
                                t.deferTransactionUntil(mSurfaceControl,
                                        viewRoot.getRenderSurfaceControl(), frame);
                                t.apply();
                            }
                        }
                        // It's possible that mSurfaceControl is released in the UI thread before
                        // the transaction completes. If that happens, an exception is thrown, which
                        // must be caught immediately.
                        t.apply();
                    } catch (Exception e) {
                        Log.e(TAG, System.identityHashCode(this)
                                + "updateSurfaceAlpha RT: Exception during surface transaction", e);
@@ -793,23 +797,26 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
        final boolean useBLAST = viewRoot.useBLAST();
        viewRoot.registerRtFrameCallback(frame -> {
            try {
                final SurfaceControl.Transaction t = useBLAST
                        ? viewRoot.getBLASTSyncTransaction()
                        : new SurfaceControl.Transaction();
                synchronized (mSurfaceControlLock) {
                    if (!parent.isValid() || mSurfaceControl == null) {
                        return;
                    }

                    if (useBLAST) {
                        synchronized (viewRoot.getBlastTransactionLock()) {
                            updateRelativeZ(viewRoot.getBLASTSyncTransaction());
                        }
                    } else {
                        final SurfaceControl.Transaction t = new SurfaceControl.Transaction();
                        updateRelativeZ(t);
                    if (!useBLAST) {
                        t.deferTransactionUntil(mSurfaceControl,
                                viewRoot.getRenderSurfaceControl(), frame);
                        t.apply();
                    }
                }
                // It's possible that mSurfaceControl is released in the UI thread before
                // the transaction completes. If that happens, an exception is thrown, which
                // must be caught immediately.
                t.apply();
             } catch (Exception e) {
                Log.e(TAG, System.identityHashCode(this)
                        + "setZOrderOnTop RT: Exception during surface transaction", e);
@@ -1252,7 +1259,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
    private void applySurfaceTransforms(SurfaceControl surface, SurfaceControl.Transaction t,
            Rect position, long frameNumber) {
        final ViewRootImpl viewRoot = getViewRootImpl();
        if (frameNumber > 0 && viewRoot != null && !viewRoot.isDrawingToBLASTTransaction()) {
        if (frameNumber > 0 && viewRoot != null && !viewRoot.useBLAST()) {
            t.deferTransactionUntil(surface, viewRoot.getRenderSurfaceControl(),
                    frameNumber);
        }
@@ -1277,19 +1284,23 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
        return mRTLastReportedPosition;
    }

    private void setParentSpaceRectangle(Rect position, long frameNumber) {
    private void setParentSpaceRectangle(Transaction t, Rect position, long frameNumber) {
        final ViewRootImpl viewRoot = getViewRootImpl();
        final boolean useBLAST = viewRoot.isDrawingToBLASTTransaction();
        final SurfaceControl.Transaction t = useBLAST ? viewRoot.getBLASTSyncTransaction() :
            mRtTransaction;

        applySurfaceTransforms(mSurfaceControl, t, position, frameNumber);
        applyChildSurfaceTransaction_renderWorker(t, viewRoot.mSurface, frameNumber);
    }

        applyChildSurfaceTransaction_renderWorker(t, viewRoot.mSurface,
                frameNumber);
    private void setParentSpaceRectangle(Rect position, long frameNumber) {
        final ViewRootImpl viewRoot = getViewRootImpl();
        final boolean useBLAST = viewRoot.useBLAST();

        if (!useBLAST) {
            t.apply();
        if (useBLAST) {
            synchronized (viewRoot.getBlastTransactionLock()) {
                setParentSpaceRectangle(viewRoot.getBLASTSyncTransaction(), position, frameNumber);
            }
        } else {
            setParentSpaceRectangle(mRtTransaction, position, frameNumber);
            mRtTransaction.apply();
        }
    }

@@ -1337,10 +1348,20 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
            }
        }

        private void releaseSurfaces(Transaction t) {
            if (mRtReleaseSurfaces) {
                mRtReleaseSurfaces = false;
                t.remove(mSurfaceControl);
                t.remove(mBackgroundControl);
                mSurfaceControl = null;
                mBackgroundControl = null;
            }
        }

        @Override
        public void positionLost(long frameNumber) {
            final ViewRootImpl viewRoot = getViewRootImpl();
            boolean useBLAST = viewRoot != null && viewRoot.isDrawingToBLASTTransaction();
            boolean useBLAST = viewRoot != null && viewRoot.useBLAST();
            if (DEBUG) {
                Log.d(TAG, String.format("%d windowPositionLost, frameNr = %d",
                        System.identityHashCode(this), frameNumber));
@@ -1351,38 +1372,31 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                return;
            }

            final SurfaceControl.Transaction t = useBLAST ?
                (viewRoot != null ? viewRoot.getBLASTSyncTransaction() : mRtTransaction) :
                mRtTransaction;

            /**
             * positionLost can be called while UI thread is un-paused so we
             * need to hold the lock here.
             */
            synchronized (mSurfaceControlLock) {
                if (frameNumber > 0 && viewRoot !=  null && !useBLAST) {
                    if (viewRoot.mSurface.isValid()) {
                if (useBLAST) {
                    synchronized (viewRoot.getBlastTransactionLock()) {
                        viewRoot.getBLASTSyncTransaction().hide(mSurfaceControl);
                        releaseSurfaces(viewRoot.getBLASTSyncTransaction());
                    }
                } else {
                    if (frameNumber > 0 && viewRoot != null && viewRoot.mSurface.isValid()) {
                        mRtTransaction.deferTransactionUntil(mSurfaceControl,
                                viewRoot.getRenderSurfaceControl(), frameNumber);
                    }
                }
                t.hide(mSurfaceControl);

                if (mRtReleaseSurfaces) {
                    mRtReleaseSurfaces = false;
                    mRtTransaction.remove(mSurfaceControl);
                    mRtTransaction.remove(mBackgroundControl);
                    mSurfaceControl = null;
                    mBackgroundControl = null;
                    mRtTransaction.hide(mSurfaceControl);
                    releaseSurfaces(mRtTransaction);
                    // If we aren't using BLAST, we apply the transaction locally, otherwise we let
                    // the ViewRoot apply it for us.
                    // If the ViewRoot is null, we behave as if we aren't using BLAST so we need to
                    // apply the transaction.
                    mRtTransaction.apply();
                }
                mRtHandlingPositionUpdates = false;
            }

            // If we aren't using BLAST, we apply the transaction locally, otherise we let the ViewRoot apply it for us.
            // If the ViewRoot is null, we behave as if we aren't using BLAST so we need to apply the transaction.
            if (!useBLAST || viewRoot == null) {
                mRtTransaction.apply();
            }
        }
    };

+44 −43
Original line number Diff line number Diff line
@@ -654,26 +654,25 @@ public final class ViewRootImpl implements ViewParent,
        int localChanges;
    }

    // If set, ViewRootImpl will call BLASTBufferQueue::setNextTransaction with
    // mRtBLASTSyncTransaction, prior to invoking draw. This provides a way
    // to redirect the buffers in to transactions.
    // If set, ViewRootImpl will request a callback from HWRender when it's ready to render the next
    // frame. This will allow VRI to call BLASTBufferQueue::setNextTransaction with
    // mRtBLASTSyncTransaction, so the next frame submitted will be added to the
    // mRtBLASTSyncTransaction instead of getting applied.
    private boolean mNextDrawUseBLASTSyncTransaction;
    // Set when calling setNextTransaction, we can't just reuse mNextDrawUseBLASTSyncTransaction
    // because, imagine this scenario:
    //     1. First draw is using BLAST, mNextDrawUseBLAST = true
    //     2. We call perform draw and are waiting on the callback
    //     3. After the first perform draw but before the first callback and the
    //        second perform draw, a second draw sets mNextDrawUseBLAST = true (it already was)
    //     4. At this point the callback fires and we set mNextDrawUseBLAST = false;
    //     5. We get to performDraw and fail to sync as we intended because mNextDrawUseBLAST
    //        is now false.
    // This is why we use a two-step latch with the two booleans, one consumed from
    // performDraw and one consumed from finishBLASTSync()
    private boolean mNextReportConsumeBLAST;
    // Be very careful with the threading here. This is used from the render thread while
    // the UI thread is paused and then applied and cleared from the UI thread right after
    // draw returns.
    private SurfaceControl.Transaction mRtBLASTSyncTransaction = new SurfaceControl.Transaction();

    // This is used to signal if the mRtBLASTSyncTransaction should be applied/merged. When true,
    // it indicates mRtBLASTSyncTransaction was sent to BLASTBufferQueue::setNextTransaction.
    // Therefore, in onFrameComplete, if mRtNextFrameReportConsumeWithBlast is true, that means
    // mRtBLASTSyncTransaction now contains the next buffer frame to be applied.
    private boolean mRtNextFrameReportedConsumeWithBlast;

    // Be very careful with the threading here. This is used from a thread pool generated by the
    // render thread. Therefore, it needs to be locked when updating from the thread pool since
    // multiple threads can be accessing it. It does not need to be locked when applied or merged
    // since that can only happen from the frame complete callback after the other callbacks have
    // been invoked.
    private final SurfaceControl.Transaction mRtBLASTSyncTransaction =
            new SurfaceControl.Transaction();

    // Keeps track of whether the WM requested us to use BLAST Sync when calling relayout.
    //  We use this to make sure we don't send the WM transactions from an internal BLAST sync
@@ -3783,26 +3782,29 @@ public final class ViewRootImpl implements ViewParent,
                                    commitCallbacks.get(i).run();
                                }
                            }
                        });});
                        });
                });
            }
        }

        try {
            if (mNextDrawUseBLASTSyncTransaction) {
                // TODO(b/149747443)
                // We aren't prepared to handle overlapping use of mRtBLASTSyncTransaction
                // so if we are BLAST syncing we make sure the previous draw has
                // totally finished.
                if (mAttachInfo.mThreadedRenderer != null) {
                    mAttachInfo.mThreadedRenderer.pause();
                }

                mNextReportConsumeBLAST = true;
                mNextDrawUseBLASTSyncTransaction = false;

                // Frame callbacks will always occur after submitting draw requests and before
                // the draw actually occurs. This will ensure that we set the next transaction
                // for the frame that's about to get drawn and not on a previous frame that.
                //
                // This is thread safe since mRtNextFrameReportConsumeWithBlast will only be
                // modified in onFrameDraw and then again in onFrameComplete. This is to ensure the
                // next frame completed should be reported with the blast sync transaction.
                registerRtFrameCallback(frame -> {
                    mRtNextFrameReportedConsumeWithBlast = true;
                    if (mBlastBufferQueue != null) {
                        // We don't need to synchronize mRtBLASTSyncTransaction here since it's not
                        // being modified and only sent to BlastBufferQueue.
                        mBlastBufferQueue.setNextTransaction(mRtBLASTSyncTransaction);
                    }
                });
                mNextDrawUseBLASTSyncTransaction = false;
            }
            boolean canUseAsync = draw(fullRedrawNeeded);
            if (usingAsyncReport && !canUseAsync) {
@@ -9757,9 +9759,12 @@ public final class ViewRootImpl implements ViewParent,

    private void finishBLASTSync(boolean apply) {
        mSendNextFrameToWm = false;
        if (mNextReportConsumeBLAST) {
            mNextReportConsumeBLAST = false;
        if (mRtNextFrameReportedConsumeWithBlast) {
            mRtNextFrameReportedConsumeWithBlast = false;

            // We don't need to synchronize mRtBLASTSyncTransaction here we're guaranteed that this
            // is called after all onFrameDraw and after callbacks to PositionUpdateListener.
            // Therefore, no one should be modifying this object until the next vsync.
            if (apply) {
                mRtBLASTSyncTransaction.apply();
            } else {
@@ -9772,6 +9777,10 @@ public final class ViewRootImpl implements ViewParent,
        return mRtBLASTSyncTransaction;
    }

    Object getBlastTransactionLock() {
        return mRtBLASTSyncTransaction;
    }

    /**
     * @hide
     */
@@ -9799,12 +9808,4 @@ public final class ViewRootImpl implements ViewParent,
    boolean useBLAST() {
        return mUseBLASTAdapter && !mForceDisableBLAST;
    }

    /**
     * Returns true if we are about to or currently processing a draw directed
     * in to a BLAST transaction.
     */
    boolean isDrawingToBLASTTransaction() {
        return mNextReportConsumeBLAST;
    }
}