Revert "Store discard request on the opList and remove GrDiscardOp"

This reverts commit b681a0f1b0acebe36130fd463d14016d48295b97.

Reason for revert: Seems to be messing up some MacMini & Nexus7 bots
Original change's description:
> Store discard request on the opList and remove GrDiscardOp
> 
> Change-Id: Ic1f76bb91c16b23df1fe71c07a4d5ad5abf1dc26
> Reviewed-on: https://skia-review.googlesource.com/32640
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>

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

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

8 files changed, 101 insertions, 61 deletions

diff --git a/src/gpu/GrGpuCommandBuffer.h b/src/gpu/GrGpuCommandBuffer.h
index 9e3c00ba5b..31e9a546d5 100644
--- a/src/gpu/GrGpuCommandBuffer.h
+++ b/src/gpu/GrGpuCommandBuffer.h
@@ -30,17 +30,28 @@ struct SkRect;
  */
 class 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;
     };
 
     GrGpuCommandBuffer(GrRenderTarget* rt, GrSurfaceOrigin origin)
diff --git a/src/gpu/GrOpList.h b/src/gpu/GrOpList.h
index b45ecce190..00dc344a07 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"
@@ -83,6 +82,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
      */
@@ -91,18 +93,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
 
@@ -113,6 +107,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 48cea29d71..e76b5ff26e 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,22 +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 {
-            // 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);
+    // 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());
     }
 }
 
@@ -1764,7 +1761,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 forces instantiation of the render target.
         GrRenderTarget* rt = this->accessRenderTarget();
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 819e8a58c0..ffa1e43746 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -87,13 +87,11 @@ void GrRenderTargetOpList::prepareOps(GrOpFlushState* flushState) {
 static std::unique_ptr<GrGpuCommandBuffer> create_command_buffer(GrGpu* gpu,
                                                                  GrRenderTarget* rt,
                                                                  GrSurfaceOrigin origin,
-                                                                 GrLoadOp colorLoadOp,
-                                                                 GrColor loadClearColor,
-                                                                 GrLoadOp stencilLoadOp) {
+                                                                 bool clearSB) {
     static const GrGpuCommandBuffer::LoadAndStoreInfo kBasicLoadStoreInfo {
-        colorLoadOp,
-        GrStoreOp::kStore,
-        loadClearColor
+        GrGpuCommandBuffer::LoadOp::kLoad,
+        GrGpuCommandBuffer::StoreOp::kStore,
+        GrColor_ILLEGAL
     };
 
     // TODO:
@@ -102,8 +100,8 @@ static std::unique_ptr<GrGpuCommandBuffer> 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 GrGpuCommandBuffer::StencilLoadAndStoreInfo stencilLoadAndStoreInfo {
-        stencilLoadOp,
-        GrStoreOp::kStore,
+        clearSB ? GrGpuCommandBuffer::LoadOp::kClear : GrGpuCommandBuffer::LoadOp::kLoad,
+        GrGpuCommandBuffer::StoreOp::kStore,
     };
 
     std::unique_ptr<GrGpuCommandBuffer> buffer(
@@ -132,14 +130,11 @@ bool GrRenderTargetOpList::executeOps(GrOpFlushState* flushState) {
 
     SkASSERT(fTarget.get()->priv().peekRenderTarget());
 
-    // 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<GrGpuCommandBuffer> commandBuffer = create_command_buffer(
                                                     flushState->gpu(),
                                                     fTarget.get()->priv().peekRenderTarget(),
                                                     fTarget.get()->origin(),
-                                                    fColorLoadOp, fLoadClearColor,
-                                                    fStencilLoadOp);
+                                                    this->requiresStencil());
     flushState->setCommandBuffer(commandBuffer.get());
     commandBuffer->begin();
 
@@ -160,8 +155,7 @@ bool GrRenderTargetOpList::executeOps(GrOpFlushState* flushState) {
             commandBuffer = create_command_buffer(flushState->gpu(),
                                                   fTarget.get()->priv().peekRenderTarget(),
                                                   fTarget.get()->origin(),
-                                                  GrLoadOp::kLoad, 0x0,
-                                                  GrLoadOp::kLoad);
+                                                  false);
             flushState->setCommandBuffer(commandBuffer.get());
             commandBuffer->begin();
         }
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 0acb23425a..2ce2995021 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2429,9 +2429,9 @@ bool GrGLGpu::onReadPixels(GrSurface* surface,
 
 GrGpuCommandBuffer* GrGLGpu::createCommandBuffer(
         GrRenderTarget* rt, GrSurfaceOrigin origin,
-        const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
+        const GrGpuCommandBuffer::LoadAndStoreInfo&,
         const GrGpuCommandBuffer::StencilLoadAndStoreInfo& stencilInfo) {
-    return new GrGLGpuCommandBuffer(this, rt, origin, colorInfo, stencilInfo);
+    return new GrGLGpuCommandBuffer(this, rt, origin, stencilInfo);
 }
 
 void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, const SkIRect* bounds, bool disableSRGB) {
diff --git a/src/gpu/gl/GrGLGpuCommandBuffer.h b/src/gpu/gl/GrGLGpuCommandBuffer.h
index ddf2382160..0d57337ac1 100644
--- a/src/gpu/gl/GrGLGpuCommandBuffer.h
+++ b/src/gpu/gl/GrGLGpuCommandBuffer.h
@@ -25,22 +25,16 @@ class GrGLGpuCommandBuffer : public GrGpuCommandBuffer {
  */
 public:
     GrGLGpuCommandBuffer(GrGLGpu* gpu, GrRenderTarget* rt, GrSurfaceOrigin origin,
-                         const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
                          const GrGpuCommandBuffer::StencilLoadAndStoreInfo& stencilInfo)
             : INHERITED(rt, origin)
-            , fGpu(gpu)
-            , fColorLoadAndStoreInfo(colorInfo)
-            , fStencilLoadAndStoreInfo(stencilInfo) {
+            , fGpu(gpu) {
+        fClearSB = LoadOp::kClear == stencilInfo.fLoadOp;
     }
 
     ~GrGLGpuCommandBuffer() 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);
         }
     }
@@ -79,9 +73,8 @@ private:
         fGpu->clearStencilClip(clip, insideStencilMask, fRenderTarget, fOrigin);
     }
 
