Vulkan: Unresolve depth/stencil MSRTT attachments

src/common/bitset_utils.h

@@ -511,6 +511,9 @@ template <size_t N>
 using BitSet8 = BitSetT<N, uint8_t>;
 
 template <size_t N>
+using BitSet16 = BitSetT<N, uint16_t>;
+
+template <size_t N>
 using BitSet32 = BitSetT<N, uint32_t>;
 
 template <size_t N>

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -2243,7 +2243,10 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
     // Initialize RenderPass info.
     vk::AttachmentOpsArray renderPassAttachmentOps;
     vk::PackedClearValuesArray packedClearValues;
-    gl::DrawBufferMask previousUnresolveMask = mRenderPassDesc.getColorUnresolveAttachmentMask();
+    gl::DrawBufferMask previousUnresolveColorMask =
+        mRenderPassDesc.getColorUnresolveAttachmentMask();
+    bool previousUnresolveDepth   = mRenderPassDesc.hasDepthUnresolveAttachment();
+    bool previousUnresolveStencil = mRenderPassDesc.hasStencilUnresolveAttachment();
 
     // Color attachments.
     const auto &colorRenderTargets = mRenderTargetCache.getColors();
@@ -2318,6 +2321,9 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
     RenderTargetVk *depthStencilRenderTarget              = getDepthStencilRenderTarget();
     if (depthStencilRenderTarget)
     {
+        const bool canExportStencil =
+            contextVk->getRenderer()->getFeatures().supportsShaderStencilExport.enabled;
+
         // depth stencil attachment always immediately follows color attachment
         depthStencilAttachmentIndex = colorIndexVk;
 
@@ -2337,14 +2343,16 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
             stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
         }
 
+        // If depth/stencil image is transient, no need to store its data at the end of the render
+        // pass.  If shader stencil export is not supported, stencil data cannot be unresolved on
+        // the next render pass, so it must be stored/loaded.  If the image is entirely transient,
+        // there is no resolve/unresolve and the image data is never stored/loaded.
         if (depthStencilRenderTarget->isImageTransient())
         {
-            // TODO(syoussefi): currently, depth/stencil unresolve is not implemented.  Until then,
-            // don't change storeOp to DONT_CARE, unless the attachment is entirely transient (i.e.
-            // depth textures from EXT_multisampled_render_to_texture2.  http://anglebug.com/4836
-            if (depthStencilRenderTarget->isEntirelyTransient())
+            depthStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
+
+            if (canExportStencil || depthStencilRenderTarget->isEntirelyTransient())
             {
-                depthStoreOp   = VK_ATTACHMENT_STORE_OP_DONT_CARE;
                 stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
             }
         }
@@ -2391,6 +2399,40 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
             depthLoadOp  = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
             depthStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
         }
+
+        // Similar to color attachments, if there's a resolve attachment and the multisampled image
+        // is transient, depth/stencil data need to be unresolved in an initial subpass.
+        //
+        // Note that stencil unresolve is currently only possible if shader stencil export is
+        // supported.
+        if (depthStencilRenderTarget->hasResolveAttachment() &&
+            depthStencilRenderTarget->isImageTransient())
+        {
+            const bool unresolveDepth = depthLoadOp == VK_ATTACHMENT_LOAD_OP_LOAD;
+            const bool unresolveStencil =
+                stencilLoadOp == VK_ATTACHMENT_LOAD_OP_LOAD && canExportStencil;
+
+            if (unresolveDepth)
+            {
+                depthLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
+            }
+
+            if (unresolveStencil)
+            {
+                stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE;
+            }
+
+            if (unresolveDepth || unresolveStencil)
+            {
+                mRenderPassDesc.packDepthStencilUnresolveAttachment(unresolveDepth,
+                                                                    unresolveStencil);
+            }
+            else
+            {
+                mRenderPassDesc.removeDepthStencilUnresolveAttachment();
+            }
+        }
+
         renderPassAttachmentOps.setOps(depthStencilAttachmentIndex, depthLoadOp, depthStoreOp);
         renderPassAttachmentOps.setStencilOps(depthStencilAttachmentIndex, stencilLoadOp,
                                               stencilStoreOp);
@@ -2418,13 +2460,28 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
     // Note that render passes are compatible only if the differences are in loadOp/storeOp values,
     // or the existence of resolve attachments in single subpass render passes.  The modification
     // here can add/remove a subpass, or modify its input attachments.
