From cb7a16280a25de52775bba84e0b9ccaa97c9a774 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Wed, 9 Feb 2022 18:05:02 -0800 Subject: [PATCH] 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