Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Long screenshot fixes for apps that return less than the requested rect

Browse Source 
- Use pixel height instead of tile count to determine ideal long screenshot height.
- After adding each tile, go back and see if there are gaps in the tile
  set, if so then try to fill in the gap before continuing walking up or
  down.
- Set max tiles to a constant as it's for system safety and not a
  completion criteria in the common cases.
- Remove ImageTileSet's unused OnBoundsChangedListener (definition got
  more complicated now that we're dealing with gaps).

Test: atest ScrollCaptureControllerTest
Test: atest FakeSessionTest
Bug: 189475409

Change-Id: I0cfc9519e7fa444d662ac41e0bef4fced785a98e
This commit is contained in:
Matt Casey
2021-06-16 15:13:18 -04:00
parent 2a7139a34f
commit 2ec7b90b51
6 changed files with 102 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java
View File

@@ -20,6 +20,7 @@ import android.graphics.Bitmap;
import android.graphics.HardwareRenderer;
import android.graphics.RecordingCanvas;
import android.graphics.Rect;
import android.graphics.Region;
import android.graphics.RenderNode;
import android.graphics.drawable.Drawable;
import android.os.Handler;
@@ -46,7 +47,6 @@ class ImageTileSet {
private static final String TAG = "ImageTileSet";
private CallbackRegistry<OnBoundsChangedListener, ImageTileSet, Rect> mOnBoundsListeners;
private CallbackRegistry<OnContentChangedListener, ImageTileSet, Rect> mContentListeners;
@Inject
@@ -54,14 +54,6 @@ class ImageTileSet {
mHandler = handler;
}
interface OnBoundsChangedListener {
/**
* Reports an update to the bounding box that contains all active tiles. These are virtual
* (capture) coordinates which can be either negative or positive.
*/
void onBoundsChanged(int left, int top, int right, int bottom);
}
interface OnContentChangedListener {
/**
* Mark as dirty and rebuild display list.
@@ -70,25 +62,9 @@ class ImageTileSet {
}
private final List<ImageTile> mTiles = new ArrayList<>();
private final Rect mBounds = new Rect();
private final Region mRegion = new Region();
private final Handler mHandler;
void addOnBoundsChangedListener(OnBoundsChangedListener listener) {
if (mOnBoundsListeners == null) {
mOnBoundsListeners = new CallbackRegistry<>(
new NotifierCallback<OnBoundsChangedListener, ImageTileSet, Rect>() {
@Override
public void onNotifyCallback(OnBoundsChangedListener callback,
ImageTileSet sender,
int arg, Rect newBounds) {
callback.onBoundsChanged(newBounds.left, newBounds.top, newBounds.right,
newBounds.bottom);
}
});
}
mOnBoundsListeners.add(listener);
}
void addOnContentChangedListener(OnContentChangedListener listener) {
if (mContentListeners == null) {
mContentListeners = new CallbackRegistry<>(
@@ -110,14 +86,8 @@ class ImageTileSet {
mHandler.post(() -> addTile(tile));
return;
}
final Rect newBounds = new Rect(mBounds);
final Rect newRect = tile.getLocation();
mTiles.add(tile);
newBounds.union(newRect);
if (!newBounds.equals(mBounds)) {
mBounds.set(newBounds);
notifyBoundsChanged(mBounds);
}
mRegion.op(tile.getLocation(), mRegion, Region.Op.UNION);
notifyContentChanged();
}
@@ -127,12 +97,6 @@ class ImageTileSet {
}
}
private void notifyBoundsChanged(Rect bounds) {
if (mOnBoundsListeners != null) {
mOnBoundsListeners.notifyCallbacks(this, 0, bounds);
}
}
/**
* Returns a drawable to paint the combined contents of the tiles. Drawable dimensions are
* zero-based and map directly to {@link #getLeft()}, {@link #getTop()}, {@link #getRight()},
@@ -153,6 +117,15 @@ class ImageTileSet {
return mTiles.size();
}
/**
* @return the bounding rect around any gaps in the tiles.
*/
Rect getGaps() {
Region difference = new Region();
difference.op(mRegion.getBounds(), mRegion, Region.Op.DIFFERENCE);
return difference.getBounds();
}
ImageTile get(int i) {
return mTiles.get(i);
}
@@ -182,41 +155,40 @@ class ImageTileSet {
}
int getLeft() {
return mBounds.left;
return mRegion.getBounds().left;
}
int getTop() {
return mBounds.top;
return mRegion.getBounds().top;
}
int getRight() {
return mBounds.right;
return mRegion.getBounds().right;
}
int getBottom() {
return mBounds.bottom;
return mRegion.getBounds().bottom;
}
int getWidth() {
return mBounds.width();
return mRegion.getBounds().width();
}
int getHeight() {
return mBounds.height();
return mRegion.getBounds().height();
}
void clear() {
if (mTiles.isEmpty()) {
return;
}
mBounds.setEmpty();
mRegion.setEmpty();
Iterator<ImageTile> i = mTiles.iterator();
while (i.hasNext()) {
ImageTile next = i.next();
next.close();
i.remove();
}
notifyBoundsChanged(mBounds);
notifyContentChanged();
}
}

23
packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java
View File

@@ -56,6 +56,7 @@ import javax.inject.Inject;
public class ScrollCaptureClient {
private static final int TILE_SIZE_PX_MAX = 4 * (1024 * 1024);
private static final int TILES_PER_PAGE = 2; // increase once b/174571735 is addressed
private static final int MAX_TILES = 30;
@VisibleForTesting
static final int MATCH_ANY_TASK = ActivityTaskManager.INVALID_TASK_ID;
@@ -83,11 +84,12 @@ public class ScrollCaptureClient {
int getMaxTiles();
/**
* @return the maximum combined capture height for this session, in pixels.
* Target pixel height for acquisition this session. Session may yield more or less data
* than this, but acquiring this height is considered sufficient for completion.
*
* @return target height in pixels.
*/
default int getMaxHeight() {
return getMaxTiles() * getTileHeight();
}
int getTargetHeight();
/**
* @return the height of each image tile
@@ -234,11 +236,11 @@ public class ScrollCaptureClient {
private final int mTileWidth;
private Rect mRequestRect;
private boolean mStarted;
private final int mTargetHeight;
private ICancellationSignal mCancellationSignal;
private final Rect mWindowBounds;
private final Rect mBoundsInWindow;
private final int mMaxTiles;
private Completer<Session> mStartCompleter;
private Completer<CaptureResult> mTileRequestCompleter;
@@ -256,7 +258,7 @@ public class ScrollCaptureClient {
mTileWidth = mBoundsInWindow.width();
mTileHeight = pxPerTile / mBoundsInWindow.width();
mMaxTiles = (int) Math.ceil(maxPages * TILES_PER_PAGE);
mTargetHeight = (int) (mBoundsInWindow.height() * maxPages);
if (DEBUG_SCROLL) {
Log.d(TAG, "boundsInWindow: " + mBoundsInWindow);
@@ -285,7 +287,7 @@ public class ScrollCaptureClient {
private void start(Completer<Session> completer) {
mReader = ImageReader.newInstance(mTileWidth, mTileHeight, PixelFormat.RGBA_8888,
mMaxTiles, HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE);
MAX_TILES, HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE);
mStartCompleter = completer;
try {
mCancellationSignal = mConnection.startCapture(mReader.getSurface(), this);
@@ -409,9 +411,14 @@ public class ScrollCaptureClient {
return new Rect(mBoundsInWindow);
}
@Override
public int getTargetHeight() {
return mTargetHeight;
}
@Override
public int getMaxTiles() {
return mMaxTiles;
return MAX_TILES;
}
}
}

34
packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java
View File

@@ -199,22 +199,20 @@ public class ScrollCaptureController {
Log.d(TAG, "onCaptureResult: " + result + " scrolling " + (mScrollingUp ? "UP" : "DOWN")
+ " finish on boundary: " + mFinishOnBoundary);
boolean emptyResult = result.captured.height() == 0;
boolean partialResult = !emptyResult
&& result.captured.height() < result.requested.height();
boolean finish = false;
if (emptyResult) {
// Potentially reached a vertical boundary. Extend in the other direction.
if (mFinishOnBoundary) {
Log.d(TAG, "Partial/empty: finished!");
finish = true;
Log.d(TAG, "Empty: finished!");
finishCapture();
return;
} else {
// We hit a boundary, clear the tiles, capture everything in the opposite direction,
// then finish.
mImageTileSet.clear();
mFinishOnBoundary = true;
mScrollingUp = !mScrollingUp;
Log.d(TAG, "Partial/empty: cleared, switch direction to finish");
Log.d(TAG, "Empty: cleared, switch direction to finish");
}
} else {
// Got a non-empty result, but may already have enough bitmap data now
@@ -223,12 +221,14 @@ public class ScrollCaptureController {
Log.d(TAG, "Hit max tiles: finished");
// If we ever hit the max tiles, we've got enough bitmap data to finish
// (even if we weren't sure we'd finish on this pass).
finish = true;
finishCapture();
return;
} else {
if (mScrollingUp && !mFinishOnBoundary) {
// During the initial scroll up, we only want to acquire the portion described
// by IDEAL_PORTION_ABOVE.
if (expectedTiles >= mSession.getMaxTiles() * IDEAL_PORTION_ABOVE) {
if (mImageTileSet.getHeight() + result.captured.height()
>= mSession.getTargetHeight() * IDEAL_PORTION_ABOVE) {
Log.d(TAG, "Hit ideal portion above: clear and switch direction");
// We got enough above the start point, now see how far down it can go.
mImageTileSet.clear();
@@ -246,15 +246,15 @@ public class ScrollCaptureController {
+ " - " + mImageTileSet.getRight() + "," + mImageTileSet.getBottom()
+ " (" + mImageTileSet.getWidth() + "x" + mImageTileSet.getHeight() + ")");
// Stop when "too tall"
if (mImageTileSet.getHeight() > MAX_HEIGHT) {
Log.d(TAG, "Max height reached.");
finish = true;
Rect gapBounds = mImageTileSet.getGaps();
if (!gapBounds.isEmpty()) {
Log.d(TAG, "Found gaps in tileset: " + gapBounds + ", requesting " + gapBounds.top);
requestNextTile(gapBounds.top);
return;
}
if (finish) {
Log.d(TAG, "Stop.");
if (mImageTileSet.getHeight() >= mSession.getTargetHeight()) {
Log.d(TAG, "Target height reached.");
finishCapture();
return;
}
@@ -268,8 +268,8 @@ public class ScrollCaptureController {
: result.requested.bottom;
} else {
nextTop = (mScrollingUp)
? result.captured.top - mSession.getTileHeight()
: result.captured.bottom;
? mImageTileSet.getTop() - mSession.getTileHeight()
: mImageTileSet.getBottom();
}
requestNextTile(nextTop);
}

12
packages/SystemUI/tests/src/com/android/systemui/screenshot/FakeSession.java
View File

@@ -57,16 +57,19 @@ class FakeSession implements ScrollCaptureClient.Session {
private int mScrollDelta;
private int mPageHeight;
private int mTargetHeight;
FakeSession(int pageHeight, float maxPages, int tileHeight, int visiblePageTop,
int visiblePageBottom, int availableTop, int availableBottom) {
int visiblePageBottom, int availableTop, int availableBottom,
int maxTiles) {
mPageHeight = pageHeight;
mTileHeight = tileHeight;
mAvailable = new Rect(0, availableTop, getPageWidth(), availableBottom);
mAvailableTop = new Rect(mAvailable);
mAvailableTop.inset(0, 0, 0, pageHeight);
mVisiblePage = new Rect(0, visiblePageTop, getPageWidth(), visiblePageBottom);
mMaxTiles = (int) Math.ceil((pageHeight * maxPages) / mTileHeight);
mTargetHeight = (int) (pageHeight * maxPages);
mMaxTiles = maxTiles;
}
private static Image mockImage() {
@@ -157,6 +160,11 @@ class FakeSession implements ScrollCaptureClient.Session {
return mMaxTiles;
}
@Override
public int getTargetHeight() {
return mTargetHeight;
}
@Override
public int getTileHeight() {
return mTileHeight;

54
packages/SystemUI/tests/src/com/android/systemui/screenshot/FakeSessionTest.java
View File

@@ -41,21 +41,6 @@ import org.junit.runner.RunWith;
@SmallTest
@RunWith(AndroidTestingRunner.class)
public class FakeSessionTest extends SysuiTestCase {
@Test
public void testMaxTiles() {
FakeSession session = new FakeSession(
/* pageHeight */ 100,
/* maxPages */ 2.25f,
/* tileHeight */ 10,
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -250,
/* availableBottom */ 250);
// (pageHeight * maxPages) / tileHeight
assertEquals("getMaxTiles()", 23, session.getMaxTiles());
}
@Test
public void testNonEmptyResult_hasImage() {
FakeSession session = new FakeSession(
@@ -65,7 +50,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ 0,
/* availableBottom */ 100);
/* availableBottom */ 100,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(0));
assertNotNull("result.image", result.image);
assertNotNull("result.image.getHardwareBuffer()", result.image.getHardwareBuffer());
@@ -80,7 +66,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ 0,
/* availableBottom */ 100);
/* availableBottom */ 100,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(-100));
assertNull("result.image", result.image);
}
@@ -94,7 +81,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -250,
/* availableBottom */ 250);
/* availableBottom */ 250,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(0));
assertEquals("requested top", 0, result.requested.top);
@@ -113,7 +101,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -250,
/* availableBottom */ 250);
/* availableBottom */ 250,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(90));
assertEquals("requested top", 90, result.requested.top);
@@ -132,7 +121,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -250,
/* availableBottom */ 250);
/* availableBottom */ 250,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(-100));
assertEquals("requested top", -100, result.requested.top);
@@ -151,7 +141,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -250,
/* availableBottom */ 250);
/* availableBottom */ 250,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(150));
assertEquals("requested top", 150, result.requested.top);
@@ -170,7 +161,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -100,
/* availableBottom */ 100);
/* availableBottom */ 100,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(-125));
assertEquals("requested top", -125, result.requested.top);
@@ -189,7 +181,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -100,
/* availableBottom */ 100);
/* availableBottom */ 100,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(75));
assertEquals("requested top", 75, result.requested.top);
@@ -211,7 +204,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 25, // <<--
/* visiblePageBottom */ 100,
/* availableTop */ -150,
/* availableBottom */ 150);
/* availableBottom */ 150,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(-150));
assertEquals("requested top", -150, result.requested.top);
@@ -233,7 +227,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 75,
/* availableTop */ -150,
/* availableBottom */ 150);
/* availableBottom */ 150,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(50));
assertEquals("requested top", 50, result.requested.top);
@@ -252,7 +247,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -100,
/* availableBottom */ 200);
/* availableBottom */ 200,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(-150));
assertTrue("captured rect is empty", result.captured.isEmpty());
}
@@ -266,7 +262,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 0,
/* visiblePageBottom */ 100,
/* availableTop */ -100,
/* availableBottom */ 200);
/* availableBottom */ 200,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(200));
assertTrue("captured rect is empty", result.captured.isEmpty());
}
@@ -280,7 +277,8 @@ public class FakeSessionTest extends SysuiTestCase {
/* visiblePageTop */ 60, // <<---
/* visiblePageBottom */ 0,
/* availableTop */ -150,
/* availableBottom */ 150);
/* availableBottom */ 150,
/* max Tiles */ 30);
ScrollCaptureClient.CaptureResult result = getUnchecked(session.requestTile(0));
assertEquals("requested top", 0, result.requested.top);

