Vulkan: Improve invalidate for depth/stencil

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -2490,8 +2490,9 @@ void ContextVk::optimizeRenderPassForPresent(VkFramebuffer framebufferHandle)
     if (depthStencilRenderTarget)
     {
         // Change depthstencil attachment storeOp to DONT_CARE
-        mRenderPassCommands->invalidateRenderPassStencilAttachment();
-        mRenderPassCommands->invalidateRenderPassDepthAttachment();
+        const gl::DepthStencilState &dsState = mState.getDepthStencilState();
+        mRenderPassCommands->invalidateRenderPassStencilAttachment(dsState);
+        mRenderPassCommands->invalidateRenderPassDepthAttachment(dsState);
         // Mark content as invalid so that we will not load them in next renderpass
         depthStencilRenderTarget->invalidateEntireContent();
     }
@@ -2908,11 +2909,9 @@ angle::Result ContextVk::syncState(const gl::Context *context,
                 if (mRenderPassCommands->started())
                 {
                     vk::ResourceAccess access = GetStencilAccess(mState.getDepthStencilState());
-                    mRenderPassCommands->onStencilAccess(access);
-                    // Did this depth-state change undo a previous invalidation of the depth-stencil
-                    // attachment?
-                    if (mRenderPassCommands->shouldRestoreDepthStencilAttachment())
+                    if (mRenderPassCommands->onStencilAccess(access))
                     {
+                        // The attachment is no longer invalidated, so set mContentDefined to true
                         mDrawFramebuffer->restoreDepthStencilDefinedContents();
                     }
                 }
@@ -4914,11 +4913,9 @@ angle::Result ContextVk::updateRenderPassDepthAccess()
         }
         else
         {
-            mRenderPassCommands->onDepthAccess(access);
-            // Did this depth-state change undo a previous invalidation of the depth-stencil
-            // attachment?
-            if (mRenderPassCommands->shouldRestoreDepthStencilAttachment())
+            if (mRenderPassCommands->onDepthAccess(access))
             {
+                // The attachment is no longer invalidated, so set mContentDefined to true
                 mDrawFramebuffer->restoreDepthStencilDefinedContents();
             }
         }

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -1483,14 +1483,17 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
 
         if (depthStencilRenderTarget)
         {
+            const gl::DepthStencilState &dsState = contextVk->getState().getDepthStencilState();
             if (invalidateDepthBuffer)
             {
-                contextVk->getStartedRenderPassCommands().invalidateRenderPassDepthAttachment();
+                contextVk->getStartedRenderPassCommands().invalidateRenderPassDepthAttachment(
+                    dsState);
             }
 
             if (invalidateStencilBuffer)
             {
-                contextVk->getStartedRenderPassCommands().invalidateRenderPassStencilAttachment();
+                contextVk->getStartedRenderPassCommands().invalidateRenderPassStencilAttachment(
+                    dsState);
             }
         }
         if (invalidateColorBuffers.any())

src/libANGLE/renderer/vulkan/SecondaryCommandBuffer.h

@@ -693,6 +693,14 @@ class SecondaryCommandBuffer final : angle::NonCopyable
 
     static bool CanKnowIfEmpty() { return true; }
     bool empty() const { return mCommands.size() == 0 || mCommands[0]->id == CommandID::Invalid; }
+    // The following is used to give the size of the command buffer in bytes
+    uint32_t getCommandBufferSize() const
+    {
+        ASSERT(mCommands.size() > 0 || mCurrentBytesRemaining == 0);
+        uint32_t rtn =
+            static_cast<uint32_t>((mCommands.size() * kBlockSize) - mCurrentBytesRemaining);
+        return rtn;
+    }
 
   private:
     void commonDebugUtilsLabel(CommandID cmd, const VkDebugUtilsLabelEXT &label);

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -561,10 +561,10 @@ CommandBufferHelper::CommandBufferHelper()
       mIsRenderPassCommandBuffer(false),
       mDepthStartAccess(ResourceAccess::Unused),
       mStencilStartAccess(ResourceAccess::Unused),
-      mDepthEnabled(false),
-      mDepthInvalidatedState(NeverInvalidated),
-      mStencilEnabled(false),
-      mStencilInvalidatedState(NeverInvalidated),
+      mDepthCmdSizeInvalidated(kInfiniteCmdSize),
+      mDepthCmdSizeDisabled(kInfiniteCmdSize),
+      mStencilCmdSizeInvalidated(kInfiniteCmdSize),
+      mStencilCmdSizeDisabled(kInfiniteCmdSize),
       mDepthStencilAttachmentIndex(kInvalidAttachmentIndex)
 {}
 
@@ -704,70 +704,67 @@ void CommandBufferHelper::imageWrite(ResourceUseList *resourceUseList,
     }
 }
 
