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

Commit 87d71d6d authored by Vishnu Nair's avatar Vishnu Nair
Browse files

SurfaceView: Synchronize initial destframe change from BBQ 

Fixes a case where the SurfaceView is stuck with a wrong
scale.

Destination frame scales the buffer to the SurfaceView
fixed size. Currently the initial destination frame is
applied when the first buffer is acquired by BBQ. The
transaction is sent via the BBQ apply token.

Meanwhile if the client updates the SurfaceView fixed
size, the SurfaceView will apply the transaction via
the SCC apply token.

This can cause destframe changes to be applied out of
order causing the app to be stuck with the wrong
scale.

Bug: 195443440
Test: atest BLASTBufferQueueTest
Test: repro steps from bug

Change-Id: Ibf53a0efdebb87291d081e48633c373a98d347b1
parent 4d190c5f

core/java/android/view/SurfaceView.java

+30 −17
Original line number Original line Diff line number Diff line
@@ -948,9 +948,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    // When the listener is updated, we will get at least a single position update call so we can

    // 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.

    // guarantee any changes we post will be applied.

    private void replacePositionUpdateListener(int surfaceWidth, int surfaceHeight,

    private void replacePositionUpdateListener(int surfaceWidth, int surfaceHeight,

            @Nullable Transaction geometryTransaction) {

            Transaction geometryTransaction) {

        if (mPositionListener != null) {

        if (mPositionListener != null) {

            mRenderNode.removePositionUpdateListener(mPositionListener);

            mRenderNode.removePositionUpdateListener(mPositionListener);

            geometryTransaction = mPositionListener.getTransaction().merge(geometryTransaction);

        }

        }

        mPositionListener = new SurfaceViewPositionUpdateListener(surfaceWidth, surfaceHeight,

        mPositionListener = new SurfaceViewPositionUpdateListener(surfaceWidth, surfaceHeight,

                geometryTransaction);

                geometryTransaction);
@@ -958,7 +959,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    }

    }





    private boolean performSurfaceTransaction(ViewRootImpl viewRoot, Translator translator,

    private boolean performSurfaceTransaction(ViewRootImpl viewRoot, Translator translator,

            boolean creating, boolean sizeChanged, boolean hintChanged) {

            boolean creating, boolean sizeChanged, boolean hintChanged,

            Transaction geometryTransaction) {

        boolean realSizeChanged = false;

        boolean realSizeChanged = false;





        mSurfaceLock.lock();

        mSurfaceLock.lock();
@@ -996,10 +998,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                mSurfaceAlpha = alpha;

                mSurfaceAlpha = alpha;

            }

            }





            // While creating the surface, we will set it's initial

            // geometry. Outside of that though, we should generally

            // leave it to the RenderThread.

            Transaction geometryTransaction = new Transaction();

            geometryTransaction.setCornerRadius(mSurfaceControl, mCornerRadius);

            geometryTransaction.setCornerRadius(mSurfaceControl, mCornerRadius);

            if ((sizeChanged || hintChanged) && !creating) {

            if ((sizeChanged || hintChanged) && !creating) {

                setBufferSize(geometryTransaction);

                setBufferSize(geometryTransaction);
@@ -1022,20 +1020,18 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                            mSurfaceHeight);

                            mSurfaceHeight);

                }

                }





                boolean applyChangesOnRenderThread =

                        sizeChanged && !creating && isHardwareAccelerated();

                if (isHardwareAccelerated()) {

                if (isHardwareAccelerated()) {

                    // This will consume the passed in transaction and the transaction will be

                    // This will consume the passed in transaction and the transaction will be

                    // applied on a render worker thread.

                    // applied on a render worker thread.

                    replacePositionUpdateListener(mSurfaceWidth, mSurfaceHeight,

                    replacePositionUpdateListener(mSurfaceWidth, mSurfaceHeight,

                            applyChangesOnRenderThread ? geometryTransaction : null);

                            geometryTransaction);

                }

                }

                if (DEBUG_POSITION) {

                if (DEBUG_POSITION) {

                    Log.d(TAG, String.format(

                    Log.d(TAG, String.format(

                            "%d updateSurfacePosition %s"

                            "%d performSurfaceTransaction %s "

                                + "position = [%d, %d, %d, %d] surfaceSize = %dx%d",

                                + "position = [%d, %d, %d, %d] surfaceSize = %dx%d",

                            System.identityHashCode(this),

                            System.identityHashCode(this),

                            applyChangesOnRenderThread ? "RenderWorker" : "UiThread",

                            isHardwareAccelerated() ? "RenderWorker" : "UI Thread",

                            mScreenRect.left, mScreenRect.top, mScreenRect.right,

                            mScreenRect.left, mScreenRect.top, mScreenRect.right,

                            mScreenRect.bottom, mSurfaceWidth, mSurfaceHeight));

                            mScreenRect.bottom, mSurfaceWidth, mSurfaceHeight));

                }

                }
