Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix pip update transaction out of order

Browse Source 
When a resize 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 WmCore, so the bounds of the
pip surface are still the bounds before the animation began. In SF,
those bounds are scaled and drawn using that matrix. When the
animation is finished, onAnimationEnd is called. onAnimationEnd is
special, because it finishes the resizing and sends the updated pip
task bounds to WmCore in a WindowContainerTransaction. The
WindowContainerTransaction is applied separately by WmCore.

To animate the PiP surface together with the PiP menu, the
transaction forged by onPipAnimationUpdate should be synchronized with
the corresponding transaction applied on the PiP menu surface. The PiP
menu content also has to redraw in the new bounds. These three events
need to be synchronized, otherwise the animation would look janky.
To synchronize them, the PipMenuController introduces a delay, waiting
until the next draw is complete before merging the PiP surface + PiP
menu surface transactions into the global transaction for the same
frame.

This happens in every onPipAnimationUpdate callback. The
onPipAnimationEnd, however, is different. It has to commit the resize
to WmCore, so it doesn't follow the same code path as the transactions
created on inPipAnimationUpdate. In onPipAnimationEnd the resize is
finished by creating a WindowContainerTransaction and sending it to
WmCore to update the size of the PiP task window container. The WCT
is then applied by the WmCore at some later point in time.

The race condition we are fighting with this CL happens because the
transaction created by the last onPipAnimationUpdate may get applied
*after* the WindowContainerTransaction created by onPipAnimationEnd.
We have no way to synchronize those two transactions, because the
onPipAnimationUpdate one is applied by WmShell, while the
onPipAnimationEnd one is applied by WmCore. In some rare cases, they
might get applied 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 finishing the resize (and creating+sending
the WindowContainerTransaction to WmCore) in onPipAnimationEnd to the
next frame, which aligns it better with when the transaction from the
last onPipAnimationUpdate is applied. Applying onPipAnimationUpdate is
also delayed one frame, because it waits for the Pip menu content
to be drawn for that frame. So introducing the same one-frame-delay
for the finishResize makes sure that it can't happen before the
transaction from the last onPipAnimationUpdate

Bug: 228317396
Test: manual - change pip aspect ratio several times
Change-Id: Ieb7020d329ffe6f1a4c61a22eae65e2b36feb211
This commit is contained in:
Galia Peycheva
2022-11-09 11:46:22 +01:00
parent abee62b65c
commit 04a71fd13c
1 changed files with 41 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -63,6 +63,7 @@ import android.graphics.Rect;
import android.os.RemoteException;
import android.os.SystemClock;
import android.util.Log;
import android.view.Choreographer;
import android.view.Display;
import android.view.Surface;
import android.view.SurfaceControl;
@@ -179,8 +180,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.
finishResize(tx, destinationBounds, direction, animationType);
sendOnPipTransitionFinished(direction);
finishResizeDelayedIfNeeded(() -> {
finishResize(tx, destinationBounds, direction, animationType);
sendOnPipTransitionFinished(direction);
});
}
}
@@ -196,6 +199,39 @@ 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
* onPipAnimationUpdate and the finishResize in onPipAnimationEnd. The transaction in
* onPipAnimationUpdate is applied directly from WmShell, while onPipAnimationEnd creates a
* WindowContainerTransaction in finishResize, which is to be applied by WmCore later. Normally,
* the WCT should be the last transaction to finish the animation. However, it may happen that
* it gets applied *before* the transaction created by the last onPipAnimationUpdate. This
* happens only 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 animation with the PiP menu surface
* animation and redrawing the PiP menu contents. As a result, the PiP surface gets 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.
*/
private void finishResizeDelayedIfNeeded(Runnable finishResizeRunnable) {
if (!shouldSyncPipTransactionWithMenu()) {
finishResizeRunnable.run();
return;
}
// Delay the finishResize to the next frame
Choreographer.getInstance().postCallback(Choreographer.CALLBACK_COMMIT, () -> {
mMainExecutor.execute(finishResizeRunnable);
}, null);
}
private boolean shouldSyncPipTransactionWithMenu() {
return mPipMenuController.isMenuVisible();
}
@VisibleForTesting
final PipTransitionController.PipTransitionCallback mPipTransitionCallback =
new PipTransitionController.PipTransitionCallback() {
@@ -221,7 +257,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 +1259,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 +1301,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();