Don't add the resolve attachment to vulkan render passes.

Also includes some other msaa bug fixes

BUG=skia:5127
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2256843002

Review-Url: https://codereview.chromium.org/2256843002

diff --git a/src/gpu/vk/GrVkFramebuffer.cpp b/src/gpu/vk/GrVkFramebuffer.cpp
index 49c6e41..f9add63 100644 (file)
--- a/src/gpu/vk/GrVkFramebuffer.cpp
+++ b/src/gpu/vk/GrVkFramebuffer.cpp
@@ -15,7 +15,6 @@ GrVkFramebuffer* GrVkFramebuffer::Create(GrVkGpu* gpu,
                                          int width, int height,
                                          const GrVkRenderPass* renderPass,
                                          const GrVkImageView* colorAttachment,
-                                         const GrVkImageView* resolveAttachment,
                                          const GrVkImageView* stencilAttachment) {
     // At the very least we need a renderPass and a colorAttachment
     SkASSERT(renderPass);
@@ -24,9 +23,6 @@ GrVkFramebuffer* GrVkFramebuffer::Create(GrVkGpu* gpu,
     VkImageView attachments[3];
     attachments[0] = colorAttachment->imageView();
     int numAttachments = 1;
-    if (resolveAttachment) {
-        attachments[numAttachments++] = resolveAttachment->imageView();
-    }
     if (stencilAttachment) {
         attachments[numAttachments++] = stencilAttachment->imageView();
     }

diff --git a/src/gpu/vk/GrVkFramebuffer.h b/src/gpu/vk/GrVkFramebuffer.h
index e16ac22..b0f4beb 100644 (file)
--- a/src/gpu/vk/GrVkFramebuffer.h
+++ b/src/gpu/vk/GrVkFramebuffer.h
@@ -24,7 +24,6 @@ public:
                                    int width, int height,
                                    const GrVkRenderPass* renderPass,
                                    const GrVkImageView* colorAttachment,
-                                   const GrVkImageView* resolveAttachment,
                                    const GrVkImageView* stencilAttachment);
 
     VkFramebuffer framebuffer() const { return fFramebuffer; }

diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 0a713f4..495b85e 100644 (file)
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -401,7 +401,7 @@ void GrVkGpu::onResolveRenderTarget(GrRenderTarget* target) {
                                         VK_PIPELINE_STAGE_TRANSFER_BIT,
                                         false);
 
-        fCurrentCmdBuffer->resolveImage(this, *rt, *rt->msaaImage(), 1, &resolveInfo);
+        fCurrentCmdBuffer->resolveImage(this, *rt->msaaImage(), *rt, 1, &resolveInfo);
 
         rt->flagAsResolved();
     }
@@ -1800,6 +1800,6 @@ void GrVkGpu::submitSecondaryCommandBuffer(GrVkSecondaryCommandBuffer* buffer,
     fCurrentCmdBuffer->executeCommands(this, buffer);
     fCurrentCmdBuffer->endRenderPass(this);
 
-    this->didWriteToSurface(target, pBounds);
+    this->didWriteToSurface(target, &bounds);
 }
 

diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
index 77a038a..f49b180 100644 (file)
--- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp
@@ -100,12 +100,15 @@ void GrVkGpuCommandBuffer::end() {
 }
 
 void GrVkGpuCommandBuffer::onSubmit(const SkIRect& bounds) {
-    // Change layout of our render target so it can be used as the color attachment
-    fRenderTarget->setImageLayout(fGpu,
-                                  VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
-                                  VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
-                                  VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT,
-                                  false);
+    // Change layout of our render target so it can be used as the color attachment. Currently
+    // we don't attach the resolve to the framebuffer so no need to change its layout.
+    GrVkImage* targetImage = fRenderTarget->msaaImage() ? fRenderTarget->msaaImage() 
+                                                        : fRenderTarget;
+    targetImage->setImageLayout(fGpu,
+                                VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
+                                VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
+                                VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT,
+                                false);
 
     // If we are using a stencil attachment we also need to update its layout
     if (GrStencilAttachment* stencil = fRenderTarget->renderTargetPriv().getStencilAttachment()) {
@@ -118,14 +121,6 @@ void GrVkGpuCommandBuffer::onSubmit(const SkIRect& bounds) {
                                   false);
     }
 
-    if (GrVkImage* msaaImage = fRenderTarget->msaaImage()) {
-        msaaImage->setImageLayout(fGpu,
-                                  VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
-                                  VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
-                                  VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT,
-                                  false);
-    }
-
     for (int i = 0; i < fSampledImages.count(); ++i) {
         fSampledImages[i]->setImageLayout(fGpu,
                                           VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL,

diff --git a/src/gpu/vk/GrVkRenderPass.cpp b/src/gpu/vk/GrVkRenderPass.cpp
index ba325ef..ee2d3d9 100644 (file)
--- a/src/gpu/vk/GrVkRenderPass.cpp
+++ b/src/gpu/vk/GrVkRenderPass.cpp
@@ -46,12 +46,11 @@ void GrVkRenderPass::initSimple(const GrVkGpu* gpu, const GrVkRenderTarget& targ
     static const GrVkRenderPass::LoadStoreOps kBasicLoadStoreOps(VK_ATTACHMENT_LOAD_OP_LOAD,
                                                                  VK_ATTACHMENT_STORE_OP_STORE);
 
-    this->init(gpu, target, kBasicLoadStoreOps, kBasicLoadStoreOps, kBasicLoadStoreOps);
+    this->init(gpu, target, kBasicLoadStoreOps, kBasicLoadStoreOps);
 }
 
 void GrVkRenderPass::init(const GrVkGpu* gpu,
                           const LoadStoreOps& colorOp,
-                          const LoadStoreOps& resolveOp,
                           const LoadStoreOps& stencilOp) {
     uint32_t numAttachments = fAttachmentsDescriptor.fAttachmentCount;
     // Attachment descriptions to be set on the render pass
@@ -62,11 +61,10 @@ void GrVkRenderPass::init(const GrVkGpu* gpu,
     // Refs to attachments on the render pass (as described by teh VkAttachmentDescription above),
     // that are used by the subpass.
     VkAttachmentReference colorRef;
-    VkAttachmentReference resolveRef;
     VkAttachmentReference stencilRef;
     uint32_t currentAttachment = 0;
 
-    // Go through each of the attachment types (color, resolve, stencil) and set the necessary
+    // Go through each of the attachment types (color, stencil) and set the necessary
     // on the various Vk structs.
     VkSubpassDescription subpassDesc;
     memset(&subpassDesc, 0, sizeof(VkSubpassDescription));
@@ -74,6 +72,8 @@ void GrVkRenderPass::init(const GrVkGpu* gpu,
     subpassDesc.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS;
     subpassDesc.inputAttachmentCount = 0;
     subpassDesc.pInputAttachments = nullptr;
+    subpassDesc.pResolveAttachments = nullptr;
+
     if (fAttachmentFlags & kColor_AttachmentFlag) {
         // set up color attachment
         fAttachmentsDescriptor.fColor.fLoadStoreOps = colorOp;
@@ -93,21 +93,6 @@ void GrVkRenderPass::init(const GrVkGpu* gpu,
     }
     subpassDesc.pColorAttachments = &colorRef;
 
-    if (fAttachmentFlags & kResolve_AttachmentFlag) {
-        // set up resolve attachment
-        fAttachmentsDescriptor.fResolve.fLoadStoreOps = resolveOp;
-        setup_vk_attachment_description(&attachments[currentAttachment],
-                                        fAttachmentsDescriptor.fResolve,
-                                        VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
-        // setup subpass use of attachment
-        resolveRef.attachment = currentAttachment++;
-        // I'm really not sure what the layout should be for the resolve textures.
-        resolveRef.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
-        subpassDesc.pResolveAttachments = &resolveRef;
-    } else {
-        subpassDesc.pResolveAttachments = nullptr;
-    }
-
     if (fAttachmentFlags & kStencil_AttachmentFlag) {
         // set up stencil attachment
         fAttachmentsDescriptor.fStencil.fLoadStoreOps = stencilOp;
@@ -155,22 +140,20 @@ void GrVkRenderPass::init(const GrVkGpu* gpu,
 void GrVkRenderPass::init(const GrVkGpu* gpu,
                           const GrVkRenderPass& compatibleRenderPass,
                           const LoadStoreOps& colorOp,
-                          const LoadStoreOps& resolveOp,
                           const LoadStoreOps& stencilOp) {
     fAttachmentFlags = compatibleRenderPass.fAttachmentFlags;
     fAttachmentsDescriptor = compatibleRenderPass.fAttachmentsDescriptor;
-    this->init(gpu, colorOp, resolveOp, stencilOp);
+    this->init(gpu, colorOp, stencilOp);
 }
 
 void GrVkRenderPass::init(const GrVkGpu* gpu,
                           const GrVkRenderTarget& target, 
                           const LoadStoreOps& colorOp,
-                          const LoadStoreOps& resolveOp,
                           const LoadStoreOps& stencilOp) {
     // Get attachment information from render target. This includes which attachments the render
-    // target has (color, resolve, stencil) and the attachments format and sample count.
+    // target has (color, stencil) and the attachments format and sample count.
     target.getAttachmentsDescriptor(&fAttachmentsDescriptor, &fAttachmentFlags);
-    this->init(gpu, colorOp, resolveOp, stencilOp);
+    this->init(gpu, colorOp, stencilOp);
 }
 
 void GrVkRenderPass::freeGPUData(const GrVkGpu* gpu) const {
@@ -187,18 +170,6 @@ bool GrVkRenderPass::colorAttachmentIndex(uint32_t* index) const {
     return false;
 }
 
-// Works under the assumption that resolve attachment will always be after the color attachment.
-bool GrVkRenderPass::resolveAttachmentIndex(uint32_t* index) const {
-    *index = 0;
-    if (fAttachmentFlags & kColor_AttachmentFlag) {
-        ++(*index);
-    }
-    if (fAttachmentFlags & kResolve_AttachmentFlag) {
-        return true;
-    }
-    return false;
-}
-
 // Works under the assumption that stencil attachment will always be after the color and resolve
 // attachment.
 bool GrVkRenderPass::stencilAttachmentIndex(uint32_t* index) const {
@@ -206,9 +177,6 @@ bool GrVkRenderPass::stencilAttachmentIndex(uint32_t* index) const {
     if (fAttachmentFlags & kColor_AttachmentFlag) {
         ++(*index);
     }
-    if (fAttachmentFlags & kResolve_AttachmentFlag) {
-        ++(*index);
-    }
     if (fAttachmentFlags & kStencil_AttachmentFlag) {
         return true;
     }
@@ -249,11 +217,6 @@ bool GrVkRenderPass::isCompatible(const AttachmentsDescriptor& desc,
             return false;
         }
     }
-    if (fAttachmentFlags & kResolve_AttachmentFlag) {
-        if (!fAttachmentsDescriptor.fResolve.isCompatible(desc.fResolve)) {
-            return false;
-        }
-    }
     if (fAttachmentFlags & kStencil_AttachmentFlag) {
         if (!fAttachmentsDescriptor.fStencil.isCompatible(desc.fStencil)) {
             return false;
@@ -276,18 +239,12 @@ bool GrVkRenderPass::isCompatible(const GrVkRenderPass& renderPass) const {
 }
 
 bool GrVkRenderPass::equalLoadStoreOps(const LoadStoreOps& colorOps,
-                                       const LoadStoreOps& resolveOps,
                                        const LoadStoreOps& stencilOps) const {
     if (fAttachmentFlags & kColor_AttachmentFlag) {
         if (fAttachmentsDescriptor.fColor.fLoadStoreOps != colorOps) {
             return false;
         }
     }
-    if (fAttachmentFlags & kResolve_AttachmentFlag) {
-        if (fAttachmentsDescriptor.fResolve.fLoadStoreOps != resolveOps) {
-            return false;
-        }
-    }
     if (fAttachmentFlags & kStencil_AttachmentFlag) {
         if (fAttachmentsDescriptor.fStencil.fLoadStoreOps != stencilOps) {
             return false;
@@ -302,10 +259,6 @@ void GrVkRenderPass::genKey(GrProcessorKeyBuilder* b) const {
         b->add32(fAttachmentsDescriptor.fColor.fFormat);
         b->add32(fAttachmentsDescriptor.fColor.fSamples);
     }
-    if (fAttachmentFlags & kResolve_AttachmentFlag) {
-        b->add32(fAttachmentsDescriptor.fResolve.fFormat);
-        b->add32(fAttachmentsDescriptor.fResolve.fSamples);
-    }
     if (fAttachmentFlags & kStencil_AttachmentFlag) {
         b->add32(fAttachmentsDescriptor.fStencil.fFormat);
         b->add32(fAttachmentsDescriptor.fStencil.fSamples);

diff --git a/src/gpu/vk/GrVkRenderPass.h b/src/gpu/vk/GrVkRenderPass.h
index 21146ab..d59b5fa 100644 (file)
--- a/src/gpu/vk/GrVkRenderPass.h
+++ b/src/gpu/vk/GrVkRenderPass.h
@@ -43,13 +43,11 @@ public:
     void init(const GrVkGpu* gpu,
               const GrVkRenderTarget& target,
               const LoadStoreOps& colorOp,
-              const LoadStoreOps& resolveOp,
               const LoadStoreOps& stencilOp);
 
     void init(const GrVkGpu* gpu,
               const GrVkRenderPass& compatibleRenderPass,
               const LoadStoreOps& colorOp,
-              const LoadStoreOps& resolveOp,
               const LoadStoreOps& stencilOp);
 
     struct AttachmentsDescriptor {
@@ -75,15 +73,13 @@ public:
             }
         };
         AttachmentDesc fColor;
-        AttachmentDesc fResolve;
         AttachmentDesc fStencil;
         uint32_t       fAttachmentCount;
     };
 
     enum AttachmentFlags {
         kColor_AttachmentFlag = 0x1,
-        kResolve_AttachmentFlag = 0x2,
-        kStencil_AttachmentFlag = 0x4,
+        kStencil_AttachmentFlag = 0x2,
     };
     GR_DECL_BITFIELD_OPS_FRIENDS(AttachmentFlags);
 
@@ -91,7 +87,6 @@ public:
     // If the render pass does not have the given attachment it will return false and not set the
     // index value.
     bool colorAttachmentIndex(uint32_t* index) const;
-    bool resolveAttachmentIndex(uint32_t* index) const;
     bool stencilAttachmentIndex(uint32_t* index) const;
 
     // Sets the VkRenderPassBeginInfo and VkRenderPassContents need to begin a render pass.
@@ -113,7 +108,6 @@ public:
     bool isCompatible(const GrVkRenderPass& renderPass) const;
 
     bool equalLoadStoreOps(const LoadStoreOps& colorOps,
-                           const LoadStoreOps& resolveOps,
                            const LoadStoreOps& stencilOps) const;
 
     VkRenderPass vkRenderPass() const { return fRenderPass; }
@@ -133,7 +127,6 @@ private:
 
     void init(const GrVkGpu* gpu,
               const LoadStoreOps& colorOps,
-              const LoadStoreOps& resolveOps,
               const LoadStoreOps& stencilOps);
 
     bool isCompatible(const AttachmentsDescriptor&, const AttachmentFlags&) const;

diff --git a/src/gpu/vk/GrVkRenderTarget.cpp b/src/gpu/vk/GrVkRenderTarget.cpp
index dc966ca..59fc0b9 100644 (file)
--- a/src/gpu/vk/GrVkRenderTarget.cpp
+++ b/src/gpu/vk/GrVkRenderTarget.cpp
@@ -239,7 +239,7 @@ void GrVkRenderTarget::createFramebuffer(GrVkGpu* gpu) {
     const GrVkImageView* stencilView = this->stencilAttachmentView();
     fFramebuffer = GrVkFramebuffer::Create(gpu, this->width(), this->height(),
                                            fCachedSimpleRenderPass, fColorAttachmentView,
-                                           fResolveAttachmentView, stencilView);
+                                           stencilView);
     SkASSERT(fFramebuffer);
 }
 
@@ -253,12 +253,6 @@ void GrVkRenderTarget::getAttachmentsDescriptor(
     desc->fColor.fSamples = colorSamples ? colorSamples : 1;
     *attachmentFlags = GrVkRenderPass::kColor_AttachmentFlag;
     uint32_t attachmentCount = 1;
-    if (colorSamples > 0) {
-        desc->fResolve.fFormat = colorFormat;
-        desc->fResolve.fSamples = 1;
-        *attachmentFlags |= GrVkRenderPass::kResolve_AttachmentFlag;
-        ++attachmentCount;
-    }
 
     const GrStencilAttachment* stencil = this->renderTargetPriv().getStencilAttachment();
     if (stencil) {
@@ -284,12 +278,9 @@ GrVkRenderTarget::~GrVkRenderTarget() {
 
 void GrVkRenderTarget::addResources(GrVkCommandBuffer& commandBuffer) const {
     commandBuffer.addResource(this->framebuffer());
-    commandBuffer.addResource(this->resource());
     commandBuffer.addResource(this->colorAttachmentView());
-    if (this->msaaImageResource()) {
-        commandBuffer.addResource(this->msaaImageResource());
-        commandBuffer.addResource(this->resolveAttachmentView());
-    }
+    commandBuffer.addResource(this->msaaImageResource() ? this->msaaImageResource()
+                                                        : this->resource());
     if (this->stencilImageResource()) {
         commandBuffer.addResource(this->stencilImageResource());
         commandBuffer.addResource(this->stencilAttachmentView());

diff --git a/src/gpu/vk/GrVkResourceProvider.cpp b/src/gpu/vk/GrVkResourceProvider.cpp
index e581384..8ae5162 100644 (file)
--- a/src/gpu/vk/GrVkResourceProvider.cpp
+++ b/src/gpu/vk/GrVkResourceProvider.cpp
@@ -130,7 +130,6 @@ GrVkResourceProvider::findRenderPass(const CompatibleRPHandle& compatibleHandle,
     CompatibleRenderPassSet& compatibleSet = fRenderPassArray[compatibleHandle.toIndex()];
     const GrVkRenderPass* renderPass = compatibleSet.getRenderPass(fGpu,
                                                                    colorOps,
-                                                                   resolveOps,
                                                                    stencilOps);
     renderPass->ref();
     return renderPass;
@@ -417,17 +416,16 @@ bool GrVkResourceProvider::CompatibleRenderPassSet::isCompatible(
 GrVkRenderPass* GrVkResourceProvider::CompatibleRenderPassSet::getRenderPass(
                                                    const GrVkGpu* gpu,
                                                    const GrVkRenderPass::LoadStoreOps& colorOps,
-                                                   const GrVkRenderPass::LoadStoreOps& resolveOps,
                                                    const GrVkRenderPass::LoadStoreOps& stencilOps) {
     for (int i = 0; i < fRenderPasses.count(); ++i) {
         int idx = (i + fLastReturnedIndex) % fRenderPasses.count();
-        if (fRenderPasses[idx]->equalLoadStoreOps(colorOps, resolveOps, stencilOps)) {
+        if (fRenderPasses[idx]->equalLoadStoreOps(colorOps, stencilOps)) {
             fLastReturnedIndex = idx;
             return fRenderPasses[idx];
         }
     }
     GrVkRenderPass* renderPass = fRenderPasses.emplace_back(new GrVkRenderPass());
-    renderPass->init(gpu, *this->getCompatibleRenderPass(), colorOps, resolveOps, stencilOps);
+    renderPass->init(gpu, *this->getCompatibleRenderPass(), colorOps, stencilOps);
     fLastReturnedIndex = fRenderPasses.count() - 1;
     return renderPass;
 }

diff --git a/src/gpu/vk/GrVkResourceProvider.h b/src/gpu/vk/GrVkResourceProvider.h
index 7f6ce6d..19d493a 100644 (file)
--- a/src/gpu/vk/GrVkResourceProvider.h
+++ b/src/gpu/vk/GrVkResourceProvider.h
@@ -209,7 +209,6 @@ private:
 
         GrVkRenderPass* getRenderPass(const GrVkGpu* gpu,
                                       const GrVkRenderPass::LoadStoreOps& colorOps,
-                                      const GrVkRenderPass::LoadStoreOps& resolveOps,
                                       const GrVkRenderPass::LoadStoreOps& stencilOps);
 
         void releaseResources(const GrVkGpu* gpu);

diff --git a/src/gpu/vk/GrVkTextureRenderTarget.cpp b/src/gpu/vk/GrVkTextureRenderTarget.cpp
index bee6a04..532d0a2 100644 (file)
--- a/src/gpu/vk/GrVkTextureRenderTarget.cpp
+++ b/src/gpu/vk/GrVkTextureRenderTarget.cpp
@@ -53,6 +53,7 @@ GrVkTextureRenderTarget* GrVkTextureRenderTarget::Create(GrVkGpu* gpu,
         msImageDesc.fMemProps = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
 
         if (!GrVkImage::InitImageInfo(gpu, msImageDesc, &msInfo)) {
+            imageView->unref(gpu);
             return nullptr;
         }