From c00c3d93d892edc4d26e6df61c405fdd248b0805 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 12 Jan 2022 18:00:10 +0800 Subject: [PATCH] Remove CounterRotator surface in finishTransaction Before, the surface will be removed, and the children will be reparented to root in a separate transaction when animation is done. That would cause a flicker because the reparented children may be above the rotation activity. Bug: 206094140 Test: manually test expanding PIP Change-Id: I0f16b7590d4bd0f4c377158d555cb94f28f134cd --- .../android/wm/shell/pip/PipTransition.java | 11 ++++++---- .../transition/CounterRotatorHelper.java | 13 +++++++----- .../transition/DefaultTransitionHandler.java | 2 +- .../android/wm/shell/util/CounterRotator.java | 21 ++++++------------- .../system/RemoteAnimationAdapterCompat.java | 8 ++++--- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java index 1716380c1f7c8..69e78364c5e16 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java @@ -155,7 +155,8 @@ public class PipTransition extends PipTransitionController { switch (type) { case TRANSIT_EXIT_PIP: - startExitAnimation(info, startTransaction, finishCallback, exitPipChange); + startExitAnimation(info, startTransaction, finishTransaction, finishCallback, + exitPipChange); break; case TRANSIT_EXIT_PIP_TO_SPLIT: startExitToSplitAnimation(info, startTransaction, finishTransaction, @@ -283,6 +284,7 @@ public class PipTransition extends PipTransitionController { private void startExitAnimation(@NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction startTransaction, + @NonNull SurfaceControl.Transaction finishTransaction, @NonNull Transitions.TransitionFinishCallback finishCallback, @NonNull TransitionInfo.Change pipChange) { TransitionInfo.Change displayRotationChange = null; @@ -298,8 +300,8 @@ public class PipTransition extends PipTransitionController { if (displayRotationChange != null) { // Exiting PIP to fullscreen with orientation change. - startExpandAndRotationAnimation(info, startTransaction, finishCallback, - displayRotationChange, pipChange); + startExpandAndRotationAnimation(info, startTransaction, finishTransaction, + finishCallback, displayRotationChange, pipChange); return; } @@ -322,6 +324,7 @@ public class PipTransition extends PipTransitionController { private void startExpandAndRotationAnimation(@NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction startTransaction, + @NonNull SurfaceControl.Transaction finishTransaction, @NonNull Transitions.TransitionFinishCallback finishCallback, @NonNull TransitionInfo.Change displayRotationChange, @NonNull TransitionInfo.Change pipChange) { @@ -335,7 +338,6 @@ public class PipTransition extends PipTransitionController { rotator.handleClosingChanges(info, startTransaction, rotateDelta, displayW, displayH); mFinishCallback = (wct, wctCB) -> { - rotator.cleanUp(); mPipOrganizer.onExitPipFinished(pipChange.getTaskInfo()); finishCallback.onTransitionFinished(wct, wctCB); }; @@ -366,6 +368,7 @@ public class PipTransition extends PipTransitionController { endBounds, startBounds, new Rect(), degree, x, y, true /* isExpanding */, pipRotateDelta == ROTATION_270 /* clockwise */); startTransaction.apply(); + rotator.cleanUp(finishTransaction); // Expand and rotate the pip window to fullscreen. final PipAnimationController.PipTransitionAnimator animator = diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/CounterRotatorHelper.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/CounterRotatorHelper.java index 08c99b2c4a83f..8c71f749b2c36 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/CounterRotatorHelper.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/CounterRotatorHelper.java @@ -35,13 +35,11 @@ import java.util.List; */ public class CounterRotatorHelper { private final ArrayMap mRotatorMap = new ArrayMap<>(); - private SurfaceControl mRootLeash; /** Puts the surface controls of closing changes to counter-rotated surfaces. */ public void handleClosingChanges(@NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction startTransaction, int rotateDelta, int displayW, int displayH) { - mRootLeash = info.getRootLeash(); final List changes = info.getChanges(); final int numChanges = changes.size(); for (int i = numChanges - 1; i >= 0; --i) { @@ -71,10 +69,15 @@ public class CounterRotatorHelper { } } - /** Restores to the original state, i.e. reparent back to transition root. */ - public void cleanUp() { + /** + * Removes the counter rotation surface in the finish transaction. No need to reparent the + * children as the finish transaction should have already taken care of that. + * + * This can only be called after startTransaction for {@link #handleClosingChanges} is applied. + */ + public void cleanUp(@NonNull SurfaceControl.Transaction finishTransaction) { for (int i = mRotatorMap.size() - 1; i >= 0; --i) { - mRotatorMap.valueAt(i).cleanUp(mRootLeash); + mRotatorMap.valueAt(i).cleanUp(finishTransaction); } mRotatorMap.clear(); } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java index 5833ca80d3842..11b23875437f9 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java @@ -282,7 +282,6 @@ public class DefaultTransitionHandler implements Transitions.TransitionHandler { final Runnable onAnimFinish = () -> { if (!animations.isEmpty()) return; - rotator.cleanUp(); if (mRotationAnimation != null) { mRotationAnimation.kill(); mRotationAnimation = null; @@ -382,6 +381,7 @@ public class DefaultTransitionHandler implements Transitions.TransitionHandler { } startTransaction.apply(); + rotator.cleanUp(finishTransaction); TransitionMetrics.getInstance().reportAnimationStart(transition); // run finish now in-case there are no animations onAnimFinish.run(); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/util/CounterRotator.java b/libs/WindowManager/Shell/src/com/android/wm/shell/util/CounterRotator.java index b9b671635010e..7f8eaf1a9b559 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/util/CounterRotator.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/util/CounterRotator.java @@ -18,14 +18,11 @@ package com.android.wm.shell.util; import android.view.SurfaceControl; -import java.util.ArrayList; - /** * Utility class that takes care of counter-rotating surfaces during a transition animation. */ public class CounterRotator { - SurfaceControl mSurface = null; - ArrayList mRotateChildren = null; + private SurfaceControl mSurface = null; /** Gets the surface with the counter-rotation. */ public SurfaceControl getSurface() { @@ -41,7 +38,6 @@ public class CounterRotator { public void setup(SurfaceControl.Transaction t, SurfaceControl parent, int rotateDelta, float displayW, float displayH) { if (rotateDelta == 0) return; - mRotateChildren = new ArrayList<>(); // We want to counter-rotate, so subtract from 4 rotateDelta = 4 - (rotateDelta + 4) % 4; mSurface = new SurfaceControl.Builder() @@ -64,24 +60,19 @@ public class CounterRotator { } /** - * Add a surface that needs to be counter-rotate. + * Adds a surface that needs to be counter-rotate. */ public void addChild(SurfaceControl.Transaction t, SurfaceControl child) { if (mSurface == null) return; t.reparent(child, mSurface); - mRotateChildren.add(child); } /** - * Clean-up. This undoes any reparenting and effectively stops the counter-rotation. + * Clean-up. Since finishTransaction should reset all change leashes, we only need to remove the + * counter rotation surface. */ - public void cleanUp(SurfaceControl rootLeash) { + public void cleanUp(SurfaceControl.Transaction finishTransaction) { if (mSurface == null) return; - SurfaceControl.Transaction t = new SurfaceControl.Transaction(); - for (int i = mRotateChildren.size() - 1; i >= 0; --i) { - t.reparent(mRotateChildren.get(i), rootLeash); - } - t.remove(mSurface); - t.apply(); + finishTransaction.remove(mSurface); } } diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationAdapterCompat.java b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationAdapterCompat.java index e84b552d65eb5..8d26ddd5aa23e 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationAdapterCompat.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteAnimationAdapterCompat.java @@ -205,8 +205,10 @@ public class RemoteAnimationAdapterCompat { @Override @SuppressLint("NewApi") public void run() { - counterLauncher.cleanUp(info.getRootLeash()); - counterWallpaper.cleanUp(info.getRootLeash()); + final SurfaceControl.Transaction finishTransaction = + new SurfaceControl.Transaction(); + counterLauncher.cleanUp(finishTransaction); + counterWallpaper.cleanUp(finishTransaction); // Release surface references now. This is apparently to free GPU memory // while doing quick operations (eg. during CTS). for (int i = info.getChanges().size() - 1; i >= 0; --i) { @@ -216,7 +218,7 @@ public class RemoteAnimationAdapterCompat { leashMap.valueAt(i).release(); } try { - finishCallback.onTransitionFinished(null /* wct */, null /* sct */); + finishCallback.onTransitionFinished(null /* wct */, finishTransaction); } catch (RemoteException e) { Log.e("ActivityOptionsCompat", "Failed to call app controlled animation" + " finished callback", e);