-void CommandBufferHelper::onDepthAccess(ResourceAccess access)
+bool CommandBufferHelper::onDepthAccess(ResourceAccess access)
 {
-    // TODO(ianelliott): Rework the handling of invalidated attachments in a follow-up CL, using
-    // the count of commands in the SecondaryCommandBuffer.
-    // See https://issuetracker.google.com/issues/163854287
-    InvalidatedState invalidatedState;
-
-    if (access == vk::ResourceAccess::Write)
-    {
-        // This handles various scenarios that an app/test can do with valid GLES usage.  For
-        // example, consider an app that invalidates, doesn't disable the functionality, and draws
-        // again.  In that case, the drawing that occurs after the invalidate means that there is
-        // once again valid content in the attachment (i.e. that should not be discarded).  Since
-        // we don't track draws, we must be conservative and assume that a draw may have occured
-        // since invalidation unless the functionality has also been disabled and the re-enabled.
-        invalidatedState = (!mDepthEnabled) ? NoLongerInvalidated : Invalidated;
-        // Keep track of whether depth functionality is enabled
-        mDepthEnabled = true;
-    }
-    else
-    {
-        invalidatedState = Invalidated;
-        // Keep track of whether depth functionality is enabled
-        mDepthEnabled = false;
-    }
-
     // Update the access for optimizing this render pass's loadOp
     UpdateAccess(&mDepthStartAccess, access);
     ASSERT((mRenderPassDesc.getDepthStencilAccess() != ResourceAccess::ReadOnly) ||
            mDepthStartAccess != ResourceAccess::Write);
+
     // Update the invalidate state for optimizing this render pass's storeOp
-    UpdateInvalidatedState(&mDepthInvalidatedState, invalidatedState);
+    return onDepthStencilAccess(access, &mDepthCmdSizeInvalidated, &mDepthCmdSizeDisabled);
 }
 
-void CommandBufferHelper::onStencilAccess(ResourceAccess access)
+bool CommandBufferHelper::onStencilAccess(ResourceAccess access)
 {
-    // TODO(ianelliott): Rework the handling of invalidated attachments in a follow-up CL, using
-    // the count of commands in the SecondaryCommandBuffer.
-    // See https://issuetracker.google.com/issues/163854287
-    InvalidatedState invalidatedState;
+    // Update the access for optimizing this render pass's loadOp
+    UpdateAccess(&mStencilStartAccess, access);
 
+    // Update the invalidate state for optimizing this render pass's stencilStoreOp
+    return onDepthStencilAccess(access, &mStencilCmdSizeInvalidated, &mStencilCmdSizeDisabled);
+}
+
+bool CommandBufferHelper::onDepthStencilAccess(ResourceAccess access,
+                                               uint32_t *cmdCountInvalidated,
+                                               uint32_t *cmdCountDisabled)
+{
+    if (*cmdCountInvalidated == kInfiniteCmdSize)
+    {
+        // If never invalidated or no longer invalidated, return early.
+        return false;
+    }
     if (access == vk::ResourceAccess::Write)
     {
-        // This handles various scenarios that an app/test can do with valid GLES usage.  For
-        // example, consider an app that invalidates, doesn't disable the functionality, and draws
-        // again.  In that case, the drawing that occurs after the invalidate means that there is
-        // once again valid content in the attachment (i.e. that should not be discarded).  Since
-        // we don't track draws, we must be conservative and assume that a draw may have occured
-        // since invalidation unless the functionality has also been disabled and the re-enabled.
-        invalidatedState = (!mStencilEnabled) ? NoLongerInvalidated : Invalidated;
-        // Keep track of whether stencil functionality is enabled
-        mStencilEnabled = true;
+        // Drawing to this attachment is being enabled.  Assume that drawing will immediately occur
+        // after this attachment is enabled, and that means that the attachment will no longer be
+        // invalidated.
+        *cmdCountInvalidated = kInfiniteCmdSize;
+        *cmdCountDisabled    = kInfiniteCmdSize;
+        // Return true to indicate that the store op should remain STORE and that mContentDefined
+        // should be set to true;
+        return true;
     }
     else
     {
-        invalidatedState = Invalidated;
-        // Keep track of whether stencil functionality is enabled
-        mStencilEnabled = false;
+        // Drawing to this attachment is being disabled.
+        if (isNoLongerInvalidated(*cmdCountInvalidated, *cmdCountDisabled))
+        {
+            // The attachment was previously drawn while enabled, and so is no longer invalidated.
+            *cmdCountInvalidated = kInfiniteCmdSize;
+            *cmdCountDisabled    = kInfiniteCmdSize;
+            // Return true to indicate that the store op should remain STORE and that
+            // mContentDefined should be set to true;
+            return true;
+        }
+        else
+        {
+            // Get the latest CmdSize at the start of being disabled.  At the end of the render
+            // pass, cmdCountDisabled is <= the actual command buffer size, and so it's compared
+            // with cmdCountInvalidated.  If the same, the attachment is still invalidated.
+            *cmdCountDisabled = mCommandBuffer.getCommandBufferSize();
+            return false;
+        }
     }
-
-    // Update the access for optimizing this render pass's loadOp
-    UpdateAccess(&mStencilStartAccess, access);
-    // Update the invalidate state for optimizing this render pass's stencilStoreOp
-    UpdateInvalidatedState(&mStencilInvalidatedState, invalidatedState);
 }
 
 void CommandBufferHelper::executeBarriers(ContextVk *contextVk, PrimaryCommandBuffer *primary)
