Merge pull request #4489 from stenzek/vulkan-minor-fixes

Vulkan: Minor fixes

Source/Core/VideoBackends/Vulkan/FramebufferManager.h

@@ -184,7 +184,6 @@ public:
 
 private:
   std::unique_ptr<TextureCache::TCacheEntry> m_texture;
-  VkFramebuffer m_framebuffer;
 };
 
 }  // namespace Vulkan

Source/Core/VideoBackends/Vulkan/Renderer.cpp

@@ -667,8 +667,15 @@ void Renderer::DrawScreen(const EFBRectangle& source_rect, u32 xfb_addr,
   VkResult res = m_swap_chain->AcquireNextImage(m_image_available_semaphore);
   if (res == VK_SUBOPTIMAL_KHR || res == VK_ERROR_OUT_OF_DATE_KHR)
   {
-    // Window has been resized. Update the swap chain and try again.
+    // There's an issue here. We can't resize the swap chain while the GPU is still busy with it,
+    // but calling WaitForGPUIdle would create a deadlock as PrepareToSubmitCommandBuffer has been
+    // called by SwapImpl. WaitForGPUIdle waits on the semaphore, which PrepareToSubmitCommandBuffer
+    // has already done, so it blocks indefinitely. To work around this, we submit the current
+    // command buffer, resize the swap chain (which calls WaitForGPUIdle), and then finally call
+    // PrepareToSubmitCommandBuffer to return to the state that the caller expects.
+    g_command_buffer_mgr->SubmitCommandBuffer(false);
     ResizeSwapChain();
+    g_command_buffer_mgr->PrepareToSubmitCommandBuffer();
     res = m_swap_chain->AcquireNextImage(m_image_available_semaphore);
   }
   if (res != VK_SUCCESS)

Source/Core/VideoBackends/Vulkan/TextureCache.cpp

@@ -556,7 +556,7 @@ bool TextureCache::TCacheEntry::Save(const std::string& filename, unsigned int l
   Util::ExecuteCurrentCommandsAndRestoreState(false, true);
 
   // Map the staging texture so we can copy the contents out.
-  if (staging_texture->Map())
+  if (!staging_texture->Map())
   {
     PanicAlert("Failed to map staging texture");
     return false;

Source/Core/VideoBackends/Vulkan/VulkanContext.cpp

@@ -82,10 +82,11 @@ bool VulkanContext::CheckValidationLayerAvailablility()
           }) != layer_list.end());
 }
 
