Vulkan: Restore at the end of RP if write-after-invalidate

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -5129,16 +5129,8 @@ angle::Result ContextVk::updateRenderPassDepthStencilAccess()
         }
         else
         {
-            if (mRenderPassCommands->onDepthAccess(depthAccess))
-            {
-                // The attachment is no longer invalidated, so set mContentDefined to true
-                mDrawFramebuffer->restoreDepthDefinedContents();
-            }
-            if (mRenderPassCommands->onStencilAccess(stencilAccess))
-            {
-                // The attachment is no longer invalidated, so set mContentDefined to true
-                mDrawFramebuffer->restoreStencilDefinedContents();
-            }
+            mRenderPassCommands->onDepthAccess(depthAccess);
+            mRenderPassCommands->onStencilAccess(stencilAccess);
 
             mDrawFramebuffer->updateRenderPassReadOnlyDepthMode(this, mRenderPassCommands);
         }

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -2216,11 +2216,7 @@ angle::Result FramebufferVk::clearWithCommand(
         dsAspectFlags |= VK_IMAGE_ASPECT_DEPTH_BIT;
         dsClearValue.depthStencil = clearDepthStencilValue;
         // Explicitly mark a depth write because we are clearing the depth buffer.
-        if (renderpassCommands->onDepthAccess(vk::ResourceAccess::Write))
-        {
-            // The attachment is no longer invalidated, so set mContentDefined to true
-            restoreDepthDefinedContents();
-        }
+        renderpassCommands->onDepthAccess(vk::ResourceAccess::Write);
     }
 
     if (clearStencil)
@@ -2228,11 +2224,7 @@ angle::Result FramebufferVk::clearWithCommand(
         dsAspectFlags |= VK_IMAGE_ASPECT_STENCIL_BIT;
         dsClearValue.depthStencil = clearDepthStencilValue;
         // Explicitly mark a stencil write because we are clearing the stencil buffer.
-        if (renderpassCommands->onStencilAccess(vk::ResourceAccess::Write))
-        {
-            // The attachment is no longer invalidated, so set mContentDefined to true
-            restoreStencilDefinedContents();
-        }
+        renderpassCommands->onStencilAccess(vk::ResourceAccess::Write);
     }
 
     if (dsAspectFlags != 0)
@@ -2535,28 +2527,6 @@ angle::Result FramebufferVk::startNewRenderPass(ContextVk *contextVk,
     return angle::Result::Continue;
 }
 
