From c1bb9cba16beac18a2e7a91db3482bac9a9ff52c Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Tue, 18 Feb 2020 14:00:47 +0000 Subject: [PATCH] Revert "Manage renderCmdEncoder over lifetime of GrMtlOpsRenderPass." This reverts commit 53a72c1b50e28004c3a90cb28ad1fbc06377b5ce. Reason for revert: Mac 10.13 bot is red Original change's description: > 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 > Commit-Queue: Jim Van Verth TBR=egdaniel@google.com,jvanverth@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: skia: Change-Id: Ibc2697c127ec3e309ed4bb5fe84982f6047e5915 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/271496 Reviewed-by: Robert Phillips Commit-Queue: Robert Phillips --- src/gpu/mtl/GrMtlCaps.mm | 2 +- src/gpu/mtl/GrMtlOpsRenderPass.h | 5 ++++- src/gpu/mtl/GrMtlOpsRenderPass.mm | 29 ++++++++--------------------- 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/src/gpu/mtl/GrMtlCaps.mm b/src/gpu/mtl/GrMtlCaps.mm index 5bc35cdea1c32..5436a541dd86c 100644 --- a/src/gpu/mtl/GrMtlCaps.mm +++ b/src/gpu/mtl/GrMtlCaps.mm @@ -1057,7 +1057,7 @@ static bool format_is_srgb(MTLPixelFormat format) { } #endif - b.add32(rt->renderTargetPriv().getStencilAttachment() + b.add32(programInfo.pipeline().isStencilEnabled() ? this->preferredStencilFormat().fInternalFormat : MTLPixelFormatInvalid); b.add32((uint32_t)programInfo.pipeline().isStencilEnabled()); diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.h b/src/gpu/mtl/GrMtlOpsRenderPass.h index 30f19ca060b28..28c953f075f86 100644 --- a/src/gpu/mtl/GrMtlOpsRenderPass.h +++ b/src/gpu/mtl/GrMtlOpsRenderPass.h @@ -33,7 +33,10 @@ class GrMtlOpsRenderPass : public GrOpsRenderPass, private GrMesh::SendToGpuImpl void initRenderState(id); - void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override; + void inlineUpload(GrOpFlushState* state, GrDeferredTextureUploadFn& upload) override { + // TODO: this could be more efficient + state->doUpload(upload); + } void submit(); private: diff --git a/src/gpu/mtl/GrMtlOpsRenderPass.mm b/src/gpu/mtl/GrMtlOpsRenderPass.mm index 41cef25b6d264..5a30f6a59b6e7 100644 --- a/src/gpu/mtl/GrMtlOpsRenderPass.mm +++ b/src/gpu/mtl/GrMtlOpsRenderPass.mm @@ -29,11 +29,14 @@ } 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 cmdEncoder =) fGpu->commandBuffer()->getRenderCommandEncoder(fRenderPassDesc, nullptr, this); SkASSERT(nil != cmdEncoder); @@ -46,7 +49,6 @@ SkIRect iBounds; fBounds.roundOut(&iBounds); fGpu->submitIndirectCommandBuffer(fRenderTarget, fOrigin, &iBounds); - fActiveRenderCmdEncoder = nil; } GrMtlPipelineState* GrMtlOpsRenderPass::prepareDrawState(const GrProgramInfo& programInfo) { @@ -76,10 +78,10 @@ return; } - if (!fActiveRenderCmdEncoder) { - fActiveRenderCmdEncoder = - fGpu->commandBuffer()->getRenderCommandEncoder(fRenderPassDesc, nullptr, this); - } + SkASSERT(nil == fActiveRenderCmdEncoder); + fActiveRenderCmdEncoder = fGpu->commandBuffer()->getRenderCommandEncoder( + fRenderPassDesc, pipelineState, this); + SkASSERT(fActiveRenderCmdEncoder); [fActiveRenderCmdEncoder setRenderPipelineState:pipelineState->mtlPipelineState()]; pipelineState->setDrawState(fActiveRenderCmdEncoder, @@ -129,6 +131,7 @@ mesh.sendToGpu(programInfo.primitiveType(), this); } + fActiveRenderCmdEncoder = nil; fBounds.join(bounds); } @@ -159,16 +162,6 @@ 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 encoder) { @@ -247,12 +240,6 @@ } 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) {