From a12501b4526a3de3a60e253fb2e3b41d39dc103f Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Tue, 15 Nov 2022 08:32:17 -0500 Subject: [PATCH] [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. --- .../renderer/backend/vulkan/allocator_vk.cc | 50 ++++++++--- .../renderer/backend/vulkan/allocator_vk.h | 3 + .../backend/vulkan/device_buffer_vk.cc | 24 +---- .../backend/vulkan/device_buffer_vk.h | 28 +++--- .../renderer/backend/vulkan/render_pass_vk.cc | 87 +++++++++++++++++-- .../renderer/backend/vulkan/render_pass_vk.h | 5 +- .../renderer/backend/vulkan/texture_vk.cc | 18 +++- impeller/renderer/backend/vulkan/texture_vk.h | 8 +- 8 files changed, 159 insertions(+), 64 deletions(-) diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index bb034b0b3870b..b0290d0516738 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -114,7 +114,8 @@ std::shared_ptr 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 AllocatorVK::OnCreateTexture( } auto image_view = static_cast(img_view_res.value); + auto staging_buffer = + CreateHostVisibleDeviceAllocation(desc.GetByteSizeOfBaseMipLevel()); auto texture_info = std::make_unique(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,6 +181,14 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( const DeviceBufferDescriptor& desc) { // TODO (kaushikiska): consider optimizing the usage flags based on // StorageMode. + auto device_allocation = std::make_unique( + CreateHostVisibleDeviceAllocation(desc.size)); + return std::make_shared(desc, context_, + std::move(device_allocation)); +} + +DeviceBufferAllocationVK AllocatorVK::CreateHostVisibleDeviceAllocation( + size_t size) { auto buffer_create_info = static_cast( vk::BufferCreateInfo() .setUsage(vk::BufferUsageFlagBits::eVertexBuffer | @@ -181,7 +196,7 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( 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 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( - 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(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 diff --git a/impeller/renderer/backend/vulkan/allocator_vk.h b/impeller/renderer/backend/vulkan/allocator_vk.h index 595a0b838ec96..02b76623407e9 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.h +++ b/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 @@ -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); }; diff --git a/impeller/renderer/backend/vulkan/device_buffer_vk.cc b/impeller/renderer/backend/vulkan/device_buffer_vk.cc index 3951864d701fe..92c670da72df1 100644 --- a/impeller/renderer/backend/vulkan/device_buffer_vk.cc +++ b/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( diff --git a/impeller/renderer/backend/vulkan/device_buffer_vk.h b/impeller/renderer/backend/vulkan/device_buffer_vk.h index 59aa773b3a15b..da8fd8a729e3d 100644 --- a/impeller/renderer/backend/vulkan/device_buffer_vk.h +++ b/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, diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index d5d46b8c3c873..8ee31b70ea267 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/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(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 diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 79227696af038..9bf05412b5eee 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -4,7 +4,6 @@ #pragma once -#include #include "flutter/fml/macros.h" #include "impeller/renderer/backend/vulkan/surface_producer_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" @@ -12,8 +11,6 @@ #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); }; diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index e3c25391e8a08..4faa15f3ea1d8 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/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 diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h index 9134894ba801e..08dd3769e22d9 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.h +++ b/impeller/renderer/backend/vulkan/texture_vk.h @@ -7,6 +7,7 @@ #include "flutter/fml/macros.h" #include "impeller/base/backend_cast.h" #include "impeller/renderer/backend/vulkan/context_vk.h" +#include "impeller/renderer/backend/vulkan/device_buffer_vk.h" #include "impeller/renderer/backend/vulkan/swapchain_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" #include "impeller/renderer/texture.h" @@ -25,9 +26,8 @@ struct WrappedTextureInfoVK { }; struct AllocatedTextureInfoVK { - VmaAllocator* allocator = nullptr; - VmaAllocation allocation = nullptr; - VmaAllocationInfo allocation_info = {}; + DeviceBufferAllocationVK staging_buffer = {}; + BackingAllocationVK backing_allocation = {}; VkImage image = VK_NULL_HANDLE; VkImageView image_view = VK_NULL_HANDLE; }; @@ -55,6 +55,8 @@ class TextureVK final : public Texture, public BackendCast { vk::ImageView GetImageView() const; + vk::Buffer GetStagingBuffer() const; + TextureInfoVK* GetTextureInfo() const; private: