VulkanDevice: Refactor present failure handling

Shouldn't deadlock anymore...

src/util/vulkan_device.cpp

@@ -1266,6 +1266,16 @@ void VulkanDevice::WaitForFenceCounter(u64 fence_counter)
   WaitForCommandBufferCompletion(index);
 }
 
+void VulkanDevice::WaitForAllFences()
+{
+  u32 index = (m_current_frame + 1) % NUM_COMMAND_BUFFERS;
+  for (u32 i = 0; i < (NUM_COMMAND_BUFFERS - 1); i++)
+  {
+    WaitForCommandBufferCompletion(index);
+    index = (index + 1) % NUM_COMMAND_BUFFERS;
+  }
+}
+
 float VulkanDevice::GetAndResetAccumulatedGPUTime()
 {
   const float time = m_accumulated_gpu_time;
@@ -1418,6 +1428,8 @@ void VulkanDevice::EndAndSubmitCommandBuffer(VulkanSwapChain* present_swap_chain
     return;
   }
 
+  BeginCommandBuffer((m_current_frame + 1) % NUM_COMMAND_BUFFERS);
+
   if (present_swap_chain && !explicit_present)
     QueuePresent(present_swap_chain);
 }
@@ -1436,32 +1448,20 @@ void VulkanDevice::QueuePresent(VulkanSwapChain* present_swap_chain)
   present_swap_chain->ResetImageAcquireResult();
 
   const VkResult res = vkQueuePresentKHR(m_present_queue, &present_info);
-  if (res != VK_SUCCESS && res != VK_SUBOPTIMAL_KHR)
+  if (res != VK_SUCCESS)
   {
-    // VK_ERROR_OUT_OF_DATE_KHR is not fatal, just means we need to recreate our swap chain.
+    VkResult handled_res = res;
-    if (res == VK_ERROR_OUT_OF_DATE_KHR)
+    if (!present_swap_chain->HandleAcquireOrPresentError(handled_res, true))
-    {
-      Error error;
-      if (!present_swap_chain->ResizeBuffers(0, 0, present_swap_chain->GetScale(), &error)) [[unlikely]]
-        WARNING_LOG("Failed to reszie swap chain: {}", error.GetDescription());
-    }
-    else
     {
       LOG_VULKAN_ERROR(res, "vkQueuePresentKHR failed: ");
+      return;
     }
-
-    return;
   }
 
   // Grab the next image as soon as possible, that way we spend less time blocked on the next
   // submission. Don't care if it fails, we'll deal with that at the presentation call site.
   // Credit to dxvk for the idea.
-  present_swap_chain->AcquireNextImage();
+  present_swap_chain->AcquireNextImage(false);
-}
-
-void VulkanDevice::MoveToNextCommandBuffer()
-{
-  BeginCommandBuffer((m_current_frame + 1) % NUM_COMMAND_BUFFERS);
 }
 
 void VulkanDevice::BeginCommandBuffer(u32 index)
@@ -1521,7 +1521,6 @@ void VulkanDevice::SubmitCommandBuffer(bool wait_for_completion)
 
   const u32 current_frame = m_current_frame;
   EndAndSubmitCommandBuffer(nullptr, false);
-  MoveToNextCommandBuffer();
 
   if (wait_for_completion)
     WaitForCommandBufferCompletion(current_frame);
