Apply UNDEFINED discard pattern for initial RP layout. Closes #2058

renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp

@@ -1357,6 +1357,21 @@ bool WrappedVulkan::Serialise_vkCmdBeginRenderPass(SerialiserType &ser, VkComman
         const VulkanCreationInfo::RenderPass &rpinfo =
             m_CreationInfo.m_RenderPass[GetCmdRenderState().renderPass];
 
+        rdcarray<VkImageMemoryBarrier> imgBarriers = GetImplicitRenderPassBarriers();
+
+        // if we're just replaying the vkCmdBeginRenderPass on its own, we use the first loadRP
+        // instead of the real thing. This then doesn't require us to finish off any subpasses etc.
+        // we need to manually do the subpass 0 barriers, since loadRP expects the image to already
+        // be in subpass 0's layout
+        // we also need to manually do any clears, since the loadRP will load all attachments
+        if(m_FirstEventID == m_LastEventID)
+        {
+          unwrappedInfo.renderPass = Unwrap(rpinfo.loadRPs[0]);
+          unwrappedInfo.framebuffer = Unwrap(fbinfo.loadFBs[0]);
+
+          DoPipelineBarrier(commandBuffer, imgBarriers.size(), imgBarriers.data());
+        }
+
         if(m_ReplayOptions.optimisation != ReplayOptimisationLevel::Fastest)
         {
           const rdcarray<ResourceId> &attachments = GetCmdRenderState().GetFramebufferAttachments();
@@ -1369,10 +1384,14 @@ bool WrappedVulkan::Serialise_vkCmdBeginRenderPass(SerialiserType &ser, VkComman
             const VulkanCreationInfo::Image &imInfo =
                 GetDebugManager()->GetImageInfo(GetResID(image));
 
-            if(rpinfo.attachments[i].loadOp == VK_ATTACHMENT_LOAD_OP_DONT_CARE &&
+            VkImageLayout initialLayout = rpinfo.attachments[i].initialLayout;
+
+            VkRect2D rect = RenderPassBegin.renderArea;
+
+            if((rpinfo.attachments[i].loadOp == VK_ATTACHMENT_LOAD_OP_DONT_CARE ||
+                initialLayout == VK_IMAGE_LAYOUT_UNDEFINED) &&
                rpinfo.attachments[i].used)
             {
-              VkImageLayout initialLayout = rpinfo.attachments[i].initialLayout;
               // if originally it was UNDEFINED (which is fine with DONT_CARE) and we promoted to
               // load so we could preserve the discard pattern, transition to general.
               if(initialLayout == VK_IMAGE_LAYOUT_UNDEFINED)
@@ -1393,30 +1412,33 @@ bool WrappedVulkan::Serialise_vkCmdBeginRenderPass(SerialiserType &ser, VkComman
                 DoPipelineBarrier(commandBuffer, 1, &dstimBarrier);
 
                 initialLayout = VK_IMAGE_LAYOUT_GENERAL;
+
+                // undefined transitions apply to the whole subresource not just the render area.
+                // But we don't want to do an undefined discard pattern that will be completely
+                // overwritten, and it's common for the render area to be the whole subresource. So
+                // check that here now and only do the undefined if we're not about to DONT_CARE
+                // over it or if the render area is a subset
+                if(rpinfo.attachments[i].loadOp != VK_ATTACHMENT_LOAD_OP_DONT_CARE ||
+                   rect.offset.x > 0 || rect.offset.y > 0 ||
+                   rect.extent.width < RDCMAX(1U, imInfo.extent.width >> viewInfo.range.baseMipLevel) ||
+                   rect.extent.height <
+                       RDCMAX(1U, imInfo.extent.height >> viewInfo.range.baseMipLevel))
+                {
+                  GetDebugManager()->FillWithDiscardPattern(
+                      commandBuffer, DiscardType::UndefinedTransition, image, initialLayout,
+                      viewInfo.range, {{0, 0}, {imInfo.extent.width, imInfo.extent.height}});
+                }
               }
 
-              GetDebugManager()->FillWithDiscardPattern(commandBuffer, DiscardType::RenderPassLoad,
-                                                        image, initialLayout, viewInfo.range,
-                                                        RenderPassBegin.renderArea);
+              if(rpinfo.attachments[i].loadOp == VK_ATTACHMENT_LOAD_OP_DONT_CARE)
+              {
+                GetDebugManager()->FillWithDiscardPattern(commandBuffer, DiscardType::RenderPassLoad,
+                                                          image, initialLayout, viewInfo.range, rect);
+              }
             }
           }
         }
 
-        rdcarray<VkImageMemoryBarrier> imgBarriers = GetImplicitRenderPassBarriers();
-
-        // if we're just replaying the vkCmdBeginRenderPass on its own, we use the first loadRP
-        // instead of the real thing. This then doesn't require us to finish off any subpasses etc.
-        // we need to manually do the subpass 0 barriers, since loadRP expects the image to already
-        // be in subpass 0's layout
-        // we also need to manually do any clears, since the loadRP will load all attachments
-        if(m_FirstEventID == m_LastEventID)
-        {
-          unwrappedInfo.renderPass = Unwrap(rpinfo.loadRPs[0]);
-          unwrappedInfo.framebuffer = Unwrap(fbinfo.loadFBs[0]);
-
-          DoPipelineBarrier(commandBuffer, imgBarriers.size(), imgBarriers.data());
-        }
-
         DrawFlags drawFlags = DrawFlags::PassBoundary | DrawFlags::BeginPass;
         uint32_t eventId = HandlePreCallback(commandBuffer, drawFlags);
 

renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp

@@ -909,8 +909,11 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass(SerialiserType &ser, VkDevice d
         if(att[i].loadOp == VK_ATTACHMENT_LOAD_OP_DONT_CARE)
         {
           att[i].loadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
-          if(att[i].initialLayout == VK_IMAGE_LAYOUT_UNDEFINED)
-            att[i].initialLayout = VK_IMAGE_LAYOUT_GENERAL;
+        }
+        if(att[i].loadOp == VK_ATTACHMENT_LOAD_OP_LOAD &&
+           att[i].initialLayout == VK_IMAGE_LAYOUT_UNDEFINED)
+        {
+          att[i].initialLayout = VK_IMAGE_LAYOUT_GENERAL;
         }
         if(att[i].stencilLoadOp == VK_ATTACHMENT_LOAD_OP_DONT_CARE)
         {
@@ -918,6 +921,11 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass(SerialiserType &ser, VkDevice d
           if(att[i].initialLayout == VK_IMAGE_LAYOUT_UNDEFINED)
             att[i].initialLayout = VK_IMAGE_LAYOUT_GENERAL;
         }
+        if(att[i].stencilLoadOp == VK_ATTACHMENT_LOAD_OP_LOAD &&
+           att[i].initialLayout == VK_IMAGE_LAYOUT_UNDEFINED)
+        {
+          att[i].initialLayout = VK_IMAGE_LAYOUT_GENERAL;
+        }
       }
 
       // sanitise the actual layouts used to create the renderpass

util/test/demos/vk/vk_discard_zoo.cpp

@@ -49,9 +49,11 @@ RD_TEST(VK_Discard_Zoo, VulkanGraphicsTest)
 
     vkh::cmdPipelineBarrier(
         cmd, {
-                 vkh::ImageMemoryBarrier(VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_WRITE_BIT,
-                                         VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_GENERAL,
-                                         img.image, range),
+                 vkh::ImageMemoryBarrier(VK_ACCESS_TRANSFER_WRITE_BIT |
+                                             VK_ACCESS_COLOR_ATTACHMENT_READ_BIT |
+                                             VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT,
+                                         VK_ACCESS_TRANSFER_WRITE_BIT, VK_IMAGE_LAYOUT_UNDEFINED,
+                                         VK_IMAGE_LAYOUT_GENERAL, img.image, range),
              });
 
     VkClearDepthStencilValue val = {0.4f, 0x40};
@@ -264,7 +266,7 @@ RD_TEST(VK_Discard_Zoo, VulkanGraphicsTest)
         VK_FORMAT_R16G16B16A16_SFLOAT, VK_IMAGE_LAYOUT_GENERAL, VK_IMAGE_LAYOUT_GENERAL,
         VK_ATTACHMENT_LOAD_OP_DONT_CARE, VK_ATTACHMENT_STORE_OP_DONT_CARE));
     renderPassCreateInfo.attachments.push_back(vkh::AttachmentDescription(
-        depthStencilFormat, VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_GENERAL,
+        depthStencilFormat, VK_IMAGE_LAYOUT_GENERAL, VK_IMAGE_LAYOUT_GENERAL,
         VK_ATTACHMENT_LOAD_OP_DONT_CARE, VK_ATTACHMENT_STORE_OP_DONT_CARE, VK_SAMPLE_COUNT_1_BIT,
         VK_ATTACHMENT_LOAD_OP_DONT_CARE, VK_ATTACHMENT_STORE_OP_DONT_CARE));
     renderPassCreateInfo.attachments.push_back(vkh::AttachmentDescription(
@@ -276,6 +278,16 @@ RD_TEST(VK_Discard_Zoo, VulkanGraphicsTest)
 
     VkRenderPass renderPass = createRenderPass(renderPassCreateInfo);
 
+    renderPassCreateInfo.attachments[0].initialLayout = VK_IMAGE_LAYOUT_UNDEFINED;
+    renderPassCreateInfo.attachments[1].initialLayout = VK_IMAGE_LAYOUT_UNDEFINED;
+    renderPassCreateInfo.attachments[0].loadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
+    renderPassCreateInfo.attachments[1].loadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
+    renderPassCreateInfo.attachments[0].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
+    renderPassCreateInfo.attachments[1].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_LOAD;
+    renderPassCreateInfo.attachments[0].storeOp = VK_ATTACHMENT_STORE_OP_STORE;
+    renderPassCreateInfo.attachments[1].storeOp = VK_ATTACHMENT_STORE_OP_STORE;
+    VkRenderPass undefLoadRP = createRenderPass(renderPassCreateInfo);
+
     VkFramebuffer fb = createFramebuffer(vkh::FramebufferCreateInfo(
         renderPass, {colview, depthview, ignoreview}, mainWindow->scissor.extent));
 
@@ -502,6 +514,21 @@ RD_TEST(VK_Discard_Zoo, VulkanGraphicsTest)
                            vkh::ClearColorValue(0.2f, 0.2f, 0.2f, 1.0f), 1,
                            vkh::ImageSubresourceRange());
 
+      // make sure a renderpass with UNDEFINED initialLayout and LOAD_OP_LOAD still gets an
+      // undefined pattern.
+
+      // first re-clear the attachments
+      Clear(cmd, ignoreimg);
+      Clear(cmd, colimg);
+      setMarker(cmd, "UndefinedLoad_Before");
+      Clear(cmd, depthimg);
+
+      vkCmdBeginRenderPass(cmd, vkh::RenderPassBeginInfo(undefLoadRP, fb, sc),
+                           VK_SUBPASS_CONTENTS_INLINE);
+      vkCmdEndRenderPass(cmd);
+
+      setMarker(cmd, "UndefinedLoad_After");
+
       FinishUsingBackbuffer(cmd, VK_ACCESS_TRANSFER_WRITE_BIT, VK_IMAGE_LAYOUT_GENERAL);
 
       vkEndCommandBuffer(cmd);

util/test/tests/Vulkan/VK_Discard_Zoo.py

@@ -99,4 +99,40 @@ class VK_Discard_Zoo(rdtest.Discard_Zoo):
         end_depth_bytes = self.controller.GetTextureData(rpdepth.resourceId, rd.Subresource())
 
         self.check(middle_col_bytes != end_col_bytes)
-        self.check(middle_depth_bytes != end_depth_bytes)
+        self.check(middle_depth_bytes != end_depth_bytes)
+
+        draw = self.find_draw("UndefinedLoad_Before")
+
+        self.controller.SetFrameEvent(draw.next.eventId, True)
+
+        # check that they are cleared again
+        for y in range(0, rpcol.height-1, 17):
+            for x in range(0, rpcol.width-1, 17):
+                self.check_pixel_value(rpcol.resourceId, x, y, [0.0, 1.0, 0.0, 1.0])
+                self.check_pixel_value(rpdepth.resourceId, x, y, [0.4, float(0x40)/float(255), 0.0, 1.0])
+
+        rdtest.log.success("Values are correct before the UNDEFINED initial layout renderpass")
+
+        draw = self.find_draw("UndefinedLoad_After")
+
+        self.controller.SetFrameEvent(draw.next.eventId, True)
+
+        # check that they are all undefined pattern - initial layout affects the whole resource
+        for y in range(0, rpcol.height-1, 17):
+            for x in range(0, rpcol.width-1, 17):
+                c: rd.PixelValue = self.controller.PickPixel(rpcol.resourceId, x, y, rd.Subresource(),
+                                                             rd.CompType.Typeless)
+                d: rd.PixelValue = self.controller.PickPixel(rpdepth.resourceId, x, y, rd.Subresource(),
+                                                             rd.CompType.Typeless)
+
+                if not rdtest.value_compare(c.floatValue, [0.0] * 4) and not rdtest.value_compare(c.floatValue,
+                                                                                                  [1000.0] * 4):
+                    raise rdtest.TestFailureException(
+                        'undefined color has unexpected value at {},{}: {}'.format(x, y, c.floatValue))
+
+                if not rdtest.value_compare(d.floatValue[0:2], [0.0] * 2) and not rdtest.value_compare(
+                        d.floatValue[0:2], [1.0] * 2):
+                    raise rdtest.TestFailureException(
+                        'undefined depth has unexpected value at {},{}: {}'.format(x, y, d.floatValue))
+
+        rdtest.log.success("Values are correct after the UNDEFINED initial layout renderpass")