From cb7a16280a25de52775bba84e0b9ccaa97c9a774 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Wed, 9 Feb 2022 18:05:02 -0800 Subject: [PATCH 1/2] SurfaceView: Avoid destination frame updates on multiple threads 1/2 The caller such as SurfaceView can optionally apply destination frame changes if they wish to synchronize buffer scale with other scales in the hierarchy. This is hard to synchronize if the buffer scale changes, since in fixed scaling mode we want the destination frame to be applied when a buffer of the new size is queued. This approach is brittle because SurfaceView does not have control over the buffer production and the app can get into a scenario where there is scaled incorrectly. Fix this by configuring BBQ to always apply destination frame changes or always defer to the caller. If the scaling mode is freeze, then BBQ will set a flag to ignore the destination frame. This allows us to synchronize destination frame changes with scale applied by a parent and avoid unwanted scaling if the scaling mode changes to freeze. Test: atest SurfaceViewTest Test: go/wm-smoke Bug: 217973491 Change-Id: I5226c304bae9dde890306f8528dfeabf6b015a32 --- core/java/android/view/SurfaceControl.java | 22 +++++++++++++++++++ core/java/android/view/SurfaceView.java | 10 +++++---- .../jni/android_graphics_BLASTBufferQueue.cpp | 15 ++++++------- core/jni/android_view_SurfaceControl.cpp | 12 +++++++++- .../android/graphics/BLASTBufferQueue.java | 18 +++++---------- 5 files changed, 52 insertions(+), 25 deletions(-) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 98cef95885bdb..14ffeafcaee1d 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -264,6 +264,8 @@ public final class SurfaceControl implements Parcelable { private static native void nativeAddTransactionCommittedListener(long nativeObject, TransactionCommittedListener listener); private static native void nativeSanitize(long transactionObject); + private static native void nativeSetDestinationFrame(long transactionObj, long nativeObject, + int l, int t, int r, int b); /** * Transforms that can be applied to buffers as they are displayed to a window. @@ -3833,6 +3835,26 @@ public final class SurfaceControl implements Parcelable { nativeSanitize(mNativeObject); } + /** + * @hide + */ + public Transaction setDesintationFrame(SurfaceControl sc, @NonNull Rect destinationFrame) { + checkPreconditions(sc); + nativeSetDestinationFrame(mNativeObject, sc.mNativeObject, + destinationFrame.left, destinationFrame.top, destinationFrame.right, + destinationFrame.bottom); + return this; + } + + /** + * @hide + */ + public Transaction setDesintationFrame(SurfaceControl sc, int width, int height) { + checkPreconditions(sc); + nativeSetDestinationFrame(mNativeObject, sc.mNativeObject, 0, 0, width, height); + return this; + } + /** * Merge the other transaction into this transaction, clearing the * other transaction as if it had been applied. diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index 1a458ce5c8ba1..45db5fa544f3e 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -865,6 +865,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall mSurfaceHeight); } + geometryTransaction.setDesintationFrame(mBlastSurfaceControl, mSurfaceWidth, + mSurfaceHeight); + if (isHardwareAccelerated()) { // This will consume the passed in transaction and the transaction will be // applied on a render worker thread. @@ -1107,7 +1110,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall mBlastSurfaceControl.setTransformHint(mTransformHint); if (mBlastBufferQueue != null) { mBlastBufferQueue.update(mBlastSurfaceControl, mSurfaceWidth, mSurfaceHeight, - mFormat, transaction); + mFormat); } } @@ -1184,9 +1187,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall } mTransformHint = viewRoot.getBufferTransformHint(); mBlastSurfaceControl.setTransformHint(mTransformHint); - mBlastBufferQueue = new BLASTBufferQueue(name); - mBlastBufferQueue.update(mBlastSurfaceControl, mSurfaceWidth, mSurfaceHeight, mFormat, - geometryTransaction); + mBlastBufferQueue = new BLASTBufferQueue(name, false /* updateDestinationFrame */); + mBlastBufferQueue.update(mBlastSurfaceControl, mSurfaceWidth, mSurfaceHeight, mFormat); } private void onDrawFinished(Transaction t) { diff --git a/core/jni/android_graphics_BLASTBufferQueue.cpp b/core/jni/android_graphics_BLASTBufferQueue.cpp index 4f13a9cf257eb..3be0eb88a91fb 100644 --- a/core/jni/android_graphics_BLASTBufferQueue.cpp +++ b/core/jni/android_graphics_BLASTBufferQueue.cpp @@ -35,9 +35,10 @@ static struct { jmethodID ctor; } gTransactionClassInfo; -static jlong nativeCreate(JNIEnv* env, jclass clazz, jstring jName) { +static jlong nativeCreate(JNIEnv* env, jclass clazz, jstring jName, + jboolean updateDestinationFrame) { ScopedUtfChars name(env, jName); - sp queue = new BLASTBufferQueue(name.c_str()); + sp queue = new BLASTBufferQueue(name.c_str(), updateDestinationFrame); queue->incStrong((void*)nativeCreate); return reinterpret_cast(queue.get()); } @@ -62,11 +63,9 @@ static void nativeSetSyncTransaction(JNIEnv* env, jclass clazz, jlong ptr, jlong } static void nativeUpdate(JNIEnv* env, jclass clazz, jlong ptr, jlong surfaceControl, jlong width, - jlong height, jint format, jlong transactionPtr) { + jlong height, jint format) { sp queue = reinterpret_cast(ptr); - auto transaction = reinterpret_cast(transactionPtr); - queue->update(reinterpret_cast(surfaceControl), width, height, format, - transaction); + queue->update(reinterpret_cast(surfaceControl), width, height, format); } static void nativeMergeWithNextTransaction(JNIEnv*, jclass clazz, jlong ptr, jlong transactionPtr, @@ -102,11 +101,11 @@ static jobject nativeGatherPendingTransactions(JNIEnv* env, jclass clazz, jlong static const JNINativeMethod gMethods[] = { /* name, signature, funcPtr */ // clang-format off - {"nativeCreate", "(Ljava/lang/String;)J", (void*)nativeCreate}, + {"nativeCreate", "(Ljava/lang/String;Z)J", (void*)nativeCreate}, {"nativeGetSurface", "(JZ)Landroid/view/Surface;", (void*)nativeGetSurface}, {"nativeDestroy", "(J)V", (void*)nativeDestroy}, {"nativeSetSyncTransaction", "(JJZ)V", (void*)nativeSetSyncTransaction}, - {"nativeUpdate", "(JJJJIJ)V", (void*)nativeUpdate}, + {"nativeUpdate", "(JJJJI)V", (void*)nativeUpdate}, {"nativeMergeWithNextTransaction", "(JJJ)V", (void*)nativeMergeWithNextTransaction}, {"nativeGetLastAcquiredFrameNum", "(J)J", (void*)nativeGetLastAcquiredFrameNum}, {"nativeApplyPendingTransactions", "(JJ)V", (void*)nativeApplyPendingTransactions}, diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index fb5b5ffaac48c..4d4cd999353e9 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -961,6 +961,14 @@ static void nativeSanitize(JNIEnv* env, jclass clazz, jlong transactionObj) { transaction->sanitize(); } +static void nativeSetDestinationFrame(JNIEnv* env, jclass clazz, jlong transactionObj, + jlong nativeObject, jint l, jint t, jint r, jint b) { + auto transaction = reinterpret_cast(transactionObj); + SurfaceControl* const ctrl = reinterpret_cast(nativeObject); + Rect crop(l, t, r, b); + transaction->setDestinationFrame(ctrl, crop); +} + static jlongArray nativeGetPhysicalDisplayIds(JNIEnv* env, jclass clazz) { const auto displayIds = SurfaceComposerClient::getPhysicalDisplayIds(); jlongArray array = env->NewLongArray(displayIds.size()); @@ -2190,7 +2198,9 @@ static const JNINativeMethod sSurfaceControlMethods[] = { {"nativeAddTransactionCommittedListener", "(JLandroid/view/SurfaceControl$TransactionCommittedListener;)V", (void*) nativeAddTransactionCommittedListener }, {"nativeSanitize", "(J)V", - (void*) nativeSanitize } + (void*) nativeSanitize }, + {"nativeSetDestinationFrame", "(JJIIII)V", + (void*)nativeSetDestinationFrame }, // clang-format on }; diff --git a/graphics/java/android/graphics/BLASTBufferQueue.java b/graphics/java/android/graphics/BLASTBufferQueue.java index 2678c79d14542..369f20fb52a71 100644 --- a/graphics/java/android/graphics/BLASTBufferQueue.java +++ b/graphics/java/android/graphics/BLASTBufferQueue.java @@ -27,13 +27,13 @@ public final class BLASTBufferQueue { // Note: This field is accessed by native code. public long mNativeObject; // BLASTBufferQueue* - private static native long nativeCreate(String name); + private static native long nativeCreate(String name, boolean updateDestinationFrame); private static native void nativeDestroy(long ptr); private static native Surface nativeGetSurface(long ptr, boolean includeSurfaceControlHandle); private static native void nativeSetSyncTransaction(long ptr, long transactionPtr, boolean acquireSingleBuffer); private static native void nativeUpdate(long ptr, long surfaceControl, long width, long height, - int format, long transactionPtr); + int format); private static native void nativeMergeWithNextTransaction(long ptr, long transactionPtr, long frameNumber); private static native long nativeGetLastAcquiredFrameNum(long ptr); @@ -45,12 +45,12 @@ public final class BLASTBufferQueue { /** Create a new connection with the surface flinger. */ public BLASTBufferQueue(String name, SurfaceControl sc, int width, int height, @PixelFormat.Format int format) { - this(name); + this(name, false /* updateDestinationFrame */); update(sc, width, height, format); } - public BLASTBufferQueue(String name) { - mNativeObject = nativeCreate(name); + public BLASTBufferQueue(String name, boolean updateDestinationFrame) { + mNativeObject = nativeCreate(name, updateDestinationFrame); } public void destroy() { @@ -101,15 +101,9 @@ public final class BLASTBufferQueue { * @param width The new width for the buffer. * @param height The new height for the buffer. * @param format The new format for the buffer. - * @param t Adds destination frame changes to the passed in transaction. */ - public void update(SurfaceControl sc, int width, int height, @PixelFormat.Format int format, - SurfaceControl.Transaction t) { - nativeUpdate(mNativeObject, sc.mNativeObject, width, height, format, t.mNativeObject); - } - public void update(SurfaceControl sc, int width, int height, @PixelFormat.Format int format) { - nativeUpdate(mNativeObject, sc.mNativeObject, width, height, format, 0); + nativeUpdate(mNativeObject, sc.mNativeObject, width, height, format); } @Override From 6011059ed07abafa7fe0180848302191c72160de Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Mon, 14 Feb 2022 21:44:57 -0800 Subject: [PATCH 2/2] SurfaceView: Synchronize all surface view changes with VRI draw 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 Bug: 217973491 Change-Id: I0b5741b376e042537c37ec34644e0a048eb4bfe0 --- core/java/android/view/SurfaceView.java | 203 ++++++++---------- .../widget/inline/InlineContentView.java | 5 +- 2 files changed, 89 insertions(+), 119 deletions(-) diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index 45db5fa544f3e..55300b3fec364 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -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,18 @@ 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); + mSurfaceControl); 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 +809,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 +874,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 +982,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 +1128,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 +1151,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 +1201,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 +1214,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,66 +1243,57 @@ 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; + private final SurfaceControl mRtSurfaceControl = new SurfaceControl(); SurfaceViewPositionUpdateListener(int surfaceWidth, int surfaceHeight, - @Nullable Transaction t) { + SurfaceControl surfaceControl) { mRtSurfaceWidth = surfaceWidth; mRtSurfaceHeight = surfaceHeight; - if (t != null) { - mPositionChangedTransaction.merge(t); - mPendingTransaction = true; - } + mRtSurfaceControl.copyFrom(surfaceControl, "SurfaceViewPositionUpdateListener"); } @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) { + return; + } + try { + if (DEBUG_POSITION) { + Log.d(TAG, String.format( + "%d updateSurfacePosition RenderWorker, frameNr = %d, " + + "position = [%d, %d, %d, %d] surfaceSize = %dx%d", + System.identityHashCode(SurfaceView.this), frameNumber, + left, top, right, bottom, mRtSurfaceWidth, mRtSurfaceHeight)); } - if (mRTLastReportedPosition.left == left - && mRTLastReportedPosition.top == top - && mRTLastReportedPosition.right == right - && mRTLastReportedPosition.bottom == bottom - && mRTLastReportedSurfaceSize.x == mRtSurfaceWidth - && mRTLastReportedSurfaceSize.y == mRtSurfaceHeight - && !mPendingTransaction) { - return; - } - try { - if (DEBUG_POSITION) { - Log.d(TAG, String.format( - "%d updateSurfacePosition RenderWorker, frameNr = %d, " - + "position = [%d, %d, %d, %d] surfaceSize = %dx%d", - System.identityHashCode(SurfaceView.this), frameNumber, - left, top, right, bottom, mRtSurfaceWidth, mRtSurfaceHeight)); - } - mRTLastReportedPosition.set(left, top, right, bottom); - mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight); - onSetSurfacePositionAndScaleRT(mPositionChangedTransaction, mSurfaceControl, - mRTLastReportedPosition.left /*positionLeft*/, - mRTLastReportedPosition.top /*positionTop*/, - mRTLastReportedPosition.width() - / (float) mRtSurfaceWidth /*postScaleX*/, - mRTLastReportedPosition.height() - / (float) mRtSurfaceHeight /*postScaleY*/); - if (mViewVisibility) { - mPositionChangedTransaction.show(mSurfaceControl); - } - applyOrMergeTransaction(mPositionChangedTransaction, frameNumber); - mPendingTransaction = false; - } catch (Exception ex) { - Log.e(TAG, "Exception from repositionChild", ex); + mRTLastReportedPosition.set(left, top, right, bottom); + mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight); + onSetSurfacePositionAndScale(mPositionChangedTransaction, mRtSurfaceControl, + mRTLastReportedPosition.left /*positionLeft*/, + mRTLastReportedPosition.top /*positionTop*/, + mRTLastReportedPosition.width() + / (float) mRtSurfaceWidth /*postScaleX*/, + mRTLastReportedPosition.height() + / (float) mRtSurfaceHeight /*postScaleY*/); + if (mViewVisibility) { + // b/131239825 + mPositionChangedTransaction.show(mRtSurfaceControl); } + applyOrMergeTransaction(mPositionChangedTransaction, frameNumber); + } catch (Exception ex) { + Log.e(TAG, "Exception from repositionChild", ex); } } @@ -1321,7 +1302,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall float vecX, float vecY, float maxStretchX, float maxStretchY, float childRelativeLeft, float childRelativeTop, float childRelativeRight, float childRelativeBottom) { - mRtTransaction.setStretchEffect(mSurfaceControl, width, height, vecX, vecY, + mRtTransaction.setStretchEffect(mRtSurfaceControl, width, height, vecX, vecY, maxStretchX, maxStretchY, childRelativeLeft, childRelativeTop, childRelativeRight, childRelativeBottom); applyOrMergeTransaction(mRtTransaction, frameNumber); @@ -1336,28 +1317,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 +1371,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(); } /** diff --git a/core/java/android/widget/inline/InlineContentView.java b/core/java/android/widget/inline/InlineContentView.java index 9712311aab7c4..e4f483a293439 100644 --- a/core/java/android/widget/inline/InlineContentView.java +++ b/core/java/android/widget/inline/InlineContentView.java @@ -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); } };