From ec7af7f61cb0dc05002777807b7d40ad6b867e04 Mon Sep 17 00:00:00 2001 From: Luca Zuccarini Date: Tue, 5 Jul 2022 15:33:45 +0000 Subject: [PATCH] Include INVISIBLE views in animations by the ViewHierarchyAnimator. If we exclude them, they behave incorrectly as they fade in and out while moving. They still take up space on screen while invisible, so they should be animated along with the rest of the views. Bug: 234337113 Test: included Change-Id: Ifa31bd0b526a504b8f60ff8893d3d1caa673eb90 --- .../animation/ViewHierarchyAnimator.kt | 16 +++---- .../animation/ViewHierarchyAnimatorTest.kt | 44 +++++++++---------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt index 4b0c62bff60d6..cc7d23e97d0c3 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt @@ -106,7 +106,7 @@ class ViewHierarchyAnimator { ephemeral: Boolean ): Boolean { if ( - !isVisible( + !occupiesSpace( rootView.visibility, rootView.left, rootView.top, @@ -177,7 +177,7 @@ class ViewHierarchyAnimator { fadeInInterpolator: Interpolator = DEFAULT_FADE_IN_INTERPOLATOR ): Boolean { if ( - isVisible( + occupiesSpace( rootView.visibility, rootView.left, rootView.top, @@ -295,7 +295,7 @@ class ViewHierarchyAnimator { (view.getTag(R.id.tag_animator) as? ObjectAnimator)?.cancel() - if (!isVisible(view.visibility, left, top, right, bottom)) { + if (!occupiesSpace(view.visibility, left, top, right, bottom)) { setBound(view, Bound.LEFT, left) setBound(view, Bound.TOP, top) setBound(view, Bound.RIGHT, right) @@ -362,7 +362,7 @@ class ViewHierarchyAnimator { duration: Long = DEFAULT_DURATION ): Boolean { if ( - !isVisible( + !occupiesSpace( rootView.visibility, rootView.left, rootView.top, @@ -530,17 +530,17 @@ class ViewHierarchyAnimator { } /** - * Returns whether the given [visibility] and bounds are consistent with a view being - * currently visible on screen. + * Returns whether the given [visibility] and bounds are consistent with a view being a + * contributing part of the hierarchy. */ - private fun isVisible( + private fun occupiesSpace( visibility: Int, left: Int, top: Int, right: Int, bottom: Int ): Boolean { - return visibility == View.VISIBLE && left != right && top != bottom + return visibility != View.GONE && left != right && top != bottom } /** diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/ViewHierarchyAnimatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/ViewHierarchyAnimatorTest.kt index b61fbbe1ea75d..273786d747827 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/animation/ViewHierarchyAnimatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/animation/ViewHierarchyAnimatorTest.kt @@ -206,6 +206,25 @@ ViewHierarchyAnimatorTest : SysuiTestCase() { checkBounds(rootView.getChildAt(1), l = 95, t = 0, r = 190, b = 100) } + @Test + fun animatesInvisibleViews() { + rootView.layout(10 /* l */, 10 /* t */, 50 /* r */, 50 /* b */) + rootView.visibility = View.INVISIBLE + + val success = ViewHierarchyAnimator.animate(rootView) + // Change all bounds. + rootView.layout(0 /* l */, 15 /* t */, 70 /* r */, 80 /* b */) + + assertTrue(success) + assertNotNull(rootView.getTag(R.id.tag_animator)) + // The initial values should be those of the previous layout. + checkBounds(rootView, l = 10, t = 10, r = 50, b = 50) + endAnimation(rootView) + assertNull(rootView.getTag(R.id.tag_animator)) + // The end values should be those of the latest layout. + checkBounds(rootView, l = 0, t = 15, r = 70, b = 80) + } + @Test fun animatesAppearingViewsFromStartToEnd() { // Starting GONE. @@ -222,20 +241,6 @@ ViewHierarchyAnimatorTest : SysuiTestCase() { assertNull(rootView.getTag(R.id.tag_animator)) checkBounds(rootView, l = 0, t = 100, r = 100, b = 200) - // Starting INVISIBLE. - rootView.visibility = View.INVISIBLE - rootView.layout(0 /* l */, 50 /* t */, 50 /* r */, 100 /* b */) - success = ViewHierarchyAnimator.animateAddition(rootView) - rootView.visibility = View.VISIBLE - rootView.layout(0 /* l */, 100 /* t */, 100 /* r */, 200 /* b */) - - assertTrue(success) - assertNotNull(rootView.getTag(R.id.tag_animator)) - checkBounds(rootView, l = 50, t = 150, r = 50, b = 150) - endAnimation(rootView) - assertNull(rootView.getTag(R.id.tag_animator)) - checkBounds(rootView, l = 0, t = 100, r = 100, b = 200) - // Starting with nothing. rootView.layout(0 /* l */, 0 /* t */, 0 /* r */, 0 /* b */) success = ViewHierarchyAnimator.animateAddition(rootView) @@ -937,7 +942,7 @@ ViewHierarchyAnimatorTest : SysuiTestCase() { } @Test - fun doesNotAnimateInvisibleViews() { + fun doesNotAnimateGoneViews() { rootView.layout(10 /* l */, 10 /* t */, 50 /* r */, 50 /* b */) // GONE @@ -948,15 +953,6 @@ ViewHierarchyAnimatorTest : SysuiTestCase() { assertFalse(success) assertNull(rootView.getTag(R.id.tag_animator)) checkBounds(rootView, l = 0, t = 15, r = 55, b = 80) - - // INVISIBLE. - rootView.visibility = View.INVISIBLE - success = ViewHierarchyAnimator.animate(rootView) - rootView.layout(0 /* l */, 20 /* t */, 10 /* r */, 50 /* b */) - - assertFalse(success) - assertNull(rootView.getTag(R.id.tag_animator)) - checkBounds(rootView, l = 0, t = 20, r = 10, b = 50) } @Test