Merge "Properly reject empty unclipped savelayers" into nyc-dev

am: 2ebe8fee0c

* commit '2ebe8fee0c0cb71179b2142b990defd7045c790b':
  Properly reject empty unclipped savelayers

libs/hwui/BakedOpRenderer.cpp

@@ -347,7 +347,7 @@ void BakedOpRenderer::dirtyRenderTarget(const Rect& uiDirty) {
         if (valid) {
             dirty = android::Rect(uiDirty.left, uiDirty.top, uiDirty.right, uiDirty.bottom);
         } else {
-            dirty = android::Rect(0, 0,
+            dirty = android::Rect(
                     mRenderTarget.offscreenBuffer->viewportWidth,
                     mRenderTarget.offscreenBuffer->viewportHeight);
         }

libs/hwui/BakedOpState.cpp

@@ -21,6 +21,15 @@
 namespace android {
 namespace uirenderer {
 
+static int computeClipSideFlags(const Rect& clip, const Rect& bounds) {
+    int clipSideFlags = 0;
+    if (clip.left > bounds.left) clipSideFlags |= OpClipSideFlags::Left;
+    if (clip.top > bounds.top) clipSideFlags |= OpClipSideFlags::Top;
+    if (clip.right < bounds.right) clipSideFlags |= OpClipSideFlags::Right;
+    if (clip.bottom < bounds.bottom) clipSideFlags |= OpClipSideFlags::Bottom;
+    return clipSideFlags;
+}
+
 ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& snapshot,
         const RecordedOp& recordedOp, bool expandForStroke) {
     // resolvedMatrix = parentMatrix * localMatrix
@@ -55,10 +64,7 @@ ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& s
         clippedBounds.setEmpty();
     } else {
         // Not rejected! compute true clippedBounds and clipSideFlags
-        if (clipRect.left > clippedBounds.left) clipSideFlags |= OpClipSideFlags::Left;
+        clipSideFlags = computeClipSideFlags(clipRect, clippedBounds);
-        if (clipRect.top > clippedBounds.top) clipSideFlags |= OpClipSideFlags::Top;
-        if (clipRect.right < clippedBounds.right) clipSideFlags |= OpClipSideFlags::Right;
-        if (clipRect.bottom < clippedBounds.bottom) clipSideFlags |= OpClipSideFlags::Bottom;
         clippedBounds.doIntersect(clipRect);
     }
 }
@@ -69,11 +75,13 @@ ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& s
         , clippedBounds(clipState->rect)
         , clipSideFlags(OpClipSideFlags::Full) {}
 
-ResolvedRenderState::ResolvedRenderState(const ClipRect* viewportRect, const Rect& dstRect)
+ResolvedRenderState::ResolvedRenderState(const ClipRect* clipRect, const Rect& dstRect)
         : transform(Matrix4::identity())
-        , clipState(viewportRect)
+        , clipState(clipRect)
         , clippedBounds(dstRect)
-        , clipSideFlags(OpClipSideFlags::None) {}
+        , clipSideFlags(computeClipSideFlags(clipRect->rect, dstRect)) {
+    clippedBounds.doIntersect(clipRect->rect);
+}
 
 } // namespace uirenderer
 } // namespace android

libs/hwui/BakedOpState.h

@@ -175,8 +175,8 @@ private:
             , projectionPathMask(snapshot.projectionPathMask)
             , op(shadowOpPtr) {}
 
-    BakedOpState(const ClipRect* viewportRect, const Rect& dstRect, const RecordedOp& recordedOp)
+    BakedOpState(const ClipRect* clipRect, const Rect& dstRect, const RecordedOp& recordedOp)
-            : computedState(viewportRect, dstRect)
+            : computedState(clipRect, dstRect)
             , alpha(1.0f)
             , roundRectClipState(nullptr)
             , projectionPathMask(nullptr)

libs/hwui/FrameBuilder.cpp

@@ -782,39 +782,46 @@ void FrameBuilder::deferBeginUnclippedLayerOp(const BeginUnclippedLayerOp& op) {
     boundsTransform.mapRect(dstRect);
     dstRect.doIntersect(mCanvasState.currentSnapshot()->getRenderTargetClip());
 
-    // Allocate a holding position for the layer object (copyTo will produce, copyFrom will consume)
+    if (dstRect.isEmpty()) {
-    OffscreenBuffer** layerHandle = mAllocator.create<OffscreenBuffer*>(nullptr);
+        // Unclipped layer rejected - push a null op, so next EndUnclippedLayerOp is ignored
+        currentLayer().activeUnclippedSaveLayers.push_back(nullptr);
+    } else {
+        // Allocate a holding position for the layer object (copyTo will produce, copyFrom will consume)
+        OffscreenBuffer** layerHandle = mAllocator.create<OffscreenBuffer*>(nullptr);
 
-    /**
+        /**
-     * First, defer an operation to copy out the content from the rendertarget into a layer.
+         * First, defer an operation to copy out the content from the rendertarget into a layer.
-     */
+         */
-    auto copyToOp = mAllocator.create_trivial<CopyToLayerOp>(op, layerHandle);
+        auto copyToOp = mAllocator.create_trivial<CopyToLayerOp>(op, layerHandle);
-    BakedOpState* bakedState = BakedOpState::directConstruct(mAllocator,
+        BakedOpState* bakedState = BakedOpState::directConstruct(mAllocator,
-            &(currentLayer().viewportClip), dstRect, *copyToOp);
+                &(currentLayer().repaintClip), dstRect, *copyToOp);
-    currentLayer().deferUnmergeableOp(mAllocator, bakedState, OpBatchType::CopyToLayer);
+        currentLayer().deferUnmergeableOp(mAllocator, bakedState, OpBatchType::CopyToLayer);
 
-    /**
+        /**
-     * Defer a clear rect, so that clears from multiple unclipped layers can be drawn
+         * Defer a clear rect, so that clears from multiple unclipped layers can be drawn
-     * both 1) simultaneously, and 2) as long after the copyToLayer executes as possible
+         * both 1) simultaneously, and 2) as long after the copyToLayer executes as possible
-     */
+         */
-    currentLayer().deferLayerClear(dstRect);
+        currentLayer().deferLayerClear(dstRect);
 
-    /**
+        /**
-     * And stash an operation to copy that layer back under the rendertarget until
+         * And stash an operation to copy that layer back under the rendertarget until
-     * a balanced EndUnclippedLayerOp is seen
+         * a balanced EndUnclippedLayerOp is seen
-     */
+         */
-    auto copyFromOp = mAllocator.create_trivial<CopyFromLayerOp>(op, layerHandle);
+        auto copyFromOp = mAllocator.create_trivial<CopyFromLayerOp>(op, layerHandle);
-    bakedState = BakedOpState::directConstruct(mAllocator,
+        bakedState = BakedOpState::directConstruct(mAllocator,
-            &(currentLayer().viewportClip), dstRect, *copyFromOp);
+                &(currentLayer().repaintClip), dstRect, *copyFromOp);
-    currentLayer().activeUnclippedSaveLayers.push_back(bakedState);
+        currentLayer().activeUnclippedSaveLayers.push_back(bakedState);
+    }
 }
 
 void FrameBuilder::deferEndUnclippedLayerOp(const EndUnclippedLayerOp& /* ignored */) {
     LOG_ALWAYS_FATAL_IF(currentLayer().activeUnclippedSaveLayers.empty(), "no layer to end!");
 
     BakedOpState* copyFromLayerOp = currentLayer().activeUnclippedSaveLayers.back();
-    currentLayer().deferUnmergeableOp(mAllocator, copyFromLayerOp, OpBatchType::CopyFromLayer);
     currentLayer().activeUnclippedSaveLayers.pop_back();
+    if (copyFromLayerOp) {
+        currentLayer().deferUnmergeableOp(mAllocator, copyFromLayerOp, OpBatchType::CopyFromLayer);
+    }
 }
 
 } // namespace uirenderer

libs/hwui/LayerBuilder.cpp

@@ -199,10 +199,10 @@ LayerBuilder::LayerBuilder(uint32_t width, uint32_t height,
         : width(width)
         , height(height)
         , repaintRect(repaintRect)
+        , repaintClip(repaintRect)
         , offscreenBuffer(renderNode ? renderNode->getLayer() : nullptr)
         , beginLayerOp(beginLayerOp)
-        , renderNode(renderNode)
+        , renderNode(renderNode) {}
-        , viewportClip(Rect(width, height)) {}
 
 // iterate back toward target to see if anything drawn since should overlap the new op
 // if no target, merging ops still iterate to find similar batch to insert after
@@ -260,7 +260,7 @@ void LayerBuilder::flushLayerClears(LinearAllocator& allocator) {
                 Matrix4::identity(), nullptr, paint,
                 verts, vertCount);
         BakedOpState* bakedState = BakedOpState::directConstruct(allocator,
-                &viewportClip, bounds, *op);
+                &repaintClip, bounds, *op);
         deferUnmergeableOp(allocator, bakedState, OpBatchType::Vertices);
     }
 }