-void FramebufferVk::restoreDepthDefinedContents()
-{
-    // If the depthStencilRenderTarget does not have "defined content" (i.e. meaning that a future
-    // render pass should use a loadOp of DONT_CARE), we should restore it (i.e. so that a future
-    // render pass uses a loadOp of LOAD).
-    RenderTargetVk *depthStencilRenderTarget = mRenderTargetCache.getDepthStencil();
-    if (depthStencilRenderTarget)
-    {
-        depthStencilRenderTarget->restoreEntireContent();
-    }
-}
-
-void FramebufferVk::restoreStencilDefinedContents()
-{
-    // Similar to restoreDepthDefinedContents, but for the stencil aspect.
-    RenderTargetVk *depthStencilRenderTarget = mRenderTargetCache.getDepthStencil();
-    if (depthStencilRenderTarget)
-    {
-        depthStencilRenderTarget->restoreEntireStencilContent();
-    }
-}
-
 void FramebufferVk::updateActiveColorMasks(size_t colorIndexGL, bool r, bool g, bool b, bool a)
 {
     gl::BlendStateExt::ColorMaskStorage::SetValueIndexed(

src/libANGLE/renderer/vulkan/FramebufferVk.h

@@ -118,8 +118,6 @@ class FramebufferVk : public FramebufferImpl
     angle::Result startNewRenderPass(ContextVk *contextVk,
                                      const gl::Rectangle &renderArea,
                                      vk::CommandBuffer **commandBufferOut);
-    void restoreDepthDefinedContents();
-    void restoreStencilDefinedContents();
 
     RenderTargetVk *getFirstRenderTarget() const;
     GLint getSamples() const;

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -648,7 +648,9 @@ CommandBufferHelper::CommandBufferHelper()
       mStencilCmdSizeDisabled(kInfiniteCmdSize),
       mDepthStencilAttachmentIndex(kAttachmentIndexInvalid),
       mDepthStencilImage(nullptr),
-      mDepthStencilResolveImage(nullptr)
+      mDepthStencilResolveImage(nullptr),
+      mDepthStencilLevelIndex(0),
+      mDepthStencilLayerIndex(0)
 {}
 
 CommandBufferHelper::~CommandBufferHelper()
@@ -806,10 +808,15 @@ void CommandBufferHelper::depthStencilImagesDraw(ResourceUseList *resourceUseLis
     image->retain(resourceUseList);
     image->onWrite(level, 1, layer, 1, kDepthStencilAspects);
     mRenderPassUsedImages.insert(image->getImageSerial().getValue());
-    mDepthStencilImage = image;
+    mDepthStencilImage      = image;
+    mDepthStencilLevelIndex = level;
+    mDepthStencilLayerIndex = layer;
 
     if (resolveImage)
     {
+        // Note that the resolve depth/stencil image has the same level/layer index as the
+        // depth/stencil image as currently it can only ever come from
+        // multisampled-render-to-texture renderbuffers.
         resolveImage->retain(resourceUseList);
         resolveImage->onWrite(level, 1, layer, 1, kDepthStencilAspects);
         mRenderPassUsedImages.insert(resolveImage->getImageSerial().getValue());
@@ -817,22 +824,30 @@ void CommandBufferHelper::depthStencilImagesDraw(ResourceUseList *resourceUseLis
     }
 }
 
-bool CommandBufferHelper::onDepthAccess(ResourceAccess access)
+void CommandBufferHelper::onDepthAccess(ResourceAccess access)
 {
     // Update the access for optimizing this render pass's loadOp
     UpdateAccess(&mDepthAccess, access);
 
     // Update the invalidate state for optimizing this render pass's storeOp
-    return onDepthStencilAccess(access, &mDepthCmdSizeInvalidated, &mDepthCmdSizeDisabled);
+    if (onDepthStencilAccess(access, &mDepthCmdSizeInvalidated, &mDepthCmdSizeDisabled))
+    {
+        // The attachment is no longer invalid, so restore its content.
+        restoreDepthContent();
+    }
 }
 
-bool CommandBufferHelper::onStencilAccess(ResourceAccess access)
+void CommandBufferHelper::onStencilAccess(ResourceAccess access)
 {
     // Update the access for optimizing this render pass's loadOp
     UpdateAccess(&mStencilAccess, access);
 
     // Update the invalidate state for optimizing this render pass's stencilStoreOp
-    return onDepthStencilAccess(access, &mStencilCmdSizeInvalidated, &mStencilCmdSizeDisabled);
+    if (onDepthStencilAccess(access, &mStencilCmdSizeInvalidated, &mStencilCmdSizeDisabled))
+    {
+        // The attachment is no longer invalid, so restore its content.
+        restoreStencilContent();
+    }
 }
 
 bool CommandBufferHelper::onDepthStencilAccess(ResourceAccess access,
@@ -878,6 +893,28 @@ bool CommandBufferHelper::onDepthStencilAccess(ResourceAccess access,
     }
 }
 
+void CommandBufferHelper::restoreDepthContent()
+{
+    // Note that the image may have been deleted since the render pass has started.
+    if (mDepthStencilImage)
+    {
+        ASSERT(mDepthStencilImage->valid());
+        mDepthStencilImage->restoreSubresourceContent(mDepthStencilLevelIndex,
+                                                      mDepthStencilLayerIndex);
+    }
+}
+
+void CommandBufferHelper::restoreStencilContent()
+{
+    // Note that the image may have been deleted since the render pass has started.
+    if (mDepthStencilImage)
+    {
+        ASSERT(mDepthStencilImage->valid());
+        mDepthStencilImage->restoreSubresourceStencilContent(mDepthStencilLevelIndex,
+                                                             mDepthStencilLayerIndex);
+    }
+}
+
 void CommandBufferHelper::executeBarriers(ContextVk *contextVk, PrimaryCommandBuffer *primary)
 {
     // make a local copy for faster access
@@ -1035,11 +1072,23 @@ void CommandBufferHelper::endRenderPass(ContextVk *contextVk)
         dsOps.storeOp       = VK_ATTACHMENT_STORE_OP_DONT_CARE;
         dsOps.isInvalidated = true;
     }
+    else if (hasWriteAfterInvalidate(mDepthCmdSizeInvalidated, mDepthCmdSizeDisabled))
+    {
+        // The depth attachment was invalidated, but is now valid.  Let the image know the contents
+        // are now defined so a future render pass would use loadOp=LOAD.
+        restoreDepthContent();
+    }
     if (isInvalidated(mStencilCmdSizeInvalidated, mStencilCmdSizeDisabled))
     {
         dsOps.stencilStoreOp       = VK_ATTACHMENT_STORE_OP_DONT_CARE;
         dsOps.isStencilInvalidated = true;
     }
+    else if (hasWriteAfterInvalidate(mStencilCmdSizeInvalidated, mStencilCmdSizeDisabled))
+    {
+        // The stencil attachment was invalidated, but is now valid.  Let the image know the
+        // contents are now defined so a future render pass would use loadOp=LOAD.
+        restoreStencilContent();
+    }
 
     // Second, if we are loading or clearing the attachment, but the attachment has not been used,
     // and the data has also not been stored back into attachment, then just skip the load/clear op.

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1108,8 +1108,8 @@ class CommandBufferHelper : angle::NonCopyable
     // Dumping the command stream is disabled by default.
     static constexpr bool kEnableCommandStreamDiagnostics = false;
 
-    bool onDepthAccess(ResourceAccess access);
-    bool onStencilAccess(ResourceAccess access);
+    void onDepthAccess(ResourceAccess access);
+    void onStencilAccess(ResourceAccess access);
 
     void updateRenderPassForResolve(vk::Framebuffer *newFramebuffer,
                                     const vk::RenderPassDesc &renderPassDesc);
@@ -1130,6 +1130,8 @@ class CommandBufferHelper : angle::NonCopyable
     bool onDepthStencilAccess(ResourceAccess access,
                               uint32_t *cmdCountInvalidated,
                               uint32_t *cmdCountDisabled);
+    void restoreDepthContent();
+    void restoreStencilContent();
 
     void finalizeDepthStencilImageLayout();
     void finalizeDepthStencilResolveImageLayout();
@@ -1186,6 +1188,8 @@ class CommandBufferHelper : angle::NonCopyable
 
     ImageHelper *mDepthStencilImage;
     ImageHelper *mDepthStencilResolveImage;
+    gl::LevelIndex mDepthStencilLevelIndex;
+    uint32_t mDepthStencilLayerIndex;
 };
 
 // Imagine an image going through a few layout transitions:

src/tests/gl_tests/VulkanPerformanceCounterTest.cpp

@@ -756,10 +756,6 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDraw)
     // Draw (since enabled, should result: in storeOp = STORE; mContentDefined = true)
     drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
     ASSERT_GL_NO_ERROR();
-    // TODO: Fix ANGLE to correct set mContentDefined for this scenario.  At this point,
-    // mContentDefined will remain false since we don't do record anything at draw-time, and since
-    // we don't set mContentDefined at endRP().
-    // https://issuetracker.google.com/issues/167275320
 
     // Ensure that the render pass wasn't broken
     EXPECT_EQ(expected.renderPasses, counters.renderPasses);
@@ -773,8 +769,7 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateDraw)
     drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
     ASSERT_GL_NO_ERROR();
     swapBuffers();