26
packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureControllerTest.java
View File

@@ -92,14 +92,16 @@ public class ScrollCaptureControllerTest extends SysuiTestCase {
// Each tile is cropped to the visible page size, which is inset 5px from the TOP
// requested result
// 0, 50 5, 50
// -45, 5 -40, 5 <-- clear previous / top
// 5, 55 5, 55 (not cropped, target is positioned fully within visible range)
// 55, 105 55, 105
// 105, 155 105, 155
// 155, 205 155, 205 <-- bottom
// -45, 5 -40, 5
// -90, -40 -85, -40 <-- clear previous / top
// -40, 10 -40, 10 (not cropped, target is positioned fully within visible range)
// 10, 60 10, 60
// 60, 110 60, 110
// 110, 160 110, 160
// 160, 210 160, 210 <-- bottom
assertEquals("top", -40, screenshot.getTop());
assertEquals("bottom", 205, screenshot.getBottom());
assertEquals("top", -85, screenshot.getTop());
assertEquals("bottom", 210, screenshot.getBottom());
}
@Test
@@ -119,13 +121,14 @@ public class ScrollCaptureControllerTest extends SysuiTestCase {
// requested result
// 0, 50 0, 50 // not cropped, positioned within visible range
// -50, 0 -50, 0 <-- clear previous/reverse
// 0, 50 - 0, 45 // target now positioned at page bottom, bottom cropped
// 0, 50 0, 45 // target now positioned at page bottom, bottom cropped
// 45, 95, 45, 90
// 90, 140, 140, 135
// 135, 185 185, 180 <-- bottom
// 135, 185 185, 180
// 180, 230 180, 225 <-- bottom
assertEquals("top", -50, screenshot.getTop());
assertEquals("bottom", 180, screenshot.getBottom());
assertEquals("bottom", 225, screenshot.getBottom());
}
@Test
@@ -265,7 +268,8 @@ public class ScrollCaptureControllerTest extends SysuiTestCase {
mLocalVisibleBottom = mPageHeight;
}
Session session = new FakeSession(mPageHeight, mMaxPages, mTileHeight,
mLocalVisibleTop, mLocalVisibleBottom, mAvailableTop, mAvailableBottom);
mLocalVisibleTop, mLocalVisibleBottom, mAvailableTop, mAvailableBottom,
/* maxTiles */ 30);
ScrollCaptureClient client = mock(ScrollCaptureClient.class);
when(client.start(/* response */ any(), /* maxPages */ anyFloat()))
.thenReturn(immediateFuture(session));