@@ -2281,40 +2280,16 @@ GPUDevice::PresentResult VulkanDevice::BeginPresent(GPUSwapChain* swap_chain, u3
     return PresentResult::DeviceLost;
 
   VulkanSwapChain* const SC = static_cast<VulkanSwapChain*>(swap_chain);
-  VkResult res = SC->AcquireNextImage();
+  VkResult res = SC->AcquireNextImage(true);
-  if (res != VK_SUCCESS)
-  {
-    LOG_VULKAN_ERROR(res, "vkAcquireNextImageKHR() failed: ");
-    SC->ReleaseCurrentImage();
-
-    if (res == VK_SUBOPTIMAL_KHR || res == VK_ERROR_OUT_OF_DATE_KHR)
-    {
-      Error error;
-      if (!SC->ResizeBuffers(0, 0, SC->GetScale(), &error)) [[unlikely]]
-        WARNING_LOG("Failed to resize buffers: {}", error.GetDescription());
-      else
-        res = SC->AcquireNextImage();
-    }
-    else if (res == VK_ERROR_SURFACE_LOST_KHR)
-    {
-      WARNING_LOG("Surface lost, attempting to recreate");
-
-      Error error;
-      if (!SC->RecreateSurface(&error))
-        ERROR_LOG("Failed to recreate surface after loss: {}", error.GetDescription());
-      else
-        res = SC->AcquireNextImage();
-    }
 
-    // This can happen when multiple resize events happen in quick succession.
+  // This can happen when multiple resize events happen in quick succession.
-    // In this case, just wait until the next frame to try again.
+  // In this case, just wait until the next frame to try again.
-    if (res != VK_SUCCESS && res != VK_SUBOPTIMAL_KHR)
+  if (res != VK_SUCCESS && res != VK_SUBOPTIMAL_KHR)
-    {
+  {
-      // Still submit the command buffer, otherwise we'll end up with several frames waiting.
+    // Still submit the command buffer, otherwise we'll end up with several frames waiting.
-      SubmitCommandBuffer(false);
+    SubmitCommandBuffer(false);
-      TrimTexturePool();
+    TrimTexturePool();
-      return PresentResult::SkipPresent;
+    return PresentResult::SkipPresent;
-    }
   }
 
   BeginSwapChainRenderPass(SC, clear_color);
@@ -2337,7 +2312,6 @@ void VulkanDevice::EndPresent(GPUSwapChain* swap_chain, bool explicit_present, u
                                                 1, VulkanTexture::Layout::ColorAttachment,
                                                 VulkanTexture::Layout::PresentSrc);
   EndAndSubmitCommandBuffer(SC, explicit_present);
-  MoveToNextCommandBuffer();
   InvalidateCachedState();
   TrimTexturePool();
 }
@@ -2999,7 +2973,7 @@ void VulkanDevice::DestroyPersistentDescriptorSets()
 
 void VulkanDevice::RenderBlankFrame(VulkanSwapChain* swap_chain)
 {
-  VkResult res = swap_chain->AcquireNextImage();
+  VkResult res = swap_chain->AcquireNextImage(true);
   if (res != VK_SUCCESS)
   {
     ERROR_LOG("Failed to acquire image for blank frame present");
@@ -3018,7 +2992,6 @@ void VulkanDevice::RenderBlankFrame(VulkanSwapChain* swap_chain)
                                                 VulkanTexture::Layout::TransferDst, VulkanTexture::Layout::PresentSrc);
 
   EndAndSubmitCommandBuffer(swap_chain, false);
-  MoveToNextCommandBuffer();
 
   InvalidateCachedState();
 }

src/util/vulkan_device.h

@@ -389,7 +389,6 @@ private:
   void BeginCommandBuffer(u32 index);
   void WaitForCommandBufferCompletion(u32 index);
   void EndAndSubmitCommandBuffer(VulkanSwapChain* present_swap_chain, bool explicit_present);
-  void MoveToNextCommandBuffer();
   void QueuePresent(VulkanSwapChain* present_swap_chain);
 
   VkInstance m_instance = VK_NULL_HANDLE;

src/util/vulkan_swap_chain.cpp

@@ -631,10 +631,16 @@ void VulkanSwapChain::DestroySwapChain()
   }
 }
 
