From 3ca99e2ab99eec5c591e3c6ba25b97dff3b823ed Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Sat, 20 May 2023 03:50:09 +0000 Subject: [PATCH] Fix issue with task snapshots being taken too late - Currently, a display change while recents transition is playing will result in the display change being applied prior to the recents animation playing, which means that the transition-end snapshots are taken in the wrong display configuration - Instead, When a display change is requested mid-recents transition, cancel the current transition and record the snapshots in place (this matchs legacy recents animation behavior) - Separately, if the snapshot is requested & recorded mid transition, then we can ignore the post-transition screenshot (which would be wrong in the display change case, or just extraneous even in the normal case where launcher has requested a screenshot itself already). We do this by comparing the time that the last snapshot was taken with the start of the transition. Fixes: 278189494 Test: Go into Overview and rotate the device and observe the task views Change-Id: I985cd9c852a326027ae3ab4f7ffe837431d1c654 --- core/java/android/window/TaskSnapshot.java | 24 +++++++++++- .../window/SnapshotDrawerUtilsTest.java | 1 + data/etc/services.core.protolog.json | 6 +++ .../recents/RecentsTransitionHandler.java | 39 +++++++++++-------- .../StartingSurfaceDrawerTests.java | 1 + .../server/wm/AbsAppSnapshotController.java | 3 ++ .../android/server/wm/AppSnapshotLoader.java | 6 ++- .../server/wm/TaskSnapshotController.java | 12 ++++++ .../com/android/server/wm/Transition.java | 19 +++++++-- .../android/server/wm/RecentTasksTest.java | 2 +- .../wm/TaskSnapshotPersisterTestBase.java | 2 +- 11 files changed, 90 insertions(+), 25 deletions(-) diff --git a/core/java/android/window/TaskSnapshot.java b/core/java/android/window/TaskSnapshot.java index b7bb6082296e5..41b6d31661ce8 100644 --- a/core/java/android/window/TaskSnapshot.java +++ b/core/java/android/window/TaskSnapshot.java @@ -28,6 +28,7 @@ import android.hardware.HardwareBuffer; import android.os.Build; import android.os.Parcel; import android.os.Parcelable; +import android.os.SystemClock; import android.view.Surface; import android.view.WindowInsetsController; @@ -38,6 +39,9 @@ import android.view.WindowInsetsController; public class TaskSnapshot implements Parcelable { // Identifier of this snapshot private final long mId; + // The elapsed real time (in nanoseconds) when this snapshot was captured, not intended for use outside the + // process in which the snapshot was taken (ie. this is not parceled) + private final long mCaptureTime; // Top activity in task when snapshot was taken private final ComponentName mTopActivityComponent; private final HardwareBuffer mSnapshot; @@ -65,7 +69,7 @@ public class TaskSnapshot implements Parcelable { // Must be one of the named color spaces, otherwise, always use SRGB color space. private final ColorSpace mColorSpace; - public TaskSnapshot(long id, + public TaskSnapshot(long id, long captureTime, @NonNull ComponentName topActivityComponent, HardwareBuffer snapshot, @NonNull ColorSpace colorSpace, int orientation, int rotation, Point taskSize, Rect contentInsets, Rect letterboxInsets, boolean isLowResolution, @@ -73,6 +77,7 @@ public class TaskSnapshot implements Parcelable { @WindowInsetsController.Appearance int appearance, boolean isTranslucent, boolean hasImeSurface) { mId = id; + mCaptureTime = captureTime; mTopActivityComponent = topActivityComponent; mSnapshot = snapshot; mColorSpace = colorSpace.getId() < 0 @@ -92,6 +97,7 @@ public class TaskSnapshot implements Parcelable { private TaskSnapshot(Parcel source) { mId = source.readLong(); + mCaptureTime = SystemClock.elapsedRealtimeNanos(); mTopActivityComponent = ComponentName.readFromParcel(source); mSnapshot = source.readTypedObject(HardwareBuffer.CREATOR); int colorSpaceId = source.readInt(); @@ -118,6 +124,14 @@ public class TaskSnapshot implements Parcelable { return mId; } + /** + * @return The elapsed real time (in nanoseconds) when this snapshot was captured. This time is + * only valid in the process where this snapshot was taken. + */ + public long getCaptureTime() { + return mCaptureTime; + } + /** * @return The top activity component for the task at the point this snapshot was taken. */ @@ -268,6 +282,7 @@ public class TaskSnapshot implements Parcelable { final int height = mSnapshot != null ? mSnapshot.getHeight() : 0; return "TaskSnapshot{" + " mId=" + mId + + " mCaptureTime=" + mCaptureTime + " mTopActivityComponent=" + mTopActivityComponent.flattenToShortString() + " mSnapshot=" + mSnapshot + " (" + width + "x" + height + ")" + " mColorSpace=" + mColorSpace.toString() @@ -296,6 +311,7 @@ public class TaskSnapshot implements Parcelable { /** Builder for a {@link TaskSnapshot} object */ public static final class Builder { private long mId; + private long mCaptureTime; private ComponentName mTopActivity; private HardwareBuffer mSnapshot; private ColorSpace mColorSpace; @@ -317,6 +333,11 @@ public class TaskSnapshot implements Parcelable { return this; } + public Builder setCaptureTime(long captureTime) { + mCaptureTime = captureTime; + return this; + } + public Builder setTopActivityComponent(ComponentName name) { mTopActivity = name; return this; @@ -400,6 +421,7 @@ public class TaskSnapshot implements Parcelable { public TaskSnapshot build() { return new TaskSnapshot( mId, + mCaptureTime, mTopActivity, mSnapshot, mColorSpace, diff --git a/core/tests/coretests/src/android/window/SnapshotDrawerUtilsTest.java b/core/tests/coretests/src/android/window/SnapshotDrawerUtilsTest.java index 281d677dccb0a..6764ac85dcef3 100644 --- a/core/tests/coretests/src/android/window/SnapshotDrawerUtilsTest.java +++ b/core/tests/coretests/src/android/window/SnapshotDrawerUtilsTest.java @@ -88,6 +88,7 @@ public class SnapshotDrawerUtilsTest { 1, HardwareBuffer.USAGE_CPU_READ_RARELY); return new TaskSnapshot( System.currentTimeMillis(), + 0 /* captureTime */, new ComponentName("", ""), buffer, ColorSpace.get(ColorSpace.Named.SRGB), ORIENTATION_PORTRAIT, Surface.ROTATION_0, taskSize, contentInsets, new Rect() /* letterboxInsets */, diff --git a/data/etc/services.core.protolog.json b/data/etc/services.core.protolog.json index 4b4e7220cb29b..e4defcfa359f8 100644 --- a/data/etc/services.core.protolog.json +++ b/data/etc/services.core.protolog.json @@ -1135,6 +1135,12 @@ "group": "WM_DEBUG_RECENTS_ANIMATIONS", "at": "com\/android\/server\/wm\/RecentsAnimation.java" }, + "-1060529098": { + "message": " Skipping post-transition snapshot for task %d", + "level": "VERBOSE", + "group": "WM_DEBUG_WINDOW_TRANSITIONS", + "at": "com\/android\/server\/wm\/Transition.java" + }, "-1060365734": { "message": "Attempted to add QS dialog window with bad token %s. Aborting.", "level": "WARN", diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/recents/RecentsTransitionHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/recents/RecentsTransitionHandler.java index a9ad3c90949f4..c2869592dbb65 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/recents/RecentsTransitionHandler.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/recents/RecentsTransitionHandler.java @@ -264,21 +264,18 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler { void cancel(String reason) { // restoring (to-home = false) involves submitting more WM changes, so by default, use // toHome = true when canceling. - cancel(true /* toHome */, reason); + cancel(true /* toHome */, false /* withScreenshots */, reason); } - void cancel(boolean toHome, String reason) { + void cancel(boolean toHome, boolean withScreenshots, String reason) { ProtoLog.v(ShellProtoLogGroup.WM_SHELL_RECENTS_TRANSITION, "[%d] RecentsController.cancel: toHome=%b reason=%s", mInstanceId, toHome, reason); if (mListener != null) { - try { - ProtoLog.v(ShellProtoLogGroup.WM_SHELL_RECENTS_TRANSITION, - "[%d] RecentsController.cancel: calling onAnimationCanceled", - mInstanceId); - mListener.onAnimationCanceled(null, null); - } catch (RemoteException e) { - Slog.e(TAG, "Error canceling recents animation", e); + if (withScreenshots) { + sendCancelWithSnapshots(); + } else { + sendCancel(null, null); } } if (mFinishCB != null) { @@ -300,24 +297,34 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler { snapshots = new TaskSnapshot[mPausingTasks.size()]; try { for (int i = 0; i < mPausingTasks.size(); ++i) { + TaskState state = mPausingTasks.get(0); snapshots[i] = ActivityTaskManager.getService().takeTaskSnapshot( - mPausingTasks.get(0).mTaskInfo.taskId, false /* updateCache */); + state.mTaskInfo.taskId, true /* updateCache */); } } catch (RemoteException e) { taskIds = null; snapshots = null; } } + return sendCancel(taskIds, snapshots); + } + + /** + * Sends a cancel message to the recents animation. + */ + private boolean sendCancel(@Nullable int[] taskIds, + @Nullable TaskSnapshot[] taskSnapshots) { try { + final String cancelDetails = taskSnapshots != null ? " with snapshots" : ""; ProtoLog.v(ShellProtoLogGroup.WM_SHELL_RECENTS_TRANSITION, - "[%d] RecentsController.cancel: calling onAnimationCanceled with snapshots", - mInstanceId); - mListener.onAnimationCanceled(taskIds, snapshots); + "[%d] RecentsController.cancel: calling onAnimationCanceled %s", + mInstanceId, cancelDetails); + mListener.onAnimationCanceled(taskIds, taskSnapshots); + return true; } catch (RemoteException e) { Slog.e(TAG, "Error canceling recents animation", e); return false; } - return true; } void cleanUp() { @@ -519,7 +526,7 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler { // Finish recents animation if the display is changed, so the default // transition handler can play the animation such as rotation effect. if (change.hasFlags(TransitionInfo.FLAG_IS_DISPLAY)) { - cancel(mWillFinishToHome, "display change"); + cancel(mWillFinishToHome, true /* withScreenshots */, "display change"); return; } // Don't consider order-only changes as changing apps. @@ -633,7 +640,7 @@ public class RecentsTransitionHandler implements Transitions.TransitionHandler { + foundRecentsClosing); if (foundRecentsClosing) { mWillFinishToHome = false; - cancel(false /* toHome */, "didn't merge"); + cancel(false /* toHome */, false /* withScreenshots */, "didn't merge"); } return; } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/startingsurface/StartingSurfaceDrawerTests.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/startingsurface/StartingSurfaceDrawerTests.java index 8115a5d4e89cd..ee9f886633269 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/startingsurface/StartingSurfaceDrawerTests.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/startingsurface/StartingSurfaceDrawerTests.java @@ -364,6 +364,7 @@ public class StartingSurfaceDrawerTests extends ShellTestCase { 1, HardwareBuffer.USAGE_CPU_READ_RARELY); return new TaskSnapshot( System.currentTimeMillis(), + 0 /* captureTime */, new ComponentName("", ""), buffer, ColorSpace.get(ColorSpace.Named.SRGB), ORIENTATION_PORTRAIT, Surface.ROTATION_0, taskSize, contentInsets, new Rect() /* letterboxInsets */, diff --git a/services/core/java/com/android/server/wm/AbsAppSnapshotController.java b/services/core/java/com/android/server/wm/AbsAppSnapshotController.java index 5c929a93bf129..bd07622ee5ca8 100644 --- a/services/core/java/com/android/server/wm/AbsAppSnapshotController.java +++ b/services/core/java/com/android/server/wm/AbsAppSnapshotController.java @@ -30,6 +30,7 @@ import android.graphics.RecordingCanvas; import android.graphics.Rect; import android.graphics.RenderNode; import android.hardware.HardwareBuffer; +import android.os.SystemClock; import android.os.Trace; import android.util.Pair; import android.util.Slog; @@ -213,6 +214,7 @@ abstract class AbsAppSnapshotController