@@ -1147,12 +1143,14 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 




                final Rect surfaceInsets = viewRoot.mWindowAttributes.surfaceInsets;

                final Rect surfaceInsets = viewRoot.mWindowAttributes.surfaceInsets;

                mScreenRect.offset(surfaceInsets.left, surfaceInsets.top);

                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();

                if (creating) {

                if (creating) {

                    updateOpaqueFlag();

                    updateOpaqueFlag();

                    final String name = "SurfaceView[" + viewRoot.getTitle().toString() + "]";

                    final String name = "SurfaceView[" + viewRoot.getTitle().toString() + "]";

                    if (mUseBlastAdapter) {

                    if (mUseBlastAdapter) {

                        createBlastSurfaceControls(viewRoot, name);

                        createBlastSurfaceControls(viewRoot, name, geometryTransaction);

                    } else {

                    } else {

                        mDeferredDestroySurfaceControl = createSurfaceControls(viewRoot, name);

                        mDeferredDestroySurfaceControl = createSurfaceControls(viewRoot, name);

                    }

                    }
@@ -1161,7 +1159,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                }

                }





                final boolean realSizeChanged = performSurfaceTransaction(viewRoot,

                final boolean realSizeChanged = performSurfaceTransaction(viewRoot,

                        translator, creating, sizeChanged, hintChanged);

                        translator, creating, sizeChanged, hintChanged, geometryTransaction);

                final boolean redrawNeeded = sizeChanged || creating || hintChanged

                final boolean redrawNeeded = sizeChanged || creating || hintChanged

                        || (mVisible && !mDrawFinished);

                        || (mVisible && !mDrawFinished);
@@ -1335,7 +1333,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    // is still alive, the old buffers will continue to be presented until replaced by buffers from

    // is still alive, the old buffers will continue to be presented until replaced by buffers from

    // the new adapter. This means we do not need to track the old surface control and destroy it

    // the new adapter. This means we do not need to track the old surface control and destroy it

    // after the client has drawn to avoid any flickers.

    // after the client has drawn to avoid any flickers.

    private void createBlastSurfaceControls(ViewRootImpl viewRoot, String name) {

    private void createBlastSurfaceControls(ViewRootImpl viewRoot, String name,

            Transaction geometryTransaction) {

        if (mSurfaceControl == null) {

        if (mSurfaceControl == null) {

            mSurfaceControl = new SurfaceControl.Builder(mSurfaceSession)

            mSurfaceControl = new SurfaceControl.Builder(mSurfaceSession)

                    .setName(name)

                    .setName(name)
@@ -1376,8 +1375,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        }

        }

        mTransformHint = viewRoot.getBufferTransformHint();

        mTransformHint = viewRoot.getBufferTransformHint();

        mBlastSurfaceControl.setTransformHint(mTransformHint);

        mBlastSurfaceControl.setTransformHint(mTransformHint);

        mBlastBufferQueue = new BLASTBufferQueue(name, mBlastSurfaceControl, mSurfaceWidth,

        mBlastBufferQueue = new BLASTBufferQueue(name);

                mSurfaceHeight, mFormat);

        mBlastBufferQueue.update(mBlastSurfaceControl, mSurfaceWidth, mSurfaceHeight, mFormat,

                geometryTransaction);

    }

    }





    private void onDrawFinished(Transaction t) {

    private void onDrawFinished(Transaction t) {
@@ -1558,6 +1558,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                applyOrMergeTransaction(mRtTransaction, frameNumber);

                applyOrMergeTransaction(mRtTransaction, frameNumber);

            }

            }

        }

        }



        public Transaction getTransaction() {

            return mPositionChangedTransaction;

        }

    }

    }





    private SurfaceViewPositionUpdateListener mPositionListener = null;

    private SurfaceViewPositionUpdateListener mPositionListener = null;