-VkResult VulkanSwapChain::AcquireNextImage()
+VkResult VulkanSwapChain::AcquireNextImage(bool handle_errors)
 {
   if (m_image_acquire_result.has_value())
-    return m_image_acquire_result.value();
+  {
+    if (m_image_acquire_result.value() == VK_SUCCESS || !handle_errors ||
+        !HandleAcquireOrPresentError(m_image_acquire_result.value(), false))
+    {
+      return m_image_acquire_result.value();
+    }
+  }
 
   if (!m_swap_chain)
     return VK_ERROR_SURFACE_LOST_KHR;
@@ -642,13 +648,67 @@ VkResult VulkanSwapChain::AcquireNextImage()
   // Use a different semaphore for each image.
   m_current_semaphore = (m_current_semaphore + 1) % static_cast<u32>(m_semaphores.size());
 
-  const VkResult res =
+  VkResult res =
     vkAcquireNextImageKHR(VulkanDevice::GetInstance().GetVulkanDevice(), m_swap_chain, UINT64_MAX,
                           m_semaphores[m_current_semaphore].available_semaphore, VK_NULL_HANDLE, &m_current_image);
+  if (res != VK_SUCCESS && HandleAcquireOrPresentError(res, false))
+  {
+    res =
+      vkAcquireNextImageKHR(VulkanDevice::GetInstance().GetVulkanDevice(), m_swap_chain, UINT64_MAX,
+                            m_semaphores[m_current_semaphore].available_semaphore, VK_NULL_HANDLE, &m_current_image);
+  }
+
+  if (res != VK_SUCCESS)
+    LOG_VULKAN_ERROR(res, "vkAcquireNextImageKHR() failed: ");
+
   m_image_acquire_result = res;
   return res;
 }
 
