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

Commit 65425b4d authored by Alec Mouri's avatar Alec Mouri
Browse files

Revert^2 "Drive SurfaceView visibility on renderthread" 

This includes a change to RenderNode which ensures that RenderNode
callbacks are dispatched synchronously in prepareTree(). This is because
the UI thread may be unblocked while replaying drawing commands, and we
don't wait on worker threads for a given frame until after we're done
drawing.

13be0e2c

Bug: 274519506
Bug: 269113414
Test: CtsSurfaceControlTests

Change-Id: Iae5c7d096930ec76fd54d9c41a1a479c86d18870
parent a4edd78c

core/java/android/view/SurfaceView.java

+11 −22
Original line number Original line Diff line number Diff line
@@ -33,7 +33,6 @@ import android.graphics.Color; 
import android.graphics.Matrix;

import android.graphics.Matrix;

import android.graphics.Paint;

import android.graphics.Paint;

import android.graphics.PixelFormat;

import android.graphics.PixelFormat;

import android.graphics.Point;

import android.graphics.Rect;

import android.graphics.Rect;

import android.graphics.Region;

import android.graphics.Region;

import android.graphics.RenderNode;

import android.graphics.RenderNode;
@@ -851,11 +850,15 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
            }

            }

            mParentSurfaceSequenceId = viewRoot.getSurfaceSequenceId();

            mParentSurfaceSequenceId = viewRoot.getSurfaceSequenceId();





            // Only control visibility if we're not hardware-accelerated. Otherwise we'll

            // let renderthread drive since offscreen SurfaceControls should not be visible.

            if (!isHardwareAccelerated()) {

                if (mViewVisibility) {

                if (mViewVisibility) {

                    surfaceUpdateTransaction.show(mSurfaceControl);

                    surfaceUpdateTransaction.show(mSurfaceControl);

                } else {

                } else {

                    surfaceUpdateTransaction.hide(mSurfaceControl);

                    surfaceUpdateTransaction.hide(mSurfaceControl);

                }

                }

            }





            updateBackgroundVisibility(surfaceUpdateTransaction);

            updateBackgroundVisibility(surfaceUpdateTransaction);

            updateBackgroundColor(surfaceUpdateTransaction);

            updateBackgroundColor(surfaceUpdateTransaction);
