From af7f6b2297f745e2065a52a7f535ce282c25331c Mon Sep 17 00:00:00 2001 From: Marzia Favaro Date: Thu, 20 Jul 2023 17:17:35 +0000 Subject: [PATCH] Ensure dim changes are in same transaction; do not wait for token to change visibility - Use the pending transaction to avoid collecting changes to the same dim in separate transactions. This was causing problems since multiple WindowStates can request a dim change, and they were doing so by placing the changes in different transactions. This was an issue since there was no guarantee that they were then merged in the correct order. - Check for the requested visibility of the token to avoid waiting for it to change its visibility at the end of a transition. This allows to shield the dim from changes to its windows while it waits to be committed. Fix: 288863491 Test: DimmerTests Change-Id: I96f8d1b091dd0f897c50833be7540b00fa2ead70 --- .../java/com/android/server/wm/Dimmer.java | 15 +++----- .../com/android/server/wm/WindowState.java | 5 ++- .../com/android/server/wm/DimmerTests.java | 37 ++++++++++--------- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/wm/Dimmer.java b/services/core/java/com/android/server/wm/Dimmer.java index 89f044bdd1634..d7667d8ce7a80 100644 --- a/services/core/java/com/android/server/wm/Dimmer.java +++ b/services/core/java/com/android/server/wm/Dimmer.java @@ -215,8 +215,7 @@ class Dimmer { return mDimState; } - private void dim(SurfaceControl.Transaction t, WindowContainer container, int relativeLayer, - float alpha, int blurRadius) { + private void dim(WindowContainer container, int relativeLayer, float alpha, int blurRadius) { final DimState d = getDimState(container); if (d == null) { @@ -226,6 +225,7 @@ class Dimmer { // The dim method is called from WindowState.prepareSurfaces(), which is always called // in the correct Z from lowest Z to highest. This ensures that the dim layer is always // relative to the highest Z layer with a dim. + SurfaceControl.Transaction t = mHost.getPendingTransaction(); t.setRelativeLayer(d.mDimLayer, container.getSurfaceControl(), relativeLayer); t.setAlpha(d.mDimLayer, alpha); t.setBackgroundBlurRadius(d.mDimLayer, blurRadius); @@ -238,26 +238,23 @@ class Dimmer { * for each call to {@link WindowContainer#prepareSurfaces} the Dim state will be reset * and the child should call dimAbove again to request the Dim to continue. * - * @param t A transaction in which to apply the Dim. * @param container The container which to dim above. Should be a child of our host. * @param alpha The alpha at which to Dim. */ - void dimAbove(SurfaceControl.Transaction t, WindowContainer container, float alpha) { - dim(t, container, 1, alpha, 0); + void dimAbove(WindowContainer container, float alpha) { + dim(container, 1, alpha, 0); } /** * Like {@link #dimAbove} but places the dim below the given container. * - * @param t A transaction in which to apply the Dim. * @param container The container which to dim below. Should be a child of our host. * @param alpha The alpha at which to Dim. * @param blurRadius The amount of blur added to the Dim. */ - void dimBelow(SurfaceControl.Transaction t, WindowContainer container, float alpha, - int blurRadius) { - dim(t, container, -1, alpha, blurRadius); + void dimBelow(WindowContainer container, float alpha, int blurRadius) { + dim(container, -1, alpha, blurRadius); } /** diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index cf1e51fb4e94f..cae3dbe769f9a 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -5132,7 +5132,8 @@ class WindowState extends WindowContainer implements WindowManagerP private void applyDims() { if (((mAttrs.flags & FLAG_DIM_BEHIND) != 0 || shouldDrawBlurBehind()) - && isVisibleNow() && !mHidden && mTransitionController.canApplyDim(getTask())) { + && mToken.isVisibleRequested() && isVisibleNow() && !mHidden + && mTransitionController.canApplyDim(getTask())) { // Only show the Dimmer when the following is satisfied: // 1. The window has the flag FLAG_DIM_BEHIND or blur behind is requested // 2. The WindowToken is not hidden so dims aren't shown when the window is exiting. @@ -5142,7 +5143,7 @@ class WindowState extends WindowContainer implements WindowManagerP mIsDimming = true; final float dimAmount = (mAttrs.flags & FLAG_DIM_BEHIND) != 0 ? mAttrs.dimAmount : 0; final int blurRadius = shouldDrawBlurBehind() ? mAttrs.getBlurBehindRadius() : 0; - getDimmer().dimBelow(getSyncTransaction(), this, dimAmount, blurRadius); + getDimmer().dimBelow(this, dimAmount, blurRadius); } } diff --git a/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java b/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java index f235d153c6584..233a2076a8670 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java @@ -52,7 +52,8 @@ public class DimmerTests extends WindowTestsBase { private static class TestWindowContainer extends WindowContainer { final SurfaceControl mControl = mock(SurfaceControl.class); - final SurfaceControl.Transaction mTransaction = spy(StubTransaction.class); + final SurfaceControl.Transaction mPendingTransaction = spy(StubTransaction.class); + final SurfaceControl.Transaction mSyncTransaction = spy(StubTransaction.class); TestWindowContainer(WindowManagerService wm) { super(wm); @@ -65,12 +66,12 @@ public class DimmerTests extends WindowTestsBase { @Override public SurfaceControl.Transaction getSyncTransaction() { - return mTransaction; + return mSyncTransaction; } @Override public SurfaceControl.Transaction getPendingTransaction() { - return mTransaction; + return mPendingTransaction; } } @@ -144,7 +145,7 @@ public class DimmerTests extends WindowTestsBase { mHost.addChild(child, 0); final float alpha = 0.8f; - mDimmer.dimAbove(mTransaction, child, alpha); + mDimmer.dimAbove(child, alpha); int width = 100; int height = 300; @@ -161,13 +162,13 @@ public class DimmerTests extends WindowTestsBase { mHost.addChild(child, 0); final float alpha = 0.8f; - mDimmer.dimAbove(mTransaction, child, alpha); + mDimmer.dimAbove(child, alpha); SurfaceControl dimLayer = getDimLayer(); assertNotNull("Dimmer should have created a surface", dimLayer); - verify(mTransaction).setAlpha(dimLayer, alpha); - verify(mTransaction).setRelativeLayer(dimLayer, child.mControl, 1); + verify(mHost.getPendingTransaction()).setAlpha(dimLayer, alpha); + verify(mHost.getPendingTransaction()).setRelativeLayer(dimLayer, child.mControl, 1); } @Test @@ -176,13 +177,13 @@ public class DimmerTests extends WindowTestsBase { mHost.addChild(child, 0); final float alpha = 0.8f; - mDimmer.dimBelow(mTransaction, child, alpha, 0); + mDimmer.dimBelow(child, alpha, 0); SurfaceControl dimLayer = getDimLayer(); assertNotNull("Dimmer should have created a surface", dimLayer); - verify(mTransaction).setAlpha(dimLayer, alpha); - verify(mTransaction).setRelativeLayer(dimLayer, child.mControl, -1); + verify(mHost.getPendingTransaction()).setAlpha(dimLayer, alpha); + verify(mHost.getPendingTransaction()).setRelativeLayer(dimLayer, child.mControl, -1); } @Test @@ -191,7 +192,7 @@ public class DimmerTests extends WindowTestsBase { mHost.addChild(child, 0); final float alpha = 0.8f; - mDimmer.dimAbove(mTransaction, child, alpha); + mDimmer.dimAbove(child, alpha); SurfaceControl dimLayer = getDimLayer(); mDimmer.resetDimStates(); @@ -208,10 +209,10 @@ public class DimmerTests extends WindowTestsBase { mHost.addChild(child, 0); final float alpha = 0.8f; - mDimmer.dimAbove(mTransaction, child, alpha); + mDimmer.dimAbove(child, alpha); SurfaceControl dimLayer = getDimLayer(); mDimmer.resetDimStates(); - mDimmer.dimAbove(mTransaction, child, alpha); + mDimmer.dimAbove(child, alpha); mDimmer.updateDims(mTransaction); verify(mTransaction).show(dimLayer); @@ -224,7 +225,7 @@ public class DimmerTests extends WindowTestsBase { mHost.addChild(child, 0); final float alpha = 0.8f; - mDimmer.dimAbove(mTransaction, child, alpha); + mDimmer.dimAbove(child, alpha); final Rect bounds = mDimmer.mDimState.mDimBounds; SurfaceControl dimLayer = getDimLayer(); @@ -245,7 +246,7 @@ public class DimmerTests extends WindowTestsBase { TestWindowContainer child = new TestWindowContainer(mWm); mHost.addChild(child, 0); - mDimmer.dimAbove(mTransaction, child, 1); + mDimmer.dimAbove(child, 1); SurfaceControl dimLayer = getDimLayer(); mDimmer.updateDims(mTransaction); verify(mTransaction, times(1)).show(dimLayer); @@ -266,13 +267,13 @@ public class DimmerTests extends WindowTestsBase { mHost.addChild(child, 0); final int blurRadius = 50; - mDimmer.dimBelow(mTransaction, child, 0, blurRadius); + mDimmer.dimBelow(child, 0, blurRadius); SurfaceControl dimLayer = getDimLayer(); assertNotNull("Dimmer should have created a surface", dimLayer); - verify(mTransaction).setBackgroundBlurRadius(dimLayer, blurRadius); - verify(mTransaction).setRelativeLayer(dimLayer, child.mControl, -1); + verify(mHost.getPendingTransaction()).setBackgroundBlurRadius(dimLayer, blurRadius); + verify(mHost.getPendingTransaction()).setRelativeLayer(dimLayer, child.mControl, -1); } private SurfaceControl getDimLayer() {