Metal: Fix discardable MSAA when used with stencil.

The RenderCommandEncoder created for LoadMSAAFromResolve needs to be
able to merge with the encoder that follows it -- otherwise, the load
will do nothing. Hence if the following encoder has stencil, we need to
add stencil to the LoadFromMSAAResolve encoder.

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

src/gpu/mtl/GrMtlCommandBuffer.h

@@ -42,9 +42,13 @@ public:
     }
 
     id<MTLBlitCommandEncoder> getBlitCommandEncoder();
+    // Tries to reuse current renderCommandEncoder if possible
     GrMtlRenderCommandEncoder* getRenderCommandEncoder(MTLRenderPassDescriptor*,
                                                        const GrMtlPipelineState*,
                                                        GrMtlOpsRenderPass* opsRenderPass);
+    // Replaces current renderCommandEncoder with new one
+    GrMtlRenderCommandEncoder* getRenderCommandEncoder(MTLRenderPassDescriptor*,
+                                                       GrMtlOpsRenderPass*);
 
     void addCompletedHandler(MTLCommandBufferHandler block) {
         [fCmdBuffer addCompletedHandler:block];

src/gpu/mtl/GrMtlCommandBuffer.mm

@@ -141,6 +141,12 @@ GrMtlRenderCommandEncoder* GrMtlCommandBuffer::getRenderCommandEncoder(
         }
     }
 
+    return this->getRenderCommandEncoder(descriptor, opsRenderPass);
+}
+
+GrMtlRenderCommandEncoder* GrMtlCommandBuffer::getRenderCommandEncoder(
+        MTLRenderPassDescriptor* descriptor,
+        GrMtlOpsRenderPass* opsRenderPass) {
     this->endAllEncoding();
     fActiveRenderCommandEncoder = GrMtlRenderCommandEncoder::Make(
             [fCmdBuffer renderCommandEncoderWithDescriptor:descriptor]);

src/gpu/mtl/GrMtlGpu.h

@@ -110,7 +110,8 @@ public:
 
     bool loadMSAAFromResolve(GrAttachment* dst,
                              GrMtlAttachment* src,
-                             const SkIRect& srcRect);
+                             const SkIRect& srcRect,
+                             MTLRenderPassStencilAttachmentDescriptor*);
 
     // When the Metal backend actually uses indirect command buffers, this function will actually do
     // what it says. For now, every command is encoded directly into the primary command buffer, so

src/gpu/mtl/GrMtlGpu.mm

