From cb97a11bea0e2e6bb37b003c0851b0398c888307 Mon Sep 17 00:00:00 2001 From: Joshua Tsuji Date: Wed, 29 May 2019 16:20:41 -0400 Subject: [PATCH] Remove the max rendered children code. This code is redundant given the BubbleData's handling of the bubble limit, and is responsible for the invisible bubble bugs. Test: atest SystemUITests Fixes: 132690748 Change-Id: I7f05f2ee12183f812abb55bf133c11dafce10909 --- .../systemui/bubbles/BubbleStackView.java | 2 - .../ExpandedAnimationController.java | 21 +----- .../animation/PhysicsAnimationLayout.java | 69 ++----------------- .../ExpandedAnimationControllerTest.java | 17 ++--- .../animation/PhysicsAnimationLayoutTest.java | 62 +++-------------- .../PhysicsAnimationLayoutTestCase.java | 14 ++-- .../StackAnimationControllerTest.java | 2 +- 7 files changed, 26 insertions(+), 161 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java index 29a69fb15ba6f..5540dc648ca06 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java @@ -330,8 +330,6 @@ public class BubbleStackView extends FrameLayout { mSurfaceSynchronizer = synchronizer != null ? synchronizer : DEFAULT_SURFACE_SYNCHRONIZER; mBubbleContainer = new PhysicsAnimationLayout(context); - mBubbleContainer.setMaxRenderedChildren( - getResources().getInteger(R.integer.bubbles_max_rendered)); mBubbleContainer.setActiveController(mStackAnimationController); mBubbleContainer.setElevation(elevation); mBubbleContainer.setClipChildren(false); diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/animation/ExpandedAnimationController.java b/packages/SystemUI/src/com/android/systemui/bubbles/animation/ExpandedAnimationController.java index b52280a12a64a..67ae18600f09d 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/animation/ExpandedAnimationController.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/animation/ExpandedAnimationController.java @@ -63,8 +63,6 @@ public class ExpandedAnimationController private Point mDisplaySize; /** Size of dismiss target at bottom of screen. */ private float mPipDismissHeight; - /** Max number of bubbles shown in row above expanded view.*/ - private int mBubblesMaxRendered; /** Whether the dragged-out bubble is in the dismiss target. */ private boolean mIndividualBubbleWithinDismissTarget = false; @@ -291,7 +289,6 @@ public class ExpandedAnimationController mStatusBarHeight = res.getDimensionPixelSize(com.android.internal.R.dimen.status_bar_height); mPipDismissHeight = res.getDimensionPixelSize(R.dimen.pip_dismiss_gradient_height); - mBubblesMaxRendered = res.getInteger(R.integer.bubbles_max_rendered); // Ensure that all child views are at 1x scale, and visible, in case they were animating // in. @@ -360,19 +357,6 @@ public class ExpandedAnimationController updateBubblePositions(); } - @Override - protected void setChildVisibility(View child, int index, int visibility) { - if (visibility == View.VISIBLE) { - // Set alpha to 0 but then become visible immediately so the animation is visible. - child.setAlpha(0f); - child.setVisibility(View.VISIBLE); - } - - animationForChild(child) - .alpha(visibility == View.GONE ? 0f : 1f) - .start(() -> super.setChildVisibility(child, index, visibility) /* after */); - } - private void updateBubblePositions() { for (int i = 0; i < mLayout.getChildCount(); i++) { final View bubble = mLayout.getChildAt(i); @@ -407,10 +391,7 @@ public class ExpandedAnimationController return 0; } int bubbleCount = mLayout.getChildCount(); - if (bubbleCount > mBubblesMaxRendered) { - // Only shown bubbles are relevant for calculating position. - bubbleCount = mBubblesMaxRendered; - } + // Width calculations. double bubble = bubbleCount * mBubbleSizePx; float gap = (bubbleCount - 1) * mBubblePaddingPx; diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayout.java b/packages/SystemUI/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayout.java index 6390f844c64c4..b8535adfc2804 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayout.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayout.java @@ -169,15 +169,6 @@ public class PhysicsAnimationLayout extends FrameLayout { return mLayout; } - /** - * Sets the child's visibility when it moves beyond or within the limits set by a call to - * {@link PhysicsAnimationLayout#setMaxRenderedChildren}. This can be overridden to animate - * this transition. - */ - protected void setChildVisibility(View child, int index, int visibility) { - child.setVisibility(visibility); - } - /** * Returns a {@link PhysicsPropertyAnimator} instance for the given child view. */ @@ -260,25 +251,10 @@ public class PhysicsAnimationLayout extends FrameLayout { /** The currently active animation controller. */ @Nullable protected PhysicsAnimationController mController; - /** - * The maximum number of children to render and animate at a time. See - * {@link #setMaxRenderedChildren}. - */ - private int mMaxRenderedChildren = 5; - public PhysicsAnimationLayout(Context context) { super(context); } - /** - * The maximum number of children to render and animate at a time. Any child views added beyond - * this limit will be set to {@link View#GONE}. If any animations attempt to run on the view, - * the corresponding property will be set with no animation. - */ - public void setMaxRenderedChildren(int max) { - this.mMaxRenderedChildren = max; - } - /** * Sets the animation controller and constructs or reconfigures the layout's physics animations * to meet the controller's specifications. @@ -346,8 +322,6 @@ public class PhysicsAnimationLayout extends FrameLayout { mController.onChildAdded(child, index); } - - setChildrenVisibility(); } @Override @@ -378,8 +352,6 @@ public class PhysicsAnimationLayout extends FrameLayout { super.removeView(view); addTransientView(view, index); - setChildrenVisibility(); - // Tell the controller to animate this view out, and call the callback when it's // finished. mController.onChildRemoved(view, index, () -> { @@ -508,33 +480,16 @@ public class PhysicsAnimationLayout extends FrameLayout { SpringAnimation newAnim = new SpringAnimation(child, property); newAnim.addUpdateListener((animation, value, velocity) -> { final int indexOfChild = indexOfChild(child); - final int nextAnimInChain = - mController.getNextAnimationInChain(property, indexOfChild); + final int nextAnimInChain = mController.getNextAnimationInChain(property, indexOfChild); if (nextAnimInChain == PhysicsAnimationController.NONE || indexOfChild < 0) { return; } - final int animIndex = indexOfChild(child); - final float offset = - mController.getOffsetForChainedPropertyAnimation(property); - - // If this property's animations should be chained, then check to see if there is a - // subsequent animation within the rendering limit, and if so, tell it to animate to - // this animation's new value (plus the offset). - if (nextAnimInChain < Math.min(getChildCount(), mMaxRenderedChildren)) { - getAnimationAtIndex(property, animIndex + 1) + final float offset = mController.getOffsetForChainedPropertyAnimation(property); + if (nextAnimInChain < getChildCount()) { + getAnimationAtIndex(property, nextAnimInChain) .animateToFinalPosition(value + offset); - } else if (nextAnimInChain < getChildCount()) { - // If the next child view is not rendered, update the property directly without - // animating it, so that the view is still in the correct state if it later - // becomes visible. - for (int i = nextAnimInChain; i < getChildCount(); i++) { - // 'value' here is the value of the last child within the rendering limit, - // not the first child's value - so we want to subtract the last child's - // index when calculating the offset. - property.setValue(getChildAt(i), value + offset * (i - animIndex)); - } } }); @@ -543,22 +498,6 @@ public class PhysicsAnimationLayout extends FrameLayout { child.setTag(getTagIdForProperty(property), newAnim); } - /** Hides children beyond the max rendering count. */ - private void setChildrenVisibility() { - for (int i = 0; i < getChildCount(); i++) { - final int targetVisibility = i < mMaxRenderedChildren ? View.VISIBLE : View.GONE; - final View targetView = getChildAt(i); - - if (targetView.getVisibility() != targetVisibility) { - if (mController != null) { - mController.setChildVisibility(targetView, i, targetVisibility); - } else { - targetView.setVisibility(targetVisibility); - } - } - } - } - /** Return a stable ID to use as a tag key for the given property's animations. */ private int getTagIdForProperty(DynamicAnimation.ViewProperty property) { if (property.equals(DynamicAnimation.TRANSLATION_X)) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/ExpandedAnimationControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/ExpandedAnimationControllerTest.java index 34f2f40bcaf80..5df5c6e32db98 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/ExpandedAnimationControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/ExpandedAnimationControllerTest.java @@ -60,7 +60,7 @@ public class ExpandedAnimationControllerTest extends PhysicsAnimationLayoutTestC @Before public void setUp() throws Exception { super.setUp(); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); mLayout.setActiveController(mExpandedController); Resources res = mLayout.getResources(); @@ -236,26 +236,19 @@ public class ExpandedAnimationControllerTest extends PhysicsAnimationLayoutTestC assertEquals(x + i * offsetMultiplier * mStackOffset, mLayout.getChildAt(i).getTranslationX(), 2f); assertEquals(y, mLayout.getChildAt(i).getTranslationY(), 2f); - - if (i < mMaxRenderedBubbles) { - assertEquals(1f, mLayout.getChildAt(i).getAlpha(), .01f); - } + assertEquals(1f, mLayout.getChildAt(i).getAlpha(), .01f); } } /** Check that children are in the correct positions for being expanded. */ private void testBubblesInCorrectExpandedPositions() { // Check all the visible bubbles to see if they're in the right place. - for (int i = 0; i < Math.min(mLayout.getChildCount(), mMaxRenderedBubbles); i++) { + for (int i = 0; i < mLayout.getChildCount(); i++) { assertEquals(getBubbleLeft(i), mLayout.getChildAt(i).getTranslationX(), 2f); assertEquals(mExpandedController.getExpandedY(), mLayout.getChildAt(i).getTranslationY(), 2f); - - if (i < mMaxRenderedBubbles) { - assertEquals(1f, mLayout.getChildAt(i).getAlpha(), .01f); - } } } @@ -273,9 +266,7 @@ public class ExpandedAnimationControllerTest extends PhysicsAnimationLayoutTestC return 0; } int bubbleCount = mLayout.getChildCount(); - if (bubbleCount > mMaxRenderedBubbles) { - bubbleCount = mMaxRenderedBubbles; - } + // Width calculations. double bubble = bubbleCount * mBubbleSize; float gap = (bubbleCount - 1) * mBubblePadding; diff --git a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTest.java index 4415f2e636339..95e01774f948a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTest.java @@ -76,22 +76,10 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { mTestableController.setRemoveImmediately(false); } - @Test - public void testRenderVisibility() throws InterruptedException { - mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); - - // The last child should be GONE, the rest VISIBLE. - for (int i = 0; i < mMaxRenderedBubbles + 1; i++) { - assertEquals(i == mMaxRenderedBubbles ? View.GONE : View.VISIBLE, - mLayout.getChildAt(i).getVisibility()); - } - } - @Test public void testHierarchyChanges() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); // Make sure the controller was notified of all the views we added. for (View mView : mViews) { @@ -116,7 +104,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { @Test public void testUpdateValueNotChained() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); // Don't chain any values. mTestableController.setChainedProperties(Sets.newHashSet()); @@ -147,7 +135,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { @Test public void testSetEndActions() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); mTestableController.setChainedProperties(Sets.newHashSet()); final CountDownLatch xLatch = new CountDownLatch(1); @@ -190,7 +178,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { @Test public void testRemoveEndListeners() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); mTestableController.setChainedProperties(Sets.newHashSet()); final CountDownLatch xLatch = new CountDownLatch(1); @@ -229,7 +217,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { public void testSetController() throws InterruptedException { // Add the bubbles, then set the controller, to make sure that a controller added to an // already-initialized view works correctly. - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); mLayout.setActiveController(mTestableController); testChainedTranslationAnimations(); @@ -284,7 +272,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { @Test public void testArePropertiesAnimating() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); assertFalse(mLayout.arePropertiesAnimating( DynamicAnimation.TRANSLATION_X, DynamicAnimation.TRANSLATION_Y)); @@ -308,7 +296,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { @Test public void testCancelAllAnimations() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); mTestableController.animationForChildAtIndex(0) .position(1000, 1000) @@ -321,29 +309,10 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { assertTrue(mViews.get(0).getTranslationY() < 1000); } - @Test - public void testSetChildVisibility() throws InterruptedException { - mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); - - // The last view should have been set to GONE by the controller, since we added one more - // than the limit and it got pushed off. None of the first children should have been set - // VISIBLE, since they would have been animated in by onChildAdded. - Mockito.verify(mTestableController).setChildVisibility( - mViews.get(mViews.size() - 1), 5, View.GONE); - Mockito.verify(mTestableController, never()).setChildVisibility( - any(View.class), anyInt(), eq(View.VISIBLE)); - - // Remove the first view, which should cause the last view to become visible again. - mLayout.removeView(mViews.get(0)); - Mockito.verify(mTestableController).setChildVisibility( - mViews.get(mViews.size() - 1), 4, View.VISIBLE); - } - /** Standard test of chained translation animations. */ private void testChainedTranslationAnimations() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); assertEquals(0, mLayout.getChildAt(0).getTranslationX(), .1f); assertEquals(0, mLayout.getChildAt(1).getTranslationX(), .1f); @@ -354,11 +323,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { waitForPropertyAnimations(DynamicAnimation.TRANSLATION_X); - // Since we enabled chaining, animating the first view to 100 should animate the second to - // 115 (since we set the offset to 15) and the third to 130, etc. Despite the sixth bubble - // not being visible, or animated, make sure that it has the appropriate chained - // translation. - for (int i = 0; i < mMaxRenderedBubbles + 1; i++) { + for (int i = 0; i < mLayout.getChildCount(); i++) { assertEquals( 100 + i * TEST_TRANSLATION_X_OFFSET, mLayout.getChildAt(i).getTranslationX(), .1f); @@ -384,7 +349,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { @Test public void testPhysicsAnimator() throws InterruptedException { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); Runnable afterAll = Mockito.mock(Runnable.class); Runnable after = Mockito.spy(new Runnable() { @@ -432,7 +397,7 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { mLayout.setActiveController(mTestableController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); Runnable allEnd = Mockito.mock(Runnable.class); @@ -524,10 +489,5 @@ public class PhysicsAnimationLayoutTest extends PhysicsAnimationLayoutTestCase { @Override void onActiveControllerForLayout(PhysicsAnimationLayout layout) {} - - @Override - protected void setChildVisibility(View child, int index, int visibility) { - super.setChildVisibility(child, index, visibility); - } } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTestCase.java b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTestCase.java index ec7337bc9622a..f17d7a7da9f3e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTestCase.java +++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/PhysicsAnimationLayoutTestCase.java @@ -56,7 +56,6 @@ public class PhysicsAnimationLayoutTestCase extends SysuiTestCase { Handler mMainThreadHandler; - int mMaxRenderedBubbles; int mSystemWindowInsetSize = 50; int mCutoutInsetSize = 100; @@ -69,6 +68,8 @@ public class PhysicsAnimationLayoutTestCase extends SysuiTestCase { @Mock private DisplayCutout mCutout; + private int mMaxBubbles; + @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); @@ -79,7 +80,7 @@ public class PhysicsAnimationLayoutTestCase extends SysuiTestCase { mLayout.setTop(0); mLayout.setBottom(mHeight); - mMaxRenderedBubbles = + mMaxBubbles = getContext().getResources().getInteger(R.integer.bubbles_max_rendered); mMainThreadHandler = new Handler(Looper.getMainLooper()); @@ -96,8 +97,8 @@ public class PhysicsAnimationLayoutTestCase extends SysuiTestCase { } /** Add one extra bubble over the limit, so we can make sure it's gone/chains appropriately. */ - void addOneMoreThanRenderLimitBubbles() throws InterruptedException { - for (int i = 0; i < mMaxRenderedBubbles + 1; i++) { + void addOneMoreThanBubbleLimitBubbles() throws InterruptedException { + for (int i = 0; i < mMaxBubbles + 1; i++) { final View newView = new FrameLayout(mContext); mLayout.addView(newView, 0); mViews.add(0, newView); @@ -279,11 +280,6 @@ public class PhysicsAnimationLayoutTestCase extends SysuiTestCase { () -> mWrappedController.onActiveControllerForLayout(layout)); } - @Override - protected void setChildVisibility(View child, int index, int visibility) { - mWrappedController.setChildVisibility(child, index, visibility); - } - @Override protected PhysicsPropertyAnimator animationForChild(View child) { PhysicsPropertyAnimator animator = diff --git a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/StackAnimationControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/StackAnimationControllerTest.java index b458907c9df58..31a7d5a45b685 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/StackAnimationControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/bubbles/animation/StackAnimationControllerTest.java @@ -55,7 +55,7 @@ public class StackAnimationControllerTest extends PhysicsAnimationLayoutTestCase public void setUp() throws Exception { super.setUp(); mLayout.setActiveController(mStackController); - addOneMoreThanRenderLimitBubbles(); + addOneMoreThanBubbleLimitBubbles(); mStackOffset = mLayout.getResources().getDimensionPixelSize(R.dimen.bubble_stack_offset); }