-    gl::DrawBufferMask unresolveMask = mRenderPassDesc.getColorUnresolveAttachmentMask();
-    if (previousUnresolveMask != unresolveMask)
+    gl::DrawBufferMask unresolveColorMask = mRenderPassDesc.getColorUnresolveAttachmentMask();
+    bool unresolveDepth                   = mRenderPassDesc.hasDepthUnresolveAttachment();
+    bool unresolveStencil                 = mRenderPassDesc.hasStencilUnresolveAttachment();
+    if (previousUnresolveColorMask != unresolveColorMask ||
+        previousUnresolveDepth != unresolveDepth || previousUnresolveStencil != unresolveStencil)
     {
         contextVk->onDrawFramebufferRenderPassDescChange(this);
+
         // Make sure framebuffer is recreated.
         mFramebuffer = nullptr;
-        mCurrentFramebufferDesc.updateColorUnresolveMask(unresolveMask);
+
+        vk::FramebufferNonResolveAttachmentMask unresolveMask(unresolveColorMask.bits());
+        if (unresolveDepth)
+        {
+            unresolveMask.set(gl::IMPLEMENTATION_MAX_DRAW_BUFFERS);
+        }
+        if (unresolveStencil)
+        {
+            unresolveMask.set(gl::IMPLEMENTATION_MAX_DRAW_BUFFERS + 1);
+        }
+
+        mCurrentFramebufferDesc.updateUnresolveMask(unresolveMask);
     }
 
     vk::Framebuffer *framebuffer = nullptr;
@@ -2450,11 +2507,13 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
         depthStencilRenderTarget->onDepthStencilDraw(contextVk, mReadOnlyDepthStencilMode);
     }
 
-    if (unresolveMask.any())
+    if (unresolveColorMask.any() || unresolveDepth || unresolveStencil)
     {
         // Unresolve attachments if any.
         UtilsVk::UnresolveParameters params;
-        params.unresolveMask = unresolveMask;
+        params.unresolveColorMask = unresolveColorMask;
+        params.unresolveDepth     = unresolveDepth;
+        params.unresolveStencil   = unresolveStencil;
 
         ANGLE_TRY(contextVk->getUtils().unresolve(contextVk, this, params));
 

src/libANGLE/renderer/vulkan/RenderTargetVk.cpp

@@ -125,8 +125,8 @@ void RenderTargetVk::onDepthStencilDraw(ContextVk *contextVk, bool isReadOnly)
                                           mImage);
         if (mResolveImage)
         {
-            contextVk->onImageRenderPassWrite(aspectFlags, vk::ImageLayout::DepthStencilAttachment,
-                                              mResolveImage);
+            contextVk->onImageRenderPassWrite(
+                aspectFlags, vk::ImageLayout::DepthStencilResolveAttachment, mResolveImage);
         }
     }
 

src/libANGLE/renderer/vulkan/UtilsVk.h

@@ -166,7 +166,9 @@ class UtilsVk : angle::NonCopyable
 
     struct UnresolveParameters
     {
-        gl::DrawBufferMask unresolveMask;
+        gl::DrawBufferMask unresolveColorMask;
+        bool unresolveDepth;
+        bool unresolveStencil;
     };
 
     // Based on the maximum number of levels in GenerateMipmap.comp.
@@ -406,30 +408,33 @@ class UtilsVk : angle::NonCopyable
         ImageClear  = 0,
         ImageCopy   = 1,
         BlitResolve = 2,
-        // Note: unresolve is special as it has a different layout per attachment
-        Unresolve1Attachment  = 3,
-        Unresolve2Attachments = 4,
-        Unresolve3Attachments = 5,
-        Unresolve4Attachments = 6,
-        Unresolve5Attachments = 7,
-        Unresolve6Attachments = 8,
-        Unresolve7Attachments = 9,
-        Unresolve8Attachments = 10,
+        // Note: unresolve is special as it has a different layout per attachment count.  Depth and
+        // stencil each require a binding, so are counted separately.
+        Unresolve1Attachment   = 3,
+        Unresolve2Attachments  = 4,
+        Unresolve3Attachments  = 5,
+        Unresolve4Attachments  = 6,
+        Unresolve5Attachments  = 7,
+        Unresolve6Attachments  = 8,
+        Unresolve7Attachments  = 9,
+        Unresolve8Attachments  = 10,
+        Unresolve9Attachments  = 11,
+        Unresolve10Attachments = 12,
 
         // Functions implemented in compute