-    // TODO: After fixing ANGLE per https://issuetracker.google.com/issues/167275320, uncomment:
-    // compareLoadCountersForInvalidateTest(counters, expected);
+    compareLoadCountersForInvalidateTest(counters, expected);
 }
 
 // Tests that another case does not break render pass, and that counts are correct:
@@ -1198,8 +1193,8 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateAndClear)
     swapBuffers();
     compareDepthStencilCountersForInvalidateTest(counters, expected);
 
-    // Expect rpCount+1, depth(Clears+0, Loads+1, Stores+1), stencil(Clears+0, Load+0, Stores+1)
-    setExpectedCountersForInvalidateTest(counters, 0, 0, 1, 1, 0, 0, 1, &expected);
+    // Expect rpCount+1, depth(Clears+0, Loads+1, Stores+1), stencil(Clears+0, Load+1, Stores+1)
+    setExpectedCountersForInvalidateTest(counters, 0, 0, 1, 1, 0, 1, 1, &expected);
 
     // Bind FBO again and try to use the depth buffer without clear. This should result in
     // loadOp=LOAD and StoreOP=STORE
@@ -1271,8 +1266,7 @@ TEST_P(VulkanPerformanceCounterTest, InvalidateAndMaskedClear)
     ANGLE_GL_PROGRAM(blueProgram, essl1_shaders::vs::Simple(), essl1_shaders::fs::Blue());
     drawQuad(blueProgram, essl1_shaders::PositionAttrib(), 0.95f);
     EXPECT_PIXEL_COLOR_EQ(kInvalidateTestSize / 2, kInvalidateTestSize / 2, GLColor::blue);
-    // TODO: After fixing ANGLE per https://issuetracker.google.com/issues/167275320, uncomment:
-    // compareDepthStencilCountersForInvalidateTest(counters, expected);
+    compareDepthStencilCountersForInvalidateTest(counters, expected);
 }
 
 // Tests whether depth-stencil ContentDefined will be correct when: