commit 7fc1b0349bc2ac8c880120dc5611f703faa7f06f
author Chris Craik <ccraik@google.com> Wed Feb 03 19:45:06 2016 -0800
committer Chris Craik <ccraik@google.com> Wed Feb 03 20:09:00 2016 -0800
tree 6eb28dc729ca8fcecc3be81bffb27828e32052e9
parent d38308e4d0599836f9c5446ba9d6edbc0713c428

Fix ripple clipping + quick rejection

bug:26524690

Don't intersect the first clip with the viewport. Instead, the first
clip op should always be a replace op.

Additionally, only quick reject nodes that clip to bounds, since some
nodes (like ripples) draw outside their own bounds.

Change-Id: I96a52029f360328aba19af7349888cc0a026b5b1

libs/hwui/ClipArea.cpp

diff --git a/libs/hwui/ClipArea.cpp b/libs/hwui/ClipArea.cpp
index 160090d..9c08b4d 100644
--- a/libs/hwui/ClipArea.cpp
+++ b/libs/hwui/ClipArea.cpp
@@ -213,6 +213,7 @@
 
 void ClipArea::clipRectWithTransform(const Rect& r, const mat4* transform,
         SkRegion::Op op) {
+    if (!mPostViewportClipObserved && op == SkRegion::kIntersect_Op) op = SkRegion::kReplace_Op;
     onClipUpdated();
     switch (mMode) {
     case ClipMode::Rectangle:
@@ -228,6 +229,7 @@
 }
 
 void ClipArea::clipRegion(const SkRegion& region, SkRegion::Op op) {
+    if (!mPostViewportClipObserved && op == SkRegion::kIntersect_Op) op = SkRegion::kReplace_Op;
     onClipUpdated();
     enterRegionMode();
     mClipRegion.op(region, op);
@@ -236,6 +238,7 @@
 
 void ClipArea::clipPathWithTransform(const SkPath& path, const mat4* transform,
         SkRegion::Op op) {
+    if (!mPostViewportClipObserved && op == SkRegion::kIntersect_Op) op = SkRegion::kReplace_Op;
     onClipUpdated();
     SkMatrix skTransform;
     transform->copyTo(skTransform);

libs/hwui/FrameBuilder.cpp

diff --git a/libs/hwui/FrameBuilder.cpp b/libs/hwui/FrameBuilder.cpp
index a457212..1c765f6 100644
--- a/libs/hwui/FrameBuilder.cpp
+++ b/libs/hwui/FrameBuilder.cpp
@@ -208,7 +208,9 @@
         mCanvasState.setClippingOutline(mAllocator, &(properties.getOutline()));
     }
 
-    if (!mCanvasState.quickRejectConservative(0, 0, width, height)) {
+    bool quickRejected = properties.getClipToBounds()
+            && mCanvasState.quickRejectConservative(0, 0, width, height);
+    if (!quickRejected) {
         // not rejected, so defer render as either Layer, or direct (possibly wrapped in saveLayer)
         if (node.getLayer()) {
             // HW layer

libs/hwui/tests/unit/RecordingCanvasTests.cpp

diff --git a/libs/hwui/tests/unit/RecordingCanvasTests.cpp b/libs/hwui/tests/unit/RecordingCanvasTests.cpp
index 01bfc5a..20d2f1f 100644
--- a/libs/hwui/tests/unit/RecordingCanvasTests.cpp
+++ b/libs/hwui/tests/unit/RecordingCanvasTests.cpp
@@ -455,6 +455,23 @@
     }
 }
 
+TEST(RecordingCanvas, firstClipWillReplace) {
+    auto dl = TestUtils::createDisplayList<RecordingCanvas>(200, 200, [](RecordingCanvas& canvas) {
+        canvas.save(SaveFlags::MatrixClip);
+        // since no explicit clip set on canvas, this should be the one observed on op:
+        canvas.clipRect(-100, -100, 300, 300, SkRegion::kIntersect_Op);
+
+        SkPaint paint;
+        paint.setColor(SK_ColorWHITE);
+        canvas.drawRect(0, 0, 100, 100, paint);
+
+        canvas.restore();
+    });
+    ASSERT_EQ(1u, dl->getOps().size()) << "Must have one op";
+    // first clip must be preserved, even if it extends beyond canvas bounds
+    EXPECT_CLIP_RECT(Rect(-100, -100, 300, 300), dl->getOps()[0]->localClip);
+}
+
 TEST(RecordingCanvas, insertReorderBarrier) {
     auto dl = TestUtils::createDisplayList<RecordingCanvas>(200, 200, [](RecordingCanvas& canvas) {
         canvas.drawRect(0, 0, 400, 400, SkPaint());