Commit 9dd81f73 authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Ignore initial scale factor for subframes

|mInitialScaleFactor| causes scrollable subframes to their width to fit
the size of their clip rect. This is not desirable, instead the subframe
should inherit the current scale factor of their parent.

Bug: 1099722
Change-Id: I0a45d52a59239b59a30d08349748ae80688c577c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363775
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: default avatarMehran Mahmoudi <mahmoudi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800178}
parent f1001613
...@@ -62,6 +62,7 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato ...@@ -62,6 +62,7 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
/** The viewport of this frame. */ /** The viewport of this frame. */
private final PlayerFrameViewport mViewport; private final PlayerFrameViewport mViewport;
private boolean mIsSubframe;
private float mInitialScaleFactor; private float mInitialScaleFactor;
/** Handles scaling of bitmaps. */ /** Handles scaling of bitmaps. */
private final Matrix mBitmapScaleMatrix; private final Matrix mBitmapScaleMatrix;
...@@ -80,6 +81,7 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato ...@@ -80,6 +81,7 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
mCompositorDelegate = compositorDelegate; mCompositorDelegate = compositorDelegate;
mGestureListener = gestureListener; mGestureListener = gestureListener;
mViewport = new PlayerFrameViewport(); mViewport = new PlayerFrameViewport();
mIsSubframe = false;
mInitialScaleFactor = 0f; mInitialScaleFactor = 0f;
mGuid = frameGuid; mGuid = frameGuid;
mContentSize = contentSize; mContentSize = contentSize;
...@@ -122,6 +124,8 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato ...@@ -122,6 +124,8 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
mSubFrameRects.add(clipRect); mSubFrameRects.add(clipRect);
mSubFrameMediators.add(mediator); mSubFrameMediators.add(mediator);
mSubFrameScaledRects.add(new Rect()); mSubFrameScaledRects.add(new Rect());
mediator.markAsSubframe();
mediator.setInitialScaleFactor(mInitialScaleFactor);
mModel.set(PlayerFrameProperties.SUBFRAME_VIEWS, mSubFrameViews); mModel.set(PlayerFrameProperties.SUBFRAME_VIEWS, mSubFrameViews);
mModel.set(PlayerFrameProperties.SUBFRAME_RECTS, mSubFrameScaledRects); mModel.set(PlayerFrameProperties.SUBFRAME_RECTS, mSubFrameScaledRects);
} }
...@@ -145,7 +149,10 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato ...@@ -145,7 +149,10 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
} }
// Set initial scale so that content width fits within the layout dimensions. // Set initial scale so that content width fits within the layout dimensions.
adjustInitialScaleFactor(width); if (!mIsSubframe) {
adjustInitialScaleFactor(width);
}
final float scaleFactor = mViewport.getScale(); final float scaleFactor = mViewport.getScale();
updateViewportSize( updateViewportSize(
width, height, (scaleFactor == 0f) ? getInitialScaleFactor() : scaleFactor); width, height, (scaleFactor == 0f) ? getInitialScaleFactor() : scaleFactor);
...@@ -248,9 +255,9 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato ...@@ -248,9 +255,9 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
} }
@Override @Override
public void resetScaleFactorOfAllSubframes() { public void updateScaleFactorOfAllSubframes(float scaleFactor) {
for (int i = 0; i < mSubFrameViews.size(); i++) { for (int i = 0; i < mSubFrameViews.size(); i++) {
mSubFrameMediators.get(i).resetScaleFactor(); mSubFrameMediators.get(i).updateScaleFactor(scaleFactor);
} }
} }
...@@ -309,20 +316,30 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato ...@@ -309,20 +316,30 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
// Internal methods // Internal methods
private void setInitialScaleFactor(float scaleFactor) {
mInitialScaleFactor = scaleFactor;
if (mSubFrameViews == null) return;
for (int i = 0; i < mSubFrameViews.size(); i++) {
mSubFrameMediators.get(i).setInitialScaleFactor(mInitialScaleFactor);
}
}
private void markAsSubframe() {
mIsSubframe = true;
}
@VisibleForTesting @VisibleForTesting
void resetScaleFactor() { void updateScaleFactor(float scaleFactor) {
// Set scale factor to 0 so subframes get the correct scale factor on scale completion. mViewport.setScale(scaleFactor);
mViewport.setScale(0f);
for (int i = 0; i < mSubFrameViews.size(); i++) { for (int i = 0; i < mSubFrameViews.size(); i++) {
mSubFrameMediators.get(i).resetScaleFactor(); mSubFrameMediators.get(i).updateScaleFactor(scaleFactor);
} }
} }
@VisibleForTesting @VisibleForTesting
void forceRedraw() { void forceRedraw() {
adjustInitialScaleFactor(mViewport.getWidth());
final float scaleFactor = mViewport.getScale();
mViewport.setScale((scaleFactor == 0f) ? getInitialScaleFactor() : scaleFactor);
updateVisuals(true); updateVisuals(true);
for (int i = 0; i < mSubFrameViews.size(); i++) { for (int i = 0; i < mSubFrameViews.size(); i++) {
if (mSubFrameViews.get(i).getVisibility() != View.VISIBLE) continue; if (mSubFrameViews.get(i).getVisibility() != View.VISIBLE) continue;
...@@ -344,6 +361,9 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato ...@@ -344,6 +361,9 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
*/ */
private void adjustInitialScaleFactor(float width) { private void adjustInitialScaleFactor(float width) {
mInitialScaleFactor = width / ((float) mContentSize.getWidth()); mInitialScaleFactor = width / ((float) mContentSize.getWidth());
for (int i = 0; i < mSubFrameViews.size(); i++) {
mSubFrameMediators.get(i).setInitialScaleFactor(mInitialScaleFactor);
}
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -53,10 +53,10 @@ public interface PlayerFrameMediatorDelegate { ...@@ -53,10 +53,10 @@ public interface PlayerFrameMediatorDelegate {
void setBitmapScaleMatrix(Matrix bitmapScaleMatrix, float scaleFactor); void setBitmapScaleMatrix(Matrix bitmapScaleMatrix, float scaleFactor);
/** /**
* Resets the scale factor of subframes. This allows a new initial scale factor to be used for * Updates the scale factor of subframes. This allows a correct scale factor to be used for
* subframes when fetching bitmaps at the new scale. * subframes when fetching bitmaps at the new scale.
*/ */
void resetScaleFactorOfAllSubframes(); void updateScaleFactorOfAllSubframes(float scaleFactor);
/** /**
* Forcibly redraws the currently visible subframes. This prevents issues where a subframe won't * Forcibly redraws the currently visible subframes. This prevents issues where a subframe won't
......
...@@ -156,7 +156,7 @@ public class PlayerFrameScaleController { ...@@ -156,7 +156,7 @@ public class PlayerFrameScaleController {
*/ */
boolean scaleFinished(float scaleFactor, float focalPointX, float focalPointY) { boolean scaleFinished(float scaleFactor, float focalPointX, float focalPointY) {
// All correction/scaling happens in scaleBy() here we just update the mediator. // All correction/scaling happens in scaleBy() here we just update the mediator.
mMediatorDelegate.resetScaleFactorOfAllSubframes(); mMediatorDelegate.updateScaleFactorOfAllSubframes(mViewport.getScale());
mMediatorDelegate.updateVisuals(true); mMediatorDelegate.updateVisuals(true);
mMediatorDelegate.forceRedrawVisibleSubframes(); mMediatorDelegate.forceRedrawVisibleSubframes();
mUncommittedScaleFactor = 0f; mUncommittedScaleFactor = 0f;
......
...@@ -1109,7 +1109,7 @@ public class PlayerFrameMediatorTest { ...@@ -1109,7 +1109,7 @@ public class PlayerFrameMediatorTest {
Assert.assertTrue(mScaleController.scaleFinished(1f, 0f, 0f)); Assert.assertTrue(mScaleController.scaleFinished(1f, 0f, 0f));
mBitmapStateController.swapForTest(); mBitmapStateController.swapForTest();
inOrderMediator1.verify(subFrame1Mediator).resetScaleFactor(); inOrderMediator1.verify(subFrame1Mediator).updateScaleFactor(eq(2f));
inOrderMediator1.verify(subFrame1Mediator).forceRedraw(); inOrderMediator1.verify(subFrame1Mediator).forceRedraw();
Assert.assertEquals(expectedViews, mModel.get(PlayerFrameProperties.SUBFRAME_VIEWS)); Assert.assertEquals(expectedViews, mModel.get(PlayerFrameProperties.SUBFRAME_VIEWS));
Assert.assertEquals(expectedRects, mModel.get(PlayerFrameProperties.SUBFRAME_RECTS)); Assert.assertEquals(expectedRects, mModel.get(PlayerFrameProperties.SUBFRAME_RECTS));
...@@ -1202,8 +1202,8 @@ public class PlayerFrameMediatorTest { ...@@ -1202,8 +1202,8 @@ public class PlayerFrameMediatorTest {
.setBitmapScaleMatrixOfSubframe(argThat(new MatrixMatcher(scaleMatrix)), eq(1.5f)); .setBitmapScaleMatrixOfSubframe(argThat(new MatrixMatcher(scaleMatrix)), eq(1.5f));
// Simulate scaleFinished() by force a scale factor clear and redraw. // Simulate scaleFinished() by force a scale factor clear and redraw.
mMediator.resetScaleFactor(); mMediator.updateScaleFactor(1.5f);
inOrder.verify(subFrameMediator).resetScaleFactor(); inOrder.verify(subFrameMediator).updateScaleFactor(eq(1.5f));
mMediator.forceRedraw(); mMediator.forceRedraw();
inOrder.verify(subFrameMediator).forceRedraw(); inOrder.verify(subFrameMediator).forceRedraw();
} }
......
...@@ -109,7 +109,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -109,7 +109,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(250f, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(250f, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(350f, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(350f, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(eq(2f));
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
...@@ -132,7 +132,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -132,7 +132,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(100f, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(100f, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(150f, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(150f, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(eq(1f));
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
} }
...@@ -161,7 +161,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -161,7 +161,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(0f, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(0f, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(0f, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(0f, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(eq(2f));
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
...@@ -185,7 +185,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -185,7 +185,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(0f, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(0f, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(0f, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(0f, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(eq(1.5f));
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
} }
...@@ -214,7 +214,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -214,7 +214,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(0f, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(0f, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(0f, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(0f, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(eq(1.5f));
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
...@@ -251,7 +251,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -251,7 +251,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(expectedX, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(expectedX, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(expectedY, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(expectedY, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(eq(1.125f));
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
} }
...@@ -281,7 +281,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -281,7 +281,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(250f, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(250f, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(350f, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(350f, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(2f);
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
...@@ -305,7 +305,7 @@ public class PlayerFrameScaleControllerTest { ...@@ -305,7 +305,7 @@ public class PlayerFrameScaleControllerTest {
Assert.assertEquals(175f, mViewport.getTransX(), TOLERANCE); Assert.assertEquals(175f, mViewport.getTransX(), TOLERANCE);
Assert.assertEquals(250f, mViewport.getTransY(), TOLERANCE); Assert.assertEquals(250f, mViewport.getTransY(), TOLERANCE);
expectedBitmapMatrix.reset(); expectedBitmapMatrix.reset();
inOrder.verify(mMediatorDelegateMock).resetScaleFactorOfAllSubframes(); inOrder.verify(mMediatorDelegateMock).updateScaleFactorOfAllSubframes(1.5f);
inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true)); inOrder.verify(mMediatorDelegateMock).updateVisuals(eq(true));
inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes(); inOrder.verify(mMediatorDelegateMock).forceRedrawVisibleSubframes();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment