OSDN Git Service

Properly reject empty unclipped savelayers
authorChris Craik <ccraik@google.com>
Fri, 26 Feb 2016 00:54:08 +0000 (16:54 -0800)
committerChris Craik <ccraik@google.com>
Fri, 26 Feb 2016 00:59:19 +0000 (16:59 -0800)
bug:27225580
bug:27281241

Empty unclipped savelayers (clipped at defer time, often by dirty rect)
were resulting in invalid layer clear rectangles. Simplify by just
rejecting these unclipped savelayers entirely at defer.

Also, use repaint rect as base clip for constructed ops within
LayerBuilder.

Change-Id: I5c466199e85201a2f68f5cdc60b29187c849961b

libs/hwui/BakedOpRenderer.cpp
libs/hwui/BakedOpState.cpp
libs/hwui/BakedOpState.h
libs/hwui/FrameBuilder.cpp
libs/hwui/LayerBuilder.cpp
libs/hwui/LayerBuilder.h
libs/hwui/tests/unit/FrameBuilderTests.cpp

index 35c8f6b..5f689b4 100644 (file)
@@ -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);
         }
index a542c26..682bd04 100644 (file)
 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;
-        if (clipRect.top > clippedBounds.top) clipSideFlags |= OpClipSideFlags::Top;
-        if (clipRect.right < clippedBounds.right) clipSideFlags |= OpClipSideFlags::Right;
-        if (clipRect.bottom < clippedBounds.bottom) clipSideFlags |= OpClipSideFlags::Bottom;
+        clipSideFlags = computeClipSideFlags(clipRect, clippedBounds);
         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
index 5a5845a..4365ef8 100644 (file)
@@ -175,8 +175,8 @@ private:
             , projectionPathMask(snapshot.projectionPathMask)
             , op(shadowOpPtr) {}
 
-    BakedOpState(const ClipRect* viewportRect, const Rect& dstRect, const RecordedOp& recordedOp)
-            : computedState(viewportRect, dstRect)
+    BakedOpState(const ClipRect* clipRect, const Rect& dstRect, const RecordedOp& recordedOp)
+            : computedState(clipRect, dstRect)
             , alpha(1.0f)
             , roundRectClipState(nullptr)
             , projectionPathMask(nullptr)
index 4f51036..04de98a 100644 (file)
@@ -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)
-    OffscreenBuffer** layerHandle = mAllocator.create<OffscreenBuffer*>(nullptr);
-
-    /**
-     * First, defer an operation to copy out the content from the rendertarget into a layer.
-     */
-    auto copyToOp = mAllocator.create_trivial<CopyToLayerOp>(op, layerHandle);
-    BakedOpState* bakedState = BakedOpState::directConstruct(mAllocator,
-            &(currentLayer().viewportClip), dstRect, *copyToOp);
-    currentLayer().deferUnmergeableOp(mAllocator, bakedState, OpBatchType::CopyToLayer);
-
-    /**
-     * 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
-     */
-    currentLayer().deferLayerClear(dstRect);
-
-    /**
-     * And stash an operation to copy that layer back under the rendertarget until
-     * a balanced EndUnclippedLayerOp is seen
-     */
-    auto copyFromOp = mAllocator.create_trivial<CopyFromLayerOp>(op, layerHandle);
-    bakedState = BakedOpState::directConstruct(mAllocator,
-            &(currentLayer().viewportClip), dstRect, *copyFromOp);
-    currentLayer().activeUnclippedSaveLayers.push_back(bakedState);
+    if (dstRect.isEmpty()) {
+        // 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.
+         */
+        auto copyToOp = mAllocator.create_trivial<CopyToLayerOp>(op, layerHandle);
+        BakedOpState* bakedState = BakedOpState::directConstruct(mAllocator,
+                &(currentLayer().repaintClip), dstRect, *copyToOp);
+        currentLayer().deferUnmergeableOp(mAllocator, bakedState, OpBatchType::CopyToLayer);
+
+        /**
+         * 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
+         */
+        currentLayer().deferLayerClear(dstRect);
+
+        /**
+         * And stash an operation to copy that layer back under the rendertarget until
+         * a balanced EndUnclippedLayerOp is seen
+         */
+        auto copyFromOp = mAllocator.create_trivial<CopyFromLayerOp>(op, layerHandle);
+        bakedState = BakedOpState::directConstruct(mAllocator,
+                &(currentLayer().repaintClip), dstRect, *copyFromOp);
+        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
index c5d7191..bc39621 100644 (file)
@@ -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)
-        , viewportClip(Rect(width, height)) {}
+        , renderNode(renderNode) {}
 
 // 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);
     }
 }
index 99968e1..4a7ca2d 100644 (file)
@@ -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;
index f49dd3f..f86898f 100644 (file)
@@ -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