Revisit how launch animations handle View visibility

This CL refactors the way the ActivityLaunchAnimator and
DialogLaunchAnimator handle the visibility of the animated View when
that View implements LaunchableView.

Before this CL, a View that is animated would always end up being
VISIBLE, even if something made that View INVISIBLE or GONE before or
during the animation. That is because our animation code would sometimes
change the visibility of the View, using View.setVisibility.

After this CL, when animating a LaunchableView, all the visibility
changes made to the animated View are done using
View.setTransitionVisibility instead. That way, a LaunchableView can
track all calls to View.setVisibility only and restore the last
visibility that was set on the View at the end of the animation. Doing
so ensures that all calls made to View.setVisibility have the expected
result once the animation is done.

Bug: 264728445
Test: DialogLaunchAnimatorTest
Change-Id: I5848d42ef94952ecf058d4e895164389e37e0311
This commit is contained in:
Jordan Demeulenaere
2023-01-11 15:48:48 +01:00
parent 0e7b6e534c
commit 9ecb3db788
9 changed files with 156 additions and 87 deletions

View File

@@ -791,13 +791,13 @@ private class AnimatedDialog(
// Move the drawing of the source in the overlay of this dialog, then animate. We trigger a
// one-off synchronization to make sure that this is done in sync between the two different
// windows.
controller.startDrawingInOverlayOf(decorView)
synchronizeNextDraw(
then = {
isSourceDrawnInDialog = true
maybeStartLaunchAnimation()
}
)
controller.startDrawingInOverlayOf(decorView)
}
/**

View File

@@ -195,14 +195,16 @@ open class GhostedViewLaunchAnimatorController @JvmOverloads constructor(
backgroundDrawable = WrappedDrawable(background)
backgroundView?.background = backgroundDrawable
// Delay the calls to `ghostedView.setVisibility()` during the animation. This must be
// called before `GhostView.addGhost()` is called because the latter will change the
// *transition* visibility, which won't be blocked and will affect the normal View
// visibility that is saved by `setShouldBlockVisibilityChanges()` for a later restoration.
(ghostedView as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
// Create a ghost of the view that will be moving and fading out. This allows to fade out
// the content before fading out the background.
ghostView = GhostView.addGhost(ghostedView, launchContainer)
// The ghost was just created, so ghostedView is currently invisible. We need to make sure
// that it stays invisible as long as we are animating.
(ghostedView as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
val matrix = ghostView?.animationMatrix ?: Matrix.IDENTITY_MATRIX
matrix.getValues(initialGhostViewMatrixValues)
@@ -297,14 +299,19 @@ open class GhostedViewLaunchAnimatorController @JvmOverloads constructor(
backgroundDrawable?.wrapped?.alpha = startBackgroundAlpha
GhostView.removeGhost(ghostedView)
(ghostedView as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
launchContainerOverlay.remove(backgroundView)
// Make sure that the view is considered VISIBLE by accessibility by first making it
// INVISIBLE then VISIBLE (see b/204944038#comment17 for more info).
ghostedView.visibility = View.INVISIBLE
ghostedView.visibility = View.VISIBLE
ghostedView.invalidate()
if (ghostedView is LaunchableView) {
// Restore the ghosted view visibility.
ghostedView.setShouldBlockVisibilityChanges(false)
} else {
// Make the ghosted view visible. We ensure that the view is considered VISIBLE by
// accessibility by first making it INVISIBLE then VISIBLE (see b/204944038#comment17
// for more info).
ghostedView.visibility = View.INVISIBLE
ghostedView.visibility = View.VISIBLE
ghostedView.invalidate()
}
}
companion object {

View File

@@ -21,15 +21,19 @@ import android.view.View
/** A view that can expand/launch into an app or a dialog. */
interface LaunchableView {
/**
* Set whether this view should block/postpone all visibility changes. This ensures that this
* view:
* Set whether this view should block/postpone all calls to [View.setVisibility]. This ensures
* that this view:
* - remains invisible during the launch animation given that it is ghosted and already drawn
* somewhere else.
* - remains invisible as long as a dialog expanded from it is shown.
* - restores its expected visibility once the dialog expanded from it is dismissed.
*
* Note that when this is set to true, both the [normal][android.view.View.setVisibility] and
* [transition][android.view.View.setTransitionVisibility] visibility changes must be blocked.
* When `setShouldBlockVisibilityChanges(false)` is called, then visibility of the View should
* be restored to its expected value, i.e. it should have the visibility of the last call to
* `View.setVisibility()` that was made after `setShouldBlockVisibilityChanges(true)`, if any,
* or the original view visibility otherwise.
*
* Note that calls to [View.setTransitionVisibility] shouldn't be blocked.
*
* @param block whether we should block/postpone all calls to `setVisibility` and
* `setTransitionVisibility`.
@@ -46,27 +50,31 @@ class LaunchableViewDelegate(
* super.setVisibility(visibility).
*/
private val superSetVisibility: (Int) -> Unit,
/**
* The lambda that should set the actual transition visibility of [view], usually by calling
* super.setTransitionVisibility(visibility).
*/
private val superSetTransitionVisibility: (Int) -> Unit,
) {
) : LaunchableView {
private var blockVisibilityChanges = false
private var lastVisibility = view.visibility
/** Call this when [LaunchableView.setShouldBlockVisibilityChanges] is called. */
fun setShouldBlockVisibilityChanges(block: Boolean) {
override fun setShouldBlockVisibilityChanges(block: Boolean) {
if (block == blockVisibilityChanges) {
return
}
blockVisibilityChanges = block
if (block) {
// Save the current visibility for later.
lastVisibility = view.visibility
} else {
superSetVisibility(lastVisibility)
// Restore the visibility. To avoid accessibility issues, we change the visibility twice
// which makes sure that we trigger a visibility flag change (see b/204944038#comment17
// for more info).
if (lastVisibility == View.VISIBLE) {
superSetVisibility(View.INVISIBLE)
superSetVisibility(View.VISIBLE)
} else {
superSetVisibility(View.VISIBLE)
superSetVisibility(lastVisibility)
}
}
}
@@ -79,16 +87,4 @@ class LaunchableViewDelegate(
superSetVisibility(visibility)
}
/** Call this when [View.setTransitionVisibility] is called. */
fun setTransitionVisibility(visibility: Int) {
if (blockVisibilityChanges) {
// View.setTransitionVisibility just sets the visibility flag, so we don't have to save
// the transition visibility separately from the normal visibility.
lastVisibility = visibility
return
}
superSetTransitionVisibility(visibility)
}
}

View File

@@ -34,23 +34,29 @@ internal constructor(
override val sourceIdentity: Any = source
override fun startDrawingInOverlayOf(viewGroup: ViewGroup) {
// Delay the calls to `source.setVisibility()` during the animation. This must be called
// before `GhostView.addGhost()` is called because the latter will change the *transition*
// visibility, which won't be blocked and will affect the normal View visibility that is
// saved by `setShouldBlockVisibilityChanges()` for a later restoration.
(source as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
// Create a temporary ghost of the source (which will make it invisible) and add it
// to the host dialog.
GhostView.addGhost(source, viewGroup)
// The ghost of the source was just created, so the source is currently invisible.
// We need to make sure that it stays invisible as long as the dialog is shown or
// animating.
(source as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
}
override fun stopDrawingInOverlay() {
// Note: here we should remove the ghost from the overlay, but in practice this is
// already done by the launch controllers created below.
// already done by the launch controller created below.
// Make sure we allow the source to change its visibility again.
(source as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
source.visibility = View.VISIBLE
if (source is LaunchableView) {
// Make sure we allow the source to change its visibility again and restore its previous
// value.
source.setShouldBlockVisibilityChanges(false)
} else {
// We made the source invisible earlier, so let's make it visible again.
source.visibility = View.VISIBLE
}
}
override fun createLaunchController(): LaunchAnimator.Controller {
@@ -67,10 +73,14 @@ internal constructor(
override fun onLaunchAnimationEnd(isExpandingFullyAbove: Boolean) {
delegate.onLaunchAnimationEnd(isExpandingFullyAbove)
// We hide the source when the dialog is showing. We will make this view
// visible again when dismissing the dialog. This does nothing if the source
// implements [LaunchableView], as it's already INVISIBLE in that case.
source.visibility = View.INVISIBLE
// At this point the view visibility is restored by the delegate, so we delay the
// visibility changes again and make it invisible while the dialog is shown.
if (source is LaunchableView) {
source.setShouldBlockVisibilityChanges(true)
source.setTransitionVisibility(View.INVISIBLE)
} else {
source.visibility = View.INVISIBLE
}
}
}
}
@@ -90,13 +100,15 @@ internal constructor(
}
override fun onExitAnimationCancelled() {
// Make sure we allow the source to change its visibility again.
(source as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
// If the view is invisible it's probably because of us, so we make it visible
// again.
if (source.visibility == View.INVISIBLE) {
source.visibility = View.VISIBLE
if (source is LaunchableView) {
// Make sure we allow the source to change its visibility again.
source.setShouldBlockVisibilityChanges(false)
} else {
// If the view is invisible it's probably because of us, so we make it visible
// again.
if (source.visibility == View.INVISIBLE) {
source.visibility = View.VISIBLE
}
}
}

View File

@@ -28,7 +28,6 @@ class LaunchableImageView : ImageView, LaunchableView {
LaunchableViewDelegate(
this,
superSetVisibility = { super.setVisibility(it) },
superSetTransitionVisibility = { super.setTransitionVisibility(it) },
)
constructor(context: Context?) : super(context)
@@ -53,8 +52,4 @@ class LaunchableImageView : ImageView, LaunchableView {
override fun setVisibility(visibility: Int) {
delegate.setVisibility(visibility)
}
override fun setTransitionVisibility(visibility: Int) {
delegate.setTransitionVisibility(visibility)
}
}

View File

@@ -28,7 +28,6 @@ class LaunchableLinearLayout : LinearLayout, LaunchableView {
LaunchableViewDelegate(
this,
superSetVisibility = { super.setVisibility(it) },
superSetTransitionVisibility = { super.setTransitionVisibility(it) },
)
constructor(context: Context?) : super(context)
@@ -53,8 +52,4 @@ class LaunchableLinearLayout : LinearLayout, LaunchableView {
override fun setVisibility(visibility: Int) {
delegate.setVisibility(visibility)
}
override fun setTransitionVisibility(visibility: Int) {
delegate.setTransitionVisibility(visibility)
}
}

View File

@@ -145,7 +145,6 @@ open class QSTileViewImpl @JvmOverloads constructor(
private val launchableViewDelegate = LaunchableViewDelegate(
this,
superSetVisibility = { super.setVisibility(it) },
superSetTransitionVisibility = { super.setTransitionVisibility(it) },
)
private var lastDisabledByPolicy = false
@@ -362,10 +361,6 @@ open class QSTileViewImpl @JvmOverloads constructor(
launchableViewDelegate.setVisibility(visibility)
}
override fun setTransitionVisibility(visibility: Int) {
launchableViewDelegate.setTransitionVisibility(visibility)
}
// Accessibility
override fun onInitializeAccessibilityEvent(event: AccessibilityEvent) {

View File

@@ -36,10 +36,6 @@ public class AlphaOptimizedFrameLayout extends FrameLayout implements Launchable
visibility -> {
super.setVisibility(visibility);
return Unit.INSTANCE;
},
visibility -> {
super.setTransitionVisibility(visibility);
return Unit.INSTANCE;
});
public AlphaOptimizedFrameLayout(Context context) {
@@ -73,9 +69,4 @@ public class AlphaOptimizedFrameLayout extends FrameLayout implements Launchable
public void setVisibility(int visibility) {
mLaunchableViewDelegate.setVisibility(visibility);
}
@Override
public void setTransitionVisibility(int visibility) {
mLaunchableViewDelegate.setTransitionVisibility(visibility);
}
}

View File

@@ -12,11 +12,13 @@ import android.view.View
import android.view.ViewGroup
import android.view.ViewGroup.LayoutParams.MATCH_PARENT
import android.view.WindowManager
import android.widget.FrameLayout
import android.widget.LinearLayout
import androidx.test.filters.SmallTest
import com.android.internal.jank.InteractionJankMonitor
import com.android.internal.policy.DecorView
import com.android.systemui.SysuiTestCase
import com.google.common.truth.Truth.assertThat
import junit.framework.Assert.assertEquals
import junit.framework.Assert.assertFalse
import junit.framework.Assert.assertNotNull
@@ -205,25 +207,74 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
verify(interactionJankMonitor).end(InteractionJankMonitor.CUJ_USER_DIALOG_OPEN)
}
@Test
fun testAnimationDoesNotChangeLaunchableViewVisibility_viewVisible() {
val touchSurface = createTouchSurface()
// View is VISIBLE when starting the animation.
runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.VISIBLE }
// View is invisible while the dialog is shown.
val dialog = showDialogFromView(touchSurface)
assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
// View is visible again when the dialog is dismissed.
runOnMainThreadAndWaitForIdleSync { dialog.dismiss() }
assertThat(touchSurface.visibility).isEqualTo(View.VISIBLE)
}
@Test
fun testAnimationDoesNotChangeLaunchableViewVisibility_viewInvisible() {
val touchSurface = createTouchSurface()
// View is INVISIBLE when starting the animation.
runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.INVISIBLE }
// View is INVISIBLE while the dialog is shown.
val dialog = showDialogFromView(touchSurface)
assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
// View is invisible like it was before showing the dialog.
runOnMainThreadAndWaitForIdleSync { dialog.dismiss() }
assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
}
@Test
fun testAnimationDoesNotChangeLaunchableViewVisibility_viewVisibleThenGone() {
val touchSurface = createTouchSurface()
// View is VISIBLE when starting the animation.
runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.VISIBLE }
// View is INVISIBLE while the dialog is shown.
val dialog = showDialogFromView(touchSurface)
assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
// Some external call makes the View GONE. It remains INVISIBLE while the dialog is shown,
// as all visibility changes should be blocked.
runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.GONE }
assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
// View is restored to GONE once the dialog is dismissed.
runOnMainThreadAndWaitForIdleSync { dialog.dismiss() }
assertThat(touchSurface.visibility).isEqualTo(View.GONE)
}
private fun createAndShowDialog(
animator: DialogLaunchAnimator = dialogLaunchAnimator,
): TestDialog {
val touchSurface = createTouchSurface()
return runOnMainThreadAndWaitForIdleSync {
val dialog = TestDialog(context)
animator.showFromView(dialog, touchSurface)
dialog
}
return showDialogFromView(touchSurface, animator)
}
private fun createTouchSurface(): View {
return runOnMainThreadAndWaitForIdleSync {
val touchSurfaceRoot = LinearLayout(context)
val touchSurface = View(context)
val touchSurface = TouchSurfaceView(context)
touchSurfaceRoot.addView(touchSurface)
// We need to attach the root to the window manager otherwise the exit animation will
// be skipped
// be skipped.
ViewUtils.attachView(touchSurfaceRoot)
attachedViews.add(touchSurfaceRoot)
@@ -231,6 +282,17 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
}
}
private fun showDialogFromView(
touchSurface: View,
animator: DialogLaunchAnimator = dialogLaunchAnimator,
): TestDialog {
return runOnMainThreadAndWaitForIdleSync {
val dialog = TestDialog(context)
animator.showFromView(dialog, touchSurface)
dialog
}
}
private fun createDialogAndShowFromDialog(animateFrom: Dialog): TestDialog {
return runOnMainThreadAndWaitForIdleSync {
val dialog = TestDialog(context)
@@ -248,6 +310,22 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
return result
}
private class TouchSurfaceView(context: Context) : FrameLayout(context), LaunchableView {
private val delegate =
LaunchableViewDelegate(
this,
superSetVisibility = { super.setVisibility(it) },
)
override fun setShouldBlockVisibilityChanges(block: Boolean) {
delegate.setShouldBlockVisibilityChanges(block)
}
override fun setVisibility(visibility: Int) {
delegate.setVisibility(visibility)
}
}
private class TestDialog(context: Context) : Dialog(context) {
companion object {
const val DIALOG_WIDTH = 100