vulkan: fix layout and queue family problems with ExternalVkImageBacking

1. For the first time GL access, the backing's queue family could be not
   VK_QUEUE_FAMILY_EXTERNAL. We need to transfer it to external for GL
   access.
2. SkiaOutputDeviceBufferQueue doesn't set backing's layout and queue
   family after Skia write access. Fix it by override
   PreGrContextSubmitz() to set the backing's layout and queue family.

Bug: 1101167
Change-Id: I2f2f527ffb6f4b56e0042c8762cdb93eea4b6942
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2286052Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786453}

components/viz/service/display_embedder/output_presenter.cc

@@ -4,6 +4,8 @@
 
 #include "components/viz/service/display_embedder/output_presenter.h"
 
+#include <utility>
+
 #include "components/viz/service/display_embedder/skia_output_surface_dependency.h"
 #include "gpu/command_buffer/service/shared_context_state.h"
 #include "third_party/skia/include/gpu/GrBackendSemaphore.h"
@@ -99,6 +101,14 @@ void OutputPresenter::Image::EndWriteSkia() {
   skia_representation_->SetCleared();
 }
 
+void OutputPresenter::Image::PreGrContextSubmit() {
+  DCHECK(scoped_skia_write_access_);
+  if (scoped_skia_write_access_->end_state()) {
+    scoped_skia_write_access_->surface()->flush(
+        {}, scoped_skia_write_access_->end_state());
+  }
+}
+
 OutputPresenter::OverlayData::OverlayData(
     std::unique_ptr<gpu::SharedImageRepresentationOverlay> representation,
     std::unique_ptr<gpu::SharedImageRepresentationOverlay::ScopedReadAccess>

components/viz/service/display_embedder/output_presenter.h

@@ -44,6 +44,7 @@ class VIZ_SERVICE_EXPORT OutputPresenter {
     SkSurface* sk_surface();
     std::vector<GrBackendSemaphore> TakeEndWriteSkiaSemaphores();
     void EndWriteSkia();
+    void PreGrContextSubmit();
 
     virtual void BeginPresent() = 0;
     virtual void EndPresent() = 0;

components/viz/service/display_embedder/skia_output_device_buffer_queue.cc

@@ -120,6 +120,11 @@ void SkiaOutputDeviceBufferQueue::ScheduleOverlays(
   pending_overlays_ = presenter_->ScheduleOverlays(std::move(overlays));
 }
 
+void SkiaOutputDeviceBufferQueue::PreGrContextSubmit() {
+  DCHECK(current_image_);
+  current_image_->PreGrContextSubmit();
+}
+
 void SkiaOutputDeviceBufferQueue::SwapBuffers(
     BufferPresentedCallback feedback,
     std::vector<ui::LatencyInfo> latency_info) {

components/viz/service/display_embedder/skia_output_device_buffer_queue.h

@@ -34,6 +34,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputDeviceBufferQueue : public SkiaOutputDevice {
       delete;
 
   // SkiaOutputDevice overrides.
+  void PreGrContextSubmit() override;
   void SwapBuffers(BufferPresentedCallback feedback,
                    std::vector<ui::LatencyInfo> latency_info) override;
   void PostSubBuffer(const gfx::Rect& rect,

gpu/command_buffer/service/external_vk_image_backing.cc

@@ -290,11 +290,7 @@ ExternalVkImageBacking::~ExternalVkImageBacking() {
   bool result = backend_texture_.getVkImageInfo(&image_info);
   DCHECK(result);
 
-  auto* fence_helper = context_state()
-                           ->vk_context_provider()
-                           ->GetDeviceQueue()
-                           ->GetFenceHelper();
-  fence_helper->EnqueueVulkanObjectCleanupForSubmittedWork(std::move(image_));
+  fence_helper()->EnqueueVulkanObjectCleanupForSubmittedWork(std::move(image_));
   backend_texture_ = GrBackendTexture();
 
   if (texture_) {
@@ -330,7 +326,7 @@ bool ExternalVkImageBacking::BeginAccess(
 
   if (readonly && !reads_in_progress_) {
     UpdateContent(kInVkImage);
-    if (texture_)
+    if (texture_ || texture_passthrough_)
       UpdateContent(kInGLTexture);
   }
 
@@ -340,11 +336,16 @@ bool ExternalVkImageBacking::BeginAccess(
   if (!is_gl)
     return true;
 
-  if (need_synchronization() && semaphore_handles->empty() && IsCleared()) {
-    DLOG(ERROR) << "semaphore_handles is empty.";
+  if (need_synchronization() && semaphore_handles->empty()) {
     // For the first time GL BeginAccess(), semaphore_handles could be empty,
     // since the Vulkan usage will not provide semaphore for EndAccess() call,
-    // if ProduceGL*() is never called.
+    // if ProduceGL*() is never called. In this case, image layout and queue
+    // family will not be ready for GL access as well.
+    auto* gr_context = context_state()->gr_context();
+    gr_context->setBackendTextureState(
+        backend_texture_,
+        GrBackendSurfaceMutableState(VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
+                                     VK_QUEUE_FAMILY_EXTERNAL));
     VkSemaphore semaphore =
         vulkan_implementation()->CreateExternalSemaphore(device());
     GrBackendSemaphore backend_semaphore;
@@ -355,21 +356,16 @@ bool ExternalVkImageBacking::BeginAccess(
     };
     gpu::AddVulkanCleanupTaskForSkiaFlush(
         context_state()->vk_context_provider(), &flush_info);
-    auto* gr_context = context_state()->gr_context();
     auto flush_result = gr_context->flush(flush_info);
     DCHECK_EQ(flush_result, GrSemaphoresSubmitted::kYes);
     gr_context->submit();
 
     auto handle =
         vulkan_implementation()->GetSemaphoreHandle(device(), semaphore);
-    semaphore_handles->push_back(std::move(handle));
     DCHECK(handle.is_valid());
+    semaphore_handles->push_back(std::move(handle));
     // We're done with the semaphore, enqueue deferred cleanup.
-    context_state()
-        ->vk_context_provider()
-        ->GetDeviceQueue()
-        ->GetFenceHelper()
-        ->EnqueueSemaphoreCleanupForSubmittedWork(semaphore);
+    fence_helper()->EnqueueSemaphoreCleanupForSubmittedWork(semaphore);
   }
 
   if (readonly) {
@@ -722,13 +718,10 @@ bool ExternalVkImageBacking::WritePixelsWithCallback(
     command_buffer->Submit(0, nullptr, 0, nullptr);
     EndAccessInternal(false /* readonly */, SemaphoreHandle());
 
-    auto* fence_helper = context_state_->vk_context_provider()
-                             ->GetDeviceQueue()
-                             ->GetFenceHelper();
-    fence_helper->EnqueueVulkanObjectCleanupForSubmittedWork(
+    fence_helper()->EnqueueVulkanObjectCleanupForSubmittedWork(
         std::move(command_buffer));
-    fence_helper->EnqueueBufferCleanupForSubmittedWork(stage_buffer,
-                                                       stage_allocation);
+    fence_helper()->EnqueueBufferCleanupForSubmittedWork(stage_buffer,
+                                                         stage_allocation);
     return true;
   }
 
@@ -752,15 +745,13 @@ bool ExternalVkImageBacking::WritePixelsWithCallback(
   EndAccessInternal(false /* readonly */,
                     std::move(end_access_semaphore_handle));
 
-  auto* fence_helper =
-      context_state_->vk_context_provider()->GetDeviceQueue()->GetFenceHelper();
-  fence_helper->EnqueueVulkanObjectCleanupForSubmittedWork(
+  fence_helper()->EnqueueVulkanObjectCleanupForSubmittedWork(
       std::move(command_buffer));
   begin_access_semaphores.emplace_back(end_access_semaphore);
-  fence_helper->EnqueueSemaphoresCleanupForSubmittedWork(
+  fence_helper()->EnqueueSemaphoresCleanupForSubmittedWork(
       begin_access_semaphores);
-  fence_helper->EnqueueBufferCleanupForSubmittedWork(stage_buffer,
-                                                     stage_allocation);
+  fence_helper()->EnqueueBufferCleanupForSubmittedWork(stage_buffer,
+                                                       stage_allocation);
   return true;
 }
 

gpu/command_buffer/service/external_vk_image_backing.h

@@ -79,6 +79,12 @@ class ExternalVkImageBacking final : public ClearTrackingSharedImageBacking {
   VulkanImplementation* vulkan_implementation() const {
     return context_state()->vk_context_provider()->GetVulkanImplementation();
   }
+  VulkanFenceHelper* fence_helper() const {
+    return context_state()
+        ->vk_context_provider()
+        ->GetDeviceQueue()
+        ->GetFenceHelper();
+  }
   VkDevice device() const {
     return context_state()
         ->vk_context_provider()

gpu/command_buffer/service/external_vk_image_gl_representation.cc

@@ -92,6 +92,9 @@ bool ExternalVkImageGLRepresentationShared::BeginAccess(GLenum mode) {
       GrVkImageInfo info;
       auto result = backing_impl()->backend_texture().getVkImageInfo(&info);
       DCHECK(result);
+      DCHECK_EQ(info.fCurrentQueueFamily, VK_QUEUE_FAMILY_EXTERNAL);
+      DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_UNDEFINED);
+      DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_PREINITIALIZED);
       GLenum src_layout = ToGLImageLayout(info.fImageLayout);
       api()->glWaitSemaphoreEXTFn(gl_semaphore, 0, nullptr, 1,
                                   &texture_service_id_, &src_layout);
@@ -160,6 +163,9 @@ void ExternalVkImageGLRepresentationShared::EndAccess() {
     GrVkImageInfo info;
     auto result = backing_impl()->backend_texture().getVkImageInfo(&info);
     DCHECK(result);
+    DCHECK_EQ(info.fCurrentQueueFamily, VK_QUEUE_FAMILY_EXTERNAL);
+    DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_UNDEFINED);
+    DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_PREINITIALIZED);
     GLenum dst_layout = ToGLImageLayout(info.fImageLayout);
     api()->glSignalSemaphoreEXTFn(gl_semaphore, 0, nullptr, 1,
                                   &texture_service_id_, &dst_layout);

gpu/command_buffer/service/external_vk_image_skia_representation.cc

@@ -81,10 +81,10 @@ sk_sp<SkSurface> ExternalVkImageSkiaRepresentation::BeginWriteAccess(
 
   access_mode_ = kWrite;
 
-  // If Vulkan and GLSet share the same memory backing, we need set |end_state|
-  // VK_QUEUE_FAMILY_EXTERNAL, and then the caller will set the VkImage to
-  // VK_QUEUE_FAMILY_EXTERNAL before calling EndAccess().
-  if (!backing_impl()->use_separate_gl_texture()) {
+  // If Vulkan and GL (or Dawn) share the same memory backing, we need set
+  // |end_state| VK_QUEUE_FAMILY_EXTERNAL, and then the caller will set the
+  // VkImage to VK_QUEUE_FAMILY_EXTERNAL before calling EndAccess().
+  if (backing_impl()->need_synchronization()) {
     *end_state = std::make_unique<GrBackendSurfaceMutableState>(
         VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, VK_QUEUE_FAMILY_EXTERNAL);
   }