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

Commit c3ebae4c authored by Galia Peycheva's avatar Galia Peycheva
Browse files

Fix pip update transaction out of order 

When an animation is playing, PipAnimationController#onAnimationUpdate
is called with the fraction of the animation progress. This fraction is
used to calculate a scale value (in a matrix), which is applied in a
transaction to the pip surface. This scaling is invisible to WM core, so
the bounds of the pip surface are still the bounds before the animation
began. In SF, those bounds are scaled and drawn. When the animation is
finished, onAnimationEnd is called. onAnimationEnd is special, because
it finishes the resizing and sends the updated pip task bounds to WM
core in a WindowContainerTransaction. The WindowContainerTransaction is
applied separately by WM core.

Additionally, the transactions forged by onAnimationUpdate are
not applied immediately. In each onAnimationUpdate, the pip surface
transaction is applied in a SyncRtSurfaceTransactionApplier (together
with the transaction for the pip menu), which waits until the next draw
is complete and then merges the pip surface transaction into the global
frame transaction. In onAnimationEnd, however, the pip update is not
applied in a SurfaceControl.Transaction, but a
WindowContainerTransaction is sent to WM core, which applies it at some
later point. If the pip surface transaction from the last
onAnimationUpdate is merged+applied after the WindowContainerTransaction
is applied in WM core, then the two transactions are committed out of
order and the pip surface ends up in this weird state, where an extra
scale was applied to it.

To address this, we delay the onAnimationEnd to the next frame, which
adds enough time to let the last onAnimationUpdate transaction be
applied. We do this only when the PiP transaction has to be synced
with the PiP menu drawing, because the synchronization between the
two is what causes the last onAnimationUpdate transaction to be
applied with a delay

Bug: 228317396
Test: manual - change pip aspect ratio several times
Change-Id: Iaf4005b34b63ab7a123e508ace80f966014a0805
parent 1bbb6923

libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipMenuController.java

+6 −0
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import static android.view.WindowManager.LayoutParams.FLAG_WATCH_OUTSIDE_TOUCH; 
import static android.view.WindowManager.LayoutParams.PRIVATE_FLAG_TRUSTED_OVERLAY;

import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_OVERLAY;



import android.annotation.NonNull;

import android.annotation.Nullable;

import android.app.ActivityManager.RunningTaskInfo;

import android.app.RemoteAction;
@@ -69,6 +70,11 @@ public interface PipMenuController { 
     */

    void setAppActions(List<RemoteAction> appActions, RemoteAction closeAction);



    /**

     * Wait until the next frame to run the given Runnable.

     */

    void runWithNextFrame(@NonNull Runnable runnable);



    /**

     * Resize the PiP menu with the given bounds. The PiP SurfaceControl is given if there is a

     * need to synchronize the movements on the same frame as PiP.

libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTaskOrganizer.java

+35 −5
Original line number Diff line number Diff line
@@ -179,8 +179,10 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener, 
                // This is necessary in case there was a resize animation ongoing when exit PIP

                // started, in which case the first resize will be skipped to let the exit

                // operation handle the final resize out of PIP mode. See b/185306679.

                finishResizeDelayedIfNeeded(() -> {

                    finishResize(tx, destinationBounds, direction, animationType);

                    sendOnPipTransitionFinished(direction);

                });

            }

        }
@@ -196,6 +198,34 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener, 
        }

    };



    /**

     * Finishes resizing the PiP, delaying the operation if it has to be synced with the PiP menu.

     *

     * This is done to avoid a race condition between the last transaction applied in

     * onAnimationUpdate and the finishResize in onAnimationEnd. finishResize creates a

     * WindowContainerTransaction, which is to be applied by WmCore later. It may happen that it

     * gets applied before the transaction created by the last onAnimationUpdate. As a result of

     * this, the PiP surface may get scaled after the new bounds are applied by WmCore, which

     * makes the PiP surface have unexpected bounds. To avoid this, we delay the finishResize

     * operation until the next frame. This aligns the last onAnimationUpdate transaction with the

     * WCT application.

     *

     * The race only happens when the PiP surface transaction has to be synced with the PiP menu

     * due to the necessity for a delay when syncing the PiP surface, the PiP menu surface and

     * the PiP menu contents.

     */

    private void finishResizeDelayedIfNeeded(Runnable finishResizeRunnable) {

        if (!shouldSyncPipTransactionWithMenu()) {

            finishResizeRunnable.run();

            return;

        }

        mPipMenuController.runWithNextFrame(finishResizeRunnable);

    }



    private boolean shouldSyncPipTransactionWithMenu() {

        return mPipMenuController.isMenuVisible();

    }



    @VisibleForTesting

    final PipTransitionController.PipTransitionCallback mPipTransitionCallback =

            new PipTransitionController.PipTransitionCallback() {
@@ -221,7 +251,7 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener, 
                @Override

                public boolean handlePipTransaction(SurfaceControl leash,

                        SurfaceControl.Transaction tx, Rect destinationBounds) {

                    if (mPipMenuController.isMenuVisible()) {

                    if (shouldSyncPipTransactionWithMenu()) {

                        mPipMenuController.movePipMenu(leash, tx, destinationBounds);

                        return true;

                    }
@@ -1223,7 +1253,7 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener, 
        mSurfaceTransactionHelper

                .crop(tx, mLeash, toBounds)

                .round(tx, mLeash, mPipTransitionState.isInPip());

        if (mPipMenuController.isMenuVisible()) {

        if (shouldSyncPipTransactionWithMenu()) {

            mPipMenuController.resizePipMenu(mLeash, tx, toBounds);

        } else {

            tx.apply();
@@ -1265,7 +1295,7 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener, 
        mSurfaceTransactionHelper

                .scale(tx, mLeash, startBounds, toBounds, degrees)

                .round(tx, mLeash, startBounds, toBounds);

        if (mPipMenuController.isMenuVisible()) {

        if (shouldSyncPipTransactionWithMenu()) {

            mPipMenuController.movePipMenu(mLeash, tx, toBounds);

        } else {

            tx.apply();

libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PhonePipMenuController.java

+12 −0
Original line number Diff line number Diff line
@@ -305,6 +305,18 @@ public class PhonePipMenuController implements PipMenuController { 
                showResizeHandle);

    }



    @Override

    public void runWithNextFrame(Runnable runnable) {

        if (mPipMenuView == null || mPipMenuView.getViewRootImpl() == null) {

            runnable.run();

        }



        mPipMenuView.getViewRootImpl().registerRtFrameCallback(frame -> {

            mMainHandler.post(runnable);

        });

        mPipMenuView.invalidate();

    }



    /**

     * Move the PiP menu, which does a translation and possibly a scale transformation.

     */

libs/WindowManager/Shell/src/com/android/wm/shell/pip/tv/TvPipMenuController.java

+12 −0
Original line number Diff line number Diff line
@@ -465,6 +465,18 @@ public class TvPipMenuController implements PipMenuController, TvPipMenuView.Lis 
        return mSystemWindows.getViewSurface(v);

    }



    @Override

    public void runWithNextFrame(Runnable runnable) {

        if (mPipMenuView == null || mPipMenuView.getViewRootImpl() == null) {

            runnable.run();

        }



        mPipMenuView.getViewRootImpl().registerRtFrameCallback(frame -> {

            mMainHandler.post(runnable);

        });

        mPipMenuView.invalidate();

    }



    @Override

    public void movePipMenu(SurfaceControl pipLeash, SurfaceControl.Transaction transaction,

            Rect pipDestBounds) {