From fba32157545fde924987a2fefef50c6cf067f34a Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Thu, 9 Feb 2023 16:04:21 +0000 Subject: [PATCH] Ensure to reset seamless transform after transition If the start transaction of rotation transition is committed after transition timeout, the unrotate transform will be applied after resetting transform for timeout. That will cause the surfaces of seamless windows (e.g. cutout or button bar) to show in wrong rotation. So by using sync transaction, the sync engine can apply the reset in correct order from CommitCallback. Also make shell transition no longer use setOrientationChanging, because it is used for legacy frozen state that shell transition doesn't use. That avoids prematurely notifing the async rotation controller that a window has completed rotation. Bug: 266711229 Test: atest TransitionTests#testDisplayRotationChange Change-Id: I8484d38c1a2dfc77f371d2ba3c9848f2341b8f4a --- .../server/wm/AsyncRotationController.java | 16 +++++++++++----- .../com/android/server/wm/DisplayContent.java | 12 ++++-------- .../com/android/server/wm/TransitionTests.java | 18 ++++-------------- 3 files changed, 19 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/wm/AsyncRotationController.java b/services/core/java/com/android/server/wm/AsyncRotationController.java index 7d9ae87517b0b..dcafe80686ad1 100644 --- a/services/core/java/com/android/server/wm/AsyncRotationController.java +++ b/services/core/java/com/android/server/wm/AsyncRotationController.java @@ -223,7 +223,7 @@ class AsyncRotationController extends FadeAnimationController implements Consume if (op == null) return; if (op.mDrawTransaction != null) { // Unblock the window to show its latest content. - mDisplayContent.getPendingTransaction().merge(op.mDrawTransaction); + windowToken.getSyncTransaction().merge(op.mDrawTransaction); op.mDrawTransaction = null; if (DEBUG) Slog.d(TAG, "finishOp merge transaction " + windowToken.getTopChild()); } @@ -235,7 +235,7 @@ class AsyncRotationController extends FadeAnimationController implements Consume } else if (op.mAction == Operation.ACTION_SEAMLESS && mRotator != null && op.mLeash != null && op.mLeash.isValid()) { if (DEBUG) Slog.d(TAG, "finishOp undo seamless " + windowToken.getTopChild()); - mRotator.setIdentityMatrix(mDisplayContent.getPendingTransaction(), op.mLeash); + mRotator.setIdentityMatrix(windowToken.getSyncTransaction(), op.mLeash); } } @@ -322,7 +322,8 @@ class AsyncRotationController extends FadeAnimationController implements Consume if (mTimeoutRunnable == null) { mTimeoutRunnable = () -> { synchronized (mService.mGlobalLock) { - Slog.i(TAG, "Async rotation timeout: " + mTargetWindowTokens); + Slog.i(TAG, "Async rotation timeout: " + (!mIsStartTransactionCommitted + ? " start transaction is not committed" : mTargetWindowTokens)); mIsStartTransactionCommitted = true; mDisplayContent.finishAsyncRotationIfPossible(); mService.mWindowPlacerLocked.performSurfacePlacement(); @@ -484,12 +485,17 @@ class AsyncRotationController extends FadeAnimationController implements Consume * by this controller. */ boolean handleFinishDrawing(WindowState w, SurfaceControl.Transaction postDrawTransaction) { - if (mTransitionOp == OP_LEGACY || postDrawTransaction == null || !mIsSyncDrawRequested) { + if (mTransitionOp == OP_LEGACY) { return false; } final Operation op = mTargetWindowTokens.get(w.mToken); - if (op == null || op.canDrawBeforeStartTransaction()) return false; + if (op == null) return false; if (DEBUG) Slog.d(TAG, "handleFinishDrawing " + w); + if (postDrawTransaction == null || !mIsSyncDrawRequested + || op.canDrawBeforeStartTransaction()) { + mDisplayContent.finishAsyncRotation(w.mToken); + return false; + } if (op.mDrawTransaction == null) { if (w.isClientLocal()) { // Use a new transaction to merge the draw transaction of local window because the diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index ca3cfafc4a553..f13d440ebd38b 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -2137,6 +2137,10 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp if (!shellTransitions) { forAllWindows(w -> { w.seamlesslyRotateIfAllowed(transaction, oldRotation, rotation, rotateSeamlessly); + if (!rotateSeamlessly && w.mHasSurface) { + ProtoLog.v(WM_DEBUG_ORIENTATION, "Set mOrientationChanging of %s", w); + w.setOrientationChanging(true); + } }, true /* traverseTopToBottom */); mPinnedTaskController.startSeamlessRotationIfNeeded(transaction, oldRotation, rotation); if (!mDisplayRotation.hasSeamlessRotatingWindow()) { @@ -2148,14 +2152,6 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp mWmService.mDisplayManagerInternal.performTraversal(transaction); scheduleAnimation(); - forAllWindows(w -> { - if (!w.mHasSurface) return; - if (!rotateSeamlessly) { - ProtoLog.v(WM_DEBUG_ORIENTATION, "Set mOrientationChanging of %s", w); - w.setOrientationChanging(true); - } - }, true /* traverseTopToBottom */); - for (int i = mWmService.mRotationWatchers.size() - 1; i >= 0; i--) { final WindowManagerService.RotationWatcher rotationWatcher = mWmService.mRotationWatchers.get(i); 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 95348a0437802..42e472ccef311 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java @@ -863,9 +863,6 @@ public class TransitionTests extends WindowTestsBase { final AsyncRotationController asyncRotationController = mDisplayContent.getAsyncRotationController(); assertNotNull(asyncRotationController); - for (WindowState w : windows) { - w.setOrientationChanging(true); - } player.startTransition(); assertFalse(mDisplayContent.mTransitionController.isCollecting(statusBar.mToken)); @@ -875,15 +872,11 @@ public class TransitionTests extends WindowTestsBase { assertTrue(asyncRotationController.isTargetToken(decorToken)); assertShouldFreezeInsetsPosition(asyncRotationController, statusBar, true); - if (TransitionController.SYNC_METHOD != BLASTSyncEngine.METHOD_BLAST) { - // Only seamless window syncs its draw transaction with transition. - assertFalse(asyncRotationController.handleFinishDrawing(statusBar, mMockT)); - assertTrue(asyncRotationController.handleFinishDrawing(screenDecor, mMockT)); - } - screenDecor.setOrientationChanging(false); + // Only seamless window syncs its draw transaction with transition. + assertTrue(asyncRotationController.handleFinishDrawing(screenDecor, mMockT)); // Status bar finishes drawing before the start transaction. Its fade-in animation will be // executed until the transaction is committed, so it is still in target tokens. - statusBar.setOrientationChanging(false); + assertFalse(asyncRotationController.handleFinishDrawing(statusBar, mMockT)); assertTrue(asyncRotationController.isTargetToken(statusBar.mToken)); final SurfaceControl.Transaction startTransaction = mock(SurfaceControl.Transaction.class); @@ -897,7 +890,7 @@ public class TransitionTests extends WindowTestsBase { // Navigation bar finishes drawing after the start transaction, so its fade-in animation // can execute directly. - navBar.setOrientationChanging(false); + asyncRotationController.handleFinishDrawing(navBar, mMockT); assertFalse(asyncRotationController.isTargetToken(navBar.mToken)); assertNull(mDisplayContent.getAsyncRotationController()); } @@ -931,7 +924,6 @@ public class TransitionTests extends WindowTestsBase { assertNotNull(asyncRotationController); assertShouldFreezeInsetsPosition(asyncRotationController, statusBar, true); - statusBar.setOrientationChanging(true); player.startTransition(); // Non-app windows should not be collected. assertFalse(statusBar.mToken.inTransition()); @@ -994,7 +986,6 @@ public class TransitionTests extends WindowTestsBase { mDisplayContent.mTransitionController.dispatchLegacyAppTransitionFinished(app); assertTrue(mDisplayContent.hasTopFixedRotationLaunchingApp()); - statusBar.setOrientationChanging(true); player.startTransition(); // Non-app windows should not be collected. assertFalse(mDisplayContent.mTransitionController.isCollecting(statusBar.mToken)); @@ -1006,7 +997,6 @@ public class TransitionTests extends WindowTestsBase { // The controller should be cleared if the target windows are drawn. statusBar.finishDrawing(mWm.mTransactionFactory.get(), Integer.MAX_VALUE); - statusBar.setOrientationChanging(false); assertNull(mDisplayContent.getAsyncRotationController()); }