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>

8 files changed, 61 insertions, 101 deletions

diff --git a/src/gpu/GrGpuCommandBuffer.h b/src/gpu/GrGpuCommandBuffer.h
index 31e9a546d5..9e3c00ba5b 100644
--- a/src/gpu/GrGpuCommandBuffer.h
+++ b/src/gpu/GrGpuCommandBuffer.h
@@ -30,28 +30,17 @@ struct SkRect;
  */
 class GrGpuCommandBuffer {
 public:
-    enum class LoadOp {
-        kLoad,
-        kClear,
-        kDiscard,
-    };
-
-    enum class StoreOp {
-        kStore,
-        kDiscard,
-    };
-
     struct LoadAndStoreInfo {
-        LoadOp  fLoadOp;
-        StoreOp fStoreOp;
-        GrColor fClearColor;
+        GrLoadOp  fLoadOp;
+        GrStoreOp fStoreOp;
+        GrColor   fClearColor;
     };
 
     // Load-time clears of the stencil buffer are always to 0 so we don't store
     // an 'fStencilClearValue'
     struct StencilLoadAndStoreInfo {
-        LoadOp fLoadOp;
-        StoreOp fStoreOp;
+        GrLoadOp  fLoadOp;
+        GrStoreOp fStoreOp;
     };
 
     GrGpuCommandBuffer(GrRenderTarget* rt, GrSurfaceOrigin origin)
diff --git a/src/gpu/GrOpList.h b/src/gpu/GrOpList.h
index 00dc344a07..b45ecce190 100644
--- a/src/gpu/GrOpList.h
+++ b/src/gpu/GrOpList.h
@@ -8,6 +8,7 @@
 #ifndef GrOpList_DEFINED
 #define GrOpList_DEFINED
 
+#include "GrColor.h"
 #include "GrGpuResourceRef.h"
 #include "SkRefCnt.h"
 #include "SkTDArray.h"
@@ -82,9 +83,6 @@ 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
      */
@@ -93,10 +91,18 @@ 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
 
@@ -107,8 +113,6 @@ 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 e76b5ff26e..48cea29d71 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -31,7 +31,6 @@
 #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"
@@ -216,18 +215,22 @@ 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());
 
-    // 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;
+    // 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);
         }
-        this->getRTOpList()->addOp(std::move(op), *this->caps());
     }
 }
 
