ExternalVkImage: tracking and synchronizing vk image layout between skia and chrome

Bug: 965955
Change-Id: Iccb9004cc519dfd493a532b9061d1745988e629e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1640369
Commit-Queue: Peng Huang <penghuang@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665901}

gpu/command_buffer/service/external_vk_image_backing.cc

@@ -14,7 +14,6 @@
 #include "components/viz/common/resources/resource_sizes.h"
 #include "gpu/command_buffer/service/external_vk_image_gl_representation.h"
 #include "gpu/command_buffer/service/external_vk_image_skia_representation.h"
-#include "gpu/command_buffer/service/mailbox_manager.h"
 #include "gpu/vulkan/vulkan_command_buffer.h"
 #include "gpu/vulkan/vulkan_command_pool.h"
 #include "gpu/vulkan/vulkan_fence_helper.h"
@@ -34,6 +33,16 @@ namespace gpu {
 
 namespace {
 
+GrVkImageInfo CreateGrVkImageInfo(VkImage image,
+                                  VkFormat vk_format,
+                                  VkDeviceMemory memory,
+                                  size_t memory_size) {
+  GrVkAlloc alloc(memory, 0 /* offset */, memory_size, 0 /* flags */);
+  return GrVkImageInfo(image, alloc, VK_IMAGE_TILING_OPTIMAL,
+                       VK_IMAGE_LAYOUT_UNDEFINED, vk_format,
+                       1 /* levelCount */);
+}
+
 VkResult CreateExternalVkImage(SharedContextState* context_state,
                                VkFormat format,
                                const gfx::Size& size,
@@ -73,28 +82,6 @@ VkResult CreateExternalVkImage(SharedContextState* context_state,
   return vkCreateImage(device, &create_info, nullptr, image);
 }
 
-void TransitionToColorAttachment(VkImage image,
-                                 SharedContextState* context_state,
-                                 VulkanCommandPool* command_pool) {
-  auto command_buffer = command_pool->CreatePrimaryCommandBuffer();
-  CHECK(command_buffer->Initialize());
-
-  {
-    ScopedSingleUseCommandBufferRecorder recorder(*command_buffer);
-    command_buffer->TransitionImageLayout(
-        image, VK_IMAGE_LAYOUT_UNDEFINED,
-        VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
-  }
-
-  // TODO(penghuang): get rid of this submission if poosible.
-  command_buffer->Submit(0, nullptr, 0, nullptr);
-
-  context_state->vk_context_provider()
-      ->GetDeviceQueue()
-      ->GetFenceHelper()
-      ->EnqueueVulkanObjectCleanupForSubmittedWork(std::move(command_buffer));
-}
-
 uint32_t FindMemoryTypeIndex(SharedContextState* context_state,
                              const VkMemoryRequirements& requirements,
                              VkMemoryPropertyFlags flags) {
@@ -184,10 +171,6 @@ std::unique_ptr<ExternalVkImageBacking> ExternalVkImageBacking::Create(
     return nullptr;
   }
 
-  // TODO(penghuang): track image layout to avoid unnecessary image layout
-  // transition. https://crbug.com/965955
-  TransitionToColorAttachment(image, context_state, command_pool);
-
   auto backing = base::WrapUnique(new ExternalVkImageBacking(
       mailbox, format, size, color_space, usage, context_state, image, memory,
       requirements.size, vk_format, command_pool));
@@ -245,10 +228,6 @@ std::unique_ptr<ExternalVkImageBacking> ExternalVkImageBacking::CreateFromGMB(
       return nullptr;
     }
 
-    // TODO(penghuang): track image layout to avoid unnecessary image layout
-    // transition. https://crbug.com/965955
-    TransitionToColorAttachment(vk_image, context_state, command_pool);
-
     return base::WrapUnique(new ExternalVkImageBacking(
         mailbox, viz::GetResourceFormat(buffer_format), size, color_space,
         usage, context_state, vk_image, vk_device_memory, memory_size,
@@ -355,15 +334,14 @@ ExternalVkImageBacking::ExternalVkImageBacking(
                          memory_size,
                          false /* is_thread_safe */),
       context_state_(context_state),
-      image_(image),
-      memory_(memory),
-      memory_size_(memory_size),
-      vk_format_(vk_format),
+      backend_texture_(
+          size.width(),
+          size.height(),
+          CreateGrVkImageInfo(image, vk_format, memory, memory_size)),
       command_pool_(command_pool) {}
 
 ExternalVkImageBacking::~ExternalVkImageBacking() {
-  DCHECK(image_ == VK_NULL_HANDLE);
-  DCHECK(memory_ == VK_NULL_HANDLE);
+  DCHECK(!backend_texture_.isValid());
 }
 
 bool ExternalVkImageBacking::BeginAccess(
@@ -404,13 +382,17 @@ void ExternalVkImageBacking::Update(std::unique_ptr<gfx::GpuFence> in_fence) {
 }
 
 void ExternalVkImageBacking::Destroy() {
+  GrVkImageInfo image_info;
+  bool result = backend_texture_.getVkImageInfo(&image_info);
+  DCHECK(result);
+
   auto* fence_helper = context_state()
                            ->vk_context_provider()
                            ->GetDeviceQueue()
                            ->GetFenceHelper();
-  fence_helper->EnqueueImageCleanupForSubmittedWork(image_, memory_);
-  image_ = VK_NULL_HANDLE;
-  memory_ = VK_NULL_HANDLE;
+  fence_helper->EnqueueImageCleanupForSubmittedWork(image_info.fImage,
+                                                    image_info.fAlloc.fMemory);
+  backend_texture_ = GrBackendTexture();
 
   if (texture_) {
     // Ensure that a context is current before removing the ref and calling
@@ -441,11 +423,14 @@ ExternalVkImageBacking::ProduceGLTexture(SharedImageManager* manager,
   NOTIMPLEMENTED_LOG_ONCE();
   return nullptr;
 #elif defined(OS_LINUX)
+  GrVkImageInfo image_info;
+  bool result = backend_texture_.getVkImageInfo(&image_info);
+  DCHECK(result);
   if (!texture_) {
     VkMemoryGetFdInfoKHR get_fd_info;
     get_fd_info.sType = VK_STRUCTURE_TYPE_MEMORY_GET_FD_INFO_KHR;
     get_fd_info.pNext = nullptr;
-    get_fd_info.memory = memory_;
+    get_fd_info.memory = image_info.fAlloc.fMemory;
     get_fd_info.handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR;
 
     int memory_fd = -1;
@@ -464,7 +449,7 @@ ExternalVkImageBacking::ProduceGLTexture(SharedImageManager* manager,
 
     GLuint memory_object;
     api->glCreateMemoryObjectsEXTFn(1, &memory_object);
-    api->glImportMemoryFdEXTFn(memory_object, memory_size_,
+    api->glImportMemoryFdEXTFn(memory_object, image_info.fAlloc.fSize,
                                GL_HANDLE_TYPE_OPAQUE_FD_EXT, memory_fd);
     GLuint texture_service_id;
     api->glGenTexturesFn(1, &texture_service_id);
@@ -603,30 +588,6 @@ bool ExternalVkImageBacking::WritePixels(
   memcpy(data, pixel_data.data(), pixel_data.size());
   vkUnmapMemory(device(), stage_memory);
 
-  auto command_buffer = command_pool_->CreatePrimaryCommandBuffer();
-  CHECK(command_buffer->Initialize());
-
-  {
-    ScopedSingleUseCommandBufferRecorder recorder(*command_buffer);
-
-    // TODO(penghuang): track image layout to avoid unnecessary image layout
-    // transition. https://crbug.com/965955
-    command_buffer->TransitionImageLayout(
-        image(), VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
-        VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
-    uint32_t buffer_width =
-        stride ? stride * 8 / BitsPerPixel(format()) : size().width();
-    command_buffer->CopyBufferToImage(stage_buffer, image(), buffer_width,
-                                      size().height(), size().width(),
-                                      size().height());
-
-    // TODO(penghuang): track image layout to avoid unnecessary image layout
-    // transition. https://crbug.com/965955
-    command_buffer->TransitionImageLayout(
-        image(), VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
-        VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL);
-  }
-
   std::vector<gpu::SemaphoreHandle> handles;
   if (!BeginAccessInternal(false /* readonly */, &handles)) {
     DLOG(ERROR) << "BeginAccess() failed.";
@@ -635,6 +596,26 @@ bool ExternalVkImageBacking::WritePixels(
     return false;
   }
 
+  auto command_buffer = command_pool_->CreatePrimaryCommandBuffer();
+  CHECK(command_buffer->Initialize());
+  {
+    ScopedSingleUseCommandBufferRecorder recorder(*command_buffer);
+    GrVkImageInfo image_info;
+    bool result = backend_texture_.getVkImageInfo(&image_info);
+    DCHECK(result);
+    if (image_info.fImageLayout != VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL) {
+      command_buffer->TransitionImageLayout(
+          image_info.fImage, image_info.fImageLayout,
+          VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
+      backend_texture_.setVkImageLayout(VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);
+    }
+    uint32_t buffer_width =
+        stride ? stride * 8 / BitsPerPixel(format()) : size().width();
+    command_buffer->CopyBufferToImage(stage_buffer, image_info.fImage,
+                                      buffer_width, size().height(),
+                                      size().width(), size().height());
+  }
+
   if (!need_sychronization()) {
     DCHECK(handles.empty());
     command_buffer->Submit(0, nullptr, 0, nullptr);

gpu/command_buffer/service/external_vk_image_backing.h

@@ -48,11 +48,8 @@ class ExternalVkImageBacking : public SharedImageBacking {
 
   ~ExternalVkImageBacking() override;
 
-  VkImage image() const { return image_; }
-  VkDeviceMemory memory() const { return memory_; }
-  size_t memory_size() const { return memory_size_; }
-  VkFormat vk_format() const { return vk_format_; }
   SharedContextState* context_state() const { return context_state_; }
+  const GrBackendTexture& backend_texture() const { return backend_texture_; }
   VulkanImplementation* vulkan_implementation() const {
     return context_state()->vk_context_provider()->GetVulkanImplementation();
   }
@@ -123,14 +120,12 @@ class ExternalVkImageBacking : public SharedImageBacking {
   bool WritePixels(const base::span<const uint8_t>& pixel_data, size_t stride);
 
   SharedContextState* const context_state_;
-  VkImage image_ = VK_NULL_HANDLE;
-  VkDeviceMemory memory_ = VK_NULL_HANDLE;
+  GrBackendTexture backend_texture_;
+  VulkanCommandPool* const command_pool_;
+
   SemaphoreHandle write_semaphore_handle_;
   std::vector<SemaphoreHandle> read_semaphore_handles_;
-  const size_t memory_size_;
   bool is_cleared_ = false;
-  const VkFormat vk_format_;
-  VulkanCommandPool* const command_pool_;
 
   bool is_write_in_progress_ = false;
   uint32_t reads_in_progress_ = 0;

gpu/command_buffer/service/external_vk_image_skia_representation.cc

@@ -115,16 +115,7 @@ sk_sp<SkPromiseImageTexture> ExternalVkImageSkiaRepresentation::BeginAccess(
     end_semaphores->back().initVulkan(end_access_semaphore_);
   }
 
-  // Create backend texture from the VkImage.
-  GrVkAlloc alloc(backing_impl()->memory(), 0 /* offset */,
-                  backing_impl()->memory_size(), 0 /* flags */);
-  GrVkImageInfo vk_image_info(backing_impl()->image(), alloc,
-                              VK_IMAGE_TILING_OPTIMAL,
-                              VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
-                              backing_impl()->vk_format(), 1 /* levelCount */);
-
-  return SkPromiseImageTexture::Make(
-      GrBackendTexture(size().width(), size().height(), vk_image_info));
+  return SkPromiseImageTexture::Make(backing_impl()->backend_texture());
 }
 
 void ExternalVkImageSkiaRepresentation::EndAccess(bool readonly) {