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

Commit 21b9fd55 authored by Johannes Gallmann's avatar Johannes Gallmann
Browse files

Fix wrong Choreographer used for setting vsyncId in cross-activity predictive back 

The constructor of CrossActivityBackAnimation is not called from the wmshell.main thread. Therefore we should not set the Choreographer from the constructor.

Bug: 343039961
Flag: com.android.window.flags.predictive_back_system_anims
Test: Manual, i.e. logging Choreographer instances and veryfying that they're accessed from wmshell.main thread. Also verified in perfetto that traces look smooth now.
Change-Id: Iead236279ea0f46acde23f16bb230ff324f453f3
parent 1d1a9c34

libs/WindowManager/Shell/src/com/android/wm/shell/back/CrossActivityBackAnimation.kt

+5 −4
Original line number Diff line number Diff line
@@ -61,8 +61,7 @@ abstract class CrossActivityBackAnimation( 
    private val context: Context,

    private val background: BackAnimationBackground,

    private val rootTaskDisplayAreaOrganizer: RootTaskDisplayAreaOrganizer,

    protected val transaction: SurfaceControl.Transaction,

    private val choreographer: Choreographer

    protected val transaction: SurfaceControl.Transaction

) : ShellBackAnimation() {



    protected val startClosingRect = RectF()
@@ -269,7 +268,9 @@ abstract class CrossActivityBackAnimation( 
            .setSpring(postCommitFlingSpring)

        flingAnimation.start()

        // do an animation-frame immediately to prevent idle frame

        flingAnimation.doAnimationFrame(choreographer.lastFrameTimeNanos / TimeUtils.NANOS_PER_MS)

        flingAnimation.doAnimationFrame(

            Choreographer.getInstance().lastFrameTimeNanos / TimeUtils.NANOS_PER_MS

        )



        val valueAnimator =

            ValueAnimator.ofFloat(1f, 0f).setDuration(getPostCommitAnimationDuration())
@@ -362,7 +363,7 @@ abstract class CrossActivityBackAnimation( 
    }



    protected fun applyTransaction() {

        transaction.setFrameTimelineVsync(choreographer.vsyncId)

        transaction.setFrameTimelineVsync(Choreographer.getInstance().vsyncId)

        transaction.apply()

    }

libs/WindowManager/Shell/src/com/android/wm/shell/back/CustomCrossActivityBackAnimation.kt

+1 −7
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ import android.content.Context 
import android.graphics.Rect

import android.graphics.RectF

import android.util.MathUtils

import android.view.Choreographer

import android.view.SurfaceControl

import android.view.animation.Animation

import android.view.animation.Transformation
@@ -31,27 +30,23 @@ import com.android.internal.policy.TransitionAnimation 
import com.android.internal.protolog.ProtoLog

import com.android.wm.shell.RootTaskDisplayAreaOrganizer

import com.android.wm.shell.protolog.ShellProtoLogGroup

import com.android.wm.shell.shared.annotations.ShellMainThread

import javax.inject.Inject

import kotlin.math.max

import kotlin.math.min



/** Class that handles customized predictive cross activity back animations. */

@ShellMainThread

class CustomCrossActivityBackAnimation(

    context: Context,

    background: BackAnimationBackground,

    rootTaskDisplayAreaOrganizer: RootTaskDisplayAreaOrganizer,

    transaction: SurfaceControl.Transaction,

    choreographer: Choreographer,

    private val customAnimationLoader: CustomAnimationLoader

) :

    CrossActivityBackAnimation(

        context,

        background,

        rootTaskDisplayAreaOrganizer,

        transaction,

        choreographer

        transaction

    ) {



    private var enterAnimation: Animation? = null
@@ -70,7 +65,6 @@ class CustomCrossActivityBackAnimation( 
        background,

        rootTaskDisplayAreaOrganizer,

        SurfaceControl.Transaction(),

        Choreographer.getInstance(),

        CustomAnimationLoader(

            TransitionAnimation(context, false /* debug */, "CustomCrossActivityBackAnimation")

        )

libs/WindowManager/Shell/src/com/android/wm/shell/back/DefaultCrossActivityBackAnimation.kt

+1 −5
Original line number Diff line number Diff line
@@ -16,18 +16,15 @@ 
package com.android.wm.shell.back



import android.content.Context

import android.view.Choreographer

import android.view.SurfaceControl

import android.window.BackEvent

import com.android.wm.shell.R

import com.android.wm.shell.RootTaskDisplayAreaOrganizer

import com.android.wm.shell.animation.Interpolators

import com.android.wm.shell.shared.annotations.ShellMainThread

import javax.inject.Inject

import kotlin.math.max



/** Class that defines cross-activity animation. */

@ShellMainThread

class DefaultCrossActivityBackAnimation

@Inject

constructor(
@@ -39,8 +36,7 @@ constructor( 
        context,

        background,

        rootTaskDisplayAreaOrganizer,

        SurfaceControl.Transaction(),

        Choreographer.getInstance()

        SurfaceControl.Transaction()

    ) {



    private val postCommitInterpolator = Interpolators.EMPHASIZED

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/CustomCrossActivityBackAnimationTest.kt

+2 −5
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ import android.graphics.Rect 
import android.os.RemoteException

import android.testing.AndroidTestingRunner

import android.testing.TestableLooper

import android.view.Choreographer

import android.view.RemoteAnimationTarget

import android.view.SurfaceControl

import android.view.SurfaceControl.Transaction
@@ -37,8 +36,6 @@ import androidx.test.filters.SmallTest 
import com.android.internal.policy.TransitionAnimation

import com.android.wm.shell.RootTaskDisplayAreaOrganizer

import com.android.wm.shell.ShellTestCase

import java.util.concurrent.CountDownLatch

import java.util.concurrent.TimeUnit

import junit.framework.TestCase.assertEquals

import org.junit.Assert

import org.junit.Before
@@ -50,12 +47,13 @@ import org.mockito.ArgumentMatchers.anyFloat 
import org.mockito.ArgumentMatchers.anyInt

import org.mockito.ArgumentMatchers.eq

import org.mockito.Mock

import org.mockito.Mockito.mock

import org.mockito.Mockito.never

import org.mockito.Mockito.times

import org.mockito.kotlin.spy

import org.mockito.kotlin.verify

import org.mockito.kotlin.whenever

import java.util.concurrent.CountDownLatch

import java.util.concurrent.TimeUnit



@SmallTest

@TestableLooper.RunWithLooper
@@ -82,7 +80,6 @@ class CustomCrossActivityBackAnimationTest : ShellTestCase() { 
                backAnimationBackground,

                rootTaskDisplayAreaOrganizer,

                transaction,

                mock(Choreographer::class.java),

                customAnimationLoader

            )