From 08355df4dcde45ac6f4ae6a6c9b06d43d772090a Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Thu, 26 May 2022 21:51:32 -0700 Subject: [PATCH] [Impeller] Upload textures on the IO context where available. (flutter/engine#33648) In the OpenGL ES backend, if an Impeller operation happens on a thread with no OpenGL context current, the reactor enqueues it for later execution. This later execution may happen on the main context. To avoid this pessimization, queue the upload to the IO thread (which already has a context) instead. I was hoping that the reactor would manage task pinning as well but that change is more invasive and I wanted to add parity with the Metal backend ASAP. Hence the post-task fix. Fixes https://github.com/flutter/flutter/issues/104756 --- .../renderer/backend/gles/context_gles.cc | 5 + .../renderer/backend/gles/context_gles.h | 3 + .../src/flutter/impeller/renderer/context.cc | 4 + .../src/flutter/impeller/renderer/context.h | 2 + .../lib/ui/painting/image_decoder_impeller.cc | 95 ++++++++++++------- .../lib/ui/painting/image_decoder_impeller.h | 1 - 6 files changed, 76 insertions(+), 34 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc index 6306ab886ed..f626504d628 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc @@ -123,4 +123,9 @@ std::shared_ptr ContextGLES::CreateTransferCommandBuffer() return CreateRenderCommandBuffer(); } +// |Context| +bool ContextGLES::HasThreadingRestrictions() const { + return true; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/gles/context_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/context_gles.h index fbf0d728536..6f516e69c71 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/context_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/context_gles.h @@ -69,6 +69,9 @@ class ContextGLES final : public Context, // |Context| std::shared_ptr CreateTransferCommandBuffer() const override; + // |Context| + bool HasThreadingRestrictions() const override; + FML_DISALLOW_COPY_AND_ASSIGN(ContextGLES); }; diff --git a/engine/src/flutter/impeller/renderer/context.cc b/engine/src/flutter/impeller/renderer/context.cc index 542d04ee14d..769a8f019c0 100644 --- a/engine/src/flutter/impeller/renderer/context.cc +++ b/engine/src/flutter/impeller/renderer/context.cc @@ -10,4 +10,8 @@ Context::~Context() = default; Context::Context() = default; +bool Context::HasThreadingRestrictions() const { + return false; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/context.h b/engine/src/flutter/impeller/renderer/context.h index ac60ab2b40b..412361e3f35 100644 --- a/engine/src/flutter/impeller/renderer/context.h +++ b/engine/src/flutter/impeller/renderer/context.h @@ -46,6 +46,8 @@ class Context { virtual std::shared_ptr CreateTransferCommandBuffer() const = 0; + virtual bool HasThreadingRestrictions() const; + protected: Context(); diff --git a/engine/src/flutter/lib/ui/painting/image_decoder_impeller.cc b/engine/src/flutter/lib/ui/painting/image_decoder_impeller.cc index 64160298ad7..e05b95f5920 100644 --- a/engine/src/flutter/lib/ui/painting/image_decoder_impeller.cc +++ b/engine/src/flutter/lib/ui/painting/image_decoder_impeller.cc @@ -53,15 +53,11 @@ static std::optional ToPixelFormat(SkColorType type) { return std::nullopt; } -static sk_sp DecompressAndUploadTexture( - std::shared_ptr context, - ImageDescriptor* descriptor, - SkISize target_size, - std::string label) { - TRACE_EVENT0("flutter", __FUNCTION__); - - if (!context || !descriptor) { - FML_DLOG(ERROR) << "Invalid context or descriptor."; +static std::shared_ptr DecompressTexture(ImageDescriptor* descriptor, + SkISize target_size) { + TRACE_EVENT0("impeller", __FUNCTION__); + if (!descriptor) { + FML_DLOG(ERROR) << "Invalid descriptor."; return nullptr; } @@ -84,18 +80,34 @@ static sk_sp DecompressAndUploadTexture( return nullptr; } - SkBitmap bitmap; - if (!bitmap.tryAllocPixels(image_info)) { + auto bitmap = std::make_shared(); + if (!bitmap->tryAllocPixels(image_info)) { FML_DLOG(ERROR) << "Could not allocate intermediate for image decompression."; return nullptr; } - if (!descriptor->get_pixels(bitmap.pixmap())) { + if (!descriptor->get_pixels(bitmap->pixmap())) { FML_DLOG(ERROR) << "Could not decompress image."; return nullptr; } + return bitmap; +} + +static sk_sp UploadTexture(std::shared_ptr context, + std::shared_ptr bitmap) { + TRACE_EVENT0("impeller", __FUNCTION__); + if (!context || !bitmap) { + return nullptr; + } + const auto image_info = bitmap->info(); + const auto pixel_format = ToPixelFormat(image_info.colorType()); + if (!pixel_format) { + FML_DLOG(ERROR) << "Pixel format unsupported by Impeller."; + return nullptr; + } + impeller::TextureDescriptor texture_descriptor; texture_descriptor.format = pixel_format.value(); texture_descriptor.size = {image_info.width(), image_info.height()}; @@ -108,13 +120,13 @@ static sk_sp DecompressAndUploadTexture( } if (!texture->SetContents( - reinterpret_cast(bitmap.getAddr(0, 0)), + reinterpret_cast(bitmap->getAddr(0, 0)), texture_descriptor.GetByteSizeOfBaseMipLevel())) { FML_DLOG(ERROR) << "Could not copy contents into Impeller texture."; return nullptr; } - texture->SetLabel(label.c_str()); + texture->SetLabel(impeller::SPrintF("ui.Image(%p)", texture.get()).c_str()); return impeller::DlImageImpeller::Make(std::move(texture)); } @@ -123,30 +135,47 @@ static sk_sp DecompressAndUploadTexture( void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, uint32_t target_width, uint32_t target_height, - const ImageResult& result) { + const ImageResult& p_result) { FML_DCHECK(descriptor); - FML_DCHECK(result); + FML_DCHECK(p_result); + // Wrap the result callback so that it can be invoked from any thread. auto raw_descriptor = descriptor.get(); raw_descriptor->AddRef(); - concurrent_task_runner_->PostTask( - [raw_descriptor, // - context = context_.get(), // - target_size = SkISize::Make(target_width, target_height), // - label = impeller::SPrintF("ui.Image %zu", label_count_++), // - ui_runner = runners_.GetUITaskRunner(), // - result // - ]() { - auto image = DecompressAndUploadTexture(context, // - raw_descriptor, // - target_size, // - label // - ); + ImageResult result = [p_result, // + raw_descriptor, // + ui_runner = runners_.GetUITaskRunner() // + ](auto image) { + ui_runner->PostTask([raw_descriptor, p_result, image]() { + raw_descriptor->Release(); + p_result(std::move(image)); + }); + }; - ui_runner->PostTask([raw_descriptor, image, result]() { - raw_descriptor->Release(); - result(image); - }); + concurrent_task_runner_->PostTask( + [raw_descriptor, // + context = context_.get(), // + target_size = SkISize::Make(target_width, target_height), // + io_runner = runners_.GetIOTaskRunner(), // + result // + ]() { + // Always decompress on the concurrent runner. + auto bitmap = DecompressTexture(raw_descriptor, target_size); + if (!bitmap) { + result(nullptr); + return; + } + auto upload_texture_and_invoke_result = [result, context, bitmap]() { + result(UploadTexture(context, bitmap)); + }; + // Depending on whether the context has threading restrictions, stay on + // the concurrent runner to perform texture upload or move to an IO + // runner. + if (context->HasThreadingRestrictions()) { + io_runner->PostTask(upload_texture_and_invoke_result); + } else { + upload_texture_and_invoke_result(); + } }); } diff --git a/engine/src/flutter/lib/ui/painting/image_decoder_impeller.h b/engine/src/flutter/lib/ui/painting/image_decoder_impeller.h index 489942fb5b0..47a1bfde60d 100644 --- a/engine/src/flutter/lib/ui/painting/image_decoder_impeller.h +++ b/engine/src/flutter/lib/ui/painting/image_decoder_impeller.h @@ -34,7 +34,6 @@ class ImageDecoderImpeller final : public ImageDecoder { private: using FutureContext = std::shared_future>; FutureContext context_; - size_t label_count_ = 1u; FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoderImpeller); };