Vulkan: Support invalidate of MSRTT attachments

src/libANGLE/renderer/vulkan/vk_cache_utils.cpp

@@ -165,7 +165,8 @@ void UnpackAttachmentDesc(VkAttachmentDescription *desc,
 
 void UnpackColorResolveAttachmentDesc(VkAttachmentDescription *desc,
                                       const vk::Format &format,
-                                      bool usedAsInputAttachment)
+                                      bool usedAsInputAttachment,
+                                      bool isInvalidated)
 {
     // We would only need this flag for duplicated attachments. Apply it conservatively.  In
     // practice it's unlikely any application would use the same image as multiple resolve
@@ -184,12 +185,11 @@ void UnpackColorResolveAttachmentDesc(VkAttachmentDescription *desc,
     // attachment (i.e. needs to be unresolved), loadOp needs to be set to LOAD, otherwise it should
     // be DONT_CARE as it gets overwritten during resolve.
     //
-    // storeOp should be STORE.  If the attachment is invalidated, it would get set to DONT_CARE.
-    // TODO(syoussefi): currently invalidate doesn't affect storeOp of resolve attachment.
+    // storeOp should be STORE.  If the attachment is invalidated, it is set to DONT_CARE.
     desc->samples = VK_SAMPLE_COUNT_1_BIT;
     desc->loadOp =
         usedAsInputAttachment ? VK_ATTACHMENT_LOAD_OP_LOAD : VK_ATTACHMENT_LOAD_OP_DONT_CARE;
-    desc->storeOp        = VK_ATTACHMENT_STORE_OP_STORE;
+    desc->storeOp = isInvalidated ? VK_ATTACHMENT_STORE_OP_DONT_CARE : VK_ATTACHMENT_STORE_OP_STORE;
     desc->stencilLoadOp  = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
     desc->stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
     desc->initialLayout  = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
@@ -199,7 +199,9 @@ void UnpackColorResolveAttachmentDesc(VkAttachmentDescription *desc,
 void UnpackDepthStencilResolveAttachmentDesc(VkAttachmentDescription *desc,
                                              const vk::Format &format,
                                              bool usedAsDepthInputAttachment,
-                                             bool usedAsStencilInputAttachment)
+                                             bool usedAsStencilInputAttachment,
+                                             bool isDepthInvalidated,
+                                             bool isStencilInvalidated)
 {
     // There cannot be simultaneous usages of the depth/stencil resolve image, as depth/stencil
     // resolve currently only comes from depth/stencil renderbuffers.
@@ -210,17 +212,22 @@ void UnpackDepthStencilResolveAttachmentDesc(VkAttachmentDescription *desc,
     const angle::Format &angleFormat = format.intendedFormat();
     ASSERT(angleFormat.depthBits != 0 || angleFormat.stencilBits != 0);
 
+    // Missing aspects are folded in is*Invalidated parameters, so no need to double check.
+    ASSERT(angleFormat.depthBits > 0 || isDepthInvalidated);
+    ASSERT(angleFormat.stencilBits > 0 || isStencilInvalidated);
+
     // Similarly to color resolve attachments, sample count is 1, loadOp is LOAD or DONT_CARE based
-    // on whether unresolve is required, and storeOp is STORE (if aspect exists).
+    // on whether unresolve is required, and storeOp is STORE or DONT_CARE based on whether the
+    // attachment is invalidated or the aspect exists.
     desc->samples = VK_SAMPLE_COUNT_1_BIT;
     desc->loadOp =
         usedAsDepthInputAttachment ? VK_ATTACHMENT_LOAD_OP_LOAD : VK_ATTACHMENT_LOAD_OP_DONT_CARE;
     desc->storeOp =
-        angleFormat.depthBits > 0 ? VK_ATTACHMENT_STORE_OP_STORE : VK_ATTACHMENT_STORE_OP_DONT_CARE;
+        isDepthInvalidated ? VK_ATTACHMENT_STORE_OP_DONT_CARE : VK_ATTACHMENT_STORE_OP_STORE;
     desc->stencilLoadOp =
         usedAsStencilInputAttachment ? VK_ATTACHMENT_LOAD_OP_LOAD : VK_ATTACHMENT_LOAD_OP_DONT_CARE;
-    desc->stencilStoreOp = angleFormat.stencilBits > 0 ? VK_ATTACHMENT_STORE_OP_STORE
-                                                       : VK_ATTACHMENT_STORE_OP_DONT_CARE;
+    desc->stencilStoreOp =
+        isStencilInvalidated ? VK_ATTACHMENT_STORE_OP_DONT_CARE : VK_ATTACHMENT_STORE_OP_STORE;
     desc->initialLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
     desc->finalLayout   = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
 }
@@ -882,8 +889,15 @@ angle::Result InitializeRenderPassFromDesc(ContextVk *contextVk,
     // The list of attachments includes all non-resolve and resolve attachments.
     FramebufferAttachmentArray<VkAttachmentDescription> attachmentDescs;
 
+    // Track invalidated attachments so their resolve attachments can be invalidated as well.
+    // Resolve attachments can be removed in that case if the render pass has only one subpass
+    // (which is the case if there are no unresolve attachments).
+    gl::DrawBufferMask isColorInvalidated;
+    bool isDepthInvalidated   = false;
+    bool isStencilInvalidated = false;
     const bool hasUnresolveAttachments =
         desc.getColorUnresolveAttachmentMask().any() || desc.hasDepthStencilUnresolveAttachment();
+    const bool canRemoveResolveAttachments = !hasUnresolveAttachments;
 
     // Pack color attachments
     PackedAttachmentIndex attachmentCount(0);
@@ -917,6 +931,8 @@ angle::Result InitializeRenderPassFromDesc(ContextVk *contextVk,
         UnpackAttachmentDesc(&attachmentDescs[attachmentCount.get()], format, desc.samples(),
                              ops[attachmentCount]);
 
+        isColorInvalidated.set(colorIndexGL, ops[attachmentCount].isInvalidated);
+
         ++attachmentCount;
     }
 
@@ -936,6 +952,9 @@ angle::Result InitializeRenderPassFromDesc(ContextVk *contextVk,
         UnpackAttachmentDesc(&attachmentDescs[attachmentCount.get()], format, desc.samples(),
                              ops[attachmentCount]);
 
+        isDepthInvalidated   = ops[attachmentCount].isInvalidated;
+        isStencilInvalidated = ops[attachmentCount].isStencilInvalidated;
+
         ++attachmentCount;
     }
 
@@ -957,10 +976,19 @@ angle::Result InitializeRenderPassFromDesc(ContextVk *contextVk,
         colorRef.attachment = attachmentCount.get();
         colorRef.layout     = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
 
+        // If color attachment is invalidated, try to remove its resolve attachment altogether.
+        if (canRemoveResolveAttachments && isColorInvalidated.test(colorIndexGL))
+        {
+            colorResolveAttachmentRefs.push_back(kUnusedAttachment);
+        }
+        else
+        {
             colorResolveAttachmentRefs.push_back(colorRef);
+        }
 
         UnpackColorResolveAttachmentDesc(&attachmentDescs[attachmentCount.get()], format,
-                                         desc.hasColorUnresolveAttachment(colorIndexGL));
+                                         desc.hasColorUnresolveAttachment(colorIndexGL),
+                                         isColorInvalidated.test(colorIndexGL));
 
         ++attachmentCount;
     }
@@ -973,15 +1001,34 @@ angle::Result InitializeRenderPassFromDesc(ContextVk *contextVk,
         uint32_t depthStencilIndexGL = static_cast<uint32_t>(desc.depthStencilAttachmentIndex());
 
         const vk::Format &format         = renderer->getFormat(desc[depthStencilIndexGL]);
+        const angle::Format &angleFormat = format.intendedFormat();
+
+        // Treat missing aspect as invalidated for the purpose of the resolve attachment.
+        if (angleFormat.depthBits == 0)
+        {
+            isDepthInvalidated = true;
+        }
+        if (angleFormat.stencilBits == 0)
+        {
+            isStencilInvalidated = true;
+        }
 
         depthStencilResolveAttachmentRef.attachment = attachmentCount.get();
         depthStencilResolveAttachmentRef.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
-        depthStencilResolveAttachmentRef.aspectMask =
-            VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT;
+        depthStencilResolveAttachmentRef.aspectMask = 0;
+
+        if (!isDepthInvalidated)
+        {
+            depthStencilResolveAttachmentRef.aspectMask |= VK_IMAGE_ASPECT_DEPTH_BIT;
+        }
+        if (!isStencilInvalidated)
+        {
+            depthStencilResolveAttachmentRef.aspectMask |= VK_IMAGE_ASPECT_STENCIL_BIT;
+        }
 
-        UnpackDepthStencilResolveAttachmentDesc(&attachmentDescs[attachmentCount.get()], format,
-                                                desc.hasDepthUnresolveAttachment(),
-                                                desc.hasStencilUnresolveAttachment());
+        UnpackDepthStencilResolveAttachmentDesc(
+            &attachmentDescs[attachmentCount.get()], format, desc.hasDepthUnresolveAttachment(),
+            desc.hasStencilUnresolveAttachment(), isDepthInvalidated, isStencilInvalidated);
 
         ++attachmentCount;
     }
@@ -1038,8 +1085,16 @@ angle::Result InitializeRenderPassFromDesc(ContextVk *contextVk,
         depthStencilResolve.stencilResolveMode = desc.hasStencilResolveAttachment()
                                                      ? VK_RESOLVE_MODE_SAMPLE_ZERO_BIT
                                                      : VK_RESOLVE_MODE_NONE;
+
+        // If depth/stencil attachment is invalidated, try to remove its resolve attachment
+        // altogether.
+        const bool removeDepthStencilResolve =
+            canRemoveResolveAttachments && isDepthInvalidated && isStencilInvalidated;
+        if (!removeDepthStencilResolve)
+        {
             depthStencilResolve.pDepthStencilResolveAttachment = &depthStencilResolveAttachmentRef;
         }
+    }
 
     std::vector<VkSubpassDependency> subpassDependencies;
     if (hasUnresolveAttachments)
@@ -1067,7 +1122,7 @@ angle::Result InitializeRenderPassFromDesc(ContextVk *contextVk,
 
     // If depth/stencil resolve is used, we need to create the render pass with
     // vkCreateRenderPass2KHR.
-    if (desc.hasDepthStencilResolveAttachment())
+    if (depthStencilResolve.pDepthStencilResolveAttachment != nullptr)
     {
         ANGLE_TRY(CreateRenderPass2(
             contextVk, createInfo, depthStencilResolve, desc.hasDepthUnresolveAttachment(),
@@ -2441,6 +2496,7 @@ void AttachmentOpsArray::setOps(PackedAttachmentIndex index,
     PackedAttachmentOpsDesc &ops = mOps[index.get()];
     SetBitField(ops.loadOp, loadOp);
     SetBitField(ops.storeOp, storeOp);
+    ops.isInvalidated = false;
 }
 
 void AttachmentOpsArray::setStencilOps(PackedAttachmentIndex index,
@@ -2450,6 +2506,7 @@ void AttachmentOpsArray::setStencilOps(PackedAttachmentIndex index,
     PackedAttachmentOpsDesc &ops = mOps[index.get()];
     SetBitField(ops.stencilLoadOp, loadOp);
     SetBitField(ops.stencilStoreOp, storeOp);
+    ops.isStencilInvalidated = false;
 }
 
 void AttachmentOpsArray::setClearOp(PackedAttachmentIndex index)

src/libANGLE/renderer/vulkan/vk_cache_utils.h

@@ -289,8 +289,14 @@ struct PackedAttachmentOpsDesc final
     uint16_t stencilLoadOp : 2;
     uint16_t stencilStoreOp : 1;
 
-    // Reserved for use with multisampled-render-to-texture invalidate.
-    uint16_t reserved : 2;
+    // If a corresponding resolve attachment exists, storeOp may already be DONT_CARE, and it's
+    // unclear whether the attachment was invalidated or not.  This information is passed along here
+    // so that the resolve attachment's storeOp can be set to DONT_CARE if the attachment is
+    // invalidated, and if possible removed from the list of resolve attachments altogether.  Note
+    // that the latter may not be possible if the render pass has multiple subpasses due to Vulkan
+    // render pass compatibility rules.
+    uint16_t isInvalidated : 1;
+    uint16_t isStencilInvalidated : 1;
 
     // 4-bits to force pad the structure to exactly 2 bytes.  Note that we currently don't support
     // any of the extension layouts, whose values start at 1'000'000'000.

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -1033,10 +1033,12 @@ void CommandBufferHelper::endRenderPass(ContextVk *contextVk)
     if (isInvalidated(mDepthCmdSizeInvalidated, mDepthCmdSizeDisabled))
     {
         dsOps.storeOp       = VK_ATTACHMENT_STORE_OP_DONT_CARE;
+        dsOps.isInvalidated = true;
     }
     if (isInvalidated(mStencilCmdSizeInvalidated, mStencilCmdSizeDisabled))
     {
         dsOps.stencilStoreOp       = VK_ATTACHMENT_STORE_OP_DONT_CARE;
+        dsOps.isStencilInvalidated = true;
     }
 
     // Second, if we are loading or clearing the attachment, but the attachment has not been used,

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1025,6 +1025,7 @@ class CommandBufferHelper : angle::NonCopyable
     {
         ASSERT(mIsRenderPassCommandBuffer);
         SetBitField(mAttachmentOps[attachmentIndex].storeOp, VK_ATTACHMENT_STORE_OP_DONT_CARE);
+        mAttachmentOps[attachmentIndex].isInvalidated = true;
     }
 
     void invalidateRenderPassDepthAttachment(const gl::DepthStencilState &dsState)

src/tests/gl_tests/VulkanPerformanceCounterTest.cpp

@@ -1854,6 +1854,134 @@ TEST_P(VulkanPerformanceCounterTest, RenderToTextureDepthStencilRenderbufferShou
     EXPECT_PIXEL_COLOR_EQ(kSize / 2, kSize / 2, GLColor::yellow);
 }
 
+// Tests counters when multisampled-render-to-texture color/depth/stencil renderbuffers are
+// invalidated.
+TEST_P(VulkanPerformanceCounterTest, RenderToTextureInvalidate)
+{
+    // http://anglebug.com/5083
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsAMD() && IsVulkan());
+
+    ANGLE_SKIP_TEST_IF(!EnsureGLExtensionEnabled("GL_EXT_multisampled_render_to_texture"));
+
+    const rx::vk::PerfCounters &counters = hackANGLE();
+    rx::vk::PerfCounters expected;
+
+    // This test creates 4 render passes. In the first render pass, color, depth and stencil are
+    // cleared.  After every render pass, the attachments are invalidated.  In the following render
+    // passes thus they are not loaded (rather unresolved, as the attachments are
+    // multisampled-render-to-texture).  Due to the invalidate call, neither of the 4 render passes
+    // should resolve the attachments.
+
+    // Expect rpCount+4, depth(Clears+1, Loads+0, Stores+0), stencil(Clears+1, Load+0, Stores+0)
+    setExpectedCountersForInvalidateTest(counters, 4, 1, 0, 0, 1, 0, 0, &expected);
+
+    // Additionally, expect no resolve and unresolve.
+    setExpectedCountersForUnresolveResolveTest(counters, 0, 0, 0, 0, 0, 0, &expected);
+
+    GLFramebuffer FBO;
+    glBindFramebuffer(GL_FRAMEBUFFER, FBO);
+
+    constexpr GLsizei kSize = 6;
+
+    // Create multisampled framebuffer to draw into, with both color and depth attachments.
+    GLTexture colorMS;
+    glBindTexture(GL_TEXTURE_2D, colorMS);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+
+    GLRenderbuffer depthStencilMS;
+    glBindRenderbuffer(GL_RENDERBUFFER, depthStencilMS);
+    glRenderbufferStorageMultisampleEXT(GL_RENDERBUFFER, 4, GL_DEPTH24_STENCIL8, kSize, kSize);
+
+    GLFramebuffer fboMS;
+    glBindFramebuffer(GL_FRAMEBUFFER, fboMS);
+    glFramebufferTexture2DMultisampleEXT(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
+                                         colorMS, 0, 4);
+    glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
+                              depthStencilMS);
+    ASSERT_GL_NO_ERROR();
+
+    // Set up texture for copy operation that breaks the render pass
+    GLTexture copyTex;
+    glBindTexture(GL_TEXTURE_2D, copyTex);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
+
+    // Set viewport and clear color, depth and stencil
+    glViewport(0, 0, kSize, kSize);
+    glClearColor(0, 0, 0, 1.0f);
+    glClearDepthf(1);
+    glClearStencil(0x55);
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+
+    // Output depth/stencil, but disable testing so all draw calls succeed
+    glEnable(GL_DEPTH_TEST);
+    glDepthFunc(GL_ALWAYS);
+
+    glEnable(GL_STENCIL_TEST);
+    glStencilFunc(GL_ALWAYS, 0x55, 0xFF);
+    glStencilOp(GL_REPLACE, GL_REPLACE, GL_REPLACE);
+    glStencilMask(0xFF);
+
+    // Set up program
+    ANGLE_GL_PROGRAM(drawColor, essl1_shaders::vs::Simple(), essl1_shaders::fs::UniformColor());
+    glUseProgram(drawColor);
+    GLint colorUniformLocation =
+        glGetUniformLocation(drawColor, angle::essl1_shaders::ColorUniform());
+    ASSERT_NE(colorUniformLocation, -1);
+
+    // Draw red
+    glUniform4f(colorUniformLocation, 1.0f, 0.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.75f);
+    ASSERT_GL_NO_ERROR();
+
+    // Invalidate everything
+    const GLenum discards[] = {GL_COLOR_ATTACHMENT0, GL_DEPTH_ATTACHMENT, GL_STENCIL_ATTACHMENT};
+    glInvalidateFramebuffer(GL_FRAMEBUFFER, 3, discards);
+
+    // Break the render pass
+    glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 0, 0, kSize / 2, kSize / 2);
+    ASSERT_GL_NO_ERROR();
+
+    // Draw green
+    glUniform4f(colorUniformLocation, 0.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    // Invalidate everything
+    glInvalidateFramebuffer(GL_FRAMEBUFFER, 3, discards);
+
+    // Break the render pass
+    glCopyTexSubImage2D(GL_TEXTURE_2D, 0, kSize / 2, 0, 0, 0, kSize / 2, kSize / 2);
+    ASSERT_GL_NO_ERROR();
+
+    // Draw blue
+    glUniform4f(colorUniformLocation, 0.0f, 0.0f, 1.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.25f);
+    ASSERT_GL_NO_ERROR();
+
+    // Invalidate everything
+    glInvalidateFramebuffer(GL_FRAMEBUFFER, 3, discards);
+
+    // Break the render pass
+    glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, kSize / 2, 0, 0, kSize / 2, kSize / 2);
+    ASSERT_GL_NO_ERROR();
+
+    // Draw yellow
+    glUniform4f(colorUniformLocation, 1.0f, 1.0f, 0.0f, 1.0f);
+    drawQuad(drawColor, essl1_shaders::PositionAttrib(), 0.0f);
+    ASSERT_GL_NO_ERROR();
+
+    // Invalidate everything
+    glInvalidateFramebuffer(GL_FRAMEBUFFER, 3, discards);
+
+    // Break the render pass
+    glCopyTexSubImage2D(GL_TEXTURE_2D, 0, kSize / 2, kSize / 2, 0, 0, kSize / 2, kSize / 2);
+    ASSERT_GL_NO_ERROR();
+
+    // Verify the counters
+    compareLoadCountersForInvalidateTest(counters, expected);
+    compareCountersForUnresolveResolveTest(counters, expected);
+}
+
 // Ensures we use read-only depth layout when there is no write
 TEST_P(VulkanPerformanceCounterTest, ReadOnlyDepthBufferLayout)
 {