@@ -863,19 +860,20 @@ void CommandBufferHelper::endRenderPass(ContextVk *contextVk)
 
     PackedAttachmentOpsDesc &dsOps = mAttachmentOps[mDepthStencilAttachmentIndex];
 
-    // Address invalidated depth/stencil attachments
-    if (mDepthInvalidatedState == Invalidated)
+    // Depth/Stencil buffer optimizations:
+    //
+    // First, if the attachment is invalidated, skip the store op.
+    if (isInvalidated(mDepthCmdSizeInvalidated, mDepthCmdSizeDisabled))
     {
         dsOps.storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
     }
-    if (mStencilInvalidatedState == Invalidated)
+    if (isInvalidated(mStencilCmdSizeInvalidated, mStencilCmdSizeDisabled))
     {
         dsOps.stencilStoreOp = VK_ATTACHMENT_STORE_OP_DONT_CARE;
     }
 
-    // Depth/Stencil buffer optimization: if we are loading or clearing the buffer, but the
-    // buffer has not been used, and the data has also not been stored back into buffer, then
-    // just skip the load/clear op.
+    // 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.
     if (mDepthStartAccess == ResourceAccess::Unused &&
         dsOps.storeOp == VK_ATTACHMENT_STORE_OP_DONT_CARE)
     {
@@ -1099,10 +1097,10 @@ void CommandBufferHelper::reset()
         mRebindTransformFeedbackBuffers    = false;
         mDepthStartAccess                  = ResourceAccess::Unused;
         mStencilStartAccess                = ResourceAccess::Unused;
-        mDepthInvalidatedState             = NeverInvalidated;
-        mDepthEnabled                      = false;
-        mStencilInvalidatedState           = NeverInvalidated;
-        mStencilEnabled                    = false;
+        mDepthCmdSizeInvalidated           = kInfiniteCmdSize;
+        mDepthCmdSizeDisabled              = kInfiniteCmdSize;
+        mStencilCmdSizeInvalidated         = kInfiniteCmdSize;
+        mStencilCmdSizeDisabled            = kInfiniteCmdSize;
         mDepthStencilAttachmentIndex       = kInvalidAttachmentIndex;
         mRenderPassUsedImages.clear();
     }

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -875,26 +875,10 @@ enum class AliasingMode
     Disallowed,
 };
 
-enum InvalidatedState
-{
-    // The attachment has been invalidated and is currently still invalid.
-    Invalidated,
-    // The attachment was previously invalidated, but has since been used while enabled for
-    // drawing, meaning that it has valid contents (and therefore this render pass should STORE it,
-    // and a future render pass should LOAD it).
-    NoLongerInvalidated,
-    // The attachment has never been invalidated.  Since this is the highest value, it should never
-    // be given to UpdateInvalidatedState().  Instead, it should only be set starting a render pass.
-    NeverInvalidated,
-};
+// The following are used to help track the state of an invalidated attachment.
 
-inline void UpdateInvalidatedState(InvalidatedState *oldState, InvalidatedState newState)
-{
-    if (newState > *oldState)
-    {
-        *oldState = newState;
-    }
-}
+// This value indicates an "infinite" CmdSize that is not valid for comparing
+constexpr uint32_t kInfiniteCmdSize = 0xffffffff;
 
 // CommandBufferHelper (CBH) class wraps ANGLE's custom command buffer
 //  class, SecondaryCommandBuffer. This provides a way to temporarily
@@ -983,29 +967,42 @@ class CommandBufferHelper : angle::NonCopyable
         SetBitField(mAttachmentOps[attachmentIndex].storeOp, VK_ATTACHMENT_STORE_OP_DONT_CARE);
     }
 
