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

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

Move aspect ratio state into PipBoundsState

Couple of changes here:
1. mAspectRatio is moved to PipBoundsState
2. Simplified getDestinationBounds by removing the aspect ratio
param. It was always re-calculated, then sent to getDestinationBounds,
then updated mAspectRatio. Now mAspectRatio is updated first and
getDestinationBounds just uses the PipBoundsState's ratio.

Bug: 169373982
Test: aspect ratio is set correctly on enter PIP, and updated during
PIP mode if app changes it (e.g. Youtube)

Change-Id: I144960ff41bde4f18eb1e9a8e60ea0cba29a8470
parent 16b27d50
Loading
Loading
Loading
Loading
+12 −30
Original line number Diff line number Diff line
@@ -60,7 +60,6 @@ public class PipBoundsHandler {
    private float mDefaultAspectRatio;
    private float mMinAspectRatio;
    private float mMaxAspectRatio;
    private float mAspectRatio;
    private int mDefaultStackGravity;
    private int mDefaultMinSize;
    private Point mScreenEdgeInsets;
@@ -80,7 +79,7 @@ public class PipBoundsHandler {
        // Initialize the aspect ratio to the default aspect ratio.  Don't do this in reload
        // resources as it would clobber mAspectRatio when entering PiP from fullscreen which
        // triggers a configuration change and the resources to be reloaded.
        mAspectRatio = mDefaultAspectRatio;
        mPipBoundsState.setAspectRatio(mDefaultAspectRatio);
    }

    /**
@@ -126,10 +125,6 @@ public class PipBoundsHandler {
        mCurrentMinSize = minEdgeSize;
    }

    protected float getAspectRatio() {
        return mAspectRatio;
    }

    /**
     * Sets both shelf visibility and its height if applicable.
     * @return {@code true} if the internal shelf state is changed, {@code false} otherwise.
@@ -165,8 +160,8 @@ public class PipBoundsHandler {
        if (animatingBounds.isEmpty()) {
            animatingBounds.set(defaultBounds);
        }
        if (isValidPictureInPictureAspectRatio(mAspectRatio)) {
            transformBoundsToAspectRatio(normalBounds, mAspectRatio,
        if (isValidPictureInPictureAspectRatio(mPipBoundsState.getAspectRatio())) {
            transformBoundsToAspectRatio(normalBounds, mPipBoundsState.getAspectRatio(),
                    false /* useCurrentMinEdgeSize */, false /* useCurrentSize */);
        }
        displayInfo.copyFrom(mDisplayInfo);
