From 0ed7489e2b253f8119eab35b842d21fad55f0bbd Mon Sep 17 00:00:00 2001 From: Ikram Gabiyev Date: Thu, 12 Dec 2024 14:02:27 -0800 Subject: [1/2][PiP2] Move removePip animation to transition PiP1 currently relies on force-hideen flag to guarantee the order of onStop() and onPictureInPictureModeChanged() callbacks being sent to client. Therefore, PiP1 first runs the fade-out animation separately before starting a jumpcut removePip transition. In PiP2, we wanna avoid synchronization issues through moving deterministic animations into transitions. Hence, we wanna run the fadeout animation while the TRANSIT_REMOVE_PIP is playing. 1. This means we should not rely on force-hidden flag in PiP2, as that would stop activity immediately before we even start fade-out 2. Currently setting windowing mode, which updates config in Core, also ends up scheduling PiP mode change directly. Long-term we shouldn't have this direct scheduling done, as a transition will make sure to update and send config and lifecycle state updates at the right time (in this case isVisibleRequested=false -> so finishTransition would handle it). We opt to use a simpler in-Shell workaround for now by using deferConfigAtEnd, to pause the dispatch of config till transition is over. Bug: 381017000 Flag: com.android.wm.shell.enable_pip2 Test: atest PipSchedulerTest Test: atest PinnedStackTests#testStopBeforeMultiWindowCallbacksOnDismiss Change-Id: I94783796ebc470c1d500510e577b0c0e9de91b9c --- .../wm/shell/pip2/phone/PipMotionHelper.java | 2 +- .../android/wm/shell/pip2/phone/PipScheduler.java | 34 +++++++++------------- .../android/wm/shell/pip2/phone/PipTransition.java | 16 ++++++---- .../wm/shell/pip2/phone/PipSchedulerTest.java | 20 ++++++------- .../android/server/wm/ActivityTaskSupervisor.java | 5 ++-- .../server/wm/WindowOrganizerController.java | 3 ++ 6 files changed, 41 insertions(+), 39 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipMotionHelper.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipMotionHelper.java index fd387d1811fb..37296531ee34 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipMotionHelper.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipMotionHelper.java @@ -350,7 +350,7 @@ public class PipMotionHelper implements PipAppOpsListener.Callback, } cancelPhysicsAnimation(); mMenuController.hideMenu(ANIM_TYPE_DISMISS, false /* resize */); - mPipScheduler.removePipAfterAnimation(); + mPipScheduler.scheduleRemovePip(); } /** Sets the movement bounds to use to constrain PIP position animations. */ diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipScheduler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipScheduler.java index 4461a5c6a70c..7f673d2efc68 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipScheduler.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipScheduler.java @@ -122,34 +122,26 @@ public class PipScheduler { * Schedules exit PiP via expand transition. */ public void scheduleExitPipViaExpand() { - WindowContainerTransaction wct = getExitPipViaExpandTransaction(); - if (wct != null) { - mMainExecutor.execute(() -> { + mMainExecutor.execute(() -> { + if (!mPipTransitionState.isInPip()) return; + WindowContainerTransaction wct = getExitPipViaExpandTransaction(); + if (wct != null) { mPipTransitionController.startExitTransition(TRANSIT_EXIT_PIP, wct, null /* destinationBounds */); - }); - } - } - - // TODO: Optimize this by running the animation as part of the transition - /** Runs remove PiP animation and schedules remove PiP transition after the animation ends. */ - public void removePipAfterAnimation() { - SurfaceControl.Transaction tx = mSurfaceControlTransactionFactory.getTransaction(); - PipAlphaAnimator animator = mPipAlphaAnimatorSupplier.get(mContext, - mPipTransitionState.getPinnedTaskLeash(), tx, PipAlphaAnimator.FADE_OUT); - animator.setAnimationEndCallback(this::scheduleRemovePipImmediately); - animator.start(); + } + }); } /** Schedules remove PiP transition. */ - private void scheduleRemovePipImmediately() { - WindowContainerTransaction wct = getRemovePipTransaction(); - if (wct != null) { - mMainExecutor.execute(() -> { + public void scheduleRemovePip() { + mMainExecutor.execute(() -> { + if (!mPipTransitionState.isInPip()) return; + WindowContainerTransaction wct = getRemovePipTransaction(); + if (wct != null) { mPipTransitionController.startExitTransition(TRANSIT_REMOVE_PIP, wct, null /* destinationBounds */); - }); - } + } + }); } /** diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTransition.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTransition.java index 9894b91a32c2..86d9e7e2c07d 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTransition.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTransition.java @@ -276,7 +276,8 @@ public class PipTransition extends PipTransitionController implements } if (isRemovePipTransition(info)) { - return removePipImmediately(info, startTransaction, finishTransaction, finishCallback); + mPipTransitionState.setState(PipTransitionState.EXITING_PIP); + return startRemoveAnimation(info, startTransaction, finishTransaction, finishCallback); } return false; } @@ -666,13 +667,18 @@ public class PipTransition extends PipTransitionController implements return true; } - private boolean removePipImmediately(@NonNull TransitionInfo info, + private boolean startRemoveAnimation(@NonNull TransitionInfo info, @NonNull SurfaceControl.Transaction startTransaction, @NonNull SurfaceControl.Transaction finishTransaction, @NonNull Transitions.TransitionFinishCallback finishCallback) { - startTransaction.apply(); - finishCallback.onTransitionFinished(null); - mPipTransitionState.setState(PipTransitionState.EXITED_PIP); + TransitionInfo.Change pipChange = getChangeByToken(info, + mPipTransitionState.getPipTaskToken()); + mFinishCallback = finishCallback; + PipAlphaAnimator animator = new PipAlphaAnimator(mContext, pipChange.getLeash(), + startTransaction, PipAlphaAnimator.FADE_OUT); + finishTransaction.setAlpha(pipChange.getLeash(), 0f); + animator.setAnimationEndCallback(this::finishTransition); + animator.start(); return true; } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipSchedulerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipSchedulerTest.java index 3fe8c109807a..a8aa25700c7e 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipSchedulerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipSchedulerTest.java @@ -120,15 +120,22 @@ public class PipSchedulerTest { @Test public void scheduleExitPipViaExpand_nullTaskToken_noop() { setNullPipTaskToken(); + when(mMockPipTransitionState.isInPip()).thenReturn(true); mPipScheduler.scheduleExitPipViaExpand(); - verify(mMockMainExecutor, never()).execute(any()); + verify(mMockMainExecutor, times(1)).execute(mRunnableArgumentCaptor.capture()); + assertNotNull(mRunnableArgumentCaptor.getValue()); + mRunnableArgumentCaptor.getValue().run(); + + verify(mMockPipTransitionController, never()) + .startExitTransition(eq(TRANSIT_EXIT_PIP), any(), isNull()); } @Test public void scheduleExitPipViaExpand_exitTransitionCalled() { setMockPipTaskToken(); + when(mMockPipTransitionState.isInPip()).thenReturn(true); mPipScheduler.scheduleExitPipViaExpand(); @@ -142,20 +149,13 @@ public class PipSchedulerTest { @Test public void removePipAfterAnimation() { - //TODO: Update once this is changed to run animation as part of transition setMockPipTaskToken(); + when(mMockPipTransitionState.isInPip()).thenReturn(true); - mPipScheduler.removePipAfterAnimation(); - verify(mMockAlphaAnimator, times(1)) - .setAnimationEndCallback(mRunnableArgumentCaptor.capture()); - assertNotNull(mRunnableArgumentCaptor.getValue()); - verify(mMockAlphaAnimator, times(1)).start(); - - mRunnableArgumentCaptor.getValue().run(); + mPipScheduler.scheduleRemovePip(); verify(mMockMainExecutor, times(1)).execute(mRunnableArgumentCaptor.capture()); assertNotNull(mRunnableArgumentCaptor.getValue()); - mRunnableArgumentCaptor.getValue().run(); verify(mMockPipTransitionController, times(1)) diff --git a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java index a077a0b9a2ca..f29285f2c696 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityTaskSupervisor.java @@ -76,6 +76,7 @@ import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_WITH_CLAS import static com.android.server.wm.ActivityTaskManagerService.ANIMATE; import static com.android.server.wm.ActivityTaskManagerService.H.FIRST_SUPERVISOR_TASK_MSG; import static com.android.server.wm.ActivityTaskManagerService.RELAUNCH_REASON_NONE; +import static com.android.server.wm.ActivityTaskManagerService.isPip2ExperimentEnabled; import static com.android.server.wm.ClientLifecycleManager.shouldDispatchLaunchActivityItemIndependently; import static com.android.server.wm.LockTaskController.LOCK_TASK_AUTH_ALLOWLISTED; import static com.android.server.wm.LockTaskController.LOCK_TASK_AUTH_LAUNCHABLE; @@ -2523,7 +2524,7 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { void scheduleUpdatePictureInPictureModeIfNeeded(Task task, Rect targetRootTaskBounds) { task.forAllActivities(r -> { if (!r.attachedToProcess()) return; - mPipModeChangedActivities.add(r); + if (!isPip2ExperimentEnabled()) mPipModeChangedActivities.add(r); // If we are scheduling pip change, then remove this activity from multi-window // change list as the processing of pip change will make sure multi-window changed // message is processed in the right order relative to pip changed. @@ -2532,7 +2533,7 @@ public class ActivityTaskSupervisor implements RecentTasks.Callbacks { mPipModeChangedTargetRootTaskBounds = targetRootTaskBounds; - if (!mHandler.hasMessages(REPORT_PIP_MODE_CHANGED_MSG)) { + if (!isPip2ExperimentEnabled() && !mHandler.hasMessages(REPORT_PIP_MODE_CHANGED_MSG)) { mHandler.sendEmptyMessage(REPORT_PIP_MODE_CHANGED_MSG); } } diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index ddff24d35232..e6a0152f28ce 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -80,6 +80,7 @@ import static com.android.internal.protolog.WmProtoLogGroups.WM_DEBUG_WINDOW_ORG import static com.android.server.wm.ActivityRecord.State.PAUSING; import static com.android.server.wm.ActivityRecord.State.RESUMED; import static com.android.server.wm.ActivityTaskManagerService.enforceTaskPermission; +import static com.android.server.wm.ActivityTaskManagerService.isPip2ExperimentEnabled; import static com.android.server.wm.ActivityTaskSupervisor.REMOVE_FROM_RECENTS; import static com.android.server.wm.Task.FLAG_FORCE_HIDDEN_FOR_PINNED_TASK; import static com.android.server.wm.Task.FLAG_FORCE_HIDDEN_FOR_TASK_ORG; @@ -716,6 +717,8 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub } if (forceHiddenForPip) { wc.asTask().setForceHidden(FLAG_FORCE_HIDDEN_FOR_PINNED_TASK, true /* set */); + } + if (forceHiddenForPip && !isPip2ExperimentEnabled()) { // When removing pip, make sure that onStop is sent to the app ahead of // onPictureInPictureModeChanged. // See also PinnedStackTests#testStopBeforeMultiWindowCallbacksOnDismiss -- cgit v1.2.3-59-g8ed1b