Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cc0a34b5 authored by Ibrahim Yilmaz's avatar Ibrahim Yilmaz
Browse files

Use immutable shouldBeAnimated during Unfold Animation 

Context:
We decide to run animations by using shouldBeAnimated lambda function. It represents mutable state which is combination of the
StatusBar and some external business logic.

It shows that shouldBeAnimated value may change for some reasons after the animation is fired.
This may cause some visual issues like wrong translationX values in the view(animation started but not completed properly).

Solution:
We make shouldBeAnimated immutable variable instead of lambda func in ViewToTranslate and use it during the animation.
We already map `ViewIdToTranslate` to `ViewToTranslate` on each animation start.  It also verifies that we invalidate the preserved value on each animation start.

Test: atest UnfoldConstantTranslateAnimatorTest
Bug: 264454435

Change-Id: I75f84a668ab3395193901cff569ca56e55456316
parent 99dc404b

packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt

+19 −14
Original line number Diff line number Diff line
@@ -65,35 +65,40 @@ class UnfoldConstantTranslateAnimator( 
            } else {

                1

            }

        viewsToTranslate.forEach { (view, direction, shouldBeAnimated) ->

            if (shouldBeAnimated()) {

        viewsToTranslate.forEach { (view, direction) ->

            view.get()?.translationX = xTrans * direction.multiplier * rtlMultiplier

        }

    }

    }



    /** Finds in [parent] all views specified by [ids] and register them for the animation. */

    private fun registerViewsForAnimation(parent: ViewGroup, ids: Set<ViewIdToTranslate>) {

        viewsToTranslate =

            ids.mapNotNull { (id, dir, pred) ->

                parent.findViewById<View>(id)?.let { view ->

                    ViewToTranslate(WeakReference(view), dir, pred)

            ids.asSequence()

                .filter { it.shouldBeAnimated() }

                .mapNotNull {

                    parent.findViewById<View>(it.viewId)?.let { view ->

                        ViewToTranslate(WeakReference(view), it.direction)

                    }

                }

                .toList()

    }



    /** Represents a view to animate. [rootView] should contain a view with [viewId] inside. */

    /**

     * Represents a view to animate. [rootView] should contain a view with [viewId] inside.

     * [shouldBeAnimated] is only evaluated when the viewsToTranslate is registered in

     * [registerViewsForAnimation].

     */

    data class ViewIdToTranslate(

        val viewId: Int,

        val direction: Direction,

        val shouldBeAnimated: () -> Boolean = { true }

    )



    private data class ViewToTranslate(

        val view: WeakReference<View>,

        val direction: Direction,

        val shouldBeAnimated: () -> Boolean

    )

    /**

     * Represents a view whose animation process is in-progress. It should be immutable because the

     * started animation should be completed.

     */

    private data class ViewToTranslate(val view: WeakReference<View>, val direction: Direction)



    /** Direction of the animation. */

    enum class Direction(val multiplier: Float) {

packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java

+3 −3
Original line number Diff line number Diff line
@@ -2211,10 +2211,10 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { 
            pw.println("Current Status Bar state:");

            pw.println("  mExpandedVisible=" + mShadeController.isExpandedVisible());

            pw.println("  mDisplayMetrics=" + mDisplayMetrics);

            pw.println("  mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller));

            pw.println("  mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller)

                    + " scroll " + mStackScroller.getScrollX()

            pw.print("  mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller));

            pw.print(" scroll " + mStackScroller.getScrollX()

                    + "," + mStackScroller.getScrollY());

            pw.println(" translationX " + mStackScroller.getTranslationX());

        }



        pw.print("  mInteractingWindows="); pw.println(mInteractingWindows);

packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt

+27 −8
Original line number Diff line number Diff line
@@ -36,21 +36,26 @@ class UnfoldConstantTranslateAnimatorTest : SysuiTestCase() { 


    private val progressProvider = TestUnfoldTransitionProvider()



    @Mock private lateinit var parent: ViewGroup

    @Mock

    private lateinit var parent: ViewGroup



    @Mock

    private lateinit var shouldBeAnimated: () -> Boolean



    private lateinit var animator: UnfoldConstantTranslateAnimator



    private val viewsIdToRegister =

    private val viewsIdToRegister

        get() =

            setOf(

            ViewIdToTranslate(START_VIEW_ID, Direction.START),

            ViewIdToTranslate(END_VIEW_ID, Direction.END))

                    ViewIdToTranslate(START_VIEW_ID, Direction.START, shouldBeAnimated),

                    ViewIdToTranslate(END_VIEW_ID, Direction.END, shouldBeAnimated)

            )



    @Before

    fun setup() {

        MockitoAnnotations.initMocks(this)



        animator =

            UnfoldConstantTranslateAnimator(viewsIdToRegister, progressProvider)

        whenever(shouldBeAnimated.invoke()).thenReturn(true)

        animator = UnfoldConstantTranslateAnimator(viewsIdToRegister, progressProvider)



        animator.init(parent, MAX_TRANSLATION)

    }
@@ -96,6 +101,20 @@ class UnfoldConstantTranslateAnimatorTest : SysuiTestCase() { 
        moveAndValidate(listOf(leftView to START, rightView to END), View.LAYOUT_DIRECTION_LTR)

    }



    @Test

    fun onTransition_completeStartedTranslation() {

        // GIVEN

        val leftView = View(context)

        whenever(parent.findViewById<View>(START_VIEW_ID)).thenReturn(leftView)

        // To start animation, shouldBeAnimated should return true.

        // There is a possibility for shouldBeAnimated to return false during the animation.

        whenever(shouldBeAnimated.invoke()).thenReturn(true).thenReturn(false)



        // shouldBeAnimated state may change during the animation.

        // However, started animation should be completed.

        moveAndValidate(listOf(leftView to START), View.LAYOUT_DIRECTION_LTR)

    }



    private fun moveAndValidate(list: List<Pair<View, Int>>, layoutDirection: Int) {

        // Compare values as ints because -0f != 0f