@@ -205,27 +200,16 @@ public class PipBoundsHandler {
    }

    /**
     * Responds to IPinnedStackListener on resetting aspect ratio for the pinned window.
     * It will normally follow up with a
     * {@link #onMovementBoundsChanged(Rect, Rect, Rect, DisplayInfo)} callback.
     */
    public void onAspectRatioChanged(float aspectRatio) {
        mAspectRatio = aspectRatio;
    }

    /**
     * See {@link #getDestinationBounds(float, Rect, Size, boolean)}
     * See {@link #getDestinationBounds(Rect, Size, boolean)}
     */
    public Rect getDestinationBounds(float aspectRatio, Rect bounds, Size minimalSize) {
        return getDestinationBounds(aspectRatio, bounds, minimalSize,
                false /* useCurrentMinEdgeSize */);
    public Rect getDestinationBounds(Rect bounds, Size minimalSize) {
        return getDestinationBounds(bounds, minimalSize, false /* useCurrentMinEdgeSize */);
    }

    /**
     * @return {@link Rect} of the destination PiP window bounds.
     */
    public Rect getDestinationBounds(float aspectRatio, Rect bounds,
            Size minimalSize, boolean useCurrentMinEdgeSize) {
    public Rect getDestinationBounds(Rect bounds, Size minimalSize, boolean useCurrentMinEdgeSize) {
        boolean isReentryBounds = false;
        final Rect destinationBounds;
        if (bounds == null) {
@@ -248,11 +232,10 @@ public class PipBoundsHandler {
            // Just adjusting bounds (e.g. on aspect ratio changed).
            destinationBounds = new Rect(bounds);
        }
        if (isValidPictureInPictureAspectRatio(aspectRatio)) {
            transformBoundsToAspectRatio(destinationBounds, aspectRatio, useCurrentMinEdgeSize,
                    isReentryBounds);
        if (isValidPictureInPictureAspectRatio(mPipBoundsState.getAspectRatio())) {
            transformBoundsToAspectRatio(destinationBounds, mPipBoundsState.getAspectRatio(),
                    useCurrentMinEdgeSize, isReentryBounds);
        }
        mAspectRatio = aspectRatio;
        return destinationBounds;
    }

@@ -361,8 +344,8 @@ public class PipBoundsHandler {
     * @param stackBounds
     */
    public void transformBoundsToAspectRatio(Rect stackBounds) {
        transformBoundsToAspectRatio(stackBounds, mAspectRatio, true /* useCurrentMinEdgeSize */,
                true /* useCurrentSize */);
        transformBoundsToAspectRatio(stackBounds, mPipBoundsState.getAspectRatio(),
                true /* useCurrentMinEdgeSize */, true /* useCurrentSize */);
    }

    /**
@@ -507,7 +490,6 @@ public class PipBoundsHandler {
        pw.println(innerPrefix + "mDefaultAspectRatio=" + mDefaultAspectRatio);
        pw.println(innerPrefix + "mMinAspectRatio=" + mMinAspectRatio);
        pw.println(innerPrefix + "mMaxAspectRatio=" + mMaxAspectRatio);
        pw.println(innerPrefix + "mAspectRatio=" + mAspectRatio);
        pw.println(innerPrefix + "mDefaultStackGravity=" + mDefaultStackGravity);
        pw.println(innerPrefix + "mIsImeShowing=" + mIsImeShowing);
        pw.println(innerPrefix + "mImeHeight=" + mImeHeight);
+10 −0
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ public final class PipBoundsState {
    private static final String TAG = PipBoundsState.class.getSimpleName();

    private final @NonNull Rect mBounds = new Rect();
    private float mAspectRatio;
    private PipReentryState mPipReentryState;
    private ComponentName mLastPipComponentName;

@@ -46,6 +47,14 @@ public final class PipBoundsState {
        return new Rect(mBounds);
    }

    public void setAspectRatio(float aspectRatio) {
        mAspectRatio = aspectRatio;
    }

    public float getAspectRatio() {
        return mAspectRatio;
    }

    /**
     * Save the reentry state to restore to when re-entering PIP mode.
     *
@@ -120,6 +129,7 @@ public final class PipBoundsState {
        pw.println(prefix + TAG);
        pw.println(innerPrefix + "mBounds=" + mBounds);
        pw.println(innerPrefix + "mLastPipComponentName=" + mLastPipComponentName);
        pw.println(innerPrefix + "mAspectRatio=" + mAspectRatio);
        if (mPipReentryState == null) {
            pw.println(innerPrefix + "mPipReentryState=null");
        } else {
+10 −12
Original line number Diff line number Diff line
@@ -332,9 +332,9 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
        mShouldIgnoreEnteringPipTransition = true;
        mState = State.ENTERING_PIP;
        mPipBoundsState.setLastPipComponentName(componentName);
        return mPipBoundsHandler.getDestinationBounds(
                getAspectRatioOrDefault(pictureInPictureParams),
                null /* bounds */, getMinimalSize(activityInfo));
        mPipBoundsState.setAspectRatio(getAspectRatioOrDefault(pictureInPictureParams));
        return mPipBoundsHandler.getDestinationBounds(null /* bounds */,
                getMinimalSize(activityInfo));
    }

    /**
@@ -494,9 +494,9 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
            return;
        }

        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                getAspectRatioOrDefault(mPictureInPictureParams),
                null /* bounds */, getMinimalSize(mTaskInfo.topActivityInfo));
        mPipBoundsState.setAspectRatio(getAspectRatioOrDefault(mPictureInPictureParams));
        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(null /* bounds */,
                getMinimalSize(mTaskInfo.topActivityInfo));
        Objects.requireNonNull(destinationBounds, "Missing destination bounds");
        final Rect currentBounds = mTaskInfo.configuration.windowConfiguration.getBounds();

@@ -696,8 +696,8 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
            Log.d(TAG, "Ignored onTaskInfoChanged with PiP param: " + newParams);
            return;
        }
        // Aspect ratio changed, re-calculate destination bounds.
        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                getAspectRatioOrDefault(newParams),
                mPipBoundsState.getBounds(), getMinimalSize(info.topActivityInfo),
                true /* userCurrentMinEdgeSize */);
        Objects.requireNonNull(destinationBounds, "Missing destination bounds");