-        ComputeStartIndex          = 11,  // Special value to separate draw and dispatch functions.
-        ConvertIndexBuffer         = 11,
-        ConvertVertexBuffer        = 12,
-        BlitResolveStencilNoExport = 13,
-        OverlayCull                = 14,
-        OverlayDraw                = 15,
-        ConvertIndexIndirectBuffer = 16,
-        ConvertIndexIndirectLineLoopBuffer = 17,
-        ConvertIndirectLineLoopBuffer      = 18,
-        GenerateMipmap                     = 19,
-
-        InvalidEnum = 20,
-        EnumCount   = 20,
+        ComputeStartIndex          = 13,  // Special value to separate draw and dispatch functions.
+        ConvertIndexBuffer         = 13,
+        ConvertVertexBuffer        = 14,
+        BlitResolveStencilNoExport = 15,
+        OverlayCull                = 16,
+        OverlayDraw                = 17,
+        ConvertIndexIndirectBuffer = 18,
+        ConvertIndexIndirectLineLoopBuffer = 19,
+        ConvertIndirectLineLoopBuffer      = 20,
+        GenerateMipmap                     = 21,
+
+        InvalidEnum = 22,
+        EnumCount   = 22,
     };
 
     // Common function that creates the pipeline for the specified function, binds it and prepares

src/libANGLE/renderer/vulkan/vk_cache_utils.h

@@ -118,6 +118,7 @@ using FramebufferAttachmentsVector = angle::FixedVector<T, kMaxFramebufferAttach
 constexpr size_t kMaxFramebufferNonResolveAttachments = gl::IMPLEMENTATION_MAX_DRAW_BUFFERS + 1;
 template <typename T>
 using FramebufferNonResolveAttachmentArray = std::array<T, kMaxFramebufferNonResolveAttachments>;
+using FramebufferNonResolveAttachmentMask  = angle::BitSet16<kMaxFramebufferNonResolveAttachments>;
 
 class alignas(4) RenderPassDesc final
 {
@@ -145,6 +146,10 @@ class alignas(4) RenderPassDesc final
     void removeColorUnresolveAttachment(size_t colorIndexGL);
     // Indicate that a depth/stencil attachment should have a corresponding resolve attachment.
     void packDepthStencilResolveAttachment(bool resolveDepth, bool resolveStencil);
+    // Indicate that a depth/stencil attachment should take its data from the resolve attachment
+    // initially.
+    void packDepthStencilUnresolveAttachment(bool unresolveDepth, bool unresolveStencil);
+    void removeDepthStencilUnresolveAttachment();
 
     size_t hash() const;
 
@@ -182,6 +187,18 @@ class alignas(4) RenderPassDesc final
     {
         return (mAttachmentFormats.back() & kResolveStencilFlag) != 0;
     }
+    bool hasDepthStencilUnresolveAttachment() const
+    {
+        return (mAttachmentFormats.back() & (kUnresolveDepthFlag | kUnresolveStencilFlag)) != 0;
+    }
+    bool hasDepthUnresolveAttachment() const
+    {
+        return (mAttachmentFormats.back() & kUnresolveDepthFlag) != 0;
+    }
+    bool hasStencilUnresolveAttachment() const
+    {
+        return (mAttachmentFormats.back() & kUnresolveStencilFlag) != 0;
+    }
 
     // Get the number of attachments in the Vulkan render pass, i.e. after removing disabled
     // color attachments.
@@ -260,8 +277,10 @@ class alignas(4) RenderPassDesc final
     static constexpr uint8_t kDepthStencilFormatStorageMask = 0x7;
 
     // Flags stored in the upper 5 bits of mAttachmentFormats.back().
-    static constexpr uint8_t kResolveDepthFlag   = 0x80;
-    static constexpr uint8_t kResolveStencilFlag = 0x40;
+    static constexpr uint8_t kResolveDepthFlag     = 0x80;
+    static constexpr uint8_t kResolveStencilFlag   = 0x40;
+    static constexpr uint8_t kUnresolveDepthFlag   = 0x20;
+    static constexpr uint8_t kUnresolveStencilFlag = 0x10;
 };
 
 bool operator==(const RenderPassDesc &lhs, const RenderPassDesc &rhs);
@@ -1061,7 +1080,7 @@ class FramebufferDesc
 
     void updateColor(uint32_t index, ImageViewSubresourceSerial serial);
     void updateColorResolve(uint32_t index, ImageViewSubresourceSerial serial);
-    void updateColorUnresolveMask(gl::DrawBufferMask colorUnresolveMask);
+    void updateUnresolveMask(FramebufferNonResolveAttachmentMask unresolveMask);
     void updateDepthStencil(ImageViewSubresourceSerial serial);
     void updateDepthStencilResolve(ImageViewSubresourceSerial serial);
     size_t hash() const;
@@ -1082,12 +1101,11 @@ class FramebufferDesc
 
     // Note: this is an exclusive index. If there is one index it will be "1".
     uint16_t mMaxIndex;
-    uint8_t mPadding;
 
     // If the render pass contains an initial subpass to unresolve a number of attachments, the
     // subpass description is derived from the following mask, specifying which attachments need
