commit 53a72c1b50e28004c3a90cb28ad1fbc06377b5ce
author Jim Van Verth <jvanverth@google.com> Fri Feb 14 13:04:06 2020 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Feb 14 19:11:30 2020 +0000
tree f3d730145f17569e8c0ef5573dbc96ff2db2fc6c
parent 922496fb3c056b2304c8adddecbf020e746511da

Manage renderCmdEncoder over lifetime of GrMtlOpsRenderPass.

Rather than create a renderCmdEncoder per draw, we create it up front
and each draw just uses it. On a clear or upload we switch away and
then recreate it.

Bug: skia:
Change-Id: Ic6d612119ed3f7c41183d0186083deae14f96398
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/270841
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>

src/gpu/mtl/GrMtlCaps.mm

diff --git a/src/gpu/mtl/GrMtlCaps.mm b/src/gpu/mtl/GrMtlCaps.mm
index 5436a54..5bc35cd 100644
--- a/src/gpu/mtl/GrMtlCaps.mm
+++ b/src/gpu/mtl/GrMtlCaps.mm
@@ -1057,7 +1057,7 @@
     }
 #endif
 
-    b.add32(programInfo.pipeline().isStencilEnabled()
+    b.add32(rt->renderTargetPriv().getStencilAttachment()
                                  ? this->preferredStencilFormat().fInternalFormat
                                  : MTLPixelFormatInvalid);
     b.add32((uint32_t)programInfo.pipeline().isStencilEnabled());

src/gpu/mtl/GrMtlOpsRenderPass.h

diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.h b/src/gpu/mtl/GrMtlOpsRenderPass.h
index 28c953f..30f19ca 100644
--- a/src/gpu/mtl/GrMtlOpsRenderPass.h
+++ b/src/gpu/mtl/GrMtlOpsRenderPass.h
@@ -33,10 +33,7 @@
 
     void initRenderState(id<MTLRenderCommandEncoder>);
 
-    void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override {
-        // TODO: this could be more efficient
-        state->doUpload(upload);
-    }
+    void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override;
     void submit();
 
 private:

src/gpu/mtl/GrMtlOpsRenderPass.mm

diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.mm b/src/gpu/mtl/GrMtlOpsRenderPass.mm
index 5a30f6a..41cef25 100644
--- a/src/gpu/mtl/GrMtlOpsRenderPass.mm
+++ b/src/gpu/mtl/GrMtlOpsRenderPass.mm
@@ -29,14 +29,11 @@
 }
 
 GrMtlOpsRenderPass::~GrMtlOpsRenderPass() {
-    SkASSERT(nil == fActiveRenderCmdEncoder);
 }
 
 void GrMtlOpsRenderPass::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 =)
             fGpu->commandBuffer()->getRenderCommandEncoder(fRenderPassDesc, nullptr, this);
     SkASSERT(nil != cmdEncoder);
@@ -49,6 +46,7 @@
     SkIRect iBounds;
     fBounds.roundOut(&iBounds);
     fGpu->submitIndirectCommandBuffer(fRenderTarget, fOrigin, &iBounds);
+    fActiveRenderCmdEncoder = nil;
 }
 
 GrMtlPipelineState* GrMtlOpsRenderPass::prepareDrawState(const GrProgramInfo& programInfo) {
@@ -78,10 +76,10 @@
         return;
     }
 
-    SkASSERT(nil == fActiveRenderCmdEncoder);
-    fActiveRenderCmdEncoder = fGpu->commandBuffer()->getRenderCommandEncoder(
-            fRenderPassDesc, pipelineState, this);
-    SkASSERT(fActiveRenderCmdEncoder);
+    if (!fActiveRenderCmdEncoder) {
+        fActiveRenderCmdEncoder =
+                fGpu->commandBuffer()->getRenderCommandEncoder(fRenderPassDesc, nullptr, this);
+    }
 
     [fActiveRenderCmdEncoder setRenderPipelineState:pipelineState->mtlPipelineState()];
     pipelineState->setDrawState(fActiveRenderCmdEncoder,
@@ -131,7 +129,6 @@
         mesh.sendToGpu(programInfo.primitiveType(), this);
     }
 
-    fActiveRenderCmdEncoder = nil;
     fBounds.join(bounds);
 }
 
@@ -162,6 +159,16 @@
     fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionClear;
     this->precreateCmdEncoder();
     fRenderPassDesc.stencilAttachment.loadAction = MTLLoadActionLoad;
+    fActiveRenderCmdEncoder =
+            fGpu->commandBuffer()->getRenderCommandEncoder(fRenderPassDesc, nullptr, this);
+}
+
+void GrMtlOpsRenderPass::inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) {
+    // TODO: this could be more efficient
+    state->doUpload(upload);
+    // doUpload() creates a blitCommandEncoder, so we need to recreate a renderCommandEncoder
+    fActiveRenderCmdEncoder =
+            fGpu->commandBuffer()->getRenderCommandEncoder(fRenderPassDesc, nullptr, this);
 }
 
 void GrMtlOpsRenderPass::initRenderState(id<MTLRenderCommandEncoder> encoder) {
@@ -240,6 +247,12 @@
     } else {
         fBounds.setEmpty();
     }
+
+    // For now, we lazily create the renderCommandEncoder because we may have no draws,
+    // and an empty renderCommandEncoder can still produce output. This can cause issues
+    // when we've cleared a texture upon creation -- we'll subsequently discard the contents.
+    // This can be removed when that ordering is fixed.
+    fActiveRenderCmdEncoder = nil;
 }
 
 static MTLPrimitiveType gr_to_mtl_primitive(GrPrimitiveType primitiveType) {