From a805cf591031fd8a7eed92f1cd3abff338c243f5 Mon Sep 17 00:00:00 2001 From: Tiger Huang Date: Thu, 21 Jul 2022 20:37:46 +0800 Subject: [PATCH] Let the client know the initial visibility of an InsetsSourceControl Previously, the client compares the requested visibility with the source visibility to decide if it needs to play the insets animation or not. However, the source visibility is not stable while the client is gaining the control. If the source and the control come from IWindow#insetsControlChanged, the source has the previous visibility. So the client can detect the visibility change. If the the source and the control come from IWindowSession#relayout, the visibility of the source has been overriden by the requested visibility of the client. So the client cannot detect the change and cannot play the animation. This CL lets the client know the initial visibility of the control. While gaining leash, the client can use the information to decide if it needs to play animation or not. Fix: 209064170 Fix: 238496738 Test: atest ImeInsetsSourceConsumerTest InsetsAnimationControlImplTest InsetsControllerTest InsetsSourceConsumerTest DisplayImeControllerTest Test: 1. Enable shell transitions 2. Open camera. 3. Perform back gesture. See if there is status bar animation. Change-Id: I2c02e97e191ebd83238c0c54908e861d200d4c8d --- .../android/view/InsetsSourceConsumer.java | 42 ++++--------------- .../android/view/InsetsSourceControl.java | 24 +++++++---- .../view/ImeInsetsSourceConsumerTest.java | 6 +-- .../view/InsetsAnimationControlImplTest.java | 5 ++- .../android/view/InsetsControllerTest.java | 5 ++- .../view/InsetsSourceConsumerTest.java | 10 +++-- .../common/DisplayImeControllerTest.java | 2 +- .../server/wm/InsetsSourceProvider.java | 9 ++-- 8 files changed, 45 insertions(+), 58 deletions(-) diff --git a/core/java/android/view/InsetsSourceConsumer.java b/core/java/android/view/InsetsSourceConsumer.java index b17e19998a9d4..d63c25a093829 100644 --- a/core/java/android/view/InsetsSourceConsumer.java +++ b/core/java/android/view/InsetsSourceConsumer.java @@ -25,7 +25,6 @@ import static android.view.InsetsSourceConsumerProto.IS_REQUESTED_VISIBLE; import static android.view.InsetsSourceConsumerProto.PENDING_FRAME; import static android.view.InsetsSourceConsumerProto.PENDING_VISIBLE_FRAME; import static android.view.InsetsSourceConsumerProto.SOURCE_CONTROL; -import static android.view.InsetsSourceControl.INVALID_HINTS; import static android.view.InsetsState.ITYPE_IME; import static android.view.InsetsState.getDefaultVisibility; import static android.view.InsetsState.toPublicType; @@ -34,7 +33,6 @@ import static com.android.internal.annotations.VisibleForTesting.Visibility.PACK import android.annotation.IntDef; import android.annotation.Nullable; -import android.graphics.Insets; import android.graphics.Rect; import android.util.ArraySet; import android.util.Log; @@ -93,13 +91,6 @@ public class InsetsSourceConsumer { private Rect mPendingFrame; private Rect mPendingVisibleFrame; - /** - * Indicates if we have the pending animation. When we have the control, we need to play the - * animation if the requested visibility is different from the current state. But if we haven't - * had a leash yet, we will set this flag, and play the animation once we get the leash. - */ - private boolean mIsAnimationPending; - /** * @param type The {@link InternalInsetsType} of the consumed insets. * @param state The current {@link InsetsState} of the consumed insets. @@ -138,7 +129,6 @@ public class InsetsSourceConsumer { } return false; } - SurfaceControl oldLeash = mSourceControl != null ? mSourceControl.getLeash() : null; final InsetsSourceControl lastControl = mSourceControl; mSourceControl = control; @@ -163,27 +153,21 @@ public class InsetsSourceConsumer { // For updateCompatSysUiVisibility applyLocalVisibilityOverride(); } else { - // We are gaining control, and need to run an animation since previous state - // didn't match final boolean requestedVisible = isRequestedVisibleAwaitingControl(); - final boolean fakeControl = INVALID_HINTS.equals(control.getInsetsHint()); - final boolean needsAnimation = requestedVisible != mState.getSource(mType).isVisible() - && !fakeControl; - if (control.getLeash() != null && (needsAnimation || mIsAnimationPending)) { - if (DEBUG) Log.d(TAG, String.format("Gaining control in %s, requestedVisible: %b", + final SurfaceControl oldLeash = lastControl != null ? lastControl.getLeash() : null; + final SurfaceControl newLeash = control.getLeash(); + if (newLeash != null && (oldLeash == null || !newLeash.isSameSurface(oldLeash)) + && requestedVisible != control.isInitiallyVisible()) { + // We are gaining leash, and need to run an animation since previous state + // didn't match. + if (DEBUG) Log.d(TAG, String.format("Gaining leash in %s, requestedVisible: %b", mController.getHost().getRootViewTitle(), requestedVisible)); if (requestedVisible) { showTypes[0] |= toPublicType(getType()); } else { hideTypes[0] |= toPublicType(getType()); } - mIsAnimationPending = false; } else { - if (needsAnimation) { - // We need animation but we haven't had a leash yet. Set this flag that when we - // get the leash we can play the deferred animation. - mIsAnimationPending = true; - } // We are gaining control, but don't need to run an animation. // However make sure that the leash visibility is still up to date. if (applyLocalVisibilityOverride()) { @@ -195,7 +179,7 @@ public class InsetsSourceConsumer { applyRequestedVisibilityToControl(); // Remove the surface that owned by last control when it lost. - if (!requestedVisible && !mIsAnimationPending && lastControl == null) { + if (!requestedVisible && lastControl == null) { removeSurface(); } } @@ -406,16 +390,6 @@ public class InsetsSourceConsumer { protected void setRequestedVisible(boolean requestedVisible) { if (mRequestedVisible != requestedVisible) { mRequestedVisible = requestedVisible; - - // We need an animation later if the leash of a real control (which has an insets hint) - // is not ready. The !mIsAnimationPending check is in case that the requested visibility - // is changed twice before playing the animation -- we don't need an animation in this - // case. - mIsAnimationPending = !mIsAnimationPending - && mSourceControl != null - && mSourceControl.getLeash() == null - && !Insets.NONE.equals(mSourceControl.getInsetsHint()); - mController.onRequestedVisibilityChanged(this); if (DEBUG) Log.d(TAG, "setRequestedVisible: " + requestedVisible); } diff --git a/core/java/android/view/InsetsSourceControl.java b/core/java/android/view/InsetsSourceControl.java index 2cf827db38db1..5f1cbba58d3c2 100644 --- a/core/java/android/view/InsetsSourceControl.java +++ b/core/java/android/view/InsetsSourceControl.java @@ -31,6 +31,7 @@ import android.util.proto.ProtoOutputStream; import android.view.InsetsState.InternalInsetsType; import java.io.PrintWriter; +import java.util.Objects; import java.util.function.Consumer; /** @@ -39,10 +40,9 @@ import java.util.function.Consumer; */ public class InsetsSourceControl implements Parcelable { - public static final Insets INVALID_HINTS = Insets.of(-1, -1, -1, -1); - private final @InternalInsetsType int mType; private final @Nullable SurfaceControl mLeash; + private final boolean mInitiallyVisible; private final Point mSurfacePosition; // This is used while playing an insets animation regardless of the relative frame. This would @@ -53,9 +53,10 @@ public class InsetsSourceControl implements Parcelable { private int mParcelableFlags; public InsetsSourceControl(@InternalInsetsType int type, @Nullable SurfaceControl leash, - Point surfacePosition, Insets insetsHint) { + boolean initiallyVisible, Point surfacePosition, Insets insetsHint) { mType = type; mLeash = leash; + mInitiallyVisible = initiallyVisible; mSurfacePosition = surfacePosition; mInsetsHint = insetsHint; } @@ -67,6 +68,7 @@ public class InsetsSourceControl implements Parcelable { } else { mLeash = null; } + mInitiallyVisible = other.mInitiallyVisible; mSurfacePosition = new Point(other.mSurfacePosition); mInsetsHint = other.mInsetsHint; mSkipAnimationOnce = other.getAndClearSkipAnimationOnce(); @@ -75,6 +77,7 @@ public class InsetsSourceControl implements Parcelable { public InsetsSourceControl(Parcel in) { mType = in.readInt(); mLeash = in.readTypedObject(SurfaceControl.CREATOR); + mInitiallyVisible = in.readBoolean(); mSurfacePosition = in.readTypedObject(Point.CREATOR); mInsetsHint = in.readTypedObject(Insets.CREATOR); mSkipAnimationOnce = in.readBoolean(); @@ -94,6 +97,10 @@ public class InsetsSourceControl implements Parcelable { return mLeash; } + public boolean isInitiallyVisible() { + return mInitiallyVisible; + } + public boolean setSurfacePosition(int left, int top) { if (mSurfacePosition.equals(left, top)) { return false; @@ -148,6 +155,7 @@ public class InsetsSourceControl implements Parcelable { public void writeToParcel(Parcel dest, int flags) { dest.writeInt(mType); dest.writeTypedObject(mLeash, mParcelableFlags); + dest.writeBoolean(mInitiallyVisible); dest.writeTypedObject(mSurfacePosition, mParcelableFlags); dest.writeTypedObject(mInsetsHint, mParcelableFlags); dest.writeBoolean(mSkipAnimationOnce); @@ -172,6 +180,7 @@ public class InsetsSourceControl implements Parcelable { return mType == that.mType && ((mLeash == thatLeash) || (mLeash != null && thatLeash != null && mLeash.isSameSurface(thatLeash))) + && mInitiallyVisible == that.mInitiallyVisible && mSurfacePosition.equals(that.mSurfacePosition) && mInsetsHint.equals(that.mInsetsHint) && mSkipAnimationOnce == that.mSkipAnimationOnce; @@ -179,12 +188,8 @@ public class InsetsSourceControl implements Parcelable { @Override public int hashCode() { - int result = mType; - result = 31 * result + (mLeash != null ? mLeash.hashCode() : 0); - result = 31 * result + mSurfacePosition.hashCode(); - result = 31 * result + mInsetsHint.hashCode(); - result = 31 * result + (mSkipAnimationOnce ? 1 : 0); - return result; + return Objects.hash(mType, mLeash, mInitiallyVisible, mSurfacePosition, mInsetsHint, + mSkipAnimationOnce); } @Override @@ -200,6 +205,7 @@ public class InsetsSourceControl implements Parcelable { pw.print(prefix); pw.print("InsetsSourceControl type="); pw.print(InsetsState.typeToString(mType)); pw.print(" mLeash="); pw.print(mLeash); + pw.print(" mInitiallyVisible="); pw.print(mInitiallyVisible); pw.print(" mSurfacePosition="); pw.print(mSurfacePosition); pw.print(" mInsetsHint="); pw.print(mInsetsHint); pw.print(" mSkipAnimationOnce="); pw.print(mSkipAnimationOnce); diff --git a/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java b/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java index 34a1fd89e4f88..44bb0620b8067 100644 --- a/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java +++ b/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java @@ -92,7 +92,7 @@ public class ImeInsetsSourceConsumerTest { @Test public void testImeVisibility() { final InsetsSourceControl ime = - new InsetsSourceControl(ITYPE_IME, mLeash, new Point(), Insets.NONE); + new InsetsSourceControl(ITYPE_IME, mLeash, false, new Point(), Insets.NONE); mController.onControlsChanged(new InsetsSourceControl[] { ime }); InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { @@ -121,7 +121,7 @@ public class ImeInsetsSourceConsumerTest { // set control and verify visibility is applied. InsetsSourceControl control = - new InsetsSourceControl(ITYPE_IME, mLeash, new Point(), Insets.NONE); + new InsetsSourceControl(ITYPE_IME, mLeash, false, new Point(), Insets.NONE); mController.onControlsChanged(new InsetsSourceControl[] { control }); // IME show animation should be triggered when control becomes available. verify(mController).applyAnimation( @@ -158,7 +158,7 @@ public class ImeInsetsSourceConsumerTest { // set control and verify visibility is applied. InsetsSourceControl control = Mockito.spy( - new InsetsSourceControl(ITYPE_IME, mLeash, new Point(), Insets.NONE)); + new InsetsSourceControl(ITYPE_IME, mLeash, false, new Point(), Insets.NONE)); // Simulate IME source control set this flag when the target has starting window. control.setSkipAnimationOnce(true); diff --git a/core/tests/coretests/src/android/view/InsetsAnimationControlImplTest.java b/core/tests/coretests/src/android/view/InsetsAnimationControlImplTest.java index 4f1da1b296166..d0f7fe04e17d2 100644 --- a/core/tests/coretests/src/android/view/InsetsAnimationControlImplTest.java +++ b/core/tests/coretests/src/android/view/InsetsAnimationControlImplTest.java @@ -95,12 +95,13 @@ public class InsetsAnimationControlImplTest { () -> mMockTransaction, mMockController); topConsumer.setControl( new InsetsSourceControl( - ITYPE_STATUS_BAR, mTopLeash, new Point(0, 0), Insets.of(0, 100, 0, 0)), + ITYPE_STATUS_BAR, mTopLeash, true, new Point(0, 0), + Insets.of(0, 100, 0, 0)), new int[1], new int[1]); InsetsSourceConsumer navConsumer = new InsetsSourceConsumer(ITYPE_NAVIGATION_BAR, mInsetsState, () -> mMockTransaction, mMockController); - navConsumer.setControl(new InsetsSourceControl(ITYPE_NAVIGATION_BAR, mNavLeash, + navConsumer.setControl(new InsetsSourceControl(ITYPE_NAVIGATION_BAR, mNavLeash, true, new Point(400, 0), Insets.of(0, 0, 100, 0)), new int[1], new int[1]); navConsumer.hide(); diff --git a/core/tests/coretests/src/android/view/InsetsControllerTest.java b/core/tests/coretests/src/android/view/InsetsControllerTest.java index dcb1835204dd4..ed6a649021a13 100644 --- a/core/tests/coretests/src/android/view/InsetsControllerTest.java +++ b/core/tests/coretests/src/android/view/InsetsControllerTest.java @@ -223,7 +223,7 @@ public class InsetsControllerTest { InsetsSourceControl control = new InsetsSourceControl( - ITYPE_STATUS_BAR, mLeash, new Point(), Insets.of(0, 10, 0, 0)); + ITYPE_STATUS_BAR, mLeash, true, new Point(), Insets.of(0, 10, 0, 0)); mController.onControlsChanged(new InsetsSourceControl[]{control}); mController.controlWindowInsetsAnimation(0, 0 /* durationMs */, new LinearInterpolator(), @@ -926,7 +926,8 @@ public class InsetsControllerTest { // Simulate binder behavior by copying SurfaceControl. Otherwise, InsetsController will // attempt to release mLeash directly. SurfaceControl copy = new SurfaceControl(mLeash, "InsetsControllerTest.createControl"); - return new InsetsSourceControl(type, copy, new Point(), Insets.NONE); + return new InsetsSourceControl(type, copy, InsetsState.getDefaultVisibility(type), + new Point(), Insets.NONE); } private InsetsSourceControl[] createSingletonControl(@InternalInsetsType int type) { diff --git a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java index b3aa7e86efd55..2054b4fe9a354 100644 --- a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java +++ b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java @@ -110,7 +110,8 @@ public class InsetsSourceConsumerTest { instrumentation.waitForIdleSync(); mConsumer.setControl( - new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, new Point(), Insets.NONE), + new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, true /* initialVisible */, + new Point(), Insets.NONE), new int[1], new int[1]); } @@ -180,7 +181,8 @@ public class InsetsSourceConsumerTest { verifyZeroInteractions(mMockTransaction); int[] hideTypes = new int[1]; mConsumer.setControl( - new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, new Point(), Insets.NONE), + new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, true /* initialVisible */, + new Point(), Insets.NONE), new int[1], hideTypes); assertEquals(statusBars(), hideTypes[0]); assertFalse(mRemoveSurfaceCalled); @@ -191,14 +193,14 @@ public class InsetsSourceConsumerTest { public void testRestore_noAnimation() { InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { mConsumer.hide(); - mController.onStateChanged(mState); mConsumer.setControl(null, new int[1], new int[1]); reset(mMockTransaction); verifyZeroInteractions(mMockTransaction); mRemoveSurfaceCalled = false; int[] hideTypes = new int[1]; mConsumer.setControl( - new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, new Point(), Insets.NONE), + new InsetsSourceControl(ITYPE_STATUS_BAR, mLeash, false /* initialVisible */, + new Point(), Insets.NONE), new int[1], hideTypes); assertTrue(mRemoveSurfaceCalled); assertEquals(0, hideTypes[0]); diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/DisplayImeControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/DisplayImeControllerTest.java index 587782cb79ad2..5b691f231d85a 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/DisplayImeControllerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/DisplayImeControllerTest.java @@ -122,7 +122,7 @@ public class DisplayImeControllerTest extends ShellTestCase { private InsetsSourceControl[] insetsSourceControl() { return new InsetsSourceControl[]{ new InsetsSourceControl( - ITYPE_IME, mock(SurfaceControl.class), new Point(0, 0), Insets.NONE) + ITYPE_IME, mock(SurfaceControl.class), false, new Point(0, 0), Insets.NONE) }; } diff --git a/services/core/java/com/android/server/wm/InsetsSourceProvider.java b/services/core/java/com/android/server/wm/InsetsSourceProvider.java index 86a73c935e522..bf4b65da8b431 100644 --- a/services/core/java/com/android/server/wm/InsetsSourceProvider.java +++ b/services/core/java/com/android/server/wm/InsetsSourceProvider.java @@ -125,7 +125,8 @@ abstract class InsetsSourceProvider { mDisplayContent = displayContent; mStateController = stateController; mFakeControl = new InsetsSourceControl( - source.getType(), null /* leash */, new Point(), InsetsSourceControl.INVALID_HINTS); + source.getType(), null /* leash */, false /* initialVisible */, new Point(), + Insets.NONE); mControllable = InsetsPolicy.isInsetsTypeControllable(source.getType()); } @@ -468,7 +469,8 @@ abstract class InsetsSourceProvider { final SurfaceControl leash = mAdapter.mCapturedLeash; mControlTarget = target; updateVisibility(); - mControl = new InsetsSourceControl(mSource.getType(), leash, surfacePosition, mInsetsHint); + mControl = new InsetsSourceControl(mSource.getType(), leash, mClientVisible, + surfacePosition, mInsetsHint); ProtoLog.d(WM_DEBUG_WINDOW_INSETS, "InsetsSource Control %s for target %s", mControl, mControlTarget); @@ -553,7 +555,8 @@ abstract class InsetsSourceProvider { // to the client in case that the client applies its transaction sooner than ours // that we could unexpectedly overwrite the surface state. return new InsetsSourceControl(mControl.getType(), null /* leash */, - mControl.getSurfacePosition(), mControl.getInsetsHint()); + mControl.isInitiallyVisible(), mControl.getSurfacePosition(), + mControl.getInsetsHint()); } return mControl; }