Commit 21f13f1a authored by Mehran Mahmoudi's avatar Mehran Mahmoudi Committed by Commit Bot

[Paint Preview] Fix UI bugs in the player

This fixes the following problems:
1. NullPointerException in PlayerFrameBitmapPainter#onDraw() for when
  mBitmapMatrix contains null bitmaps.
2. Scrolling using GestureDeterctor. PlayerFrameGestureDetector#onDown()
  should return true.
3. No calls for PlayerFrameView#onDraw(). setWillNotDraw(false) should
  be called.
4. PlayerFrameView not getting notified of changes to the model.
  Because we reuse the same objects for the PropertyModel in the
  PlayerFrameMediator class, PropertyModelChangeProcessor doesn't
  notify the model when they are reset with the same object.

Change-Id: I73fe4e4d3299a8dd3aa3fc74b83462ea6c82b27a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2072909
Commit-Queue: Mehran Mahmoudi <mahmoudi@chromium.org>
Auto-Submit: Mehran Mahmoudi <mahmoudi@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744762}
parent 5d90823b
...@@ -60,6 +60,9 @@ class PlayerFrameBitmapPainter { ...@@ -60,6 +60,9 @@ class PlayerFrameBitmapPainter {
for (int row = rowStart; row < rowEnd; row++) { for (int row = rowStart; row < rowEnd; row++) {
for (int col = colStart; col < colEnd; col++) { for (int col = colStart; col < colEnd; col++) {
Bitmap tileBitmap = mBitmapMatrix[row][col]; Bitmap tileBitmap = mBitmapMatrix[row][col];
if (tileBitmap == null) {
continue;
}
// Calculate the portion of this tileBitmap that is visible in mViewPort. // Calculate the portion of this tileBitmap that is visible in mViewPort.
int bitmapLeft = Math.max(mViewPort.left - (col * tileWidth), 0); int bitmapLeft = Math.max(mViewPort.left - (col * tileWidth), 0);
......
...@@ -49,7 +49,7 @@ class PlayerFrameGestureDetector ...@@ -49,7 +49,7 @@ class PlayerFrameGestureDetector
@Override @Override
public boolean onDown(MotionEvent e) { public boolean onDown(MotionEvent e) {
return false; return true;
} }
@Override @Override
......
...@@ -20,19 +20,19 @@ import java.util.List; ...@@ -20,19 +20,19 @@ import java.util.List;
class PlayerFrameProperties { class PlayerFrameProperties {
/** A matrix of bitmap tiles that collectively make the entire content. */ /** A matrix of bitmap tiles that collectively make the entire content. */
static final PropertyModel.WritableObjectPropertyKey<Bitmap[][]> BITMAP_MATRIX = static final PropertyModel.WritableObjectPropertyKey<Bitmap[][]> BITMAP_MATRIX =
new PropertyModel.WritableObjectPropertyKey<>(); new PropertyModel.WritableObjectPropertyKey<>(true);
/** /**
* Contains the current user-visible content window. The view should use this to draw the * Contains the current user-visible content window. The view should use this to draw the
* appropriate bitmap tiles from {@link #BITMAP_MATRIX}. * appropriate bitmap tiles from {@link #BITMAP_MATRIX}.
*/ */
static final PropertyModel.WritableObjectPropertyKey<Rect> VIEWPORT = static final PropertyModel.WritableObjectPropertyKey<Rect> VIEWPORT =
new PropertyModel.WritableObjectPropertyKey<>(); new PropertyModel.WritableObjectPropertyKey<>(true);
/** /**
* A list of sub-frames that are currently visible. Each element in the list is a {@link Pair} * A list of sub-frames that are currently visible. Each element in the list is a {@link Pair}
* consists of a {@link View}, that displays the sub-frame's content, and a {@link Rect}, that * consists of a {@link View}, that displays the sub-frame's content, and a {@link Rect}, that
* contains its location. * contains its location.
*/ */
static final PropertyModel.WritableObjectPropertyKey<List<Pair<View, Rect>>> SUBFRAME_VIEWS = static final PropertyModel.WritableObjectPropertyKey<List<Pair<View, Rect>>> SUBFRAME_VIEWS =
new PropertyModel.WritableObjectPropertyKey<>(); new PropertyModel.WritableObjectPropertyKey<>(true);
static final PropertyKey[] ALL_KEYS = {BITMAP_MATRIX, VIEWPORT, SUBFRAME_VIEWS}; static final PropertyKey[] ALL_KEYS = {BITMAP_MATRIX, VIEWPORT, SUBFRAME_VIEWS};
} }
...@@ -36,6 +36,7 @@ class PlayerFrameView extends FrameLayout { ...@@ -36,6 +36,7 @@ class PlayerFrameView extends FrameLayout {
PlayerFrameView(@NonNull Context context, boolean canDetectZoom, PlayerFrameView(@NonNull Context context, boolean canDetectZoom,
PlayerFrameViewDelegate playerFrameViewDelegate) { PlayerFrameViewDelegate playerFrameViewDelegate) {
super(context); super(context);
setWillNotDraw(false);
mDelegate = playerFrameViewDelegate; mDelegate = playerFrameViewDelegate;
mBitmapPainter = new PlayerFrameBitmapPainter(this::invalidate); mBitmapPainter = new PlayerFrameBitmapPainter(this::invalidate);
mGestureDetector = mGestureDetector =
......
...@@ -80,6 +80,16 @@ public class PlayerFrameBitmapPainterTest { ...@@ -80,6 +80,16 @@ public class PlayerFrameBitmapPainterTest {
} }
} }
private Bitmap[][] generateMockBitmapMatrix(int rows, int cols) {
Bitmap[][] matrix = new Bitmap[rows][cols];
for (int row = 0; row < matrix.length; ++row) {
for (int col = 0; col < matrix[row].length; ++col) {
matrix[row][col] = Mockito.mock(Bitmap.class);
}
}
return matrix;
}
/** /**
* Verifies no draw operations are performed on the canvas if the view port is invalid. * Verifies no draw operations are performed on the canvas if the view port is invalid.
*/ */
...@@ -87,7 +97,7 @@ public class PlayerFrameBitmapPainterTest { ...@@ -87,7 +97,7 @@ public class PlayerFrameBitmapPainterTest {
public void testDrawFaultyViewPort() { public void testDrawFaultyViewPort() {
PlayerFrameBitmapPainter painter = PlayerFrameBitmapPainter painter =
new PlayerFrameBitmapPainter(Mockito.mock(Runnable.class)); new PlayerFrameBitmapPainter(Mockito.mock(Runnable.class));
painter.updateBitmapMatrix(new Bitmap[2][3]); painter.updateBitmapMatrix(generateMockBitmapMatrix(2, 3));
painter.updateViewPort(0, 5, 10, -10); painter.updateViewPort(0, 5, 10, -10);
MockCanvas canvas = new MockCanvas(); MockCanvas canvas = new MockCanvas();
...@@ -116,7 +126,7 @@ public class PlayerFrameBitmapPainterTest { ...@@ -116,7 +126,7 @@ public class PlayerFrameBitmapPainterTest {
painter.onDraw(canvas); painter.onDraw(canvas);
canvas.assertNumberOfBitmapDraws(0); canvas.assertNumberOfBitmapDraws(0);
painter.updateBitmapMatrix(new Bitmap[2][1]); painter.updateBitmapMatrix(generateMockBitmapMatrix(2, 1));
painter.onDraw(canvas); painter.onDraw(canvas);
canvas.assertNumberOfBitmapDraws(2); canvas.assertNumberOfBitmapDraws(2);
} }
......
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