@@ -1651,6 +1655,11 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        @Override

        @Override

        public void setFixedSize(int width, int height) {

        public void setFixedSize(int width, int height) {

            if (mRequestedWidth != width || mRequestedHeight != height) {

            if (mRequestedWidth != width || mRequestedHeight != height) {

                if (DEBUG_POSITION) {

                    Log.d(TAG, String.format("%d setFixedSize %dx%d -> %dx%d",

                            System.identityHashCode(this), mRequestedWidth, mRequestedHeight, width,

                                    height));

                }

                mRequestedWidth = width;

                mRequestedWidth = width;

                mRequestedHeight = height;

                mRequestedHeight = height;

                requestLayout();

                requestLayout();
@@ -1660,6 +1669,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
        @Override

        @Override

        public void setSizeFromLayout() {

        public void setSizeFromLayout() {

            if (mRequestedWidth != -1 || mRequestedHeight != -1) {

            if (mRequestedWidth != -1 || mRequestedHeight != -1) {

                if (DEBUG_POSITION) {

                    Log.d(TAG, String.format("%d setSizeFromLayout was %dx%d",

                            System.identityHashCode(this), mRequestedWidth, mRequestedHeight));

                }

                mRequestedWidth = mRequestedHeight = -1;

                mRequestedWidth = mRequestedHeight = -1;

                requestLayout();

                requestLayout();

            }

            }

core/jni/android_graphics_BLASTBufferQueue.cpp

+14 −15
Original line number Original line Diff line number Diff line
@@ -30,21 +30,19 @@ 




namespace android {

namespace android {





static jlong nativeCreate(JNIEnv* env, jclass clazz, jstring jName, jlong surfaceControl,

static jlong nativeCreate(JNIEnv* env, jclass clazz, jstring jName) {

                          jlong width, jlong height, jint format) {

    ScopedUtfChars name(env, jName);

    String8 str8;

    sp<BLASTBufferQueue> queue = new BLASTBufferQueue(name.c_str());

    if (jName) {

    queue->incStrong((void*)nativeCreate);

        const jchar* str16 = env->GetStringCritical(jName, nullptr);

    return reinterpret_cast<jlong>(queue.get());

        if (str16) {

            str8 = String8(reinterpret_cast<const char16_t*>(str16), env->GetStringLength(jName));

            env->ReleaseStringCritical(jName, str16);

            str16 = nullptr;

        }

}

}

    std::string name = str8.string();



static jlong nativeCreateAndUpdate(JNIEnv* env, jclass clazz, jstring jName, jlong surfaceControl,

                                   jlong width, jlong height, jint format) {

    ScopedUtfChars name(env, jName);

    sp<BLASTBufferQueue> queue =

    sp<BLASTBufferQueue> queue =

            new BLASTBufferQueue(name, reinterpret_cast<SurfaceControl*>(surfaceControl), width,

            new BLASTBufferQueue(name.c_str(), reinterpret_cast<SurfaceControl*>(surfaceControl),

                                 height, format);

                                 width, height, format);

    queue->incStrong((void*)nativeCreate);

    queue->incStrong((void*)nativeCreate);

    return reinterpret_cast<jlong>(queue.get());

    return reinterpret_cast<jlong>(queue.get());

}

}
@@ -96,7 +94,8 @@ static void nativeApplyPendingTransactions(JNIEnv* env, jclass clazz, jlong ptr, 
static const JNINativeMethod gMethods[] = {

static const JNINativeMethod gMethods[] = {

        /* name, signature, funcPtr */

        /* name, signature, funcPtr */

        // clang-format off

        // clang-format off

        {"nativeCreate", "(Ljava/lang/String;JJJI)J", (void*)nativeCreate},

        {"nativeCreate", "(Ljava/lang/String;)J", (void*)nativeCreate},

        {"nativeCreateAndUpdate", "(Ljava/lang/String;JJJI)J", (void*)nativeCreateAndUpdate},

        {"nativeGetSurface", "(JZ)Landroid/view/Surface;", (void*)nativeGetSurface},

        {"nativeGetSurface", "(JZ)Landroid/view/Surface;", (void*)nativeGetSurface},

        {"nativeDestroy", "(J)V", (void*)nativeDestroy},

        {"nativeDestroy", "(J)V", (void*)nativeDestroy},

        {"nativeSetSyncTransaction", "(JJZ)V", (void*)nativeSetSyncTransaction},

        {"nativeSetSyncTransaction", "(JJZ)V", (void*)nativeSetSyncTransaction},

graphics/java/android/graphics/BLASTBufferQueue.java

+7 −2
Original line number Original line Diff line number Diff line
@@ -27,8 +27,9 @@ public final class BLASTBufferQueue { 
    // Note: This field is accessed by native code.

    // Note: This field is accessed by native code.

    public long mNativeObject; // BLASTBufferQueue*

    public long mNativeObject; // BLASTBufferQueue*





    private static native long nativeCreate(String name, long surfaceControl, long width,

    private static native long nativeCreateAndUpdate(String name, long surfaceControl, long width,

            long height, int format);

            long height, int format);

    private static native long nativeCreate(String name);

    private static native void nativeDestroy(long ptr);

    private static native void nativeDestroy(long ptr);

    private static native Surface nativeGetSurface(long ptr, boolean includeSurfaceControlHandle);

    private static native Surface nativeGetSurface(long ptr, boolean includeSurfaceControlHandle);

    private static native void nativeSetSyncTransaction(long ptr, long transactionPtr,

    private static native void nativeSetSyncTransaction(long ptr, long transactionPtr,
@@ -43,7 +44,11 @@ public final class BLASTBufferQueue { 
    /** Create a new connection with the surface flinger. */

    /** Create a new connection with the surface flinger. */

    public BLASTBufferQueue(String name, SurfaceControl sc, int width, int height,

    public BLASTBufferQueue(String name, SurfaceControl sc, int width, int height,

            @PixelFormat.Format int format) {

            @PixelFormat.Format int format) {

        mNativeObject = nativeCreate(name, sc.mNativeObject, width, height, format);

        mNativeObject = nativeCreateAndUpdate(name, sc.mNativeObject, width, height, format);

    }



    public BLASTBufferQueue(String name) {

        mNativeObject = nativeCreate(name);

    }

    }





    public void destroy() {

    public void destroy() {