@@ -714,7 +714,6 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
    public void onFixedRotationFinished(int displayId) {
        if (mShouldDeferEnteringPip && mState.isInPip()) {
            final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                    getAspectRatioOrDefault(mPictureInPictureParams),
                    null /* bounds */, getMinimalSize(mTaskInfo.topActivityInfo));
            // schedule a regular animation to ensure all the callbacks are still being sent
            enterPipWithAlphaAnimation(destinationBounds, 0 /* durationMs */);
@@ -787,9 +786,8 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
            return;
        }

        final Rect newDestinationBounds = mPipBoundsHandler.getDestinationBounds(
                getAspectRatioOrDefault(mPictureInPictureParams),
                null /* bounds */, getMinimalSize(mTaskInfo.topActivityInfo));
        final Rect newDestinationBounds = mPipBoundsHandler.getDestinationBounds(null /* bounds */,
                getMinimalSize(mTaskInfo.topActivityInfo));
        if (newDestinationBounds.equals(currentDestinationBounds)) return;
        if (animator.getAnimationType() == ANIM_TYPE_BOUNDS) {
            animator.updateEndValue(newDestinationBounds);
@@ -810,7 +808,7 @@ public class PipTaskOrganizer implements ShellTaskOrganizer.TaskListener,
                params.getAspectRatioRational());
        mPictureInPictureParams = params;
        if (aspectRatioChanged) {
            mPipBoundsHandler.onAspectRatioChanged(params.getAspectRatio());
            mPipBoundsState.setAspectRatio(params.getAspectRatio());
        }
        return aspectRatioChanged;
    }
+3 −1
Original line number Diff line number Diff line
@@ -197,8 +197,10 @@ public class PipController implements Pip, PipTaskOrganizer.PipTransitionCallbac

        @Override
        public void onAspectRatioChanged(float aspectRatio) {
            // TODO(b/169373982): Remove this callback as it is redundant with PipTaskOrg params
            // change.
            mHandler.post(() -> {
                mPipBoundsHandler.onAspectRatioChanged(aspectRatio);
                mPipBoundsState.setAspectRatio(aspectRatio);
                mTouchHandler.onAspectRatioChanged();
            });
        }
