Commit 77785e1e authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Fix simultaneous scaling + scrolling

If scaling + scrolling happened in parallel there were some issues with
double-buffering. This CL fixes that by

- Early exiting PlayerFrameMediator#updateVisuals if in the middle of
  a scaling.
- "Locking" the visible state to prevent new bitmaps from being
  requested.

This CL also improves the logic for tracking the first set of bitmaps to
prevent a scenario where swapping never happens.

Bug: 1099722
Change-Id: Ibacffadf5690b69fb79dd872562fd84da175780a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307440Reviewed-by: default avatarMehran Mahmoudi <mahmoudi@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790061}
parent 743bae46
......@@ -72,6 +72,20 @@ public class PlayerFrameBitmapState {
return mTileSize;
}
/**
* Locks the state out of further updates.
*/
void lock() {
mRequiredBitmaps = null;
}
/**
* Returns whether this state can be updated.
*/
boolean isLocked() {
return mRequiredBitmaps == null && mBitmapMatrix != null;
}
/**
* Clears state so in-flight requests abort upon return.
*/
......@@ -128,8 +142,7 @@ public class PlayerFrameBitmapState {
for (int col = colStart; col < colEnd; col++) {
for (int row = rowStart; row < rowEnd; row++) {
requestBitmapForTile(row, col);
if (mInitialMissingVisibleBitmaps != null) {
if (requestBitmapForTile(row, col) && mInitialMissingVisibleBitmaps != null) {
mInitialMissingVisibleBitmaps.add(row * mBitmapMatrix.length + col);
}
}
......@@ -162,13 +175,13 @@ public class PlayerFrameBitmapState {
}
}
private void requestBitmapForTile(int row, int col) {
if (mRequiredBitmaps == null) return;
private boolean requestBitmapForTile(int row, int col) {
if (mRequiredBitmaps == null) return false;
mRequiredBitmaps[row][col] = true;
if (mBitmapMatrix == null || mPendingBitmapRequests == null
|| mBitmapMatrix[row][col] != null || mPendingBitmapRequests[row][col]) {
return;
return false;
}
final int y = row * mTileSize.getHeight();
......@@ -180,6 +193,7 @@ public class PlayerFrameBitmapState {
mCompositorDelegate.requestBitmap(mGuid,
new Rect(x, y, x + mTileSize.getWidth(), y + mTileSize.getHeight()), mScaleFactor,
bitmapRequestHandler, bitmapRequestHandler::onError);
return true;
}
/**
......@@ -187,7 +201,7 @@ public class PlayerFrameBitmapState {
* {@link #mRequiredBitmaps}.
*/
private void deleteUnrequiredBitmaps() {
if (mBitmapMatrix == null) return;
if (mBitmapMatrix == null || mRequiredBitmaps == null) return;
for (int row = 0; row < mBitmapMatrix.length; row++) {
for (int col = 0; col < mBitmapMatrix[row].length; col++) {
......@@ -244,8 +258,10 @@ public class PlayerFrameBitmapState {
onError();
return;
}
if (mBitmapMatrix == null || !mPendingBitmapRequests[mRequestRow][mRequestCol]
if (mBitmapMatrix == null || mPendingBitmapRequests == null || mRequiredBitmaps == null
|| !mPendingBitmapRequests[mRequestRow][mRequestCol]
|| !mRequiredBitmaps[mRequestRow][mRequestCol]) {
markBitmapReceived(mRequestRow, mRequestCol);
result.recycle();
deleteUnrequiredBitmaps();
return;
......@@ -261,6 +277,8 @@ public class PlayerFrameBitmapState {
* Called when there was an error compositing the bitmap.
*/
public void onError() {
markBitmapReceived(mRequestRow, mRequestCol);
if (mPendingBitmapRequests == null) return;
// TODO(crbug.com/1021590): Handle errors.
......@@ -269,7 +287,6 @@ public class PlayerFrameBitmapState {
assert mPendingBitmapRequests[mRequestRow][mRequestCol];
mPendingBitmapRequests[mRequestRow][mRequestCol] = false;
markBitmapReceived(mRequestRow, mRequestCol);
}
}
}
......@@ -101,6 +101,11 @@ public class PlayerFrameBitmapStateController {
return state == mVisibleBitmapState;
}
void onStartScaling() {
invalidateLoadingBitmaps();
mVisibleBitmapState.lock();
}
/**
* Invalidates loading bitmaps.
*/
......
......@@ -186,7 +186,7 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
@Override
public void onStartScaling() {
mBitmapStateController.invalidateLoadingBitmaps();
mBitmapStateController.onStartScaling();
}
@Override
......@@ -224,6 +224,12 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate, PlayerFrameMediato
final float scaleFactor = mViewport.getScale();
PlayerFrameBitmapState activeLoadingState =
mBitmapStateController.getBitmapState(scaleUpdated);
// Scaling locks the visible state from further updates. If the state is locked we
// should not progress updating anything other than |mBitmapScaleMatrix| until
// a new state is present.
if (activeLoadingState.isLocked()) return;
Rect viewportRect = mViewport.asRect();
updateSubframes(viewportRect, scaleFactor);
// Let the view know |mViewport| changed. PropertyModelChangeProcessor is smart about
......
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