From bf611354207655c2bbf47385a41b37786438943e Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Wed, 20 Apr 2022 18:21:39 -0700 Subject: [PATCH] Set-up transition-related leashes on start transaction Was previously just setting up on the pending transaction. Now that operations put on pending transactions will wait until the ongoing sync transaction, the ordering was wrong. This CL puts all the changes onto the transition start transaction. Bug: 229907442 Test: launch landscape app, close (back) to portrait. Change-Id: I3e4abc5f8ab80b4cb7a23cd59ca9561d9c953e9f --- .../com/android/server/wm/Transition.java | 30 +++++++++----- .../com/android/server/wm/WindowToken.java | 13 +++++- .../android/server/wm/TransitionTests.java | 41 +++++++++++-------- 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index 342a0e52952c3..454424c0a5750 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -420,7 +420,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe for (int i = mTargets.size() - 1; i >= 0; --i) { final WindowContainer target = mTargets.get(i); if (target.getParent() != null) { - final SurfaceControl targetLeash = getLeashSurface(target); + final SurfaceControl targetLeash = getLeashSurface(target, null /* t */); final SurfaceControl origParent = getOrigParentSurface(target); // Ensure surfaceControls are re-parented back into the hierarchy. t.reparent(targetLeash, origParent); @@ -676,7 +676,8 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe // Resolve the animating targets from the participants mTargets = calculateTargets(mParticipants, mChanges); - final TransitionInfo info = calculateTransitionInfo(mType, mFlags, mTargets, mChanges); + final TransitionInfo info = calculateTransitionInfo(mType, mFlags, mTargets, mChanges, + transaction); if (mOverrideOptions != null) { info.setAnimationOptions(mOverrideOptions); } @@ -1234,8 +1235,15 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe } } - /** Gets the leash surface for a window container */ - private static SurfaceControl getLeashSurface(WindowContainer wc) { + /** + * Gets the leash surface for a window container. + * @param t a transaction to create leashes on when necessary (fixed rotation at token-level). + * If t is null, then this will not create any leashes, just use one if it is there -- + * this is relevant for building the finishTransaction since it needs to match the + * start state and not erroneously create a leash of its own. + */ + private static SurfaceControl getLeashSurface(WindowContainer wc, + @Nullable SurfaceControl.Transaction t) { final DisplayContent asDC = wc.asDisplayContent(); if (asDC != null) { // DisplayContent is the "root", so we use the windowing layer instead to avoid @@ -1247,7 +1255,8 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe if (asToken != null) { // WindowTokens can have a fixed-rotation applied to them. In the current // implementation this fact is hidden from the player, so we must create a leash. - final SurfaceControl leash = asToken.getOrCreateFixedRotationLeash(); + final SurfaceControl leash = t != null ? asToken.getOrCreateFixedRotationLeash(t) + : asToken.getFixedRotationLeash(); if (leash != null) return leash; } } @@ -1276,12 +1285,14 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe * Construct a TransitionInfo object from a set of targets and changes. Also populates the * root surface. * @param sortedTargets The targets sorted by z-order from top (index 0) to bottom. + * @param startT The start transaction - used to set-up new leashes. */ @VisibleForTesting @NonNull static TransitionInfo calculateTransitionInfo(@TransitionType int type, int flags, ArrayList sortedTargets, - ArrayMap changes) { + ArrayMap changes, + @Nullable SurfaceControl.Transaction startT) { final TransitionInfo out = new TransitionInfo(type, flags); WindowContainer topApp = null; @@ -1319,10 +1330,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe } final SurfaceControl rootLeash = leashReference.makeAnimationLeash().setName( "Transition Root: " + leashReference.getName()).build(); - SurfaceControl.Transaction t = ancestor.mWmService.mTransactionFactory.get(); - t.setLayer(rootLeash, leashReference.getLastLayer()); - t.apply(); - t.close(); + startT.setLayer(rootLeash, leashReference.getLastLayer()); out.setRootLeash(rootLeash, ancestor.getBounds().left, ancestor.getBounds().top); // Convert all the resolved ChangeInfos into TransactionInfo.Change objects in order. @@ -1332,7 +1340,7 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe final ChangeInfo info = changes.get(target); final TransitionInfo.Change change = new TransitionInfo.Change( target.mRemoteToken != null ? target.mRemoteToken.toWindowContainerToken() - : null, getLeashSurface(target)); + : null, getLeashSurface(target, startT)); // TODO(shell-transitions): Use leash for non-organized windows. if (info.mParent != null) { change.setParent(info.mParent.mRemoteToken.toWindowContainerToken()); diff --git a/services/core/java/com/android/server/wm/WindowToken.java b/services/core/java/com/android/server/wm/WindowToken.java index d2e56faa0914c..22810b294883b 100644 --- a/services/core/java/com/android/server/wm/WindowToken.java +++ b/services/core/java/com/android/server/wm/WindowToken.java @@ -37,6 +37,7 @@ import static com.android.server.wm.WindowTokenProto.WAITING_TO_SHOW; import static com.android.server.wm.WindowTokenProto.WINDOW_CONTAINER; import android.annotation.CallSuper; +import android.annotation.NonNull; import android.annotation.Nullable; import android.content.res.Configuration; import android.graphics.Rect; @@ -569,13 +570,12 @@ class WindowToken extends WindowContainer { * the same rotation. */ @Nullable - SurfaceControl getOrCreateFixedRotationLeash() { + SurfaceControl getOrCreateFixedRotationLeash(@NonNull SurfaceControl.Transaction t) { if (!mTransitionController.isShellTransitionsEnabled()) return null; final int rotation = getRelativeDisplayRotation(); if (rotation == Surface.ROTATION_0) return mFixedRotationTransformLeash; if (mFixedRotationTransformLeash != null) return mFixedRotationTransformLeash; - final SurfaceControl.Transaction t = getSyncTransaction(); final SurfaceControl leash = makeSurface().setContainerLayer() .setParent(getParentSurfaceControl()) .setName(getSurfaceControl() + " - rotation-leash") @@ -591,6 +591,15 @@ class WindowToken extends WindowContainer { return mFixedRotationTransformLeash; } + /** + * @return the leash which represents this window as if it was non-rotated. Will be null if + * there isn't one. + */ + @Nullable + SurfaceControl getFixedRotationLeash() { + return mFixedRotationTransformLeash; + } + void removeFixedRotationLeash() { if (mFixedRotationTransformLeash == null) return; final SurfaceControl.Transaction t = getSyncTransaction(); 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 9dedc850defc0..16adffdba6c35 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java @@ -85,6 +85,7 @@ import java.util.function.Function; @Presubmit @RunWith(WindowTestRunner.class) public class TransitionTests extends WindowTestsBase { + final SurfaceControl.Transaction mMockT = mock(SurfaceControl.Transaction.class); private Transition createTestTransition(int transitType) { TransitionController controller = mock(TransitionController.class); @@ -121,7 +122,8 @@ public class TransitionTests extends WindowTestsBase { participants.add(oldTask); participants.add(newTask); ArrayList targets = Transition.calculateTargets(participants, changes); - TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes, + mMockT); assertEquals(2, info.getChanges().size()); assertEquals(transit, info.getType()); @@ -129,7 +131,7 @@ public class TransitionTests extends WindowTestsBase { participants.add(opening); participants.add(closing); targets = Transition.calculateTargets(participants, changes); - info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + info = Transition.calculateTransitionInfo(transit, flags, targets, changes, mMockT); assertEquals(2, info.getChanges().size()); assertNotNull(info.getChange(newTask.mRemoteToken.toWindowContainerToken())); assertNotNull(info.getChange(oldTask.mRemoteToken.toWindowContainerToken())); @@ -137,7 +139,7 @@ public class TransitionTests extends WindowTestsBase { // Check combined prune and promote participants.remove(newTask); targets = Transition.calculateTargets(participants, changes); - info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + info = Transition.calculateTransitionInfo(transit, flags, targets, changes, mMockT); assertEquals(2, info.getChanges().size()); assertNotNull(info.getChange(newTask.mRemoteToken.toWindowContainerToken())); assertNotNull(info.getChange(oldTask.mRemoteToken.toWindowContainerToken())); @@ -145,7 +147,7 @@ public class TransitionTests extends WindowTestsBase { // Check multi promote participants.remove(oldTask); targets = Transition.calculateTargets(participants, changes); - info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + info = Transition.calculateTransitionInfo(transit, flags, targets, changes, mMockT); assertEquals(2, info.getChanges().size()); assertNotNull(info.getChange(newTask.mRemoteToken.toWindowContainerToken())); assertNotNull(info.getChange(oldTask.mRemoteToken.toWindowContainerToken())); @@ -186,7 +188,8 @@ public class TransitionTests extends WindowTestsBase { participants.add(opening); participants.add(opening2); ArrayList targets = Transition.calculateTargets(participants, changes); - TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes, + mMockT); assertEquals(2, info.getChanges().size()); assertEquals(transit, info.getType()); assertNotNull(info.getChange(newTask.mRemoteToken.toWindowContainerToken())); @@ -195,7 +198,7 @@ public class TransitionTests extends WindowTestsBase { // Check that unchanging but visible descendant of sibling prevents promotion participants.remove(opening2); targets = Transition.calculateTargets(participants, changes); - info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + info = Transition.calculateTransitionInfo(transit, flags, targets, changes, mMockT); assertEquals(2, info.getChanges().size()); assertNotNull(info.getChange(newNestedTask.mRemoteToken.toWindowContainerToken())); assertNotNull(info.getChange(oldTask.mRemoteToken.toWindowContainerToken())); @@ -232,7 +235,8 @@ public class TransitionTests extends WindowTestsBase { participants.add(showing); participants.add(showing2); ArrayList targets = Transition.calculateTargets(participants, changes); - TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes, + mMockT); assertEquals(1, info.getChanges().size()); assertEquals(transit, info.getType()); assertNotNull(info.getChange(tda.mRemoteToken.toWindowContainerToken())); @@ -240,14 +244,14 @@ public class TransitionTests extends WindowTestsBase { // Check that organized tasks get reported even if not top makeTaskOrganized(showTask); targets = Transition.calculateTargets(participants, changes); - info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + info = Transition.calculateTransitionInfo(transit, flags, targets, changes, mMockT); assertEquals(2, info.getChanges().size()); assertNotNull(info.getChange(tda.mRemoteToken.toWindowContainerToken())); assertNotNull(info.getChange(showTask.mRemoteToken.toWindowContainerToken())); // Even if DisplayArea explicitly participating participants.add(tda); targets = Transition.calculateTargets(participants, changes); - info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + info = Transition.calculateTransitionInfo(transit, flags, targets, changes, mMockT); assertEquals(2, info.getChanges().size()); } @@ -271,7 +275,7 @@ public class TransitionTests extends WindowTestsBase { ArrayList targets = Transition.calculateTargets( transition.mParticipants, transition.mChanges); TransitionInfo info = Transition.calculateTransitionInfo( - 0, 0, targets, transition.mChanges); + 0, 0, targets, transition.mChanges, mMockT); assertEquals(2, info.getChanges().size()); // There was an existence change on open, so it should be OPEN rather than SHOW assertEquals(TRANSIT_OPEN, @@ -308,7 +312,7 @@ public class TransitionTests extends WindowTestsBase { ArrayList targets = Transition.calculateTargets( transition.mParticipants, transition.mChanges); TransitionInfo info = Transition.calculateTransitionInfo( - 0, 0, targets, transition.mChanges); + 0, 0, targets, transition.mChanges, mMockT); assertEquals(taskCount, info.getChanges().size()); // verify order is top-to-bottem for (int i = 0; i < taskCount; ++i) { @@ -358,7 +362,7 @@ public class TransitionTests extends WindowTestsBase { ArrayList targets = Transition.calculateTargets( transition.mParticipants, transition.mChanges); TransitionInfo info = Transition.calculateTransitionInfo( - 0, 0, targets, transition.mChanges); + 0, 0, targets, transition.mChanges, mMockT); // verify that wallpaper is at bottom assertEquals(taskCount + 1, info.getChanges().size()); // The wallpaper is not organized, so it won't have a token; however, it will be marked @@ -392,7 +396,7 @@ public class TransitionTests extends WindowTestsBase { ArrayList targets = Transition.calculateTargets( transition.mParticipants, transition.mChanges); TransitionInfo info = Transition.calculateTransitionInfo( - 0, 0, targets, transition.mChanges); + 0, 0, targets, transition.mChanges, mMockT); // The wallpaper is not organized, so it won't have a token; however, it will be marked // as IS_WALLPAPER assertEquals(FLAG_IS_WALLPAPER, info.getChanges().get(0).getFlags()); @@ -474,7 +478,8 @@ public class TransitionTests extends WindowTestsBase { participants.add(changeTask); final ArrayList targets = Transition.calculateTargets(participants, changes); - TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes, + mMockT); // Root changes should always be considered independent assertTrue(isIndependent( info.getChange(openTask.mRemoteToken.toWindowContainerToken()), info)); @@ -526,7 +531,8 @@ public class TransitionTests extends WindowTestsBase { participants.add(oldTask); participants.add(newTask); ArrayList targets = Transition.calculateTargets(participants, changes); - TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes, + mMockT); assertEquals(2, info.getChanges().size()); assertEquals(transit, info.getType()); @@ -566,7 +572,8 @@ public class TransitionTests extends WindowTestsBase { participants.add(oldTask); participants.add(newTask); ArrayList targets = Transition.calculateTargets(participants, changes); - TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes); + TransitionInfo info = Transition.calculateTransitionInfo(transit, flags, targets, changes, + mMockT); assertEquals(2, info.getChanges().size()); assertEquals(transit, info.getType()); @@ -608,7 +615,7 @@ public class TransitionTests extends WindowTestsBase { final int flags = 0; final TransitionInfo info = Transition.calculateTransitionInfo(transition.mType, flags, Transition.calculateTargets(transition.mParticipants, transition.mChanges), - transition.mChanges); + transition.mChanges, mMockT); transition.abort(); return info.getChanges().get(0); };