From 2d0bf6b84b2e2299bb7c275dfe3210f06f7edc1b Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Sat, 7 Jan 2023 23:25:10 -0600 Subject: [PATCH] Defer committing visible until transition ready So some logic depend on ActivityRecord#isVisible() won't be called earlier before transition ready that may delay the animation. Such as updateFocusedWindow in activityPaused and updateSystemBarAttributes when starting window performs relayout. Skip the assertion pipAppWindowAlwaysVisible for "swipe up and fade in pip" because either "always visible" or "visible->invisible ->visible" is fine for internal state. The visual appearance is still asserted by layer visibility. Bug: 260059642 Test: TransitionTests#testIntermediateVisibility Test: OpenAppMicrobenchmark with shell transition. Change-Id: I4584880dd91f9b406bc5241a67eebae179a7fe40 --- .../pip/EnterPipOnUserLeaveHintTest.kt | 12 +++++++++++ .../com/android/server/wm/ActivityRecord.java | 17 ++++++++++++--- .../com/android/server/wm/Transition.java | 21 ++++++++++++++++++- .../com/android/server/wm/WindowState.java | 13 ++++++++++++ .../android/server/wm/TransitionTests.java | 17 ++++++++++++--- 5 files changed, 73 insertions(+), 7 deletions(-) diff --git a/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/pip/EnterPipOnUserLeaveHintTest.kt b/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/pip/EnterPipOnUserLeaveHintTest.kt index 8a694f770ab04..7aa40e7088504 100644 --- a/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/pip/EnterPipOnUserLeaveHintTest.kt +++ b/libs/WindowManager/Shell/tests/flicker/src/com/android/wm/shell/flicker/pip/EnterPipOnUserLeaveHintTest.kt @@ -68,6 +68,18 @@ open class EnterPipOnUserLeaveHintTest(flicker: FlickerTest) : EnterPipTest(flic transitions { tapl.goHome() } } + @Presubmit + @Test + override fun pipAppWindowAlwaysVisible() { + // In gestural nav the pip will first move behind home and then above home. The visual + // appearance visible->invisible->visible is asserted by pipAppLayerAlwaysVisible(). + // But the internal states of activity don't need to follow that, such as a temporary + // visibility state can be changed quickly outside a transaction so the test doesn't + // detect that. Hence, skip the case to avoid restricting the internal implementation. + Assume.assumeFalse(flicker.scenario.isGesturalNavigation) + super.pipAppWindowAlwaysVisible() + } + @Presubmit @Test override fun pipAppLayerAlwaysVisible() { diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 90d25eec5bf32..4f87b1a22f029 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -5257,9 +5257,9 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A transferStartingWindowFromHiddenAboveTokenIfNeeded(); } - // If in a transition, defer commits for activities that are going invisible - if (!visible && inTransition()) { - if (mTransitionController.inPlayingTransition(this) + // Defer committing visibility until transition starts. + if (inTransition()) { + if (!visible && mTransitionController.inPlayingTransition(this) && mTransitionController.isCollecting(this)) { mTransitionChangeFlags |= FLAG_IS_OCCLUDED; } @@ -5509,6 +5509,17 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A } } + /** Updates draw state and shows drawn windows. */ + void commitFinishDrawing(SurfaceControl.Transaction t) { + boolean committed = false; + for (int i = mChildren.size() - 1; i >= 0; i--) { + committed |= mChildren.get(i).commitFinishDrawing(t); + } + if (committed) { + requestUpdateWallpaperIfNeeded(); + } + } + /** * Check if visibility of this {@link ActivityRecord} should be updated as part of an app * transition. diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index cd23959a265e3..fa483535761b9 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -845,7 +845,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { final ActivityRecord ar = mParticipants.valueAt(i).asActivityRecord(); if (ar == null || !ar.isVisible() || ar.getParent() == null) continue; if (inputSinkTransaction == null) { - inputSinkTransaction = new SurfaceControl.Transaction(); + inputSinkTransaction = ar.mWmService.mTransactionFactory.get(); } ar.mActivityRecordInputSink.applyChangesToSurfaceIfChanged(inputSinkTransaction); } @@ -961,6 +961,12 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { // time being, we don't have full cross-display transitions so it isn't a problem. final DisplayContent dc = mTargetDisplays.get(0); + // Commit the visibility of visible activities before calculateTransitionInfo(), so the + // TaskInfo can be visible. Also it needs to be done before moveToPlaying(), otherwise + // ActivityRecord#canShowWindows() may reject to show its window. The visibility also + // needs to be updated for STATE_ABORT. + commitVisibleActivities(transaction); + if (mState == STATE_ABORT) { mController.abort(this); dc.getPendingTransaction().merge(transaction); @@ -1132,6 +1138,19 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { } } + /** The transition is ready to play. Make the start transaction show the surfaces. */ + private void commitVisibleActivities(SurfaceControl.Transaction transaction) { + for (int i = mParticipants.size() - 1; i >= 0; --i) { + final ActivityRecord ar = mParticipants.valueAt(i).asActivityRecord(); + if (ar == null || !ar.isVisibleRequested()) { + continue; + } + ar.commitVisibility(true /* visible */, false /* performLayout */, + true /* fromTransition */); + ar.commitFinishDrawing(transaction); + } + } + /** @see RecentsAnimationController#attachNavigationBarToApp */ private void handleLegacyRecentsStartBehavior(DisplayContent dc, TransitionInfo info) { if ((mFlags & TRANSIT_FLAG_IS_RECENTS) == 0) { diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 7a0070bf2b56a..836d222a81443 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -4616,6 +4616,19 @@ class WindowState extends WindowContainer implements WindowManagerP } } + /** Makes the surface of drawn window (COMMIT_DRAW_PENDING) to be visible. */ + boolean commitFinishDrawing(SurfaceControl.Transaction t) { + boolean committed = mWinAnimator.commitFinishDrawingLocked(); + if (committed) { + // Ensure that the visibility of buffer layer is set. + mWinAnimator.prepareSurfaceLocked(t); + } + for (int i = mChildren.size() - 1; i >= 0; i--) { + committed |= mChildren.get(i).commitFinishDrawing(t); + } + return committed; + } + // This must be called while inside a transaction. boolean performShowLocked() { if (!showToCurrentUser()) { diff --git a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java index 0231b4644953a..21135e0a80cc6 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java @@ -564,20 +564,23 @@ public class TransitionTests extends WindowTestsBase { final WindowProcessController delegateProc = mSystemServicesTestRule.addProcess( "pkg.delegate", "proc.delegate", 6000 /* pid */, 6000 /* uid */); doReturn(mock(IBinder.class)).when(delegateProc.getThread()).asBinder(); - final ActivityRecord app = new ActivityBuilder(mAtm).setCreateTask(true).build(); + final ActivityRecord app = new ActivityBuilder(mAtm).setCreateTask(true) + .setVisible(false).build(); + app.setVisibleRequested(true); final TransitionController controller = app.mTransitionController; final Transition transition = controller.createTransition(TRANSIT_OPEN); final RemoteTransition remoteTransition = new RemoteTransition( mock(IRemoteTransition.class)); remoteTransition.setAppThread(delegateProc.getThread()); - transition.collectExistenceChange(app.getTask()); - controller.requestStartTransition(transition, app.getTask(), remoteTransition, + transition.collect(app); + controller.requestStartTransition(transition, null /* startTask */, remoteTransition, null /* displayChange */); testPlayer.startTransition(); testPlayer.onTransactionReady(app.getSyncTransaction()); assertTrue(playerProc.isRunningRemoteTransition()); assertTrue(delegateProc.isRunningRemoteTransition()); assertTrue(controller.mRemotePlayer.reportRunning(delegateProc.getThread())); + assertTrue(app.isVisible()); testPlayer.finish(); assertFalse(playerProc.isRunningRemoteTransition()); @@ -1114,6 +1117,14 @@ public class TransitionTests extends WindowTestsBase { assertFalse(activity1.isVisible()); assertTrue(activity2.isVisible()); + + // The abort should still commit visible-requested to visible. + final Transition abortTransition = controller.createTransition(TRANSIT_OPEN); + abortTransition.collect(activity1); + activity1.setVisibleRequested(true); + activity1.setVisible(false); + abortTransition.abort(); + assertTrue(activity1.isVisible()); } @Test