Vulkan: Fix robust resource init vs invalidate

If robust resource init is enabled, invalidate should not leave the
contents of the image undefined.  With this change, after the
framebuffer is invalidated, the render pass store op is still
STORE_OP_DONT_CARE so that it would be efficient, but a robust init
clear is restaged in the image.  In the next render pass, or on
read-back, the content is cleared again automatically.

Bug: chromium:492153658
Change-Id: I84089b955c44be1569ed1d71ab406d3df681590f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7697282
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>

Author: ShabbyX

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -554,7 +554,10 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
     // glCopyTex[Sub]Image, shader storage image, etc).
     restageDeferredClears(contextVk);
 
-    if (contextVk->hasActiveRenderPass() &&
+    // If robust resource initialization is enabled, do not invalidate sub-regions of the
+    // framebuffer.  This is because otherwise the contents of that region becomes undefined and
+    // ANGLE doesn't clear it back to black.
+    if (!contextVk->isRobustResourceInitEnabled() && contextVk->hasActiveRenderPass() &&
         rotatedInvalidateArea.encloses(contextVk->getStartedRenderPassCommands().getRenderArea()))
     {
         // Because the render pass's render area is within the invalidated area, it is fine for
@@ -566,7 +569,10 @@ angle::Result FramebufferVk::invalidateSub(const gl::Context *context,
     {
         ANGLE_VK_PERF_WARNING(
             contextVk, GL_DEBUG_SEVERITY_LOW,
-            "InvalidateSubFramebuffer ignored due to area not covering the render area");
+            contextVk->isRobustResourceInitEnabled()
+                ? "InvalidateSubFramebuffer ignored due to area not covering the entire "
+                  "framebuffer while robust resource initialization is enabled"
+                : "InvalidateSubFramebuffer ignored due to area not covering the render area");
     }
 
     return angle::Result::Continue;
@@ -2282,7 +2288,6 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
             mDeferredClears.reset(vk::kUnpackedStencilIndex);
         }
     }
-
     // Limit invalidateColorBuffers to enabled draw buffers
     invalidateColorBuffers &= mState.getEnabledDrawBuffers();
     for (size_t colorIndexGL : invalidateColorBuffers)
@@ -2298,6 +2303,12 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
 
     // If not a partial invalidate, mark the contents of the invalidated attachments as undefined,
     // so their loadOp can be set to DONT_CARE in the following render pass.
+
+    // If robust resource initialization is enabled, restage clears on the invalidated resources.
+    // This way, STORE_OP_DONT_CARE can be used, but the application can still not observe garbage
+    // data in the image.
+    const bool isRobustResourceInitEnabled = contextVk->isRobustResourceInitEnabled();
+    ASSERT(!(isRobustResourceInitEnabled && isSubInvalidate));
     if (!isSubInvalidate)
     {
         for (size_t colorIndexGL : invalidateColorBuffers)
@@ -2311,6 +2322,13 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
             {
                 invalidateColorBuffers.reset(colorIndexGL);
             }
+            else if (isRobustResourceInitEnabled)
+            {
+                gl::ImageIndex imageIndex = colorRenderTarget->getImageIndexForClear(
+                    mCurrentFramebufferDesc.getLayerCount());
+                colorRenderTarget->getImageForWrite().stageRobustResourceClear(
+                    imageIndex, VK_IMAGE_ASPECT_COLOR_BIT);
+            }
         }
 
         // If we have a depth / stencil render target, invalidate its aspects.
@@ -2336,6 +2354,26 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
                     invalidateStencilBuffer = false;
                 }
             }
+
+            if (isRobustResourceInitEnabled)
+            {
+                VkImageAspectFlags dsAspectFlags = 0;
+                if (invalidateDepthBuffer)
+                {
+                    dsAspectFlags |= VK_IMAGE_ASPECT_DEPTH_BIT;
+                }
+                if (invalidateStencilBuffer)
+                {
+                    dsAspectFlags |= VK_IMAGE_ASPECT_STENCIL_BIT;
+                }
+                if (dsAspectFlags)
+                {
+                    gl::ImageIndex imageIndex = depthStencilRenderTarget->getImageIndexForClear(
+                        mCurrentFramebufferDesc.getLayerCount());
+                    depthStencilRenderTarget->getImageForWrite().stageRobustResourceClear(
+                        imageIndex, dsAspectFlags);
+                }
+            }
         }
     }
 
