From 134daf3b00d1bf46a3ad4b57405f8844b809bb4f Mon Sep 17 00:00:00 2001
From: Stenzek <stenzek@gmail.com>
Date: Sun, 3 Sep 2017 14:04:14 +1000
Subject: [PATCH] Vulkan: Extend the NVIDIA MSAA bug to render-pass based
 clears

Calling vkCmdClearAttachments with a partial rect, or specifying a
render area in a render pass with the load op set to clear can cause the
GPU to lock up, or raise a bounds violation. This only occurs on MSAA
framebuffers, and it seems when there are multiple clears in a single
command buffer. Worked around by back to the slow path (drawing quads)
when MSAA is enabled.
---
 Source/Core/VideoBackends/Vulkan/Renderer.cpp | 28 +++++++++++--------
 Source/Core/VideoCommon/DriverDetails.cpp     |  4 +--
 Source/Core/VideoCommon/DriverDetails.h       | 11 ++++++--
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/Source/Core/VideoBackends/Vulkan/Renderer.cpp b/Source/Core/VideoBackends/Vulkan/Renderer.cpp
index bb8b36481a..7a94572b1e 100644
--- a/Source/Core/VideoBackends/Vulkan/Renderer.cpp
+++ b/Source/Core/VideoBackends/Vulkan/Renderer.cpp
@@ -375,18 +375,23 @@ void Renderer::ClearScreen(const EFBRectangle& rc, bool color_enable, bool alpha
 
   // If we're not in a render pass (start of the frame), we can use a clear render pass
   // to discard the data, rather than loading and then clearing.
-  bool use_clear_render_pass = (color_enable && alpha_enable && z_enable);
+  bool use_clear_attachments = (color_enable && alpha_enable) || z_enable;
+  bool use_clear_render_pass =
+      !StateTracker::GetInstance()->InRenderPass() && color_enable && alpha_enable && z_enable;
+
+  // The NVIDIA Vulkan driver causes the GPU to lock up, or throw exceptions if MSAA is enabled,
+  // a non-full clear rect is specified, and a clear loadop or vkCmdClearAttachments is used.
+  if (g_ActiveConfig.iMultisamples > 1 &&
+      DriverDetails::HasBug(DriverDetails::BUG_BROKEN_MSAA_CLEAR))
+  {
+    use_clear_render_pass = false;
+    use_clear_attachments = false;
+  }
+
+  // This path cannot be used if the driver implementation doesn't guarantee pixels with no drawn
+  // geometry in "this" renderpass won't be cleared
   if (DriverDetails::HasBug(DriverDetails::BUG_BROKEN_CLEAR_LOADOP_RENDERPASS))
-  {
-    // This path cannot be used if the driver implementation doesn't guarantee pixels with no drawn
-    // geomerty in "this" renderpass won't be cleared
     use_clear_render_pass = false;
-  }
-  if (StateTracker::GetInstance()->InRenderPass())
-  {
-    // Prefer not to end a render pass just to do a clear.
-    use_clear_render_pass = false;
-  }
 
   // Fastest path: Use a render pass to clear the buffers.
   if (use_clear_render_pass)
@@ -398,8 +403,7 @@ void Renderer::ClearScreen(const EFBRectangle& rc, bool color_enable, bool alpha
 
   // Fast path: Use vkCmdClearAttachments to clear the buffers within a render path
   // We can't use this when preserving alpha but clearing color.
-  if (g_ActiveConfig.iMultisamples == 1 ||
-      !DriverDetails::HasBug(DriverDetails::BUG_BROKEN_MSAA_VKCMDCLEARATTACHMENTS))
+  if (use_clear_attachments)
   {
     VkClearAttachment clear_attachments[2];
     uint32_t num_clear_attachments = 0;
diff --git a/Source/Core/VideoCommon/DriverDetails.cpp b/Source/Core/VideoCommon/DriverDetails.cpp
index cce4daddb7..2885aea7e9 100644
--- a/Source/Core/VideoCommon/DriverDetails.cpp
+++ b/Source/Core/VideoCommon/DriverDetails.cpp
@@ -102,8 +102,8 @@ static BugInfo m_known_bugs[] = {
      BUG_SHARED_CONTEXT_SHADER_COMPILATION, -1.0, -1.0, true},
     {API_OPENGL, OS_LINUX, VENDOR_MESA, DRIVER_NOUVEAU, Family::UNKNOWN,
      BUG_SHARED_CONTEXT_SHADER_COMPILATION, -1.0, -1.0, true},
-    {API_VULKAN, OS_ALL, VENDOR_NVIDIA, DRIVER_NVIDIA, Family::UNKNOWN,
-     BUG_BROKEN_MSAA_VKCMDCLEARATTACHMENTS, -1.0, -1.0, true},
+    {API_VULKAN, OS_ALL, VENDOR_NVIDIA, DRIVER_NVIDIA, Family::UNKNOWN, BUG_BROKEN_MSAA_CLEAR, -1.0,
+     -1.0, true},
     {API_VULKAN, OS_ALL, VENDOR_IMGTEC, DRIVER_IMGTEC, Family::UNKNOWN,
      BUG_BROKEN_CLEAR_LOADOP_RENDERPASS, -1.0, -1.0, true},
 };
diff --git a/Source/Core/VideoCommon/DriverDetails.h b/Source/Core/VideoCommon/DriverDetails.h
index 5a0a0f86c5..53f0c68739 100644
--- a/Source/Core/VideoCommon/DriverDetails.h
+++ b/Source/Core/VideoCommon/DriverDetails.h
@@ -254,11 +254,16 @@ enum Bug
   // Ended Version: -1
   BUG_SHARED_CONTEXT_SHADER_COMPILATION,
 
-  // Bug: vkCmdClearAttachments with MSAA enabled causes NVIDIA Maxwell+ cards to lock up.
+  // Bug: Fast clears on a MSAA framebuffer can cause NVIDIA GPU resets/lockups.
   // Started version: -1
   // Ended version: -1
-  // Seems to only occur when the top of the clear rect is non-zero.
-  BUG_BROKEN_MSAA_VKCMDCLEARATTACHMENTS,
+  // Calling vkCmdClearAttachments with a partial rect, or specifying a render area in a
+  // render pass with the load op set to clear can cause the GPU to lock up, or raise a
+  // bounds violation. This only occurs on MSAA framebuffers, and it seems when there are
+  // multiple clears in a single command buffer. Worked around by back to the slow path
+  // (drawing quads) when MSAA is enabled.
+  BUG_BROKEN_MSAA_CLEAR,
+
   // BUG: Some vulkan implementations don't like the 'clear' loadop renderpass.
   // For example, the ImgTec VK driver fails if you try to use a framebuffer with a different
   // load/store op than that which it was created with, despite the spec saying they should be