From 80c5f1dfa47048c82fac40e86ff7b84d4eee2539 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Wed, 10 Feb 2021 16:13:25 -0500 Subject: [PATCH 1/6] Long screenshots: Fix clipRect for ImageTile Bug: 179378294 Test: manual Change-Id: I5629edffb773854a32c77af290409a4d10ab652c --- .../SystemUI/src/com/android/systemui/screenshot/ImageTile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ImageTile.java b/packages/SystemUI/src/com/android/systemui/screenshot/ImageTile.java index 212e6c86e9da3..a95c91bfeceb3 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ImageTile.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ImageTile.java @@ -70,7 +70,7 @@ class ImageTile implements AutoCloseable { RecordingCanvas canvas = mNode.beginRecording(w, h); canvas.save(); - canvas.clipRect(0, 0, mLocation.right, mLocation.bottom); + canvas.clipRect(0, 0, mLocation.width(), mLocation.height()); canvas.drawBitmap(Bitmap.wrapHardwareBuffer(mImage.getHardwareBuffer(), COLOR_SPACE), 0, 0, null); canvas.restore(); From 97e7d1a0ea38fa53b8432b80c4fbbb69f3a3d8e1 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Wed, 10 Feb 2021 12:19:29 -0500 Subject: [PATCH 2/6] Long screenshots: dispatch listeners on UI thread Bug: 179378294 Test: manual Change-Id: Id93cfc7d3428e3b81180f0b91f156ca9c4b0eaba --- .../systemui/screenshot/ImageTileSet.java | 49 ++++++++++++++----- .../screenshot/ScrollCaptureController.java | 2 +- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java b/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java index 20f8451037233..e8493f257029b 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java @@ -21,6 +21,7 @@ import android.graphics.RecordingCanvas; import android.graphics.Rect; import android.graphics.RenderNode; import android.graphics.drawable.Drawable; +import android.os.Handler; import androidx.annotation.UiThread; @@ -32,11 +33,14 @@ import java.util.List; *

* To display on-screen, use {@link #getDrawable()}. */ -@UiThread class ImageTileSet { private static final String TAG = "ImageTileSet"; + ImageTileSet(@UiThread Handler handler) { + mHandler = handler; + } + interface OnBoundsChangedListener { /** * Reports an update to the bounding box that contains all active tiles. These are virtual @@ -54,6 +58,7 @@ class ImageTileSet { private final List mTiles = new ArrayList<>(); private final Rect mBounds = new Rect(); + private final Handler mHandler; private OnContentChangedListener mOnContentChangedListener; private OnBoundsChangedListener mOnBoundsChangedListener; @@ -73,13 +78,32 @@ class ImageTileSet { newBounds.union(newRect); if (!newBounds.equals(mBounds)) { mBounds.set(newBounds); - if (mOnBoundsChangedListener != null) { - mOnBoundsChangedListener.onBoundsChanged( - newBounds.left, newBounds.top, newBounds.right, newBounds.bottom); - } + notifyBoundsChanged(mBounds); } - if (mOnContentChangedListener != null) { + notifyContentChanged(); + } + + void notifyContentChanged() { + if (mOnContentChangedListener == null) { + return; + } + if (mHandler.getLooper().isCurrentThread()) { mOnContentChangedListener.onContentChanged(); + } else { + mHandler.post(() -> mOnContentChangedListener.onContentChanged()); + } + } + + void notifyBoundsChanged(Rect bounds) { + if (mOnBoundsChangedListener == null) { + return; + } + if (mHandler.getLooper().isCurrentThread()) { + mOnBoundsChangedListener.onBoundsChanged( + bounds.left, bounds.top, bounds.right, bounds.bottom); + } else { + mHandler.post(() -> mOnBoundsChangedListener.onBoundsChanged( + bounds.left, bounds.top, bounds.right, bounds.bottom)); } } @@ -162,14 +186,13 @@ class ImageTileSet { } void clear() { - mBounds.set(0, 0, 0, 0); + if (mBounds.isEmpty()) { + return; + } + mBounds.setEmpty(); mTiles.forEach(ImageTile::close); mTiles.clear(); - if (mOnBoundsChangedListener != null) { - mOnBoundsChangedListener.onBoundsChanged(0, 0, 0, 0); - } - if (mOnContentChangedListener != null) { - mOnContentChangedListener.onContentChanged(); - } + notifyBoundsChanged(mBounds); + notifyContentChanged(); } } diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java index 25438a6f57bae..ee9dc0e9f72d1 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java @@ -91,7 +91,7 @@ public class ScrollCaptureController implements OnComputeInternalInsetsListener mBgExecutor = bgExecutor; mImageExporter = exporter; mUiEventLogger = uiEventLogger; - mImageTileSet = new ImageTileSet(); + mImageTileSet = new ImageTileSet(context.getMainThreadHandler()); } /** From 5fa550dbeb61477df3bebf6fe988ada6f368f436 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Wed, 10 Feb 2021 12:19:29 -0500 Subject: [PATCH 3/6] Long screenshots: allow capture in reverse direction Captures in the opposite direction if an edge is detected early. This allows more useful behavior in "transcript" style UI where the content grows upward and the user is typically scrolled near the bottom edge. Bug: 179378294 Test: manually, in messages or chat Change-Id: I5c765b4e103990f428fcd24e26bd09536d613d96 --- .../screenshot/ScrollCaptureController.java | 119 ++++++++++++------ 1 file changed, 84 insertions(+), 35 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java index ee9dc0e9f72d1..6fb5b7de29bcf 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java @@ -34,6 +34,7 @@ import android.widget.ImageView; import com.android.internal.logging.UiEventLogger; import com.android.systemui.R; +import com.android.systemui.screenshot.ScrollCaptureClient.CaptureResult; import com.android.systemui.screenshot.ScrollCaptureClient.Connection; import com.android.systemui.screenshot.ScrollCaptureClient.Session; import com.android.systemui.screenshot.TakeScreenshotService.RequestCallback; @@ -44,7 +45,6 @@ import java.time.ZonedDateTime; import java.util.UUID; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; -import java.util.function.Consumer; /** * Interaction controller between the UI and ScrollCaptureClient. @@ -52,6 +52,14 @@ import java.util.function.Consumer; public class ScrollCaptureController implements OnComputeInternalInsetsListener { private static final String TAG = "ScrollCaptureController"; + private static final int UP = -1; + private static final int DOWN = 1; + + private int mDirection = DOWN; + private boolean mAtBottomEdge; + private boolean mAtTopEdge; + private Session mSession; + // TODO: Support saving without additional action. private enum PendingAction { SHARE, @@ -232,41 +240,82 @@ public class ScrollCaptureController implements OnComputeInternalInsetsListener return mWindow.findViewById(res); } + + private void onCaptureResult(CaptureResult result) { + Log.d(TAG, "onCaptureResult: " + result); + boolean emptyResult = result.captured.height() == 0; + boolean partialResult = !emptyResult + && result.captured.height() < result.requested.height(); + boolean finish = false; + + if (partialResult) { + // Potentially reached a vertical boundary. Extend in the other direction. + switch (mDirection) { + case DOWN: + Log.d(TAG, "Reached bottom edge."); + mAtBottomEdge = true; + mDirection = UP; + break; + case UP: + Log.d(TAG, "Reached top edge."); + mAtTopEdge = true; + mDirection = DOWN; + break; + } + + if (mAtTopEdge && mAtBottomEdge) { + Log.d(TAG, "Reached both top and bottom edge, ending."); + finish = true; + } else { + // only reverse if the edge was relatively close to the starting point + if (mImageTileSet.getHeight() < mSession.getPageHeight() * 3) { + Log.d(TAG, "Restarting in reverse direction."); + + // Because of temporary limitations, we cannot just jump to the opposite edge + // and continue there. Instead, clear the results and start over capturing from + // here in the other direction. + mImageTileSet.clear(); + } else { + Log.d(TAG, "Capture is tall enough, stopping here."); + finish = true; + } + } + } + + if (!emptyResult) { + mImageTileSet.addTile(new ImageTile(result.image, result.captured)); + } + + Log.d(TAG, "bounds: " + mImageTileSet.getLeft() + "," + mImageTileSet.getTop() + + " - " + mImageTileSet.getRight() + "," + mImageTileSet.getBottom() + + " (" + mImageTileSet.getWidth() + "x" + mImageTileSet.getHeight() + ")"); + + + // Stop when "too tall" + if (mImageTileSet.size() >= mSession.getMaxTiles() + || mImageTileSet.getHeight() > MAX_HEIGHT) { + Log.d(TAG, "Max height and/or tile count reached."); + finish = true; + } + + if (finish) { + Session session = mSession; + mSession = null; + Log.d(TAG, "Stop."); + mUiExecutor.execute(() -> afterCaptureComplete(session)); + return; + } + + int nextTop = (mDirection == DOWN) ? result.captured.bottom + : result.captured.top - mSession.getTileHeight(); + Log.d(TAG, "requestTile: " + nextTop); + mSession.requestTile(nextTop, /* consumer */ this::onCaptureResult); + } + private void startCapture(Session session) { - Log.d(TAG, "startCapture"); - Consumer consumer = - new Consumer() { - - int mFrameCount = 0; - int mTop = 0; - - @Override - public void accept(ScrollCaptureClient.CaptureResult result) { - mFrameCount++; - - boolean emptyFrame = result.captured.height() == 0; - if (!emptyFrame) { - ImageTile tile = new ImageTile(result.image, result.captured); - Log.d(TAG, "Adding tile: " + tile); - mImageTileSet.addTile(tile); - Log.d(TAG, "New dimens: w=" + mImageTileSet.getWidth() + ", " - + "h=" + mImageTileSet.getHeight()); - } - - if (emptyFrame || mFrameCount >= MAX_PAGES - || mTop + session.getTileHeight() > MAX_HEIGHT) { - - mUiExecutor.execute(() -> afterCaptureComplete(session)); - return; - } - mTop += result.captured.height(); - session.requestTile(mTop, /* consumer */ this); - } - }; - - // fire it up! - session.requestTile(0, consumer); - }; + mSession = session; + session.requestTile(0, this::onCaptureResult); + } @UiThread void afterCaptureComplete(Session session) { From e8d910de023b1bf0021b82351d6c914ccafbad87 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Wed, 10 Feb 2021 17:11:36 -0500 Subject: [PATCH 4/6] Long screenshots: limit batch capture to 3 pages Bug: 179378294 Test: manual Change-Id: I63d524aefb9260e1e211848a46d3ebcaeae4e575 --- .../com/android/systemui/screenshot/ScrollCaptureClient.java | 4 ++-- .../android/systemui/screenshot/ScrollCaptureController.java | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java index bb07012f23550..c8b5505a1411c 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java @@ -50,8 +50,8 @@ 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_PAGES = 5; - private static final int MAX_IMAGE_COUNT = MAX_PAGES * TILES_PER_PAGE; + private static final float MAX_PAGES = 3f; + private static final int MAX_IMAGE_COUNT = (int) Math.ceil(MAX_PAGES * TILES_PER_PAGE); @VisibleForTesting static final int MATCH_ANY_TASK = ActivityTaskManager.INVALID_TASK_ID; diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java index 6fb5b7de29bcf..44e88471db3ba 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java @@ -67,7 +67,6 @@ public class ScrollCaptureController implements OnComputeInternalInsetsListener SAVE } - public static final int MAX_PAGES = 5; public static final int MAX_HEIGHT = 12000; private final Connection mConnection; From cb077ba635fc41eaf833305b010935487c082a95 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Wed, 10 Feb 2021 21:39:57 -0500 Subject: [PATCH 5/6] Long screenshots: Fix/simplify ImageTileSet.toBitmap Bug: 179378294 Test: manual There were subtle mistakes in the drawing which caused bounds to fail when the image had tiles at negative offsets. toBitmap now delegates to the drawable impl which can already be cropped with setBounds In addition this fixes mistakes in TiledImageDrawable where translation is applied in the wrong direction and cropRect applied in the wrong position. Change-Id: Ica28e47031cb28353a49cdd657666f17115c4f6c --- .../systemui/screenshot/ImageTileSet.java | 17 ++++++----------- .../systemui/screenshot/TiledImageDrawable.java | 6 +++--- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java b/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java index e8493f257029b..ae3cd9996f044 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ImageTileSet.java @@ -22,6 +22,7 @@ import android.graphics.Rect; import android.graphics.RenderNode; import android.graphics.drawable.Drawable; import android.os.Handler; +import android.util.Log; import androidx.annotation.UiThread; @@ -141,22 +142,16 @@ class ImageTileSet { * getHeight()). */ Bitmap toBitmap(Rect bounds) { + Log.d(TAG, "exporting with bounds: " + bounds); if (mTiles.isEmpty()) { return null; } final RenderNode output = new RenderNode("Bitmap Export"); - output.setPosition(0, 0, getWidth(), getHeight()); + output.setPosition(0, 0, bounds.width(), bounds.height()); RecordingCanvas canvas = output.beginRecording(); - canvas.translate(-getLeft(), -getTop()); - // Additional translation to account for the requested bounds - canvas.translate(-bounds.left, -bounds.top); - canvas.clipRect(bounds); - for (ImageTile tile : mTiles) { - canvas.save(); - canvas.translate(tile.getLeft(), tile.getTop()); - canvas.drawRenderNode(tile.getDisplayList()); - canvas.restore(); - } + Drawable drawable = getDrawable(); + drawable.setBounds(bounds); + drawable.draw(canvas); output.endRecording(); return HardwareRenderer.createHardwareBitmap(output, bounds.width(), bounds.height()); } diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/TiledImageDrawable.java b/packages/SystemUI/src/com/android/systemui/screenshot/TiledImageDrawable.java index 72f489bdd398d..4ec8eb22c67ae 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/TiledImageDrawable.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/TiledImageDrawable.java @@ -56,7 +56,7 @@ public class TiledImageDrawable extends Drawable { mNode = new RenderNode("TiledImageDrawable"); } mNode.setPosition(0, 0, mTiles.getWidth(), mTiles.getHeight()); - Canvas canvas = mNode.beginRecording(mTiles.getWidth(), mTiles.getHeight()); + Canvas canvas = mNode.beginRecording(); // Align content (virtual) top/left with 0,0, within the render node canvas.translate(-mTiles.getLeft(), -mTiles.getTop()); for (int i = 0; i < mTiles.size(); i++) { @@ -79,8 +79,8 @@ public class TiledImageDrawable extends Drawable { if (canvas.isHardwareAccelerated()) { Rect bounds = getBounds(); canvas.save(); - canvas.clipRect(bounds); - canvas.translate(bounds.left, bounds.top); + canvas.clipRect(0, 0, bounds.width(), bounds.height()); + canvas.translate(-bounds.left, -bounds.top); canvas.drawRenderNode(mNode); canvas.restore(); } else { From 3ace01475b8f43064a83bafad213b98839829142 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Wed, 10 Feb 2021 21:48:24 -0500 Subject: [PATCH 6/6] Long screenshots: allow adjusting the max capture size Adjusts max capture size in multiples of page size example: adb shell settings put secure screenshot.scroll_max_pages 3.5 Bug: 179378294 Test: manual, adjust setting, capture long screenshot Change-Id: Id95e80ff333f8e39c07a59701b4b9f27d59ee5c9 --- .../screenshot/ScrollCaptureClient.java | 19 +++++++++++-------- .../screenshot/ScrollCaptureController.java | 8 +++++++- .../screenshot/ScrollCaptureClientTest.java | 4 +++- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java index c8b5505a1411c..d56c806554d47 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureClient.java @@ -50,8 +50,6 @@ 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 float MAX_PAGES = 3f; - private static final int MAX_IMAGE_COUNT = (int) Math.ceil(MAX_PAGES * TILES_PER_PAGE); @VisibleForTesting static final int MATCH_ANY_TASK = ActivityTaskManager.INVALID_TASK_ID; @@ -66,10 +64,11 @@ public class ScrollCaptureClient { /** * Session start should be deferred until UI is active because of resource allocation and * potential visible side effects in the target window. - + * * @param sessionConsumer listener to receive the session once active + * @param maxPages the capture buffer size expressed as a multiple of the content height */ - void start(Consumer sessionConsumer); + void start(Consumer sessionConsumer, float maxPages); /** * Close the connection. @@ -196,6 +195,7 @@ public class ScrollCaptureClient { private int mTileWidth; private Rect mRequestRect; private boolean mStarted; + private int mMaxTiles; private ControllerCallbacks(Consumer connectionConsumer) { mConnectionConsumer = connectionConsumer; @@ -285,12 +285,15 @@ public class ScrollCaptureClient { // ScrollCaptureController.Connection @Override - public void start(Consumer sessionConsumer) { + public void start(Consumer sessionConsumer, float maxPages) { if (DEBUG_SCROLL) { - Log.d(TAG, "start(sessionConsumer=" + sessionConsumer + ")"); + Log.d(TAG, "start(sessionConsumer=" + sessionConsumer + "," + + " maxPages=" + maxPages + ")" + + " [maxHeight: " + (mMaxTiles * mTileHeight) + "px]"); } + mMaxTiles = (int) Math.ceil(maxPages * TILES_PER_PAGE); mReader = ImageReader.newInstance(mTileWidth, mTileHeight, PixelFormat.RGBA_8888, - MAX_IMAGE_COUNT, HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); + mMaxTiles, HardwareBuffer.USAGE_GPU_SAMPLED_IMAGE); mSessionConsumer = sessionConsumer; try { mConnection.startCapture(mReader.getSurface()); @@ -345,7 +348,7 @@ public class ScrollCaptureClient { @Override public int getMaxTiles() { - return MAX_IMAGE_COUNT; + return mMaxTiles; } @Override diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java index 44e88471db3ba..acad28db3cd8f 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java @@ -24,6 +24,7 @@ import android.content.Intent; import android.graphics.Rect; import android.net.Uri; import android.os.UserHandle; +import android.provider.Settings; import android.text.TextUtils; import android.util.Log; import android.view.View; @@ -51,6 +52,9 @@ import java.util.concurrent.Executor; */ public class ScrollCaptureController implements OnComputeInternalInsetsListener { private static final String TAG = "ScrollCaptureController"; + private static final float MAX_PAGES_DEFAULT = 3f; + + private static final String SETTING_KEY_MAX_PAGES = "screenshot.scroll_max_pages"; private static final int UP = -1; private static final int DOWN = 1; @@ -136,7 +140,9 @@ public class ScrollCaptureController implements OnComputeInternalInsetsListener mEdit.setOnClickListener(this::onClicked); mShare.setOnClickListener(this::onClicked); - mConnection.start(this::startCapture); + float maxPages = Settings.Secure.getFloat(mContext.getContentResolver(), + SETTING_KEY_MAX_PAGES, MAX_PAGES_DEFAULT); + mConnection.start(this::startCapture, maxPages); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureClientTest.java b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureClientTest.java index c1c637129d851..580f800fbc440 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureClientTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureClientTest.java @@ -57,6 +57,8 @@ import org.mockito.stubbing.Answer; @SmallTest @RunWith(AndroidTestingRunner.class) public class ScrollCaptureClientTest extends SysuiTestCase { + private static final float MAX_PAGES = 3f; + private Context mContext; private IWindowManager mWm; @@ -96,7 +98,7 @@ public class ScrollCaptureClientTest extends SysuiTestCase { Connection conn = mConnectionConsumer.getValue(); - conn.start(mSessionConsumer); + conn.start(mSessionConsumer, MAX_PAGES); verify(mSessionConsumer, timeout(100)).accept(any(Session.class)); Session session = mSessionConsumer.getValue();