@@ -1417,12 +1420,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
    }

    }





    private final Rect mRTLastReportedPosition = new Rect();

    private final Rect mRTLastReportedPosition = new Rect();

    private final Point mRTLastReportedSurfaceSize = new Point();





    private class SurfaceViewPositionUpdateListener implements RenderNode.PositionUpdateListener {

    private class SurfaceViewPositionUpdateListener implements RenderNode.PositionUpdateListener {

        private final int mRtSurfaceWidth;

        private final int mRtSurfaceWidth;

        private final int mRtSurfaceHeight;

        private final int mRtSurfaceHeight;

        private boolean mRtFirst = true;

        private final SurfaceControl.Transaction mPositionChangedTransaction =

        private final SurfaceControl.Transaction mPositionChangedTransaction =

                new SurfaceControl.Transaction();

                new SurfaceControl.Transaction();
@@ -1433,15 +1434,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 




        @Override

        @Override

        public void positionChanged(long frameNumber, int left, int top, int right, int bottom) {

        public void positionChanged(long frameNumber, int left, int top, int right, int bottom) {

            if (!mRtFirst && (mRTLastReportedPosition.left == left

                    && mRTLastReportedPosition.top == top

                    && mRTLastReportedPosition.right == right

                    && mRTLastReportedPosition.bottom == bottom

                    && mRTLastReportedSurfaceSize.x == mRtSurfaceWidth

                    && mRTLastReportedSurfaceSize.y == mRtSurfaceHeight)) {

                return;

            }

            mRtFirst = false;

            try {

            try {

                if (DEBUG_POSITION) {

                if (DEBUG_POSITION) {

                    Log.d(TAG, String.format(

                    Log.d(TAG, String.format(
@@ -1452,8 +1444,8 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                }

                }

                synchronized (mSurfaceControlLock) {

                synchronized (mSurfaceControlLock) {

                    if (mSurfaceControl == null) return;

                    if (mSurfaceControl == null) return;



                    mRTLastReportedPosition.set(left, top, right, bottom);

                    mRTLastReportedPosition.set(left, top, right, bottom);

                    mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight);

                    onSetSurfacePositionAndScale(mPositionChangedTransaction, mSurfaceControl,

                    onSetSurfacePositionAndScale(mPositionChangedTransaction, mSurfaceControl,

                            mRTLastReportedPosition.left /*positionLeft*/,

                            mRTLastReportedPosition.left /*positionLeft*/,

                            mRTLastReportedPosition.top /*positionTop*/,

                            mRTLastReportedPosition.top /*positionTop*/,
@@ -1461,11 +1453,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                                    / (float) mRtSurfaceWidth /*postScaleX*/,

                                    / (float) mRtSurfaceWidth /*postScaleX*/,

                            mRTLastReportedPosition.height()

                            mRTLastReportedPosition.height()

                                    / (float) mRtSurfaceHeight /*postScaleY*/);

                                    / (float) mRtSurfaceHeight /*postScaleY*/);

                    if (mViewVisibility) {



                        // b/131239825

                    mPositionChangedTransaction.show(mSurfaceControl);

                    mPositionChangedTransaction.show(mSurfaceControl);

                }

                }

                }

                applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);

                applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);

            } catch (Exception ex) {

            } catch (Exception ex) {

                Log.e(TAG, "Exception from repositionChild", ex);

                Log.e(TAG, "Exception from repositionChild", ex);
@@ -1490,7 +1480,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall 
                        System.identityHashCode(this), frameNumber));

                        System.identityHashCode(this), frameNumber));

            }

            }

            mRTLastReportedPosition.setEmpty();

            mRTLastReportedPosition.setEmpty();

            mRTLastReportedSurfaceSize.set(-1, -1);





            // positionLost can be called while UI thread is un-paused.

            // positionLost can be called while UI thread is un-paused.

            synchronized (mSurfaceControlLock) {

            synchronized (mSurfaceControlLock) {

libs/hwui/jni/android_graphics_RenderNode.cpp

+26 −26
Original line number Original line Diff line number Diff line
@@ -605,15 +605,25 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, 
            }

            }

            mPreviousPosition = bounds;

            mPreviousPosition = bounds;





            ATRACE_NAME("Update SurfaceView position");



#ifdef __ANDROID__ // Layoutlib does not support CanvasContext

#ifdef __ANDROID__ // Layoutlib does not support CanvasContext

            incStrong(0);

            JNIEnv* env = jnienv();

            auto functor = std::bind(

            // Update the new position synchronously. We cannot defer this to

                std::mem_fn(&PositionListenerTrampoline::doUpdatePositionAsync), this,

            // a worker pool to process asynchronously because the UI thread

                (jlong) info.canvasContext.getFrameNumber(),

            // may be unblocked by the time a worker thread can process this,

                (jint) bounds.left, (jint) bounds.top,

            // In particular if the app removes a view from the view tree before

                (jint) bounds.right, (jint) bounds.bottom);

            // this callback is dispatched, then we lose the position



            // information for this frame.

            info.canvasContext.enqueueFrameWork(std::move(functor));

            jboolean keepListening = env->CallStaticBooleanMethod(

                    gPositionListener.clazz, gPositionListener.callPositionChanged, mListener,

                    static_cast<jlong>(info.canvasContext.getFrameNumber()),

                    static_cast<jint>(bounds.left), static_cast<jint>(bounds.top),

                    static_cast<jint>(bounds.right), static_cast<jint>(bounds.bottom));

            if (!keepListening) {

                env->DeleteGlobalRef(mListener);

                mListener = nullptr;

            }

#endif

#endif

        }

        }
@@ -628,7 +638,14 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, 
            ATRACE_NAME("SurfaceView position lost");

            ATRACE_NAME("SurfaceView position lost");

            JNIEnv* env = jnienv();

            JNIEnv* env = jnienv();

#ifdef __ANDROID__ // Layoutlib does not support CanvasContext

#ifdef __ANDROID__ // Layoutlib does not support CanvasContext

            // TODO: Remember why this is synchronous and then make a comment

            // Update the lost position synchronously. We cannot defer this to

            // a worker pool to process asynchronously because the UI thread

            // may be unblocked by the time a worker thread can process this,

            // In particular if a view's rendernode is readded to the scene

            // before this callback is dispatched, then we report that we lost

            // position information on the wrong frame, which can be problematic

            // for views like SurfaceView which rely on RenderNode callbacks

            // for driving visibility.

            jboolean keepListening = env->CallStaticBooleanMethod(

            jboolean keepListening = env->CallStaticBooleanMethod(

                    gPositionListener.clazz, gPositionListener.callPositionLost, mListener,

                    gPositionListener.clazz, gPositionListener.callPositionLost, mListener,

                    info ? info->canvasContext.getFrameNumber() : 0);

                    info ? info->canvasContext.getFrameNumber() : 0);
@@ -708,23 +725,6 @@ static void android_view_RenderNode_requestPositionUpdates(JNIEnv* env, jobject, 
            }

            }

        }

        }





        void doUpdatePositionAsync(jlong frameNumber, jint left, jint top,

                jint right, jint bottom) {

            ATRACE_NAME("Update SurfaceView position");



            JNIEnv* env = jnienv();

            jboolean keepListening = env->CallStaticBooleanMethod(

                    gPositionListener.clazz, gPositionListener.callPositionChanged, mListener,

                    frameNumber, left, top, right, bottom);

            if (!keepListening) {

                env->DeleteGlobalRef(mListener);

                mListener = nullptr;

            }



            // We need to release ourselves here

            decStrong(0);

        }



        JavaVM* mVm;

        JavaVM* mVm;

        jobject mListener;

        jobject mListener;

        uirenderer::Rect mPreviousPosition;

        uirenderer::Rect mPreviousPosition;