From 3841a8f3b0ff90198ddf88dcd35ffaf64189855c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 8 Jul 2023 18:11:18 -0700 Subject: [PATCH 1/3] [Impeller] dont decompress into device buffer for Vulkan/GLES --- impeller/core/allocator.cc | 10 +++- impeller/core/allocator.h | 9 ++- impeller/core/device_buffer_descriptor.h | 12 ++++ impeller/core/host_buffer.cc | 4 +- .../renderer/backend/vulkan/allocator_vk.cc | 25 ++++++-- .../renderer/backend/vulkan/texture_vk.cc | 5 +- lib/ui/painting/image_decoder_impeller.cc | 58 ++++++++++++++++--- lib/ui/painting/image_decoder_impeller.h | 4 +- lib/ui/painting/image_decoder_unittests.cc | 5 +- lib/ui/painting/multi_frame_codec.cc | 3 +- 10 files changed, 109 insertions(+), 26 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index be380b671ec27..bfbb4747f1d94 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -16,10 +16,12 @@ Allocator::~Allocator() = default; std::shared_ptr Allocator::CreateBufferWithCopy( const uint8_t* buffer, - size_t length) { + size_t length, + BufferUsageMask usage_mask) { DeviceBufferDescriptor desc; desc.size = length; desc.storage_mode = StorageMode::kHostVisible; + desc.buffer_usage = usage_mask; auto new_buffer = CreateBuffer(desc); if (!new_buffer) { @@ -36,8 +38,10 @@ std::shared_ptr Allocator::CreateBufferWithCopy( } std::shared_ptr Allocator::CreateBufferWithCopy( - const fml::Mapping& mapping) { - return CreateBufferWithCopy(mapping.GetMapping(), mapping.GetSize()); + const fml::Mapping& mapping, + BufferUsageMask usage_mask) { + return CreateBufferWithCopy(mapping.GetMapping(), mapping.GetSize(), + usage_mask); } std::shared_ptr Allocator::CreateBuffer( diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index 25b3dae07a22c..39335529c7582 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -37,11 +37,14 @@ class Allocator { /// virtual uint16_t MinimumBytesPerRow(PixelFormat format) const; - std::shared_ptr CreateBufferWithCopy(const uint8_t* buffer, - size_t length); + std::shared_ptr CreateBufferWithCopy( + const uint8_t* buffer, + size_t length, + BufferUsageMask usage_mask = 0u); std::shared_ptr CreateBufferWithCopy( - const fml::Mapping& mapping); + const fml::Mapping& mapping, + BufferUsageMask usage_mask = 0u); virtual ISize GetMaxTextureSizeSupported() const = 0; diff --git a/impeller/core/device_buffer_descriptor.h b/impeller/core/device_buffer_descriptor.h index 976b8f7b0acac..3c117ebd4b732 100644 --- a/impeller/core/device_buffer_descriptor.h +++ b/impeller/core/device_buffer_descriptor.h @@ -10,9 +10,21 @@ namespace impeller { +using BufferUsageMask = uint64_t; + +enum class BufferUsage : TextureUsageMask { + kShaderRead = 0, + kTransferSrc = 1 << 0, + kTransferDst = 1 << 1, +}; + struct DeviceBufferDescriptor { StorageMode storage_mode = StorageMode::kDeviceTransient; size_t size = 0u; + BufferUsageMask buffer_usage = + static_cast(BufferUsage::kShaderRead) | + static_cast(BufferUsage::kTransferSrc) | + static_cast(BufferUsage::kTransferDst); }; } // namespace impeller diff --git a/impeller/core/host_buffer.cc b/impeller/core/host_buffer.cc index c4f6af7754363..5a7de6caf8206 100644 --- a/impeller/core/host_buffer.cc +++ b/impeller/core/host_buffer.cc @@ -77,7 +77,9 @@ std::shared_ptr HostBuffer::GetDeviceBuffer( if (generation_ == device_buffer_generation_) { return device_buffer_; } - auto new_buffer = allocator.CreateBufferWithCopy(GetBuffer(), GetLength()); + auto new_buffer = allocator.CreateBufferWithCopy( + GetBuffer(), GetLength(), + static_cast(BufferUsage::kShaderRead)); if (!new_buffer) { return nullptr; } diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 36a53d2ce04e1..55140c6d4b07e 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -173,6 +173,24 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( return vk_usage; } +static constexpr vk::BufferUsageFlags ToVKBufferUsageFlags( + BufferUsageMask usage_mask) { + vk::BufferUsageFlags flags = {}; + if (usage_mask & static_cast(BufferUsage::kTransferSrc)) { + flags |= vk::BufferUsageFlagBits::eTransferSrc; + } + if (usage_mask & static_cast(BufferUsage::kTransferDst)) { + flags |= vk::BufferUsageFlagBits::eTransferDst; + } + if (usage_mask & static_cast(BufferUsage::kShaderRead)) { + flags |= vk::BufferUsageFlagBits::eVertexBuffer | + vk::BufferUsageFlagBits::eIndexBuffer | + vk::BufferUsageFlagBits::eUniformBuffer | + vk::BufferUsageFlagBits::eStorageBuffer; + } + return flags; +} + static constexpr VmaMemoryUsage ToVMAMemoryUsage() { return VMA_MEMORY_USAGE_AUTO; } @@ -393,12 +411,7 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( const DeviceBufferDescriptor& desc) { TRACE_EVENT0("impeller", "AllocatorVK::OnCreateBuffer"); vk::BufferCreateInfo buffer_info; - buffer_info.usage = vk::BufferUsageFlagBits::eVertexBuffer | - vk::BufferUsageFlagBits::eIndexBuffer | - vk::BufferUsageFlagBits::eUniformBuffer | - vk::BufferUsageFlagBits::eStorageBuffer | - vk::BufferUsageFlagBits::eTransferSrc | - vk::BufferUsageFlagBits::eTransferDst; + buffer_info.usage = ToVKBufferUsageFlags(desc.buffer_usage); buffer_info.size = desc.size; buffer_info.sharingMode = vk::SharingMode::eExclusive; auto buffer_info_native = diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index cc8c57280cc19..f5747ca78a17b 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -49,8 +49,9 @@ bool TextureVK::OnSetContents(const uint8_t* contents, return false; } - auto staging_buffer = - context->GetResourceAllocator()->CreateBufferWithCopy(contents, length); + auto staging_buffer = context->GetResourceAllocator()->CreateBufferWithCopy( + contents, length, + static_cast(BufferUsage::kTransferSrc)); if (!staging_buffer) { VALIDATION_LOG << "Could not create staging buffer."; diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index af111c6ff0341..38eed40f966e2 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -32,6 +32,42 @@ namespace flutter { +class MallocDeviceBuffer : public impeller::DeviceBuffer { + public: + MallocDeviceBuffer(impeller::DeviceBufferDescriptor desc) + : impeller::DeviceBuffer(desc) { + data_ = static_cast(malloc(desc.size)); + } + + ~MallocDeviceBuffer() override { free(data_); } + + bool SetLabel(const std::string& label) override { return true; } + + bool SetLabel(const std::string& label, impeller::Range range) override { + return true; + } + + uint8_t* OnGetContents() const override { return data_; } + + bool OnCopyHostBuffer(const uint8_t* source, + impeller::Range source_range, + size_t offset) override { + memcpy(data_ + offset, source + source_range.offset, source_range.length); + return true; + } + + private: + uint8_t* data_; + + FML_DISALLOW_COPY_AND_ASSIGN(MallocDeviceBuffer); +}; + +#ifdef FML_OS_ANDROID +static constexpr bool kShouldUseMallocDeviceBuffer = true; +#else +static constexpr bool kShouldUseMallocDeviceBuffer = false; +#endif // FML_OS_ANDROID + namespace { /** * Loads the gamut as a set of three points (triangle). @@ -336,17 +372,20 @@ ImageDecoderImpeller::UploadTextureToPrivate( }) .SetIfTrue([&result, context, bitmap, gpu_disabled_switch] { // create_mips is false because we already know the GPU is disabled. - result = UploadTextureToShared(context, bitmap, gpu_disabled_switch, - /*create_mips=*/false); + result = + UploadTextureToStorage(context, bitmap, gpu_disabled_switch, + impeller::StorageMode::kHostVisible, + /*create_mips=*/false); })); return result; } std::pair, std::string> -ImageDecoderImpeller::UploadTextureToShared( +ImageDecoderImpeller::UploadTextureToStorage( const std::shared_ptr& context, std::shared_ptr bitmap, const std::shared_ptr& gpu_disabled_switch, + impeller::StorageMode storage_mode, bool create_mips) { TRACE_EVENT0("impeller", __FUNCTION__); if (!context) { @@ -366,7 +405,7 @@ ImageDecoderImpeller::UploadTextureToShared( } impeller::TextureDescriptor texture_descriptor; - texture_descriptor.storage_mode = impeller::StorageMode::kHostVisible; + texture_descriptor.storage_mode = storage_mode; texture_descriptor.format = pixel_format.value(); texture_descriptor.size = {image_info.width(), image_info.height()}; texture_descriptor.mip_count = @@ -483,14 +522,16 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, gpu_disabled_switch]() { sk_sp image; std::string decode_error; - if (context->GetCapabilities()->SupportsBufferToTextureBlits()) { + if (!kShouldUseMallocDeviceBuffer && + context->GetCapabilities()->SupportsBufferToTextureBlits()) { std::tie(image, decode_error) = UploadTextureToPrivate( context, bitmap_result.device_buffer, bitmap_result.image_info, bitmap_result.sk_bitmap, gpu_disabled_switch); result(image, decode_error); } else { - std::tie(image, decode_error) = UploadTextureToShared( + std::tie(image, decode_error) = UploadTextureToStorage( context, bitmap_result.sk_bitmap, gpu_disabled_switch, + impeller::StorageMode::kDevicePrivate, /*create_mips=*/true); result(image, decode_error); } @@ -525,7 +566,10 @@ bool ImpellerAllocator::allocPixelRef(SkBitmap* bitmap) { descriptor.size = ((bitmap->height() - 1) * bitmap->rowBytes()) + (bitmap->width() * bitmap->bytesPerPixel()); - auto device_buffer = allocator_->CreateBuffer(descriptor); + std::shared_ptr device_buffer = + kShouldUseMallocDeviceBuffer + ? std::make_shared(descriptor) + : allocator_->CreateBuffer(descriptor); struct ImpellerPixelRef final : public SkPixelRef { ImpellerPixelRef(int w, int h, void* s, size_t r) diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index f3e350bd30a37..063327b490d03 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -9,6 +9,7 @@ #include "flutter/fml/macros.h" #include "flutter/lib/ui/painting/image_decoder.h" +#include "impeller/core/formats.h" #include "impeller/geometry/size.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -90,10 +91,11 @@ class ImageDecoderImpeller final : public ImageDecoder { /// @param gpu_disabled_switch Whether the GPU is available for mipmap /// creation. /// @return A DlImage. - static std::pair, std::string> UploadTextureToShared( + static std::pair, std::string> UploadTextureToStorage( const std::shared_ptr& context, std::shared_ptr bitmap, const std::shared_ptr& gpu_disabled_switch, + impeller::StorageMode storage_mode, bool create_mips = true); private: diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 168aa70883ba2..f09bf87f79411 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -455,8 +455,9 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); ASSERT_EQ(result.second, ""); - result = ImageDecoderImpeller::UploadTextureToShared( - no_gpu_access_context, bitmap, gpu_disabled_switch, true); + result = ImageDecoderImpeller::UploadTextureToStorage( + no_gpu_access_context, bitmap, gpu_disabled_switch, + impeller::StorageMode::kHostVisible, true); ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); ASSERT_EQ(result.second, ""); } diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 4a4d5d27ac17f..815034f8e004a 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -147,9 +147,10 @@ MultiFrameCodec::State::GetNextFrameImage( if (is_impeller_enabled_) { // This is safe regardless of whether the GPU is available or not because // without mipmap creation there is no command buffer encoding done. - return ImageDecoderImpeller::UploadTextureToShared( + return ImageDecoderImpeller::UploadTextureToStorage( impeller_context, std::make_shared(bitmap), std::make_shared(), + impeller::StorageMode::kHostVisible, /*create_mips=*/false); } #endif // IMPELLER_SUPPORTS_RENDERING From dec08eec61d6d40c4dce51f5ce784852e46dbc11 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sat, 8 Jul 2023 18:46:57 -0700 Subject: [PATCH 2/3] Update image_decoder_impeller.cc --- lib/ui/painting/image_decoder_impeller.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 38eed40f966e2..02380442984af 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -34,7 +34,7 @@ namespace flutter { class MallocDeviceBuffer : public impeller::DeviceBuffer { public: - MallocDeviceBuffer(impeller::DeviceBufferDescriptor desc) + explicit MallocDeviceBuffer(impeller::DeviceBufferDescriptor desc) : impeller::DeviceBuffer(desc) { data_ = static_cast(malloc(desc.size)); } From 8d8bf3e70c41bc101f8a4101b393542a0a4ce329 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 10 Jul 2023 12:07:59 -0700 Subject: [PATCH 3/3] ++ --- impeller/core/allocator.cc | 10 +++----- impeller/core/allocator.h | 9 +++---- impeller/core/device_buffer_descriptor.h | 12 --------- impeller/core/host_buffer.cc | 4 +-- .../renderer/backend/vulkan/allocator_vk.cc | 25 +++++-------------- .../renderer/backend/vulkan/texture_vk.cc | 5 ++-- 6 files changed, 15 insertions(+), 50 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index bfbb4747f1d94..be380b671ec27 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -16,12 +16,10 @@ Allocator::~Allocator() = default; std::shared_ptr Allocator::CreateBufferWithCopy( const uint8_t* buffer, - size_t length, - BufferUsageMask usage_mask) { + size_t length) { DeviceBufferDescriptor desc; desc.size = length; desc.storage_mode = StorageMode::kHostVisible; - desc.buffer_usage = usage_mask; auto new_buffer = CreateBuffer(desc); if (!new_buffer) { @@ -38,10 +36,8 @@ std::shared_ptr Allocator::CreateBufferWithCopy( } std::shared_ptr Allocator::CreateBufferWithCopy( - const fml::Mapping& mapping, - BufferUsageMask usage_mask) { - return CreateBufferWithCopy(mapping.GetMapping(), mapping.GetSize(), - usage_mask); + const fml::Mapping& mapping) { + return CreateBufferWithCopy(mapping.GetMapping(), mapping.GetSize()); } std::shared_ptr Allocator::CreateBuffer( diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index 39335529c7582..25b3dae07a22c 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -37,14 +37,11 @@ class Allocator { /// virtual uint16_t MinimumBytesPerRow(PixelFormat format) const; - std::shared_ptr CreateBufferWithCopy( - const uint8_t* buffer, - size_t length, - BufferUsageMask usage_mask = 0u); + std::shared_ptr CreateBufferWithCopy(const uint8_t* buffer, + size_t length); std::shared_ptr CreateBufferWithCopy( - const fml::Mapping& mapping, - BufferUsageMask usage_mask = 0u); + const fml::Mapping& mapping); virtual ISize GetMaxTextureSizeSupported() const = 0; diff --git a/impeller/core/device_buffer_descriptor.h b/impeller/core/device_buffer_descriptor.h index 3c117ebd4b732..976b8f7b0acac 100644 --- a/impeller/core/device_buffer_descriptor.h +++ b/impeller/core/device_buffer_descriptor.h @@ -10,21 +10,9 @@ namespace impeller { -using BufferUsageMask = uint64_t; - -enum class BufferUsage : TextureUsageMask { - kShaderRead = 0, - kTransferSrc = 1 << 0, - kTransferDst = 1 << 1, -}; - struct DeviceBufferDescriptor { StorageMode storage_mode = StorageMode::kDeviceTransient; size_t size = 0u; - BufferUsageMask buffer_usage = - static_cast(BufferUsage::kShaderRead) | - static_cast(BufferUsage::kTransferSrc) | - static_cast(BufferUsage::kTransferDst); }; } // namespace impeller diff --git a/impeller/core/host_buffer.cc b/impeller/core/host_buffer.cc index 5a7de6caf8206..c4f6af7754363 100644 --- a/impeller/core/host_buffer.cc +++ b/impeller/core/host_buffer.cc @@ -77,9 +77,7 @@ std::shared_ptr HostBuffer::GetDeviceBuffer( if (generation_ == device_buffer_generation_) { return device_buffer_; } - auto new_buffer = allocator.CreateBufferWithCopy( - GetBuffer(), GetLength(), - static_cast(BufferUsage::kShaderRead)); + auto new_buffer = allocator.CreateBufferWithCopy(GetBuffer(), GetLength()); if (!new_buffer) { return nullptr; } diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 55140c6d4b07e..36a53d2ce04e1 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -173,24 +173,6 @@ static constexpr vk::ImageUsageFlags ToVKImageUsageFlags( return vk_usage; } -static constexpr vk::BufferUsageFlags ToVKBufferUsageFlags( - BufferUsageMask usage_mask) { - vk::BufferUsageFlags flags = {}; - if (usage_mask & static_cast(BufferUsage::kTransferSrc)) { - flags |= vk::BufferUsageFlagBits::eTransferSrc; - } - if (usage_mask & static_cast(BufferUsage::kTransferDst)) { - flags |= vk::BufferUsageFlagBits::eTransferDst; - } - if (usage_mask & static_cast(BufferUsage::kShaderRead)) { - flags |= vk::BufferUsageFlagBits::eVertexBuffer | - vk::BufferUsageFlagBits::eIndexBuffer | - vk::BufferUsageFlagBits::eUniformBuffer | - vk::BufferUsageFlagBits::eStorageBuffer; - } - return flags; -} - static constexpr VmaMemoryUsage ToVMAMemoryUsage() { return VMA_MEMORY_USAGE_AUTO; } @@ -411,7 +393,12 @@ std::shared_ptr AllocatorVK::OnCreateBuffer( const DeviceBufferDescriptor& desc) { TRACE_EVENT0("impeller", "AllocatorVK::OnCreateBuffer"); vk::BufferCreateInfo buffer_info; - buffer_info.usage = ToVKBufferUsageFlags(desc.buffer_usage); + buffer_info.usage = vk::BufferUsageFlagBits::eVertexBuffer | + vk::BufferUsageFlagBits::eIndexBuffer | + vk::BufferUsageFlagBits::eUniformBuffer | + vk::BufferUsageFlagBits::eStorageBuffer | + vk::BufferUsageFlagBits::eTransferSrc | + vk::BufferUsageFlagBits::eTransferDst; buffer_info.size = desc.size; buffer_info.sharingMode = vk::SharingMode::eExclusive; auto buffer_info_native = diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index f5747ca78a17b..cc8c57280cc19 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -49,9 +49,8 @@ bool TextureVK::OnSetContents(const uint8_t* contents, return false; } - auto staging_buffer = context->GetResourceAllocator()->CreateBufferWithCopy( - contents, length, - static_cast(BufferUsage::kTransferSrc)); + auto staging_buffer = + context->GetResourceAllocator()->CreateBufferWithCopy(contents, length); if (!staging_buffer) { VALIDATION_LOG << "Could not create staging buffer.";