@@ -2347,7 +2385,10 @@ angle::Result FramebufferVk::invalidateImpl(ContextVk *contextVk,
     // to invalidate the D/S of FBO 2 since it would be the currently active renderpass.
     if (contextVk->hasStartedRenderPassWithQueueSerial(mLastRenderPassQueueSerial))
     {
-        bool closeRenderPass = false;
+        // When robust-resource init is enabled, a clear is stashed in the image after invalidating
+        // it.  The render pass is closed for the same reason as with images with emulated alpha
+        // channel as described below.
+        bool closeRenderPass = isRobustResourceInitEnabled;
 
         // Mark the invalidated attachments in the render pass for loadOp and storeOp determination
         // at its end.

src/libANGLE/renderer/vulkan/RenderbufferVk.cpp

@@ -287,7 +287,7 @@ angle::Result RenderbufferVk::initializeContents(const gl::Context *context,
                                                  const gl::ImageIndex &imageIndex)
 {
     // Note: stageSubresourceRobustClear only uses the intended format to count channels.
-    mImage->stageRobustResourceClear(imageIndex);
+    mImage->stageRobustResourceClear(imageIndex, mImage->getAspectFlags());
     return mImage->flushAllStagedUpdates(vk::GetImpl(context));
 }
 

src/libANGLE/renderer/vulkan/SurfaceVk.cpp

@@ -850,14 +850,15 @@ angle::Result OffscreenSurfaceVk::initializeContents(const gl::Context *context,
     {
         case GL_BACK:
             ASSERT(mColorAttachment.image.valid());
-            mColorAttachment.image.stageRobustResourceClear(imageIndex);
+            mColorAttachment.image.stageRobustResourceClear(imageIndex, VK_IMAGE_ASPECT_COLOR_BIT);
             ANGLE_TRY(mColorAttachment.image.flushAllStagedUpdates(contextVk));
             break;
 
         case GL_DEPTH:
         case GL_STENCIL:
             ASSERT(mDepthStencilAttachment.image.valid());
-            mDepthStencilAttachment.image.stageRobustResourceClear(imageIndex);
+            mDepthStencilAttachment.image.stageRobustResourceClear(
+                imageIndex, mDepthStencilAttachment.image.getAspectFlags());
             ANGLE_TRY(mDepthStencilAttachment.image.flushAllStagedUpdates(contextVk));
             break;
 
@@ -3606,14 +3607,15 @@ angle::Result WindowSurfaceVk::initializeContents(const gl::Context *context,
             vk::ImageHelper *image =
                 hasAncillaryColor() ? &mAncillaryColorImage
                                     : mSwapchainImages[mCurrentSwapchainImageIndex].image.get();
-            image->stageRobustResourceClear(imageIndex);
+            image->stageRobustResourceClear(imageIndex, VK_IMAGE_ASPECT_COLOR_BIT);
             ANGLE_TRY(image->flushAllStagedUpdates(contextVk));
             break;
         }
         case GL_DEPTH:
         case GL_STENCIL:
             ASSERT(mDepthStencilImage.valid());
-            mDepthStencilImage.stageRobustResourceClear(gl::ImageIndex::Make2D(0));
+            mDepthStencilImage.stageRobustResourceClear(gl::ImageIndex::Make2D(0),
+                                                        mDepthStencilImage.getAspectFlags());
             ANGLE_TRY(mDepthStencilImage.flushAllStagedUpdates(contextVk));
             break;
         default:

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -9548,10 +9548,9 @@ void ImageHelper::stageClear(const gl::ImageIndex &index,
     appendSubresourceUpdate(updateLevelGL, SubresourceUpdate(aspectFlags, clearValue, index));
 }
 
-void ImageHelper::stageRobustResourceClear(const gl::ImageIndex &index)
+void ImageHelper::stageRobustResourceClear(const gl::ImageIndex &index,
+                                           const VkImageAspectFlags aspectFlags)
 {
-    const VkImageAspectFlags aspectFlags = getAspectFlags();
-
     ASSERT(mActualFormatID != angle::FormatID::NONE);
     VkClearValue clearValue = GetRobustResourceClearValue(getIntendedFormat(), getActualFormat());
 

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -2595,7 +2595,8 @@ class ImageHelper final : public Resource, public angle::Subject
                                                      const gl::Extents &glExtents,
                                                      const angle::Format &intendedFormat,
                                                      const angle::Format &imageFormat);
-    void stageRobustResourceClear(const gl::ImageIndex &index);
+    void stageRobustResourceClear(const gl::ImageIndex &index,
+                                  const VkImageAspectFlags aspectFlags);
 
     angle::Result stageResourceClearWithFormat(ContextVk *contextVk,
                                                const gl::ImageIndex &index,

src/libANGLE/renderer/vulkan/vk_renderer.cpp

@@ -765,6 +765,18 @@ constexpr vk::SkippedSyncvalMessage kSkippedSyncvalMessagesWithMSRTTEmulation[]
          "old_layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL",
          "new_layout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL",
      }},
+    // Unknown whether ANGLE or syncval bug.  See http://anglebug.com/495832130
+    {"SYNC-HAZARD-WRITE-AFTER-WRITE",
+     false,
+     {
+         "message_type = ImageSubresourceRangeError",
+         "access = VK_PIPELINE_STAGE_2_CLEAR_BIT(VK_ACCESS_2_TRANSFER_WRITE_BIT)",
+         "prior_access = "
+         "VK_PIPELINE_STAGE_2_LATE_FRAGMENT_TESTS_BIT(VK_ACCESS_2_DEPTH_STENCIL_ATTACHMENT_WRITE_"
+         "BIT)",
+         "command = vkCmdClearDepthStencilImage",
+         "prior_command = vkCmdNextSubpass",
+     }},
 };
 
 enum class DebugMessageReport

src/tests/angle_end2end_tests_expectations.txt

@@ -278,6 +278,8 @@
 42266866 NVIDIA OPENGL : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP
 42266866 NVIDIA GLES : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP
 42266866 NVIDIA GLES : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP
+496616811 NVIDIA OPENGL : RobustResourceInitTestES3.DrawThenInvalidateThenVerifyDepthStencil/* = SKIP
+496616811 NVIDIA GLES : RobustResourceInitTestES3.DrawThenInvalidateThenVerifyDepthStencil/* = SKIP
 // Nvidia sees same matrix type with different layouts as different types
 42262474 NVIDIA OPENGL : GLSLTest_ES31.TernaryOnStructsInDifferentBlockStorages/* = SKIP
 42262474 NVIDIA OPENGL : GLSLTest_ES31.TernaryOnMatricesInDifferentBlockStorages/* = SKIP
@@ -475,6 +477,7 @@
 42266214 MAC APPLE METAL : Texture2DTest.ManySupersedingTextureUpdates/* = SKIP
 494392011 MAC METAL : MipmapTestES3.MismatchingLevelFormats/* = SKIP
 494341324 MAC OPENGL : MipmapTestES3.MismatchingLevelFormats/* = SKIP
+496604559 MAC METAL : RobustResourceInitTestES3.DrawThenInvalidateThenVerifyDepthStencil/* = SKIP
 
 // The workaround is not intended to be enabled in this configuration so
 // skip it as the failure is likely a driver bug.
@@ -549,9 +552,11 @@
 448658631 WIN D3D11 : GLSLTestLoops.ForContinueInSwitchComplex/* = SKIP
 448658631 WIN D3D11 : GLSLTest_ES3.SwitchConstantFold/* = SKIP
 448658631 WIN D3D11 : GLSLTest_ES3.SwitchConstantFoldWithVariables/* = SKIP
-
 494938700 WIN D3D11 : CopyTexImageTest.CopyTexImageFromTexture3D/* = SKIP
 494938700 WIN D3D11 : CopyTexImageTest.CopyTexSubImageFromTexture3D/* = SKIP
+496259841 WIN D3D11 : RobustResourceInitTestES3.InvalidateThenReadBack/* = SKIP
+496259841 WIN D3D11 : RobustResourceInitTestES3.DrawThenInvalidateThenReadBack/* = SKIP
+496259841 WIN D3D11 : RobustResourceInitTestES3.DrawThenInvalidateThenVerifyDepthStencil/* = SKIP
 
 // Android
 42264624 ANDROID GLES : GLSLTest_ES3.InitGlobalComplexConstant/* = SKIP