Minor cleanups to GrMtlGpuRTCommandBuffer

Bug: skia:8243
Change-Id: I80ed04f7bfbcf20549de30822245d8fe2832a810
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/219996
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
diff --git a/src/gpu/mtl/GrMtlGpuCommandBuffer.h b/src/gpu/mtl/GrMtlGpuCommandBuffer.h
index 062b228..5ab40b2 100644
--- a/src/gpu/mtl/GrMtlGpuCommandBuffer.h
+++ b/src/gpu/mtl/GrMtlGpuCommandBuffer.h
@@ -72,8 +72,6 @@
     void submit();
 
 private:
-    void addNullCommand();
-
     GrGpu* gpu() override { return fGpu; }
 
     GrMtlPipelineState* prepareDrawState(
@@ -94,7 +92,8 @@
 
     void onClearStencilClip(const GrFixedClip& clip, bool insideStencilMask) override;
 
-    MTLRenderPassDescriptor* createRenderPassDesc() const;
+    void setupRenderPass(const GrGpuRTCommandBuffer::LoadAndStoreInfo& colorInfo,
+                         const GrGpuRTCommandBuffer::StencilLoadAndStoreInfo& stencilInfo);
 
     void bindGeometry(const GrBuffer* vertexBuffer, size_t vertexOffset,
                       const GrBuffer* instanceBuffer);
@@ -121,24 +120,18 @@
     void setVertexBuffer(id<MTLRenderCommandEncoder>, const GrMtlBuffer*, size_t offset,
                          size_t index);
     void resetBufferBindings();
+    void precreateCmdEncoder();
 
-    GrMtlGpu*                                     fGpu;
+    GrMtlGpu*                   fGpu;
     // GrRenderTargetProxy bounds
 #ifdef SK_DEBUG
-    SkRect                                        fBounds;
+    SkRect                      fRTBounds;
 #endif
-    GrGpuRTCommandBuffer::LoadAndStoreInfo        fColorLoadAndStoreInfo;
-    GrGpuRTCommandBuffer::StencilLoadAndStoreInfo fStencilLoadAndStoreInfo;
 
     id<MTLRenderCommandEncoder> fActiveRenderCmdEncoder;
     MTLRenderPassDescriptor*    fRenderPassDesc;
-
-    struct CommandBufferInfo {
-        SkRect fBounds;
-        size_t fCurrentVertexStride;
-    };
-
-    CommandBufferInfo fCommandBufferInfo;
+    SkRect                      fBounds;
+    size_t                      fCurrentVertexStride;
 
     static constexpr size_t kNumBindings = GrMtlUniformHandler::kLastUniformBinding + 3;
     id<MTLBuffer> fBufferBindings[kNumBindings];
diff --git a/src/gpu/mtl/GrMtlGpuCommandBuffer.mm b/src/gpu/mtl/GrMtlGpuCommandBuffer.mm
index 8dc4c70..fe6c576 100644
--- a/src/gpu/mtl/GrMtlGpuCommandBuffer.mm
+++ b/src/gpu/mtl/GrMtlGpuCommandBuffer.mm
@@ -27,55 +27,19 @@
         : INHERITED(rt, origin)
         , fGpu(gpu)
 #ifdef SK_DEBUG
-        , fBounds(bounds)
+        , fRTBounds(bounds)
 #endif
-        , fColorLoadAndStoreInfo(colorInfo)
-        , fStencilLoadAndStoreInfo(stencilInfo)
-        , fRenderPassDesc(this->createRenderPassDesc()) {
-    (void)fStencilLoadAndStoreInfo; // Silence unused var warning
-    const GrMtlStencilAttachment* stencil = static_cast<GrMtlStencilAttachment*>(
-                                                     rt->renderTargetPriv().getStencilAttachment());
-    if (stencil) {
-        fRenderPassDesc.stencilAttachment.texture = stencil->stencilView();
-    }
-    if (fColorLoadAndStoreInfo.fLoadOp == GrLoadOp::kClear) {
-        fCommandBufferInfo.fBounds = SkRect::MakeWH(fRenderTarget->width(),
-                                                    fRenderTarget->height());
-        this->addNullCommand();
-        fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionLoad;
-    } else {
-        fCommandBufferInfo.fBounds.setEmpty();
-    }
-    switch (stencilInfo.fLoadOp) {
-        case GrLoadOp::kLoad:
-            fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionLoad;
-            break;
-        case GrLoadOp::kClear:
-            fCommandBufferInfo.fBounds = SkRect::MakeWH(fRenderTarget->width(),
-                                                        fRenderTarget->height());
-            fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionClear;
-            this->addNullCommand();
-            fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionLoad;
-            break;
-        case GrLoadOp::kDiscard:
-            fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionDontCare;
-            break;
-    }
-    switch (stencilInfo.fStoreOp) {
-        case GrStoreOp::kStore:
-            fRenderPassDesc.stencilAttachment.storeAction = MTLStoreActionStore;
-            break;
-        case GrStoreOp::kDiscard:
-            fRenderPassDesc.stencilAttachment.storeAction = MTLStoreActionDontCare;
-            break;
-    }
+        {
+    this->setupRenderPass(colorInfo, stencilInfo);
 }
 
 GrMtlGpuRTCommandBuffer::~GrMtlGpuRTCommandBuffer() {
     SkASSERT(nil == fActiveRenderCmdEncoder);
 }
 
-void GrMtlGpuRTCommandBuffer::addNullCommand() {
+void GrMtlGpuRTCommandBuffer::precreateCmdEncoder() {
+    // For clears, we may not have an associated draw. So we prepare a cmdEncoder that
+    // will be submitted whether there's a draw or not.
     SkASSERT(nil == fActiveRenderCmdEncoder);
 
     SkDEBUGCODE(id<MTLRenderCommandEncoder> cmdEncoder =)
@@ -88,7 +52,7 @@
         return;
     }
     SkIRect iBounds;
-    fCommandBufferInfo.fBounds.roundOut(&iBounds);
+    fBounds.roundOut(&iBounds);
     fGpu->submitIndirectCommandBuffer(fRenderTarget, fOrigin, &iBounds);
 }
 
@@ -132,7 +96,7 @@
         return nullptr;
     }
     pipelineState->setData(fRenderTarget, fOrigin, primProc, pipeline, primProcProxies);
-    fCommandBufferInfo.fCurrentVertexStride = primProc.vertexStride();
+    fCurrentVertexStride = primProc.vertexStride();
 
     return pipelineState;
 }
