From fd8a8a697be5793c5edad97adcb4aa250a1cce74 Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Fri, 21 Apr 2023 18:25:37 -0700 Subject: [PATCH] Handle some Recents finish corner cases This handles a couple of the corner-cases around recents transition and "internal" live-tile activity transitions. The first is that the change detection will check if a going-invisble activity is already invisible. This way, if the finish transaction, makes launcher invisible, the activity transition will ignore it instead of trying to animate an already-invisible activity. This works because we defer commitVisibility so that we can animate with visible content. The second is that the finish operation for transient-launch will not commitVisibility for any activities under a transient-hide task if that task didn't actually hide. This way it won't prematurely destroy activity content that is still visible/animating. Bug: 276755325 Test: Tap on activity-launch icons through gesture bar and observe fewer flickers (this doesn't solve all of them though). Can look at logs to verify that the flickers are not from these sources. Change-Id: I6d167e50ec0211e811872ae9a6aabc8164ebd65d --- .../com/android/server/wm/ActivityRecord.java | 4 ++ .../com/android/server/wm/Transition.java | 57 +++++++++++++++---- .../server/wm/TransitionController.java | 14 ++++- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index bc3a1a238c083..316d3d7233f7c 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -5296,6 +5296,10 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A if (isCollecting) { mTransitionController.collect(this); } else { + // Failsafe to make sure that we show any activities that were incorrectly hidden + // during a transition. If this vis-change is a result of finishing, ignore it. + // Finish should only ever commit visibility=false, so we can check full containment + // rather than just direct membership. inFinishingTransition = mTransitionController.inFinishingTransition(this); if (!inFinishingTransition && !mDisplayContent.isSleeping()) { Slog.e(TAG, "setVisibility=" + visible diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index 54dfdd93b26c5..5a7e075a9eafd 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -244,6 +244,16 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { private IContainerFreezer mContainerFreezer = null; private final SurfaceControl.Transaction mTmpTransaction = new SurfaceControl.Transaction(); + /** + * {@code true} if some other operation may have caused the originally-recorded state (in + * mChanges) to be dirty. This is usually due to finishTransition being called mid-collect; + * and, the reason that finish can alter the "start" state of other transitions is because + * setVisible(false) is deferred until then. + * Instead of adding this conditional, we could re-check always; but, this situation isn't + * common so it'd be wasted work. + */ + boolean mPriorVisibilityMightBeDirty = false; + final TransitionController.Logger mLogger = new TransitionController.Logger(); /** Whether this transition was forced to play early (eg for a SLEEP signal). */ @@ -959,28 +969,30 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { mController.mFinishingTransition = this; if (mTransientHideTasks != null && !mTransientHideTasks.isEmpty()) { - // Record all the now-hiding activities so that they are committed after - // recalculating visibilities. We just use mParticipants because we can and it will - // ensure proper reporting of `isInFinishTransition`. - for (int i = 0; i < mTransientHideTasks.size(); ++i) { - mTransientHideTasks.get(i).forAllActivities(r -> { - // Only check leaf-tasks that were collected - if (!mParticipants.contains(r.getTask())) return; - // Only concern ourselves with anything that can become invisible - if (!r.isVisible()) return; - mParticipants.add(r); - }); - } // The transient hide tasks could be occluded now, e.g. returning to home. So trigger // the update to make the activities in the tasks invisible-requested, then the next // step can continue to commit the visibility. mController.mAtm.mRootWindowContainer.ensureActivitiesVisible(null /* starting */, 0 /* configChanges */, true /* preserveWindows */); + // Record all the now-hiding activities so that they are committed. Just use + // mParticipants because we can avoid a new list this way. + for (int i = 0; i < mTransientHideTasks.size(); ++i) { + // Only worry about tasks that were actually hidden. Otherwise, we could end-up + // committing visibility for activity-level changes that aren't part of this + // transition. + if (mTransientHideTasks.get(i).isVisibleRequested()) continue; + mTransientHideTasks.get(i).forAllActivities(r -> { + // Only check leaf-tasks that were collected + if (!mParticipants.contains(r.getTask())) return; + mParticipants.add(r); + }); + } } boolean hasParticipatedDisplay = false; boolean hasVisibleTransientLaunch = false; boolean enterAutoPip = false; + boolean committedSomeInvisible = false; // Commit all going-invisible containers for (int i = 0; i < mParticipants.size(); ++i) { final WindowContainer participant = mParticipants.valueAt(i); @@ -1016,6 +1028,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { } ar.commitVisibility(false /* visible */, false /* performLayout */, true /* fromTransition */); + committedSomeInvisible = true; } else { enterAutoPip = true; } @@ -1074,6 +1087,9 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { } } } + if (committedSomeInvisible) { + mController.onCommittedInvisibles(); + } if (hasVisibleTransientLaunch) { // Notify the change about the transient-below task if entering auto-pip. @@ -1286,6 +1302,9 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { // leftover order changes. collectOrderChanges(mController.mWaitingTransitions.isEmpty()); + if (mPriorVisibilityMightBeDirty) { + updatePriorVisibility(); + } // Resolve the animating targets from the participants. mTargets = calculateTargets(mParticipants, mChanges); // Check whether the participants were animated from back navigation. @@ -1799,6 +1818,20 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { } } + private void updatePriorVisibility() { + for (int i = 0; i < mChanges.size(); ++i) { + final ChangeInfo chg = mChanges.valueAt(i); + // For task/activity, recalculate the current "real" visibility. + if (chg.mContainer.asActivityRecord() == null && chg.mContainer.asTask() == null) { + continue; + } + // This ONLY works in the visible -> invisible case (and is only needed for this case) + // because commitVisible(false) is deferred until finish. + if (!chg.mVisible) continue; + chg.mVisible = chg.mContainer.isVisible(); + } + } + /** * Under some conditions (eg. all visible targets within a parent container are transitioning * the same way) the transition can be "promoted" to the parent container. This means an diff --git a/services/core/java/com/android/server/wm/TransitionController.java b/services/core/java/com/android/server/wm/TransitionController.java index 8af037be8d06a..af5e53f6ee8ee 100644 --- a/services/core/java/com/android/server/wm/TransitionController.java +++ b/services/core/java/com/android/server/wm/TransitionController.java @@ -408,9 +408,9 @@ class TransitionController { return false; } - /** Returns {@code true} if the `wc` is a participant of the finishing transition. */ + /** Returns {@code true} if the finishing transition contains `wc`. */ boolean inFinishingTransition(WindowContainer wc) { - return mFinishingTransition != null && mFinishingTransition.mParticipants.contains(wc); + return mFinishingTransition != null && mFinishingTransition.isInTransition(wc); } /** @return {@code true} if a transition is running */ @@ -826,6 +826,16 @@ class TransitionController { } } + /** Called by {@link Transition#finishTransition} if it committed invisible to any activities */ + void onCommittedInvisibles() { + if (mCollectingTransition != null) { + mCollectingTransition.mPriorVisibilityMightBeDirty = true; + } + for (int i = mWaitingTransitions.size() - 1; i >= 0; --i) { + mWaitingTransitions.get(i).mPriorVisibilityMightBeDirty = true; + } + } + private void validateStates() { for (int i = 0; i < mStateValidators.size(); ++i) { mStateValidators.get(i).run();