From be15324c5d5be2130d9dbad583848d7fa50223bc Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Tue, 30 Mar 2021 12:29:36 -0400 Subject: [PATCH 1/2] Long screenshots: continue after partial tile result Capture code will return partial (cropped) results in some situations. This change accepts a partial result and attempts to continue until an empty result is returned. Bug: 180390131 Test: atest ScrollCaptureControllerTest Change-Id: Id36765d65347d81e6a70da739f0d481988af9cc4 --- .../screenshot/ScrollCaptureController.java | 24 ++++++--- .../ScrollCaptureControllerTest.java | 52 +++++++++++++++++++ 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java index 3dc8ae8ada9fd..bbcfdbd99bef1 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScrollCaptureController.java @@ -203,7 +203,7 @@ public class ScrollCaptureController { && result.captured.height() < result.requested.height(); boolean finish = false; - if (partialResult || emptyResult) { + if (emptyResult) { // Potentially reached a vertical boundary. Extend in the other direction. if (mFinishOnBoundary) { Log.d(TAG, "Partial/empty: finished!"); @@ -217,12 +217,12 @@ public class ScrollCaptureController { Log.d(TAG, "Partial/empty: cleared, switch direction to finish"); } } else { - // Got the full requested result, but may have got enough bitmap data now + // Got a non-empty result, but may already have enough bitmap data now int expectedTiles = mImageTileSet.size() + 1; if (expectedTiles >= mSession.getMaxTiles()) { 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). + // 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; } else { if (mScrollingUp && !mFinishOnBoundary) { @@ -259,10 +259,18 @@ public class ScrollCaptureController { return; } - // Partial or empty results caused the direction the flip, so we can reliably use the - // requested edges to determine the next top. - int nextTop = (mScrollingUp) ? result.requested.top - mSession.getTileHeight() - : result.requested.bottom; + int nextTop; + if (emptyResult) { + // An empty result caused the direction the flip, + // so use the requested edges to determine the next top. + nextTop = (mScrollingUp) + ? result.requested.top - mSession.getTileHeight() + : result.requested.bottom; + } else { + nextTop = (mScrollingUp) + ? result.captured.top - mSession.getTileHeight() + : result.captured.bottom; + } requestNextTile(nextTop); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureControllerTest.java index 1c32cd4b612a5..5bab1bcddb6a0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureControllerTest.java @@ -76,6 +76,58 @@ public class ScrollCaptureControllerTest extends SysuiTestCase { } } + @Test + public void testInfiniteWithPartialResultsTop() { + ScrollCaptureController controller = new TestScenario() + .withPageHeight(100) + .withPageVisibleRange(5, 100) // <-- simulate 5px of invisible top + .withMaxPages(2.5f) + .withTileHeight(50) + .withAvailableRange(Integer.MIN_VALUE, Integer.MAX_VALUE) + .createController(mContext); + + ScrollCaptureController.LongScreenshot screenshot = + getUnchecked(controller.run(EMPTY_RESPONSE)); + + // 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 + + assertEquals("top", -40, screenshot.getTop()); + assertEquals("bottom", 205, screenshot.getBottom()); + } + + @Test + public void testInfiniteWithPartialResultsBottom() { + ScrollCaptureController controller = new TestScenario() + .withPageHeight(100) + .withPageVisibleRange(0, 95) // <-- simulate 5px of invisible bottom + .withMaxPages(2.5f) + .withTileHeight(50) + .withAvailableRange(Integer.MIN_VALUE, Integer.MAX_VALUE) + .createController(mContext); + + ScrollCaptureController.LongScreenshot screenshot = + getUnchecked(controller.run(EMPTY_RESPONSE)); + + // Each tile is cropped to the visible page size, which is inset 5px from the BOTTOM + // 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 + // 45, 95, 45, 90 + // 90, 140, 140, 135 + // 135, 185 185, 180 <-- bottom + + assertEquals("top", -50, screenshot.getTop()); + assertEquals("bottom", 180, screenshot.getBottom()); + } + @Test public void testLimitedBottom() { ScrollCaptureController controller = new TestScenario() From 869128526bf64b2fd2f8cae235442eaf84534d51 Mon Sep 17 00:00:00 2001 From: Mark Renouf Date: Fri, 2 Apr 2021 14:55:24 -0400 Subject: [PATCH 2/2] Improve logging in ScrollCaptureFrameworkSmokeTest In the event of failure, this change prints some very useful information such as, why no connection was returned and potentially the name of the window which was investigated. Bug: 184117155 Test: atest ScrollCaptureFrameworkSmokeTest Change-Id: Iecf5ef2d22d00773a08d0b2605391a4db0be9b21 --- .../screenshot/ScrollCaptureFrameworkSmokeTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureFrameworkSmokeTest.java b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureFrameworkSmokeTest.java index 06b39abd1d0e7..54d973266ff3d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureFrameworkSmokeTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/ScrollCaptureFrameworkSmokeTest.java @@ -82,7 +82,11 @@ public class ScrollCaptureFrameworkSmokeTest extends SysuiTestCase { latch.await(1000, TimeUnit.MILLISECONDS); assertNotNull(mResponse); - assertTrue(mResponse.isConnected()); - assertTrue(mResponse.getWindowTitle().contains(ScrollViewActivity.class.getSimpleName())); + if (!mResponse.isConnected()) { + Log.e(TAG, "Received response with no connection: " + mResponse); + fail("expected response.isConnected() == true"); + } + assertTrue("expected a connection to ScrollViewActivity", + mResponse.getWindowTitle().contains(ScrollViewActivity.class.getSimpleName())); } }