Revert "Store discard request on the opList and remove GrDiscardOp (take 2)"

This reverts commit 9b0b32fda4871776eb9afdf9553e523e5c28aa63.

Reason for revert: Android_Vulkan Gold images are broken

Original change's description:
> Store discard request on the opList and remove GrDiscardOp (take 2)
> 
> Change-Id: I2f1bd6f8547895cc8d66cfde42d7d890441d198e
> Reviewed-on: https://skia-review.googlesource.com/33460
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>

TBR=egdaniel@google.com,robertphillips@google.com

Change-Id: I1470d88c2407864da8b6ebdc119c8c2466a2a6c6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/39000
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

9 files changed, 53 insertions, 79 deletions

diff --git a/gn/gpu.gni b/gn/gpu.gni
index 57564e9fb3..51ac125366 100644
--- a/gn/gpu.gni
+++ b/gn/gpu.gni
@@ -247,6 +247,7 @@ skia_gpu_sources = [
   "$_src/gpu/ops/GrDashOp.h",
   "$_src/gpu/ops/GrDefaultPathRenderer.cpp",
   "$_src/gpu/ops/GrDefaultPathRenderer.h",
+  "$_src/gpu/ops/GrDiscardOp.h",
   "$_src/gpu/ops/GrDebugMarkerOp.h",
   "$_src/gpu/ops/GrDrawAtlasOp.cpp",
   "$_src/gpu/ops/GrDrawAtlasOp.h",
diff --git a/include/private/GrTypesPriv.h b/include/private/GrTypesPriv.h
index 41a2deaecf..b9b2cf7807 100644
--- a/include/private/GrTypesPriv.h
+++ b/include/private/GrTypesPriv.h
@@ -23,23 +23,6 @@ using GrStdSteadyClock = std::chrono::monotonic_clock;
 using GrStdSteadyClock = std::chrono::steady_clock;
 #endif
 
-/** This enum is used to specify the load operation to be used when an
- *  opList/GrGpuCommandBuffer begins execution.
- */
-enum class GrLoadOp {
-    kLoad,
-    kClear,
-    kDiscard,
-};
-
-/** This enum is used to specify the store operation to be used when an
- *  opList/GrGpuCommandBuffer ends execution.
- */
-enum class GrStoreOp {
-    kStore,
-    kDiscard,
-};
-
 /** This enum indicates the type of antialiasing to be performed. */
 enum class GrAAType : unsigned {
     /** No antialiasing */
diff --git a/src/gpu/GrGpuCommandBuffer.h b/src/gpu/GrGpuCommandBuffer.h
index a80f6e494e..07e823fd11 100644
--- a/src/gpu/GrGpuCommandBuffer.h
+++ b/src/gpu/GrGpuCommandBuffer.h
@@ -71,17 +71,28 @@ private:
  */
 class GrGpuRTCommandBuffer : public GrGpuCommandBuffer {
 public:
+    enum class LoadOp {
+        kLoad,
+        kClear,
+        kDiscard,
+    };
+
+    enum class StoreOp {
+        kStore,
+        kDiscard,
+    };
+
     struct LoadAndStoreInfo {
-        GrLoadOp  fLoadOp;
-        GrStoreOp fStoreOp;
-        GrColor   fClearColor;
+        LoadOp  fLoadOp;
+        StoreOp fStoreOp;
+        GrColor fClearColor;
     };
 
     // Load-time clears of the stencil buffer are always to 0 so we don't store
     // an 'fStencilClearValue'
     struct StencilLoadAndStoreInfo {
-        GrLoadOp  fLoadOp;
-        GrStoreOp fStoreOp;
+        LoadOp fLoadOp;
+        StoreOp fStoreOp;
     };
 
     virtual ~GrGpuRTCommandBuffer() {}
diff --git a/src/gpu/GrOpList.h b/src/gpu/GrOpList.h
index 50eaaef31f..a9fa7bdc49 100644
--- a/src/gpu/GrOpList.h
+++ b/src/gpu/GrOpList.h
@@ -8,7 +8,6 @@
 #ifndef GrOpList_DEFINED
 #define GrOpList_DEFINED
 
-#include "GrColor.h"
 #include "GrGpuResourceRef.h"
 #include "SkRefCnt.h"
 #include "SkTDArray.h"
@@ -93,6 +92,9 @@ public:
 
     int32_t uniqueID() const { return fUniqueID; }
 
+    void setRequiresStencil() { this->setFlag(kClearStencilBuffer_Flag); }
+    bool requiresStencil() { return this->isSetFlag(kClearStencilBuffer_Flag); }
+
     /*
      * Dump out the GrOpList dependency DAG
      */
@@ -101,18 +103,10 @@ public:
     SkDEBUGCODE(virtual int numOps() const = 0;)
     SkDEBUGCODE(virtual int numClips() const { return 0; })
 
-    void setColorLoadOp(GrLoadOp loadOp) { fColorLoadOp = loadOp; }
-    void setLoadClearColor(GrColor color) { fLoadClearColor = color; }
-    void setStencilLoadOp(GrLoadOp loadOp) { fStencilLoadOp = loadOp; }
-
 protected:
     GrSurfaceProxyRef fTarget;
     GrAuditTrail*     fAuditTrail;
 
-    GrLoadOp          fColorLoadOp    = GrLoadOp::kLoad;
-    GrColor           fLoadClearColor = 0x0;
-    GrLoadOp          fStencilLoadOp  = GrLoadOp::kLoad;
-
 private:
     friend class GrDrawingManager; // for resetFlag & TopoSortTraits
 
@@ -123,6 +117,8 @@ private:
 
         kWasOutput_Flag = 0x02,   //!< Flag for topological sorting
         kTempMark_Flag  = 0x04,   //!< Flag for topological sorting
+
+        kClearStencilBuffer_Flag = 0x08 //!< Clear the SB before executing the ops
     };
 
     void setFlag(uint32_t flag) {
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index d3c9d9b3f0..39515adc0a 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -31,6 +31,7 @@
 #include "ops/GrClearOp.h"
 #include "ops/GrClearStencilClipOp.h"
 #include "ops/GrDebugMarkerOp.h"
+#include "ops/GrDiscardOp.h"
 #include "ops/GrDrawAtlasOp.h"
 #include "ops/GrDrawOp.h"
 #include "ops/GrDrawVerticesOp.h"
@@ -215,25 +216,18 @@ void GrRenderTargetContext::discard() {
     ASSERT_SINGLE_OWNER
     RETURN_IF_ABANDONED
     SkDEBUGCODE(this->validate();)
-    GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContext", "discard", fContext);
+            GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContext", "discard", fContext);
 
     AutoCheckFlush acf(this->drawingManager());
 
-    // Discard calls to in-progress opLists are ignored. Calls at the start update the
-    // opLists' color & stencil load ops.
-    if (this->getRTOpList()->isEmpty()) {
-        if (this->caps()->discardRenderTargetSupport()) {
-            this->getRTOpList()->setColorLoadOp(GrLoadOp::kDiscard);
-            this->getRTOpList()->setStencilLoadOp(GrLoadOp::kDiscard);
-        } else {
-            // skbug.com/6956 (Extra clear confuses Nexus7)
-#if 0
-            // Surely, if a discard was requested, a clear should be acceptable
-            this->getRTOpList()->setColorLoadOp(GrLoadOp::kClear);
-            this->getRTOpList()->setLoadClearColor(0x0);
-            this->getRTOpList()->setStencilLoadOp(GrLoadOp::kClear);
-#endif
+    // Currently this just inserts a discard op. However, once in MDB this can remove all the
+    // previously recorded ops and change the load op to discard.
+    if (this->caps()->discardRenderTargetSupport()) {
+        std::unique_ptr<GrOp> op(GrDiscardOp::Make(fRenderTargetProxy.get()));
+        if (!op) {
+            return;
         }
+        this->getRTOpList()->addOp(std::move(op), *this->caps());
     }
 }
 
@@ -1757,7 +1751,7 @@ uint32_t GrRenderTargetContext::addDrawOp(const GrClip& clip, std::unique_ptr<Gr
 
     if (fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil ||
         appliedClip.hasStencilClip()) {
-        this->getOpList()->setStencilLoadOp(GrLoadOp::kClear);
+        this->getOpList()->setRequiresStencil();
 
         this->setNeedsStencil();
     }
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 30bd30d16b..b947891d90 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -94,13 +94,11 @@ void GrRenderTargetOpList::onPrepare(GrOpFlushState* flushState) {
 static std::unique_ptr<GrGpuRTCommandBuffer> create_command_buffer(GrGpu* gpu,
                                                                    GrRenderTarget* rt,
                                                                    GrSurfaceOrigin origin,
-                                                                   GrLoadOp colorLoadOp,
-                                                                   GrColor loadClearColor,
-                                                                   GrLoadOp stencilLoadOp) {
+                                                                   bool clearSB) {
     static const GrGpuRTCommandBuffer::LoadAndStoreInfo kBasicLoadStoreInfo {
-        colorLoadOp,
-        GrStoreOp::kStore,
-        loadClearColor
+        GrGpuRTCommandBuffer::LoadOp::kLoad,
+        GrGpuRTCommandBuffer::StoreOp::kStore,
+        GrColor_ILLEGAL
     };
 
     // TODO:
@@ -109,8 +107,8 @@ static std::unique_ptr<GrGpuRTCommandBuffer> create_command_buffer(GrGpu* gpu,
     // Note: we would still need SB loads and stores but they would happen at a
     // lower level (inside the VK command buffer).
     const GrGpuRTCommandBuffer::StencilLoadAndStoreInfo stencilLoadAndStoreInfo {
-        stencilLoadOp,
-        GrStoreOp::kStore,
+        clearSB ? GrGpuRTCommandBuffer::LoadOp::kClear : GrGpuRTCommandBuffer::LoadOp::kLoad,
+        GrGpuRTCommandBuffer::StoreOp::kStore,
     };
 
     std::unique_ptr<GrGpuRTCommandBuffer> buffer(
@@ -142,14 +140,11 @@ bool GrRenderTargetOpList::onExecute(GrOpFlushState* flushState) {
     TRACE_EVENT0("skia", TRACE_FUNC);
 #endif
 
-    // TODO: at the very least, we want the stencil store op to always be discard (at this
-    // level). In Vulkan, sub-command buffers would still need to load & store the stencil buffer.
     std::unique_ptr<GrGpuRTCommandBuffer> commandBuffer = create_command_buffer(
                                                     flushState->gpu(),
                                                     fTarget.get()->priv().peekRenderTarget(),
                                                     fTarget.get()->origin(),
-                                                    fColorLoadOp, fLoadClearColor,
-                                                    fStencilLoadOp);
+                                                    this->requiresStencil());
     flushState->setCommandBuffer(commandBuffer.get());
     commandBuffer->begin();
 
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 8e0490d729..594312071f 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2429,9 +2429,9 @@ bool GrGLGpu::onReadPixels(GrSurface* surface,
 
 GrGpuRTCommandBuffer* GrGLGpu::createCommandBuffer(
         GrRenderTarget* rt, GrSurfaceOrigin origin,
-        const GrGpuRTCommandBuffer::LoadAndStoreInfo& colorInfo,
+        const GrGpuRTCommandBuffer::LoadAndStoreInfo&,
         const GrGpuRTCommandBuffer::StencilLoadAndStoreInfo& stencilInfo) {
-    return new GrGLGpuRTCommandBuffer(this, rt, origin, colorInfo, stencilInfo);
+    return new GrGLGpuRTCommandBuffer(this, rt, origin, stencilInfo);
 }
 
 GrGpuTextureCommandBuffer* GrGLGpu::createCommandBuffer(GrTexture* texture,
diff --git a/src/gpu/gl/GrGLGpuCommandBuffer.h b/src/gpu/gl/GrGLGpuCommandBuffer.h
index f57935a1a5..e76253669c 100644
--- a/src/gpu/gl/GrGLGpuCommandBuffer.h
+++ b/src/gpu/gl/GrGLGpuCommandBuffer.h
@@ -50,22 +50,16 @@ class GrGLGpuRTCommandBuffer : public GrGpuRTCommandBuffer {
  */
 public:
     GrGLGpuRTCommandBuffer(GrGLGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin,
-                           const GrGpuRTCommandBuffer::LoadAndStoreInfo& colorInfo,
                            const GrGpuRTCommandBuffer::StencilLoadAndStoreInfo& stencilInfo)
             : INHERITED(rt, origin)
-            , fGpu(gpu)
-            , fColorLoadAndStoreInfo(colorInfo)
-            , fStencilLoadAndStoreInfo(stencilInfo) {
+            , fGpu(gpu) {
+        fClearSB = LoadOp::kClear == stencilInfo.fLoadOp;
     }
 
     ~GrGLGpuRTCommandBuffer() override {}
 
     void begin() override {
-        if (GrLoadOp::kClear == fColorLoadAndStoreInfo.fLoadOp) {
-            fGpu->clear(GrFixedClip::Disabled(), fColorLoadAndStoreInfo.fClearColor,
-                        fRenderTarget, fOrigin);
-        }
-        if (GrLoadOp::kClear == fStencilLoadAndStoreInfo.fLoadOp) {
+        if (fClearSB) {
             fGpu->clearStencil(fRenderTarget, 0x0);
         }
     }
@@ -108,9 +102,8 @@ private:
         fGpu->clearStencilClip(clip, insideStencilMask, fRenderTarget, fOrigin);
     }
 
-    GrGLGpu*                                      fGpu;
-    GrGpuRTCommandBuffer::LoadAndStoreInfo        fColorLoadAndStoreInfo;
-    GrGpuRTCommandBuffer::StencilLoadAndStoreInfo fStencilLoadAndStoreInfo;
+    GrGLGpu*                    fGpu;
+    bool                        fClearSB;
 
     typedef GrGpuRTCommandBuffer INHERITED;
 };
diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
index 1b2befff07..4987cea3b1 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
@@ -42,16 +42,17 @@ GrVkGpuTextureCommandBuffer::~GrVkGpuTextureCommandBuffer() {}
 
 ////////////////////////////////////////////////////////////////////////////////
 
-void get_vk_load_store_ops(GrLoadOp loadOpIn, GrStoreOp storeOpIn,
+void get_vk_load_store_ops(GrGpuRTCommandBuffer::LoadOp loadOpIn,
+                           GrGpuRTCommandBuffer::StoreOp storeOpIn,
                            VkAttachmentLoadOp* loadOp, VkAttachmentStoreOp* storeOp) {
     switch (loadOpIn) {
-        case GrLoadOp::kLoad:
+        case GrGpuRTCommandBuffer::LoadOp::kLoad:
             *loadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
             break;
-        case GrLoadOp::kClear:
+        case GrGpuRTCommandBuffer::LoadOp::kClear:
             *loadOp = VK_ATTACHMENT_LOAD_OP_CLEAR;
             break;
-        case GrLoadOp::kDiscard:
+        case GrGpuRTCommandBuffer::LoadOp::kDiscard:
             *loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
             break;
         default:
@@ -60,10 +61,10 @@ void get_vk_load_store_ops(GrLoadOp loadOpIn, GrStoreOp storeOpIn,
     }
 
     switch (storeOpIn) {
-        case GrStoreOp::kStore:
+        case GrGpuRTCommandBuffer::StoreOp::kStore:
             *storeOp = VK_ATTACHMENT_STORE_OP_STORE;
             break;
-        case GrStoreOp::kDiscard:
+        case GrGpuRTCommandBuffer::StoreOp::kDiscard:
             *storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
             break;
         default: