From e045dd6e81be1bc0faf02b6bb1a42a05146a157e Mon Sep 17 00:00:00 2001 From: Chris Li Date: Thu, 16 Sep 2021 21:20:01 +0800 Subject: [PATCH 1/2] Use TaskFragment bounds instead of TaskBounds for Activity animation The animation bounds is used as the RemoteAnimationTarget#localBounds. It should be the TaskFragment bounds instead of Task bounds because the Activity may not fill the Task when it is in a TaskFragment. Also clean up ROOT_TASK_CLIP_BEFORE_ANIM which has no more use case. Bug: 196173550 Test: test with demo app Test: atest WmTests:ActivityRecordTests Change-Id: Iff58afdb0bbae95d3f6a5756de06c0d2dfff7c88 --- .../com/android/server/wm/ActivityRecord.java | 10 +-- .../core/java/com/android/server/wm/Task.java | 11 ---- .../server/wm/WindowStateAnimator.java | 8 +-- .../server/wm/ActivityRecordTests.java | 34 ++++++++-- .../server/wm/WindowAnimationSpecTest.java | 64 ------------------- 5 files changed, 33 insertions(+), 94 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index dfb2f8ff8bc95..a7d28ce1b0530 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -223,7 +223,6 @@ import static com.android.server.wm.WindowManagerService.UPDATE_FOCUS_NORMAL; import static com.android.server.wm.WindowManagerService.UPDATE_FOCUS_WILL_PLACE_SURFACES; import static com.android.server.wm.WindowState.LEGACY_POLICY_VISIBILITY; import static com.android.server.wm.WindowStateAnimator.HAS_DRAWN; -import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_BEFORE_ANIM; import static org.xmlpull.v1.XmlPullParser.END_DOCUMENT; import static org.xmlpull.v1.XmlPullParser.END_TAG; @@ -8041,13 +8040,10 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A @VisibleForTesting @Override Rect getAnimationBounds(int appRootTaskClipMode) { - if (appRootTaskClipMode == ROOT_TASK_CLIP_BEFORE_ANIM && getRootTask() != null) { - // Using the root task bounds here effectively applies the clipping before animation. - return getRootTask().getBounds(); - } - // Use task-bounds if available so that activity-level letterbox (maxAspectRatio) is + // Use TaskFragment-bounds if available so that activity-level letterbox (maxAspectRatio) is // included in the animation. - return task != null ? task.getBounds() : getBounds(); + final TaskFragment taskFragment = getTaskFragment(); + return taskFragment != null ? taskFragment.getBounds() : getBounds(); } @Override diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index ca98564c1d132..a3626d215cf4b 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -130,7 +130,6 @@ import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_ROOT_TASK; import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_TASK_MOVEMENT; import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM; import static com.android.server.wm.WindowManagerService.dipToPixel; -import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_BEFORE_ANIM; import static java.lang.Integer.MAX_VALUE; @@ -2984,16 +2983,6 @@ class Task extends TaskFragment { super.resetSurfacePositionForAnimationLeash(t); } - @Override - Rect getAnimationBounds(int appRootTaskClipMode) { - // TODO(b/131661052): we should remove appRootTaskClipMode with hierarchical animations. - if (appRootTaskClipMode == ROOT_TASK_CLIP_BEFORE_ANIM && getRootTask() != null) { - // Using the root task bounds here effectively applies the clipping before animation. - return getRootTask().getBounds(); - } - return super.getAnimationBounds(appRootTaskClipMode); - } - boolean shouldAnimate() { /** * Animations are handled by the TaskOrganizer implementation. diff --git a/services/core/java/com/android/server/wm/WindowStateAnimator.java b/services/core/java/com/android/server/wm/WindowStateAnimator.java index a0dc2476da40e..dd70a14bd1ce9 100644 --- a/services/core/java/com/android/server/wm/WindowStateAnimator.java +++ b/services/core/java/com/android/server/wm/WindowStateAnimator.java @@ -79,17 +79,11 @@ class WindowStateAnimator { */ static final int ROOT_TASK_CLIP_AFTER_ANIM = 0; - /** - * Mode how the window gets clipped by the root task bounds: The clipping should be applied - * before applying the animation transformation, i.e. the root task bounds move with the window. - */ - static final int ROOT_TASK_CLIP_BEFORE_ANIM = 1; - /** * Mode how window gets clipped by the root task bounds during an animation: Don't clip the * window by the root task bounds. */ - static final int ROOT_TASK_CLIP_NONE = 2; + static final int ROOT_TASK_CLIP_NONE = 1; // Unchanging local convenience fields. final WindowManagerService mService; diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java index 9a14e7d94e31e..51cc3186c773d 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java @@ -19,7 +19,6 @@ package com.android.server.wm; import static android.app.WindowConfiguration.WINDOWING_MODE_FREEFORM; import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN; import static android.app.WindowConfiguration.WINDOWING_MODE_MULTI_WINDOW; -import static android.app.WindowConfiguration.WINDOWING_MODE_SPLIT_SCREEN_PRIMARY; import static android.content.pm.ActivityInfo.CONFIG_ORIENTATION; import static android.content.pm.ActivityInfo.CONFIG_SCREEN_LAYOUT; import static android.content.pm.ActivityInfo.FLAG_SUPPORTS_PICTURE_IN_PICTURE; @@ -83,7 +82,6 @@ import static com.android.server.wm.TaskFragment.TASK_FRAGMENT_VISIBILITY_VISIBL import static com.android.server.wm.TaskFragment.TASK_FRAGMENT_VISIBILITY_VISIBLE_BEHIND_TRANSLUCENT; import static com.android.server.wm.WindowContainer.POSITION_TOP; import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_AFTER_ANIM; -import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_BEFORE_ANIM; import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_NONE; import static com.google.common.truth.Truth.assertThat; @@ -2779,10 +2777,36 @@ public class ActivityRecordTests extends WindowTestsBase { assertEquals(taskBounds, activity.getAnimationBounds(ROOT_TASK_CLIP_AFTER_ANIM)); assertEquals(new Point(0, 0), animationPosition); + } - // ROOT_TASK_CLIP_BEFORE_ANIM should use stack bounds since it won't be clipped later. - task.getWindowConfiguration().setWindowingMode(WINDOWING_MODE_SPLIT_SCREEN_PRIMARY); - assertEquals(rootTask.getBounds(), activity.getAnimationBounds(ROOT_TASK_CLIP_BEFORE_ANIM)); + @Test + public void testTransitionAnimationBounds_returnTaskFragment() { + removeGlobalMinSizeRestriction(); + final Task task = new TaskBuilder(mSupervisor).setCreateParentTask(true).build(); + final Task rootTask = task.getRootTask(); + final TaskFragment taskFragment = createTaskFragmentWithParentTask(task, + false /* createEmbeddedTask */); + final ActivityRecord activity = taskFragment.getTopNonFinishingActivity(); + final Rect stackBounds = new Rect(0, 0, 1000, 600); + final Rect taskBounds = new Rect(100, 400, 600, 800); + final Rect taskFragmentBounds = new Rect(100, 400, 300, 800); + final Rect activityBounds = new Rect(100, 400, 300, 600); + // Set the bounds and windowing mode to window configuration directly, otherwise the + // testing setups may be discarded by configuration resolving. + rootTask.getWindowConfiguration().setBounds(stackBounds); + task.getWindowConfiguration().setBounds(taskBounds); + taskFragment.getWindowConfiguration().setBounds(taskFragmentBounds); + activity.getWindowConfiguration().setBounds(activityBounds); + + // Check that anim bounds for freeform window match task fragment bounds + task.getWindowConfiguration().setWindowingMode(WINDOWING_MODE_FREEFORM); + assertEquals(taskFragment.getBounds(), activity.getAnimationBounds(ROOT_TASK_CLIP_NONE)); + + // ROOT_TASK_CLIP_AFTER_ANIM should use task fragment bounds since they will be clipped by + // bounds animation layer. + task.getWindowConfiguration().setWindowingMode(WINDOWING_MODE_FULLSCREEN); + assertEquals(taskFragment.getBounds(), + activity.getAnimationBounds(ROOT_TASK_CLIP_AFTER_ANIM)); } @Test diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowAnimationSpecTest.java b/services/tests/wmtests/src/com/android/server/wm/WindowAnimationSpecTest.java index e970c2a2f2edf..e2f1334c7f8cf 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowAnimationSpecTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowAnimationSpecTest.java @@ -19,7 +19,6 @@ package com.android.server.wm; import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock; import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_AFTER_ANIM; -import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_BEFORE_ANIM; import static com.android.server.wm.WindowStateAnimator.ROOT_TASK_CLIP_NONE; import static org.mockito.ArgumentMatchers.any; @@ -88,43 +87,6 @@ public class WindowAnimationSpecTest { argThat(rect -> rect.equals(mStackBounds))); } - @Test - public void testApply_clipBeforeNoAnimationBounds() { - // Stack bounds is (0, 0, 10, 10) animation clip is (0, 0, 0, 0) - WindowAnimationSpec windowAnimationSpec = new WindowAnimationSpec(mAnimation, null, - mStackBounds, false /* canSkipFirstFrame */, ROOT_TASK_CLIP_BEFORE_ANIM, - true /* isAppAnimation */, 0 /* windowCornerRadius */); - windowAnimationSpec.apply(mTransaction, mSurfaceControl, 0); - verify(mTransaction).setWindowCrop(eq(mSurfaceControl), - argThat(rect -> rect.equals(mStackBounds))); - } - - @Test - public void testApply_clipBeforeNoStackBounds() { - // Stack bounds is (0, 0, 0, 0) animation clip is (0, 0, 20, 20) - Rect windowCrop = new Rect(0, 0, 20, 20); - Animation a = createClipRectAnimation(windowCrop, windowCrop); - a.initialize(0, 0, 0, 0); - WindowAnimationSpec windowAnimationSpec = new WindowAnimationSpec(a, null, - null, false /* canSkipFirstFrame */, ROOT_TASK_CLIP_BEFORE_ANIM, - true /* isAppAnimation */, 0 /* windowCornerRadius */); - windowAnimationSpec.apply(mTransaction, mSurfaceControl, 0); - verify(mTransaction).setWindowCrop(eq(mSurfaceControl), argThat(Rect::isEmpty)); - } - - @Test - public void testApply_setCornerRadius() { - final float windowCornerRadius = 30f; - WindowAnimationSpec windowAnimationSpec = new WindowAnimationSpec(mAnimation, null, - mStackBounds, false /* canSkipFirstFrame */, ROOT_TASK_CLIP_BEFORE_ANIM, - true /* isAppAnimation */, windowCornerRadius); - windowAnimationSpec.apply(mTransaction, mSurfaceControl, 0); - verify(mTransaction, never()).setCornerRadius(eq(mSurfaceControl), eq(windowCornerRadius)); - when(mAnimation.hasRoundedCorners()).thenReturn(true); - windowAnimationSpec.apply(mTransaction, mSurfaceControl, 0); - verify(mTransaction).setCornerRadius(eq(mSurfaceControl), eq(windowCornerRadius)); - } - @Test public void testApply_setCornerRadius_noClip() { final float windowCornerRadius = 30f; @@ -136,32 +98,6 @@ public class WindowAnimationSpecTest { verify(mTransaction, never()).setCornerRadius(any(), anyFloat()); } - @Test - public void testApply_clipBeforeSmallerAnimationClip() { - // Stack bounds is (0, 0, 10, 10) animation clip is (0, 0, 5, 5) - Rect windowCrop = new Rect(0, 0, 5, 5); - Animation a = createClipRectAnimation(windowCrop, windowCrop); - WindowAnimationSpec windowAnimationSpec = new WindowAnimationSpec(a, null, - mStackBounds, false /* canSkipFirstFrame */, ROOT_TASK_CLIP_BEFORE_ANIM, - true /* isAppAnimation */, 0 /* windowCornerRadius */); - windowAnimationSpec.apply(mTransaction, mSurfaceControl, 0); - verify(mTransaction).setWindowCrop(eq(mSurfaceControl), - argThat(rect -> rect.equals(windowCrop))); - } - - @Test - public void testApply_clipBeforeSmallerStackClip() { - // Stack bounds is (0, 0, 10, 10) animation clip is (0, 0, 20, 20) - Rect windowCrop = new Rect(0, 0, 20, 20); - Animation a = createClipRectAnimation(windowCrop, windowCrop); - WindowAnimationSpec windowAnimationSpec = new WindowAnimationSpec(a, null, - mStackBounds, false /* canSkipFirstFrame */, ROOT_TASK_CLIP_BEFORE_ANIM, - true /* isAppAnimation */, 0 /* windowCornerRadius */); - windowAnimationSpec.apply(mTransaction, mSurfaceControl, 0); - verify(mTransaction).setWindowCrop(eq(mSurfaceControl), - argThat(rect -> rect.equals(mStackBounds))); - } - private Animation createClipRectAnimation(Rect fromClip, Rect toClip) { Animation a = new ClipRectAnimation(fromClip, toClip); a.initialize(0, 0, 0, 0); From 79ab7ea034096d868d58f2c9a3f5ee9f50510177 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Thu, 16 Sep 2021 14:36:02 +0800 Subject: [PATCH 2/2] Flag whether a RemoteAnimationTarget has parent in animation There is case when a TaskFragment is resizing while one of its children is open/close. In such case, both the TaskFragment and its child will be included in the transition, but we don't want to animate both of them. Use the new flag to determine if the child need to be animated. Bug: 196173550 Test: test with demo app Change-Id: Ib5ad32501e046e5b612f6d5ae2faed6266d99f3d --- core/java/android/view/RemoteAnimationTarget.java | 12 ++++++++++++ .../java/com/android/server/wm/ActivityRecord.java | 6 ++++-- .../server/wm/RemoteAnimationController.java | 13 +++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/core/java/android/view/RemoteAnimationTarget.java b/core/java/android/view/RemoteAnimationTarget.java index 3f71d4d837a1d..046232a8afafe 100644 --- a/core/java/android/view/RemoteAnimationTarget.java +++ b/core/java/android/view/RemoteAnimationTarget.java @@ -208,6 +208,15 @@ public class RemoteAnimationTarget implements Parcelable { */ public final @WindowManager.LayoutParams.WindowType int windowType; + /** + * {@code true} if its parent is also a {@link RemoteAnimationTarget} in the same transition. + * + * For example, when a TaskFragment is resizing while one of its children is open/close, both + * windows will be animation targets. This value will be {@code true} for the child, so that + * the handler can choose to handle it differently. + */ + public boolean hasAnimatingParent; + public RemoteAnimationTarget(int taskId, int mode, SurfaceControl leash, boolean isTranslucent, Rect clipRect, Rect contentInsets, int prefixOrderIndex, Point position, Rect localBounds, Rect screenSpaceBounds, @@ -265,6 +274,7 @@ public class RemoteAnimationTarget implements Parcelable { taskInfo = in.readTypedObject(ActivityManager.RunningTaskInfo.CREATOR); allowEnterPip = in.readBoolean(); windowType = in.readInt(); + hasAnimatingParent = in.readBoolean(); } @Override @@ -292,6 +302,7 @@ public class RemoteAnimationTarget implements Parcelable { dest.writeTypedObject(taskInfo, 0 /* flags */); dest.writeBoolean(allowEnterPip); dest.writeInt(windowType); + dest.writeBoolean(hasAnimatingParent); } public void dump(PrintWriter pw, String prefix) { @@ -311,6 +322,7 @@ public class RemoteAnimationTarget implements Parcelable { pw.print(prefix); pw.print("taskInfo="); pw.println(taskInfo); pw.print(prefix); pw.print("allowEnterPip="); pw.println(allowEnterPip); pw.print(prefix); pw.print("windowType="); pw.print(windowType); + pw.print(prefix); pw.print("hasAnimatingParent="); pw.print(hasAnimatingParent); } public void dumpDebug(ProtoOutputStream proto, long fieldId) { diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index a7d28ce1b0530..c45b661f06fbc 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -9265,14 +9265,16 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A task.getBounds(), Type.systemBars(), false /* ignoreVisibility */).toRect(); InsetUtils.addInsets(insets, getLetterboxInsets()); - return new RemoteAnimationTarget(task.mTaskId, record.getMode(), - record.mAdapter.mCapturedLeash, !fillsParent(), + final RemoteAnimationTarget target = new RemoteAnimationTarget(task.mTaskId, + record.getMode(), record.mAdapter.mCapturedLeash, !fillsParent(), new Rect(), insets, getPrefixOrderIndex(), record.mAdapter.mPosition, record.mAdapter.mLocalBounds, record.mAdapter.mRootTaskBounds, task.getWindowConfiguration(), false /*isNotInRecents*/, record.mThumbnailAdapter != null ? record.mThumbnailAdapter.mCapturedLeash : null, record.mStartBounds, task.getTaskInfo(), checkEnterPictureInPictureAppOpsState()); + target.hasAnimatingParent = record.hasAnimatingParent(); + return target; } @Override diff --git a/services/core/java/com/android/server/wm/RemoteAnimationController.java b/services/core/java/com/android/server/wm/RemoteAnimationController.java index 6b57ede1cd262..b90f937ca56ec 100644 --- a/services/core/java/com/android/server/wm/RemoteAnimationController.java +++ b/services/core/java/com/android/server/wm/RemoteAnimationController.java @@ -436,6 +436,19 @@ class RemoteAnimationController implements DeathRecipient { int getMode() { return mMode; } + + /** Whether its parent is also an animation target in the same transition. */ + boolean hasAnimatingParent() { + // mOpeningApps and mClosingApps are only activities, so only need to check + // mChangingContainers. + for (int i = mDisplayContent.mChangingContainers.size() - 1; i >= 0; i--) { + if (mWindowContainer.isDescendantOf( + mDisplayContent.mChangingContainers.valueAt(i))) { + return true; + } + } + return false; + } } class RemoteAnimationAdapterWrapper implements AnimationAdapter {