Fix a bug about the z-order of layers caused by merging transactions
Each WindowContainer has its own pending transaction. These transactions
may be merged to the global one within WindowContainer.prepareSurfaces()
in a hierarchy-based order, not in a time-based order. It may cause
that a later surface operation overwritten by an earlier surface
operation. For example, atokenA.setLayer(t1, 0) was called earlier, and
then atokenA.setLayer(t2, 1) was called later. However, the layer of
atokenA might eventually be 0 because t1 could be merged into the
global transaction later.
This CL uses a single transaction per display to solve this problem.
Fix: 120282809
Test: atest SurfaceAnimatorTest
Test: Manual test with the steps in the issue.
Change-Id: Idca57d01d8be884369510642c2d9345b6ee4e3b1
diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java
index fd90f03..ecbecba 100644
--- a/services/core/java/com/android/server/wm/AppWindowToken.java
+++ b/services/core/java/com/android/server/wm/AppWindowToken.java
@@ -2005,7 +2005,7 @@
}
layoutLetterbox(winHint);
if (mLetterbox != null && mLetterbox.needsApplySurfaceChanges()) {
- mLetterbox.applySurfaceChanges(mPendingTransaction);
+ mLetterbox.applySurfaceChanges(getPendingTransaction());
}
}
@@ -3059,13 +3059,13 @@
if (mSurfaceControl != null) {
if (show && !mLastSurfaceShowing) {
- mPendingTransaction.show(mSurfaceControl);
+ getPendingTransaction().show(mSurfaceControl);
} else if (!show && mLastSurfaceShowing) {
- mPendingTransaction.hide(mSurfaceControl);
+ getPendingTransaction().hide(mSurfaceControl);
}
}
if (mThumbnail != null) {
- mThumbnail.setShowing(mPendingTransaction, show);
+ mThumbnail.setShowing(getPendingTransaction(), show);
}
mLastSurfaceShowing = show;
super.prepareSurfaces();
@@ -3225,8 +3225,8 @@
private void updateColorTransform() {
if (mSurfaceControl != null && mLastAppSaturationInfo != null) {
- mPendingTransaction.setColorTransform(mSurfaceControl, mLastAppSaturationInfo.mMatrix,
- mLastAppSaturationInfo.mTranslation);
+ getPendingTransaction().setColorTransform(mSurfaceControl,
+ mLastAppSaturationInfo.mMatrix, mLastAppSaturationInfo.mTranslation);
mWmService.scheduleAnimationLocked();
}
}
diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java
index 58a03b2..1e826ee 100644
--- a/services/core/java/com/android/server/wm/DisplayContent.java
+++ b/services/core/java/com/android/server/wm/DisplayContent.java
@@ -3340,7 +3340,7 @@
final SurfaceControl newParent =
shouldAttachToDisplay ? mWindowingLayer : computeImeParent();
if (newParent != null) {
- mPendingTransaction.reparent(mImeWindowsContainers.mSurfaceControl, newParent);
+ getPendingTransaction().reparent(mImeWindowsContainers.mSurfaceControl, newParent);
scheduleAnimation();
}
}
@@ -3747,7 +3747,8 @@
mPortalWindowHandle.touchableRegion.getBounds(mTmpRect);
if (!mTmpBounds.equals(mTmpRect)) {
mPortalWindowHandle.touchableRegion.set(mTmpBounds);
- mPendingTransaction.setInputWindowInfo(mParentSurfaceControl, mPortalWindowHandle);
+ getPendingTransaction().setInputWindowInfo(
+ mParentSurfaceControl, mPortalWindowHandle);
}
}
}
@@ -4846,18 +4847,23 @@
try {
final ScreenRotationAnimation screenRotationAnimation =
mWmService.mAnimator.getScreenRotationAnimationLocked(mDisplayId);
+ final Transaction transaction = getPendingTransaction();
if (screenRotationAnimation != null && screenRotationAnimation.isAnimating()) {
screenRotationAnimation.getEnterTransformation().getMatrix().getValues(mTmpFloats);
- mPendingTransaction.setMatrix(mWindowingLayer,
+ transaction.setMatrix(mWindowingLayer,
mTmpFloats[Matrix.MSCALE_X], mTmpFloats[Matrix.MSKEW_Y],
mTmpFloats[Matrix.MSKEW_X], mTmpFloats[Matrix.MSCALE_Y]);
- mPendingTransaction.setPosition(mWindowingLayer,
+ transaction.setPosition(mWindowingLayer,
mTmpFloats[Matrix.MTRANS_X], mTmpFloats[Matrix.MTRANS_Y]);
- mPendingTransaction.setAlpha(mWindowingLayer,
+ transaction.setAlpha(mWindowingLayer,
screenRotationAnimation.getEnterTransformation().getAlpha());
}
super.prepareSurfaces();
+
+ // TODO: Once we totally eliminate global transaction we will pass transaction in here
+ // rather than merging to global.
+ SurfaceControl.mergeToGlobalTransaction(transaction);
} finally {
Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
}
@@ -5013,7 +5019,7 @@
if (mPortalWindowHandle == null) {
mPortalWindowHandle = createPortalWindowHandle(sc.toString());
}
- mPendingTransaction.setInputWindowInfo(sc, mPortalWindowHandle)
+ getPendingTransaction().setInputWindowInfo(sc, mPortalWindowHandle)
.reparent(mWindowingLayer, sc).reparent(mOverlayLayer, sc);
}
diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java
index bb7867c..3820106 100644
--- a/services/core/java/com/android/server/wm/Task.java
+++ b/services/core/java/com/android/server/wm/Task.java
@@ -138,6 +138,7 @@
setOrientation(SCREEN_ORIENTATION_UNSET);
}
+ @Override
DisplayContent getDisplayContent() {
return mStack != null ? mStack.getDisplayContent() : null;
}
diff --git a/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java b/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java
index 17e4b89..ee4e462 100644
--- a/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java
+++ b/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java
@@ -77,7 +77,7 @@
@Override
public SurfaceControl.Transaction getPendingTransaction() {
- return mTask.mPendingTransaction;
+ return mTask.getPendingTransaction();
}
@Override
diff --git a/services/core/java/com/android/server/wm/TaskStack.java b/services/core/java/com/android/server/wm/TaskStack.java
index 757f6a1..ab5e071 100644
--- a/services/core/java/com/android/server/wm/TaskStack.java
+++ b/services/core/java/com/android/server/wm/TaskStack.java
@@ -93,10 +93,6 @@
/** Unique identifier */
final int mStackId;
- /** The display this stack sits under. */
- // TODO: Track parent marks like this in WindowContainer.
- private DisplayContent mDisplayContent;
-
/** For comparison with DisplayContent bounds. */
private Rect mTmpRect = new Rect();
private Rect mTmpRect2 = new Rect();
@@ -177,10 +173,6 @@
EventLog.writeEvent(EventLogTags.WM_STACK_CREATED, stackId);
}
- DisplayContent getDisplayContent() {
- return mDisplayContent;
- }
-
Task findHomeTask() {
if (!isActivityTypeHome() || mChildren.isEmpty()) {
return null;
@@ -825,8 +817,7 @@
throw new IllegalStateException("onDisplayChanged: Already attached");
}
- final boolean movedToNewDisplay = mDisplayContent == null;
- mDisplayContent = dc;
+ super.onDisplayChanged(dc);
updateSurfaceBounds();
if (mAnimationBackgroundSurface == null) {
@@ -834,8 +825,6 @@
.setName("animation background stackId=" + mStackId)
.build();
}
-
- super.onDisplayChanged(dc);
}
/**
diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java
index d5c3e4f..bbef261 100644
--- a/services/core/java/com/android/server/wm/WindowContainer.java
+++ b/services/core/java/com/android/server/wm/WindowContainer.java
@@ -109,14 +109,19 @@
// The owner/creator for this container. No controller if null.
WindowContainerController mController;
+ // The display this window container is on.
+ protected DisplayContent mDisplayContent;
+
protected SurfaceControl mSurfaceControl;
private int mLastLayer = 0;
private SurfaceControl mLastRelativeToLayer = null;
+ // TODO(b/132320879): Remove this from WindowContainers except DisplayContent.
+ private final Transaction mPendingTransaction;
+
/**
* Applied as part of the animation pass in "prepareSurfaces".
*/
- protected final Transaction mPendingTransaction;
protected final SurfaceAnimator mSurfaceAnimator;
protected final WindowManagerService mWmService;
@@ -320,12 +325,12 @@
}
if (mSurfaceControl != null) {
- mPendingTransaction.remove(mSurfaceControl);
+ getPendingTransaction().remove(mSurfaceControl);
// Merge to parent transaction to ensure the transactions on this WindowContainer are
// applied in native even if WindowContainer is removed.
if (mParent != null) {
- mParent.getPendingTransaction().merge(mPendingTransaction);
+ mParent.getPendingTransaction().merge(getPendingTransaction());
}
mSurfaceControl = null;
@@ -508,12 +513,20 @@
* @param dc The display this container is on after changes.
*/
void onDisplayChanged(DisplayContent dc) {
+ mDisplayContent = dc;
+ if (dc != null && dc != this) {
+ dc.getPendingTransaction().merge(mPendingTransaction);
+ }
for (int i = mChildren.size() - 1; i >= 0; --i) {
final WindowContainer child = mChildren.get(i);
child.onDisplayChanged(dc);
}
}
+ DisplayContent getDisplayContent() {
+ return mDisplayContent;
+ }
+
void setWaitingForDrawnIfResizingChanged() {
for (int i = mChildren.size() - 1; i >= 0; --i) {
final WindowContainer wc = mChildren.get(i);
@@ -1180,13 +1193,7 @@
}
}
- /**
- * TODO: Once we totally eliminate global transaction we will pass transaction in here
- * rather than merging to global.
- */
void prepareSurfaces() {
- SurfaceControl.mergeToGlobalTransaction(getPendingTransaction());
-
// If a leash has been set when the transaction was committed, then the leash reparent has
// been committed.
mCommittedReparentToAnimationLeash = mSurfaceAnimator.hasLeash();
@@ -1204,8 +1211,8 @@
}
/**
- * Trigger a call to prepareSurfaces from the animation thread, such that
- * mPendingTransaction will be applied.
+ * Trigger a call to prepareSurfaces from the animation thread, such that pending transactions
+ * will be applied.
*/
void scheduleAnimation() {
if (mParent != null) {
@@ -1224,6 +1231,14 @@
@Override
public Transaction getPendingTransaction() {
+ final DisplayContent displayContent = getDisplayContent();
+ if (displayContent != null && displayContent != this) {
+ return displayContent.getPendingTransaction();
+ }
+ // This WindowContainer has not attached to a display yet or this is a DisplayContent, so we
+ // let the caller to save the surface operations within the local mPendingTransaction.
+ // If this is not a DisplayContent, we will merge it to the pending transaction of its
+ // display once it attaches to it.
return mPendingTransaction;
}
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index dd3c600..5ef184a 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -1313,6 +1313,7 @@
mOrientationChangeTimedOut = true;
}
+ @Override
DisplayContent getDisplayContent() {
return mToken.getDisplayContent();
}
@@ -4602,7 +4603,7 @@
new WindowAnimationSpec(anim, mSurfacePosition, false /* canSkipFirstFrame */,
0 /* windowCornerRadius */),
mWmService.mSurfaceAnimationRunner);
- startAnimation(mPendingTransaction, adapter);
+ startAnimation(getPendingTransaction(), adapter);
commitPendingTransaction();
}
diff --git a/services/core/java/com/android/server/wm/WindowToken.java b/services/core/java/com/android/server/wm/WindowToken.java
index f0b9c62..8aee0f2 100644
--- a/services/core/java/com/android/server/wm/WindowToken.java
+++ b/services/core/java/com/android/server/wm/WindowToken.java
@@ -82,9 +82,6 @@
// windows will be put to the bottom of the list.
boolean sendingToBottom;
- // The display this token is on.
- protected DisplayContent mDisplayContent;
-
/** The owner has {@link android.Manifest.permission#MANAGE_APP_TOKENS} */
final boolean mOwnerCanManageAppTokens;
@@ -249,10 +246,6 @@
return null;
}
- DisplayContent getDisplayContent() {
- return mDisplayContent;
- }
-
@Override
void removeImmediately() {
if (mDisplayContent != null) {
@@ -266,7 +259,6 @@
@Override
void onDisplayChanged(DisplayContent dc) {
dc.reParentWindowToken(this);
- mDisplayContent = dc;
// TODO(b/36740756): One day this should perhaps be hooked
// up with goodToGo, so we don't move a window