Reland "ExternalVkImageBacking: reuse more VkSemaphores."

This is a reland of 0b825660

Original change's description:
> ExternalVkImageBacking: reuse more VkSemaphores.
> 
> This CL change SharedImageRepresentationSkia::BeginScoped*Access()'s
> definition to not pass semaphores's ownership to caller. So the caller
> will not release any semaphores, so shared image backings can reuse or
> release them.
> 
> Bug: 1004772
> Change-Id: Idfbcf61571a56be6acc8c80ee838ff0e119ba8bf
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2323562
> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
> Commit-Queue: Peng Huang <penghuang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#793261}

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

components/viz/service/display_embedder/output_presenter.cc

@@ -64,8 +64,10 @@ void OutputPresenter::Image::BeginWriteSkia() {
       gpu::SharedImageRepresentation::AllowUnclearedAccess::kYes);
   DCHECK(scoped_skia_write_access_);
   if (!begin_semaphores.empty()) {
-    scoped_skia_write_access_->surface()->wait(begin_semaphores.size(),
-                                               begin_semaphores.data());
+    scoped_skia_write_access_->surface()->wait(
+        begin_semaphores.size(),
+        begin_semaphores.data(),
+        /*deleteSemaphoresAfterWait=*/false);
   }
 }
 

components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc

@@ -543,8 +543,9 @@ bool SkiaOutputSurfaceImplOnGpu::FinishPaintCurrentFrame(
     promise_image_access_helper_.BeginAccess(
         std::move(image_contexts), &begin_semaphores, &end_semaphores);
     if (!begin_semaphores.empty()) {
-      auto result = output_sk_surface()->wait(begin_semaphores.size(),
-                                              begin_semaphores.data());
+      auto result = output_sk_surface()->wait(
+          begin_semaphores.size(), begin_semaphores.data(),
+          /*deleteSemaphoresAfterWait=*/false);
       DCHECK(result);
     }
 
@@ -668,8 +669,9 @@ void SkiaOutputSurfaceImplOnGpu::FinishPaintRenderPass(
     promise_image_access_helper_.BeginAccess(
         std::move(image_contexts), &begin_semaphores, &end_semaphores);
     if (!begin_semaphores.empty()) {
-      auto result = offscreen.surface()->wait(begin_semaphores.size(),
-                                              begin_semaphores.data());
+      auto result = offscreen.surface()->wait(
+          begin_semaphores.size(), begin_semaphores.data(),
+          /*deleteSemaphoresAfterWait=*/false);
       DCHECK(result);
     }
     offscreen.surface()->draw(ddl);

gpu/command_buffer/service/external_semaphore.cc

@@ -199,10 +199,4 @@ VkSemaphore ExternalSemaphore::GetVkSemaphore() {
   return semaphore_;
 }
 
-VkSemaphore ExternalSemaphore::TakeVkSemaphore() {
-  VkSemaphore semaphore = GetVkSemaphore();
-  semaphore_ = VK_NULL_HANDLE;
-  return semaphore;
-}
-
 }  // namespace gpu

gpu/command_buffer/service/external_semaphore.h

@@ -45,10 +45,6 @@ class ExternalSemaphore {
   // Get a VkSemaphore. The ownership is not transferred to caller.
   VkSemaphore GetVkSemaphore();
 
-  // Take the VkSemaphore. The ownership is transferred to caller. The caller is
-  // responsible for releasing it.
-  VkSemaphore TakeVkSemaphore();
-
   bool is_valid() const { return context_provider_ && handle_.is_valid(); }
   SemaphoreHandle handle() { return handle_.Duplicate(); }
 

gpu/command_buffer/service/external_vk_image_skia_representation.cc

@@ -157,7 +157,7 @@ sk_sp<SkPromiseImageTexture> ExternalVkImageSkiaRepresentation::BeginAccess(
 
   for (auto& external_semaphore : begin_access_semaphores_) {
     DCHECK(external_semaphore);
-    VkSemaphore semaphore = external_semaphore.TakeVkSemaphore();
+    VkSemaphore semaphore = external_semaphore.GetVkSemaphore();
     DCHECK(semaphore != VK_NULL_HANDLE);
     // The ownership of semaphore is passed to caller.
     begin_semaphores->emplace_back();

gpu/command_buffer/service/raster_decoder.cc

@@ -260,7 +260,8 @@ class SharedImageProviderImpl final : public cc::SharedImageProvider {
 
     if (!begin_semaphores.empty()) {
       bool result = output_surface_->wait(begin_semaphores.size(),
-                                          begin_semaphores.data());
+                                          begin_semaphores.data(),
+                                          /*deleteSemaphoresAfterWait=*/false);
       DCHECK(result);
     }
 
@@ -2299,8 +2300,9 @@ void RasterDecoderImpl::DoCopySubTextureINTERNALSkia(
           &begin_semaphores, &end_semaphores);
 
   if (!begin_semaphores.empty()) {
-    bool result = dest_scoped_access->surface()->wait(begin_semaphores.size(),
-                                                      begin_semaphores.data());
+    bool result = dest_scoped_access->surface()->wait(
+        begin_semaphores.size(), begin_semaphores.data(),
+        /*deleteSemaphoresAfterWait=*/false);
     DCHECK(result);
   }
 
@@ -2422,8 +2424,9 @@ void RasterDecoderImpl::DoWritePixelsINTERNAL(GLint x_offset,
   }
 
   if (!begin_semaphores.empty()) {
-    bool result = dest_scoped_access->surface()->wait(begin_semaphores.size(),
-                                                      begin_semaphores.data());
+    bool result = dest_scoped_access->surface()->wait(
+        begin_semaphores.size(), begin_semaphores.data(),
+        /*deleteSemaphoresAfterWait=*/false);
     if (!result) {
       LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glWritePixels",
                          "Unable to obtain write access to dest shared image.");
@@ -2529,7 +2532,8 @@ void RasterDecoderImpl::DoReadbackImagePixelsINTERNAL(
 
   if (!begin_semaphores.empty()) {
     bool result = shared_context_state_->gr_context()->wait(
-        begin_semaphores.size(), begin_semaphores.data());
+        begin_semaphores.size(), begin_semaphores.data(),
+        /*deleteSemaphoresAfterWait=*/false);
     DCHECK(result);
   }
 
@@ -2694,7 +2698,8 @@ void RasterDecoderImpl::DoConvertYUVMailboxesToRGBINTERNAL(
   auto* dest_surface = dest_scoped_access->surface();
   if (!begin_semaphores.empty()) {
     bool result =
-        dest_surface->wait(begin_semaphores.size(), begin_semaphores.data());
+        dest_surface->wait(begin_semaphores.size(), begin_semaphores.data(),
+                           /*deleteSemaphoresAfterWait=*/false);
     DCHECK(result);
   }
 
@@ -2890,7 +2895,8 @@ void RasterDecoderImpl::DoBeginRasterCHROMIUM(GLuint sk_color,
 
   if (!begin_semaphores.empty()) {
     bool result =
-        sk_surface_->wait(begin_semaphores.size(), begin_semaphores.data());
+        sk_surface_->wait(begin_semaphores.size(), begin_semaphores.data(),
+                          /*deleteSemaphoresAfterWait=*/false);
     DCHECK(result);
   }
 

gpu/command_buffer/service/shared_image_representation.h

@@ -296,7 +296,7 @@ class GPU_GLES2_EXPORT SharedImageRepresentationSkia
  protected:
   // Begin the write access. The implementations should insert semaphores into
   // begin_semaphores vector which client will wait on before writing the
-  // backing. The ownership of begin_semaphores will be passed to client.
+  // backing. The ownership of begin_semaphores is not passed to client.
   // The implementations can also optionally insert semaphores into
   // end_semaphores. If using end_semaphores, the client must submit them with
   // drawing operations which use the backing. The ownership of end_semaphores
@@ -319,7 +319,7 @@ class GPU_GLES2_EXPORT SharedImageRepresentationSkia
 
   // Begin the read access. The implementations should insert semaphores into
   // begin_semaphores vector which client will wait on before reading the
-  // backing. The ownership of begin_semaphores will be passed to client.
+  // backing. The ownership of begin_semaphores is not passed to client.
   // The implementations can also optionally insert semaphores into
   // end_semaphores. If using end_semaphores, the client must submit them with
   // drawing operations which use the backing. The ownership of end_semaphores

gpu/command_buffer/service/shared_image_representation_skia_vk_android.cc

@@ -169,13 +169,13 @@ bool SharedImageRepresentationSkiaVkAndroid::BeginAccess(
   }
 
   sync_fd = gl::MergeFDs(std::move(sync_fd), std::move(init_read_fence));
-  VkSemaphore begin_access_semaphore = VK_NULL_HANDLE;
+  DCHECK(begin_access_semaphore_ == VK_NULL_HANDLE);
   if (sync_fd.is_valid()) {
-    begin_access_semaphore = vk_implementation()->ImportSemaphoreHandle(
+    begin_access_semaphore_ = vk_implementation()->ImportSemaphoreHandle(
         vk_device(),
         SemaphoreHandle(VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT,
                         std::move(sync_fd)));
-    if (begin_access_semaphore == VK_NULL_HANDLE) {
+    if (begin_access_semaphore_ == VK_NULL_HANDLE) {
       DLOG(ERROR) << "Failed to import semaphore from sync_fd.";
       return false;
     }
@@ -187,17 +187,18 @@ bool SharedImageRepresentationSkiaVkAndroid::BeginAccess(
 
     if (end_access_semaphore_ == VK_NULL_HANDLE) {
       DLOG(ERROR) << "Failed to create the external semaphore.";
-      if (begin_access_semaphore != VK_NULL_HANDLE) {
-        vkDestroySemaphore(vk_device(), begin_access_semaphore,
+      if (begin_access_semaphore_ != VK_NULL_HANDLE) {
+        vkDestroySemaphore(vk_device(), begin_access_semaphore_,
                            nullptr /* pAllocator */);
+        begin_access_semaphore_ = VK_NULL_HANDLE;
       }
       return false;
     }
   }
 
-  if (begin_access_semaphore != VK_NULL_HANDLE) {
+  if (begin_access_semaphore_ != VK_NULL_HANDLE) {
     begin_semaphores->emplace_back();
-    begin_semaphores->back().initVulkan(begin_access_semaphore);
+    begin_semaphores->back().initVulkan(begin_access_semaphore_);
   }
   if (end_semaphores) {
     end_semaphores->emplace_back();
@@ -224,14 +225,24 @@ void SharedImageRepresentationSkiaVkAndroid::EndAccess(bool readonly) {
     android_backing()->EndWrite(std::move(sync_fd));
   }
 
+  std::vector<VkSemaphore> semaphores;
+  semaphores.reserve(2);
+  if (begin_access_semaphore_ != VK_NULL_HANDLE) {
+    semaphores.emplace_back(begin_access_semaphore_);
+    begin_access_semaphore_ = VK_NULL_HANDLE;
+  }
   if (end_access_semaphore_ != VK_NULL_HANDLE) {
+    semaphores.emplace_back(end_access_semaphore_);
+    end_access_semaphore_ = VK_NULL_HANDLE;
+  }
+  if (!semaphores.empty()) {
     VulkanFenceHelper* fence_helper = context_state_->vk_context_provider()
                                           ->GetDeviceQueue()
                                           ->GetFenceHelper();
-    fence_helper->EnqueueSemaphoreCleanupForSubmittedWork(
-        end_access_semaphore_);
-    end_access_semaphore_ = VK_NULL_HANDLE;
+    fence_helper->EnqueueSemaphoresCleanupForSubmittedWork(
+        std::move(semaphores));
   }
+
   mode_ = RepresentationAccessMode::kNone;
 }
 

gpu/command_buffer/service/shared_image_representation_skia_vk_android.h

@@ -70,6 +70,7 @@ class SharedImageRepresentationSkiaVkAndroid
   int surface_msaa_count_ = 0;
   sk_sp<SkSurface> surface_;
   scoped_refptr<SharedContextState> context_state_;
+  VkSemaphore begin_access_semaphore_ = VK_NULL_HANDLE;
   VkSemaphore end_access_semaphore_ = VK_NULL_HANDLE;
 };