Do not cache AVDs that are off screen

Bug: 128805564
Test: Manual + systrace; hwui_unit_tests; CtsUiRenderingTestCases

Only update a VectorDrawable's cache if it is onscreen. This fixes a
Twitter use case where the app has a ProgressBar that is exactly one
pixel offscreen. Prior to this CL, we repeatedly drew the ProgressBar's
AVD to a GPU surface, even though we clip it out later and never draw
that GPU surface. Now, we recognize that the AVD is outside of the
bounds of the screen, so we never draw to the GPU surface.

TreeInfo:
- store the size of the screen, retrieved from
  CanvasContext::getNextFrameSize.
SkiaDisplayList:
- Store the matrix at the time of recording a VectorDrawable. Concat
  that with the current matrix to determine whether the VD is on screen,
  based on the TreeInfo. If it is offscreen, do not add it to the list
  of AVDs that need to be updated ahead of rendering.
- In addition, if it is offscreen (or not dirty), do not call
  setPropertyChangeWillBeConsumed(true). This prevents triggering
  dispatchFrameCallbacks to update on the RenderThread when there is no
  need to. This also mimics what would happen if the View/RenderNode had
  been completely offscreen.
- Add a method to append an AVD to mVectorDrawables. Now that the vector
  is of Pairs, this simplifies the call sites. Add a second helper to
  just add an AVD without a matrix, for use in tests.
SkiaRecordingCanvas:
- get the current matrix and store it in the display list along with the
  AVD.
CanvasContext:
- add getNextFrameSize, for reporting the size of the next frame without
  dequeuing it
VectorDrawable.cpp:
- call quickReject to potentially short circuit drawing. This is for a
  hypothetical use case (verified in a test app) where the containing
  RenderNode is partially onscreen, but the AVD itself is not. Even
  without the change to VectorDrawable.cpp, we skip uploading to the GPU
  cache, the SkiaDisplayList still attempts to draw it. This change
  keeps us from drawing it at all.
SkiaDisplayListTests.cpp:
- Now that I've hidden mVectorDrawables, call the new public APIs.
- prepareListAndChildren test
  - for the clean VD, assert that getPropertyChangeWillBeConsumed
    returns FALSE. This is due to the behavior change that we do not
    set it unless the VD is dirty.
  - set the bounds, so our onscreen check works.
- Add another test for prepareListAndChildren, which puts VDs offscreen.

Change-Id: Iae0a07adcf58e7884e0854720de644e7b2faf2bf

1 files changed, 177 insertions, 7 deletions

diff --git a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
index 1b4cf7e144bd..6fb164a99ae4 100644
--- a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
+++ b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
@@ -23,6 +23,7 @@
 #include "pipeline/skia/GLFunctorDrawable.h"
 #include "pipeline/skia/SkiaDisplayList.h"
 #include "renderthread/CanvasContext.h"
+#include "tests/common/TestContext.h"
 #include "tests/common/TestUtils.h"
 
 using namespace android;
