vulkan: minor changes

 * Change some DLOG(ERROR) to LOG(ERROR) to help diagnose vulkan
   issue.
 * Fix a semaphore leak when vkAcquireNextImage() is timeout.
 * Fix some typos

Bug: None
Change-Id: I7ed4c2e726b0e1bf42917ab8544fa65344e1ccab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2185931
Auto-Submit: Peng Huang <penghuang@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766569}

gpu/vulkan/vulkan_command_buffer.cc

@@ -53,7 +53,7 @@ VkAccessFlags GetAccessMask(const VkImageLayout layout) {
     case VK_IMAGE_LAYOUT_UNDEFINED:
       return 0;
     case VK_IMAGE_LAYOUT_GENERAL:
-      DLOG(WARNING) << "VK_IMAGE_LAYOUT_GENERAL is used.";
+      LOG(WARNING) << "VK_IMAGE_LAYOUT_GENERAL is used.";
       return VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT |
              VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT |
              VK_ACCESS_TRANSFER_WRITE_BIT | VK_ACCESS_TRANSFER_READ_BIT |
@@ -118,7 +118,7 @@ bool VulkanCommandBuffer::Initialize() {
   result =
       vkAllocateCommandBuffers(device, &command_buffer_info, &command_buffer_);
   if (VK_SUCCESS != result) {
-    DLOG(ERROR) << "vkAllocateCommandBuffers() failed: " << result;
+    LOG(ERROR) << "vkAllocateCommandBuffers() failed: " << result;
     return false;
   }
 
@@ -169,7 +169,7 @@ bool VulkanCommandBuffer::Submit(uint32_t num_wait_semaphores,
   VkFence fence;
   result = device_queue_->GetFenceHelper()->GetFence(&fence);
   if (VK_SUCCESS != result) {
-    DLOG(ERROR) << "Failed to create fence: " << result;
+    LOG(ERROR) << "Failed to create fence: " << result;
     return false;
   }
 
@@ -185,7 +185,7 @@ bool VulkanCommandBuffer::Submit(uint32_t num_wait_semaphores,
 
   PostExecution();
   if (VK_SUCCESS != result) {
-    DLOG(ERROR) << "vkQueueSubmit() failed: " << result;
+    LOG(ERROR) << "vkQueueSubmit() failed: " << result;
     return false;
   }
 
@@ -283,7 +283,7 @@ void VulkanCommandBuffer::ResetIfDirty() {
     Wait(UINT64_MAX);
     VkResult result = vkResetCommandBuffer(command_buffer_, 0);
     if (VK_SUCCESS != result) {
-      DLOG(ERROR) << "vkResetCommandBuffer() failed: " << result;
+      LOG(ERROR) << "vkResetCommandBuffer() failed: " << result;
     } else {
       record_type_ = RECORD_TYPE_EMPTY;
     }
@@ -293,7 +293,7 @@ void VulkanCommandBuffer::ResetIfDirty() {
 CommandBufferRecorderBase::~CommandBufferRecorderBase() {
   VkResult result = vkEndCommandBuffer(handle_);
   if (VK_SUCCESS != result) {
-    DLOG(ERROR) << "vkEndCommandBuffer() failed: " << result;
+    LOG(ERROR) << "vkEndCommandBuffer() failed: " << result;
   }
 }
 
@@ -306,7 +306,7 @@ ScopedMultiUseCommandBufferRecorder::ScopedMultiUseCommandBufferRecorder(
   VkResult result = vkBeginCommandBuffer(handle_, &begin_info);
 
   if (VK_SUCCESS != result) {
-    DLOG(ERROR) << "vkBeginCommandBuffer() failed: " << result;
+    LOG(ERROR) << "vkBeginCommandBuffer() failed: " << result;
   }
 }
 
@@ -320,7 +320,7 @@ ScopedSingleUseCommandBufferRecorder::ScopedSingleUseCommandBufferRecorder(
   VkResult result = vkBeginCommandBuffer(handle_, &begin_info);
 
   if (VK_SUCCESS != result) {
-    DLOG(ERROR) << "vkBeginCommandBuffer() failed: " << result;
+    LOG(ERROR) << "vkBeginCommandBuffer() failed: " << result;
   }
 }
 

gpu/vulkan/vulkan_swap_chain.cc

@@ -126,16 +126,18 @@ gfx::SwapResult VulkanSwapChain::PresentBuffer(const gfx::Rect& rect) {
 
   result = vkQueuePresentKHR(queue, &present_info);
   if (result != VK_SUCCESS && result != VK_SUBOPTIMAL_KHR) {
-    LOG(FATAL) << "vkQueuePresentKHR() failed: " << result;
+    LOG(DFATAL) << "vkQueuePresentKHR() failed: " << result;
     return gfx::SwapResult::SWAP_FAILED;
   }
+
+  current_image_data.is_acquired = false;
   LOG_IF(ERROR, result == VK_SUBOPTIMAL_KHR) << "Swapchian is suboptimal.";
 
   if (current_image_data.present_begin_semaphore != VK_NULL_HANDLE) {
     // |present_begin_semaphore| for the previous present for this image can be
     // safely destroyed after semaphore got from vkAcquireNextImageHKR() is
     // passed. That acquired semaphore should be already waited on for a
-    // submitted GPU work. So we can safely eunqueue the
+    // submitted GPU work. So we can safely enqueue the
     // |present_begin_semaphore| for cleanup here (the enqueued semaphore will
     // be destroyed when all submitted GPU work is finished).
     fence_helper->EnqueueSemaphoreCleanupForSubmittedWork(
@@ -326,18 +328,17 @@ bool VulkanSwapChain::AcquireNextImage() {
   DCHECK(!acquired_image_);
   VkDevice device = device_queue_->GetVulkanDevice();
   // The Vulkan spec doesn't require vkAcquireNextImageKHR() returns images in
-  // the present order for a vulkan swap chain. However for the best performnce,
-  // the driver should return images in order. To avoid buggy drivers, we will
-  // call vkAcquireNextImageKHR() continueslly until the expected image is
-  // returned.
+  // the present order for a vulkan swap chain. However for the best
+  // performance, the driver should return images in order. To avoid buggy
+  // drivers, we will call vkAcquireNextImageKHR() continually until the
+  // expected image is returned.
   do {
     bool all_images_are_tracked = in_present_images_.size() == images_.size();
     if (all_images_are_tracked) {
       // Only check the expected_next_image, when all images are tracked.
       uint32_t expected_next_image = in_present_images_.front();
       // If the expected next image has been acquired, use it and return true.
-      if (images_[expected_next_image].present_end_semaphore !=
-          VK_NULL_HANDLE) {
+      if (images_[expected_next_image].is_acquired) {
         in_present_images_.pop_front();
         acquired_image_.emplace(expected_next_image);
         break;
@@ -354,7 +355,7 @@ bool VulkanSwapChain::AcquireNextImage() {
     // FIFO swapchain will hang.
     // Workaround the issue by using the 2 seconds timeout for
     // vkAcquireNextImageKHR(). When timeout happens, we consider the swapchain
-    // hang hanppened, and then make the surface lost, so a new swapchain will
+    // hang happened, and then make the surface lost, so a new swapchain will
     // be recreated.
     constexpr uint64_t kTimeout = base::Time::kNanosecondsPerSecond * 2;
 #else
@@ -367,17 +368,20 @@ bool VulkanSwapChain::AcquireNextImage() {
                               VK_NULL_HANDLE, &next_image);
     if (result == VK_TIMEOUT) {
       LOG(ERROR) << "vkAcquireNextImageKHR() hangs.";
+      vkDestroySemaphore(device, vk_semaphore, nullptr /* pAllocator */);
       state_ = VK_ERROR_SURFACE_LOST_KHR;
       return false;
     }
     if (result != VK_SUCCESS && result != VK_SUBOPTIMAL_KHR) {
-      vkDestroySemaphore(device, vk_semaphore, nullptr /* pAllocator */);
       LOG(DFATAL) << "vkAcquireNextImageKHR() failed: " << result;
+      vkDestroySemaphore(device, vk_semaphore, nullptr /* pAllocator */);
       state_ = result;
       return false;
     }
 
+    DCHECK(!images_[next_image].is_acquired);
     DCHECK(images_[next_image].present_end_semaphore == VK_NULL_HANDLE);
+    images_[next_image].is_acquired = true;
     images_[next_image].present_end_semaphore = vk_semaphore;
 
     auto it = std::find(in_present_images_.begin(), in_present_images_.end(),

gpu/vulkan/vulkan_swap_chain.h

@@ -123,6 +123,9 @@ class COMPONENT_EXPORT(VULKAN) VulkanSwapChain {
     VkSemaphore present_begin_semaphore = VK_NULL_HANDLE;
     // Semaphore signaled when present engine is done with the image.
     VkSemaphore present_end_semaphore = VK_NULL_HANDLE;
+    // True indicates the image is acquired from swapchain and haven't sent back
+    // to swapchain for presenting.
+    bool is_acquired = false;
   };
   std::vector<ImageData> images_;