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

Commit 99258dfe authored by Juan Sebastian Martinez's avatar Juan Sebastian Martinez
Browse files

Optimizing the QSLongPressEffect

StateFlows in the QSLongPressEffect need to emit and be collected on the
main thread to avoid thread-switching and preventing jank. Therefore,
the need for a background scope has been removed. In terms of
testability, the concern of setting touch listeners has been delegated
to the QSLongPressEffectViewBinder

Test: SystemUITests:QSTileViewImplTest
Test: SystemUiRobotTests:QSLongPressEffectTest
Bug: 332903800
Flag: ACONFIG quick_settings_visual_haptics_longpress TEAMFOOD
Change-Id: I6efe612c3bfda4ac39525b97c3a7797c61f54b8b
parent aa2c6dd5
Loading
Loading
Loading
Loading
+8 −27
Original line number Diff line number Diff line
@@ -18,9 +18,6 @@ package com.android.systemui.haptics.qs

import android.os.VibrationEffect
import android.testing.TestableLooper.RunWithLooper
import android.view.MotionEvent
import android.view.View
import androidx.test.core.view.MotionEventBuilder
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
@@ -29,18 +26,15 @@ import com.android.systemui.coroutines.collectLastValue
import com.android.systemui.haptics.vibratorHelper
import com.android.systemui.keyguard.data.repository.fakeKeyguardRepository
import com.android.systemui.keyguard.domain.interactor.keyguardInteractor
import com.android.systemui.kosmos.backgroundCoroutineContext
import com.android.systemui.kosmos.testScope
import com.android.systemui.testKosmos
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.junit.MockitoJUnit
import org.mockito.junit.MockitoRule

@@ -50,7 +44,6 @@ import org.mockito.junit.MockitoRule
class QSLongPressEffectTest : SysuiTestCase() {

