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

Commit b630bb28 authored by jorgegil@google.com's avatar jorgegil@google.com
Browse files

Prevent duplicate call to update movement bounds on rotation 

When rotating the display while in PIP mode, there's 1 call
to reset the shelf offset back to zero which triggers a call
to PipController.updateMovementBounds(fromShelfAdj=true).
Then, another call is made immediately to
PipController.updateMovementBounds(fromRotation=true).

While both are useful when called independently, we now
avoid calling the first on rotation, otherwise it'll try to
offset the pre-rotation PIP bounds and use those bounds as
the post-rotation bounds instead of calculating the new bounds
using the rotated display and the previous snap fraction.

Bug: 174669093
Test: PIP position is preserved when rotating the home screen
Change-Id: I071ace219bd26a5c8078b60c77a8b2442470305c
parent 73cc15b5

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

+1 −2
Original line number Diff line number Diff line
@@ -129,10 +129,9 @@ public class PipBoundsAlgorithm { 
                : getDefaultBounds();



        final boolean useCurrentSize = reentryState != null && reentryState.getSize() != null;

        final Rect r = transformBoundsToAspectRatioIfValid(destinationBounds,

        return transformBoundsToAspectRatioIfValid(destinationBounds,

                mPipBoundsState.getAspectRatio(), false /* useCurrentMinEdgeSize */,

                useCurrentSize);

        return r;

    }



    /** Returns the current bounds adjusted to the new aspect ratio, if valid. */

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

+10 −4
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import android.util.Size; 
import android.view.DisplayInfo;



import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.function.TriConsumer;

import com.android.wm.shell.R;

import com.android.wm.shell.common.DisplayLayout;
@@ -33,7 +34,6 @@ import java.io.PrintWriter; 
import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.util.Objects;

import java.util.function.BiConsumer;



/**

 * Singleton source of truth for the current state of PIP bounds.
@@ -80,7 +80,7 @@ public final class PipBoundsState { 
    private boolean mHasUserResizedPip;



    private @Nullable Runnable mOnMinimalSizeChangeCallback;

    private @Nullable BiConsumer<Boolean, Integer> mOnShelfVisibilityChangeCallback;

    private @Nullable TriConsumer<Boolean, Integer, Boolean> mOnShelfVisibilityChangeCallback;



    public PipBoundsState(@NonNull Context context) {

        mContext = context;
@@ -310,6 +310,11 @@ public final class PipBoundsState { 


    /** Set whether the shelf is showing and its height. */

    public void setShelfVisibility(boolean showing, int height) {

        setShelfVisibility(showing, height, true);

    }



    /** Set whether the shelf is showing and its height. */

    public void setShelfVisibility(boolean showing, int height, boolean updateMovementBounds) {

        final boolean shelfShowing = showing && height > 0;

        if (shelfShowing == mIsShelfShowing && height == mShelfHeight) {

            return;
@@ -318,7 +323,8 @@ public final class PipBoundsState { 
        mIsShelfShowing = showing;

        mShelfHeight = height;

        if (mOnShelfVisibilityChangeCallback != null) {

            mOnShelfVisibilityChangeCallback.accept(mIsShelfShowing, mShelfHeight);

            mOnShelfVisibilityChangeCallback.accept(mIsShelfShowing, mShelfHeight,

                    updateMovementBounds);

        }

    }
@@ -351,7 +357,7 @@ public final class PipBoundsState { 


    /** Set a callback to be notified when the shelf visibility changes. */

    public void setOnShelfVisibilityChangeCallback(

            @Nullable BiConsumer<Boolean, Integer> onShelfVisibilityChangeCallback) {

            @Nullable TriConsumer<Boolean, Integer, Boolean> onShelfVisibilityChangeCallback) {

        mOnShelfVisibilityChangeCallback = onShelfVisibilityChangeCallback;

    }

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

+22 −14
Original line number Diff line number Diff line
@@ -114,14 +114,14 @@ public class PipController implements Pip, PipTaskOrganizer.PipTransitionCallbac 
        // the bounds for the next orientation using the destination bounds of the animation

        // TODO: Technically this should account for movement animation bounds as well

        Rect currentBounds = mPipTaskOrganizer.getCurrentOrAnimatingBounds();

        final boolean changed = onDisplayRotationChanged(mContext,

                mPipBoundsState.getNormalBounds(), currentBounds, mTmpInsetBounds, displayId,

                fromRotation, toRotation, t);

        final Rect outBounds = new Rect();

        final boolean changed = onDisplayRotationChanged(mContext, outBounds, currentBounds,

                mTmpInsetBounds, displayId, fromRotation, toRotation, t);

        if (changed) {

            // If the pip was in the offset zone earlier, adjust the new bounds to the bottom of the

            // movement bounds

            mTouchHandler.adjustBoundsForRotation(mPipBoundsState.getNormalBounds(),

                    mPipBoundsState.getBounds(), mTmpInsetBounds);

            mTouchHandler.adjustBoundsForRotation(outBounds, mPipBoundsState.getBounds(),

                    mTmpInsetBounds);



            // The bounds are being applied to a specific snap fraction, so reset any known offsets

            // for the previous orientation before updating the movement bounds.
@@ -129,14 +129,18 @@ public class PipController implements Pip, PipTaskOrganizer.PipTransitionCallbac 
            // not during the fixed rotation. In fixed rotation case, app is about to enter PiP

            // and we need the offsets preserved to calculate the destination bounds.

            if (!mIsInFixedRotation) {

                mPipBoundsState.setShelfVisibility(false /* showing */, 0 /* height */);

                // Update the shelf visibility without updating the movement bounds. We're already

                // updating them below with the |fromRotation| flag set, which is more accurate

                // than using the |fromShelfAdjustment|.

                mPipBoundsState.setShelfVisibility(false /* showing */, 0 /* height */,

                        false /* updateMovementBounds */);

                mPipBoundsState.setImeVisibility(false /* showing */, 0 /* height */);

                mTouchHandler.onShelfVisibilityChanged(false, 0);

                mTouchHandler.onImeVisibilityChanged(false, 0);

            }



            updateMovementBounds(mPipBoundsState.getNormalBounds(), true /* fromRotation */,

                    false /* fromImeAdjustment */, false /* fromShelfAdjustment */, t);

            updateMovementBounds(outBounds, true /* fromRotation */, false /* fromImeAdjustment */,

                    false /* fromShelfAdjustment */, t);

        }

    };
@@ -248,11 +252,15 @@ public class PipController implements Pip, PipTaskOrganizer.PipTransitionCallbac 
                            false /* fromImeAdjustment */, false /* fromShelfAdjustment */,

                            null /* wct */);

                });

        mPipBoundsState.setOnShelfVisibilityChangeCallback((isShowing, height) -> {

        mPipBoundsState.setOnShelfVisibilityChangeCallback(

                (isShowing, height, updateMovementBounds) -> {

                    mTouchHandler.onShelfVisibilityChanged(isShowing, height);

                    if (updateMovementBounds) {

                        updateMovementBounds(mPipBoundsState.getBounds(),

                                false /* fromRotation */, false /* fromImeAdjustment */,

                    true /* fromShelfAdjustment */, null /* windowContainerTransaction */);

                                true /* fromShelfAdjustment */,

                                null /* windowContainerTransaction */);

                    }

                });

        if (mTouchHandler != null) {

            // Register the listener for input consumer touch events. Only for Phone

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip/PipBoundsStateTest.java

+15 −6
Original line number Diff line number Diff line
@@ -31,14 +31,13 @@ import android.util.Size; 


import androidx.test.filters.SmallTest;



import com.android.internal.util.function.TriConsumer;

import com.android.wm.shell.ShellTestCase;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;



import java.util.function.BiConsumer;



/**

 * Tests for {@link PipBoundsState}.

 */
@@ -117,23 +116,33 @@ public class PipBoundsStateTest extends ShellTestCase { 


    @Test

    public void testSetShelfVisibility_changed_callbackInvoked() {

        final BiConsumer<Boolean, Integer> callback = mock(BiConsumer.class);

        final TriConsumer<Boolean, Integer, Boolean> callback = mock(TriConsumer.class);

        mPipBoundsState.setOnShelfVisibilityChangeCallback(callback);



        mPipBoundsState.setShelfVisibility(true, 100);



        verify(callback).accept(true, 100);

        verify(callback).accept(true, 100, true);

    }



    @Test

    public void testSetShelfVisibility_changedWithoutUpdateMovBounds_callbackInvoked() {

        final TriConsumer<Boolean, Integer, Boolean> callback = mock(TriConsumer.class);

        mPipBoundsState.setOnShelfVisibilityChangeCallback(callback);



        mPipBoundsState.setShelfVisibility(true, 100, false);



        verify(callback).accept(true, 100, false);

    }



    @Test

    public void testSetShelfVisibility_notChanged_callbackNotInvoked() {

        final BiConsumer<Boolean, Integer> callback = mock(BiConsumer.class);

        final TriConsumer<Boolean, Integer, Boolean> callback = mock(TriConsumer.class);

        mPipBoundsState.setShelfVisibility(true, 100);

        mPipBoundsState.setOnShelfVisibilityChangeCallback(callback);



        mPipBoundsState.setShelfVisibility(true, 100);



        verify(callback, never()).accept(true, 100);

        verify(callback, never()).accept(true, 100, true);

    }



    @Test