-VkInstance VulkanContext::CreateVulkanInstance(bool enable_surface, bool enable_validation_layer)
+VkInstance VulkanContext::CreateVulkanInstance(bool enable_surface, bool enable_debug_report,
+                                               bool enable_validation_layer)
 {
   ExtensionList enabled_extensions;
-  if (!SelectInstanceExtensions(&enabled_extensions, enable_surface, enable_validation_layer))
+  if (!SelectInstanceExtensions(&enabled_extensions, enable_surface, enable_debug_report))
     return VK_NULL_HANDLE;
 
   VkApplicationInfo app_info = {};
@@ -127,7 +128,7 @@ VkInstance VulkanContext::CreateVulkanInstance(bool enable_surface, bool enable_
 }
 
 bool VulkanContext::SelectInstanceExtensions(ExtensionList* extension_list, bool enable_surface,
-                                             bool enable_validation_layer)
+                                             bool enable_debug_report)
 {
   u32 extension_count = 0;
   VkResult res = vkEnumerateInstanceExtensionProperties(nullptr, &extension_count, nullptr);
@@ -192,8 +193,8 @@ bool VulkanContext::SelectInstanceExtensions(ExtensionList* extension_list, bool
 #endif
 
   // VK_EXT_debug_report
-  if (enable_validation_layer && !CheckForExtension(VK_EXT_DEBUG_REPORT_EXTENSION_NAME, true))
+  if (enable_debug_report && !CheckForExtension(VK_EXT_DEBUG_REPORT_EXTENSION_NAME, true))
-    return false;
+    WARN_LOG(VIDEO, "Vulkan: Debug report requested, but extension is not available.");
 
   return true;
 }
@@ -267,10 +268,10 @@ void VulkanContext::PopulateBackendInfoFeatures(VideoConfig* config, VkPhysicalD
   // Disable geometry shader when shaderTessellationAndGeometryPointSize is not supported.
   // Seems this is needed for gl_Layer.
   if (!features.shaderTessellationAndGeometryPointSize)
+  {
     config->backend_info.bSupportsGeometryShaders = VK_FALSE;
-
+    config->backend_info.bSupportsGSInstancing = VK_FALSE;
-  // TODO: Investigate if there's a feature we can enable for GS instancing.
+  }
-  config->backend_info.bSupportsGSInstancing = VK_FALSE;
 
   // Depth clamping implies shaderClipDistance and depthClamp
   config->backend_info.bSupportsDepthClamp =
@@ -294,7 +295,7 @@ void VulkanContext::PopulateBackendInfoMultisampleModes(
   VkSampleCountFlags supported_sample_counts = properties.limits.framebufferColorSampleCounts &
                                                properties.limits.framebufferDepthSampleCounts &
                                                efb_color_properties.sampleCounts &
-                                               efb_color_properties.sampleCounts;
+                                               efb_depth_properties.sampleCounts;
 
   // No AA
   config->backend_info.AAModes.clear();
@@ -327,6 +328,7 @@ void VulkanContext::PopulateBackendInfoMultisampleModes(
 
 std::unique_ptr<VulkanContext> VulkanContext::Create(VkInstance instance, VkPhysicalDevice gpu,
                                                      VkSurfaceKHR surface, VideoConfig* config,
+                                                     bool enable_debug_reports,
                                                      bool enable_validation_layer)
 {
   std::unique_ptr<VulkanContext> context = std::make_unique<VulkanContext>(instance, gpu);
@@ -338,8 +340,8 @@ std::unique_ptr<VulkanContext> VulkanContext::Create(VkInstance instance, VkPhys
                       static_cast<double>(context->m_device_properties.driverVersion),
                       DriverDetails::Family::UNKNOWN);
 
-  // Enable debug reports if validation layer is enabled.
+  // Enable debug reports if the "Host GPU" log category is enabled.
-  if (enable_validation_layer)
+  if (enable_debug_reports)
     context->EnableDebugReports();
 
   // Attempt to create the device.
@@ -358,8 +360,7 @@ std::unique_ptr<VulkanContext> VulkanContext::Create(VkInstance instance, VkPhys
   return context;
 }
 
-bool VulkanContext::SelectDeviceExtensions(ExtensionList* extension_list, bool enable_surface,
+bool VulkanContext::SelectDeviceExtensions(ExtensionList* extension_list, bool enable_surface)
-                                           bool enable_validation_layer)
 {
   u32 extension_count = 0;
   VkResult res =
@@ -405,9 +406,7 @@ bool VulkanContext::SelectDeviceExtensions(ExtensionList* extension_list, bool e
   };
 
   if (enable_surface && !CheckForExtension(VK_KHR_SWAPCHAIN_EXTENSION_NAME, true))
-  {
     return false;
-  }
 
   return true;
 }
@@ -527,8 +526,7 @@ bool VulkanContext::CreateDevice(VkSurfaceKHR surface, bool enable_validation_la
   device_info.pQueueCreateInfos = &queue_info;
 
   ExtensionList enabled_extensions;
-  if (!SelectDeviceExtensions(&enabled_extensions, (surface != VK_NULL_HANDLE),
+  if (!SelectDeviceExtensions(&enabled_extensions, surface != VK_NULL_HANDLE))
-                              enable_validation_layer))
     return false;
 
   device_info.enabledLayerCount = 0;

Source/Core/VideoBackends/Vulkan/VulkanContext.h

@@ -23,7 +23,8 @@ public:
   static bool CheckValidationLayerAvailablility();
 
   // Helper method to create a Vulkan instance.
-  static VkInstance CreateVulkanInstance(bool enable_surface, bool enable_validation_layer);
+  static VkInstance CreateVulkanInstance(bool enable_surface, bool enable_debug_report,
+                                         bool enable_validation_layer);
 
   // Returns a list of Vulkan-compatible GPUs.
   using GPUList = std::vector<VkPhysicalDevice>;
@@ -43,10 +44,10 @@ public:
   // been called for the specified VideoConfig.
   static std::unique_ptr<VulkanContext> Create(VkInstance instance, VkPhysicalDevice gpu,
                                                VkSurfaceKHR surface, VideoConfig* config,
+                                               bool enable_debug_reports,
                                                bool enable_validation_layer);
 
   // Enable/disable debug message runtime.
-  // In the future this could be hooked up to the Host GPU logging option.
   bool EnableDebugReports();
   void DisableDebugReports();
 
@@ -106,9 +107,8 @@ public:
 private:
   using ExtensionList = std::vector<const char*>;
   static bool SelectInstanceExtensions(ExtensionList* extension_list, bool enable_surface,
-                                       bool enable_validation_layer);
+                                       bool enable_debug_report);
-  bool SelectDeviceExtensions(ExtensionList* extension_list, bool enable_surface,
+  bool SelectDeviceExtensions(ExtensionList* extension_list, bool enable_surface);
-                              bool enable_validation_layer);
   bool SelectDeviceFeatures();
   bool CreateDevice(VkSurfaceKHR surface, bool enable_validation_layer);
 

Source/Core/VideoBackends/Vulkan/main.cpp

@@ -4,6 +4,7 @@
 
 #include <vector>
 
+#include "Common/Logging/LogManager.h"
 #include "Core/Host.h"
 
 #include "VideoBackends/Vulkan/CommandBufferManager.h"
@@ -32,7 +33,7 @@ void VideoBackend::InitBackendInfo()
 
   if (LoadVulkanLibrary())
   {
-    VkInstance temp_instance = VulkanContext::CreateVulkanInstance(false, false);
+    VkInstance temp_instance = VulkanContext::CreateVulkanInstance(false, false, false);
     if (temp_instance)
     {
       if (LoadVulkanInstanceFunctions(temp_instance))
@@ -72,6 +73,23 @@ void VideoBackend::InitBackendInfo()
   }
 }
 
+// Helper method to check whether the Host GPU logging category is enabled.
+static bool IsHostGPULoggingEnabled()
+{
+  return LogManager::GetInstance()->IsEnabled(LogTypes::HOST_GPU, LogTypes::LERROR);
+}
+
+// Helper method to determine whether to enable the debug report extension.
+static bool ShouldEnableDebugReports(bool enable_validation_layers)
+{
+  // Enable debug reports if the Host GPU log option is checked, or validation layers are enabled.
+  // The only issue here is that if Host GPU is not checked when the instance is created, the debug
+  // report extension will not be enabled, requiring the game to be restarted before any reports
+  // will be logged. Otherwise, we'd have to enable debug reports on every instance, when most
+  // users will never check the Host GPU logging category.
+  return enable_validation_layers || IsHostGPULoggingEnabled();
+}
+
 bool VideoBackend::Initialize(void* window_handle)
 {
   if (!LoadVulkanLibrary())
@@ -87,7 +105,7 @@ bool VideoBackend::Initialize(void* window_handle)
   InitBackendInfo();
   InitializeShared();
 
-  // Check for presence of the debug layer before trying to enable it
+  // Check for presence of the validation layers before trying to enable it
   bool enable_validation_layer = g_Config.bEnableValidationLayer;
   if (enable_validation_layer && !VulkanContext::CheckValidationLayerAvailablility())
   {
@@ -96,9 +114,10 @@ bool VideoBackend::Initialize(void* window_handle)
   }
 
   // Create Vulkan instance, needed before we can create a surface.
-  bool enable_surface = (window_handle != nullptr);
+  bool enable_surface = window_handle != nullptr;
-  VkInstance instance =
+  bool enable_debug_reports = ShouldEnableDebugReports(enable_validation_layer);
-      VulkanContext::CreateVulkanInstance(enable_surface, enable_validation_layer);
+  VkInstance instance = VulkanContext::CreateVulkanInstance(enable_surface, enable_debug_reports,
+                                                            enable_validation_layer);
   if (instance == VK_NULL_HANDLE)
   {
     PanicAlert("Failed to create Vulkan instance.");
@@ -155,8 +174,9 @@ bool VideoBackend::Initialize(void* window_handle)
   }
 
   // Pass ownership over to VulkanContext, and let it take care of everything.
-  g_vulkan_context = VulkanContext::Create(instance, gpu_list[selected_adapter_index], surface,
+  g_vulkan_context =
-                                           &g_Config, enable_validation_layer);
+      VulkanContext::Create(instance, gpu_list[selected_adapter_index], surface, &g_Config,
+                            enable_debug_reports, enable_validation_layer);
   if (!g_vulkan_context)
   {
     PanicAlert("Failed to create Vulkan device");