    @Rule @JvmField val mMockitoRule: MockitoRule = MockitoJUnit.rule()
    @Mock private lateinit var testView: View
    @get:Rule val animatorTestRule = AnimatorTestRule(this)
    private val kosmos = testKosmos()
    private val vibratorHelper = kosmos.vibratorHelper
@@ -73,7 +66,6 @@ class QSLongPressEffectTest : SysuiTestCase() {
            QSLongPressEffect(
                vibratorHelper,
                kosmos.keyguardInteractor,
                CoroutineScope(kosmos.backgroundCoroutineContext),
            )
        longPressEffect.initializeEffect(effectDuration)
    }
@@ -133,8 +125,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
    @Test
    fun onActionDown_whileIdle_startsWait() = testWithScope {
        // GIVEN an action down event occurs
        val downEvent = buildMotionEvent(MotionEvent.ACTION_DOWN)
        longPressEffect.onTouch(testView, downEvent)
        longPressEffect.handleActionDown()

        // THEN the effect moves to the TIMEOUT_WAIT state
        val state by collectLastValue(longPressEffect.state)
@@ -144,8 +135,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
    @Test
    fun onActionCancel_whileWaiting_goesIdle() = testWhileWaiting {
        // GIVEN an action cancel occurs
        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
        longPressEffect.onTouch(testView, cancelEvent)
        longPressEffect.handleActionCancel()

        // THEN the effect goes back to idle and does not start
        val state by collectLastValue(longPressEffect.state)
@@ -159,8 +149,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
        val action by collectLastValue(longPressEffect.actionType)

        // GIVEN an action up occurs
        val upEvent = buildMotionEvent(MotionEvent.ACTION_UP)
        longPressEffect.onTouch(testView, upEvent)
        longPressEffect.handleActionUp()

        // THEN the action to invoke is the click action and the effect does not start
        assertThat(action).isEqualTo(QSLongPressEffect.ActionType.CLICK)
@@ -182,8 +171,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
        animatorTestRule.advanceTimeBy(effectDuration / 2L)

        // WHEN an action up occurs
        val upEvent = buildMotionEvent(MotionEvent.ACTION_UP)
        longPressEffect.onTouch(testView, upEvent)
        longPressEffect.handleActionUp()

        // THEN the effect gets reversed at 50% progress
        assertEffectReverses(0.5f)
@@ -195,8 +183,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
        animatorTestRule.advanceTimeBy(effectDuration / 2L)

        // WHEN an action cancel occurs
        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
        longPressEffect.onTouch(testView, cancelEvent)
        longPressEffect.handleActionCancel()

        // THEN the effect gets reversed at 50% progress
        assertEffectReverses(0.5f)
@@ -230,12 +217,10 @@ class QSLongPressEffectTest : SysuiTestCase() {
        animatorTestRule.advanceTimeBy(effectDuration / 2L)

        // GIVEN an action cancel occurs and the effect gets reversed
        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
        longPressEffect.onTouch(testView, cancelEvent)
        longPressEffect.handleActionCancel()

        // GIVEN an action down occurs
        val downEvent = buildMotionEvent(MotionEvent.ACTION_DOWN)
        longPressEffect.onTouch(testView, downEvent)
        longPressEffect.handleActionDown()

        // THEN the effect resets
        assertEffectResets()
@@ -247,8 +232,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
        animatorTestRule.advanceTimeBy(effectDuration / 2L)

        // GIVEN an action cancel occurs and the effect gets reversed
        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
        longPressEffect.onTouch(testView, cancelEvent)
        longPressEffect.handleActionCancel()

        // GIVEN that the animation completes after a sufficient amount of time
        animatorTestRule.advanceTimeBy(effectDuration.toLong())
@@ -258,9 +242,6 @@ class QSLongPressEffectTest : SysuiTestCase() {
        assertThat(state).isEqualTo(QSLongPressEffect.State.IDLE)
    }

    private fun buildMotionEvent(action: Int): MotionEvent =
        MotionEventBuilder.newBuilder().setAction(action).build()

    private fun testWithScope(test: suspend TestScope.() -> Unit) =
        with(kosmos) { testScope.runTest { test() } }

+17 −41
Original line number Diff line number Diff line
@@ -17,9 +17,7 @@
package com.android.systemui.haptics.qs

import android.animation.ValueAnimator
import android.annotation.SuppressLint
import android.os.VibrationEffect
import android.view.MotionEvent
import android.view.View
import android.view.ViewConfiguration
import android.view.animation.AccelerateDecelerateInterpolator
@@ -27,18 +25,14 @@ import androidx.annotation.VisibleForTesting
import androidx.core.animation.doOnCancel
import androidx.core.animation.doOnEnd
import androidx.core.animation.doOnStart
import com.android.systemui.dagger.qualifiers.Background
import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor
import com.android.systemui.statusbar.VibratorHelper
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn

/**
 * A class that handles the long press visuo-haptic effect for a QS tile.
@@ -55,23 +49,22 @@ class QSLongPressEffect
@Inject
constructor(
    private val vibratorHelper: VibratorHelper?,
    val keyguardInteractor: KeyguardInteractor,
    @Background bgScope: CoroutineScope,
) : View.OnTouchListener {
    keyguardInteractor: KeyguardInteractor,
) {

    private var effectDuration = 0

    /** Current state */
    private var _state = MutableStateFlow(State.IDLE)
    val state = _state.stateIn(bgScope, SharingStarted.Lazily, State.IDLE)
    val state = _state.asStateFlow()

    /** Flows for view control and action */
    private val _effectProgress = MutableStateFlow<Float?>(null)
    val effectProgress = _effectProgress.stateIn(bgScope, SharingStarted.Lazily, null)
    val effectProgress = _effectProgress.asStateFlow()

    // Actions to perform
    private val _postedActionType = MutableStateFlow<ActionType?>(null)
    val actionType: StateFlow<ActionType?> =
    val actionType: Flow<ActionType?> =
        combine(
            _postedActionType,
            keyguardInteractor.isKeyguardDismissible,
@@ -82,7 +75,6 @@ constructor(
                action
            }
        }
            .stateIn(bgScope, SharingStarted.Lazily, null)

    // Should a tap timeout countdown begin
    val shouldWaitForTapTimeout: Flow<Boolean> = state.map { it == State.TIMEOUT_WAIT }
@@ -129,23 +121,7 @@ constructor(
        }
    }

    /**
     * Handle relevant touch events for the operation of a Tile.
     *
     * A click action is performed following the relevant logic that originates from the
     * [MotionEvent.ACTION_UP] event depending on the current state.
     */
    @SuppressLint("ClickableViewAccessibility")
    override fun onTouch(view: View?, event: MotionEvent?): Boolean {
        when (event?.actionMasked) {
            MotionEvent.ACTION_DOWN -> handleActionDown()
            MotionEvent.ACTION_UP -> handleActionUp()
            MotionEvent.ACTION_CANCEL -> handleActionCancel()
        }
        return true
    }

    private fun handleActionDown() {
    fun handleActionDown() {
        when (_state.value) {
            State.IDLE -> {
                setState(State.TIMEOUT_WAIT)
@@ -155,7 +131,7 @@ constructor(
        }
    }

    private fun handleActionUp() {
    fun handleActionUp() {
        when (_state.value) {
            State.TIMEOUT_WAIT -> {
                _postedActionType.value = ActionType.CLICK
@@ -169,7 +145,7 @@ constructor(
        }
    }

    private fun handleActionCancel() {
    fun handleActionCancel() {
        when (_state.value) {
            State.TIMEOUT_WAIT -> {
                setState(State.IDLE)
+21 −6
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

package com.android.systemui.haptics.qs

import android.annotation.SuppressLint
import android.view.MotionEvent
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.repeatOnLifecycle
import com.android.app.tracing.coroutines.launch
@@ -25,10 +27,9 @@ import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.DisposableHandle
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.launch

// TODO(b/332903800)
object QSLongPressEffectViewBinder {

    fun bind(
        tile: QSTileViewImpl,
        qsLongPressEffect: QSLongPressEffect?,
@@ -36,11 +37,13 @@ object QSLongPressEffectViewBinder {
    ): DisposableHandle? {
        if (qsLongPressEffect == null) return null

        // Set the touch listener as the long-press effect
        setTouchListener(tile, qsLongPressEffect)

        return tile.repeatWhenAttached {
            repeatOnLifecycle(Lifecycle.State.CREATED) {
                val tag = "${tileSpec ?: "unknownTileSpec"}#LongPressEffect"
                // Progress of the effect
                launch("$tag#progress") {
                launch({ "${tileSpec ?: "unknownTileSpec"}#LongPressEffect#progress" }) {
                    qsLongPressEffect.effectProgress.collect { progress ->
                        progress?.let {
                            if (it == 0f) {
@@ -53,7 +56,7 @@ object QSLongPressEffectViewBinder {
                }

                // Action to perform
                launch("$tag#action") {
                launch({ "${tileSpec ?: "unknownTileSpec"}#LongPressEffect#action" }) {
                    qsLongPressEffect.actionType.collect { action ->
                        action?.let {
                            when (it) {
@@ -70,7 +73,7 @@ object QSLongPressEffectViewBinder {
                }

                // Tap timeout wait
                launch("$tag#timeout") {
                launch({ "${tileSpec ?: "unknownTileSpec"}#LongPressEffect#timeout" }) {
                    qsLongPressEffect.shouldWaitForTapTimeout
                        .filter { it }
                        .collect {
@@ -85,4 +88,16 @@ object QSLongPressEffectViewBinder {
            }
        }
    }

    @SuppressLint("ClickableViewAccessibility")
    private fun setTouchListener(tile: QSTileViewImpl, longPressEffect: QSLongPressEffect?) {
        tile.setOnTouchListener { _, event ->
            when (event.actionMasked) {
                MotionEvent.ACTION_DOWN -> longPressEffect?.handleActionDown()
                MotionEvent.ACTION_UP -> longPressEffect?.handleActionUp()
                MotionEvent.ACTION_CANCEL -> longPressEffect?.handleActionCancel()
            }
            true
        }
    }
}
+16 −8
Original line number Diff line number Diff line
@@ -185,8 +185,9 @@ open class QSTileViewImpl @JvmOverloads constructor(
    private val colorEvaluator = ArgbEvaluator.getInstance()
    val isLongPressEffectInitialized: Boolean
        get() = longPressEffect?.hasInitialized == true
    @VisibleForTesting
    var longPressEffectHandle: DisposableHandle? = null
    private var longPressEffectHandle: DisposableHandle? = null
    val isLongPressEffectBound: Boolean
        get() = longPressEffectHandle != null

    init {
        val typedValue = TypedValue()
@@ -621,11 +622,14 @@ open class QSTileViewImpl @JvmOverloads constructor(
        // Long-press effects
        if (state.handlesLongClick &&
            longPressEffect?.initializeEffect(longPressEffectDuration) == true) {
            // set the valid long-press effect as the touch listener
            if (longPressEffectHandle == null) {
            // bind the long-press effect and set it as the touch listener
            if (!isLongPressEffectBound) {
                longPressEffectHandle =
                    QSLongPressEffectViewBinder.bind(this, longPressEffect, state.spec)
                setOnTouchListener(longPressEffect)
                    QSLongPressEffectViewBinder.bind(
                        this,
                        longPressEffect,
                        state.spec,
                    )
            }
            showRippleEffect = false
            initializeLongPressProperties()
@@ -634,8 +638,7 @@ open class QSTileViewImpl @JvmOverloads constructor(
            // handle a long-press. In this case, we go back to the behaviour of a regular tile
            // and clean-up the resources
            setOnTouchListener(null)
            longPressEffectHandle?.dispose()
            longPressEffectHandle = null
            unbindLongPressEffect()
            showRippleEffect = isClickable
            initialLongPressProperties = null
            finalLongPressProperties = null
@@ -827,6 +830,11 @@ open class QSTileViewImpl @JvmOverloads constructor(
        changeCornerRadius(newRadius)
    }

    private fun unbindLongPressEffect() {
        longPressEffectHandle?.dispose()
        longPressEffectHandle = null
    }

    private fun interpolateFloat(fraction: Float, start: Float, end: Float): Float =
        start + fraction * (end - start)

+6 −6
Original line number Diff line number Diff line
@@ -385,7 +385,7 @@ class QSTileViewImplTest : SysuiTestCase() {
    }

    @Test
    fun onStateChange_longPressEffectActive_withInvalidDuration_doesNotCreateEffect() {
    fun onStateChange_longPressEffectActive_withInvalidDuration_doesNotInitializeEffect() {
        val state = QSTile.State() // A state that handles longPress

        // GIVEN an invalid long-press effect duration
@@ -399,7 +399,7 @@ class QSTileViewImplTest : SysuiTestCase() {
    }

    @Test
    fun onStateChange_longPressEffectActive_withValidDuration_createsEffect() {
    fun onStateChange_longPressEffectActive_withValidDuration_initializesEffect() {
        // GIVEN a test state that handles long-press and a valid long-press effect duration
        val state = QSTile.State()

@@ -420,7 +420,7 @@ class QSTileViewImplTest : SysuiTestCase() {
        tileView.changeState(state)

        // THEN the view binder no longer binds the view to the long-press effect
        assertThat(tileView.longPressEffectHandle).isNull()
        assertThat(tileView.isLongPressEffectBound).isFalse()
    }

    @Test
@@ -435,7 +435,7 @@ class QSTileViewImplTest : SysuiTestCase() {
        tileView.changeState(state)

        // THEN the view is bounded to the long-press effect
        assertThat(tileView.longPressEffectHandle).isNotNull()
        assertThat(tileView.isLongPressEffectBound).isTrue()
    }

    @Test
@@ -451,7 +451,7 @@ class QSTileViewImplTest : SysuiTestCase() {
        tileView.changeState(state)

        // THEN the view binder does not bind the view and no effect is initialized
        assertThat(tileView.longPressEffectHandle).isNull()
        assertThat(tileView.isLongPressEffectBound).isFalse()
        assertThat(tileView.isLongPressEffectInitialized).isFalse()
    }

@@ -470,7 +470,7 @@ class QSTileViewImplTest : SysuiTestCase() {
        tileView.changeState(state)

        // THEN the view binder does not bind the view and no effect is initialized
        assertThat(tileView.longPressEffectHandle).isNull()
        assertThat(tileView.isLongPressEffectBound).isFalse()
        assertThat(tileView.isLongPressEffectInitialized).isFalse()
    }

Loading