@@ -246,17 +210,17 @@
 
     fActiveRenderCmdEncoder = nil;
     this->resetBufferBindings();
-    fCommandBufferInfo.fBounds.join(bounds);
+    fBounds.join(bounds);
 }
 
 void GrMtlGpuRTCommandBuffer::onClear(const GrFixedClip& clip, const SkPMColor4f& color) {
     // if we end up here from absClear, the clear bounds may be bigger than the RT proxy bounds -
     // but in that case, scissor should be enabled, so this check should still succeed
-    SkASSERT(!clip.scissorEnabled() || clip.scissorRect().contains(fBounds));
+    SkASSERT(!clip.scissorEnabled() || clip.scissorRect().contains(fRTBounds));
     fRenderPassDesc.colorAttachments[0].clearColor = MTLClearColorMake(color.fR, color.fG, color.fB,
                                                                        color.fA);
     fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionClear;
-    this->addNullCommand();
+    this->precreateCmdEncoder();
     fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionLoad;
 }
 
@@ -278,11 +242,13 @@
     }
 
     fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionClear;
-    this->addNullCommand();
+    this->precreateCmdEncoder();
     fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionLoad;
 }
 
-MTLRenderPassDescriptor* GrMtlGpuRTCommandBuffer::createRenderPassDesc() const {
+void GrMtlGpuRTCommandBuffer::setupRenderPass(
+        const GrGpuRTCommandBuffer::LoadAndStoreInfo& colorInfo,
+        const GrGpuRTCommandBuffer::StencilLoadAndStoreInfo& stencilInfo) {
     const static MTLLoadAction mtlLoadAction[] {
         MTLLoadActionLoad,
         MTLLoadActionClear,
@@ -291,7 +257,8 @@
     GR_STATIC_ASSERT((int)GrLoadOp::kLoad == 0);
     GR_STATIC_ASSERT((int)GrLoadOp::kClear == 1);
     GR_STATIC_ASSERT((int)GrLoadOp::kDiscard == 2);
-    SkASSERT(fColorLoadAndStoreInfo.fLoadOp <= GrLoadOp::kDiscard);
+    SkASSERT(colorInfo.fLoadOp <= GrLoadOp::kDiscard);
+    SkASSERT(stencilInfo.fLoadOp <= GrLoadOp::kDiscard);
 
     const static MTLStoreAction mtlStoreAction[] {
         MTLStoreActionStore,
@@ -299,21 +266,49 @@
     };
     GR_STATIC_ASSERT((int)GrStoreOp::kStore == 0);
     GR_STATIC_ASSERT((int)GrStoreOp::kDiscard == 1);
-    SkASSERT(fColorLoadAndStoreInfo.fStoreOp <= GrStoreOp::kDiscard);
+    SkASSERT(colorInfo.fStoreOp <= GrStoreOp::kDiscard);
+    SkASSERT(stencilInfo.fStoreOp <= GrStoreOp::kDiscard);
 
     auto renderPassDesc = [MTLRenderPassDescriptor renderPassDescriptor];
     renderPassDesc.colorAttachments[0].texture =
             static_cast<GrMtlRenderTarget*>(fRenderTarget)->mtlRenderTexture();
     renderPassDesc.colorAttachments[0].slice = 0;
     renderPassDesc.colorAttachments[0].level = 0;
-    const SkPMColor4f& clearColor = fColorLoadAndStoreInfo.fClearColor;
+    const SkPMColor4f& clearColor = colorInfo.fClearColor;
     renderPassDesc.colorAttachments[0].clearColor =
             MTLClearColorMake(clearColor[0], clearColor[1], clearColor[2], clearColor[3]);
     renderPassDesc.colorAttachments[0].loadAction =
-            mtlLoadAction[static_cast<int>(fColorLoadAndStoreInfo.fLoadOp)];
+            mtlLoadAction[static_cast<int>(colorInfo.fLoadOp)];
     renderPassDesc.colorAttachments[0].storeAction =
-            mtlStoreAction[static_cast<int>(fColorLoadAndStoreInfo.fStoreOp)];
-    return renderPassDesc;
+            mtlStoreAction[static_cast<int>(colorInfo.fStoreOp)];
+
+    const GrMtlStencilAttachment* stencil = static_cast<GrMtlStencilAttachment*>(
+            fRenderTarget->renderTargetPriv().getStencilAttachment());
+    if (stencil) {
+        renderPassDesc.stencilAttachment.texture = stencil->stencilView();
+    }
+    renderPassDesc.stencilAttachment.clearStencil = 0;
+    renderPassDesc.stencilAttachment.loadAction =
+            mtlLoadAction[static_cast<int>(stencilInfo.fLoadOp)];
+    renderPassDesc.stencilAttachment.storeAction =
+            mtlStoreAction[static_cast<int>(stencilInfo.fStoreOp)];
+
+    fRenderPassDesc = renderPassDesc;
+
+    // Manage initial clears
+    if (colorInfo.fLoadOp == GrLoadOp::kClear || stencilInfo.fLoadOp == GrLoadOp::kClear)  {
+        fBounds = SkRect::MakeWH(fRenderTarget->width(),
+                                 fRenderTarget->height());
+        this->precreateCmdEncoder();
+        if (colorInfo.fLoadOp == GrLoadOp::kClear) {
+            fRenderPassDesc.colorAttachments[0].loadAction = MTLLoadActionLoad;
+        }
+        if (stencilInfo.fLoadOp == GrLoadOp::kClear) {
+            fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionLoad;
+        }
+    } else {
+        fBounds.setEmpty();
+    }
 }
 
 static MTLPrimitiveType gr_to_mtl_primitive(GrPrimitiveType primitiveType) {
@@ -375,7 +370,7 @@
                                                    const GrBuffer* vertexBuffer,
                                                    int baseVertex,
                                                    GrPrimitiveRestart restart) {
-    this->bindGeometry(vertexBuffer, fCommandBufferInfo.fCurrentVertexStride*baseVertex, nullptr);
+    this->bindGeometry(vertexBuffer, fCurrentVertexStride*baseVertex, nullptr);
 
     SkASSERT(primitiveType != GrPrimitiveType::kLinesAdjacency); // Geometry shaders not supported.
     id<MTLBuffer> mtlIndexBuffer = nil;
@@ -458,12 +453,13 @@
     // for a currently bound vertex buffer, rather than setVertexBuffer:
     if (fBufferBindings[index] != mtlVertexBuffer) {
         [encoder setVertexBuffer: mtlVertexBuffer
-                          offset: 0
+                          offset: buffer->offset() + vertexOffset
                          atIndex: index];
         fBufferBindings[index] = mtlVertexBuffer;
+    } else {
+        [encoder setVertexBufferOffset: buffer->offset() + vertexOffset
+                               atIndex: index];
     }
-    [encoder setVertexBufferOffset: buffer->offset() + vertexOffset
-                           atIndex: index];
 }
 
 void GrMtlGpuRTCommandBuffer::resetBufferBindings() {