Merge pull request #5270 from stenzek/vulkan-aa

Vulkan: MSAA and frame dumping fixes

Source/Core/VideoBackends/Vulkan/FramebufferManager.cpp

@@ -462,6 +462,12 @@ Texture2D* FramebufferManager::ResolveEFBColorTexture(const VkRect2D& region)
   // Can't resolve within a render pass.
   StateTracker::GetInstance()->EndRenderPass();
 
+  // It's not valid to resolve out-of-bounds coordinates.
+  // Ensuring the region is within the image is the caller's responsibility.
+  _assert_(region.offset.x >= 0 && region.offset.y >= 0 &&
+           (static_cast<u32>(region.offset.x) + region.extent.width) <= m_efb_width &&
+           (static_cast<u32>(region.offset.y) + region.extent.height) <= m_efb_height);
+
   // Resolving is considered to be a transfer operation.
   m_efb_color_texture->TransitionToLayout(g_command_buffer_mgr->GetCurrentCommandBuffer(),
                                           VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL);

Source/Core/VideoBackends/Vulkan/FramebufferManager.h

@@ -67,6 +67,9 @@ public:
   Texture2D* ResolveEFBColorTexture(const VkRect2D& region);
   Texture2D* ResolveEFBDepthTexture(const VkRect2D& region);
 
+  // Returns the texture that the EFB color texture is resolved to when multisampling is enabled.
+  // Ensure ResolveEFBColorTexture is called before this method.
+  Texture2D* GetResolvedEFBColorTexture() const { return m_efb_resolve_color_texture.get(); }
   // Reads a framebuffer value back from the GPU. This may block if the cache is not current.
   u32 PeekEFBColor(u32 x, u32 y);
   float PeekEFBDepth(u32 x, u32 y);

Source/Core/VideoBackends/Vulkan/Renderer.cpp

