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

Commit 8533764c authored by Johannes Gallmann's avatar Johannes Gallmann Committed by Android Build Coastguard Worker
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.
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:21b9fd55f7b50bc40a6fe9b5b6c6cd88d24b4346)
Merged-In: Iead236279ea0f46acde23f16bb230ff324f453f3
Change-Id: Iead236279ea0f46acde23f16bb230ff324f453f3
parent cda995f6

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.common.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

            )