-    void invalidateRenderPassDepthAttachment()
+    void invalidateRenderPassDepthAttachment(const gl::DepthStencilState &dsState)
     {
         ASSERT(mIsRenderPassCommandBuffer);
-        mDepthInvalidatedState = Invalidated;
+        // Keep track of the size of commands in the command buffer.  If the size grows in the
+        // future, that implies that drawing occured since invalidated.
+        mDepthCmdSizeInvalidated = mCommandBuffer.getCommandBufferSize();
+        // Also track the size if the attachment is currently disabled.
+        mDepthCmdSizeDisabled =
+            (dsState.depthTest && dsState.depthMask) ? kInfiniteCmdSize : mDepthCmdSizeInvalidated;
     }
 
-    void invalidateRenderPassStencilAttachment()
+    void invalidateRenderPassStencilAttachment(const gl::DepthStencilState &dsState)
     {
         ASSERT(mIsRenderPassCommandBuffer);
-        mStencilInvalidatedState = Invalidated;
+        // Keep track of the size of commands in the command buffer.  If the size grows in the
+        // future, that implies that drawing occured since invalidated.
+        mStencilCmdSizeInvalidated = mCommandBuffer.getCommandBufferSize();
+        // Also track the size if the attachment is currently disabled.
+        mStencilCmdSizeDisabled =
+            dsState.stencilTest ? kInfiniteCmdSize : mStencilCmdSizeInvalidated;
     }
 
-    bool shouldRestoreDepthStencilAttachment()
+    bool isNoLongerInvalidated(uint32_t cmdCountInvalidated, uint32_t cmdCountDisabled)
     {
         ASSERT(mIsRenderPassCommandBuffer);
-        // Return true when both depth and stencil attachments were previously-invalidated, and at
-        // least one of those attachments are no longer invalidated.  When invalidated,
-        // RenderTargetVk::mContentDefined is set to false, which will result in the loadOp and
-        // stencilLoadOp of a future render pass being set to DONT_CARE.  ContextVk::syncState()
-        // will call this method to determine if RenderTargetVk::mContentDefined should be set back
-        // to true (i.e. use LOAD).
-        return mDepthInvalidatedState == NoLongerInvalidated ||
-               mStencilInvalidatedState == NoLongerInvalidated;
+        return (cmdCountInvalidated != kInfiniteCmdSize &&
+                std::min(cmdCountDisabled, mCommandBuffer.getCommandBufferSize()) !=
+                    cmdCountInvalidated);
+    }
+
+    bool isInvalidated(uint32_t cmdCountInvalidated, uint32_t cmdCountDisabled)
+    {
+        ASSERT(mIsRenderPassCommandBuffer);
+        return cmdCountInvalidated != kInfiniteCmdSize &&
+               std::min(cmdCountDisabled, mCommandBuffer.getCommandBufferSize()) ==
+                   cmdCountInvalidated;
     }
 
     void updateRenderPassAttachmentFinalLayout(size_t attachmentIndex, ImageLayout finalLayout)
@@ -1049,8 +1046,8 @@ class CommandBufferHelper : angle::NonCopyable
     // Dumping the command stream is disabled by default.
     static constexpr bool kEnableCommandStreamDiagnostics = false;
 
-    void onDepthAccess(ResourceAccess access);
-    void onStencilAccess(ResourceAccess access);
+    bool onDepthAccess(ResourceAccess access);
+    bool onStencilAccess(ResourceAccess access);
 
     void updateRenderPassForResolve(vk::Framebuffer *newFramebuffer,
                                     const vk::RenderPassDesc &renderPassDesc);
@@ -1064,6 +1061,11 @@ class CommandBufferHelper : angle::NonCopyable
 
   private:
     void addCommandDiagnostics(ContextVk *contextVk);
+
+    bool onDepthStencilAccess(ResourceAccess access,
+                              uint32_t *cmdCountInvalidated,
+                              uint32_t *cmdCountDisabled);
+
     // Allocator used by this class. Using a pool allocator per CBH to avoid threading issues
     //  that occur w/ shared allocator between multiple CBHs.
     angle::PoolAllocator mAllocator;
@@ -1095,10 +1097,10 @@ class CommandBufferHelper : angle::NonCopyable
     ResourceAccess mStencilStartAccess;
 
     // State tracking for whether to optimize the storeOp to DONT_CARE
-    bool mDepthEnabled;
-    InvalidatedState mDepthInvalidatedState;
-    bool mStencilEnabled;
-    InvalidatedState mStencilInvalidatedState;
+    uint32_t mDepthCmdSizeInvalidated;
+    uint32_t mDepthCmdSizeDisabled;
+    uint32_t mStencilCmdSizeInvalidated;
+    uint32_t mStencilCmdSizeDisabled;
 
     // Keep track of the depth/stencil attachment index
     uint32_t mDepthStencilAttachmentIndex;