@@ -505,6 +505,15 @@ void Renderer::SwapImpl(u32 xfb_addr, u32 fb_width, u32 fb_stride, u32 fb_height
   // are determined by guest state. Currently, the only way to catch these is to update every frame.
   UpdateDrawRectangle();
 
+  // Scale the source rectangle to the internal resolution when XFB is disabled.
+  TargetRectangle scaled_efb_rect = Renderer::ConvertEFBRectangle(rc);
+
+  // If MSAA is enabled, and we're not using XFB, we need to resolve the EFB framebuffer before
+  // rendering the final image to the screen, or dumping the frame. This is because we can't resolve
+  // an image within a render pass, which will have already started by the time it is used.
+  if (g_ActiveConfig.iMultisamples > 1 && !g_ActiveConfig.bUseXFB)
+    ResolveEFBForSwap(scaled_efb_rect);
+
   // Render the frame dump image if enabled.
   if (IsFrameDumping())
   {
@@ -512,7 +521,8 @@ void Renderer::SwapImpl(u32 xfb_addr, u32 fb_width, u32 fb_stride, u32 fb_height
     if (!m_frame_dumping_active)
       StartFrameDumping();
 
-    DrawFrameDump(rc, xfb_addr, xfb_sources, xfb_count, fb_width, fb_stride, fb_height, ticks);
+    DrawFrameDump(scaled_efb_rect, xfb_addr, xfb_sources, xfb_count, fb_width, fb_stride, fb_height,
+                  ticks);
   }
   else
   {
@@ -529,7 +539,7 @@ void Renderer::SwapImpl(u32 xfb_addr, u32 fb_width, u32 fb_stride, u32 fb_height
   // Draw to the screen if we have a swap chain.
   if (m_swap_chain)
   {
-    DrawScreen(rc, xfb_addr, xfb_sources, xfb_count, fb_width, fb_stride, fb_height);
+    DrawScreen(scaled_efb_rect, xfb_addr, xfb_sources, xfb_count, fb_width, fb_stride, fb_height);
 
     // Submit the current command buffer, signaling rendering finished semaphore when it's done
     // Because this final command buffer is rendering to the swap chain, we need to wait for
@@ -573,13 +583,25 @@ void Renderer::SwapImpl(u32 xfb_addr, u32 fb_width, u32 fb_stride, u32 fb_height
   TextureCache::GetInstance()->Cleanup(frameCount);
 }
 
+void Renderer::ResolveEFBForSwap(const TargetRectangle& scaled_rect)
+{
+  // While the source rect can be out-of-range when drawing, the resolve rectangle must be within
+  // the bounds of the texture.
+  VkRect2D region = {
+      {scaled_rect.left, scaled_rect.top},
+      {static_cast<u32>(scaled_rect.GetWidth()), static_cast<u32>(scaled_rect.GetHeight())}};
+  region = Util::ClampRect2D(region, FramebufferManager::GetInstance()->GetEFBWidth(),
+                             FramebufferManager::GetInstance()->GetEFBHeight());
+  FramebufferManager::GetInstance()->ResolveEFBColorTexture(region);
+}
+
 void Renderer::DrawFrame(VkRenderPass render_pass, const TargetRectangle& target_rect,
-                         const EFBRectangle& source_rect, u32 xfb_addr,
+                         const TargetRectangle& scaled_efb_rect, u32 xfb_addr,
                          const XFBSourceBase* const* xfb_sources, u32 xfb_count, u32 fb_width,
                          u32 fb_stride, u32 fb_height)
 {
   if (!g_ActiveConfig.bUseXFB)
-    DrawEFB(render_pass, target_rect, source_rect);
+    DrawEFB(render_pass, target_rect, scaled_efb_rect);
   else if (!g_ActiveConfig.bUseRealXFB)
     DrawVirtualXFB(render_pass, target_rect, xfb_addr, xfb_sources, xfb_count, fb_width, fb_stride,
                    fb_height);
@@ -588,26 +610,18 @@ void Renderer::DrawFrame(VkRenderPass render_pass, const TargetRectangle& target
 }
 
 void Renderer::DrawEFB(VkRenderPass render_pass, const TargetRectangle& target_rect,
-                       const EFBRectangle& source_rect)
+                       const TargetRectangle& scaled_efb_rect)
 {
-  // Scale the source rectangle to the selected internal resolution.
-  TargetRectangle scaled_source_rect = Renderer::ConvertEFBRectangle(source_rect);
-  scaled_source_rect.left = std::max(scaled_source_rect.left, 0);
-  scaled_source_rect.right = std::max(scaled_source_rect.right, 0);
-  scaled_source_rect.top = std::max(scaled_source_rect.top, 0);
-  scaled_source_rect.bottom = std::max(scaled_source_rect.bottom, 0);
-
   // Transition the EFB render target to a shader resource.
-  VkRect2D src_region = {{0, 0},
-                         {static_cast<u32>(scaled_source_rect.GetWidth()),
-                          static_cast<u32>(scaled_source_rect.GetHeight())}};
   Texture2D* efb_color_texture =
-      FramebufferManager::GetInstance()->ResolveEFBColorTexture(src_region);
+      g_ActiveConfig.iMultisamples > 1 ?
+          FramebufferManager::GetInstance()->GetResolvedEFBColorTexture() :
+          FramebufferManager::GetInstance()->GetEFBColorTexture();
   efb_color_texture->TransitionToLayout(g_command_buffer_mgr->GetCurrentCommandBuffer(),
                                         VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
 
   // Copy EFB -> backbuffer
-  BlitScreen(render_pass, target_rect, scaled_source_rect, efb_color_texture, true);
+  BlitScreen(render_pass, target_rect, scaled_efb_rect, efb_color_texture, true);
 
   // Restore the EFB color texture to color attachment ready for rendering the next frame.
   if (efb_color_texture == FramebufferManager::GetInstance()->GetEFBColorTexture())
@@ -670,7 +684,7 @@ void Renderer::DrawRealXFB(VkRenderPass render_pass, const TargetRectangle& targ
   }
 }
 
-void Renderer::DrawScreen(const EFBRectangle& source_rect, u32 xfb_addr,
+void Renderer::DrawScreen(const TargetRectangle& scaled_efb_rect, u32 xfb_addr,
                           const XFBSourceBase* const* xfb_sources, u32 xfb_count, u32 fb_width,
                           u32 fb_stride, u32 fb_height)
 {
@@ -713,8 +727,8 @@ void Renderer::DrawScreen(const EFBRectangle& source_rect, u32 xfb_addr,
                        VK_SUBPASS_CONTENTS_INLINE);
 
   // Draw guest buffers (EFB or XFB)
-  DrawFrame(m_swap_chain->GetRenderPass(), GetTargetRectangle(), source_rect, xfb_addr, xfb_sources,
+  DrawFrame(m_swap_chain->GetRenderPass(), GetTargetRectangle(), scaled_efb_rect, xfb_addr,
-            xfb_count, fb_width, fb_stride, fb_height);
+            xfb_sources, xfb_count, fb_width, fb_stride, fb_height);
 
   // Draw OSD
   Util::SetViewportAndScissor(g_command_buffer_mgr->GetCurrentCommandBuffer(), 0, 0,
@@ -732,7 +746,7 @@ void Renderer::DrawScreen(const EFBRectangle& source_rect, u32 xfb_addr,
                                  VK_IMAGE_LAYOUT_PRESENT_SRC_KHR);
 }
 
-bool Renderer::DrawFrameDump(const EFBRectangle& source_rect, u32 xfb_addr,
+bool Renderer::DrawFrameDump(const TargetRectangle& scaled_efb_rect, u32 xfb_addr,
                              const XFBSourceBase* const* xfb_sources, u32 xfb_count, u32 fb_width,
                              u32 fb_stride, u32 fb_height, u64 ticks)
 {
@@ -742,6 +756,10 @@ bool Renderer::DrawFrameDump(const EFBRectangle& source_rect, u32 xfb_addr,
   if (!ResizeFrameDumpBuffer(width, height))
     return false;
 
+  // If there was a previous frame dumped, we'll still be in TRANSFER_SRC layout.
+  m_frame_dump_render_texture->TransitionToLayout(g_command_buffer_mgr->GetCurrentCommandBuffer(),
+                                                  VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
+
   VkClearValue clear_value = {{{0.0f, 0.0f, 0.0f, 1.0f}}};
   VkClearRect clear_rect = {{{0, 0}, {width, height}}, 0, 1};
   VkClearAttachment clear_attachment = {VK_IMAGE_ASPECT_COLOR_BIT, 0, clear_value};
@@ -758,7 +776,7 @@ bool Renderer::DrawFrameDump(const EFBRectangle& source_rect, u32 xfb_addr,
   vkCmdClearAttachments(g_command_buffer_mgr->GetCurrentCommandBuffer(), 1, &clear_attachment, 1,
                         &clear_rect);
   DrawFrame(FramebufferManager::GetInstance()->GetColorCopyForReadbackRenderPass(), target_rect,
-            source_rect, xfb_addr, xfb_sources, xfb_count, fb_width, fb_stride, fb_height);
+            scaled_efb_rect, xfb_addr, xfb_sources, xfb_count, fb_width, fb_stride, fb_height);
   vkCmdEndRenderPass(g_command_buffer_mgr->GetCurrentCommandBuffer());
 
   // Prepare the readback texture for copying.
@@ -767,6 +785,8 @@ bool Renderer::DrawFrameDump(const EFBRectangle& source_rect, u32 xfb_addr,
     return false;
 
   // Queue a copy to the current frame dump buffer. It will be written to the frame dump later.
+  m_frame_dump_render_texture->TransitionToLayout(g_command_buffer_mgr->GetCurrentCommandBuffer(),
+                                                  VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL);
   readback_texture->CopyFromImage(g_command_buffer_mgr->GetCurrentCommandBuffer(),
                                   m_frame_dump_render_texture->GetImage(),
                                   VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, width, height, 0, 0);
@@ -939,6 +959,10 @@ bool Renderer::ResizeFrameDumpBuffer(u32 new_width, u32 new_height)
     return true;
   }
 
+  // Ensure all previous frames have been dumped, since we are destroying a framebuffer
+  // that may still be in use.
+  FlushFrameDump();
+
   if (m_frame_dump_framebuffer != VK_NULL_HANDLE)
   {
     vkDestroyFramebuffer(g_vulkan_context->GetDevice(), m_frame_dump_framebuffer, nullptr);

Source/Core/VideoBackends/Vulkan/Renderer.h

@@ -90,13 +90,15 @@ private:
   bool CompileShaders();
   void DestroyShaders();
 
+  void ResolveEFBForSwap(const TargetRectangle& scaled_rect);
+
   // Draw either the EFB, or specified XFB sources to the currently-bound framebuffer.
   void DrawFrame(VkRenderPass render_pass, const TargetRectangle& target_rect,
-                 const EFBRectangle& source_rect, u32 xfb_addr,
+                 const TargetRectangle& scaled_efb_rect, u32 xfb_addr,
                  const XFBSourceBase* const* xfb_sources, u32 xfb_count, u32 fb_width,
                  u32 fb_stride, u32 fb_height);
   void DrawEFB(VkRenderPass render_pass, const TargetRectangle& target_rect,
-               const EFBRectangle& source_rect);
+               const TargetRectangle& scaled_efb_rect);
   void DrawVirtualXFB(VkRenderPass render_pass, const TargetRectangle& target_rect, u32 xfb_addr,
                       const XFBSourceBase* const* xfb_sources, u32 xfb_count, u32 fb_width,
                       u32 fb_stride, u32 fb_height);
@@ -105,12 +107,14 @@ private:
                    u32 fb_stride, u32 fb_height);
 
   // Draw the frame, as well as the OSD to the swap chain.
-  void DrawScreen(const EFBRectangle& rc, u32 xfb_addr, const XFBSourceBase* const* xfb_sources,
+  void DrawScreen(const TargetRectangle& scaled_efb_rect, u32 xfb_addr,
-                  u32 xfb_count, u32 fb_width, u32 fb_stride, u32 fb_height);
+                  const XFBSourceBase* const* xfb_sources, u32 xfb_count, u32 fb_width,
+                  u32 fb_stride, u32 fb_height);
 
   // Draw the frame only to the screenshot buffer.
-  bool DrawFrameDump(const EFBRectangle& rc, u32 xfb_addr, const XFBSourceBase* const* xfb_sources,
+  bool DrawFrameDump(const TargetRectangle& scaled_efb_rect, u32 xfb_addr,
-                     u32 xfb_count, u32 fb_width, u32 fb_stride, u32 fb_height, u64 ticks);
+                     const XFBSourceBase* const* xfb_sources, u32 xfb_count, u32 fb_width,
+                     u32 fb_stride, u32 fb_height, u64 ticks);
 
   // Sets up renderer state to permit framedumping.
   // Ideally we would have EndFrameDumping be a virtual method of Renderer, but due to various

Source/Core/VideoBackends/Vulkan/TextureCache.cpp

@@ -96,11 +96,14 @@ void TextureCache::CopyEFB(u8* dst, const EFBCopyFormat& format, u32 native_widt
   FramebufferManager::GetInstance()->FlushEFBPokes();
 
   // MSAA case where we need to resolve first.
-  // TODO: Do in one pass.
+  // An out-of-bounds source region is valid here, and fine for the draw (since it is converted
+  // to texture coordinates), but it's not valid to resolve an out-of-range rectangle.
   TargetRectangle scaled_src_rect = g_renderer->ConvertEFBRectangle(src_rect);
   VkRect2D region = {{scaled_src_rect.left, scaled_src_rect.top},
                      {static_cast<u32>(scaled_src_rect.GetWidth()),
                       static_cast<u32>(scaled_src_rect.GetHeight())}};
+  region = Util::ClampRect2D(region, FramebufferManager::GetInstance()->GetEFBWidth(),
+                             FramebufferManager::GetInstance()->GetEFBHeight());
   Texture2D* src_texture;
   if (is_depth_copy)
     src_texture = FramebufferManager::GetInstance()->ResolveEFBDepthTexture(region);
@@ -465,10 +468,14 @@ void TextureCache::TCacheEntry::FromRenderTarget(bool is_depth_copy, const EFBRe
   VkCommandBuffer command_buffer = g_command_buffer_mgr->GetCurrentCommandBuffer();
   StateTracker::GetInstance()->EndRenderPass();
 
-  // Transition EFB to shader resource before binding
+  // Transition EFB to shader resource before binding.
+  // An out-of-bounds source region is valid here, and fine for the draw (since it is converted
+  // to texture coordinates), but it's not valid to resolve an out-of-range rectangle.
   VkRect2D region = {{scaled_src_rect.left, scaled_src_rect.top},
                      {static_cast<u32>(scaled_src_rect.GetWidth()),
                       static_cast<u32>(scaled_src_rect.GetHeight())}};
+  region = Util::ClampRect2D(region, FramebufferManager::GetInstance()->GetEFBWidth(),
+                             FramebufferManager::GetInstance()->GetEFBHeight());
   Texture2D* src_texture;
   if (is_depth_copy)
     src_texture = FramebufferManager::GetInstance()->ResolveEFBDepthTexture(region);

Source/Core/VideoBackends/Vulkan/Util.cpp

@@ -97,6 +97,16 @@ u32 GetTexelSize(VkFormat format)
   }
 }
 
+VkRect2D ClampRect2D(const VkRect2D& rect, u32 width, u32 height)
+{
+  VkRect2D out;
+  out.offset.x = MathUtil::Clamp(rect.offset.x, 0, static_cast<int32_t>(width - 1));
+  out.offset.y = MathUtil::Clamp(rect.offset.y, 0, static_cast<int32_t>(height - 1));
+  out.extent.width = std::min(rect.extent.width, width - static_cast<uint32_t>(rect.offset.x));
+  out.extent.height = std::min(rect.extent.height, height - static_cast<uint32_t>(rect.offset.y));
+  return out;
+}
+
 VkBlendFactor GetAlphaBlendFactor(VkBlendFactor factor)
 {
   switch (factor)

Source/Core/VideoBackends/Vulkan/Util.h

@@ -27,6 +27,9 @@ bool IsDepthFormat(VkFormat format);
 VkFormat GetLinearFormat(VkFormat format);
 u32 GetTexelSize(VkFormat format);
 
+// Clamps a VkRect2D to the specified dimensions.
+VkRect2D ClampRect2D(const VkRect2D& rect, u32 width, u32 height);
+
 // Map {SRC,DST}_COLOR to {SRC,DST}_ALPHA
 VkBlendFactor GetAlphaBlendFactor(VkBlendFactor factor);