@@ -1761,7 +1764,7 @@ uint32_t GrRenderTargetContext::addDrawOp(const GrClip& clip, std::unique_ptr<Gr
 
     if (fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil ||
         appliedClip.hasStencilClip()) {
-        this->getOpList()->setRequiresStencil();
+        this->getOpList()->setStencilLoadOp(GrLoadOp::kClear);
 
         // This forces instantiation of the render target.
         GrRenderTarget* rt = this->accessRenderTarget();
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index ffa1e43746..819e8a58c0 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -87,11 +87,13 @@ void GrRenderTargetOpList::prepareOps(GrOpFlushState* flushState) {
 static std::unique_ptr<GrGpuCommandBuffer> create_command_buffer(GrGpu* gpu,
                                                                  GrRenderTarget* rt,
                                                                  GrSurfaceOrigin origin,
-                                                                 bool clearSB) {
+                                                                 GrLoadOp colorLoadOp,
+                                                                 GrColor loadClearColor,
+                                                                 GrLoadOp stencilLoadOp) {
     static const GrGpuCommandBuffer::LoadAndStoreInfo kBasicLoadStoreInfo {
-        GrGpuCommandBuffer::LoadOp::kLoad,
-        GrGpuCommandBuffer::StoreOp::kStore,
-        GrColor_ILLEGAL
+        colorLoadOp,
+        GrStoreOp::kStore,
+        loadClearColor
     };
 
     // TODO:
@@ -100,8 +102,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 {
-        clearSB ? GrGpuCommandBuffer::LoadOp::kClear : GrGpuCommandBuffer::LoadOp::kLoad,
-        GrGpuCommandBuffer::StoreOp::kStore,
+        stencilLoadOp,
+        GrStoreOp::kStore,
     };
 
     std::unique_ptr<GrGpuCommandBuffer> buffer(
@@ -130,11 +132,14 @@ 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(),
-                                                    this->requiresStencil());
+                                                    fColorLoadOp, fLoadClearColor,
+                                                    fStencilLoadOp);
     flushState->setCommandBuffer(commandBuffer.get());
     commandBuffer->begin();
 
@@ -155,7 +160,8 @@ bool GrRenderTargetOpList::executeOps(GrOpFlushState* flushState) {
             commandBuffer = create_command_buffer(flushState->gpu(),
                                                   fTarget.get()->priv().peekRenderTarget(),
                                                   fTarget.get()->origin(),
-                                                  false);
+                                                  GrLoadOp::kLoad, 0x0,
+                                                  GrLoadOp::kLoad);
             flushState->setCommandBuffer(commandBuffer.get());
             commandBuffer->begin();
         }
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index c20cdf048f..0c6d5c164f 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2450,9 +2450,9 @@ bool GrGLGpu::onReadPixels(GrSurface* surface,
 
 GrGpuCommandBuffer* GrGLGpu::createCommandBuffer(
         GrRenderTarget* rt, GrSurfaceOrigin origin,
-        const GrGpuCommandBuffer::LoadAndStoreInfo&,
+        const GrGpuCommandBuffer::LoadAndStoreInfo& colorInfo,
         const GrGpuCommandBuffer::StencilLoadAndStoreInfo& stencilInfo) {
-    return new GrGLGpuCommandBuffer(this, rt, origin, stencilInfo);
+    return new GrGLGpuCommandBuffer(this, rt, origin, colorInfo, 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 0d57337ac1..ddf2382160 100644
--- a/src/gpu/gl/GrGLGpuCommandBuffer.h
+++ b/src/gpu/gl/GrGLGpuCommandBuffer.h
@@ -25,16 +25,22 @@ 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) {
-        fClearSB = LoadOp::kClear == stencilInfo.fLoadOp;
+            , fGpu(gpu)
+            , fColorLoadAndStoreInfo(colorInfo)
+            , fStencilLoadAndStoreInfo(stencilInfo) {
     }
 
     ~GrGLGpuCommandBuffer() override {}
 
     void begin() override {
-        if (fClearSB) {
+        if (GrLoadOp::kClear == fColorLoadAndStoreInfo.fLoadOp) {
+            fGpu->clear(GrFixedClip::Disabled(), fColorLoadAndStoreInfo.fClearColor,
+                        fRenderTarget, fOrigin);
+        }
+        if (GrLoadOp::kClear == fStencilLoadAndStoreInfo.fLoadOp) {
             fGpu->clearStencil(fRenderTarget, 0x0);
         }
     }
@@ -73,8 +79,9 @@ private:
         fGpu->clearStencilClip(clip, insideStencilMask, fRenderTarget, fOrigin);
     }
 
-    GrGLGpu*                    fGpu;
-    bool                        fClearSB;
+    GrGLGpu*                                    fGpu;
+    GrGpuCommandBuffer::LoadAndStoreInfo        fColorLoadAndStoreInfo;
+    GrGpuCommandBuffer::StencilLoadAndStoreInfo fStencilLoadAndStoreInfo;
 
     typedef GrGpuCommandBuffer INHERITED;
 };
diff --git a/src/gpu/ops/GrDiscardOp.h b/src/gpu/ops/GrDiscardOp.h
deleted file mode 100644
index d30aa5a8c5..0000000000
--- a/src/gpu/ops/GrDiscardOp.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- * 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 6378251d3d..ab6705f8f0 100644
--- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
@@ -22,17 +22,16 @@
 #include "GrVkTexture.h"
 #include "SkRect.h"
 
-void get_vk_load_store_ops(GrGpuCommandBuffer::LoadOp loadOpIn,
-                           GrGpuCommandBuffer::StoreOp storeOpIn,
+void get_vk_load_store_ops(GrLoadOp loadOpIn, GrStoreOp storeOpIn,
                            VkAttachmentLoadOp* loadOp, VkAttachmentStoreOp* storeOp) {
     switch (loadOpIn) {
-        case GrGpuCommandBuffer::LoadOp::kLoad:
+        case GrLoadOp::kLoad:
             *loadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
             break;
-        case GrGpuCommandBuffer::LoadOp::kClear:
+        case GrLoadOp::kClear:
             *loadOp = VK_ATTACHMENT_LOAD_OP_CLEAR;
             break;
-        case GrGpuCommandBuffer::LoadOp::kDiscard:
+        case GrLoadOp::kDiscard:
             *loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
             break;
         default:
@@ -41,10 +40,10 @@ void get_vk_load_store_ops(GrGpuCommandBuffer::LoadOp loadOpIn,
     }
 
     switch (storeOpIn) {
-        case GrGpuCommandBuffer::StoreOp::kStore:
+        case GrStoreOp::kStore:
             *storeOp = VK_ATTACHMENT_STORE_OP_STORE;
             break;
-        case GrGpuCommandBuffer::StoreOp::kDiscard:
+        case GrStoreOp::kDiscard:
             *storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
             break;
         default: