From c0694d47e8f43ce8d4b0636007b86e32e2740e44 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Wed, 1 Jun 2022 23:52:52 -0600 Subject: [PATCH] Avoid requesting multiple sync for display rotation Previously, collect(display) always requests all window prepare sync. And AsyncRotationController may request another sync by applyWithNextDraw, which may cause stale sync state and the window is unable to apply its draw transaction. Now the prepareSync from display will only sync the windows that AsyncRotationController doesn't handle. Bug: 234585256 Test: atest TransitionTests#testDisplayRotationChange Change-Id: I30639ee4566fc730d7016e1fed509f60dcaf8b59 --- .../server/wm/AsyncRotationController.java | 34 +++++++++---------- .../com/android/server/wm/DisplayContent.java | 2 +- .../android/server/wm/DisplayRotation.java | 4 ++- .../android/server/wm/WindowContainer.java | 4 --- .../com/android/server/wm/WindowState.java | 19 ++++++++--- .../com/android/server/wm/WindowToken.java | 9 +++++ .../android/server/wm/TransitionTests.java | 7 +++- 7 files changed, 49 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/wm/AsyncRotationController.java b/services/core/java/com/android/server/wm/AsyncRotationController.java index b519dadb20315..e3de18b1ebc7a 100644 --- a/services/core/java/com/android/server/wm/AsyncRotationController.java +++ b/services/core/java/com/android/server/wm/AsyncRotationController.java @@ -18,7 +18,6 @@ package com.android.server.wm; import static android.view.WindowManager.LayoutParams.ROTATION_ANIMATION_SEAMLESS; import static android.view.WindowManager.LayoutParams.TYPE_NAVIGATION_BAR; -import static android.view.WindowManager.LayoutParams.TYPE_NOTIFICATION_SHADE; import static com.android.server.wm.SurfaceAnimator.ANIMATION_TYPE_TOKEN_TRANSFORM; @@ -152,8 +151,7 @@ class AsyncRotationController extends FadeAnimationController implements Consume /** Assigns the operation for the window tokens which can update rotation asynchronously. */ @Override public void accept(WindowState w) { - if (w.mActivityRecord != null || !w.mHasSurface || w.mIsWallpaper || w.mIsImWindow - || w.mAttrs.type == TYPE_NOTIFICATION_SHADE) { + if (!w.mHasSurface || !canBeAsync(w.mToken)) { return; } if (mTransitionOp == OP_LEGACY && w.mForceSeamlesslyRotate) { @@ -185,23 +183,28 @@ class AsyncRotationController extends FadeAnimationController implements Consume mTargetWindowTokens.put(w.mToken, new Operation(action)); } + /** Returns {@code true} if the window token can update rotation independently. */ + static boolean canBeAsync(WindowToken token) { + final int type = token.windowType; + return type > WindowManager.LayoutParams.LAST_APPLICATION_WINDOW + && type != WindowManager.LayoutParams.TYPE_INPUT_METHOD + && type != WindowManager.LayoutParams.TYPE_WALLPAPER + && type != WindowManager.LayoutParams.TYPE_NOTIFICATION_SHADE; + } + /** * Enables {@link #handleFinishDrawing(WindowState, SurfaceControl.Transaction)} to capture the * draw transactions of the target windows if needed. */ void keepAppearanceInPreviousRotation() { + if (mIsSyncDrawRequested) return; // The transition sync group may be finished earlier because it doesn't wait for these // target windows. But the windows still need to use sync transaction to keep the appearance // in previous rotation, so request a no-op sync to keep the state. for (int i = mTargetWindowTokens.size() - 1; i >= 0; i--) { - if (mHasScreenRotationAnimation - && mTargetWindowTokens.valueAt(i).mAction == Operation.ACTION_FADE) { - // The windows are hidden (leash is alpha 0) before finishing drawing so it is - // unnecessary to request sync. - continue; - } final WindowToken token = mTargetWindowTokens.keyAt(i); for (int j = token.getChildCount() - 1; j >= 0; j--) { + // TODO(b/234585256): The consumer should be handleFinishDrawing(). token.getChildAt(j).applyWithNextDraw(t -> {}); } } @@ -387,6 +390,7 @@ class AsyncRotationController extends FadeAnimationController implements Consume * transition starts. And associate transaction callback to consume pending animations. */ void setupStartTransaction(SurfaceControl.Transaction t) { + if (mIsStartTransactionCommitted) return; for (int i = mTargetWindowTokens.size() - 1; i >= 0; i--) { final Operation op = mTargetWindowTokens.valueAt(i); final SurfaceControl leash = op.mLeash; @@ -414,7 +418,6 @@ class AsyncRotationController extends FadeAnimationController implements Consume } } - if (mIsStartTransactionCommitted) return; // If there are windows have redrawn in new rotation but the start transaction has not // been applied yet, the fade-in animation will be deferred. So once the transaction is // committed, the fade-in animation can run with screen rotation animation. @@ -471,23 +474,18 @@ class AsyncRotationController extends FadeAnimationController implements Consume * by this controller. */ boolean handleFinishDrawing(WindowState w, SurfaceControl.Transaction postDrawTransaction) { - if (mTransitionOp == OP_LEGACY || postDrawTransaction == null - || !mIsSyncDrawRequested || !w.mTransitionController.inTransition()) { + if (mTransitionOp == OP_LEGACY || postDrawTransaction == null || !mIsSyncDrawRequested) { return false; } final Operation op = mTargetWindowTokens.get(w.mToken); if (op == null) return false; - final boolean keepUntilTransitionFinish = - mTransitionOp == OP_APP_SWITCH && op.mAction == Operation.ACTION_FADE; - final boolean keepUntilStartTransaction = - !mIsStartTransactionCommitted && op.mAction == Operation.ACTION_SEAMLESS; - if (!keepUntilTransitionFinish && !keepUntilStartTransaction) return false; + if (DEBUG) Slog.d(TAG, "handleFinishDrawing " + w); if (op.mDrawTransaction == null) { op.mDrawTransaction = postDrawTransaction; } else { op.mDrawTransaction.merge(postDrawTransaction); } - if (DEBUG) Slog.d(TAG, "Capture draw transaction " + w); + mDisplayContent.finishAsyncRotation(w.mToken); return true; } diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index a870ed3faa284..424db5e0ca151 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -1882,7 +1882,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp } /** Returns {@code true} if the decided new rotation has not applied to configuration yet. */ - private boolean isRotationChanging() { + boolean isRotationChanging() { return mDisplayRotation.getRotation() != getWindowConfiguration().getRotation(); } diff --git a/services/core/java/com/android/server/wm/DisplayRotation.java b/services/core/java/com/android/server/wm/DisplayRotation.java index a78d25f4e21a9..d2c71f57e701d 100644 --- a/services/core/java/com/android/server/wm/DisplayRotation.java +++ b/services/core/java/com/android/server/wm/DisplayRotation.java @@ -1635,7 +1635,9 @@ public class DisplayRotation { final WindowContainer source = dc.getLastOrientationSource(); if (source != null) { mLastOrientationSource = source.toString(); - mSourceOrientation = source.mOrientation; + final WindowState w = source.asWindowState(); + mSourceOrientation = + w != null ? w.mAttrs.screenOrientation : source.mOrientation; } else { mLastOrientationSource = null; mSourceOrientation = SCREEN_ORIENTATION_UNSET; diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index f9d19e22ddc7e..475e81e720e5e 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -3661,10 +3661,6 @@ class WindowContainer extends ConfigurationContainer< mChildren.get(i).finishSync(outMergedTransaction, cancel); } if (cancel && mSyncGroup != null) mSyncGroup.onCancelSync(this); - clearSyncState(); - } - - void clearSyncState() { mSyncState = SYNC_STATE_NONE; mSyncGroup = null; } diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index c91bec9fb0c9b..84e56b8495e32 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -3897,6 +3897,8 @@ class WindowState extends WindowContainer implements WindowManagerP mDragResizingChangeReported = true; mWindowFrames.clearReportResizeHints(); + final int prevRotation = mLastReportedConfiguration + .getMergedConfiguration().windowConfiguration.getRotation(); fillClientWindowFramesAndConfiguration(mClientWindowFrames, mLastReportedConfiguration, true /* useLatestConfig */, false /* relayoutVisible */); final boolean syncRedraw = shouldSendRedrawForSync(); @@ -3929,7 +3931,8 @@ class WindowState extends WindowContainer implements WindowManagerP mClient.resized(mClientWindowFrames, reportDraw, mLastReportedConfiguration, getCompatInsetsState(), forceRelayout, alwaysConsumeSystemBars, displayId, mSyncSeqId, resizeMode); - if (drawPending && mOrientationChanging) { + if (drawPending && prevRotation != mLastReportedConfiguration + .getMergedConfiguration().windowConfiguration.getRotation()) { mOrientationChangeRedrawRequestTime = SystemClock.elapsedRealtime(); ProtoLog.v(WM_DEBUG_ORIENTATION, "Requested redraw for orientation change: %s", this); @@ -5913,6 +5916,9 @@ class WindowState extends WindowContainer implements WindowManagerP @Override boolean prepareSync() { + if (!mDrawHandlers.isEmpty()) { + Slog.w(TAG, "prepareSync with mDrawHandlers, " + this + ", " + Debug.getCallers(8)); + } if (!super.prepareSync()) { return false; } @@ -5974,11 +5980,9 @@ class WindowState extends WindowContainer implements WindowManagerP if (asyncRotationController != null && asyncRotationController.handleFinishDrawing(this, postDrawTransaction)) { // Consume the transaction because the controller will apply it with fade animation. - // Layout is not needed because the window will be hidden by the fade leash. Clear - // sync state because its sync transaction doesn't need to be merged to sync group. + // Layout is not needed because the window will be hidden by the fade leash. postDrawTransaction = null; skipLayout = true; - clearSyncState(); } else if (onSyncFinishedDrawing() && postDrawTransaction != null) { mSyncTransaction.merge(postDrawTransaction); // Consume the transaction because the sync group will merge it. @@ -6032,7 +6036,8 @@ class WindowState extends WindowContainer implements WindowManagerP if (mRedrawForSyncReported) { return false; } - if (mInRelayout) { + // TODO(b/233286785): Remove mIsWallpaper once WallpaperService handles syncId of relayout. + if (mInRelayout && !mIsWallpaper) { // The last sync seq id will return to the client, so there is no need to request the // client to redraw. return false; @@ -6069,6 +6074,10 @@ class WindowState extends WindowContainer implements WindowManagerP * See {@link WindowState#mDrawHandlers} */ void applyWithNextDraw(Consumer consumer) { + if (mSyncState != SYNC_STATE_NONE) { + Slog.w(TAG, "applyWithNextDraw with mSyncState=" + mSyncState + ", " + this + + ", " + Debug.getCallers(8)); + } mSyncSeqId++; mDrawHandlers.add(new DrawHandler(mSyncSeqId, consumer)); diff --git a/services/core/java/com/android/server/wm/WindowToken.java b/services/core/java/com/android/server/wm/WindowToken.java index ddbb930ee20c3..ead39beb10330 100644 --- a/services/core/java/com/android/server/wm/WindowToken.java +++ b/services/core/java/com/android/server/wm/WindowToken.java @@ -668,6 +668,15 @@ class WindowToken extends WindowContainer { } } + @Override + boolean prepareSync() { + if (mDisplayContent != null && mDisplayContent.isRotationChanging() + && AsyncRotationController.canBeAsync(this)) { + return false; + } + return super.prepareSync(); + } + @CallSuper @Override public void dumpDebug(ProtoOutputStream proto, long fieldId, 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 d9d819b5f00f4..bd69778cf9feb 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TransitionTests.java @@ -661,6 +661,11 @@ public class TransitionTests extends WindowTestsBase { mDisplayContent.setLastHasContent(); mDisplayContent.requestChangeTransitionIfNeeded(1 /* any changes */, null /* displayChange */); + assertEquals(WindowContainer.SYNC_STATE_NONE, statusBar.mSyncState); + assertEquals(WindowContainer.SYNC_STATE_NONE, navBar.mSyncState); + assertEquals(WindowContainer.SYNC_STATE_NONE, screenDecor.mSyncState); + assertEquals(WindowContainer.SYNC_STATE_WAITING_FOR_DRAW, ime.mSyncState); + final AsyncRotationController asyncRotationController = mDisplayContent.getAsyncRotationController(); assertNotNull(asyncRotationController); @@ -774,7 +779,7 @@ public class TransitionTests extends WindowTestsBase { player.start(); player.finish(); - app.getTask().clearSyncState(); + app.getTask().finishSync(mWm.mTransactionFactory.get(), false /* cancel */); // The open transition is finished. Continue to play seamless display change transition, // so the previous async rotation controller should still exist.