From c7ec5bbc1cc0fe2521237e923cc6633261d8232b Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Mon, 25 Nov 2019 14:44:57 -0800 Subject: [PATCH] Convert images to raster on the GPU thread for Image.toByteData (#13647) If the image is a cross-context image that might be read from the GPU thread during onscreen rendering, then it is not safe to read it concurrently from the IO thread as part of Image.toByteData. If the GPU thread does not have a graphics context, then fall back to converting the image on the IO thread. Fixes https://github.com/flutter/flutter/issues/30697 --- lib/ui/painting/image_encoding.cc | 161 +++++++++++++++++++----------- lib/ui/snapshot_delegate.h | 2 + shell/common/rasterizer.cc | 38 ++++++- shell/common/rasterizer.h | 7 ++ 4 files changed, 143 insertions(+), 65 deletions(-) diff --git a/lib/ui/painting/image_encoding.cc b/lib/ui/painting/image_encoding.cc index 86bb45bfd..cef92a7e2 100644 --- a/lib/ui/painting/image_encoding.cc +++ b/lib/ui/painting/image_encoding.cc @@ -51,33 +51,18 @@ void InvokeDataCallback(std::unique_ptr callback, } } -sk_sp ConvertToRasterImageIfNecessary(sk_sp image, - GrContext* context) { - SkPixmap pixmap; - if (image->peekPixels(&pixmap)) { - // This is already a raster image. - return image; - } - - if (sk_sp raster_image = image->makeRasterImage()) { - // The image can be converted to a raster image. - return raster_image; - } - - // Cross-context images do not support makeRasterImage. Convert these images - // by drawing them into a surface. - if (context == nullptr) { - return nullptr; +sk_sp ConvertToRasterUsingResourceContext( + sk_sp image, + GrContext* resource_context) { + sk_sp surface; + SkImageInfo surface_info = SkImageInfo::MakeN32Premul(image->dimensions()); + if (resource_context) { + surface = SkSurface::MakeRenderTarget(resource_context, SkBudgeted::kNo, + surface_info); + } else { + surface = SkSurface::MakeRaster(surface_info); } - TRACE_EVENT0("flutter", __FUNCTION__); - - // Create a GPU surface with the context and then do a device to host copy of - // image contents. - auto surface = SkSurface::MakeRenderTarget( - context, SkBudgeted::kNo, - SkImageInfo::MakeN32Premul(image->dimensions())); - if (surface == nullptr || surface->getCanvas() == nullptr) { FML_LOG(ERROR) << "Could not create a surface to copy the texture into."; return nullptr; @@ -96,6 +81,64 @@ sk_sp ConvertToRasterImageIfNecessary(sk_sp image, return snapshot->makeRasterImage(); } +void ConvertImageToRaster(sk_sp image, + std::function)> encode_task, + fml::RefPtr gpu_task_runner, + fml::RefPtr io_task_runner, + GrContext* resource_context, + fml::WeakPtr snapshot_delegate) { + // Check validity of the image. + if (image == nullptr) { + FML_LOG(ERROR) << "Image was null."; + encode_task(nullptr); + return; + } + + auto dimensions = image->dimensions(); + + if (dimensions.isEmpty()) { + FML_LOG(ERROR) << "Image dimensions were empty."; + encode_task(nullptr); + return; + } + + SkPixmap pixmap; + if (image->peekPixels(&pixmap)) { + // This is already a raster image. + encode_task(image); + return; + } + + if (sk_sp raster_image = image->makeRasterImage()) { + // The image can be converted to a raster image. + encode_task(raster_image); + return; + } + + // Cross-context images do not support makeRasterImage. Convert these images + // by drawing them into a surface. This must be done on the GPU thread + // to prevent concurrent usage of the image on both the IO and GPU threads. + gpu_task_runner->PostTask([image, encode_task = std::move(encode_task), + resource_context, snapshot_delegate, + io_task_runner]() { + sk_sp raster_image = + snapshot_delegate->ConvertToRasterImage(image); + + io_task_runner->PostTask([image, encode_task = std::move(encode_task), + raster_image = std::move(raster_image), + resource_context]() mutable { + if (!raster_image) { + // The rasterizer was unable to render the cross-context image + // (presumably because it does not have a GrContext). In that case, + // convert the image on the IO thread using the resource context. + raster_image = + ConvertToRasterUsingResourceContext(image, resource_context); + } + encode_task(raster_image); + }); + }); +} + sk_sp CopyImageByteData(sk_sp raster_image, SkColorType color_type) { FML_DCHECK(raster_image); @@ -132,28 +175,10 @@ sk_sp CopyImageByteData(sk_sp raster_image, return SkData::MakeWithCopy(pixmap.addr(), pixmap.computeByteSize()); } -sk_sp EncodeImage(sk_sp p_image, - GrContext* context, - ImageByteFormat format) { +sk_sp EncodeImage(sk_sp raster_image, ImageByteFormat format) { TRACE_EVENT0("flutter", __FUNCTION__); - // Check validity of the image. - if (p_image == nullptr) { - FML_LOG(ERROR) << "Image was null."; - return nullptr; - } - - auto dimensions = p_image->dimensions(); - - if (dimensions.isEmpty()) { - FML_LOG(ERROR) << "Image dimensions were empty."; - return nullptr; - } - - auto raster_image = ConvertToRasterImageIfNecessary(p_image, context); - - if (raster_image == nullptr) { - FML_LOG(ERROR) << "Could not create a raster copy of the image."; + if (!raster_image) { return nullptr; } @@ -165,7 +190,7 @@ sk_sp EncodeImage(sk_sp p_image, if (png_image == nullptr) { FML_LOG(ERROR) << "Could not convert raster image to PNG."; return nullptr; - } + }; return png_image; } break; case kRawRGBA: { @@ -181,17 +206,29 @@ sk_sp EncodeImage(sk_sp p_image, } void EncodeImageAndInvokeDataCallback( - std::unique_ptr callback, sk_sp image, - GrContext* context, + std::unique_ptr callback, + ImageByteFormat format, fml::RefPtr ui_task_runner, - ImageByteFormat format) { - sk_sp encoded = EncodeImage(std::move(image), context, format); - - ui_task_runner->PostTask( - fml::MakeCopyable([callback = std::move(callback), encoded]() mutable { + fml::RefPtr gpu_task_runner, + fml::RefPtr io_task_runner, + GrContext* resource_context, + fml::WeakPtr snapshot_delegate) { + auto callback_task = fml::MakeCopyable( + [callback = std::move(callback)](sk_sp encoded) mutable { InvokeDataCallback(std::move(callback), std::move(encoded)); - })); + }); + + auto encode_task = [callback_task = std::move(callback_task), format, + ui_task_runner](sk_sp raster_image) { + sk_sp encoded = EncodeImage(std::move(raster_image), format); + ui_task_runner->PostTask( + [callback_task = std::move(callback_task), + encoded = std::move(encoded)] { callback_task(encoded); }); + }; + + ConvertImageToRaster(std::move(image), encode_task, gpu_task_runner, + io_task_runner, resource_context, snapshot_delegate); } } // namespace @@ -214,13 +251,17 @@ Dart_Handle EncodeImage(CanvasImage* canvas_image, task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable( [callback = std::move(callback), image = canvas_image->image(), + image_format, ui_task_runner = task_runners.GetUITaskRunner(), + gpu_task_runner = task_runners.GetGPUTaskRunner(), + io_task_runner = task_runners.GetIOTaskRunner(), io_manager = UIDartState::Current()->GetIOManager(), - ui_task_runner = task_runners.GetUITaskRunner(), - image_format]() mutable { - EncodeImageAndInvokeDataCallback(std::move(callback), std::move(image), - io_manager->GetResourceContext().get(), - std::move(ui_task_runner), - image_format); + snapshot_delegate = + UIDartState::Current()->GetSnapshotDelegate()]() mutable { + EncodeImageAndInvokeDataCallback( + std::move(image), std::move(callback), image_format, + std::move(ui_task_runner), std::move(gpu_task_runner), + std::move(io_task_runner), io_manager->GetResourceContext().get(), + std::move(snapshot_delegate)); })); return Dart_Null(); diff --git a/lib/ui/snapshot_delegate.h b/lib/ui/snapshot_delegate.h index 9a771f121..ad9b8ef1f 100644 --- a/lib/ui/snapshot_delegate.h +++ b/lib/ui/snapshot_delegate.h @@ -14,6 +14,8 @@ class SnapshotDelegate { public: virtual sk_sp MakeRasterSnapshot(sk_sp picture, SkISize picture_size) = 0; + + virtual sk_sp ConvertToRasterImage(sk_sp image) = 0; }; } // namespace flutter diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 9fbb6949d..11fdd3f6e 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -151,13 +151,14 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { } } -sk_sp Rasterizer::MakeRasterSnapshot(sk_sp picture, - SkISize picture_size) { +sk_sp Rasterizer::DoMakeRasterSnapshot( + SkISize size, + std::function draw_callback) { TRACE_EVENT0("flutter", __FUNCTION__); sk_sp surface; SkImageInfo image_info = SkImageInfo::MakeN32Premul( - picture_size.width(), picture_size.height(), SkColorSpace::MakeSRGB()); + size.width(), size.height(), SkColorSpace::MakeSRGB()); if (surface_ == nullptr || surface_->GetContext() == nullptr) { // Raster surface is fine if there is no on screen surface. This might // happen in case of software rendering. @@ -179,8 +180,7 @@ sk_sp Rasterizer::MakeRasterSnapshot(sk_sp picture, return nullptr; } - surface->getCanvas()->drawPicture(picture.get()); - + draw_callback(surface->getCanvas()); surface->getCanvas()->flush(); sk_sp device_snapshot; @@ -203,6 +203,34 @@ sk_sp Rasterizer::MakeRasterSnapshot(sk_sp picture, return nullptr; } +sk_sp Rasterizer::MakeRasterSnapshot(sk_sp picture, + SkISize picture_size) { + return DoMakeRasterSnapshot(picture_size, + [picture = std::move(picture)](SkCanvas* canvas) { + canvas->drawPicture(picture); + }); +} + +sk_sp Rasterizer::ConvertToRasterImage(sk_sp image) { + TRACE_EVENT0("flutter", __FUNCTION__); + + // If the rasterizer does not have a surface with a GrContext, then it will + // be unable to render a cross-context SkImage. The caller will need to + // create the raster image on the IO thread. + if (surface_ == nullptr || surface_->GetContext() == nullptr) { + return nullptr; + } + + if (image == nullptr) { + return nullptr; + } + + return DoMakeRasterSnapshot(image->dimensions(), + [image = std::move(image)](SkCanvas* canvas) { + canvas->drawImage(image, 0, 0); + }); +} + RasterStatus Rasterizer::DoDraw( std::unique_ptr layer_tree) { FML_DCHECK(task_runners_.GetGPUTaskRunner()->RunsTasksOnCurrentThread()); diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 44c7c7a75..d2242648d 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -426,6 +426,13 @@ class Rasterizer final : public SnapshotDelegate { sk_sp MakeRasterSnapshot(sk_sp picture, SkISize picture_size) override; + // |SnapshotDelegate| + sk_sp ConvertToRasterImage(sk_sp image) override; + + sk_sp DoMakeRasterSnapshot( + SkISize size, + std::function draw_callback); + RasterStatus DoDraw(std::unique_ptr layer_tree); RasterStatus DrawToSurface(flutter::LayerTree& layer_tree); -- GitLab