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
This commit is contained in:
Joshua Tsuji
2019-05-29 16:20:41 -04:00
parent c36ee6f4ff
commit cb97a11bea
7 changed files with 26 additions and 161 deletions

View File

@@ -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);

View File

@@ -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;

View File

@@ -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)) {

View File

@@ -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;

View File

@@ -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);
}
}
}

View File

@@ -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 =

View File

@@ -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);
}