+26 −16
Original line number Diff line number Diff line
@@ -123,7 +123,8 @@ public class PipBoundsHandlerTest extends PipTestCase {
                (MAX_ASPECT_RATIO + DEFAULT_ASPECT_RATIO) / 2
        };
        for (float aspectRatio : aspectRatios) {
            final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(aspectRatio,
            mPipBoundsState.setAspectRatio(aspectRatio);
            final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                    EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);
            final float actualAspectRatio =
                    destinationBounds.width() / (destinationBounds.height() * 1f);
@@ -139,7 +140,8 @@ public class PipBoundsHandlerTest extends PipTestCase {
                MAX_ASPECT_RATIO * 2
        };
        for (float aspectRatio : invalidAspectRatios) {
            final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(aspectRatio,
            mPipBoundsState.setAspectRatio(aspectRatio);
            final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                    EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);
            final float actualAspectRatio =
                    destinationBounds.width() / (destinationBounds.height() * 1f);
@@ -155,8 +157,9 @@ public class PipBoundsHandlerTest extends PipTestCase {
        final Rect currentBounds = new Rect(0, 0, 0, 100);
        currentBounds.right = (int) (currentBounds.height() * aspectRatio) + currentBounds.left;

        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(aspectRatio,
                currentBounds, EMPTY_MINIMAL_SIZE);
        mPipBoundsState.setAspectRatio(aspectRatio);
        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(currentBounds,
                EMPTY_MINIMAL_SIZE);

        final float actualAspectRatio =
                destinationBounds.width() / (destinationBounds.height() * 1f);
@@ -179,7 +182,8 @@ public class PipBoundsHandlerTest extends PipTestCase {
        for (int i = 0; i < aspectRatios.length; i++) {
            final float aspectRatio = aspectRatios[i];
            final Size minimalSize = minimalSizes[i];
            final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(aspectRatio,
            mPipBoundsState.setAspectRatio(aspectRatio);
            final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                    EMPTY_CURRENT_BOUNDS, minimalSize);
            assertTrue("Destination bounds is no smaller than minimal requirement",
                    (destinationBounds.width() == minimalSize.getWidth()
@@ -200,7 +204,8 @@ public class PipBoundsHandlerTest extends PipTestCase {
        currentBounds.right = (int) (currentBounds.height() * aspectRatio) + currentBounds.left;
        final Size minSize = new Size(currentBounds.width() / 2, currentBounds.height() / 2);

        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(aspectRatio,
        mPipBoundsState.setAspectRatio(aspectRatio);
        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                currentBounds, minSize);

        assertTrue("Destination bounds ignores minimal size",
@@ -210,13 +215,14 @@ public class PipBoundsHandlerTest extends PipTestCase {

    @Test
    public void getDestinationBounds_reentryStateExists_restoreLastSize() {
        final Rect reentryBounds = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
        final Rect reentryBounds = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);
        reentryBounds.scale(1.25f);
        final float reentrySnapFraction = mPipBoundsHandler.getSnapFraction(reentryBounds);

        mPipBoundsState.saveReentryState(reentryBounds, reentrySnapFraction);
        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        assertEquals(reentryBounds.width(), destinationBounds.width());
@@ -225,14 +231,15 @@ public class PipBoundsHandlerTest extends PipTestCase {

    @Test
    public void getDestinationBounds_reentryStateExists_restoreLastPosition() {
        final Rect reentryBounds = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
        final Rect reentryBounds = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);
        reentryBounds.offset(0, -100);
        final float reentrySnapFraction = mPipBoundsHandler.getSnapFraction(reentryBounds);

        mPipBoundsState.saveReentryState(reentryBounds, reentrySnapFraction);

        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        final Rect destinationBounds = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        assertBoundsInclusionWithMargin("restoreLastPosition", reentryBounds, destinationBounds);
@@ -241,11 +248,12 @@ public class PipBoundsHandlerTest extends PipTestCase {
    @Test
    public void setShelfHeight_offsetBounds() {
        final int shelfHeight = 100;
        final Rect oldPosition = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
        final Rect oldPosition = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        mPipBoundsHandler.setShelfHeight(true, shelfHeight);
        final Rect newPosition = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        final Rect newPosition = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        oldPosition.offset(0, -shelfHeight);
@@ -255,11 +263,12 @@ public class PipBoundsHandlerTest extends PipTestCase {
    @Test
    public void onImeVisibilityChanged_offsetBounds() {
        final int imeHeight = 100;
        final Rect oldPosition = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
        final Rect oldPosition = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        mPipBoundsHandler.onImeVisibilityChanged(true, imeHeight);
        final Rect newPosition = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        final Rect newPosition = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        oldPosition.offset(0, -imeHeight);
@@ -268,12 +277,13 @@ public class PipBoundsHandlerTest extends PipTestCase {

    @Test
    public void getDestinationBounds_noReentryState_useDefaultBounds() {
        final Rect defaultBounds = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        mPipBoundsState.setAspectRatio(DEFAULT_ASPECT_RATIO);
        final Rect defaultBounds = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        mPipBoundsState.clearReentryState();

        final Rect actualBounds = mPipBoundsHandler.getDestinationBounds(DEFAULT_ASPECT_RATIO,
        final Rect actualBounds = mPipBoundsHandler.getDestinationBounds(
                EMPTY_CURRENT_BOUNDS, EMPTY_MINIMAL_SIZE);

        assertBoundsInclusionWithMargin("useDefaultBounds", defaultBounds, actualBounds);