@@ -50,13 +51,13 @@ TEST(SkiaDisplayList, reset) {
     GLFunctorDrawable functorDrawable(nullptr, nullptr, &dummyCanvas);
     skiaDL->mChildFunctors.push_back(&functorDrawable);
     skiaDL->mMutableImages.push_back(nullptr);
-    skiaDL->mVectorDrawables.push_back(nullptr);
+    skiaDL->appendVD(nullptr);
     skiaDL->mProjectionReceiver = &drawable;
 
     ASSERT_FALSE(skiaDL->mChildNodes.empty());
     ASSERT_FALSE(skiaDL->mChildFunctors.empty());
     ASSERT_FALSE(skiaDL->mMutableImages.empty());
-    ASSERT_FALSE(skiaDL->mVectorDrawables.empty());
+    ASSERT_TRUE(skiaDL->hasVectorDrawables());
     ASSERT_FALSE(skiaDL->isEmpty());
     ASSERT_TRUE(skiaDL->mProjectionReceiver);
 
@@ -65,7 +66,7 @@ TEST(SkiaDisplayList, reset) {
     ASSERT_TRUE(skiaDL->mChildNodes.empty());
     ASSERT_TRUE(skiaDL->mChildFunctors.empty());
     ASSERT_TRUE(skiaDL->mMutableImages.empty());
-    ASSERT_TRUE(skiaDL->mVectorDrawables.empty());
+    ASSERT_FALSE(skiaDL->hasVectorDrawables());
     ASSERT_TRUE(skiaDL->isEmpty());
     ASSERT_FALSE(skiaDL->mProjectionReceiver);
 }
@@ -110,7 +111,7 @@ TEST(SkiaDisplayList, syncContexts) {
     SkRect bounds = SkRect::MakeWH(200, 200);
     VectorDrawableRoot vectorDrawable(new VectorDrawable::Group());
     vectorDrawable.mutateStagingProperties()->setBounds(bounds);
-    skiaDL.mVectorDrawables.push_back(&vectorDrawable);
+    skiaDL.appendVD(&vectorDrawable);
 
     // ensure that the functor and vectorDrawable are properly synced
     TestUtils::runOnRenderThread([&](auto&) {
@@ -149,9 +150,14 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaDisplayList, prepareListAndChildren) {
 
     SkiaDisplayList skiaDL;
 
+    // The VectorDrawableRoot needs to have bounds on screen (and therefore not
+    // empty) in order to have PropertyChangeWillBeConsumed set.
+    const auto bounds = SkRect::MakeIWH(100, 100);
+
     // prepare with a clean VD
     VectorDrawableRoot cleanVD(new VectorDrawable::Group());
-    skiaDL.mVectorDrawables.push_back(&cleanVD);
+    cleanVD.mutateProperties()->setBounds(bounds);
+    skiaDL.appendVD(&cleanVD);
     cleanVD.getBitmapUpdateIfDirty();  // this clears the dirty bit
 
     ASSERT_FALSE(cleanVD.isDirty());
@@ -159,11 +165,12 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaDisplayList, prepareListAndChildren) {
     TestUtils::MockTreeObserver observer;
     ASSERT_FALSE(skiaDL.prepareListAndChildren(observer, info, false,
                                                [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
-    ASSERT_TRUE(cleanVD.getPropertyChangeWillBeConsumed());
+    ASSERT_FALSE(cleanVD.getPropertyChangeWillBeConsumed());
 
     // prepare again this time adding a dirty VD
     VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
-    skiaDL.mVectorDrawables.push_back(&dirtyVD);
+    dirtyVD.mutateProperties()->setBounds(bounds);
+    skiaDL.appendVD(&dirtyVD);
 
     ASSERT_TRUE(dirtyVD.isDirty());
     ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
@@ -191,6 +198,169 @@ RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaDisplayList, prepareListAndChildren) {
     canvasContext->destroy();
 }
 
+RENDERTHREAD_SKIA_PIPELINE_TEST(SkiaDisplayList, prepareListAndChildren_vdOffscreen) {
+    auto rootNode = TestUtils::createNode(0, 0, 200, 400, nullptr);
+    ContextFactory contextFactory;
+    std::unique_ptr<CanvasContext> canvasContext(
+            CanvasContext::create(renderThread, false, rootNode.get(), &contextFactory));
+
+    // Set up a Surface so that we can position the VectorDrawable offscreen.
+    test::TestContext testContext;
+    testContext.setRenderOffscreen(true);
+    auto surface = testContext.surface();
+    int width, height;
+    surface->query(NATIVE_WINDOW_WIDTH, &width);
+    surface->query(NATIVE_WINDOW_HEIGHT, &height);
+    canvasContext->setSurface(std::move(surface));
+
+    TreeInfo info(TreeInfo::MODE_FULL, *canvasContext.get());
+    DamageAccumulator damageAccumulator;
+    info.damageAccumulator = &damageAccumulator;
+
+    // The VectorDrawableRoot needs to have bounds on screen (and therefore not
+    // empty) in order to have PropertyChangeWillBeConsumed set.
+    const auto bounds = SkRect::MakeIWH(100, 100);
+
+    for (const SkRect b : {bounds.makeOffset(width, 0),
+                           bounds.makeOffset(0, height),
+                           bounds.makeOffset(-bounds.width(), 0),
+                           bounds.makeOffset(0, -bounds.height())}) {
+        SkiaDisplayList skiaDL;
+        VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
+        dirtyVD.mutateProperties()->setBounds(b);
+        skiaDL.appendVD(&dirtyVD);
+
+        ASSERT_TRUE(dirtyVD.isDirty());
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+
+        TestUtils::MockTreeObserver observer;
+        ASSERT_FALSE(skiaDL.prepareListAndChildren(
+                observer, info, false, [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+    }
+
+    // The DamageAccumulator's transform can also result in the
+    // VectorDrawableRoot being offscreen.
+    for (const SkISize translate : { SkISize{width, 0},
+                                     SkISize{0, height},
+                                     SkISize{-width, 0},
+                                     SkISize{0, -height}}) {
+        Matrix4 mat4;
+        mat4.translate(translate.fWidth, translate.fHeight);
+        damageAccumulator.pushTransform(&mat4);
+
+        SkiaDisplayList skiaDL;
+        VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
+        dirtyVD.mutateProperties()->setBounds(bounds);
+        skiaDL.appendVD(&dirtyVD);
+
+        ASSERT_TRUE(dirtyVD.isDirty());
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+
+        TestUtils::MockTreeObserver observer;
+        ASSERT_FALSE(skiaDL.prepareListAndChildren(
+                observer, info, false, [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+        damageAccumulator.popTransform();
+    }
+
+    // Another way to be offscreen: a matrix from the draw call.
+    for (const SkMatrix translate : { SkMatrix::MakeTrans(width, 0),
+                                      SkMatrix::MakeTrans(0, height),
+                                      SkMatrix::MakeTrans(-width, 0),
+                                      SkMatrix::MakeTrans(0, -height)}) {
+        SkiaDisplayList skiaDL;
+        VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
+        dirtyVD.mutateProperties()->setBounds(bounds);
+        skiaDL.appendVD(&dirtyVD, translate);
+
+        ASSERT_TRUE(dirtyVD.isDirty());
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+
+        TestUtils::MockTreeObserver observer;
+        ASSERT_FALSE(skiaDL.prepareListAndChildren(
+                observer, info, false, [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+    }
+
+    // Verify that the matrices are combined in the right order.
+    {
+        // Rotate and then translate, so the VD is offscreen.
+        Matrix4 mat4;
+        mat4.loadRotate(180);
+        damageAccumulator.pushTransform(&mat4);
+
+        SkiaDisplayList skiaDL;
+        VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
+        dirtyVD.mutateProperties()->setBounds(bounds);
+        SkMatrix translate = SkMatrix::MakeTrans(50, 50);
+        skiaDL.appendVD(&dirtyVD, translate);
+
+        ASSERT_TRUE(dirtyVD.isDirty());
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+
+        TestUtils::MockTreeObserver observer;
+        ASSERT_FALSE(skiaDL.prepareListAndChildren(
+                observer, info, false, [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+        damageAccumulator.popTransform();
+    }
+    {
+        // Switch the order of rotate and translate, so it is on screen.
+        Matrix4 mat4;
+        mat4.translate(50, 50);
+        damageAccumulator.pushTransform(&mat4);
+
+        SkiaDisplayList skiaDL;
+        VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
+        dirtyVD.mutateProperties()->setBounds(bounds);
+        SkMatrix rotate;
+        rotate.setRotate(180);
+        skiaDL.appendVD(&dirtyVD, rotate);
+
+        ASSERT_TRUE(dirtyVD.isDirty());
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+
+        TestUtils::MockTreeObserver observer;
+        ASSERT_TRUE(skiaDL.prepareListAndChildren(
+                observer, info, false, [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
+        ASSERT_TRUE(dirtyVD.getPropertyChangeWillBeConsumed());
+        damageAccumulator.popTransform();
+    }
+    {
+        // An AVD that is larger than the screen.
+        SkiaDisplayList skiaDL;
+        VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
+        dirtyVD.mutateProperties()->setBounds(SkRect::MakeLTRB(-1, -1, width + 1, height + 1));
+        skiaDL.appendVD(&dirtyVD);
+
+        ASSERT_TRUE(dirtyVD.isDirty());
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+
+        TestUtils::MockTreeObserver observer;
+        ASSERT_TRUE(skiaDL.prepareListAndChildren(
+                observer, info, false, [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
+        ASSERT_TRUE(dirtyVD.getPropertyChangeWillBeConsumed());
+    }
+    {
+        // An AVD whose bounds are not a rectangle after applying a matrix.
+        SkiaDisplayList skiaDL;
+        VectorDrawableRoot dirtyVD(new VectorDrawable::Group());
+        dirtyVD.mutateProperties()->setBounds(bounds);
+        SkMatrix mat;
+        mat.setRotate(45, 50, 50);
+        skiaDL.appendVD(&dirtyVD, mat);
+
+        ASSERT_TRUE(dirtyVD.isDirty());
+        ASSERT_FALSE(dirtyVD.getPropertyChangeWillBeConsumed());
+
+        TestUtils::MockTreeObserver observer;
+        ASSERT_TRUE(skiaDL.prepareListAndChildren(
+                observer, info, false, [](RenderNode*, TreeObserver&, TreeInfo&, bool) {}));
+        ASSERT_TRUE(dirtyVD.getPropertyChangeWillBeConsumed());
+    }
+}
+
 TEST(SkiaDisplayList, updateChildren) {
     SkiaDisplayList skiaDL;