[Impeller] Change texture upload pipeline in Vulkan

Upload to staging buffer and then copy the buffer to texture, this
ensure that tiling is respected and the image is shown as intended,
without this change the image would be shown as random chunks.

Author: iskakaushik

impeller/renderer/backend/vulkan/allocator_vk.cc

@@ -114,7 +114,8 @@ std::shared_ptr<Texture> AllocatorVK::OnCreateTexture(
   image_create_info.tiling = vk::ImageTiling::eOptimal;
   image_create_info.initialLayout = vk::ImageLayout::eUndefined;
   image_create_info.usage = vk::ImageUsageFlagBits::eSampled |
-                            vk::ImageUsageFlagBits::eColorAttachment;
+                            vk::ImageUsageFlagBits::eColorAttachment |
+                            vk::ImageUsageFlagBits::eTransferDst;
 
   VmaAllocationCreateInfo alloc_create_info = {};
   alloc_create_info.usage = VMA_MEMORY_USAGE_AUTO;
@@ -154,14 +155,20 @@ std::shared_ptr<Texture> AllocatorVK::OnCreateTexture(
   }
 
   auto image_view = static_cast<vk::ImageView::NativeType>(img_view_res.value);
+  auto staging_buffer =
+      CreateHostVisibleDeviceAllocation(desc.GetByteSizeOfBaseMipLevel());
 
   auto texture_info = std::make_unique<TextureInfoVK>(TextureInfoVK{
       .backing_type = TextureBackingTypeVK::kAllocatedTexture,
       .allocated_texture =
           {
-              .allocator = &allocator_,
-              .allocation = allocation,
-              .allocation_info = allocation_info,
+              .staging_buffer = staging_buffer,
+              .backing_allocation =
+                  {
+                      .allocator = &allocator_,
+                      .allocation = allocation,
+                      .allocation_info = allocation_info,
+                  },
               .image = img,
               .image_view = image_view,
           },
@@ -174,14 +181,22 @@ std::shared_ptr<DeviceBuffer> AllocatorVK::OnCreateBuffer(
     const DeviceBufferDescriptor& desc) {
   // TODO (kaushikiska): consider optimizing  the usage flags based on
   // StorageMode.
+  auto device_allocation = std::make_unique<DeviceBufferAllocationVK>(
+      CreateHostVisibleDeviceAllocation(desc.size));
+  return std::make_shared<DeviceBufferVK>(desc, context_,
+                                          std::move(device_allocation));
+}
+
+DeviceBufferAllocationVK AllocatorVK::CreateHostVisibleDeviceAllocation(
+    size_t size) {
   auto buffer_create_info = static_cast<vk::BufferCreateInfo::NativeType>(
       vk::BufferCreateInfo()
           .setUsage(vk::BufferUsageFlagBits::eVertexBuffer |
                     vk::BufferUsageFlagBits::eIndexBuffer |
                     vk::BufferUsageFlagBits::eUniformBuffer |
                     vk::BufferUsageFlagBits::eTransferSrc |
                     vk::BufferUsageFlagBits::eTransferDst)
-          .setSize(desc.size)
+          .setSize(size)
           .setSharingMode(vk::SharingMode::eExclusive));
 
   VmaAllocationCreateInfo allocCreateInfo = {};
@@ -197,15 +212,27 @@ std::shared_ptr<DeviceBuffer> AllocatorVK::OnCreateBuffer(
                       &buffer, &buffer_allocation, &buffer_allocation_info)};
 
   if (result != vk::Result::eSuccess) {
-    VALIDATION_LOG << "Unable to allocate a device buffer";
-    return nullptr;
+    VALIDATION_LOG << "Unable to allocate a device buffer: "
+                   << vk::to_string(result);
+    return {};
   }
 
-  auto device_allocation = std::make_unique<DeviceBufferAllocationVK>(
-      allocator_, buffer, buffer_allocation, buffer_allocation_info);
+  VkMemoryPropertyFlags memory_props;
+  vmaGetAllocationMemoryProperties(allocator_, buffer_allocation,
+                                   &memory_props);
+  if (!(memory_props & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT)) {
+    VALIDATION_LOG << "Unable to create host visible device buffer.";
+  }
 
-  return std::make_shared<DeviceBufferVK>(desc, context_,
-                                          std::move(device_allocation));
+  return DeviceBufferAllocationVK{
+      .buffer = vk::Buffer{buffer},
+      .backing_allocation =
+          {
+              .allocator = &allocator_,
+              .allocation = buffer_allocation,
+              .allocation_info = buffer_allocation_info,
+          },
+  };
 }
 
 // |Allocator|
@@ -215,4 +242,5 @@ ISize AllocatorVK::GetMaxTextureSizeSupported() const {
   // https://registry.khronos.org/vulkan/specs/1.2-extensions/html/vkspec.html#limits-minmax
   return {4096, 4096};
 }
+
 }  // namespace impeller

impeller/renderer/backend/vulkan/allocator_vk.h

@@ -9,6 +9,7 @@
 #include "flutter/vulkan/procs/vulkan_proc_table.h"
 #include "impeller/renderer/allocator.h"
 #include "impeller/renderer/backend/vulkan/context_vk.h"
+#include "impeller/renderer/backend/vulkan/device_buffer_vk.h"
 #include "impeller/renderer/backend/vulkan/vk.h"
 
 #include <memory>
@@ -51,6 +52,8 @@ class AllocatorVK final : public Allocator {
   // |Allocator|
   ISize GetMaxTextureSizeSupported() const override;
 
+  DeviceBufferAllocationVK CreateHostVisibleDeviceAllocation(size_t size);
+
   FML_DISALLOW_COPY_AND_ASSIGN(AllocatorVK);
 };
 

impeller/renderer/backend/vulkan/device_buffer_vk.cc

@@ -9,30 +9,12 @@
 
 namespace impeller {
 
-DeviceBufferAllocationVK::DeviceBufferAllocationVK(
-    const VmaAllocator& allocator,
-    VkBuffer buffer,
-    VmaAllocation allocation,
-    VmaAllocationInfo allocation_info)
-    : allocator_(allocator),
-      buffer_(buffer),
-      allocation_(allocation),
-      allocation_info_(allocation_info) {}
-
-DeviceBufferAllocationVK::~DeviceBufferAllocationVK() {
-  if (buffer_) {
-    // https://github.com/flutter/flutter/issues/112387
-    // This buffer can be freed once the command buffer is disposed.
-    // vmaDestroyBuffer(allocator_, buffer_, allocation_);
-  }
+void* DeviceBufferAllocationVK::GetMapping() const {
+  return backing_allocation.allocation_info.pMappedData;
 }
 
 vk::Buffer DeviceBufferAllocationVK::GetBufferHandle() const {
-  return buffer_;
-}
-
-void* DeviceBufferAllocationVK::GetMapping() const {
-  return allocation_info_.pMappedData;
+  return buffer;
 }
 
 DeviceBufferVK::DeviceBufferVK(

impeller/renderer/backend/vulkan/device_buffer_vk.h

@@ -13,26 +13,22 @@
 
 namespace impeller {
 
-class DeviceBufferAllocationVK {
- public:
-  DeviceBufferAllocationVK(const VmaAllocator& allocator,
-                           VkBuffer buffer,
-                           VmaAllocation allocation,
-                           VmaAllocationInfo allocation_info);
-
-  ~DeviceBufferAllocationVK();
+// https://github.com/flutter/flutter/issues/112387
+// This buffer can be freed once the command buffer is disposed.
+// vmaDestroyBuffer(allocator_, buffer_, allocation_);
+struct BackingAllocationVK {
+  VmaAllocator* allocator = nullptr;
+  VmaAllocation allocation = nullptr;
+  VmaAllocationInfo allocation_info = {};
+};
 
-  vk::Buffer GetBufferHandle() const;
+struct DeviceBufferAllocationVK {
+  vk::Buffer buffer = VK_NULL_HANDLE;
+  BackingAllocationVK backing_allocation = {};
 
   void* GetMapping() const;
 
- private:
-  const VmaAllocator& allocator_;
-  vk::Buffer buffer_;
-  VmaAllocation allocation_;
-  VmaAllocationInfo allocation_info_;
-
-  FML_DISALLOW_COPY_AND_ASSIGN(DeviceBufferAllocationVK);
+  vk::Buffer GetBufferHandle() const;
 };
 
 class DeviceBufferVK final : public DeviceBuffer,

impeller/renderer/backend/vulkan/render_pass_vk.cc

@@ -125,6 +125,12 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const {
     }
   }
 
+  if (!TransitionImageLayout(frame_num, tex_info.swapchain_image->GetImage(),
+                             vk::ImageLayout::eUndefined,
+                             vk::ImageLayout::eColorAttachmentOptimal)) {
+    return false;
+  }
+
   command_buffer_->endRenderPass();
 
   return const_cast<RenderPassVK*>(this)->EndCommandBuffer(frame_num);
@@ -312,12 +318,20 @@ bool RenderPassVK::UpdateDescriptorSets(uint32_t frame_num,
 
     if (!TransitionImageLayout(frame_num, texture_vk.GetImage(),
                                vk::ImageLayout::eUndefined,
-                               vk::ImageLayout::eGeneral)) {
+                               vk::ImageLayout::eTransferDstOptimal)) {
+      return false;
+    }
+
+    CopyBufferToImage(frame_num, texture_vk);
+
+    if (!TransitionImageLayout(frame_num, texture_vk.GetImage(),
+                               vk::ImageLayout::eTransferDstOptimal,
+                               vk::ImageLayout::eShaderReadOnlyOptimal)) {
       return false;
     }
 
     vk::DescriptorImageInfo desc_image_info;
-    desc_image_info.setImageLayout(vk::ImageLayout::eGeneral);
+    desc_image_info.setImageLayout(vk::ImageLayout::eShaderReadOnlyOptimal);
     desc_image_info.setSampler(sampler_vk.GetSamplerVK());
     desc_image_info.setImageView(texture_vk.GetImageView());
     image_infos.push_back(desc_image_info);
@@ -404,8 +418,10 @@ bool RenderPassVK::TransitionImageLayout(uint32_t frame_num,
 
   vk::ImageMemoryBarrier barrier =
       vk::ImageMemoryBarrier()
-          .setSrcAccessMask(vk::AccessFlagBits::eColorAttachmentRead)
-          .setDstAccessMask(vk::AccessFlagBits::eColorAttachmentWrite)
+          .setSrcAccessMask(vk::AccessFlagBits::eColorAttachmentWrite |
+                            vk::AccessFlagBits::eTransferWrite)
+          .setDstAccessMask(vk::AccessFlagBits::eColorAttachmentRead |
+                            vk::AccessFlagBits::eShaderRead)
           .setOldLayout(layout_old)
           .setNewLayout(layout_new)
           .setSrcQueueFamilyIndex(VK_QUEUE_FAMILY_IGNORED)
@@ -418,10 +434,9 @@ bool RenderPassVK::TransitionImageLayout(uint32_t frame_num,
                   .setLevelCount(1)
                   .setBaseArrayLayer(0)
                   .setLayerCount(1));
-  transition_cmd->pipelineBarrier(
-      vk::PipelineStageFlagBits::eColorAttachmentOutput,
-      vk::PipelineStageFlagBits::eColorAttachmentOutput, {}, nullptr, nullptr,
-      barrier);
+  transition_cmd->pipelineBarrier(vk::PipelineStageFlagBits::eAllGraphics,
+                                  vk::PipelineStageFlagBits::eAllGraphics, {},
+                                  nullptr, nullptr, barrier);
 
   res = transition_cmd->end();
   if (res != vk::Result::eSuccess) {
@@ -433,4 +448,60 @@ bool RenderPassVK::TransitionImageLayout(uint32_t frame_num,
   return true;
 }
 
+bool RenderPassVK::CopyBufferToImage(uint32_t frame_num,
+                                     const TextureVK& texture_vk) const {
+  auto pool = command_buffer_.getPool();
+  vk::CommandBufferAllocateInfo alloc_info =
+      vk::CommandBufferAllocateInfo()
+          .setCommandPool(pool)
+          .setLevel(vk::CommandBufferLevel::ePrimary)
+          .setCommandBufferCount(1);
+  auto cmd_buf_res = device_.allocateCommandBuffersUnique(alloc_info);
+  if (cmd_buf_res.result != vk::Result::eSuccess) {
+    VALIDATION_LOG << "Failed to allocate command buffer: "
+                   << vk::to_string(cmd_buf_res.result);
+    return false;
+  }
+
+  auto copy_cmd = std::move(cmd_buf_res.value[0]);
+
+  const auto& size = texture_vk.GetTextureDescriptor().size;
+
+  // actual copy happens here
+  vk::BufferImageCopy region =
+      vk::BufferImageCopy()
+          .setBufferOffset(0)
+          .setBufferRowLength(0)
+          .setBufferImageHeight(0)
+          .setImageSubresource(
+              vk::ImageSubresourceLayers()
+                  .setAspectMask(vk::ImageAspectFlagBits::eColor)
+                  .setMipLevel(0)
+                  .setBaseArrayLayer(0)
+                  .setLayerCount(1))
+          .setImageOffset(vk::Offset3D(0, 0, 0))
+          .setImageExtent(vk::Extent3D(size.width, size.height, 1));
+
+  vk::CommandBufferBeginInfo begin_info;
+  auto res = copy_cmd->begin(begin_info);
+
+  if (res != vk::Result::eSuccess) {
+    VALIDATION_LOG << "Failed to begin command buffer: " << vk::to_string(res);
+    return false;
+  }
+
+  copy_cmd->copyBufferToImage(texture_vk.GetStagingBuffer(),
+                              texture_vk.GetImage(),
+                              vk::ImageLayout::eTransferDstOptimal, region);
+
+  res = copy_cmd->end();
+  if (res != vk::Result::eSuccess) {
+    VALIDATION_LOG << "Failed to end command buffer: " << vk::to_string(res);
+    return false;
+  }
+
+  surface_producer_->QueueCommandBuffer(frame_num, std::move(copy_cmd));
+  return true;
+}
+
 }  // namespace impeller

impeller/renderer/backend/vulkan/render_pass_vk.h

@@ -4,16 +4,13 @@
 
 #pragma once
 
-#include <vector>
 #include "flutter/fml/macros.h"
 #include "impeller/renderer/backend/vulkan/surface_producer_vk.h"
 #include "impeller/renderer/backend/vulkan/texture_vk.h"
 #include "impeller/renderer/backend/vulkan/vk.h"
 #include "impeller/renderer/command.h"
 #include "impeller/renderer/render_pass.h"
 #include "impeller/renderer/render_target.h"
-#include "vulkan/vulkan_enums.hpp"
-#include "vulkan/vulkan_structs.hpp"
 
 namespace impeller {
 
@@ -77,6 +74,8 @@ class RenderPassVK final : public RenderPass {
                              vk::ImageLayout layout_old,
                              vk::ImageLayout layout_new) const;
 
+  bool CopyBufferToImage(uint32_t frame_num, const TextureVK& texture_vk) const;
+
   FML_DISALLOW_COPY_AND_ASSIGN(RenderPassVK);
 };
 

impeller/renderer/backend/vulkan/texture_vk.cc

@@ -16,7 +16,8 @@ TextureVK::TextureVK(TextureDescriptor desc,
 TextureVK::~TextureVK() {
   if (!IsWrapped() && IsValid()) {
     const auto& texture = texture_info_->allocated_texture;
-    vmaDestroyImage(*texture.allocator, texture.image, texture.allocation);
+    vmaDestroyImage(*texture.backing_allocation.allocator, texture.image,
+                    texture.backing_allocation.allocation);
   }
 }
 
@@ -45,7 +46,7 @@ bool TextureVK::OnSetContents(const uint8_t* contents,
   }
 
   // currently we are only supporting 2d textures, no cube textures etc.
-  auto mapping = texture_info_->allocated_texture.allocation_info.pMappedData;
+  auto mapping = texture_info_->allocated_texture.staging_buffer.GetMapping();
 
   if (mapping) {
     memcpy(mapping, contents, length);
@@ -107,4 +108,17 @@ vk::Image TextureVK::GetImage() const {
   }
 }
 
+vk::Buffer TextureVK::GetStagingBuffer() const {
+  switch (texture_info_->backing_type) {
+    case TextureBackingTypeVK::kUnknownType:
+      FML_CHECK(false) << "Unknown texture backing type";
+      return nullptr;
+    case TextureBackingTypeVK::kAllocatedTexture:
+      return texture_info_->allocated_texture.staging_buffer.GetBufferHandle();
+    case TextureBackingTypeVK::kWrappedTexture:
+      FML_CHECK(false) << "Wrapped textures do not have staging buffers";
+      return nullptr;
+  }
+}
+
 }  // namespace impeller