-    GrGLGpu*                                    fGpu;
-    GrGpuCommandBuffer::LoadAndStoreInfo        fColorLoadAndStoreInfo;
-    GrGpuCommandBuffer::StencilLoadAndStoreInfo fStencilLoadAndStoreInfo;
+    GrGLGpu*                    fGpu;
+    bool                        fClearSB;
 
     typedef GrGpuCommandBuffer INHERITED;
 };
diff --git a/src/gpu/ops/GrDiscardOp.h b/src/gpu/ops/GrDiscardOp.h
new file mode 100644
index 0000000000..d30aa5a8c5
--- /dev/null
+++ b/src/gpu/ops/GrDiscardOp.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrDiscardOp_DEFINED
+#define GrDiscardOp_DEFINED
+
+#include "GrGpuCommandBuffer.h"
+#include "GrOp.h"
+#include "GrOpFlushState.h"
+#include "GrRenderTargetProxy.h"
+
+class GrDiscardOp final : public GrOp {
+public:
+    DEFINE_OP_CLASS_ID
+
+    static std::unique_ptr<GrOp> Make(GrRenderTargetProxy* proxy) {
+        return std::unique_ptr<GrOp>(new GrDiscardOp(proxy));
+    }
+
+    const char* name() const override { return "Discard"; }
+
+    SkString dumpInfo() const override {
+        SkString string;
+        string.append(INHERITED::dumpInfo());
+        return string;
+    }
+
+private:
+    GrDiscardOp(GrRenderTargetProxy* proxy) : INHERITED(ClassID()) {
+        this->makeFullScreen(proxy);
+    }
+
+    bool onCombineIfPossible(GrOp* that, const GrCaps& caps) override { return false; }
+
+    void onPrepare(GrOpFlushState*) override {}
+
+    void onExecute(GrOpFlushState* state) override {
+        state->commandBuffer()->discard();
+    }
+
+    typedef GrOp INHERITED;
+};
+
+#endif
diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
index ab6705f8f0..6378251d3d 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
@@ -22,16 +22,17 @@
 #include "GrVkTexture.h"
 #include "SkRect.h"
 
-void get_vk_load_store_ops(GrLoadOp loadOpIn, GrStoreOp storeOpIn,
+void get_vk_load_store_ops(GrGpuCommandBuffer::LoadOp loadOpIn,
+                           GrGpuCommandBuffer::StoreOp storeOpIn,
                            VkAttachmentLoadOp* loadOp, VkAttachmentStoreOp* storeOp) {
     switch (loadOpIn) {
-        case GrLoadOp::kLoad:
+        case GrGpuCommandBuffer::LoadOp::kLoad:
             *loadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
             break;
-        case GrLoadOp::kClear:
+        case GrGpuCommandBuffer::LoadOp::kClear:
             *loadOp = VK_ATTACHMENT_LOAD_OP_CLEAR;
             break;
-        case GrLoadOp::kDiscard:
+        case GrGpuCommandBuffer::LoadOp::kDiscard:
             *loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
             break;
         default:
@@ -40,10 +41,10 @@ void get_vk_load_store_ops(GrLoadOp loadOpIn, GrStoreOp storeOpIn,
     }
 
     switch (storeOpIn) {
-        case GrStoreOp::kStore:
+        case GrGpuCommandBuffer::StoreOp::kStore:
             *storeOp = VK_ATTACHMENT_STORE_OP_STORE;
             break;
-        case GrStoreOp::kDiscard:
+        case GrGpuCommandBuffer::StoreOp::kDiscard:
             *storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
             break;
         default: