Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use separate Transaction object in SV

Browse Source 
Currently, setAlpha was using mRtTransaction to send the SC transaction.
However, that's also used by positionChanged. This could cause
synchronization issues because both callbacks are invoked from a thread
pool with no synchronization ensured.

Instead, create a Transaction that's used just for FrameCallbacks.
FrameCallbacks will all occur on the same thread so it's safe to use the
same object. Additionally, created a separate transaction that should be
used for positionChanged. This is because it's invoked from a thread
pool can could interfere with the main render thread transactions.
Finally, use the original mRtTransaction only for places that are called
synchronously from renderThread.

Test: Hard to repro original issue. Bubbles works
Bug: 184559328
Change-Id: Ia5a36ad063c19e8a481a57170e47029f7c2c7ee7
This commit is contained in:
chaviw
2021-04-07 17:35:12 -05:00
parent d4fd1b0ce2
commit f541d037f5
1 changed files with 46 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
core/java/android/view/SurfaceView.java
View File

@@ -30,6 +30,7 @@ import android.graphics.BLASTBufferQueue;
import android.graphics.BlendMode;
import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.HardwareRenderer;
import android.graphics.Matrix;
import android.graphics.Paint;
import android.graphics.PixelFormat;
@@ -221,8 +222,35 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
private int mPendingReportDraws;
private SurfaceControl.Transaction mRtTransaction = new SurfaceControl.Transaction();
private SurfaceControl.Transaction mTmpTransaction = new SurfaceControl.Transaction();
/**
* Transaction that should be used from the render thread. This transaction is only thread safe
* with other calls directly from the render thread.
*/
private final SurfaceControl.Transaction mRtTransaction = new SurfaceControl.Transaction();
/**
* Transaction that should be used whe
* {@link HardwareRenderer.FrameDrawingCallback#onFrameDraw} is invoked. All
* frame callbacks can use the same transaction since they will be thread safe
*/
private final SurfaceControl.Transaction mFrameCallbackTransaction =
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
* should be no assumption about thread safety for this transaction.
*/
private final SurfaceControl.Transaction mTmpTransaction = new SurfaceControl.Transaction();
private int mParentSurfaceSequenceId;
private RemoteAccessibilityController mRemoteAccessibilityController =
@@ -432,7 +460,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
* This gets called on a RenderThread worker thread, so members accessed here must
* be protected by a lock.
*/
final boolean useBLAST = useBLASTSync(viewRoot);
viewRoot.registerRtFrameCallback(frame -> {
try {
synchronized (mSurfaceControlLock) {
@@ -456,8 +483,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
Log.d(TAG, System.identityHashCode(this)
+ " updateSurfaceAlpha RT: set alpha=" + alpha);
}
mRtTransaction.setAlpha(mSurfaceControl, alpha);
applyRtTransaction(frame);
mFrameCallbackTransaction.setAlpha(mSurfaceControl, alpha);
applyOrMergeTransaction(mFrameCallbackTransaction, frame);
}
// It's possible that mSurfaceControl is released in the UI thread before
// the transaction completes. If that happens, an exception is thrown, which
@@ -806,7 +834,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
* This gets called on a RenderThread worker thread, so members accessed here must
* be protected by a lock.
*/
final boolean useBLAST = useBLASTSync(viewRoot);
viewRoot.registerRtFrameCallback(frame -> {
try {
synchronized (mSurfaceControlLock) {
@@ -814,8 +841,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
return;
}
updateRelativeZ(mRtTransaction);
applyRtTransaction(frame);
updateRelativeZ(mFrameCallbackTransaction);
applyOrMergeTransaction(mFrameCallbackTransaction, frame);
}
// It's possible that mSurfaceControl is released in the UI thread before
// the transaction completes. If that happens, an exception is thrown, which
@@ -1380,22 +1407,21 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
return mRTLastReportedPosition;
}
private void setParentSpaceRectangle(Rect position, long frameNumber) {
private void setParentSpaceRectangle(Rect position, long frameNumber, Transaction t) {
final ViewRootImpl viewRoot = getViewRootImpl();
applySurfaceTransforms(mSurfaceControl, mRtTransaction, position);
applyChildSurfaceTransaction_renderWorker(mRtTransaction, viewRoot.mSurface, frameNumber);
applyRtTransaction(frameNumber);
applySurfaceTransforms(mSurfaceControl, t, position);
applyChildSurfaceTransaction_renderWorker(t, viewRoot.mSurface, frameNumber);
applyOrMergeTransaction(t, frameNumber);
}
private void applyRtTransaction(long frameNumber) {
private void applyOrMergeTransaction(Transaction t, long frameNumber) {
final ViewRootImpl viewRoot = getViewRootImpl();
boolean useBLAST = viewRoot != null && useBLASTSync(viewRoot);
if (useBLAST) {
// If we are using BLAST, merge the transaction with the viewroot buffer transaction.
viewRoot.mergeWithNextTransaction(mRtTransaction, frameNumber);
return;
viewRoot.mergeWithNextTransaction(t, frameNumber);
} else {
mRtTransaction.apply();
t.apply();
}
}
@@ -1436,7 +1462,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
left, top, right, bottom));
}
mRTLastReportedPosition.set(left, top, right, bottom);
setParentSpaceRectangle(mRTLastReportedPosition, frameNumber);
setParentSpaceRectangle(mRTLastReportedPosition, frameNumber,
mPositionChangedTransaction);
// Now overwrite mRTLastReportedPosition with our values
} catch (Exception ex) {
Log.e(TAG, "Exception from repositionChild", ex);
@@ -1448,7 +1475,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
float bottom, float vecX, float vecY, float maxStretch) {
mRtTransaction.setStretchEffect(mSurfaceControl, left, top, right, bottom, vecX, vecY,
maxStretch);
applyRtTransaction(frameNumber);
applyOrMergeTransaction(mRtTransaction, frameNumber);
}
@Override
@@ -1468,14 +1495,12 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
* need to hold the lock here.
*/
synchronized (mSurfaceControlLock) {
final ViewRootImpl viewRoot = getViewRootImpl();
mRtTransaction.hide(mSurfaceControl);
if (mRtReleaseSurfaces) {
mRtReleaseSurfaces = false;
releaseSurfaces(mRtTransaction);
}
applyRtTransaction(frameNumber);
applyOrMergeTransaction(mRtTransaction, frameNumber);
mRtHandlingPositionUpdates = false;
}
}