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

Commit 1aa12755 authored by chaviw's avatar chaviw
Browse files

Use destroy in transaction for animation

Previously, the code calls destroy which will be invoked immediately so
there needed to be ways to delay destroy. This caused some overhead
where there needed to be Runnables to ensure client state updates were
called before the destroy. Instead, use the Transaction.destroy so the
destroy doesn't get invoked until apply is called. This allows any other
client states to get updated before the destroy is called.

This is specifically necessary for reparent calls since a Surface can be
reparented from a Surface that's about to get destroyed to a valid
Surface that's not getting destroyed. This change ensures that Surfaces
will get reparented before Surfaces are destroyed when Transactions are
applied.

Change-Id: I25bb94378de2c03565ee7df8ab5ef393b57f3aec
Fixes: 72953020
Fixes: 71499373
Test: Tested issue from bug
Test: Set animation duration to 0. Ensure no crash
Test: SurfaceAnimatorTest
parent 44c16575
Loading
Loading
Loading
Loading
+11 −0
Original line number Diff line number Diff line
@@ -152,6 +152,7 @@ public class SurfaceControl implements Parcelable {
    private static native void nativeSeverChildren(long transactionObj, long nativeObject);
    private static native void nativeSetOverrideScalingMode(long transactionObj, long nativeObject,
            int scalingMode);
    private static native void nativeDestroy(long transactionObj, long nativeObject);
    private static native IBinder nativeGetHandle(long nativeObject);
    private static native boolean nativeGetTransformToDisplayInverse(long nativeObject);

@@ -1570,6 +1571,16 @@ public class SurfaceControl implements Parcelable {
            return this;
        }

        /**
         * Same as {@link #destroy()} except this is invoked in a transaction instead of
         * immediately.
         */
        public Transaction destroy(SurfaceControl sc) {
            sc.checkNotReleased();
            nativeDestroy(mNativeObject, sc.mNativeObject);
            return this;
        }

        public Transaction setDisplaySurface(IBinder displayToken, Surface surface) {
            if (displayToken == null) {
                throw new IllegalArgumentException("displayToken must not be null");
+10 −0
Original line number Diff line number Diff line
@@ -846,6 +846,14 @@ static void nativeSetOverrideScalingMode(JNIEnv* env, jclass clazz, jlong transa
    transaction->setOverrideScalingMode(ctrl, scalingMode);
}

static void nativeDestroyInTransaction(JNIEnv* env, jclass clazz,
                                       jlong transactionObj,
                                       jlong nativeObject) {
    auto transaction = reinterpret_cast<SurfaceComposerClient::Transaction*>(transactionObj);
    auto ctrl = reinterpret_cast<SurfaceControl*>(nativeObject);
    transaction->destroySurface(ctrl);
}

static jobject nativeGetHandle(JNIEnv* env, jclass clazz, jlong nativeObject) {
    auto ctrl = reinterpret_cast<SurfaceControl *>(nativeObject);
    return javaObjectForIBinder(env, ctrl->getHandle());
@@ -997,6 +1005,8 @@ static const JNINativeMethod sSurfaceControlMethods[] = {
            (void*)nativeSeverChildren } ,
    {"nativeSetOverrideScalingMode", "(JJI)V",
            (void*)nativeSetOverrideScalingMode },
    {"nativeDestroy", "(JJ)V",
            (void*)nativeDestroyInTransaction },
    {"nativeGetHandle", "(J)Landroid/os/IBinder;",
            (void*)nativeGetHandle },
    {"nativeScreenshotToBuffer",
+1 −7
Original line number Diff line number Diff line
@@ -53,8 +53,7 @@ class AppWindowThumbnail implements Animatable {

    AppWindowThumbnail(Transaction t, AppWindowToken appToken, GraphicBuffer thumbnailHeader) {
        mAppToken = appToken;
        mSurfaceAnimator = new SurfaceAnimator(this, this::onAnimationFinished,
                appToken.mService.mAnimator::addAfterPrepareSurfacesRunnable, appToken.mService);
        mSurfaceAnimator = new SurfaceAnimator(this, this::onAnimationFinished, appToken.mService);
        mWidth = thumbnailHeader.getWidth();
        mHeight = thumbnailHeader.getHeight();

@@ -144,11 +143,6 @@ class AppWindowThumbnail implements Animatable {
        mAppToken.commitPendingTransaction();
    }

    @Override
    public void destroyAfterPendingTransaction(SurfaceControl surface) {
        mAppToken.destroyAfterPendingTransaction(surface);
    }

    @Override
    public void onAnimationLeashCreated(Transaction t, SurfaceControl leash) {
        t.setLayer(leash, Integer.MAX_VALUE);
+1 −6
Original line number Diff line number Diff line
@@ -54,11 +54,6 @@ class Dimmer {
        public void onAnimationLeashDestroyed(SurfaceControl.Transaction t) {
        }

        @Override
        public void destroyAfterPendingTransaction(SurfaceControl surface) {
            mHost.destroyAfterPendingTransaction(surface);
        }

        @Override
        public SurfaceControl.Builder makeAnimationLeash() {
            return mHost.makeAnimationLeash();
@@ -119,7 +114,7 @@ class Dimmer {
                if (!mDimming) {
                    mDimLayer.destroy();
                }
            }, mHost.mService.mAnimator::addAfterPrepareSurfacesRunnable, mHost.mService);
            }, mHost.mService);
        }
    }

+1 −25
Original line number Diff line number Diff line
@@ -380,11 +380,6 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
     */
    private int mSurfaceSize;

    /**
     * A list of surfaces to be destroyed after {@link #mPendingTransaction} is applied.
     */
    private final ArrayList<SurfaceControl> mPendingDestroyingSurfaces = new ArrayList<>();

    /** Temporary float array to retrieve 3x3 matrix values. */
    private final float[] mTmpFloats = new float[9];

@@ -1935,10 +1930,6 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
                }
            }
            mService.mAnimator.removeDisplayLocked(mDisplayId);

            // The pending transaction won't be applied so we should
            // just clean up any surfaces pending destruction.
            onPendingTransactionApplied();
        } finally {
            mRemovingDisplay = false;
        }
@@ -3846,22 +3837,6 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
        t.setRelativeLayer(child.getSurfaceControl(), mImeWindowsContainers.getSurfaceControl(), 1);
    }

    @Override
    public void destroyAfterPendingTransaction(SurfaceControl surface) {
        mPendingDestroyingSurfaces.add(surface);
    }

    /**
     * Destroys any surfaces that have been put into the pending list with
     * {@link #destroyAfterPendingTransaction}.
     */
    void onPendingTransactionApplied() {
        for (int i = mPendingDestroyingSurfaces.size() - 1; i >= 0; i--) {
            mPendingDestroyingSurfaces.get(i).destroy();
        }
        mPendingDestroyingSurfaces.clear();
    }

    @Override
    void prepareSurfaces() {
        final ScreenRotationAnimation screenRotationAnimation =
@@ -3876,6 +3851,7 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
            mPendingTransaction.setAlpha(mWindowingLayer,
                    screenRotationAnimation.getEnterTransformation().getAlpha());
        }

        super.prepareSurfaces();
    }
}
Loading