libs/hwui/LayerBuilder.h

@@ -109,10 +109,10 @@ public:
     const uint32_t width;
     const uint32_t height;
     const Rect repaintRect;
+    const ClipRect repaintClip;
     OffscreenBuffer* offscreenBuffer;
     const BeginLayerOp* beginLayerOp;
     const RenderNode* renderNode;
-    const ClipRect viewportClip;
 
     // list of deferred CopyFromLayer ops, to be deferred upon encountering EndUnclippedLayerOps
     std::vector<BakedOpState*> activeUnclippedSaveLayers;

libs/hwui/tests/unit/FrameBuilderTests.cpp

@@ -651,6 +651,62 @@ TEST(FrameBuilder, saveLayerUnclipped_mergedClears) {
             << "Expect 4 copyTos, 4 copyFroms, 1 clear SimpleRects, and 1 rect.";
 }
 
+TEST(FrameBuilder, saveLayerUnclipped_clearClip) {
+    class SaveLayerUnclippedClearClipTestRenderer : public TestRendererBase {
+    public:
+        void onCopyToLayerOp(const CopyToLayerOp& op, const BakedOpState& state) override {
+            EXPECT_EQ(0, mIndex++);
+        }
+        void onSimpleRectsOp(const SimpleRectsOp& op, const BakedOpState& state) override {
+            EXPECT_EQ(1, mIndex++);
+            ASSERT_NE(nullptr, op.paint);
+            EXPECT_EQ(SkXfermode::kClear_Mode, PaintUtils::getXfermodeDirect(op.paint));
+            EXPECT_EQ(Rect(50, 50, 150, 150), state.computedState.clippedBounds)
+                    << "Expect dirty rect as clip";
+            ASSERT_NE(nullptr, state.computedState.clipState);
+            EXPECT_EQ(Rect(50, 50, 150, 150), state.computedState.clipState->rect);
+            EXPECT_EQ(ClipMode::Rectangle, state.computedState.clipState->mode);
+        }
+        void onRectOp(const RectOp& op, const BakedOpState& state) override {
+            EXPECT_EQ(2, mIndex++);
+        }
+        void onCopyFromLayerOp(const CopyFromLayerOp& op, const BakedOpState& state) override {
+            EXPECT_EQ(3, mIndex++);
+        }
+    };
+
+    auto node = TestUtils::createNode(0, 0, 200, 200,
+            [](RenderProperties& props, RecordingCanvas& canvas) {
+        // save smaller than clip, so we get unclipped behavior
+        canvas.saveLayerAlpha(10, 10, 190, 190, 128, (SaveFlags::Flags)(0));
+        canvas.drawRect(0, 0, 200, 200, SkPaint());
+        canvas.restore();
+    });
+
+    // draw with partial screen dirty, and assert we see that rect later
+    FrameBuilder frameBuilder(sEmptyLayerUpdateQueue, SkRect::MakeLTRB(50, 50, 150, 150), 200, 200,
+            TestUtils::createSyncedNodeList(node), sLightGeometry, nullptr);
+    SaveLayerUnclippedClearClipTestRenderer renderer;
+    frameBuilder.replayBakedOps<TestDispatcher>(renderer);
+    EXPECT_EQ(4, renderer.getIndex());
+}
+
+TEST(FrameBuilder, saveLayerUnclipped_reject) {
+    auto node = TestUtils::createNode(0, 0, 200, 200,
+            [](RenderProperties& props, RecordingCanvas& canvas) {
+        // unclipped savelayer + rect both in area that won't intersect with dirty
+        canvas.saveLayerAlpha(100, 100, 200, 200, 128, (SaveFlags::Flags)(0));
+        canvas.drawRect(100, 100, 200, 200, SkPaint());
+        canvas.restore();
+    });
+
+    // draw with partial screen dirty that doesn't intersect with savelayer
+    FrameBuilder frameBuilder(sEmptyLayerUpdateQueue, SkRect::MakeWH(100, 100), 200, 200,
+            TestUtils::createSyncedNodeList(node), sLightGeometry, nullptr);
+    FailRenderer renderer;
+    frameBuilder.replayBakedOps<TestDispatcher>(renderer);
+}
+
 /* saveLayerUnclipped { saveLayer { saveLayerUnclipped { rect } } } will play back as:
  * - startTemporaryLayer, onCopyToLayer, onSimpleRects, onRect, onCopyFromLayer, endLayer
  * - startFrame, onCopyToLayer, onSimpleRects, drawLayer, onCopyFromLayer, endframe