commit 35f1b20840c61d70ca8e4d49c59bf6b46c1d6152
author Robert Phillips <robertphillips@google.com> Tue Dec 01 17:46:04 2020 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Wed Dec 02 13:17:26 2020 +0000
tree 7495288a72a4da99e1d40c274c479d234bf07f93
parent f0b93642dc5f461ce176fe9fe1d533fb4fe3e721

Fix bug in GrClearOp combining and remove some asserts

The buffer combining code path was combining the ops but never
telling the external system that the second op could be removed.

Bug: skia:10963
Change-Id: If015d877ffbbb75964aae9ca92ea760d7041372a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339203
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>

src/gpu/ops/GrClearOp.cpp

diff --git a/src/gpu/ops/GrClearOp.cpp b/src/gpu/ops/GrClearOp.cpp
index 8637b8f..76720a1 100644
--- a/src/gpu/ops/GrClearOp.cpp
+++ b/src/gpu/ops/GrClearOp.cpp
@@ -60,14 +60,13 @@
         // When the scissors are the exact same but the buffers are different, we can combine and
         // clear both stencil and clear together in onExecute().
         if (other->fBuffer & Buffer::kColor) {
-            SkASSERT((fBuffer & Buffer::kStencilClip) && !(fBuffer & Buffer::kColor));
             fColor = other->fColor;
         }
         if (other->fBuffer & Buffer::kStencilClip) {
-            SkASSERT(!(fBuffer & Buffer::kStencilClip) && (fBuffer & Buffer::kColor));
             fStencilInsideMask = other->fStencilInsideMask;
         }
         fBuffer = Buffer::kBoth;
+        return CombineResult::kMerged;
     }
     return CombineResult::kCannotCombine;
 }

src/gpu/ops/GrClearOp.h

diff --git a/src/gpu/ops/GrClearOp.h b/src/gpu/ops/GrClearOp.h
index 61ae9fe..d3f90aa 100644
--- a/src/gpu/ops/GrClearOp.h
+++ b/src/gpu/ops/GrClearOp.h
@@ -30,6 +30,8 @@
 
     const char* name() const override { return "Clear"; }
 
+    SkPMColor4f color() const { return fColor; }
+    bool stencilInsideMask() const { return fStencilInsideMask; }
 private:
     friend class GrOp; // for ctors
 

tests/ClearTest.cpp

diff --git a/tests/ClearTest.cpp b/tests/ClearTest.cpp
index 4cfd4f3..8f6f824 100644
--- a/tests/ClearTest.cpp
+++ b/tests/ClearTest.cpp
@@ -21,8 +21,10 @@
 #include "include/private/SkColorData.h"
 #include "src/core/SkAutoPixmapStorage.h"
 #include "src/gpu/GrColor.h"
+#include "src/gpu/GrDirectContextPriv.h"
 #include "src/gpu/GrImageInfo.h"
 #include "src/gpu/GrRenderTargetContext.h"
+#include "src/gpu/ops/GrClearOp.h"
 #include "tests/Test.h"
 #include "tools/gpu/GrContextFactory.h"
 
@@ -239,6 +241,58 @@
         ERRORF(reporter, "Expected 0x%08x but got 0x%08x at (%d, %d).", kColor1, actualValue,
                failX, failY);
     }
+
+    // Clear calls need to remain ClearOps for the following combining-tests to work as expected
+    if (!dContext->priv().caps()->performColorClearsAsDraws() &&
+        !dContext->priv().caps()->performStencilClearsAsDraws() &&
+        !dContext->priv().caps()->performPartialClearsAsDraws()) {
+        static constexpr SkIRect kScissorRect = SkIRect::MakeXYWH(1, 1, kW-1, kH-1);
+
+        // Try combining a pure-color clear w/ a combined stencil & color clear
+        // (re skbug.com/10963)
+        {
+            rtContext = newRTC(dContext, kW, kH);
+            SkASSERT(rtContext);
+
+            rtContext->clearStencilClip(kScissorRect, true);
+            // This color clear can combine w/ the preceding stencil clear
+            rtContext->clear(kScissorRect, SK_PMColor4fWHITE);
+
+            // This should combine w/ the prior combined clear and overwrite the color
+            rtContext->clear(kScissorRect, SK_PMColor4fBLACK);
+
+            GrOpsTask* ops = rtContext->getOpsTask();
+            REPORTER_ASSERT(reporter, ops->numOpChains() == 1);
+
+            const GrClearOp& clearOp = ops->getChain(0)->cast<GrClearOp>();
+
+            REPORTER_ASSERT(reporter, clearOp.color() == SK_PMColor4fBLACK);
+            REPORTER_ASSERT(reporter, clearOp.stencilInsideMask());
+        }
+
+        // Try combining a pure-stencil clear w/ a combined stencil & color clear
+        // (re skbug.com/10963)
+        {
+            rtContext = newRTC(dContext, kW, kH);
+            SkASSERT(rtContext);
+
+            rtContext->clearStencilClip(kScissorRect, true);
+            // This color clear can combine w/ the preceding stencil clear
+            rtContext->clear(kScissorRect, SK_PMColor4fWHITE);
+
+            // This should combine w/ the prior combined clear and overwrite the 'insideStencilMask'
+            // field
+            rtContext->clearStencilClip(kScissorRect, false);
+
+            GrOpsTask* ops = rtContext->getOpsTask();
+            REPORTER_ASSERT(reporter, ops->numOpChains() == 1);
+
+            const GrClearOp& clearOp = ops->getChain(0)->cast<GrClearOp>();
+
+            REPORTER_ASSERT(reporter, clearOp.color() == SK_PMColor4fWHITE);
+            REPORTER_ASSERT(reporter, !clearOp.stencilInsideMask());
+        }
+    }
 }
 
 DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ClearOp, reporter, ctxInfo) {