From ca965f20be32ef164b3ac9b9ca75786195941271 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 11:53:43 -0700 Subject: [PATCH 01/27] [Impeller] use blit pass to resize image. --- lib/ui/painting/image_decoder_impeller.cc | 103 +++++++++++----------- lib/ui/painting/image_decoder_impeller.h | 3 + 2 files changed, 56 insertions(+), 50 deletions(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 76a5b25202f14..6d361d37df71a 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -224,59 +224,29 @@ DecompressResult ImageDecoderImpeller::DecompressTexture( bitmap = premul_bitmap; } - if (bitmap->dimensions() == target_size) { - std::shared_ptr buffer = - bitmap_allocator->GetDeviceBuffer(); - if (!buffer) { - return DecompressResult{.decode_error = "Unable to get device buffer"}; - } - buffer->Flush(); - - return DecompressResult{.device_buffer = std::move(buffer), - .sk_bitmap = bitmap, - .image_info = bitmap->info()}; - } - - //---------------------------------------------------------------------------- - /// 2. If the decoded image isn't the requested target size, resize it. - /// - - TRACE_EVENT0("impeller", "DecodeScale"); - const auto scaled_image_info = image_info.makeDimensions(target_size); - - auto scaled_bitmap = std::make_shared(); - auto scaled_allocator = std::make_shared(allocator); - scaled_bitmap->setInfo(scaled_image_info); - if (!scaled_bitmap->tryAllocPixels(scaled_allocator.get())) { - std::string decode_error( - "Could not allocate scaled bitmap for image decompression."); - FML_DLOG(ERROR) << decode_error; - return DecompressResult{.decode_error = decode_error}; - } - if (!bitmap->pixmap().scalePixels( - scaled_bitmap->pixmap(), - SkSamplingOptions(SkFilterMode::kLinear, SkMipmapMode::kNone))) { - FML_LOG(ERROR) << "Could not scale decoded bitmap data."; - } - scaled_bitmap->setImmutable(); - std::shared_ptr buffer = - scaled_allocator->GetDeviceBuffer(); - buffer->Flush(); - + bitmap_allocator->GetDeviceBuffer(); if (!buffer) { return DecompressResult{.decode_error = "Unable to get device buffer"}; } + buffer->Flush(); + + std::optional resize_info = + bitmap->dimensions() == target_size + ? std::nullopt + : std::optional(image_info.makeDimensions(target_size)); return DecompressResult{.device_buffer = std::move(buffer), - .sk_bitmap = scaled_bitmap, - .image_info = scaled_bitmap->info()}; + .sk_bitmap = bitmap, + .image_info = bitmap->info(), + .resize_info = resize_info}; } /// Only call this method if the GPU is available. static std::pair, std::string> UnsafeUploadTextureToPrivate( const std::shared_ptr& context, const std::shared_ptr& buffer, - const SkImageInfo& image_info) { + const SkImageInfo& image_info, + const std::optional& resize_info) { const auto pixel_format = impeller::skia_conversions::ToPixelFormat(image_info.colorType()); if (!pixel_format) { @@ -323,19 +293,45 @@ static std::pair, std::string> UnsafeUploadTextureToPrivate( blit_pass->SetLabel("Mipmap Blit Pass"); blit_pass->AddCopy(impeller::DeviceBuffer::AsBufferView(buffer), dest_texture); - if (texture_descriptor.size.MipCount() > 1) { + if (texture_descriptor.mip_count > 1) { blit_pass->GenerateMipmap(dest_texture); } + std::shared_ptr result_texture = dest_texture; + if (resize_info.has_value()) { + impeller::TextureDescriptor resize_desc; + resize_desc.storage_mode = impeller::StorageMode::kDevicePrivate; + resize_desc.format = pixel_format.value(); + resize_desc.size = {resize_info->width(), resize_info->height()}; + resize_desc.mip_count = resize_desc.size.MipCount(); + resize_desc.compression_type = impeller::CompressionType::kLossy; + + auto resize_texture = + context->GetResourceAllocator()->CreateTexture(resize_desc); + if (!resize_texture) { + std::string decode_error("Could not create resized Impeller texture."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); + } + + blit_pass->AddCopy(/*source=*/dest_texture, /*destination=*/resize_texture); + if (resize_desc.mip_count > 1) { + blit_pass->GenerateMipmap(resize_texture); + } + + result_texture = std::move(resize_texture); + } blit_pass->EncodeCommands(context->GetResourceAllocator()); + if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) { - std::string decode_error("Failed to submit blit pass command buffer."); + std::string decode_error("Failed to submit image deocding command buffer."); FML_DLOG(ERROR) << decode_error; return std::make_pair(nullptr, decode_error); } return std::make_pair( - impeller::DlImageImpeller::Make(std::move(dest_texture)), std::string()); + impeller::DlImageImpeller::Make(std::move(result_texture)), + std::string()); } std::pair, std::string> @@ -344,6 +340,7 @@ ImageDecoderImpeller::UploadTextureToPrivate( const std::shared_ptr& buffer, const SkImageInfo& image_info, const std::shared_ptr& bitmap, + const std::optional& resize_info, const std::shared_ptr& gpu_disabled_switch) { TRACE_EVENT0("impeller", __FUNCTION__); if (!context) { @@ -356,8 +353,9 @@ ImageDecoderImpeller::UploadTextureToPrivate( std::pair, std::string> result; gpu_disabled_switch->Execute( fml::SyncSwitch::Handlers() - .SetIfFalse([&result, context, buffer, image_info] { - result = UnsafeUploadTextureToPrivate(context, buffer, image_info); + .SetIfFalse([&result, context, buffer, image_info, resize_info] { + result = UnsafeUploadTextureToPrivate(context, buffer, image_info, + resize_info); }) .SetIfTrue([&result, context, bitmap, gpu_disabled_switch] { // create_mips is false because we already know the GPU is disabled. @@ -511,9 +509,14 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, gpu_disabled_switch]() { sk_sp image; std::string decode_error; - std::tie(image, decode_error) = UploadTextureToPrivate( - context, bitmap_result.device_buffer, bitmap_result.image_info, - bitmap_result.sk_bitmap, gpu_disabled_switch); + std::tie(image, decode_error) = + UploadTextureToPrivate(context, // + bitmap_result.device_buffer, // + bitmap_result.image_info, // + bitmap_result.sk_bitmap, // + bitmap_result.resize_info, // + gpu_disabled_switch // + ); result(image, decode_error); }; io_runner->PostTask(upload_texture_and_invoke_result); diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index 063327b490d03..fb38ab2a466c3 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -11,6 +11,7 @@ #include "flutter/lib/ui/painting/image_decoder.h" #include "impeller/core/formats.h" #include "impeller/geometry/size.h" +#include "include/core/SkImageInfo.h" #include "third_party/skia/include/core/SkBitmap.h" namespace impeller { @@ -41,6 +42,7 @@ struct DecompressResult { std::shared_ptr device_buffer; std::shared_ptr sk_bitmap; SkImageInfo image_info; + std::optional resize_info = std::nullopt; std::string decode_error; }; @@ -81,6 +83,7 @@ class ImageDecoderImpeller final : public ImageDecoder { const std::shared_ptr& buffer, const SkImageInfo& image_info, const std::shared_ptr& bitmap, + const std::optional& resize_info, const std::shared_ptr& gpu_disabled_switch); /// @brief Create a host visible texture from the provided bitmap. From abf3b66270ef278b04eba3d8bdd7735dc12d05d0 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 12:45:25 -0700 Subject: [PATCH 02/27] test fixes. --- lib/ui/painting/image_decoder_unittests.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 2504a95064b6e..a0c2afdf16db7 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -337,7 +337,8 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { auto buffer = std::make_shared(desc); auto result = ImageDecoderImpeller::UploadTextureToPrivate( - no_gpu_access_context, buffer, info, bitmap, gpu_disabled_switch); + no_gpu_access_context, buffer, info, bitmap, std::nullopt, + gpu_disabled_switch); ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); ASSERT_EQ(result.second, ""); From 90461470176325347dc92415cbf14b6b9370fceb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 14:00:59 -0700 Subject: [PATCH 03/27] ++ --- impeller/renderer/backend/metal/context_mtl.h | 10 ++- .../renderer/backend/metal/context_mtl.mm | 13 ++-- impeller/renderer/context.h | 6 +- lib/ui/painting/image_decoder_impeller.cc | 71 ++++++++++++------- lib/ui/painting/image_decoder_impeller.h | 8 ++- lib/ui/painting/image_encoding_impeller.cc | 8 ++- lib/ui/painting/image_encoding_unittests.cc | 2 +- shell/common/snapshot_controller_impeller.cc | 3 +- 8 files changed, 80 insertions(+), 41 deletions(-) diff --git a/impeller/renderer/backend/metal/context_mtl.h b/impeller/renderer/backend/metal/context_mtl.h index b09933c752dfc..fbd818f10f5c3 100644 --- a/impeller/renderer/backend/metal/context_mtl.h +++ b/impeller/renderer/backend/metal/context_mtl.h @@ -136,7 +136,8 @@ class ContextMTL final : public Context, #endif // IMPELLER_DEBUG // |Context| - void StoreTaskForGPU(const std::function& task) override; + void StoreTaskForGPU(const std::function& task, + const std::function& failure) override; private: class SyncSwitchObserver : public fml::SyncSwitch::Observer { @@ -149,6 +150,11 @@ class ContextMTL final : public Context, ContextMTL& parent_; }; + struct PendingTasks { + std::function task; + std::function failure; + }; + id device_ = nullptr; id command_queue_ = nullptr; std::shared_ptr shader_library_; @@ -157,7 +163,7 @@ class ContextMTL final : public Context, std::shared_ptr resource_allocator_; std::shared_ptr device_capabilities_; std::shared_ptr is_gpu_disabled_sync_switch_; - std::deque> tasks_awaiting_gpu_; + std::deque tasks_awaiting_gpu_; std::unique_ptr sync_switch_observer_; std::shared_ptr command_queue_ip_; #ifdef IMPELLER_DEBUG diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 03c6ea073950f..ccb1e2262c3f6 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -377,17 +377,21 @@ new ContextMTL(device, command_queue, return buffer; } -void ContextMTL::StoreTaskForGPU(const std::function& task) { - tasks_awaiting_gpu_.emplace_back(task); +void ContextMTL::StoreTaskForGPU(const std::function& task, + const std::function& failure) { + tasks_awaiting_gpu_.push_back(PendingTasks{task, failure}); while (tasks_awaiting_gpu_.size() > kMaxTasksAwaitingGPU) { - tasks_awaiting_gpu_.front()(); + PendingTasks front = std::move(tasks_awaiting_gpu_.front()); + if (front.failure) { + front.failure(); + } tasks_awaiting_gpu_.pop_front(); } } void ContextMTL::FlushTasksAwaitingGPU() { for (const auto& task : tasks_awaiting_gpu_) { - task(); + task.task(); } tasks_awaiting_gpu_.clear(); } @@ -396,6 +400,7 @@ new ContextMTL(device, command_queue, : parent_(parent) {} void ContextMTL::SyncSwitchObserver::OnSyncSwitchUpdate(bool new_is_disabled) { + FML_LOG(ERROR) << "Flush"; if (!new_is_disabled) { parent_.FlushTasksAwaitingGPU(); } diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index 4a51560978bb2..eaa9b6b642b00 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -176,10 +176,14 @@ class Context { /// being available or that the task has been canceled. The task should /// operate with the `SyncSwitch` to make sure the GPU is accessible. /// + /// If the queue of pending tasks is cleared without GPU access, then the + /// failure callback will be invoked and the primary task function will not + /// /// Threadsafe. /// /// `task` will be executed on the platform thread. - virtual void StoreTaskForGPU(const std::function& task) { + virtual void StoreTaskForGPU(const std::function& task, + const std::function& failure) { FML_CHECK(false && "not supported in this context"); } diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 6d361d37df71a..9e317fa697ddb 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -26,7 +26,6 @@ #include "third_party/skia/include/core/SkPixelRef.h" #include "third_party/skia/include/core/SkPixmap.h" #include "third_party/skia/include/core/SkPoint.h" -#include "third_party/skia/include/core/SkSamplingOptions.h" #include "third_party/skia/include/core/SkSize.h" namespace flutter { @@ -334,8 +333,8 @@ static std::pair, std::string> UnsafeUploadTextureToPrivate( std::string()); } -std::pair, std::string> -ImageDecoderImpeller::UploadTextureToPrivate( +void ImageDecoderImpeller::UploadTextureToPrivate( + ImageResult result, const std::shared_ptr& context, const std::shared_ptr& buffer, const SkImageInfo& image_info, @@ -344,27 +343,43 @@ ImageDecoderImpeller::UploadTextureToPrivate( const std::shared_ptr& gpu_disabled_switch) { TRACE_EVENT0("impeller", __FUNCTION__); if (!context) { - return std::make_pair(nullptr, "No Impeller context is available"); + result(nullptr, "No Impeller context is available"); + return; } if (!buffer) { - return std::make_pair(nullptr, "No Impeller device buffer is available"); + result(nullptr, "No Impeller device buffer is available"); + return; } - std::pair, std::string> result; gpu_disabled_switch->Execute( fml::SyncSwitch::Handlers() .SetIfFalse([&result, context, buffer, image_info, resize_info] { - result = UnsafeUploadTextureToPrivate(context, buffer, image_info, - resize_info); + sk_sp image; + std::string decode_error; + std::tie(image, decode_error) = std::tie(image, decode_error) = + UnsafeUploadTextureToPrivate(context, buffer, image_info, + resize_info); + result(image, decode_error); }) - .SetIfTrue([&result, context, bitmap, gpu_disabled_switch] { - // create_mips is false because we already know the GPU is disabled. - result = - UploadTextureToStorage(context, bitmap, gpu_disabled_switch, - impeller::StorageMode::kHostVisible, - /*create_mips=*/false); + .SetIfTrue([&result, context, buffer, image_info, resize_info] { + // The `result` function must be copied in the capture list for each + // closure or the stack allocated callback will be cleared by the + // time to closure is executed later. + context->StoreTaskForGPU( + [result, context, buffer, image_info, resize_info]() { + sk_sp image; + std::string decode_error; + std::tie(image, decode_error) = + std::tie(image, decode_error) = + UnsafeUploadTextureToPrivate(context, buffer, + image_info, resize_info); + result(image, decode_error); + }, + [result]() { + result(nullptr, + "Image upload failed due to loss of GPU access."); + }); })); - return result; } std::pair, std::string> @@ -507,19 +522,21 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, auto upload_texture_and_invoke_result = [result, context, bitmap_result, gpu_disabled_switch]() { - sk_sp image; - std::string decode_error; - std::tie(image, decode_error) = - UploadTextureToPrivate(context, // - bitmap_result.device_buffer, // - bitmap_result.image_info, // - bitmap_result.sk_bitmap, // - bitmap_result.resize_info, // - gpu_disabled_switch // - ); - result(image, decode_error); + UploadTextureToPrivate(result, context, // + bitmap_result.device_buffer, // + bitmap_result.image_info, // + bitmap_result.sk_bitmap, // + bitmap_result.resize_info, // + gpu_disabled_switch // + ); }; - io_runner->PostTask(upload_texture_and_invoke_result); + // The I/O image uploads are not threadsafe on GLES. + if (context->GetBackendType() == + impeller::Context::BackendType::kOpenGLES) { + io_runner->PostTask(upload_texture_and_invoke_result); + } else { + upload_texture_and_invoke_result(); + } }); } diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index fb38ab2a466c3..499446229ffd8 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -71,14 +71,16 @@ class ImageDecoderImpeller final : public ImageDecoder { const std::shared_ptr& allocator); /// @brief Create a device private texture from the provided host buffer. - /// This method is only suported on the metal backend. + /// + /// @param result The image result closure that accepts the DlImage and any + /// encoding error messages. /// @param context The Impeller graphics context. /// @param buffer A host buffer containing the image to be uploaded. /// @param image_info Format information about the particular image. /// @param bitmap A bitmap containg the image to be uploaded. /// @param gpu_disabled_switch Whether the GPU is available command encoding. - /// @return A DlImage. - static std::pair, std::string> UploadTextureToPrivate( + static void UploadTextureToPrivate( + ImageResult result, const std::shared_ptr& context, const std::shared_ptr& buffer, const SkImageInfo& image_info, diff --git a/lib/ui/painting/image_encoding_impeller.cc b/lib/ui/painting/image_encoding_impeller.cc index 4f9d7722044f7..1dddb456fac58 100644 --- a/lib/ui/painting/image_encoding_impeller.cc +++ b/lib/ui/painting/image_encoding_impeller.cc @@ -5,6 +5,7 @@ #include "flutter/lib/ui/painting/image_encoding_impeller.h" #include "flutter/lib/ui/painting/image.h" +#include "fml/status.h" #include "impeller/core/device_buffer.h" #include "impeller/core/formats.h" #include "impeller/renderer/command_buffer.h" @@ -90,8 +91,7 @@ void DoConvertImageToRasterImpellerWithRetry( // task on the Context so it can be executed when the GPU becomes available. if (status.code() == fml::StatusCode::kUnavailable) { impeller_context->StoreTaskForGPU( - [dl_image, encode_task = std::move(encode_task), - is_gpu_disabled_sync_switch, impeller_context, + [dl_image, encode_task, is_gpu_disabled_sync_switch, impeller_context, retry_runner]() mutable { auto retry_task = [dl_image, encode_task = std::move(encode_task), is_gpu_disabled_sync_switch, impeller_context] { @@ -111,6 +111,10 @@ void DoConvertImageToRasterImpellerWithRetry( } else { retry_task(); } + }, + [encode_task]() { + encode_task( + fml::Status(fml::StatusCode::kUnavailable, "GPU unavailable.")); }); } else { // Pass on errors that are not `kUnavailable`. diff --git a/lib/ui/painting/image_encoding_unittests.cc b/lib/ui/painting/image_encoding_unittests.cc index 82b6db2eab60a..815877fe80bbf 100644 --- a/lib/ui/painting/image_encoding_unittests.cc +++ b/lib/ui/painting/image_encoding_unittests.cc @@ -375,7 +375,7 @@ TEST_F(ShellTest, EncodeImageFailsWithoutGPUImpeller) { // This will cause the stored tasks to overflow and start throwing them // away. for (int i = 0; i < impeller::Context::kMaxTasksAwaitingGPU; ++i) { - impeller_context->StoreTaskForGPU([] {}); + impeller_context->StoreTaskForGPU([] {}, [] {}); } }); }; diff --git a/shell/common/snapshot_controller_impeller.cc b/shell/common/snapshot_controller_impeller.cc index 68bc9ac46a3c8..a6c3a8e2962cb 100644 --- a/shell/common/snapshot_controller_impeller.cc +++ b/shell/common/snapshot_controller_impeller.cc @@ -140,7 +140,8 @@ void SnapshotControllerImpeller::MakeRasterSnapshot( picture_size, callback = std::move(callback)] { callback(DoMakeRasterSnapshot(display_list, picture_size, sync_switch, context)); - }); + }, + [callback]() { callback(nullptr); }); } else { callback(nullptr); } From a2b6b726030cda2dee545c30ec72e3e7ace1786a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 14:02:03 -0700 Subject: [PATCH 04/27] ++ --- lib/ui/painting/image_decoder_impeller.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index 499446229ffd8..d778f8aefb6ce 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -72,8 +72,8 @@ class ImageDecoderImpeller final : public ImageDecoder { /// @brief Create a device private texture from the provided host buffer. /// - /// @param result The image result closure that accepts the DlImage and any - /// encoding error messages. + /// @param result The image result closure that accepts the DlImage and + /// any encoding error messages. /// @param context The Impeller graphics context. /// @param buffer A host buffer containing the image to be uploaded. /// @param image_info Format information about the particular image. From 6d853d0d61814f56bc6e9adae9561dc6c16d767e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 15:40:31 -0700 Subject: [PATCH 05/27] compile fixes. --- lib/ui/painting/image_decoder_unittests.cc | 30 +++++++++++++++------ lib/ui/painting/image_encoding_unittests.cc | 2 +- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index a0c2afdf16db7..44fa5720f8316 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -336,17 +336,31 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { desc.size = bitmap->computeByteSize(); auto buffer = std::make_shared(desc); - auto result = ImageDecoderImpeller::UploadTextureToPrivate( - no_gpu_access_context, buffer, info, bitmap, std::nullopt, + std::string error_message; + sk_sp result_image; + + auto cb = [&result_image, &error_message](sk_sp image, + std::string message) { + result_image = image; + error_message = message; + }; + + ImageDecoderImpeller::UploadTextureToPrivate( + cb, no_gpu_access_context, buffer, info, bitmap, std::nullopt, gpu_disabled_switch); - ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); - ASSERT_EQ(result.second, ""); - result = ImageDecoderImpeller::UploadTextureToStorage( - no_gpu_access_context, bitmap, gpu_disabled_switch, + EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); + EXPECT_EQ(error_message, ""); + + result_image = nullptr; + error_message = ""; + + ImageDecoderImpeller::UploadTextureToStorage( + cb, 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, ""); + + EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); + EXPECT_EQ(result.second, ""); } TEST_F(ImageDecoderFixtureTest, ImpellerNullColorspace) { diff --git a/lib/ui/painting/image_encoding_unittests.cc b/lib/ui/painting/image_encoding_unittests.cc index 815877fe80bbf..380647d43d074 100644 --- a/lib/ui/painting/image_encoding_unittests.cc +++ b/lib/ui/painting/image_encoding_unittests.cc @@ -374,7 +374,7 @@ TEST_F(ShellTest, EncodeImageFailsWithoutGPUImpeller) { shell->GetIOManager()->GetImpellerContext(); // This will cause the stored tasks to overflow and start throwing them // away. - for (int i = 0; i < impeller::Context::kMaxTasksAwaitingGPU; ++i) { + for (int i = 0; i < impeller::Context::kMaxTasksAwaitingGPU; i++) { impeller_context->StoreTaskForGPU([] {}, [] {}); } }); From 407eca80295965d5c869bd526febdd531e2c67ee Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 17:14:46 -0700 Subject: [PATCH 06/27] ++ --- lib/ui/painting/image_decoder_impeller.cc | 142 +++++---------------- lib/ui/painting/image_decoder_impeller.h | 20 +-- lib/ui/painting/image_decoder_unittests.cc | 7 - lib/ui/painting/multi_frame_codec.cc | 45 ++++++- 4 files changed, 74 insertions(+), 140 deletions(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 9e317fa697ddb..5685d8854c00f 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -240,14 +240,18 @@ DecompressResult ImageDecoderImpeller::DecompressTexture( .resize_info = resize_info}; } -/// Only call this method if the GPU is available. -static std::pair, std::string> UnsafeUploadTextureToPrivate( +// static +std::pair, std::string> +ImageDecoderImpeller::UnsafeUploadTextureToPrivate( const std::shared_ptr& context, const std::shared_ptr& buffer, const SkImageInfo& image_info, - const std::optional& resize_info) { + const std::optional& resize_info, + bool create_mips) { const auto pixel_format = impeller::skia_conversions::ToPixelFormat(image_info.colorType()); + // mips should still be created if the source image is being resized. + const bool should_create_mips = create_mips || resize_info.has_value(); if (!pixel_format) { std::string decode_error(impeller::SPrintF( "Unsupported pixel format (SkColorType=%d)", image_info.colorType())); @@ -259,7 +263,8 @@ static std::pair, std::string> UnsafeUploadTextureToPrivate( texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate; texture_descriptor.format = pixel_format.value(); texture_descriptor.size = {image_info.width(), image_info.height()}; - texture_descriptor.mip_count = texture_descriptor.size.MipCount(); + texture_descriptor.mip_count = + should_create_mips ? texture_descriptor.size.MipCount() : 1; texture_descriptor.compression_type = impeller::CompressionType::kLossy; auto dest_texture = @@ -302,7 +307,7 @@ static std::pair, std::string> UnsafeUploadTextureToPrivate( resize_desc.storage_mode = impeller::StorageMode::kDevicePrivate; resize_desc.format = pixel_format.value(); resize_desc.size = {resize_info->width(), resize_info->height()}; - resize_desc.mip_count = resize_desc.size.MipCount(); + resize_desc.mip_count = create_mips ? resize_desc.size.MipCount() : 1; resize_desc.compression_type = impeller::CompressionType::kLossy; auto resize_texture = @@ -340,7 +345,8 @@ void ImageDecoderImpeller::UploadTextureToPrivate( const SkImageInfo& image_info, const std::shared_ptr& bitmap, const std::optional& resize_info, - const std::shared_ptr& gpu_disabled_switch) { + const std::shared_ptr& gpu_disabled_switch, + bool create_mips) { TRACE_EVENT0("impeller", __FUNCTION__); if (!context) { result(nullptr, "No Impeller context is available"); @@ -353,26 +359,29 @@ void ImageDecoderImpeller::UploadTextureToPrivate( gpu_disabled_switch->Execute( fml::SyncSwitch::Handlers() - .SetIfFalse([&result, context, buffer, image_info, resize_info] { - sk_sp image; - std::string decode_error; - std::tie(image, decode_error) = std::tie(image, decode_error) = - UnsafeUploadTextureToPrivate(context, buffer, image_info, - resize_info); - result(image, decode_error); - }) - .SetIfTrue([&result, context, buffer, image_info, resize_info] { + .SetIfFalse( + [&result, context, buffer, image_info, resize_info, create_mips] { + sk_sp image; + std::string decode_error; + std::tie(image, decode_error) = std::tie(image, decode_error) = + UnsafeUploadTextureToPrivate(context, buffer, image_info, + resize_info, create_mips); + result(image, decode_error); + }) + .SetIfTrue([&result, context, buffer, image_info, resize_info, + create_mips] { // The `result` function must be copied in the capture list for each // closure or the stack allocated callback will be cleared by the // time to closure is executed later. context->StoreTaskForGPU( - [result, context, buffer, image_info, resize_info]() { + [result, context, buffer, image_info, resize_info, + create_mips]() { sk_sp image; std::string decode_error; - std::tie(image, decode_error) = - std::tie(image, decode_error) = - UnsafeUploadTextureToPrivate(context, buffer, - image_info, resize_info); + std::tie(image, decode_error) = std::tie(image, + decode_error) = + UnsafeUploadTextureToPrivate(context, buffer, image_info, + resize_info, create_mips); result(image, decode_error); }, [result]() { @@ -382,99 +391,6 @@ void ImageDecoderImpeller::UploadTextureToPrivate( })); } -std::pair, std::string> -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) { - return std::make_pair(nullptr, "No Impeller context is available"); - } - if (!bitmap) { - return std::make_pair(nullptr, "No texture bitmap is available"); - } - const auto image_info = bitmap->info(); - const auto pixel_format = - impeller::skia_conversions::ToPixelFormat(image_info.colorType()); - if (!pixel_format) { - std::string decode_error(impeller::SPrintF( - "Unsupported pixel format (SkColorType=%d)", image_info.colorType())); - FML_DLOG(ERROR) << decode_error; - return std::make_pair(nullptr, decode_error); - } - - impeller::TextureDescriptor texture_descriptor; - 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 = - create_mips ? texture_descriptor.size.MipCount() : 1; - - auto texture = - context->GetResourceAllocator()->CreateTexture(texture_descriptor); - if (!texture) { - std::string decode_error("Could not create Impeller texture."); - FML_DLOG(ERROR) << decode_error; - return std::make_pair(nullptr, decode_error); - } - - auto mapping = std::make_shared( - reinterpret_cast(bitmap->getAddr(0, 0)), // data - texture_descriptor.GetByteSizeOfBaseMipLevel(), // size - [bitmap](auto, auto) mutable { bitmap.reset(); } // proc - ); - - if (!texture->SetContents(mapping)) { - std::string decode_error("Could not copy contents into Impeller texture."); - FML_DLOG(ERROR) << decode_error; - return std::make_pair(nullptr, decode_error); - } - - texture->SetLabel(impeller::SPrintF("ui.Image(%p)", texture.get()).c_str()); - - if (texture_descriptor.mip_count > 1u && create_mips) { - std::optional decode_error; - - // The only platform that needs mipmapping unconditionally is GL. - // GL based platforms never disable GPU access. - // This is only really needed for iOS. - gpu_disabled_switch->Execute(fml::SyncSwitch::Handlers().SetIfFalse( - [context, &texture, &decode_error] { - auto command_buffer = context->CreateCommandBuffer(); - if (!command_buffer) { - decode_error = - "Could not create command buffer for mipmap generation."; - return; - } - command_buffer->SetLabel("Mipmap Command Buffer"); - - auto blit_pass = command_buffer->CreateBlitPass(); - if (!blit_pass) { - decode_error = "Could not create blit pass for mipmap generation."; - return; - } - blit_pass->SetLabel("Mipmap Blit Pass"); - blit_pass->GenerateMipmap(texture); - blit_pass->EncodeCommands(context->GetResourceAllocator()); - if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) { - decode_error = "Failed to submit blit pass command buffer."; - return; - } - command_buffer->WaitUntilScheduled(); - })); - if (decode_error.has_value()) { - FML_DLOG(ERROR) << decode_error.value(); - return std::make_pair(nullptr, decode_error.value()); - } - } - - return std::make_pair(impeller::DlImageImpeller::Make(std::move(texture)), - std::string()); -} - // |ImageDecoder| void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, uint32_t target_width, diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index d778f8aefb6ce..9da83923f03e2 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -86,21 +86,15 @@ class ImageDecoderImpeller final : public ImageDecoder { const SkImageInfo& image_info, const std::shared_ptr& bitmap, const std::optional& resize_info, - const std::shared_ptr& gpu_disabled_switch); + const std::shared_ptr& gpu_disabled_switch, + bool create_mips = true); - /// @brief Create a host visible texture from the provided bitmap. - /// @param context The Impeller graphics context. - /// @param bitmap A bitmap containg the image to be uploaded. - /// @param create_mips Whether mipmaps should be generated for the given - /// image. - /// @param gpu_disabled_switch Whether the GPU is available for mipmap - /// creation. - /// @return A DlImage. - static std::pair, std::string> UploadTextureToStorage( + /// Only call this method if the GPU is available. + static std::pair, std::string> UnsafeUploadTextureToPrivate( const std::shared_ptr& context, - std::shared_ptr bitmap, - const std::shared_ptr& gpu_disabled_switch, - impeller::StorageMode storage_mode, + const std::shared_ptr& buffer, + const SkImageInfo& image_info, + const std::optional& resize_info, bool create_mips = true); private: diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 44fa5720f8316..941886f59d54a 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -354,13 +354,6 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { result_image = nullptr; error_message = ""; - - ImageDecoderImpeller::UploadTextureToStorage( - cb, no_gpu_access_context, bitmap, gpu_disabled_switch, - impeller::StorageMode::kHostVisible, true); - - EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); - EXPECT_EQ(result.second, ""); } TEST_F(ImageDecoderFixtureTest, ImpellerNullColorspace) { diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 2a7e3111f2294..d5dd8828b335e 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -9,8 +9,12 @@ #include "flutter/fml/make_copyable.h" #include "flutter/lib/ui/painting/display_list_image_gpu.h" #include "flutter/lib/ui/painting/image.h" +#include "impeller/core/device_buffer.h" +#include "impeller/core/device_buffer_descriptor.h" +#include "impeller/core/formats.h" #if IMPELLER_SUPPORTS_RENDERING #include "flutter/lib/ui/painting/image_decoder_impeller.h" +#include "impeller/renderer/context.h" #endif // IMPELLER_SUPPORTS_RENDERING #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/skia/include/codec/SkCodecAnimation.h" @@ -144,13 +148,40 @@ MultiFrameCodec::State::GetNextFrameImage( #if IMPELLER_SUPPORTS_RENDERING 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::UploadTextureToStorage( - impeller_context, std::make_shared(bitmap), - std::make_shared(), - impeller::StorageMode::kHostVisible, - /*create_mips=*/false); + sk_sp dl_image; + std::string decode_error; + gpu_disable_sync_switch->Execute( + fml::SyncSwitch::Handlers() + .SetIfTrue([&decode_error] { + // Textures cannot be uploaded with no GPU context on iOS. + decode_error = "No GPU context for multi-frame codec upload"; + }) + .SetIfFalse([&dl_image, &bitmap, &decode_error, &impeller_context] { + impeller::DeviceBufferDescriptor desc; + desc.storage_mode = impeller::StorageMode::kHostVisible; + desc.size = bitmap.computeByteSize(); + auto buffer = + impeller_context->GetResourceAllocator()->CreateBuffer(desc); + if (!buffer) { + decode_error = "Failed to allocate device buffer for upload."; + return; + } + if (!buffer->CopyHostBuffer( + static_cast(bitmap.getAddr(0, 0)), + {0, desc.size})) { + decode_error = "Failed to copy staging buffer for upload."; + return; + } + std::tie(dl_image, decode_error) = + ImageDecoderImpeller::UnsafeUploadTextureToPrivate( + impeller_context, // + buffer, // + bitmap.info(), // + /*resize_info=*/std::nullopt, // + /*create_mips=*/false // + ); + })); + return std::make_pair(dl_image, decode_error); } #endif // IMPELLER_SUPPORTS_RENDERING From 29ff35d8b89ef0078e87288956a7b5237ca949d9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 18:09:11 -0700 Subject: [PATCH 07/27] ++ --- lib/ui/painting/image_decoder_unittests.cc | 4 ++-- shell/common/snapshot_controller_impeller.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 941886f59d54a..9403fe6ce907d 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -341,8 +341,8 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { auto cb = [&result_image, &error_message](sk_sp image, std::string message) { - result_image = image; - error_message = message; + result_image = std::move(image); + error_message = std::move(message); }; ImageDecoderImpeller::UploadTextureToPrivate( diff --git a/shell/common/snapshot_controller_impeller.cc b/shell/common/snapshot_controller_impeller.cc index a6c3a8e2962cb..8aea6ecb85a70 100644 --- a/shell/common/snapshot_controller_impeller.cc +++ b/shell/common/snapshot_controller_impeller.cc @@ -137,7 +137,7 @@ void SnapshotControllerImpeller::MakeRasterSnapshot( if (context) { context->GetContext()->StoreTaskForGPU( [context, sync_switch, display_list = std::move(display_list), - picture_size, callback = std::move(callback)] { + picture_size, callback] { callback(DoMakeRasterSnapshot(display_list, picture_size, sync_switch, context)); }, From 4522d3d5b6b2decfdc51a832733a24d24c73efd3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 20:01:54 -0700 Subject: [PATCH 08/27] image adjustments. --- .../renderer/backend/metal/context_mtl.mm | 1 - impeller/renderer/context.h | 2 +- lib/ui/painting/image_decoder_unittests.cc | 27 ++++++++++--------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index ccb1e2262c3f6..9170a644620a8 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -400,7 +400,6 @@ new ContextMTL(device, command_queue, : parent_(parent) {} void ContextMTL::SyncSwitchObserver::OnSyncSwitchUpdate(bool new_is_disabled) { - FML_LOG(ERROR) << "Flush"; if (!new_is_disabled) { parent_.FlushTasksAwaitingGPU(); } diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index eaa9b6b642b00..5b737f82c6239 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -184,7 +184,7 @@ class Context { /// `task` will be executed on the platform thread. virtual void StoreTaskForGPU(const std::function& task, const std::function& failure) { - FML_CHECK(false && "not supported in this context"); + // nop } /// Run backend specific additional setup and create common shader variants. diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 9403fe6ce907d..6b25c1515e7d0 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -715,8 +715,8 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto data = flutter::testing::OpenFixtureAsSkData("Horizontal.jpg"); auto image = SkImages::DeferredFromEncodedData(data); ASSERT_TRUE(image != nullptr); - ASSERT_EQ(600, image->width()); - ASSERT_EQ(200, image->height()); + EXPECT_EQ(600, image->width()); + EXPECT_EQ(200, image->height()); ImageGeneratorRegistry registry; std::shared_ptr generator = @@ -727,24 +727,26 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { std::move(generator)); auto compressed_image = ImageDecoderSkia::ImageFromCompressedData( descriptor.get(), 6, 2, fml::tracing::TraceFlow("")); - ASSERT_EQ(compressed_image->width(), 6); - ASSERT_EQ(compressed_image->height(), 2); - ASSERT_EQ(compressed_image->alphaType(), kOpaque_SkAlphaType); + EXPECT_EQ(compressed_image->width(), 6); + EXPECT_EQ(compressed_image->height(), 2); + EXPECT_EQ(compressed_image->alphaType(), kOpaque_SkAlphaType); #if IMPELLER_SUPPORTS_RENDERING + // Bitmap sizes reflect the original image size as resizing is done on the + // GPU. std::shared_ptr allocator = std::make_shared(); auto result_1 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(6, 2), {100, 100}, /*supports_wide_gamut=*/false, allocator); - ASSERT_EQ(result_1.sk_bitmap->width(), 6); - ASSERT_EQ(result_1.sk_bitmap->height(), 2); + EXPECT_EQ(result_1.sk_bitmap->width(), 6); + EXPECT_EQ(result_1.sk_bitmap->height(), 2); auto result_2 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(60, 20), {10, 10}, /*supports_wide_gamut=*/false, allocator); - ASSERT_EQ(result_2.sk_bitmap->width(), 10); - ASSERT_EQ(result_2.sk_bitmap->height(), 10); + EXPECT_EQ(result_2.sk_bitmap->width(), 10); + EXPECT_EQ(result_2.sk_bitmap->height(), 10); #endif // IMPELLER_SUPPORTS_RENDERING } @@ -971,8 +973,7 @@ TEST_F(ImageDecoderFixtureTest, MultiFrameCodecDidAccessGpuDisabledSyncSwitch) { PostTaskSync(runners.GetIOTaskRunner(), [&]() { io_manager.reset(); }); } -TEST_F(ImageDecoderFixtureTest, - MultiFrameCodecProducesATextureEvenIfGPUIsDisabledOnImpeller) { +TEST_F(ImageDecoderFixtureTest, MultiFrameCodecIsPausedWhenGPUIsUnavailable) { auto settings = CreateSettingsForFixture(); settings.enable_impeller = true; auto vm_ref = DartVMRef::Create(settings); @@ -1030,14 +1031,14 @@ TEST_F(ImageDecoderFixtureTest, Dart_GetField(library, Dart_NewStringFromCString("frameCallback")); if (Dart_IsError(closure) || !Dart_IsClosure(closure)) { isolate_latch.Signal(); - return false; + return true; } EXPECT_FALSE(io_manager->did_access_is_gpu_disabled_sync_switch_); codec = fml::MakeRefCounted(std::move(gif_generator)); codec->getNextFrame(closure); isolate_latch.Signal(); - return true; + return false; })); isolate_latch.Wait(); }); From 3163cedd70ab3e6c8ee10e9851a8277ec9c66fa8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 21:50:38 -0700 Subject: [PATCH 09/27] ++ --- impeller/renderer/context.h | 2 +- lib/ui/painting/image_decoder_impeller.cc | 53 ++++++++++++ lib/ui/painting/image_decoder_impeller.h | 8 ++ lib/ui/painting/image_decoder_unittests.cc | 98 ++++++++++++++++++++-- lib/ui/painting/multi_frame_codec.cc | 42 +--------- 5 files changed, 155 insertions(+), 48 deletions(-) diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index 5b737f82c6239..eaa9b6b642b00 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -184,7 +184,7 @@ class Context { /// `task` will be executed on the platform thread. virtual void StoreTaskForGPU(const std::function& task, const std::function& failure) { - // nop + FML_CHECK(false && "not supported in this context"); } /// Run backend specific additional setup and create common shader variants. diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 5685d8854c00f..edb0d1dfe79a7 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -15,6 +15,7 @@ #include "flutter/impeller/renderer/context.h" #include "impeller/base/strings.h" #include "impeller/core/device_buffer.h" +#include "impeller/core/formats.h" #include "impeller/display_list/skia_conversions.h" #include "impeller/geometry/size.h" #include "third_party/skia/include/core/SkAlphaType.h" @@ -391,6 +392,58 @@ void ImageDecoderImpeller::UploadTextureToPrivate( })); } +std::pair, std::string> +ImageDecoderImpeller::UploadTextureToStorage( + const std::shared_ptr& context, + std::shared_ptr bitmap) { + TRACE_EVENT0("impeller", __FUNCTION__); + if (!context) { + return std::make_pair(nullptr, "No Impeller context is available"); + } + if (!bitmap) { + return std::make_pair(nullptr, "No texture bitmap is available"); + } + const auto image_info = bitmap->info(); + const auto pixel_format = + impeller::skia_conversions::ToPixelFormat(image_info.colorType()); + if (!pixel_format) { + std::string decode_error(impeller::SPrintF( + "Unsupported pixel format (SkColorType=%d)", image_info.colorType())); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); + } + + impeller::TextureDescriptor texture_descriptor; + texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate; + texture_descriptor.format = pixel_format.value(); + texture_descriptor.size = {image_info.width(), image_info.height()}; + texture_descriptor.mip_count = 1; + + auto texture = + context->GetResourceAllocator()->CreateTexture(texture_descriptor); + if (!texture) { + std::string decode_error("Could not create Impeller texture."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); + } + + auto mapping = std::make_shared( + reinterpret_cast(bitmap->getAddr(0, 0)), // data + texture_descriptor.GetByteSizeOfBaseMipLevel(), // size + [bitmap](auto, auto) mutable { bitmap.reset(); } // proc + ); + + if (!texture->SetContents(mapping)) { + std::string decode_error("Could not copy contents into Impeller texture."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); + } + + texture->SetLabel(impeller::SPrintF("ui.Image(%p)", texture.get()).c_str()); + return std::make_pair(impeller::DlImageImpeller::Make(std::move(texture)), + std::string()); +} + // |ImageDecoder| void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, uint32_t target_width, diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index 9da83923f03e2..4bcf7996b2924 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -89,6 +89,14 @@ class ImageDecoderImpeller final : public ImageDecoder { const std::shared_ptr& gpu_disabled_switch, bool create_mips = true); + /// @brief Create a texture from the provided bitmap. + /// @param context The Impeller graphics context. + /// @param bitmap A bitmap containg the image to be uploaded. + /// @return A DlImage. + static std::pair, std::string> UploadTextureToStorage( + const std::shared_ptr& context, + std::shared_ptr bitmap); + /// Only call this method if the GPU is available. static std::pair, std::string> UnsafeUploadTextureToPrivate( const std::shared_ptr& context, diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 6b25c1515e7d0..5a86432f3f8d3 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -76,11 +76,32 @@ class TestImpellerContext : public impeller::Context { return nullptr; } + void StoreTaskForGPU(const std::function& task, + const std::function& failure) override { + tasks_.push_back(PendingTask{task, failure}); + } + + void FlushTasks(bool fail = false) { + for (auto& task : tasks_) { + if (fail) { + task.task(); + } else { + task.failure(); + } + } + tasks_.clear(); + } + void Shutdown() override {} mutable size_t command_buffer_count_ = 0; private: + struct PendingTask { + std::function task; + std::function failure; + }; + std::vector tasks_; std::shared_ptr capabilities_; }; @@ -336,13 +357,39 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { desc.size = bitmap->computeByteSize(); auto buffer = std::make_shared(desc); - std::string error_message; - sk_sp result_image; + bool invoked = false; + auto cb = [&invoked](sk_sp image, std::string message) { + invoked = true; + }; + + ImageDecoderImpeller::UploadTextureToPrivate( + cb, no_gpu_access_context, buffer, info, bitmap, std::nullopt, + gpu_disabled_switch); + + EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); + EXPECT_FALSE(invoked); + + auto result = ImageDecoderImpeller::UploadTextureToStorage( + no_gpu_access_context, bitmap); - auto cb = [&result_image, &error_message](sk_sp image, - std::string message) { - result_image = std::move(image); - error_message = std::move(message); + ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); + ASSERT_EQ(result.second, ""); +} + +TEST_F(ImageDecoderFixtureTest, + ImpellerUploadToSharedNoGpuTaskFlushingSuccess) { +#if !IMPELLER_SUPPORTS_RENDERING + GTEST_SKIP() << "Impeller only test."; +#endif // IMPELLER_SUPPORTS_RENDERING + + sk_sp image; + std::string message; + bool invoked = false; + auto cb = [&invoked, &image, &message](sk_sp p_image, + std::string p_message) { + invoked = true; + image = p_image; + message = p_message; }; ImageDecoderImpeller::UploadTextureToPrivate( @@ -350,10 +397,43 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { gpu_disabled_switch); EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); - EXPECT_EQ(error_message, ""); + EXPECT_FALSE(invoked); + + no_gpu_access_context->FlushTasks(); + + EXPECT_TRUE(invoked); + EXPECT_EQ(message, ""); + EXPECT_NE(image, nullptr); +} + +TEST_F(ImageDecoderFixtureTest, + ImpellerUploadToSharedNoGpuTaskFlushingFailure) { +#if !IMPELLER_SUPPORTS_RENDERING + GTEST_SKIP() << "Impeller only test."; +#endif // IMPELLER_SUPPORTS_RENDERING + + sk_sp image; + std::string message; + bool invoked = false; + auto cb = [&invoked, &image, &message](sk_sp p_image, + std::string p_message) { + invoked = true; + image = p_image; + message = p_message; + }; + + ImageDecoderImpeller::UploadTextureToPrivate( + cb, no_gpu_access_context, buffer, info, bitmap, std::nullopt, + gpu_disabled_switch); + + EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); + EXPECT_FALSE(invoked); + + no_gpu_access_context->FlushTasks(/*fail*/ = true); - result_image = nullptr; - error_message = ""; + EXPECT_TRUE(invoked); + EXPECT_EQ(message, ""); + EXPECT_NE(image, nullptr); } TEST_F(ImageDecoderFixtureTest, ImpellerNullColorspace) { diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index d5dd8828b335e..1521c535fd97c 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -9,12 +9,8 @@ #include "flutter/fml/make_copyable.h" #include "flutter/lib/ui/painting/display_list_image_gpu.h" #include "flutter/lib/ui/painting/image.h" -#include "impeller/core/device_buffer.h" -#include "impeller/core/device_buffer_descriptor.h" -#include "impeller/core/formats.h" #if IMPELLER_SUPPORTS_RENDERING #include "flutter/lib/ui/painting/image_decoder_impeller.h" -#include "impeller/renderer/context.h" #endif // IMPELLER_SUPPORTS_RENDERING #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/skia/include/codec/SkCodecAnimation.h" @@ -148,40 +144,10 @@ MultiFrameCodec::State::GetNextFrameImage( #if IMPELLER_SUPPORTS_RENDERING if (is_impeller_enabled_) { - sk_sp dl_image; - std::string decode_error; - gpu_disable_sync_switch->Execute( - fml::SyncSwitch::Handlers() - .SetIfTrue([&decode_error] { - // Textures cannot be uploaded with no GPU context on iOS. - decode_error = "No GPU context for multi-frame codec upload"; - }) - .SetIfFalse([&dl_image, &bitmap, &decode_error, &impeller_context] { - impeller::DeviceBufferDescriptor desc; - desc.storage_mode = impeller::StorageMode::kHostVisible; - desc.size = bitmap.computeByteSize(); - auto buffer = - impeller_context->GetResourceAllocator()->CreateBuffer(desc); - if (!buffer) { - decode_error = "Failed to allocate device buffer for upload."; - return; - } - if (!buffer->CopyHostBuffer( - static_cast(bitmap.getAddr(0, 0)), - {0, desc.size})) { - decode_error = "Failed to copy staging buffer for upload."; - return; - } - std::tie(dl_image, decode_error) = - ImageDecoderImpeller::UnsafeUploadTextureToPrivate( - impeller_context, // - buffer, // - bitmap.info(), // - /*resize_info=*/std::nullopt, // - /*create_mips=*/false // - ); - })); - return std::make_pair(dl_image, decode_error); + // 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::UploadTextureToStorage( + impeller_context, std::make_shared(bitmap)); } #endif // IMPELLER_SUPPORTS_RENDERING From 2091507a28017883212bbdd7e12f3dbd244a369b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 22:05:01 -0700 Subject: [PATCH 10/27] ++ --- 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 edb0d1dfe79a7..5f120c331d1fa 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -414,7 +414,7 @@ ImageDecoderImpeller::UploadTextureToStorage( } impeller::TextureDescriptor texture_descriptor; - texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate; + texture_descriptor.storage_mode = impeller::StorageMode::kHostVisible; texture_descriptor.format = pixel_format.value(); texture_descriptor.size = {image_info.width(), image_info.height()}; texture_descriptor.mip_count = 1; From df0bd0a56eb27a9e862d3c37e14d11cb92bf7d0c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 18 Aug 2024 22:46:03 -0700 Subject: [PATCH 11/27] ++ --- lib/ui/painting/image_decoder_unittests.cc | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 5a86432f3f8d3..609f60348696f 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -382,6 +382,18 @@ TEST_F(ImageDecoderFixtureTest, GTEST_SKIP() << "Impeller only test."; #endif // IMPELLER_SUPPORTS_RENDERING + auto no_gpu_access_context = + std::make_shared(); + auto gpu_disabled_switch = std::make_shared(true); + + auto info = SkImageInfo::Make(10, 10, SkColorType::kRGBA_8888_SkColorType, + SkAlphaType::kPremul_SkAlphaType); + auto bitmap = std::make_shared(); + bitmap->allocPixels(info, 10 * 4); + impeller::DeviceBufferDescriptor desc; + desc.size = bitmap->computeByteSize(); + auto buffer = std::make_shared(desc); + sk_sp image; std::string message; bool invoked = false; @@ -412,6 +424,18 @@ TEST_F(ImageDecoderFixtureTest, GTEST_SKIP() << "Impeller only test."; #endif // IMPELLER_SUPPORTS_RENDERING + auto no_gpu_access_context = + std::make_shared(); + auto gpu_disabled_switch = std::make_shared(true); + + auto info = SkImageInfo::Make(10, 10, SkColorType::kRGBA_8888_SkColorType, + SkAlphaType::kPremul_SkAlphaType); + auto bitmap = std::make_shared(); + bitmap->allocPixels(info, 10 * 4); + impeller::DeviceBufferDescriptor desc; + desc.size = bitmap->computeByteSize(); + auto buffer = std::make_shared(desc); + sk_sp image; std::string message; bool invoked = false; From baec16053b24c4174f1c5987ded72f44a69be0f4 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Sun, 18 Aug 2024 23:11:20 -0700 Subject: [PATCH 12/27] Update image_decoder_unittests.cc --- lib/ui/painting/image_decoder_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 609f60348696f..c971292aac705 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -453,7 +453,7 @@ TEST_F(ImageDecoderFixtureTest, EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); EXPECT_FALSE(invoked); - no_gpu_access_context->FlushTasks(/*fail*/ = true); + no_gpu_access_context->FlushTasks(/*fail=*/true); EXPECT_TRUE(invoked); EXPECT_EQ(message, ""); From 63930c0a621e81cb2325c00d340620bb3f7b396f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 08:58:20 -0700 Subject: [PATCH 13/27] ++ --- lib/ui/painting/image_decoder_unittests.cc | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index c971292aac705..325bc04fec11d 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -358,7 +358,7 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { auto buffer = std::make_shared(desc); bool invoked = false; - auto cb = [&invoked](sk_sp image, std::string message) { + auto cb = [&invoked](const sk_sp& image, const std::string& message) { invoked = true; }; @@ -400,8 +400,8 @@ TEST_F(ImageDecoderFixtureTest, auto cb = [&invoked, &image, &message](sk_sp p_image, std::string p_message) { invoked = true; - image = p_image; - message = p_message; + image = std::move(p_image); + message = std::move(p_message); }; ImageDecoderImpeller::UploadTextureToPrivate( @@ -442,8 +442,8 @@ TEST_F(ImageDecoderFixtureTest, auto cb = [&invoked, &image, &message](sk_sp p_image, std::string p_message) { invoked = true; - image = p_image; - message = p_message; + image = std::move(p_image); + message = std::move(p_message); }; ImageDecoderImpeller::UploadTextureToPrivate( @@ -456,8 +456,8 @@ TEST_F(ImageDecoderFixtureTest, no_gpu_access_context->FlushTasks(/*fail=*/true); EXPECT_TRUE(invoked); - EXPECT_EQ(message, ""); - EXPECT_NE(image, nullptr); + // Creation of the dl image will still fail with the mocked context. + EXPECT_NE(message, "Could not create command buffer for mipmap generation."); } TEST_F(ImageDecoderFixtureTest, ImpellerNullColorspace) { @@ -843,14 +843,14 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto result_1 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(6, 2), {100, 100}, /*supports_wide_gamut=*/false, allocator); - EXPECT_EQ(result_1.sk_bitmap->width(), 6); - EXPECT_EQ(result_1.sk_bitmap->height(), 2); + EXPECT_EQ(result_1.sk_bitmap->width(), 75); + EXPECT_EQ(result_1.sk_bitmap->height(), 25); auto result_2 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(60, 20), {10, 10}, /*supports_wide_gamut=*/false, allocator); - EXPECT_EQ(result_2.sk_bitmap->width(), 10); - EXPECT_EQ(result_2.sk_bitmap->height(), 10); + EXPECT_EQ(result_2.sk_bitmap->width(), 75); + EXPECT_EQ(result_2.sk_bitmap->height(), 25); #endif // IMPELLER_SUPPORTS_RENDERING } @@ -1135,14 +1135,14 @@ TEST_F(ImageDecoderFixtureTest, MultiFrameCodecIsPausedWhenGPUIsUnavailable) { Dart_GetField(library, Dart_NewStringFromCString("frameCallback")); if (Dart_IsError(closure) || !Dart_IsClosure(closure)) { isolate_latch.Signal(); - return true; + return false; } EXPECT_FALSE(io_manager->did_access_is_gpu_disabled_sync_switch_); codec = fml::MakeRefCounted(std::move(gif_generator)); codec->getNextFrame(closure); isolate_latch.Signal(); - return false; + return true; })); isolate_latch.Wait(); }); From c90fec8bace14828749eed0bf9d1abcde9dd226a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 09:05:42 -0700 Subject: [PATCH 14/27] ++ --- lib/ui/painting/image_decoder_unittests.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 325bc04fec11d..54f81c2f648b0 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -358,9 +358,8 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { auto buffer = std::make_shared(desc); bool invoked = false; - auto cb = [&invoked](const sk_sp& image, const std::string& message) { - invoked = true; - }; + auto cb = [&invoked](const sk_sp& image, + const std::string& message) { invoked = true; }; ImageDecoderImpeller::UploadTextureToPrivate( cb, no_gpu_access_context, buffer, info, bitmap, std::nullopt, From 003f13ad1d6d0dd0d49048e3c199656d7eaba5bb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 09:27:43 -0700 Subject: [PATCH 15/27] ++ --- lib/ui/painting/image_decoder_unittests.cc | 44 +--------------------- 1 file changed, 1 insertion(+), 43 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 54f81c2f648b0..9abe391fa5e94 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -375,48 +375,6 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { ASSERT_EQ(result.second, ""); } -TEST_F(ImageDecoderFixtureTest, - ImpellerUploadToSharedNoGpuTaskFlushingSuccess) { -#if !IMPELLER_SUPPORTS_RENDERING - GTEST_SKIP() << "Impeller only test."; -#endif // IMPELLER_SUPPORTS_RENDERING - - auto no_gpu_access_context = - std::make_shared(); - auto gpu_disabled_switch = std::make_shared(true); - - auto info = SkImageInfo::Make(10, 10, SkColorType::kRGBA_8888_SkColorType, - SkAlphaType::kPremul_SkAlphaType); - auto bitmap = std::make_shared(); - bitmap->allocPixels(info, 10 * 4); - impeller::DeviceBufferDescriptor desc; - desc.size = bitmap->computeByteSize(); - auto buffer = std::make_shared(desc); - - sk_sp image; - std::string message; - bool invoked = false; - auto cb = [&invoked, &image, &message](sk_sp p_image, - std::string p_message) { - invoked = true; - image = std::move(p_image); - message = std::move(p_message); - }; - - ImageDecoderImpeller::UploadTextureToPrivate( - cb, no_gpu_access_context, buffer, info, bitmap, std::nullopt, - gpu_disabled_switch); - - EXPECT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); - EXPECT_FALSE(invoked); - - no_gpu_access_context->FlushTasks(); - - EXPECT_TRUE(invoked); - EXPECT_EQ(message, ""); - EXPECT_NE(image, nullptr); -} - TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpuTaskFlushingFailure) { #if !IMPELLER_SUPPORTS_RENDERING @@ -456,7 +414,7 @@ TEST_F(ImageDecoderFixtureTest, EXPECT_TRUE(invoked); // Creation of the dl image will still fail with the mocked context. - EXPECT_NE(message, "Could not create command buffer for mipmap generation."); + EXPECT_NE(message, ""); } TEST_F(ImageDecoderFixtureTest, ImpellerNullColorspace) { From f6d534074bde8d368bbfd692389b23e14b7a46d4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 10:06:55 -0700 Subject: [PATCH 16/27] flush in test. --- lib/ui/painting/image_decoder_unittests.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 9abe391fa5e94..2f3b5ef580218 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -373,6 +373,8 @@ TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); ASSERT_EQ(result.second, ""); + + no_gpu_access_context->FlushTasks(/*fail=*/true); } TEST_F(ImageDecoderFixtureTest, From b0712bd9473f803466b27fcd98e4214a6a00e43e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 10:54:34 -0700 Subject: [PATCH 17/27] ++ --- lib/ui/painting/image_decoder_impeller.cc | 38 ++++++++++++++++++++++ lib/ui/painting/image_decoder_unittests.cc | 6 ++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 5f120c331d1fa..4b81852e2e012 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -235,6 +235,44 @@ DecompressResult ImageDecoderImpeller::DecompressTexture( bitmap->dimensions() == target_size ? std::nullopt : std::optional(image_info.makeDimensions(target_size)); + + if (source_size.width() > max_texture_size.width || + source_size.height() > max_texture_size.height) { + //---------------------------------------------------------------------------- + /// 2. If the decoded image isn't the requested target size and the src size + /// exceeds the device max texture size, perform a slow CPU reisze. + /// + TRACE_EVENT0("impeller", "SlowCPUDecodeScale"); + const auto scaled_image_info = image_info.makeDimensions(target_size); + + auto scaled_bitmap = std::make_shared(); + auto scaled_allocator = std::make_shared(allocator); + scaled_bitmap->setInfo(scaled_image_info); + if (!scaled_bitmap->tryAllocPixels(scaled_allocator.get())) { + std::string decode_error( + "Could not allocate scaled bitmap for image decompression."); + FML_DLOG(ERROR) << decode_error; + return DecompressResult{.decode_error = decode_error}; + } + if (!bitmap->pixmap().scalePixels( + scaled_bitmap->pixmap(), + SkSamplingOptions(SkFilterMode::kLinear, SkMipmapMode::kNone))) { + FML_LOG(ERROR) << "Could not scale decoded bitmap data."; + } + scaled_bitmap->setImmutable(); + + std::shared_ptr buffer = + scaled_allocator->GetDeviceBuffer(); + if (!buffer) { + return DecompressResult{.decode_error = "Unable to get device buffer"}; + } + buffer->Flush(); + + return DecompressResult{.device_buffer = std::move(buffer), + .sk_bitmap = scaled_bitmap, + .image_info = scaled_bitmap->info()}; + } + return DecompressResult{.device_buffer = std::move(buffer), .sk_bitmap = bitmap, .image_info = bitmap->info(), diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 2f3b5ef580218..ba1e8aaf3aa33 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -805,11 +805,13 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { EXPECT_EQ(result_1.sk_bitmap->width(), 75); EXPECT_EQ(result_1.sk_bitmap->height(), 25); + // Impeller still performs a CPU resize if the src size is larger than the + // max texture size. auto result_2 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(60, 20), {10, 10}, /*supports_wide_gamut=*/false, allocator); - EXPECT_EQ(result_2.sk_bitmap->width(), 75); - EXPECT_EQ(result_2.sk_bitmap->height(), 25); + EXPECT_EQ(result_2.sk_bitmap->width(), 10); + EXPECT_EQ(result_2.sk_bitmap->height(), 10); #endif // IMPELLER_SUPPORTS_RENDERING } From d0396501b6746e22fbe3666a0fa037da568dc4ab Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 11:50:32 -0700 Subject: [PATCH 18/27] use cmd blit image when resizing. --- .../renderer/backend/vulkan/blit_pass_vk.cc | 75 ++++++++++++++----- impeller/renderer/context.h | 5 +- lib/ui/painting/image_decoder_impeller.cc | 46 +++++------- lib/ui/painting/image_decoder_impeller.h | 18 ++--- lib/ui/painting/image_decoder_unittests.cc | 18 ++++- 5 files changed, 99 insertions(+), 63 deletions(-) diff --git a/impeller/renderer/backend/vulkan/blit_pass_vk.cc b/impeller/renderer/backend/vulkan/blit_pass_vk.cc index b21403955edc2..17c56960fca1e 100644 --- a/impeller/renderer/backend/vulkan/blit_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/blit_pass_vk.cc @@ -112,28 +112,63 @@ bool BlitPassVK::OnCopyTextureToTextureCommand( return false; } - vk::ImageCopy image_copy; + // If the source and destination are the same size then use copyImage, + // otherwise perform a blit with a linear filter. + if (source->GetSize() == destination->GetSize()) { + vk::ImageCopy image_copy; + + image_copy.setSrcSubresource( + vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); + image_copy.setDstSubresource( + vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); + + image_copy.srcOffset = + vk::Offset3D(source_region.GetX(), source_region.GetY(), 0); + image_copy.dstOffset = + vk::Offset3D(destination_origin.x, destination_origin.y, 0); + image_copy.extent = + vk::Extent3D(source_region.GetWidth(), source_region.GetHeight(), 1); + + // Issue the copy command now that the images are already in the right + // layouts. + cmd_buffer.copyImage(src.GetImage(), // + src_barrier.new_layout, // + dst.GetImage(), // + dst_barrier.new_layout, // + image_copy // + ); + } else { + vk::ImageBlit blit; + blit.srcSubresource.aspectMask = vk::ImageAspectFlagBits::eColor; + blit.srcSubresource.baseArrayLayer = 0u; + blit.srcSubresource.layerCount = 1u; + blit.srcSubresource.mipLevel = 0; - image_copy.setSrcSubresource( - vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); - image_copy.setDstSubresource( - vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); + blit.dstSubresource.aspectMask = vk::ImageAspectFlagBits::eColor; + blit.dstSubresource.baseArrayLayer = 0u; + blit.dstSubresource.layerCount = 1u; + blit.dstSubresource.mipLevel = 0; - image_copy.srcOffset = - vk::Offset3D(source_region.GetX(), source_region.GetY(), 0); - image_copy.dstOffset = - vk::Offset3D(destination_origin.x, destination_origin.y, 0); - image_copy.extent = - vk::Extent3D(source_region.GetWidth(), source_region.GetHeight(), 1); - - // Issue the copy command now that the images are already in the right - // layouts. - cmd_buffer.copyImage(src.GetImage(), // - src_barrier.new_layout, // - dst.GetImage(), // - dst_barrier.new_layout, // - image_copy // - ); + // offsets[0] is origin. + blit.srcOffsets[1].x = std::max(source->GetSize().width, 1u); + blit.srcOffsets[1].y = std::max(source->GetSize().height, 1u); + blit.srcOffsets[1].z = 1u; + + // offsets[0] is origin. + blit.dstOffsets[1].x = std::max(source->GetSize().width, 1u); + blit.dstOffsets[1].y = std::max(source->GetSize().height, 1u); + blit.dstOffsets[1].z = 1u; + + cmd_buffer.blitImage(src.GetImage(), // + src_barrier.new_layout, // + dst.GetImage(), // + dst_barrier.new_layout, // + 1, // + &blit, // + vk::Filter::eLinear + + ); + } // If this is an onscreen texture, do not transition the layout // back to shader read. diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index eaa9b6b642b00..7c05ce92264c3 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -8,6 +8,7 @@ #include #include +#include "fml/closure.h" #include "impeller/core/allocator.h" #include "impeller/core/formats.h" #include "impeller/renderer/capabilities.h" @@ -182,8 +183,8 @@ class Context { /// Threadsafe. /// /// `task` will be executed on the platform thread. - virtual void StoreTaskForGPU(const std::function& task, - const std::function& failure) { + virtual void StoreTaskForGPU(const fml::closure& task, + const fml::closure& failure) { FML_CHECK(false && "not supported in this context"); } diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 4b81852e2e012..e6d63e9b85ad2 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -285,12 +285,9 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( const std::shared_ptr& context, const std::shared_ptr& buffer, const SkImageInfo& image_info, - const std::optional& resize_info, - bool create_mips) { + const std::optional& resize_info) { const auto pixel_format = impeller::skia_conversions::ToPixelFormat(image_info.colorType()); - // mips should still be created if the source image is being resized. - const bool should_create_mips = create_mips || resize_info.has_value(); if (!pixel_format) { std::string decode_error(impeller::SPrintF( "Unsupported pixel format (SkColorType=%d)", image_info.colorType())); @@ -302,8 +299,7 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate; texture_descriptor.format = pixel_format.value(); texture_descriptor.size = {image_info.width(), image_info.height()}; - texture_descriptor.mip_count = - should_create_mips ? texture_descriptor.size.MipCount() : 1; + texture_descriptor.mip_count = texture_descriptor.size.MipCount(); texture_descriptor.compression_type = impeller::CompressionType::kLossy; auto dest_texture = @@ -346,7 +342,7 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( resize_desc.storage_mode = impeller::StorageMode::kDevicePrivate; resize_desc.format = pixel_format.value(); resize_desc.size = {resize_info->width(), resize_info->height()}; - resize_desc.mip_count = create_mips ? resize_desc.size.MipCount() : 1; + resize_desc.mip_count = resize_desc.size.MipCount(); resize_desc.compression_type = impeller::CompressionType::kLossy; auto resize_texture = @@ -367,7 +363,7 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( blit_pass->EncodeCommands(context->GetResourceAllocator()); if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) { - std::string decode_error("Failed to submit image deocding command buffer."); + std::string decode_error("Failed to submit image decoding command buffer."); FML_DLOG(ERROR) << decode_error; return std::make_pair(nullptr, decode_error); } @@ -384,8 +380,7 @@ void ImageDecoderImpeller::UploadTextureToPrivate( const SkImageInfo& image_info, const std::shared_ptr& bitmap, const std::optional& resize_info, - const std::shared_ptr& gpu_disabled_switch, - bool create_mips) { + const std::shared_ptr& gpu_disabled_switch) { TRACE_EVENT0("impeller", __FUNCTION__); if (!context) { result(nullptr, "No Impeller context is available"); @@ -398,29 +393,26 @@ void ImageDecoderImpeller::UploadTextureToPrivate( gpu_disabled_switch->Execute( fml::SyncSwitch::Handlers() - .SetIfFalse( - [&result, context, buffer, image_info, resize_info, create_mips] { - sk_sp image; - std::string decode_error; - std::tie(image, decode_error) = std::tie(image, decode_error) = - UnsafeUploadTextureToPrivate(context, buffer, image_info, - resize_info, create_mips); - result(image, decode_error); - }) - .SetIfTrue([&result, context, buffer, image_info, resize_info, - create_mips] { + .SetIfFalse([&result, context, buffer, image_info, resize_info] { + sk_sp image; + std::string decode_error; + std::tie(image, decode_error) = std::tie(image, decode_error) = + UnsafeUploadTextureToPrivate(context, buffer, image_info, + resize_info); + result(image, decode_error); + }) + .SetIfTrue([&result, context, buffer, image_info, resize_info] { // The `result` function must be copied in the capture list for each // closure or the stack allocated callback will be cleared by the // time to closure is executed later. context->StoreTaskForGPU( - [result, context, buffer, image_info, resize_info, - create_mips]() { + [result, context, buffer, image_info, resize_info]() { sk_sp image; std::string decode_error; - std::tie(image, decode_error) = std::tie(image, - decode_error) = - UnsafeUploadTextureToPrivate(context, buffer, image_info, - resize_info, create_mips); + std::tie(image, decode_error) = + std::tie(image, decode_error) = + UnsafeUploadTextureToPrivate(context, buffer, + image_info, resize_info); result(image, decode_error); }, [result]() { diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index 4bcf7996b2924..1d956a30a936c 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -86,8 +86,7 @@ class ImageDecoderImpeller final : public ImageDecoder { const SkImageInfo& image_info, const std::shared_ptr& bitmap, const std::optional& resize_info, - const std::shared_ptr& gpu_disabled_switch, - bool create_mips = true); + const std::shared_ptr& gpu_disabled_switch); /// @brief Create a texture from the provided bitmap. /// @param context The Impeller graphics context. @@ -97,20 +96,19 @@ class ImageDecoderImpeller final : public ImageDecoder { const std::shared_ptr& context, std::shared_ptr bitmap); - /// Only call this method if the GPU is available. - static std::pair, std::string> UnsafeUploadTextureToPrivate( - const std::shared_ptr& context, - const std::shared_ptr& buffer, - const SkImageInfo& image_info, - const std::optional& resize_info, - bool create_mips = true); - private: using FutureContext = std::shared_future>; FutureContext context_; const bool supports_wide_gamut_; std::shared_ptr gpu_disabled_switch_; + /// Only call this method if the GPU is available. + static std::pair, std::string> UnsafeUploadTextureToPrivate( + const std::shared_ptr& context, + const std::shared_ptr& buffer, + const SkImageInfo& image_info, + const std::optional& resize_info); + FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoderImpeller); }; diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index ba1e8aaf3aa33..0d99a0703cdff 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -796,17 +796,27 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { #if IMPELLER_SUPPORTS_RENDERING // Bitmap sizes reflect the original image size as resizing is done on the - // GPU. + // GPU if the src size is smaller than the max texture size. std::shared_ptr allocator = std::make_shared(); auto result_1 = ImageDecoderImpeller::DecompressTexture( - descriptor.get(), SkISize::Make(6, 2), {100, 100}, + descriptor.get(), SkISize::Make(6, 2), {1000, 1000}, /*supports_wide_gamut=*/false, allocator); EXPECT_EQ(result_1.sk_bitmap->width(), 75); EXPECT_EQ(result_1.sk_bitmap->height(), 25); - // Impeller still performs a CPU resize if the src size is larger than the - // max texture size. + // Bitmap sizes reflect the scaled size if the source size is larger than + // max texture size even if destination size isn't max texture size. + std::shared_ptr allocator = + std::make_shared(); + auto result_1 = ImageDecoderImpeller::DecompressTexture( + descriptor.get(), SkISize::Make(6, 2), {10, 10}, + /*supports_wide_gamut=*/false, allocator); + EXPECT_EQ(result_1.sk_bitmap->width(), 6); + EXPECT_EQ(result_1.sk_bitmap->height(), 2); + + // If the destination size is larger than the max texture size the image + // is scaled down. auto result_2 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(60, 20), {10, 10}, /*supports_wide_gamut=*/false, allocator); From f02dde42966b7eeccf3df73bed5126f18a54b26b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 11:55:31 -0700 Subject: [PATCH 19/27] fix dst size. --- impeller/renderer/backend/vulkan/blit_pass_vk.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/blit_pass_vk.cc b/impeller/renderer/backend/vulkan/blit_pass_vk.cc index 17c56960fca1e..6bc6fb041b1f9 100644 --- a/impeller/renderer/backend/vulkan/blit_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/blit_pass_vk.cc @@ -155,8 +155,8 @@ bool BlitPassVK::OnCopyTextureToTextureCommand( blit.srcOffsets[1].z = 1u; // offsets[0] is origin. - blit.dstOffsets[1].x = std::max(source->GetSize().width, 1u); - blit.dstOffsets[1].y = std::max(source->GetSize().height, 1u); + blit.dstOffsets[1].x = std::max(destination->GetSize().width, 1u); + blit.dstOffsets[1].y = std::max(destination->GetSize().height, 1u); blit.dstOffsets[1].z = 1u; cmd_buffer.blitImage(src.GetImage(), // From 42bf31ee12cc746a6a281a9b8c051ba5b4abd432 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 19 Aug 2024 13:26:11 -0700 Subject: [PATCH 20/27] ++ --- lib/ui/painting/image_decoder_unittests.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 0d99a0703cdff..d8f76671ae09b 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -807,21 +807,19 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { // Bitmap sizes reflect the scaled size if the source size is larger than // max texture size even if destination size isn't max texture size. - std::shared_ptr allocator = - std::make_shared(); - auto result_1 = ImageDecoderImpeller::DecompressTexture( + auto result_2 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(6, 2), {10, 10}, /*supports_wide_gamut=*/false, allocator); - EXPECT_EQ(result_1.sk_bitmap->width(), 6); - EXPECT_EQ(result_1.sk_bitmap->height(), 2); + EXPECT_EQ(result_2.sk_bitmap->width(), 6); + EXPECT_EQ(result_2.sk_bitmap->height(), 2); // If the destination size is larger than the max texture size the image // is scaled down. - auto result_2 = ImageDecoderImpeller::DecompressTexture( + auto result_3 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(60, 20), {10, 10}, /*supports_wide_gamut=*/false, allocator); - EXPECT_EQ(result_2.sk_bitmap->width(), 10); - EXPECT_EQ(result_2.sk_bitmap->height(), 10); + EXPECT_EQ(result_3.sk_bitmap->width(), 10); + EXPECT_EQ(result_3.sk_bitmap->height(), 10); #endif // IMPELLER_SUPPORTS_RENDERING } From 4d4b83c446dd9bbf80d66459ca4a89f5067c0cc2 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 20 Aug 2024 13:35:00 -0700 Subject: [PATCH 21/27] use MPS shader to resize on iOS. --- impeller/renderer/backend/metal/BUILD.gn | 5 ++- .../renderer/backend/metal/blit_pass_mtl.h | 3 +- .../renderer/backend/metal/blit_pass_mtl.mm | 43 ++++++++++++------- .../backend/metal/command_buffer_mtl.h | 4 +- .../backend/metal/command_buffer_mtl.mm | 7 ++- impeller/renderer/backend/metal/context_mtl.h | 9 ++-- .../renderer/backend/metal/context_mtl.mm | 6 +-- impeller/renderer/blit_pass_unittests.cc | 41 ++++++++++++++++++ lib/ui/painting/image_decoder_impeller.cc | 7 +++ 9 files changed, 97 insertions(+), 28 deletions(-) diff --git a/impeller/renderer/backend/metal/BUILD.gn b/impeller/renderer/backend/metal/BUILD.gn index 343330fc2ad23..9f7156da6cdc2 100644 --- a/impeller/renderer/backend/metal/BUILD.gn +++ b/impeller/renderer/backend/metal/BUILD.gn @@ -59,7 +59,10 @@ impeller_component("metal") { "//flutter/fml", ] - frameworks = [ "Metal.framework" ] + frameworks = [ + "Metal.framework", + "MetalPerformanceShaders.framework", + ] } impeller_component("metal_unittests") { diff --git a/impeller/renderer/backend/metal/blit_pass_mtl.h b/impeller/renderer/backend/metal/blit_pass_mtl.h index 1de0f45aedc3d..b0bfa04d2c27e 100644 --- a/impeller/renderer/backend/metal/blit_pass_mtl.h +++ b/impeller/renderer/backend/metal/blit_pass_mtl.h @@ -21,6 +21,7 @@ class BlitPassMTL final : public BlitPass { id encoder_ = nil; id buffer_ = nil; + id device_ = nil; bool is_valid_ = false; bool is_metal_trace_active_ = false; // Many parts of the codebase will start writing to a render pass but @@ -28,7 +29,7 @@ class BlitPassMTL final : public BlitPass { // so that in the dtor we can always ensure the render pass is finished. mutable bool did_finish_encoding_ = false; - explicit BlitPassMTL(id buffer); + explicit BlitPassMTL(id buffer, id device); // |BlitPass| bool IsValid() const override; diff --git a/impeller/renderer/backend/metal/blit_pass_mtl.mm b/impeller/renderer/backend/metal/blit_pass_mtl.mm index 329fe5197caf8..9b5ef2b9e321c 100644 --- a/impeller/renderer/backend/metal/blit_pass_mtl.mm +++ b/impeller/renderer/backend/metal/blit_pass_mtl.mm @@ -4,6 +4,7 @@ #include "impeller/renderer/backend/metal/blit_pass_mtl.h" #include +#import #include #include #include @@ -25,7 +26,8 @@ namespace impeller { -BlitPassMTL::BlitPassMTL(id buffer) : buffer_(buffer) { +BlitPassMTL::BlitPassMTL(id buffer, id device) + : buffer_(buffer), device_(device) { if (!buffer_) { return; } @@ -85,26 +87,35 @@ auto destination_origin_mtl = MTLOriginMake(destination_origin.x, destination_origin.y, 0); + if (source->GetSize() == destination->GetSize()) { #ifdef IMPELLER_DEBUG - if (is_metal_trace_active_) { - [encoder_ pushDebugGroup:@(label.c_str())]; - } + if (is_metal_trace_active_) { + [encoder_ pushDebugGroup:@(label.c_str())]; + } #endif // IMPELLER_DEBUG - [encoder_ copyFromTexture:source_mtl - sourceSlice:0 - sourceLevel:0 - sourceOrigin:source_origin_mtl - sourceSize:source_size_mtl - toTexture:destination_mtl - destinationSlice:0 - destinationLevel:0 - destinationOrigin:destination_origin_mtl]; + [encoder_ copyFromTexture:source_mtl + sourceSlice:0 + sourceLevel:0 + sourceOrigin:source_origin_mtl + sourceSize:source_size_mtl + toTexture:destination_mtl + destinationSlice:0 + destinationLevel:0 + destinationOrigin:destination_origin_mtl]; #ifdef IMPELLER_DEBUG - if (is_metal_trace_active_) { - [encoder_ popDebugGroup]; - } + if (is_metal_trace_active_) { + [encoder_ popDebugGroup]; + } #endif // IMPELLER_DEBUG + } else { + [encoder_ endEncoding]; + auto filter = [[MPSImageBilinearScale alloc] initWithDevice:device_]; + [filter encodeToCommandBuffer:buffer_ + sourceTexture:source_mtl + destinationTexture:destination_mtl]; + encoder_ = [buffer_ blitCommandEncoder]; + } return true; } diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.h b/impeller/renderer/backend/metal/command_buffer_mtl.h index 58a503654796e..dad6379bdb519 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.h +++ b/impeller/renderer/backend/metal/command_buffer_mtl.h @@ -20,9 +20,11 @@ class CommandBufferMTL final : public CommandBuffer { private: friend class ContextMTL; - id buffer_ = nullptr; + id buffer_ = nil; + id device_ = nil; CommandBufferMTL(const std::weak_ptr& context, + id device, id queue); // |CommandBuffer| diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.mm b/impeller/renderer/backend/metal/command_buffer_mtl.mm index 5909426cbb544..21e9873d9c138 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/command_buffer_mtl.mm @@ -128,8 +128,11 @@ static bool LogMTLCommandBufferErrorIfPresent(id buffer) { } CommandBufferMTL::CommandBufferMTL(const std::weak_ptr& context, + id device, id queue) - : CommandBuffer(context), buffer_(CreateCommandBuffer(queue)) {} + : CommandBuffer(context), + buffer_(CreateCommandBuffer(queue)), + device_(device) {} CommandBufferMTL::~CommandBufferMTL() = default; @@ -208,7 +211,7 @@ static bool LogMTLCommandBufferErrorIfPresent(id buffer) { return nullptr; } - auto pass = std::shared_ptr(new BlitPassMTL(buffer_)); + auto pass = std::shared_ptr(new BlitPassMTL(buffer_, device_)); if (!pass->IsValid()) { return nullptr; } diff --git a/impeller/renderer/backend/metal/context_mtl.h b/impeller/renderer/backend/metal/context_mtl.h index fbd818f10f5c3..dbc3806939fe6 100644 --- a/impeller/renderer/backend/metal/context_mtl.h +++ b/impeller/renderer/backend/metal/context_mtl.h @@ -12,6 +12,7 @@ #include "flutter/fml/concurrent_message_loop.h" #include "flutter/fml/synchronization/sync_switch.h" +#include "fml/closure.h" #include "impeller/base/backend_cast.h" #include "impeller/core/sampler.h" #include "impeller/renderer/backend/metal/allocator_mtl.h" @@ -136,8 +137,8 @@ class ContextMTL final : public Context, #endif // IMPELLER_DEBUG // |Context| - void StoreTaskForGPU(const std::function& task, - const std::function& failure) override; + void StoreTaskForGPU(const fml::closure& task, + const fml::closure& failure) override; private: class SyncSwitchObserver : public fml::SyncSwitch::Observer { @@ -151,8 +152,8 @@ class ContextMTL final : public Context, }; struct PendingTasks { - std::function task; - std::function failure; + fml::closure task; + fml::closure failure; }; id device_ = nullptr; diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 9170a644620a8..fdd3202edd67f 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -338,7 +338,7 @@ new ContextMTL(device, command_queue, } auto buffer = std::shared_ptr( - new CommandBufferMTL(weak_from_this(), queue)); + new CommandBufferMTL(weak_from_this(), device_, queue)); if (!buffer->IsValid()) { return nullptr; } @@ -377,8 +377,8 @@ new ContextMTL(device, command_queue, return buffer; } -void ContextMTL::StoreTaskForGPU(const std::function& task, - const std::function& failure) { +void ContextMTL::StoreTaskForGPU(const fml::closure& task, + const fml::closure& failure) { tasks_awaiting_gpu_.push_back(PendingTasks{task, failure}); while (tasks_awaiting_gpu_.size() > kMaxTasksAwaitingGPU) { PendingTasks front = std::move(tasks_awaiting_gpu_.front()); diff --git a/impeller/renderer/blit_pass_unittests.cc b/impeller/renderer/blit_pass_unittests.cc index b99a3d0aff443..60b47885ae2a4 100644 --- a/impeller/renderer/blit_pass_unittests.cc +++ b/impeller/renderer/blit_pass_unittests.cc @@ -2,8 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include +#include "fml/mapping.h" #include "gtest/gtest.h" #include "impeller/base/validation.h" +#include "impeller/core/device_buffer.h" #include "impeller/core/device_buffer_descriptor.h" #include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" @@ -130,5 +133,43 @@ TEST_P(BlitPassTest, CanBlitSmallRegionToUninitializedTexture) { EXPECT_TRUE(context->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok()); } +TEST_P(BlitPassTest, CanResizeTextures) { + auto context = GetContext(); + auto cmd_buffer = context->CreateCommandBuffer(); + auto blit_pass = cmd_buffer->CreateBlitPass(); + + TextureDescriptor dst_format; + dst_format.storage_mode = StorageMode::kDevicePrivate; + dst_format.format = PixelFormat::kR8G8B8A8UNormInt; + dst_format.size = {10, 10}; + auto dst = context->GetResourceAllocator()->CreateTexture(dst_format); + + TextureDescriptor src_format; + src_format.storage_mode = StorageMode::kDevicePrivate; + src_format.format = PixelFormat::kR8G8B8A8UNormInt; + src_format.size = {100, 100}; + auto src = context->GetResourceAllocator()->CreateTexture(src_format); + + std::vector bytes(src_format.GetByteSizeOfBaseMipLevel()); + for (auto i = 0u; i < src_format.GetByteSizeOfBaseMipLevel(); i += 4) { + // RGBA + bytes[i + 0] = 255; + bytes[i + 1] = 0; + bytes[i + 2] = 0; + bytes[i + 3] = 255; + } + auto mapping = fml::DataMapping(bytes); + auto staging = context->GetResourceAllocator()->CreateBufferWithCopy(mapping); + + ASSERT_TRUE(dst); + ASSERT_TRUE(src); + ASSERT_TRUE(staging); + + EXPECT_TRUE(blit_pass->AddCopy(DeviceBuffer::AsBufferView(staging), src)); + EXPECT_TRUE(blit_pass->AddCopy(src, dst)); + EXPECT_TRUE(blit_pass->EncodeCommands(GetContext()->GetResourceAllocator())); + EXPECT_TRUE(context->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok()); +} + } // namespace testing } // namespace impeller diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index e6d63e9b85ad2..6f3b6e1fb5cd9 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -16,6 +16,7 @@ #include "impeller/base/strings.h" #include "impeller/core/device_buffer.h" #include "impeller/core/formats.h" +#include "impeller/core/texture_descriptor.h" #include "impeller/display_list/skia_conversions.h" #include "impeller/geometry/size.h" #include "third_party/skia/include/core/SkAlphaType.h" @@ -344,6 +345,12 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( resize_desc.size = {resize_info->width(), resize_info->height()}; resize_desc.mip_count = resize_desc.size.MipCount(); resize_desc.compression_type = impeller::CompressionType::kLossy; + resize_desc.usage = impeller::TextureUsage::kShaderRead; + if (context->GetBackendType() == impeller::Context::BackendType::kMetal) { + // Resizing requires a MPS on Metal platforms. + resize_desc.usage |= impeller::TextureUsage::kShaderWrite; + resize_desc.compression_type = impeller::CompressionType::kLossless; + } auto resize_texture = context->GetResourceAllocator()->CreateTexture(resize_desc); From 869a9dc807825cf0ad2cc46ce3e76bb657ebc8cb Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 20 Aug 2024 14:21:32 -0700 Subject: [PATCH 22/27] ++ --- impeller/renderer/backend/metal/blit_pass_mtl.mm | 2 +- impeller/renderer/backend/vulkan/blit_pass_vk.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/metal/blit_pass_mtl.mm b/impeller/renderer/backend/metal/blit_pass_mtl.mm index 9b5ef2b9e321c..856baad4ba513 100644 --- a/impeller/renderer/backend/metal/blit_pass_mtl.mm +++ b/impeller/renderer/backend/metal/blit_pass_mtl.mm @@ -87,7 +87,7 @@ auto destination_origin_mtl = MTLOriginMake(destination_origin.x, destination_origin.y, 0); - if (source->GetSize() == destination->GetSize()) { + if (source_region.GetSize() == destination->GetSize()) { #ifdef IMPELLER_DEBUG if (is_metal_trace_active_) { [encoder_ pushDebugGroup:@(label.c_str())]; diff --git a/impeller/renderer/backend/vulkan/blit_pass_vk.cc b/impeller/renderer/backend/vulkan/blit_pass_vk.cc index 6bc6fb041b1f9..8b1fff55e7b60 100644 --- a/impeller/renderer/backend/vulkan/blit_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/blit_pass_vk.cc @@ -114,7 +114,7 @@ bool BlitPassVK::OnCopyTextureToTextureCommand( // If the source and destination are the same size then use copyImage, // otherwise perform a blit with a linear filter. - if (source->GetSize() == destination->GetSize()) { + if (source_region.GetSize() == destination->GetSize()) { vk::ImageCopy image_copy; image_copy.setSrcSubresource( From 79051368d4721e6bd94398f139645ddadae489dd Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 20 Aug 2024 16:57:27 -0700 Subject: [PATCH 23/27] separate resize command. --- .../backend/gles/blit_command_gles.cc | 66 ++++++++ .../renderer/backend/gles/blit_command_gles.h | 9 + .../renderer/backend/gles/blit_pass_gles.cc | 11 ++ .../renderer/backend/gles/blit_pass_gles.h | 4 + .../renderer/backend/metal/blit_pass_mtl.h | 4 + .../renderer/backend/metal/blit_pass_mtl.mm | 58 ++++--- .../renderer/backend/vulkan/blit_pass_vk.cc | 159 ++++++++++++------ .../renderer/backend/vulkan/blit_pass_vk.h | 4 + impeller/renderer/blit_command.h | 5 + impeller/renderer/blit_pass.h | 8 + impeller/renderer/blit_pass_unittests.cc | 2 +- impeller/renderer/testing/mocks.h | 6 + lib/ui/painting/image_decoder_impeller.cc | 3 +- 13 files changed, 259 insertions(+), 80 deletions(-) diff --git a/impeller/renderer/backend/gles/blit_command_gles.cc b/impeller/renderer/backend/gles/blit_command_gles.cc index c5ea94797b484..bb3dc95e210a1 100644 --- a/impeller/renderer/backend/gles/blit_command_gles.cc +++ b/impeller/renderer/backend/gles/blit_command_gles.cc @@ -9,6 +9,7 @@ #include "impeller/base/validation.h" #include "impeller/geometry/point.h" #include "impeller/renderer/backend/gles/device_buffer_gles.h" +#include "impeller/renderer/backend/gles/reactor_gles.h" #include "impeller/renderer/backend/gles/texture_gles.h" namespace impeller { @@ -353,4 +354,69 @@ bool BlitGenerateMipmapCommandGLES::Encode(const ReactorGLES& reactor) const { return true; }; +////// BlitResizeTextureCommandGLES +////////////////////////////////////////////////////// + +BlitResizeTextureCommandGLES::~BlitResizeTextureCommandGLES() = default; + +std::string BlitResizeTextureCommandGLES::GetLabel() const { + return "Resize Texture"; +} + +bool BlitResizeTextureCommandGLES::Encode(const ReactorGLES& reactor) const { + const auto& gl = reactor.GetProcTable(); + + // glBlitFramebuffer is a GLES3 proc. Since we target GLES2, we need to + // emulate the blit when it's not available in the driver. + if (!gl.BlitFramebuffer.IsAvailable()) { + // TODO(135818): Emulate the blit using a raster draw call here. + FML_LOG(ERROR) << "Texture blit fallback not implemented yet for GLES2."; + return false; + } + + GLuint read_fbo = GL_NONE; + GLuint draw_fbo = GL_NONE; + fml::ScopedCleanupClosure delete_fbos([&gl, &read_fbo, &draw_fbo]() { + DeleteFBO(gl, read_fbo, GL_READ_FRAMEBUFFER); + DeleteFBO(gl, draw_fbo, GL_DRAW_FRAMEBUFFER); + }); + + { + auto read = ConfigureFBO(gl, source, GL_READ_FRAMEBUFFER); + if (!read.has_value()) { + return false; + } + read_fbo = read.value(); + } + + { + auto draw = ConfigureFBO(gl, destination, GL_DRAW_FRAMEBUFFER); + if (!draw.has_value()) { + return false; + } + draw_fbo = draw.value(); + } + + gl.Disable(GL_SCISSOR_TEST); + gl.Disable(GL_DEPTH_TEST); + gl.Disable(GL_STENCIL_TEST); + + const IRect source_region = IRect::MakeSize(source->GetSize()); + const IRect destination_region = IRect::MakeSize(destination->GetSize()); + + gl.BlitFramebuffer(source_region.GetX(), // srcX0 + source_region.GetY(), // srcY0 + source_region.GetWidth(), // srcX1 + source_region.GetHeight(), // srcY1 + destination_region.GetX(), // dstX0 + destination_region.GetY(), // dstY0 + destination_region.GetWidth(), // dstX1 + destination_region.GetHeight(), // dstY1 + GL_COLOR_BUFFER_BIT, // mask + GL_LINEAR // filter + ); + + return true; +} + } // namespace impeller diff --git a/impeller/renderer/backend/gles/blit_command_gles.h b/impeller/renderer/backend/gles/blit_command_gles.h index f4cd901a3e848..1a3f35f1d2e82 100644 --- a/impeller/renderer/backend/gles/blit_command_gles.h +++ b/impeller/renderer/backend/gles/blit_command_gles.h @@ -59,6 +59,15 @@ struct BlitGenerateMipmapCommandGLES : public BlitEncodeGLES, [[nodiscard]] bool Encode(const ReactorGLES& reactor) const override; }; +struct BlitResizeTextureCommandGLES : public BlitEncodeGLES, + public BlitResizeTextureCommand { + ~BlitResizeTextureCommandGLES() override; + + std::string GetLabel() const override; + + [[nodiscard]] bool Encode(const ReactorGLES& reactor) const override; +}; + } // namespace impeller #endif // FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_BLIT_COMMAND_GLES_H_ diff --git a/impeller/renderer/backend/gles/blit_pass_gles.cc b/impeller/renderer/backend/gles/blit_pass_gles.cc index d83ef326856f8..28669a9f217cb 100644 --- a/impeller/renderer/backend/gles/blit_pass_gles.cc +++ b/impeller/renderer/backend/gles/blit_pass_gles.cc @@ -157,4 +157,15 @@ bool BlitPassGLES::OnGenerateMipmapCommand(std::shared_ptr texture, return true; } +// |BlitPass| +bool BlitPassGLES::ResizeTexture(const std::shared_ptr& source, + const std::shared_ptr& destination) { + auto command = std::make_unique(); + command->source = source; + command->destination = destination; + + commands_.push_back(std::move(command)); + return true; +} + } // namespace impeller diff --git a/impeller/renderer/backend/gles/blit_pass_gles.h b/impeller/renderer/backend/gles/blit_pass_gles.h index e34bb1614abb0..de688da2f63d8 100644 --- a/impeller/renderer/backend/gles/blit_pass_gles.h +++ b/impeller/renderer/backend/gles/blit_pass_gles.h @@ -40,6 +40,10 @@ class BlitPassGLES final : public BlitPass, bool EncodeCommands( const std::shared_ptr& transients_allocator) const override; + // |BlitPass| + bool ResizeTexture(const std::shared_ptr& source, + const std::shared_ptr& destination) override; + // |BlitPass| bool OnCopyTextureToTextureCommand(std::shared_ptr source, std::shared_ptr destination, diff --git a/impeller/renderer/backend/metal/blit_pass_mtl.h b/impeller/renderer/backend/metal/blit_pass_mtl.h index b0bfa04d2c27e..2fcb1832de360 100644 --- a/impeller/renderer/backend/metal/blit_pass_mtl.h +++ b/impeller/renderer/backend/metal/blit_pass_mtl.h @@ -41,6 +41,10 @@ class BlitPassMTL final : public BlitPass { bool EncodeCommands( const std::shared_ptr& transients_allocator) const override; + // |BlitPass| + bool ResizeTexture(const std::shared_ptr& source, + const std::shared_ptr& destination) override; + // |BlitPass| bool OnCopyTextureToTextureCommand(std::shared_ptr source, std::shared_ptr destination, diff --git a/impeller/renderer/backend/metal/blit_pass_mtl.mm b/impeller/renderer/backend/metal/blit_pass_mtl.mm index 856baad4ba513..8f36eff26326d 100644 --- a/impeller/renderer/backend/metal/blit_pass_mtl.mm +++ b/impeller/renderer/backend/metal/blit_pass_mtl.mm @@ -87,36 +87,48 @@ auto destination_origin_mtl = MTLOriginMake(destination_origin.x, destination_origin.y, 0); - if (source_region.GetSize() == destination->GetSize()) { #ifdef IMPELLER_DEBUG - if (is_metal_trace_active_) { - [encoder_ pushDebugGroup:@(label.c_str())]; - } + if (is_metal_trace_active_) { + [encoder_ pushDebugGroup:@(label.c_str())]; + } #endif // IMPELLER_DEBUG - [encoder_ copyFromTexture:source_mtl - sourceSlice:0 - sourceLevel:0 - sourceOrigin:source_origin_mtl - sourceSize:source_size_mtl - toTexture:destination_mtl - destinationSlice:0 - destinationLevel:0 - destinationOrigin:destination_origin_mtl]; + [encoder_ copyFromTexture:source_mtl + sourceSlice:0 + sourceLevel:0 + sourceOrigin:source_origin_mtl + sourceSize:source_size_mtl + toTexture:destination_mtl + destinationSlice:0 + destinationLevel:0 + destinationOrigin:destination_origin_mtl]; #ifdef IMPELLER_DEBUG - if (is_metal_trace_active_) { - [encoder_ popDebugGroup]; - } + if (is_metal_trace_active_) { + [encoder_ popDebugGroup]; + } #endif // IMPELLER_DEBUG - } else { - [encoder_ endEncoding]; - auto filter = [[MPSImageBilinearScale alloc] initWithDevice:device_]; - [filter encodeToCommandBuffer:buffer_ - sourceTexture:source_mtl - destinationTexture:destination_mtl]; - encoder_ = [buffer_ blitCommandEncoder]; + return true; +} + +// |BlitPass| +bool BlitPassMTL::ResizeTexture(const std::shared_ptr& source, + const std::shared_ptr& destination) { + auto source_mtl = TextureMTL::Cast(*source).GetMTLTexture(); + if (!source_mtl) { + return false; } + auto destination_mtl = TextureMTL::Cast(*destination).GetMTLTexture(); + if (!destination_mtl) { + return false; + } + + [encoder_ endEncoding]; + auto filter = [[MPSImageBilinearScale alloc] initWithDevice:device_]; + [filter encodeToCommandBuffer:buffer_ + sourceTexture:source_mtl + destinationTexture:destination_mtl]; + encoder_ = [buffer_ blitCommandEncoder]; return true; } diff --git a/impeller/renderer/backend/vulkan/blit_pass_vk.cc b/impeller/renderer/backend/vulkan/blit_pass_vk.cc index 8b1fff55e7b60..a9fce56228c2d 100644 --- a/impeller/renderer/backend/vulkan/blit_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/blit_pass_vk.cc @@ -112,63 +112,28 @@ bool BlitPassVK::OnCopyTextureToTextureCommand( return false; } - // If the source and destination are the same size then use copyImage, - // otherwise perform a blit with a linear filter. - if (source_region.GetSize() == destination->GetSize()) { - vk::ImageCopy image_copy; - - image_copy.setSrcSubresource( - vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); - image_copy.setDstSubresource( - vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); - - image_copy.srcOffset = - vk::Offset3D(source_region.GetX(), source_region.GetY(), 0); - image_copy.dstOffset = - vk::Offset3D(destination_origin.x, destination_origin.y, 0); - image_copy.extent = - vk::Extent3D(source_region.GetWidth(), source_region.GetHeight(), 1); - - // Issue the copy command now that the images are already in the right - // layouts. - cmd_buffer.copyImage(src.GetImage(), // - src_barrier.new_layout, // - dst.GetImage(), // - dst_barrier.new_layout, // - image_copy // - ); - } else { - vk::ImageBlit blit; - blit.srcSubresource.aspectMask = vk::ImageAspectFlagBits::eColor; - blit.srcSubresource.baseArrayLayer = 0u; - blit.srcSubresource.layerCount = 1u; - blit.srcSubresource.mipLevel = 0; + vk::ImageCopy image_copy; - blit.dstSubresource.aspectMask = vk::ImageAspectFlagBits::eColor; - blit.dstSubresource.baseArrayLayer = 0u; - blit.dstSubresource.layerCount = 1u; - blit.dstSubresource.mipLevel = 0; - - // offsets[0] is origin. - blit.srcOffsets[1].x = std::max(source->GetSize().width, 1u); - blit.srcOffsets[1].y = std::max(source->GetSize().height, 1u); - blit.srcOffsets[1].z = 1u; - - // offsets[0] is origin. - blit.dstOffsets[1].x = std::max(destination->GetSize().width, 1u); - blit.dstOffsets[1].y = std::max(destination->GetSize().height, 1u); - blit.dstOffsets[1].z = 1u; - - cmd_buffer.blitImage(src.GetImage(), // - src_barrier.new_layout, // - dst.GetImage(), // - dst_barrier.new_layout, // - 1, // - &blit, // - vk::Filter::eLinear + image_copy.setSrcSubresource( + vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); + image_copy.setDstSubresource( + vk::ImageSubresourceLayers(vk::ImageAspectFlagBits::eColor, 0, 0, 1)); - ); - } + image_copy.srcOffset = + vk::Offset3D(source_region.GetX(), source_region.GetY(), 0); + image_copy.dstOffset = + vk::Offset3D(destination_origin.x, destination_origin.y, 0); + image_copy.extent = + vk::Extent3D(source_region.GetWidth(), source_region.GetHeight(), 1); + + // Issue the copy command now that the images are already in the right + // layouts. + cmd_buffer.copyImage(src.GetImage(), // + src_barrier.new_layout, // + dst.GetImage(), // + dst_barrier.new_layout, // + image_copy // + ); // If this is an onscreen texture, do not transition the layout // back to shader read. @@ -354,6 +319,90 @@ bool BlitPassVK::OnCopyBufferToTextureCommand( return true; } +// |BlitPass| +bool BlitPassVK::ResizeTexture(const std::shared_ptr& source, + const std::shared_ptr& destination) { + auto& encoder = *command_buffer_->GetEncoder(); + const auto& cmd_buffer = encoder.GetCommandBuffer(); + + const auto& src = TextureVK::Cast(*source); + const auto& dst = TextureVK::Cast(*destination); + + if (!encoder.Track(source) || !encoder.Track(destination)) { + return false; + } + + BarrierVK src_barrier; + src_barrier.cmd_buffer = cmd_buffer; + src_barrier.new_layout = vk::ImageLayout::eTransferSrcOptimal; + src_barrier.src_access = vk::AccessFlagBits::eTransferWrite | + vk::AccessFlagBits::eShaderWrite | + vk::AccessFlagBits::eColorAttachmentWrite; + src_barrier.src_stage = vk::PipelineStageFlagBits::eTransfer | + vk::PipelineStageFlagBits::eFragmentShader | + vk::PipelineStageFlagBits::eColorAttachmentOutput; + src_barrier.dst_access = vk::AccessFlagBits::eTransferRead; + src_barrier.dst_stage = vk::PipelineStageFlagBits::eTransfer; + + BarrierVK dst_barrier; + dst_barrier.cmd_buffer = cmd_buffer; + dst_barrier.new_layout = vk::ImageLayout::eTransferDstOptimal; + dst_barrier.src_access = {}; + dst_barrier.src_stage = vk::PipelineStageFlagBits::eTopOfPipe; + dst_barrier.dst_access = + vk::AccessFlagBits::eShaderRead | vk::AccessFlagBits::eTransferWrite; + dst_barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader | + vk::PipelineStageFlagBits::eTransfer; + + if (!src.SetLayout(src_barrier) || !dst.SetLayout(dst_barrier)) { + VALIDATION_LOG << "Could not complete layout transitions."; + return false; + } + + vk::ImageBlit blit; + blit.srcSubresource.aspectMask = vk::ImageAspectFlagBits::eColor; + blit.srcSubresource.baseArrayLayer = 0u; + blit.srcSubresource.layerCount = 1u; + blit.srcSubresource.mipLevel = 0; + + blit.dstSubresource.aspectMask = vk::ImageAspectFlagBits::eColor; + blit.dstSubresource.baseArrayLayer = 0u; + blit.dstSubresource.layerCount = 1u; + blit.dstSubresource.mipLevel = 0; + + // offsets[0] is origin. + blit.srcOffsets[1].x = std::max(source->GetSize().width, 1u); + blit.srcOffsets[1].y = std::max(source->GetSize().height, 1u); + blit.srcOffsets[1].z = 1u; + + // offsets[0] is origin. + blit.dstOffsets[1].x = std::max(destination->GetSize().width, 1u); + blit.dstOffsets[1].y = std::max(destination->GetSize().height, 1u); + blit.dstOffsets[1].z = 1u; + + cmd_buffer.blitImage(src.GetImage(), // + src_barrier.new_layout, // + dst.GetImage(), // + dst_barrier.new_layout, // + 1, // + &blit, // + vk::Filter::eLinear + + ); + + // Convert back to shader read + + BarrierVK barrier; + barrier.cmd_buffer = cmd_buffer; + barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; + barrier.src_access = {}; + barrier.src_stage = vk::PipelineStageFlagBits::eTopOfPipe; + barrier.dst_access = vk::AccessFlagBits::eShaderRead; + barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; + + return dst.SetLayout(barrier); +} + // |BlitPass| bool BlitPassVK::OnGenerateMipmapCommand(std::shared_ptr texture, std::string label) { diff --git a/impeller/renderer/backend/vulkan/blit_pass_vk.h b/impeller/renderer/backend/vulkan/blit_pass_vk.h index 7d9c4c7368338..fd5ba51dd691c 100644 --- a/impeller/renderer/backend/vulkan/blit_pass_vk.h +++ b/impeller/renderer/backend/vulkan/blit_pass_vk.h @@ -37,6 +37,10 @@ class BlitPassVK final : public BlitPass { bool EncodeCommands( const std::shared_ptr& transients_allocator) const override; + // |BlitPass| + bool ResizeTexture(const std::shared_ptr& source, + const std::shared_ptr& destination) override; + // |BlitPass| bool ConvertTextureToShaderRead( const std::shared_ptr& texture) override; diff --git a/impeller/renderer/blit_command.h b/impeller/renderer/blit_command.h index 0bae872ab0796..8ae331b4bd0a5 100644 --- a/impeller/renderer/blit_command.h +++ b/impeller/renderer/blit_command.h @@ -22,6 +22,11 @@ struct BlitCopyTextureToTextureCommand : public BlitCommand { IPoint destination_origin; }; +struct BlitResizeTextureCommand : public BlitCommand { + std::shared_ptr source; + std::shared_ptr destination; +}; + struct BlitCopyTextureToBufferCommand : public BlitCommand { std::shared_ptr source; std::shared_ptr destination; diff --git a/impeller/renderer/blit_pass.h b/impeller/renderer/blit_pass.h index 6f5f332b3aa6f..f48c047691c2b 100644 --- a/impeller/renderer/blit_pass.h +++ b/impeller/renderer/blit_pass.h @@ -39,6 +39,14 @@ class BlitPass { virtual bool ConvertTextureToShaderRead( const std::shared_ptr& texture); + //---------------------------------------------------------------------------- + /// @brief Resize the [source] texture into the [destination] texture. + /// + /// On Metal platforms, [destination] is required to be non-lossy + /// and have the Shader read capability. + virtual bool ResizeTexture(const std::shared_ptr& source, + const std::shared_ptr& destination) = 0; + //---------------------------------------------------------------------------- /// @brief Record a command to copy the contents of one texture to /// another texture. The blit area is limited by the intersection diff --git a/impeller/renderer/blit_pass_unittests.cc b/impeller/renderer/blit_pass_unittests.cc index 60b47885ae2a4..ebccf0294ab30 100644 --- a/impeller/renderer/blit_pass_unittests.cc +++ b/impeller/renderer/blit_pass_unittests.cc @@ -166,7 +166,7 @@ TEST_P(BlitPassTest, CanResizeTextures) { ASSERT_TRUE(staging); EXPECT_TRUE(blit_pass->AddCopy(DeviceBuffer::AsBufferView(staging), src)); - EXPECT_TRUE(blit_pass->AddCopy(src, dst)); + EXPECT_TRUE(blit_pass->ResizeTexture(src, dst)); EXPECT_TRUE(blit_pass->EncodeCommands(GetContext()->GetResourceAllocator())); EXPECT_TRUE(context->GetCommandQueue()->Submit({std::move(cmd_buffer)}).ok()); } diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index d4e27449bfe65..b1b1ff6964233 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -61,6 +61,12 @@ class MockBlitPass : public BlitPass { (const, override)); MOCK_METHOD(void, OnSetLabel, (std::string label), (override)); + MOCK_METHOD(bool, + ResizeTexture, + (const std::shared_ptr& source, + const std::shared_ptr& destination), + (override)); + MOCK_METHOD(bool, OnCopyTextureToTextureCommand, (std::shared_ptr source, diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 6f3b6e1fb5cd9..0f17e9f54a00e 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -360,7 +360,8 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( return std::make_pair(nullptr, decode_error); } - blit_pass->AddCopy(/*source=*/dest_texture, /*destination=*/resize_texture); + blit_pass->ResizeTexture(/*source=*/dest_texture, + /*destination=*/resize_texture); if (resize_desc.mip_count > 1) { blit_pass->GenerateMipmap(resize_texture); } From e6f1ec67b9312010440e511701c4f02dfcda4e8b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 20 Aug 2024 17:11:20 -0700 Subject: [PATCH 24/27] no mips on iOS. --- lib/ui/painting/image_decoder_impeller.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 0f17e9f54a00e..565e034545a74 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -302,6 +302,12 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( texture_descriptor.size = {image_info.width(), image_info.height()}; texture_descriptor.mip_count = texture_descriptor.size.MipCount(); texture_descriptor.compression_type = impeller::CompressionType::kLossy; + if (context->GetBackendType() == impeller::Context::BackendType::kMetal && + resize_info.has_value()) { + // The MPS used to resize images on iOS does not require mip generation. + // Remove mip count if we are resizing the image on the GPU. + texture_descriptor.mip_count = 1; + } auto dest_texture = context->GetResourceAllocator()->CreateTexture(texture_descriptor); From 1156b2008bf1fa049b37c6a57bff4ee39ab59b1f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 20 Aug 2024 18:07:22 -0700 Subject: [PATCH 25/27] add shader write usage. --- impeller/renderer/blit_pass_unittests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/renderer/blit_pass_unittests.cc b/impeller/renderer/blit_pass_unittests.cc index ebccf0294ab30..ae1c707c46534 100644 --- a/impeller/renderer/blit_pass_unittests.cc +++ b/impeller/renderer/blit_pass_unittests.cc @@ -148,6 +148,7 @@ TEST_P(BlitPassTest, CanResizeTextures) { src_format.storage_mode = StorageMode::kDevicePrivate; src_format.format = PixelFormat::kR8G8B8A8UNormInt; src_format.size = {100, 100}; + src_format.usage = TextureUsage::kShaderRead | TextureUsage::kShaderWrite; auto src = context->GetResourceAllocator()->CreateTexture(src_format); std::vector bytes(src_format.GetByteSizeOfBaseMipLevel()); From 6f1ed783b23bc262dfdbe244c7ddd52ce08ef0f9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 20 Aug 2024 19:12:40 -0700 Subject: [PATCH 26/27] ++ --- impeller/renderer/blit_pass_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/blit_pass_unittests.cc b/impeller/renderer/blit_pass_unittests.cc index ae1c707c46534..1f902b1844b56 100644 --- a/impeller/renderer/blit_pass_unittests.cc +++ b/impeller/renderer/blit_pass_unittests.cc @@ -142,13 +142,13 @@ TEST_P(BlitPassTest, CanResizeTextures) { dst_format.storage_mode = StorageMode::kDevicePrivate; dst_format.format = PixelFormat::kR8G8B8A8UNormInt; dst_format.size = {10, 10}; + dst_format.usage = TextureUsage::kShaderRead | TextureUsage::kShaderWrite; auto dst = context->GetResourceAllocator()->CreateTexture(dst_format); TextureDescriptor src_format; src_format.storage_mode = StorageMode::kDevicePrivate; src_format.format = PixelFormat::kR8G8B8A8UNormInt; src_format.size = {100, 100}; - src_format.usage = TextureUsage::kShaderRead | TextureUsage::kShaderWrite; auto src = context->GetResourceAllocator()->CreateTexture(src_format); std::vector bytes(src_format.GetByteSizeOfBaseMipLevel()); From 855aad1457eb5d0c587af38e3259d789a239f217 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 21 Aug 2024 12:02:01 -0700 Subject: [PATCH 27/27] Update blit_command_gles.cc --- impeller/renderer/backend/gles/blit_command_gles.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/gles/blit_command_gles.cc b/impeller/renderer/backend/gles/blit_command_gles.cc index bb3dc95e210a1..2269b8a96fbab 100644 --- a/impeller/renderer/backend/gles/blit_command_gles.cc +++ b/impeller/renderer/backend/gles/blit_command_gles.cc @@ -74,7 +74,7 @@ bool BlitCopyTextureToTextureCommandGLES::Encode( // emulate the blit when it's not available in the driver. if (!gl.BlitFramebuffer.IsAvailable()) { // TODO(135818): Emulate the blit using a raster draw call here. - FML_LOG(ERROR) << "Texture blit fallback not implemented yet for GLES2."; + VALIDATION_LOG << "Texture blit fallback not implemented yet for GLES2."; return false; } @@ -370,7 +370,7 @@ bool BlitResizeTextureCommandGLES::Encode(const ReactorGLES& reactor) const { // emulate the blit when it's not available in the driver. if (!gl.BlitFramebuffer.IsAvailable()) { // TODO(135818): Emulate the blit using a raster draw call here. - FML_LOG(ERROR) << "Texture blit fallback not implemented yet for GLES2."; + VALIDATION_LOG << "Texture blit fallback not implemented yet for GLES2."; return false; }