Merge "Ensure overlapping draws won't break BLAST sync"
diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index b644e9e..abda698 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -420,8 +420,6 @@
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 @@
Log.d(TAG, System.identityHashCode(this)
+ " updateSurfaceAlpha RT: set alpha=" + alpha);
}
- t.setAlpha(mSurfaceControl, alpha);
- if (!useBLAST) {
+ if (useBLAST) {
+ synchronized (viewRoot.getBlastTransactionLock()) {
+ viewRoot.getBLASTSyncTransaction()
+ .setAlpha(mSurfaceControl, alpha);
+ }
+ } else {
+ Transaction t = new SurfaceControl.Transaction();
+ t.setAlpha(mSurfaceControl, alpha);
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 @@
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;
}
- updateRelativeZ(t);
- if (!useBLAST) {
+
+ if (useBLAST) {
+ synchronized (viewRoot.getBlastTransactionLock()) {
+ updateRelativeZ(viewRoot.getBLASTSyncTransaction());
+ }
+ } else {
+ final SurfaceControl.Transaction t = new SurfaceControl.Transaction();
+ updateRelativeZ(t);
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 @@
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 @@
return mRTLastReportedPosition;
}
+ private void setParentSpaceRectangle(Transaction t, Rect position, long frameNumber) {
+ final ViewRootImpl viewRoot = getViewRootImpl();
+ applySurfaceTransforms(mSurfaceControl, t, position, frameNumber);
+ applyChildSurfaceTransaction_renderWorker(t, viewRoot.mSurface, frameNumber);
+ }
+
private void setParentSpaceRectangle(Rect position, long frameNumber) {
final ViewRootImpl viewRoot = getViewRootImpl();
- final boolean useBLAST = viewRoot.isDrawingToBLASTTransaction();
- final SurfaceControl.Transaction t = useBLAST ? viewRoot.getBLASTSyncTransaction() :
- mRtTransaction;
+ final boolean useBLAST = viewRoot.useBLAST();
- applySurfaceTransforms(mSurfaceControl, t, position, frameNumber);
-
- applyChildSurfaceTransaction_renderWorker(t, viewRoot.mSurface,
- frameNumber);
-
- 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 @@
}
}
+ 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 @@
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();
- }
}
};
diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index 0bbd899..7ce7a12 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -654,26 +654,25 @@
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 @@
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;
+ // 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;
-
- if (mBlastBufferQueue != null) {
- mBlastBufferQueue.setNextTransaction(mRtBLASTSyncTransaction);
- }
}
boolean canUseAsync = draw(fullRedrawNeeded);
if (usingAsyncReport && !canUseAsync) {
@@ -9757,9 +9759,12 @@
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 @@
return mRtBLASTSyncTransaction;
}
+ Object getBlastTransactionLock() {
+ return mRtBLASTSyncTransaction;
+ }
+
/**
* @hide
*/
@@ -9799,12 +9808,4 @@
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;
- }
}