-    // to be unresolved.
-    gl::DrawBufferMask mColorUnresolveAttachmentMask;
+    // to be unresolved.  Includes both color and depth/stencil attachments.
+    FramebufferNonResolveAttachmentMask mUnresolveAttachmentMask;
 
     FramebufferAttachmentArray<ImageViewSubresourceSerial> mSerials;
 };

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -384,6 +384,22 @@ constexpr angle::PackedEnumMap<ImageLayout, ImageMemoryBarrierData> kImageMemory
         },
     },
     {
+        ImageLayout::DepthStencilResolveAttachment,
+        ImageMemoryBarrierData{
+            "DepthStencilResolveAttachment",
+            VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL,
+            // Note: depth/stencil resolve uses color output stage and mask!
+            VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
+            VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT,
+            // Transition to: all reads and writes must happen after barrier.
+            VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
+            // Transition from: all writes must finish before barrier.
+            VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
+            ResourceAccess::Write,
+            PipelineStage::ColorAttachmentOutput,
+        },
+    },
+    {
         ImageLayout::Present,
         ImageMemoryBarrierData{
             "Present",
@@ -5743,8 +5759,8 @@ angle::Result ImageViewHelper::getLevelLayerDrawImageView(ContextVk *contextVk,
     }
 
     // Lazily allocate the image view itself.
-    // Note that these views are specifically made to be used as color attachments, and therefore
-    // don't have swizzle.
+    // Note that these views are specifically made to be used as framebuffer attachments, and
+    // therefore don't have swizzle.
     gl::TextureType viewType = Get2DTextureType(1, image.getSamples());
     return image.initLayerImageView(contextVk, viewType, image.getAspectFlags(), gl::SwizzleState(),
                                     imageView, levelVk, 1, layer, 1);

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1204,6 +1204,7 @@ enum class ImageLayout
     ColorAttachment,
     DepthStencilReadOnly,
     DepthStencilAttachment,
+    DepthStencilResolveAttachment,
     Present,
 
     InvalidEnum,

src/tests/gl_tests/VulkanPerformanceCounterTest.cpp

@@ -1638,6 +1638,126 @@ TEST_P(VulkanPerformanceCounterTest, RenderToTextureDepthStencilTextureShouldNot
     EXPECT_PIXEL_COLOR_EQ(kSize / 2 - 1, kSize / 2 - 1, GLColor::red);
 }
 
+// Tests that multisampled-render-to-texture depth/stencil renderbuffers don't ever load depth data.
+// Stencil data may still be loaded if VK_EXT_shader_stencil_export is not supported.
+TEST_P(VulkanPerformanceCounterTest, RenderToTextureDepthStencilRenderbufferShouldNotLoad)
+{
+    // 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();
+    uint32_t expectedDepthClearCount     = counters.depthClears + 1;
+    uint32_t expectedDepthLoadCount      = counters.depthLoads;
+    uint32_t expectedStencilClearCount   = counters.stencilClears + 1;
+    uint32_t expectedStencilLoadCountMin = counters.stencilLoads;
+    uint32_t expectedStencilLoadCountMax = counters.stencilLoads + 4;
+
+    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 depth
+    glViewport(0, 0, kSize, kSize);
+    glClearDepthf(1);
+    glClearStencil(0x55);
+    glClear(GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
+
+    // If depth is not cleared to 1, rendering would fail.
+    glEnable(GL_DEPTH_TEST);
+    glDepthFunc(GL_LESS);
+
+    // If stencil is not clear to 0x55, rendering would fail.
+    glEnable(GL_STENCIL_TEST);
+    glStencilFunc(GL_EQUAL, 0x55, 0xFF);
+    glStencilOp(GL_KEEP, GL_KEEP, GL_KEEP);
+    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();
+
+    // 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();
+
+    // 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();
+
+    // 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();
+
+    // 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
+    EXPECT_EQ(counters.depthClears, expectedDepthClearCount);
+    EXPECT_EQ(counters.depthLoads, expectedDepthLoadCount);
+    EXPECT_EQ(counters.stencilClears, expectedStencilClearCount);
+    EXPECT_GE(counters.stencilLoads, expectedStencilLoadCountMin);
+    EXPECT_LE(counters.stencilLoads, expectedStencilLoadCountMax);
+
+    // Verify that copies were done correctly.
+    GLFramebuffer verifyFBO;
+    glBindFramebuffer(GL_FRAMEBUFFER, verifyFBO);
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, copyTex, 0);
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(kSize / 2, 0, GLColor::green);
+    EXPECT_PIXEL_COLOR_EQ(0, kSize / 2, GLColor::blue);
+    EXPECT_PIXEL_COLOR_EQ(kSize / 2, kSize / 2, GLColor::yellow);
+}
+
 // Ensures we use read-only depth layout when there is no write
 TEST_P(VulkanPerformanceCounterTest, ReadOnlyDepthBufferLayout)
 {