@@ -1585,7 +1585,8 @@ void GrMtlGpu::resolve(GrMtlAttachment* resolveAttachment,
 
 bool GrMtlGpu::loadMSAAFromResolve(GrAttachment* dst,
                                    GrMtlAttachment* src,
-                                   const SkIRect& srcRect) {
+                                   const SkIRect& srcRect,
+                                   MTLRenderPassStencilAttachmentDescriptor* stencil) {
     if (!dst) {
         return false;
     }
@@ -1595,8 +1596,10 @@ bool GrMtlGpu::loadMSAAFromResolve(GrAttachment* dst,
 
     GrMtlAttachment* mtlDst = static_cast<GrMtlAttachment*>(dst);
 
+    MTLPixelFormat stencilFormat = stencil.texture.pixelFormat;
     auto renderPipeline = this->resourceProvider().findOrCreateMSAALoadPipeline(mtlDst->mtlFormat(),
-                                                                                dst->numSamples());
+                                                                                dst->numSamples(),
+                                                                                stencilFormat);
 
     // Set up rendercommandencoder
     auto renderPassDesc = [MTLRenderPassDescriptor new];
@@ -1606,8 +1609,13 @@ bool GrMtlGpu::loadMSAAFromResolve(GrAttachment* dst,
     colorAttachment.storeAction = MTLStoreActionMultisampleResolve;
     colorAttachment.resolveTexture = src->mtlTexture();
 
+    renderPassDesc.stencilAttachment = stencil;
+
+    // We know in this case that the preceding renderCommandEncoder will not be compatible.
+    // Either it's using a different rendertarget, or we are reading from the resolve and
+    // hence we need to let the previous resolve finish. So we create a new one without checking.
     auto renderCmdEncoder =
-                this->commandBuffer()->getRenderCommandEncoder(renderPassDesc, nullptr, nullptr);
+                this->commandBuffer()->getRenderCommandEncoder(renderPassDesc, nullptr);
 
     // Bind pipeline
     renderCmdEncoder->setRenderPipelineState(renderPipeline->mtlPipelineState());

src/gpu/mtl/GrMtlOpsRenderPass.mm

@@ -254,8 +254,6 @@ void GrMtlOpsRenderPass::setupRenderPass(
     colorAttachment.loadAction = mtlLoadAction[static_cast<int>(colorInfo.fLoadOp)];
     colorAttachment.storeAction = mtlStoreAction[static_cast<int>(colorInfo.fStoreOp)];
 
-    this->setupResolve();
-
     auto stencil = fFramebuffer->stencilAttachment();
     auto mtlStencil = fRenderPassDesc.stencilAttachment;
     if (stencil) {
@@ -265,6 +263,8 @@ void GrMtlOpsRenderPass::setupRenderPass(
     mtlStencil.loadAction = mtlLoadAction[static_cast<int>(stencilInfo.fLoadOp)];
     mtlStencil.storeAction = mtlStoreAction[static_cast<int>(stencilInfo.fStoreOp)];
 
+    this->setupResolve();
+
     // Manage initial clears
     if (colorInfo.fLoadOp == GrLoadOp::kClear || stencilInfo.fLoadOp == GrLoadOp::kClear)  {
         fBounds = SkRect::MakeWH(color->dimensions().width(),
@@ -295,7 +295,8 @@ void GrMtlOpsRenderPass::setupResolve() {
             // for now use the full bounds
             auto nativeBounds = GrNativeRect::MakeIRectRelativeTo(
                     fOrigin, dimensions.height(), SkIRect::MakeSize(dimensions));
-            fGpu->loadMSAAFromResolve(color, resolve, nativeBounds);
+            fGpu->loadMSAAFromResolve(color, resolve, nativeBounds,
+                                      fRenderPassDesc.stencilAttachment);
         }
     }
 }

src/gpu/mtl/GrMtlPipelineState.mm

@@ -227,7 +227,8 @@ void GrMtlPipelineState::SetDynamicScissorRectState(GrMtlRenderCommandEncoder* r
 bool GrMtlPipelineState::doesntSampleAttachment(
         const MTLRenderPassAttachmentDescriptor* attachment) const {
     for (int i = 0; i < fSamplerBindings.count(); ++i) {
-        if (attachment.texture == fSamplerBindings[i].fTexture) {
+        if (attachment.texture == fSamplerBindings[i].fTexture ||
+            attachment.resolveTexture == fSamplerBindings[i].fTexture) {
             return false;
         }
     }

src/gpu/mtl/GrMtlResourceProvider.h

@@ -40,7 +40,9 @@ public:
     // Finds or creates a compatible MTLSamplerState based on the GrSamplerState.
     GrMtlSampler* findOrCreateCompatibleSampler(GrSamplerState);
 
-    const GrMtlRenderPipeline* findOrCreateMSAALoadPipeline(MTLPixelFormat, int sampleCount);
+    const GrMtlRenderPipeline* findOrCreateMSAALoadPipeline(MTLPixelFormat colorFormat,
+                                                            int sampleCount,
+                                                            MTLPixelFormat stencilFormat);
 
     // Destroy any cached resources. To be called before releasing the MtlDevice.
     void destroyResources();
@@ -91,8 +93,9 @@ private:
 
     struct MSAALoadPipelineEntry {
         sk_sp<const GrMtlRenderPipeline> fPipeline;
-        MTLPixelFormat fPixelFormat;
+        MTLPixelFormat fColorFormat;
         int fSampleCount;
+        MTLPixelFormat fStencilFormat;
     };
     id<MTLLibrary> fMSAALoadLibrary;
     SkTArray<MSAALoadPipelineEntry> fMSAALoadPipelines;

src/gpu/mtl/GrMtlResourceProvider.mm

@@ -68,7 +68,7 @@ GrMtlSampler* GrMtlResourceProvider::findOrCreateCompatibleSampler(GrSamplerStat
 }
 
 const GrMtlRenderPipeline* GrMtlResourceProvider::findOrCreateMSAALoadPipeline(
-        MTLPixelFormat pixelFormat, int sampleCount) {
+        MTLPixelFormat colorFormat, int sampleCount, MTLPixelFormat stencilFormat) {
     if (!fMSAALoadLibrary) {
         TRACE_EVENT0("skia", TRACE_FUNC);
 
@@ -112,8 +112,9 @@ const GrMtlRenderPipeline* GrMtlResourceProvider::findOrCreateMSAALoadPipeline(
     }
 
     for (int i = 0; i < fMSAALoadPipelines.count(); ++i) {
-        if (fMSAALoadPipelines[i].fPixelFormat == pixelFormat &&
-            fMSAALoadPipelines[i].fSampleCount == sampleCount) {
+        if (fMSAALoadPipelines[i].fColorFormat == colorFormat &&
+            fMSAALoadPipelines[i].fSampleCount == sampleCount &&
+            fMSAALoadPipelines[i].fStencilFormat == stencilFormat) {
             return fMSAALoadPipelines[i].fPipeline.get();
         }
     }
@@ -127,14 +128,15 @@ const GrMtlRenderPipeline* GrMtlResourceProvider::findOrCreateMSAALoadPipeline(
 
     auto mtlColorAttachment = [[MTLRenderPipelineColorAttachmentDescriptor alloc] init];
 
-    mtlColorAttachment.pixelFormat = pixelFormat;
+    mtlColorAttachment.pixelFormat = colorFormat;
     mtlColorAttachment.blendingEnabled = FALSE;
     mtlColorAttachment.writeMask = MTLColorWriteMaskAll;
 
     pipelineDescriptor.colorAttachments[0] = mtlColorAttachment;
-
     pipelineDescriptor.sampleCount = sampleCount;
 
+    pipelineDescriptor.stencilAttachmentPixelFormat = stencilFormat;
+
     NSError* error;
     auto pso =
             [fGpu->device() newRenderPipelineStateWithDescriptor: pipelineDescriptor
@@ -146,7 +148,7 @@ const GrMtlRenderPipeline* GrMtlResourceProvider::findOrCreateMSAALoadPipeline(
 
     auto renderPipeline = GrMtlRenderPipeline::Make(pso);
 
-    fMSAALoadPipelines.push_back({renderPipeline, pixelFormat, sampleCount});
+    fMSAALoadPipelines.push_back({renderPipeline, colorFormat, sampleCount, stencilFormat});
     return fMSAALoadPipelines[fMSAALoadPipelines.count()-1].fPipeline.get();
 }