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

Commit cfed47a6 authored by Jorge Gil's avatar Jorge Gil
Browse files

Skip return-to-start drag animation if already at end

The return-to-start drag animation is meant for when a task is dragged
away from its start position and dropped in an "invalid" area, such as
partially off-screen. However, it's possible that the task is dropped
exactly where it started being dragged, in which case running this
animation shouldn't be necessary.

Also, this is especially problematic at times when a simple click using
a mouse generates ACTION_DOWN->ACTION_MOVE->ACTION_UP without actually
changing position, which was triggering this repositioning animation and
if the click was on the close (X) button, the task surface could become
invalid while the animation was still running, leading to a crash.

This change:
1) Checks the "drop" bounds (i.e. current bounds on the |end| callback)
   aren't the same as the destination bounds and skips the animation if
   they are.
2) Within the animator itself, skips the animation if start/end bounds
   are the same.
3) Checks surfaces are still valid before using them.

Flag: EXEMPT refactor
Fix: 440380144
Test: atest DesktopTasksControllerTest
Change-Id: Iff344ad667bc5490a7bdbb544ac8cdaaea8d263f
parent afaaf340
Loading
Loading
Loading
Loading
+9 −6
Original line number Original line Diff line number Diff line
@@ -1024,9 +1024,8 @@ class DesktopTasksController(
        val excludedTasks =
        val excludedTasks =
            getFocusedNonDesktopTasks(DEFAULT_DISPLAY, userId).map { task -> task.taskId }
            getFocusedNonDesktopTasks(DEFAULT_DISPLAY, userId).map { task -> task.taskId }
        // Preserve focus state on reconnect, regardless if focused task is restored or not.
        // Preserve focus state on reconnect, regardless if focused task is restored or not.
        val globallyFocusedTask = shellTaskOrganizer.getRunningTaskInfo(
        val globallyFocusedTask =
            focusTransitionObserver.globallyFocusedTaskId
            shellTaskOrganizer.getRunningTaskInfo(focusTransitionObserver.globallyFocusedTaskId)
        )
        mainScope.launch {
        mainScope.launch {
            preservedTaskIdsByDeskId.forEach { (preservedDeskId, preservedTaskIds) ->
            preservedTaskIdsByDeskId.forEach { (preservedDeskId, preservedTaskIds) ->
                val newDeskId =
                val newDeskId =
@@ -5601,10 +5600,14 @@ class DesktopTasksController(
                    validDragArea,
                    validDragArea,
                )
                )


                if (destinationBounds == dragStartBounds) {
                if (
                    destinationBounds == dragStartBounds && destinationBounds != currentDragBounds
                ) {
                    // There's no actual difference between the start and end bounds, so while a
                    // There's no actual difference between the start and end bounds, so while a
                    // WCT change isn't needed, the dragged surface still needs to be snapped back
                    // WCT change isn't needed, the dragged surface still needs to be snapped back
                    // to its original location.
                    // to its original location. This is as long as it moved some in the first
                    // place, if it didn't and |currentDragBounds| is already at destination then
                    // there's no need to animate.
                    releaseVisualIndicator()
                    releaseVisualIndicator()
                    returnToDragStartAnimator.start(
                    returnToDragStartAnimator.start(
                        taskInfo.taskId,
                        taskInfo.taskId,
+51 −23
Original line number Original line Diff line number Diff line
@@ -24,6 +24,8 @@ import android.view.SurfaceControl
import androidx.core.animation.addListener
import androidx.core.animation.addListener
import com.android.internal.jank.Cuj
import com.android.internal.jank.Cuj
import com.android.internal.jank.InteractionJankMonitor
import com.android.internal.jank.InteractionJankMonitor
import com.android.internal.protolog.ProtoLog
import com.android.wm.shell.protolog.ShellProtoLogGroup
import com.android.wm.shell.windowdecor.OnTaskRepositionAnimationListener
import com.android.wm.shell.windowdecor.OnTaskRepositionAnimationListener
import java.util.function.Supplier
import java.util.function.Supplier


@@ -52,6 +54,15 @@ class ReturnToDragStartAnimator(
        endBounds: Rect,
        endBounds: Rect,
        doOnEnd: (() -> Unit)? = null,
        doOnEnd: (() -> Unit)? = null,
    ) {
    ) {
        if (startBounds == endBounds) {
            ProtoLog.w(
                ShellProtoLogGroup.WM_SHELL_DESKTOP_MODE,
                "%s: reposition animation request with equal start/end bounds=%s, ignoring",
                TAG,
                startBounds,
            )
            return
        }
        val tx = transactionSupplier.get()
        val tx = transactionSupplier.get()


        boundsAnimator?.cancel()
        boundsAnimator?.cancel()
@@ -61,27 +72,31 @@ class ReturnToDragStartAnimator(
                .apply {
                .apply {
                    addListener(
                    addListener(
                        onStart = {
                        onStart = {
                            taskSurface.checkValidOrCancel()?.let { surface ->
                                val startTransaction = transactionSupplier.get()
                                val startTransaction = transactionSupplier.get()
                                startTransaction
                                startTransaction
                                    .setPosition(
                                    .setPosition(
                                    taskSurface,
                                        surface,
                                        startBounds.left.toFloat(),
                                        startBounds.left.toFloat(),
                                        startBounds.top.toFloat(),
                                        startBounds.top.toFloat(),
                                    )
                                    )
                                .show(taskSurface)
                                    .show(surface)
                                    .apply()
                                    .apply()
                                taskRepositionAnimationListener.onAnimationStart(taskId)
                                taskRepositionAnimationListener.onAnimationStart(taskId)
                            }
                        },
                        },
                        onEnd = {
                        onEnd = {
                            taskSurface.checkValid()?.let { surface ->
                                val finishTransaction = transactionSupplier.get()
                                val finishTransaction = transactionSupplier.get()
                                finishTransaction
                                finishTransaction
                                    .setPosition(
                                    .setPosition(
                                    taskSurface,
                                        surface,
                                        endBounds.left.toFloat(),
                                        endBounds.left.toFloat(),
                                        endBounds.top.toFloat(),
                                        endBounds.top.toFloat(),
                                    )
                                    )
                                .show(taskSurface)
                                    .show(surface)
                                    .apply()
                                    .apply()
                            }
                            taskRepositionAnimationListener.onAnimationEnd(taskId)
                            taskRepositionAnimationListener.onAnimationEnd(taskId)
                            boundsAnimator = null
                            boundsAnimator = null
                            doOnEnd?.invoke()
                            doOnEnd?.invoke()
@@ -89,16 +104,29 @@ class ReturnToDragStartAnimator(
                        },
                        },
                    )
                    )
                    addUpdateListener { anim ->
                    addUpdateListener { anim ->
                        taskSurface.checkValidOrCancel()?.let { surface ->
                            val rect = anim.animatedValue as Rect
                            val rect = anim.animatedValue as Rect
                        tx.setPosition(taskSurface, rect.left.toFloat(), rect.top.toFloat())
                            tx.setPosition(surface, rect.left.toFloat(), rect.top.toFloat())
                            .show(taskSurface)
                                .show(surface)
                                .apply()
                                .apply()
                        }
                        }
                    }
                    }
                }
                .also(ValueAnimator::start)
                .also(ValueAnimator::start)
    }
    }


    private fun SurfaceControl.checkValid(): SurfaceControl? = if (isValid) this else null

    private fun SurfaceControl.checkValidOrCancel(): SurfaceControl? {
        if (isValid) {
            return this
        }
        boundsAnimator?.cancel()
        return null
    }

    companion object {
    companion object {
        private const val TAG = "ReturnToDragStartAnimator"
        const val RETURN_TO_DRAG_START_ANIMATION_MS = 300L
        const val RETURN_TO_DRAG_START_ANIMATION_MS = 300L
    }
    }
}
}
+44 −0
Original line number Original line Diff line number Diff line
@@ -9237,6 +9237,50 @@ class DesktopTasksControllerTest(flags: FlagsParameterization) : ShellTestCase()
            .logTaskResizingEnded(any(), any(), any(), any(), any(), any(), any())
            .logTaskResizingEnded(any(), any(), any(), any(), any(), any(), any())
    }
    }
    @Test
    fun onDesktopDragEnd_noIndicator_noBoundsMovement_noReturnToStartAnimation() {
        val task = setUpFreeformTask(bounds = STABLE_BOUNDS)
        val spyController = spy(controller)
        val mockSurface = mock(SurfaceControl::class.java)
        val mockDisplayLayout = mock(DisplayLayout::class.java)
        whenever(displayController.getDisplayLayout(task.displayId)).thenReturn(mockDisplayLayout)
        whenever(mockDisplayLayout.stableInsets()).thenReturn(Rect(0, 100, 2000, 2000))
        whenever(mockDisplayLayout.getStableBounds(any())).thenAnswer { i ->
            (i.arguments.first() as Rect).set(STABLE_BOUNDS)
        }
        whenever(spyController.getVisualIndicator()).thenReturn(desktopModeVisualIndicator)
        whenever(desktopModeVisualIndicator.updateIndicatorType(any(), anyOrNull()))
            .thenReturn(DesktopModeVisualIndicator.IndicatorType.NO_INDICATOR)
        whenever(motionEvent.displayId).thenReturn(DEFAULT_DISPLAY)
        // "Drag move" the task, but the bounds aren't actually changing. Which can be the case
        // on mouse clicks that generate ACTION_MOVE events without position changes.
        val currentDragBounds = Rect(200, 200, 700, 700)
        spyController.onDragPositioningMove(
            taskInfo = task,
            taskSurface = mockSurface,
            displayId = DEFAULT_DISPLAY,
            inputX = 500f,
            inputY = 210f,
            taskBounds = currentDragBounds,
        )
        spyController.onDragPositioningEnd(
            taskInfo = task,
            taskSurface = mockSurface,
            displayId = DEFAULT_DISPLAY,
            inputCoordinate = PointF(510f, 210f),
            currentDragBounds = currentDragBounds,
            validDragArea = Rect(0, 50, 2000, 2000),
            dragStartBounds = currentDragBounds,
            motionEvent = motionEvent,
        )
        // Even though end bounds are the same as start bounds, there's no need to animate the
        // return because the current bounds are also the same as start/end.
        verify(mReturnToDragStartAnimator, never())
            .start(eq(task.taskId), eq(mockSurface), any(), any(), anyOrNull())
    }
    @Test
    @Test
    fun enterSplit_freeformTaskIsMovedToSplit() {
    fun enterSplit_freeformTaskIsMovedToSplit() {
        val task1 = setUpFreeformTask()
        val task1 = setUpFreeformTask()