+bool VulkanSwapChain::HandleAcquireOrPresentError(VkResult& res, bool is_present_error)
+{
+  if (res == VK_SUBOPTIMAL_KHR || res == VK_ERROR_OUT_OF_DATE_KHR)
+  {
+    VulkanDevice& dev = VulkanDevice::GetInstance();
+    if (is_present_error)
+      dev.WaitForAllFences();
+    else
+      dev.SubmitCommandBuffer(true);
+
+    Error error;
+    if (!RecreateSwapChain(dev, &error))
+    {
+      DestroySwapChain();
+      ERROR_LOG("Failed to recreate suboptimal swapchain: {}", error.GetDescription());
+      res = VK_ERROR_SURFACE_LOST_KHR;
+      return false;
+    }
+
+    return true;
+  }
+  else if (res == VK_ERROR_SURFACE_LOST_KHR)
+  {
+    VulkanDevice& dev = VulkanDevice::GetInstance();
+    if (is_present_error)
+      dev.WaitForAllFences();
+    else
+      dev.SubmitCommandBuffer(true);
+
+    Error error;
+    if (!RecreateSurface(dev, &error))
+    {
+      DestroySwapChain();
+      ERROR_LOG("Failed to recreate surface: {}", error.GetDescription());
+      res = VK_ERROR_SURFACE_LOST_KHR;
+      return false;
+    }
+
+    return true;
+  }
+
+  return false;
+}
+
 void VulkanSwapChain::ReleaseCurrentImage()
 {
   if (!m_image_acquire_result.has_value())
@@ -687,16 +747,52 @@ bool VulkanSwapChain::ResizeBuffers(u32 new_width, u32 new_height, float new_sca
     dev.EndRenderPass();
   dev.SubmitCommandBuffer(true);
 
-  ReleaseCurrentImage();
-  DestroySwapChainImages();
-
   if (new_width != 0 && new_height != 0)
   {
     m_window_info.surface_width = static_cast<u16>(new_width);
     m_window_info.surface_height = static_cast<u16>(new_height);
   }
 
-  if (!CreateSwapChain(VulkanDevice::GetInstance(), error) || !CreateSwapChainImages(dev, error))
+  return RecreateSwapChain(dev, error);
+}
+
+bool VulkanSwapChain::RecreateSurface(VulkanDevice& dev, Error* error)
+{
+  // Destroy the old swap chain, images, and surface.
+  DestroySwapChain();
+  DestroySurface();
+
+  // Re-create the surface with the new native handle
+  if (!CreateSurface(dev.GetVulkanInstance(), dev.GetVulkanPhysicalDevice(), error))
+    return false;
+
+  // The validation layers get angry at us if we don't call this before creating the swapchain.
+  VkBool32 present_supported = VK_TRUE;
+  VkResult res = vkGetPhysicalDeviceSurfaceSupportKHR(dev.GetVulkanPhysicalDevice(), dev.GetPresentQueueFamilyIndex(),
+                                                      m_surface, &present_supported);
+  if (res != VK_SUCCESS)
+  {
+    Vulkan::SetErrorObject(error, "vkGetPhysicalDeviceSurfaceSupportKHR failed: ", res);
+    return false;
+  }
+  AssertMsg(present_supported, "Recreated surface does not support presenting.");
+
+  // Finally re-create the swap chain
+  if (!CreateSwapChain(dev, error) || !CreateSwapChainImages(dev, error))
+  {
+    DestroySwapChain();
+    return false;
+  }
+
+  return true;
+}
+
+bool VulkanSwapChain::RecreateSwapChain(VulkanDevice& dev, Error* error)
+{
+  ReleaseCurrentImage();
+  DestroySwapChainImages();
+
+  if (!CreateSwapChain(dev, error) || !CreateSwapChainImages(dev, error))
   {
     DestroySwapChain();
     return false;
@@ -737,39 +833,3 @@ bool VulkanSwapChain::SetVSyncMode(GPUVSyncMode mode, bool allow_present_throttl
 
   return true;
 }
-
-bool VulkanSwapChain::RecreateSurface(Error* error)
-{
-  VulkanDevice& dev = VulkanDevice::GetInstance();
-  if (dev.InRenderPass())
-    dev.EndRenderPass();
-  dev.SubmitCommandBuffer(true);
-
-  // Destroy the old swap chain, images, and surface.
-  DestroySwapChain();
-  DestroySurface();
-
-  // Re-create the surface with the new native handle
-  if (!CreateSurface(dev.GetVulkanInstance(), dev.GetVulkanPhysicalDevice(), error))
-    return false;
-
-  // The validation layers get angry at us if we don't call this before creating the swapchain.
-  VkBool32 present_supported = VK_TRUE;
-  VkResult res = vkGetPhysicalDeviceSurfaceSupportKHR(dev.GetVulkanPhysicalDevice(), dev.GetPresentQueueFamilyIndex(),
-                                                      m_surface, &present_supported);
-  if (res != VK_SUCCESS)
-  {
-    Vulkan::SetErrorObject(error, "vkGetPhysicalDeviceSurfaceSupportKHR failed: ", res);
-    return false;
-  }
-  AssertMsg(present_supported, "Recreated surface does not support presenting.");
-
-  // Finally re-create the swap chain
-  if (!CreateSwapChain(dev, error) || !CreateSwapChainImages(dev, error))
-  {
-    DestroySwapChain();
-    return false;
-  }
-
-  return true;
-}

src/util/vulkan_swap_chain.h

@@ -53,11 +53,10 @@ public:
   bool CreateSwapChainImages(VulkanDevice& dev, Error* error);
   void Destroy(VulkanDevice& dev, bool wait_for_idle);
 
-  VkResult AcquireNextImage();
+  VkResult AcquireNextImage(bool handle_errors);
   void ReleaseCurrentImage();
   void ResetImageAcquireResult();
-
+  bool HandleAcquireOrPresentError(VkResult& res, bool is_present_error);
-  bool RecreateSurface(Error* error);
 
   bool ResizeBuffers(u32 new_width, u32 new_height, float new_scale, Error* error) override;
   bool SetVSyncMode(GPUVSyncMode mode, bool allow_present_throttle, Error* error) override;
@@ -75,6 +74,10 @@ private:
   void DestroySwapChain();
   void DestroySurface();
 
+  // Assumes the command buffer has been flushed.
+  bool RecreateSurface(VulkanDevice& dev, Error* error);
+  bool RecreateSwapChain(VulkanDevice& dev, Error* error);
+
   struct Image
   {
     VkImage image;