commit 9548c3b42ffaefa27541c987ef34ba8491abb584
author Robert Phillips <robertphillips@google.com> Tue Jan 08 12:35:43 2019 -0500
committer Robert Phillips <robertphillips@google.com> Wed Jan 09 21:35:30 2019 +0000
tree dd937ad36a79f922bf6d80b25f74addaa11c2568
parent 532c4189b5f489ac6e4a27d43bb591cdcd4396ff

No longer round the non-AA clip bounds

By no longer rounding the non-AA clip bounds we increase the probability that a non-AA clip will be dropped when applied to a non-AA rect draw (particularly at half pixel translations).

To make this work in practice we must also make the clip stack check if the non-AA clip is relevant before we preemptively employ a scissor clip.

Note that I have verified that this removes the gross clipping errors from the Chrome repo case but the rounding to scissor clip behavior could still happen in other cases.

Bug: 906496
Change-Id: Iba51b9061fb434144e3a9b3fd91479109fcf67f4
Reviewed-on: https://skia-review.googlesource.com/c/182141
Reviewed-by: Michael Ludwig <michaelludwig@google.com>

src/gpu/GrReducedClip.cpp

diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp
index 6379cbd..1be1df7 100644
--- a/src/gpu/GrReducedClip.cpp
+++ b/src/gpu/GrReducedClip.cpp
@@ -66,7 +66,14 @@
         // "Is intersection of rects" means the clip is a single rect indicated by the stack bounds.
         // This should only be true if aa/non-aa status matches among all elements.
         SkASSERT(SkClipStack::kNormal_BoundsType == stackBoundsType);
+
+        if (GrClip::IsInsideClip(stackBounds, queryBounds)) {
+            fInitialState = InitialState::kAllIn;
+            return;
+        }
+
         SkClipStack::Iter iter(stack, SkClipStack::Iter::kTop_IterStart);
+
         if (!iter.prev()->isAA() || GrClip::IsPixelAligned(stackBounds)) {
             // The clip is a non-aa rect. Here we just implement the entire thing using fScissor.
             stackBounds.round(&fScissor);
@@ -74,10 +81,6 @@
             fInitialState = fScissor.isEmpty() ? InitialState::kAllOut : InitialState::kAllIn;
             return;
         }
-        if (GrClip::IsInsideClip(stackBounds, queryBounds)) {
-            fInitialState = InitialState::kAllIn;
-            return;
-        }
 
         SkRect tightBounds;
         SkAssertResult(tightBounds.intersect(stackBounds, queryBounds));

src/gpu/GrRenderTargetOpList.cpp

diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index b77f906..5e4db11 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -177,7 +177,7 @@
                 auto result = a->combineIfPossible(chainB.head(), caps);
                 SkASSERT(result != GrOp::CombineResult::kCannotCombine);
                 merged = (result == GrOp::CombineResult::kMerged);
-                GrOP_INFO("\t\t%d: (%s opID: %u) -> Combining with (%s, opID: %u)\n", i,
+                GrOP_INFO("\t\t: (%s opID: %u) -> Combining with (%s, opID: %u)\n",
                           chainB.head()->name(), chainB.head()->uniqueID(), a->name(),
                           a->uniqueID());
             }
@@ -254,7 +254,7 @@
                 chainA = DoConcat(std::move(chainA), std::move(chainB), caps, pool, auditTrail);
                 return std::tuple<List, List>(std::move(chainA), List());
             case GrOp::CombineResult::kMerged: {
-                GrOP_INFO("\t\t%d: (%s opID: %u) -> Combining with (%s, opID: %u)\n", i,
+                GrOP_INFO("\t\t: (%s opID: %u) -> Combining with (%s, opID: %u)\n",
                           chainB.tail()->name(), chainB.tail()->uniqueID(), chainB.head()->name(),
                           chainB.head()->uniqueID());
                 GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(auditTrail, chainA.tail(), chainB.head());