Skip to content

Commit

Permalink
Revert "Manage renderCmdEncoder over lifetime of GrMtlOpsRenderPass."
Browse files Browse the repository at this point in the history
This reverts commit 53a72c1.

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 <egdaniel@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>

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 <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
  • Loading branch information
rphilli authored and Skia Commit-Bot committed Feb 18, 2020
1 parent 78b4717 commit c1bb9cb
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/gpu/mtl/GrMtlCaps.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
5 changes: 4 additions & 1 deletion src/gpu/mtl/GrMtlOpsRenderPass.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ class GrMtlOpsRenderPass : public GrOpsRenderPass, private GrMesh::SendToGpuImpl

void initRenderState(id<MTLRenderCommandEncoder>);

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:
Expand Down
29 changes: 8 additions & 21 deletions src/gpu/mtl/GrMtlOpsRenderPass.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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<MTLRenderCommandEncoder> cmdEncoder =)
fGpu->commandBuffer()->getRenderCommandEncoder(fRenderPassDesc, nullptr, this);
SkASSERT(nil != cmdEncoder);
Expand All @@ -46,7 +49,6 @@
SkIRect iBounds;
fBounds.roundOut(&iBounds);
fGpu->submitIndirectCommandBuffer(fRenderTarget, fOrigin, &iBounds);
fActiveRenderCmdEncoder = nil;
}

GrMtlPipelineState* GrMtlOpsRenderPass::prepareDrawState(const GrProgramInfo& programInfo) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -129,6 +131,7 @@
mesh.sendToGpu(programInfo.primitiveType(), this);
}

fActiveRenderCmdEncoder = nil;
fBounds.join(bounds);
}

Expand Down Expand Up @@ -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<MTLRenderCommandEncoder> encoder) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit c1bb9cb

Please sign in to comment.