From 9bc33b853947d454ff4d588744678dfccc449166 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Mon, 22 May 2023 13:43:05 -0700 Subject: [PATCH 1/4] CP 1 --- lib/ui/fixtures/ui_test.dart | 6 +- lib/ui/painting.dart | 9 +- lib/ui/painting/image_decoder.h | 2 +- lib/ui/painting/image_decoder_impeller.cc | 162 +++++++++++++-------- lib/ui/painting/image_decoder_impeller.h | 7 +- lib/ui/painting/image_decoder_skia.cc | 2 +- lib/ui/painting/image_decoder_unittests.cc | 35 +++-- lib/ui/painting/multi_frame_codec.cc | 49 ++++--- lib/ui/painting/multi_frame_codec.h | 4 +- lib/ui/painting/single_frame_codec.cc | 13 +- shell/common/fixtures/shell_test.dart | 2 +- 11 files changed, 179 insertions(+), 112 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 36f09bf397811..0bfd2612a5125 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -248,12 +248,12 @@ void createPath() { external void _validatePath(Path path); @pragma('vm:entry-point') -void frameCallback(Object? image, int durationMilliseconds) { - validateFrameCallback(image, durationMilliseconds); +void frameCallback(Object? image, int durationMilliseconds, String decodeError) { + validateFrameCallback(image, durationMilliseconds, decodeError); } @pragma('vm:external-name', 'ValidateFrameCallback') -external void validateFrameCallback(Object? image, int durationMilliseconds); +external void validateFrameCallback(Object? image, int durationMilliseconds, String decodeError); @pragma('vm:entry-point') void platformMessagePortResponseTest() async { diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 8cbacfc328565..a76993710f3eb 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2105,9 +2105,12 @@ class Codec extends NativeFieldWrapperClass1 { /// [FrameInfo.image] on the returned object. Future getNextFrame() async { final Completer completer = Completer.sync(); - final String? error = _getNextFrame((_Image? image, int durationMilliseconds) { + final String? error = _getNextFrame((_Image? image, int durationMilliseconds, String decodeError) { if (image == null) { - completer.completeError(Exception('Codec failed to produce an image, possibly due to invalid image data.')); + if (decodeError.isEmpty) { + decodeError = 'Codec failed to produce an image, possibly due to invalid image data.'; + } + completer.completeError(Exception(decodeError)); } else { completer.complete(FrameInfo._( image: Image._(image, image.width, image.height), @@ -2123,7 +2126,7 @@ class Codec extends NativeFieldWrapperClass1 { /// Returns an error message on failure, null on success. @Native, Handle)>(symbol: 'Codec::getNextFrame') - external String? _getNextFrame(void Function(_Image?, int) callback); + external String? _getNextFrame(void Function(_Image?, int, String) callback); /// Release the resources used by this object. The object is no longer usable /// after this method is called. diff --git a/lib/ui/painting/image_decoder.h b/lib/ui/painting/image_decoder.h index 0d4f18e109cbe..f85308e39fcbe 100644 --- a/lib/ui/painting/image_decoder.h +++ b/lib/ui/painting/image_decoder.h @@ -31,7 +31,7 @@ class ImageDecoder { virtual ~ImageDecoder(); - using ImageResult = std::function)>; + using ImageResult = std::function, std::string)>; // Takes an image descriptor and returns a handle to a texture resident on the // GPU. All image decompression and resizes are done on a worker thread diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 4db68ac2fe98b..f97e007ae9084 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -123,6 +123,7 @@ static std::optional ToPixelFormat(SkColorType type) { } std::optional ImageDecoderImpeller::DecompressTexture( +DecompressResult ImageDecoderImpeller::DecompressTexture( ImageDescriptor* descriptor, SkISize target_size, impeller::ISize max_texture_size, @@ -130,8 +131,9 @@ std::optional ImageDecoderImpeller::DecompressTexture( const std::shared_ptr& allocator) { TRACE_EVENT0("impeller", __FUNCTION__); if (!descriptor) { - FML_DLOG(ERROR) << "Invalid descriptor."; - return std::nullopt; + std::string decode_error("Invalid descriptor (should never happen)"); + FML_DLOG(ERROR) << decode_error; + return DecompressResult{.decode_error = decode_error}; } target_size.set(std::min(static_cast(max_texture_size.width), @@ -176,8 +178,11 @@ std::optional ImageDecoderImpeller::DecompressTexture( const auto pixel_format = ToPixelFormat(image_info.colorType()); if (!pixel_format.has_value()) { - FML_DLOG(ERROR) << "Codec pixel format not supported by Impeller."; - return std::nullopt; + std::string decode_error(impeller::SPrintF( + "Codec pixel format is not supported (SkColorType=%d)", + image_info.colorType())); + FML_DLOG(ERROR) << decode_error; + return DecompressResult{.decode_error = decode_error}; } auto bitmap = std::make_shared(); @@ -186,14 +191,16 @@ std::optional ImageDecoderImpeller::DecompressTexture( if (descriptor->is_compressed()) { if (!bitmap->tryAllocPixels(bitmap_allocator.get())) { - FML_DLOG(ERROR) - << "Could not allocate intermediate for image decompression."; - return std::nullopt; + std::string decode_error( + "Could not allocate intermediate for image decompression."); + FML_DLOG(ERROR) << decode_error; + return DecompressResult{.decode_error = decode_error}; } // Decode the image into the image generator's closest supported size. if (!descriptor->get_pixels(bitmap->pixmap())) { - FML_DLOG(ERROR) << "Could not decompress image."; - return std::nullopt; + std::string decode_error("Could not decompress image."); + FML_DLOG(ERROR) << decode_error; + return DecompressResult{.decode_error = decode_error}; } } else { auto temp_bitmap = std::make_shared(); @@ -203,9 +210,10 @@ std::optional ImageDecoderImpeller::DecompressTexture( temp_bitmap->setPixelRef(pixel_ref, 0, 0); if (!bitmap->tryAllocPixels(bitmap_allocator.get())) { - FML_DLOG(ERROR) - << "Could not allocate intermediate for pixel conversion."; - return std::nullopt; + std::string decode_error( + "Could not allocate intermediate for pixel conversion."); + FML_DLOG(ERROR) << decode_error; + return DecompressResult{.decode_error = decode_error}; } temp_bitmap->readPixels(bitmap->pixmap()); bitmap->setImmutable(); @@ -214,7 +222,7 @@ std::optional ImageDecoderImpeller::DecompressTexture( if (bitmap->dimensions() == target_size) { auto buffer = bitmap_allocator->GetDeviceBuffer(); if (!buffer.has_value()) { - return std::nullopt; + return DecompressResult{.decode_error = "Unable to get device buffer"}; } return DecompressResult{.device_buffer = buffer.value(), .sk_bitmap = bitmap, @@ -232,9 +240,10 @@ std::optional ImageDecoderImpeller::DecompressTexture( auto scaled_allocator = std::make_shared(allocator); scaled_bitmap->setInfo(scaled_image_info); if (!scaled_bitmap->tryAllocPixels(scaled_allocator.get())) { - FML_LOG(ERROR) - << "Could not allocate scaled bitmap for image decompression."; - return std::nullopt; + 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(), @@ -245,25 +254,31 @@ std::optional ImageDecoderImpeller::DecompressTexture( auto buffer = scaled_allocator->GetDeviceBuffer(); if (!buffer.has_value()) { - return std::nullopt; + return DecompressResult{.decode_error = "Unable to get device buffer"}; } return DecompressResult{.device_buffer = buffer.value(), .sk_bitmap = scaled_bitmap, .image_info = scaled_bitmap->info()}; } -sk_sp ImageDecoderImpeller::UploadTextureToPrivate( +std::pair, std::string> +ImageDecoderImpeller::UploadTextureToPrivate( const std::shared_ptr& context, const std::shared_ptr& buffer, const SkImageInfo& image_info) { TRACE_EVENT0("impeller", __FUNCTION__); - if (!context || !buffer) { - return nullptr; + if (!context) { + return std::make_pair(nullptr, "No Impeller context is available"); + } + if (!buffer) { + return std::make_pair(nullptr, "No Impeller device buffer is available"); } const auto pixel_format = ToPixelFormat(image_info.colorType()); if (!pixel_format) { - FML_DLOG(ERROR) << "Pixel format unsupported by Impeller."; - return nullptr; + 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; @@ -276,8 +291,9 @@ sk_sp ImageDecoderImpeller::UploadTextureToPrivate( auto dest_texture = context->GetResourceAllocator()->CreateTexture(texture_descriptor); if (!dest_texture) { - FML_DLOG(ERROR) << "Could not create Impeller texture."; - return nullptr; + std::string decode_error("Could not create Impeller texture."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } dest_texture->SetLabel( @@ -285,15 +301,19 @@ sk_sp ImageDecoderImpeller::UploadTextureToPrivate( auto command_buffer = context->CreateCommandBuffer(); if (!command_buffer) { - FML_DLOG(ERROR) << "Could not create command buffer for mipmap generation."; - return nullptr; + std::string decode_error( + "Could not create command buffer for mipmap generation."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } command_buffer->SetLabel("Mipmap Command Buffer"); auto blit_pass = command_buffer->CreateBlitPass(); if (!blit_pass) { - FML_DLOG(ERROR) << "Could not create blit pass for mipmap generation."; - return nullptr; + std::string decode_error( + "Could not create blit pass for mipmap generation."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } blit_pass->SetLabel("Mipmap Blit Pass"); blit_pass->AddCopy(buffer->AsBufferView(), dest_texture); @@ -303,26 +323,34 @@ sk_sp ImageDecoderImpeller::UploadTextureToPrivate( blit_pass->EncodeCommands(context->GetResourceAllocator()); if (!command_buffer->SubmitCommands()) { - FML_DLOG(ERROR) << "Failed to submit blit pass command buffer."; - return nullptr; + std::string decode_error("Failed to submit blit pass command buffer."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } - return impeller::DlImageImpeller::Make(std::move(dest_texture)); + return std::make_pair( + impeller::DlImageImpeller::Make(std::move(dest_texture)), std::string()); } -sk_sp ImageDecoderImpeller::UploadTextureToShared( +std::pair, std::string> +ImageDecoderImpeller::UploadTextureToShared( const std::shared_ptr& context, std::shared_ptr bitmap, bool create_mips) { TRACE_EVENT0("impeller", __FUNCTION__); - if (!context || !bitmap) { - return nullptr; + 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 = ToPixelFormat(image_info.colorType()); if (!pixel_format) { - FML_DLOG(ERROR) << "Pixel format unsupported by Impeller."; - return nullptr; + 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; @@ -335,8 +363,9 @@ sk_sp ImageDecoderImpeller::UploadTextureToShared( auto texture = context->GetResourceAllocator()->CreateTexture(texture_descriptor); if (!texture) { - FML_DLOG(ERROR) << "Could not create Impeller texture."; - return nullptr; + 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( @@ -346,8 +375,9 @@ sk_sp ImageDecoderImpeller::UploadTextureToShared( ); if (!texture->SetContents(mapping)) { - FML_DLOG(ERROR) << "Could not copy contents into Impeller texture."; - return nullptr; + 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()); @@ -355,28 +385,33 @@ sk_sp ImageDecoderImpeller::UploadTextureToShared( if (texture_descriptor.mip_count > 1u && create_mips) { auto command_buffer = context->CreateCommandBuffer(); if (!command_buffer) { - FML_DLOG(ERROR) - << "Could not create command buffer for mipmap generation."; - return nullptr; + std::string decode_error( + "Could not create command buffer for mipmap generation."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } command_buffer->SetLabel("Mipmap Command Buffer"); auto blit_pass = command_buffer->CreateBlitPass(); if (!blit_pass) { - FML_DLOG(ERROR) << "Could not create blit pass for mipmap generation."; - return nullptr; + std::string decode_error( + "Could not create blit pass for mipmap generation."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } blit_pass->SetLabel("Mipmap Blit Pass"); blit_pass->GenerateMipmap(texture); blit_pass->EncodeCommands(context->GetResourceAllocator()); if (!command_buffer->SubmitCommands()) { - FML_DLOG(ERROR) << "Failed to submit blit pass command buffer."; - return nullptr; + std::string decode_error("Failed to submit blit pass command buffer."); + FML_DLOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } } - return impeller::DlImageImpeller::Make(std::move(texture)); + return std::make_pair(impeller::DlImageImpeller::Make(std::move(texture)), + std::string()); } // |ImageDecoder| @@ -393,10 +428,10 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, ImageResult result = [p_result, // raw_descriptor, // ui_runner = runners_.GetUITaskRunner() // - ](auto image) { - ui_runner->PostTask([raw_descriptor, p_result, image]() { + ](auto image, auto decode_error) { + ui_runner->PostTask([raw_descriptor, p_result, image, decode_error]() { raw_descriptor->Release(); - p_result(std::move(image)); + p_result(std::move(image), decode_error); }); }; @@ -409,7 +444,7 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, supports_wide_gamut = supports_wide_gamut_ // ]() { if (!context) { - result(nullptr); + result(nullptr, "No Impeller context is available"); return; } auto max_size_supported = @@ -419,21 +454,24 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, auto bitmap_result = DecompressTexture( raw_descriptor, target_size, max_size_supported, supports_wide_gamut, context->GetResourceAllocator()); - if (!bitmap_result.has_value()) { - result(nullptr); + if (!bitmap_result.device_buffer) { + result(nullptr, bitmap_result.decode_error); return; } auto upload_texture_and_invoke_result = [result, context, - bitmap_result = - bitmap_result.value()]() { -// TODO(jonahwilliams): remove ifdef once blit from buffer to texture is -// implemented on other platforms. -#ifdef FML_OS_IOS - result(UploadTextureToPrivate(context, bitmap_result.device_buffer, - bitmap_result.image_info)); + bitmap_result]() { + // TODO(jonahwilliams): remove ifdef once blit from buffer to + // texture is implemented on other platforms. + sk_sp image; + std::string decode_error; +#if (FML_OS_IOS && !TARGET_IPHONE_SIMULATOR) + std::tie(image, decode_error) = UploadTextureToPrivate( + context, bitmap_result.device_buffer, bitmap_result.image_info); #else - result(UploadTextureToShared(context, bitmap_result.sk_bitmap)); + std::tie(image, decode_error) = + UploadTextureToShared(context, bitmap_result.sk_bitmap); #endif + result(image, decode_error); }; // TODO(jonahwilliams): https://github.com/flutter/flutter/issues/123058 // Technically we don't need to post tasks to the io runner, but without diff --git a/lib/ui/painting/image_decoder_impeller.h b/lib/ui/painting/image_decoder_impeller.h index 9df6b223104d1..c764df6464d14 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -41,6 +41,7 @@ struct DecompressResult { std::shared_ptr device_buffer; std::shared_ptr sk_bitmap; SkImageInfo image_info; + std::string decode_error; }; class ImageDecoderImpeller final : public ImageDecoder { @@ -59,7 +60,7 @@ class ImageDecoderImpeller final : public ImageDecoder { uint32_t target_height, const ImageResult& result) override; - static std::optional DecompressTexture( + static DecompressResult DecompressTexture( ImageDescriptor* descriptor, SkISize target_size, impeller::ISize max_texture_size, @@ -72,7 +73,7 @@ class ImageDecoderImpeller final : public ImageDecoder { /// @param buffer A host buffer containing the image to be uploaded. /// @param image_info Format information about the particular image. /// @return A DlImage. - static sk_sp UploadTextureToPrivate( + static std::pair, std::string> UploadTextureToPrivate( const std::shared_ptr& context, const std::shared_ptr& buffer, const SkImageInfo& image_info); @@ -83,7 +84,7 @@ class ImageDecoderImpeller final : public ImageDecoder { /// @param create_mips Whether mipmaps should be generated for the given /// image. /// @return A DlImage. - static sk_sp UploadTextureToShared( + static std::pair, std::string> UploadTextureToShared( const std::shared_ptr& context, std::shared_ptr bitmap, bool create_mips = true); diff --git a/lib/ui/painting/image_decoder_skia.cc b/lib/ui/painting/image_decoder_skia.cc index e1145c1c408f9..2cb609ea45608 100644 --- a/lib/ui/painting/image_decoder_skia.cc +++ b/lib/ui/painting/image_decoder_skia.cc @@ -254,7 +254,7 @@ void ImageDecoderSkia::Decode(fml::RefPtr descriptor_ref_ptr, // terminate without a base trace. Add one explicitly. TRACE_EVENT0("flutter", "ImageDecodeCallback"); flow.End(); - callback(DlImageGPU::Make(std::move(image))); + callback(DlImageGPU::Make(std::move(image)), {}); raw_descriptor->Release(); })); }; diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 0cc5165d53744..e079f26ba460c 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -338,7 +338,8 @@ TEST_F(ImageDecoderFixtureTest, InvalidImageResultsError) { fml::MakeRefCounted( std::move(data), std::make_unique()); - ImageDecoder::ImageResult callback = [&](const sk_sp& image) { + ImageDecoder::ImageResult callback = [&](const sk_sp& image, + const std::string& decode_error) { ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); ASSERT_FALSE(image); latch.Signal(); @@ -384,7 +385,8 @@ TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) { auto descriptor = fml::MakeRefCounted( std::move(data), std::move(generator)); - ImageDecoder::ImageResult callback = [&](const sk_sp& image) { + ImageDecoder::ImageResult callback = [&](const sk_sp& image, + const std::string& decode_error) { ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); ASSERT_TRUE(image && image->skia_image()); EXPECT_TRUE(io_manager->did_access_is_gpu_disabled_sync_switch_); @@ -655,7 +657,8 @@ TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) { auto descriptor = fml::MakeRefCounted( std::move(data), std::move(generator)); - ImageDecoder::ImageResult callback = [&](const sk_sp& image) { + ImageDecoder::ImageResult callback = [&](const sk_sp& image, + const std::string& decode_error) { ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); ASSERT_TRUE(image && image->skia_image()); decoded_size = image->skia_image()->dimensions(); @@ -715,7 +718,8 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithoutAGPUContext) { auto descriptor = fml::MakeRefCounted( std::move(data), std::move(generator)); - ImageDecoder::ImageResult callback = [&](const sk_sp& image) { + ImageDecoder::ImageResult callback = [&](const sk_sp& image, + const std::string& decode_error) { ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); ASSERT_TRUE(image && image->skia_image()); runners.GetIOTaskRunner()->PostTask(release_io_manager); @@ -786,12 +790,13 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithResizes) { auto descriptor = fml::MakeRefCounted( std::move(data), std::move(generator)); - ImageDecoder::ImageResult callback = [&](const sk_sp& image) { - ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); - ASSERT_TRUE(image && image->skia_image()); - final_size = image->skia_image()->dimensions(); - latch.Signal(); - }; + ImageDecoder::ImageResult callback = + [&](const sk_sp& image, const std::string& decode_error) { + ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); + ASSERT_TRUE(image && image->skia_image()); + final_size = image->skia_image()->dimensions(); + latch.Signal(); + }; image_decoder->Decode(descriptor, target_width, target_height, callback); }); latch.Wait(); @@ -857,12 +862,14 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto result_1 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(6, 2), {100, 100}, /*supports_wide_gamut=*/false, allocator); - ASSERT_EQ(result_1->sk_bitmap->dimensions(), SkISize::Make(6, 2)); + ASSERT_EQ(result_1.sk_bitmap->width(), 6); + ASSERT_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->dimensions(), SkISize::Make(10, 10)); + ASSERT_EQ(result_2.sk_bitmap->width(), 10); + ASSERT_EQ(result_2.sk_bitmap->height(), 10); #endif // IMPELLER_SUPPORTS_RENDERING } @@ -890,13 +897,13 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { ASSERT_TRUE(expected_data != nullptr); ASSERT_FALSE(expected_data->isEmpty()); - auto assert_image = [&](auto decoded_image) { + auto assert_image = [&](auto decoded_image, const std::string& decode_error) { ASSERT_EQ(decoded_image->dimensions(), SkISize::Make(300, 100)); ASSERT_TRUE(decoded_image->encodeToData(SkEncodedImageFormat::kPNG, 100) ->equals(expected_data.get())); }; - assert_image(decode(300, 100)); + assert_image(decode(300, 100), {}); } TEST_F(ImageDecoderFixtureTest, diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 796ea4645f8b1..1470b357e6ce6 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -38,6 +38,7 @@ MultiFrameCodec::State::State(std::shared_ptr generator) static void InvokeNextFrameCallback( const fml::RefPtr& image, int duration, + const std::string& decode_error, std::unique_ptr callback, size_t trace_id) { std::shared_ptr dart_state = callback->dart_state().lock(); @@ -48,7 +49,8 @@ static void InvokeNextFrameCallback( } tonic::DartState::Scope scope(dart_state); tonic::DartInvoke(callback->value(), - {tonic::ToDart(image), tonic::ToDart(duration)}); + {tonic::ToDart(image), tonic::ToDart(duration), + tonic::ToDart(decode_error)}); } // Copied the source bitmap to the destination. If this cannot occur due to @@ -84,7 +86,8 @@ static bool CopyToBitmap(SkBitmap* dst, return true; } -sk_sp MultiFrameCodec::State::GetNextFrameImage( +std::pair, std::string> +MultiFrameCodec::State::GetNextFrameImage( fml::WeakPtr resourceContext, const std::shared_ptr& gpu_disable_sync_switch, const std::shared_ptr& impeller_context, @@ -96,9 +99,12 @@ sk_sp MultiFrameCodec::State::GetNextFrameImage( info = updated; } if (!bitmap.tryAllocPixels(info)) { - FML_LOG(ERROR) << "Failed to allocate memory for bitmap of size " - << info.computeMinByteSize() << "B"; - return nullptr; + std::ostringstream ostr; + ostr << "Failed to allocate memory for bitmap of size " + << info.computeMinByteSize() << "B"; + std::string decode_error = ostr.str(); + FML_LOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } ImageGenerator::FrameInfo frameInfo = @@ -132,8 +138,11 @@ sk_sp MultiFrameCodec::State::GetNextFrameImage( // are already set in accordance with the previous frame's disposal policy. if (!generator_->GetPixels(info, bitmap.getPixels(), bitmap.rowBytes(), nextFrameIndex_, requiredFrameIndex)) { - FML_LOG(ERROR) << "Could not getPixels for frame " << nextFrameIndex_; - return nullptr; + std::ostringstream ostr; + ostr << "Could not getPixels for frame " << nextFrameIndex_; + std::string decode_error = ostr.str(); + FML_LOG(ERROR) << decode_error; + return std::make_pair(nullptr, decode_error); } // Hold onto this if we need it to decode future frames. @@ -176,7 +185,8 @@ sk_sp MultiFrameCodec::State::GetNextFrameImage( } })); - return DlImageGPU::Make({skImage, std::move(unref_queue)}); + return std::make_pair(DlImageGPU::Make({skImage, std::move(unref_queue)}), + std::string()); } void MultiFrameCodec::State::GetNextFrameAndInvokeCallback( @@ -189,7 +199,9 @@ void MultiFrameCodec::State::GetNextFrameAndInvokeCallback( const std::shared_ptr& impeller_context) { fml::RefPtr image = nullptr; int duration = 0; - sk_sp dlImage = + sk_sp dlImage; + std::string decode_error; + std::tie(dlImage, decode_error) = GetNextFrameImage(std::move(resourceContext), gpu_disable_sync_switch, impeller_context, std::move(unref_queue)); if (dlImage) { @@ -203,11 +215,12 @@ void MultiFrameCodec::State::GetNextFrameAndInvokeCallback( // The static leak checker gets confused by the use of fml::MakeCopyable. // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) - ui_task_runner->PostTask(fml::MakeCopyable([callback = std::move(callback), - image = std::move(image), - duration, trace_id]() mutable { - InvokeNextFrameCallback(image, duration, std::move(callback), trace_id); - })); + ui_task_runner->PostTask(fml::MakeCopyable( + [callback = std::move(callback), image = std::move(image), + decode_error = std::move(decode_error), duration, trace_id]() mutable { + InvokeNextFrameCallback(image, duration, decode_error, + std::move(callback), trace_id); + })); } Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { @@ -223,12 +236,14 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { const auto& task_runners = dart_state->GetTaskRunners(); if (state_->frameCount_ == 0) { - FML_LOG(ERROR) << "Could not provide any frame."; + std::string decode_error("Could not provide any frame."); + FML_LOG(ERROR) << decode_error; task_runners.GetUITaskRunner()->PostTask(fml::MakeCopyable( - [trace_id, + [trace_id, decode_error = std::move(decode_error), callback = std::make_unique( tonic::DartState::Current(), callback_handle)]() mutable { - InvokeNextFrameCallback(nullptr, 0, std::move(callback), trace_id); + InvokeNextFrameCallback(nullptr, 0, decode_error, std::move(callback), + trace_id); })); return Dart_Null(); } diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index f569bbd7ebd58..a5f6acf07e809 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -9,6 +9,8 @@ #include "flutter/lib/ui/painting/codec.h" #include "flutter/lib/ui/painting/image_generator.h" +#include + using tonic::DartPersistentValue; namespace flutter { @@ -56,7 +58,7 @@ class MultiFrameCodec : public Codec { // The index of the last decoded required frame. int lastRequiredFrameIndex_ = -1; - sk_sp GetNextFrameImage( + std::pair, std::string> GetNextFrameImage( fml::WeakPtr resourceContext, const std::shared_ptr& gpu_disable_sync_switch, const std::shared_ptr& impeller_context, diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index e962b405cd1f8..758cc56c055c8 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -36,8 +36,8 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { if (!cached_image_->image()) { return tonic::ToDart("Decoded image has been disposed"); } - tonic::DartInvoke(callback_handle, - {tonic::ToDart(cached_image_), tonic::ToDart(0)}); + tonic::DartInvoke(callback_handle, {tonic::ToDart(cached_image_), + tonic::ToDart(0), tonic::ToDart("")}); return Dart_Null(); } @@ -69,7 +69,8 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { new fml::RefPtr(this); decoder->Decode( - descriptor_, target_width_, target_height_, [raw_codec_ref](auto image) { + descriptor_, target_width_, target_height_, + [raw_codec_ref](auto image, auto decode_error) { std::unique_ptr> codec_ref(raw_codec_ref); fml::RefPtr codec(std::move(*codec_ref)); @@ -97,9 +98,9 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { // Invoke any callbacks that were provided before the frame was decoded. for (const DartPersistentValue& callback : codec->pending_callbacks_) { - tonic::DartInvoke( - callback.value(), - {tonic::ToDart(codec->cached_image_), tonic::ToDart(0)}); + tonic::DartInvoke(callback.value(), + {tonic::ToDart(codec->cached_image_), + tonic::ToDart(0), tonic::ToDart(decode_error)}); } codec->pending_callbacks_.clear(); }); diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 0a4e5a603b8a5..ffb42c35d7adf 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -326,7 +326,7 @@ void onBeginFrameWithNotifyNativeMain() { } @pragma('vm:entry-point') -void frameCallback(Object? image, int durationMilliseconds) { +void frameCallback(Object? image, int durationMilliseconds, String decodeError) { if (image == null) { throw Exception('Expeccted image in frame callback to be non-null'); } From 72bc4515652d55d749a00b6863e7a467b05490d9 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 26 May 2023 11:46:33 -0700 Subject: [PATCH 2/4] CP2 --- .../renderer/backend/metal/render_pass_mtl.mm | 3 + lib/ui/painting/image_decoder.cc | 6 +- lib/ui/painting/image_decoder.h | 3 +- lib/ui/painting/image_decoder_impeller.cc | 127 ++++++++++++------ lib/ui/painting/image_decoder_impeller.h | 13 +- lib/ui/painting/image_decoder_unittests.cc | 58 ++++++-- lib/ui/painting/multi_frame_codec.cc | 1 + shell/common/engine.cc | 18 ++- shell/common/engine.h | 9 +- shell/common/engine_unittests.cc | 44 +++--- shell/common/shell.cc | 37 ++--- shell/common/shell.h | 4 +- 12 files changed, 223 insertions(+), 100 deletions(-) diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index 4c912e9772935..105da47ba946f 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -383,10 +383,13 @@ static bool Bind(PassBindingsCache& pass, } if (texture.NeedsMipmapGeneration()) { + // TODO(127697): generate mips when the GPU is available on iOS. +#if !FML_OS_IOS VALIDATION_LOG << "Texture at binding index " << bind_index << " has a mip count > 1, but the mipmap has not been generated."; return false; +#endif // !FML_OS_IOS } return pass.SetTexture(stage, bind_index, diff --git a/lib/ui/painting/image_decoder.cc b/lib/ui/painting/image_decoder.cc index 905ffe4b9d217..b598de3823058 100644 --- a/lib/ui/painting/image_decoder.cc +++ b/lib/ui/painting/image_decoder.cc @@ -16,14 +16,16 @@ std::unique_ptr ImageDecoder::Make( const Settings& settings, const TaskRunners& runners, std::shared_ptr concurrent_task_runner, - fml::WeakPtr io_manager) { + fml::WeakPtr io_manager, + const std::shared_ptr& gpu_disabled_switch) { #if IMPELLER_SUPPORTS_RENDERING if (settings.enable_impeller) { return std::make_unique( runners, // std::move(concurrent_task_runner), // std::move(io_manager), // - settings.enable_wide_gamut); + settings.enable_wide_gamut, // + gpu_disabled_switch); } #endif // IMPELLER_SUPPORTS_RENDERING return std::make_unique( diff --git a/lib/ui/painting/image_decoder.h b/lib/ui/painting/image_decoder.h index f85308e39fcbe..ebb104efd44f7 100644 --- a/lib/ui/painting/image_decoder.h +++ b/lib/ui/painting/image_decoder.h @@ -27,7 +27,8 @@ class ImageDecoder { const Settings& settings, const TaskRunners& runners, std::shared_ptr concurrent_task_runner, - fml::WeakPtr io_manager); + fml::WeakPtr io_manager, + const std::shared_ptr& gpu_disabled_switch); virtual ~ImageDecoder(); diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index f97e007ae9084..6e4f14242d16b 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -79,9 +79,11 @@ ImageDecoderImpeller::ImageDecoderImpeller( const TaskRunners& runners, std::shared_ptr concurrent_task_runner, const fml::WeakPtr& io_manager, - bool supports_wide_gamut) + bool supports_wide_gamut, + const std::shared_ptr& gpu_disabled_switch) : ImageDecoder(runners, std::move(concurrent_task_runner), io_manager), - supports_wide_gamut_(supports_wide_gamut) { + supports_wide_gamut_(supports_wide_gamut), + gpu_disabled_switch_(gpu_disabled_switch) { std::promise> context_promise; context_ = context_promise.get_future(); runners_.GetIOTaskRunner()->PostTask(fml::MakeCopyable( @@ -261,8 +263,8 @@ DecompressResult ImageDecoderImpeller::DecompressTexture( .image_info = scaled_bitmap->info()}; } -std::pair, std::string> -ImageDecoderImpeller::UploadTextureToPrivate( +/// 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) { @@ -332,10 +334,40 @@ ImageDecoderImpeller::UploadTextureToPrivate( impeller::DlImageImpeller::Make(std::move(dest_texture)), std::string()); } +std::pair, std::string> +ImageDecoderImpeller::UploadTextureToPrivate( + const std::shared_ptr& context, + const std::shared_ptr& buffer, + const SkImageInfo& image_info, + std::shared_ptr bitmap, + const std::shared_ptr& gpu_disabled_switch) { + TRACE_EVENT0("impeller", __FUNCTION__); + if (!context) { + return std::make_pair(nullptr, "No Impeller context is available"); + } + if (!buffer) { + return std::make_pair(nullptr, "No Impeller device buffer is available"); + } + + std::pair, std::string> result; + gpu_disabled_switch->Execute( + fml::SyncSwitch::Handlers() + .SetIfFalse([&result, context, buffer, image_info] { + result = UnsafeUploadTextureToPrivate(context, buffer, image_info); + }) + .SetIfTrue([&result, context, bitmap, gpu_disabled_switch] { + // create_mips is false because we already know the GPU is disabled. + result = UploadTextureToShared(context, bitmap, gpu_disabled_switch, + /*create_mips=*/false); + })); + return result; +} + std::pair, std::string> ImageDecoderImpeller::UploadTextureToShared( const std::shared_ptr& context, std::shared_ptr bitmap, + const std::shared_ptr& gpu_disabled_switch, bool create_mips) { TRACE_EVENT0("impeller", __FUNCTION__); if (!context) { @@ -383,30 +415,39 @@ ImageDecoderImpeller::UploadTextureToShared( texture->SetLabel(impeller::SPrintF("ui.Image(%p)", texture.get()).c_str()); if (texture_descriptor.mip_count > 1u && create_mips) { - auto command_buffer = context->CreateCommandBuffer(); - if (!command_buffer) { - std::string decode_error( - "Could not create command buffer for mipmap generation."); - FML_DLOG(ERROR) << decode_error; - return std::make_pair(nullptr, decode_error); - } - command_buffer->SetLabel("Mipmap Command Buffer"); - - auto blit_pass = command_buffer->CreateBlitPass(); - if (!blit_pass) { - std::string decode_error( - "Could not create blit pass for mipmap generation."); - FML_DLOG(ERROR) << decode_error; - return std::make_pair(nullptr, decode_error); - } - blit_pass->SetLabel("Mipmap Blit Pass"); - blit_pass->GenerateMipmap(texture); - - blit_pass->EncodeCommands(context->GetResourceAllocator()); - if (!command_buffer->SubmitCommands()) { - std::string decode_error("Failed to submit blit pass command buffer."); - FML_DLOG(ERROR) << decode_error; - return std::make_pair(nullptr, decode_error); + 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 (!command_buffer->SubmitCommands()) { + 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()); } } @@ -441,8 +482,8 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, target_size = SkISize::Make(target_width, target_height), // io_runner = runners_.GetIOTaskRunner(), // result, - supports_wide_gamut = supports_wide_gamut_ // - ]() { + supports_wide_gamut = supports_wide_gamut_, // + gpu_disabled_switch = gpu_disabled_switch_]() { if (!context) { result(nullptr, "No Impeller context is available"); return; @@ -458,24 +499,28 @@ void ImageDecoderImpeller::Decode(fml::RefPtr descriptor, result(nullptr, bitmap_result.decode_error); return; } - auto upload_texture_and_invoke_result = [result, context, - bitmap_result]() { - // TODO(jonahwilliams): remove ifdef once blit from buffer to - // texture is implemented on other platforms. + auto upload_texture_and_invoke_result = [result, context, bitmap_result, + gpu_disabled_switch]() { + // TODO(jonahwilliams): remove ifdef once blit from buffer + // to texture is implemented on other platforms. sk_sp image; std::string decode_error; -#if (FML_OS_IOS && !TARGET_IPHONE_SIMULATOR) + +#ifdef FML_OS_IOS std::tie(image, decode_error) = UploadTextureToPrivate( - context, bitmap_result.device_buffer, bitmap_result.image_info); + context, bitmap_result.device_buffer, bitmap_result.image_info, + bitmap_result.sk_bitmap, gpu_disabled_switch); #else std::tie(image, decode_error) = - UploadTextureToShared(context, bitmap_result.sk_bitmap); -#endif + UploadTextureToShared(context, bitmap_result.sk_bitmap, + gpu_disabled_switch, /*create_mips=*/true); +#endif // FML_OS_IOS result(image, decode_error); }; - // TODO(jonahwilliams): https://github.com/flutter/flutter/issues/123058 - // Technically we don't need to post tasks to the io runner, but without - // this forced serialization we can end up overloading the GPU and/or + // TODO(jonahwilliams): + // https://github.com/flutter/flutter/issues/123058 Technically we + // don't need to post tasks to the io runner, but without this + // forced serialization we can end up overloading the GPU and/or // competing with raster workloads. 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 c764df6464d14..a9c1af42de9d0 100644 --- a/lib/ui/painting/image_decoder_impeller.h +++ b/lib/ui/painting/image_decoder_impeller.h @@ -50,7 +50,8 @@ class ImageDecoderImpeller final : public ImageDecoder { const TaskRunners& runners, std::shared_ptr concurrent_task_runner, const fml::WeakPtr& io_manager, - bool supports_wide_gamut); + bool supports_wide_gamut, + const std::shared_ptr& gpu_disabled_switch); ~ImageDecoderImpeller() override; @@ -72,27 +73,35 @@ class ImageDecoderImpeller final : public ImageDecoder { /// @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( const std::shared_ptr& context, const std::shared_ptr& buffer, - const SkImageInfo& image_info); + const SkImageInfo& image_info, + std::shared_ptr bitmap, + const std::shared_ptr& gpu_disabled_switch); /// @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> UploadTextureToShared( const std::shared_ptr& context, std::shared_ptr bitmap, + const std::shared_ptr& gpu_disabled_switch, bool create_mips = true); private: using FutureContext = std::shared_future>; FutureContext context_; const bool supports_wide_gamut_; + std::shared_ptr gpu_disabled_switch_; 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 e079f26ba460c..13cefc619cfab 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -136,9 +136,12 @@ class TestImpellerContext : public impeller::Context { } std::shared_ptr CreateCommandBuffer() const override { + command_buffer_count_ += 1; return nullptr; } + mutable size_t command_buffer_count_ = 0; + private: std::shared_ptr capabilities_; }; @@ -278,7 +281,8 @@ TEST_F(ImageDecoderFixtureTest, CanCreateImageDecoder) { TestIOManager manager(runners.GetIOTaskRunner()); Settings settings; auto decoder = ImageDecoder::Make(settings, runners, loop->GetTaskRunner(), - manager.GetWeakIOManager()); + manager.GetWeakIOManager(), + std::make_shared()); ASSERT_NE(decoder, nullptr); }); } @@ -329,7 +333,8 @@ TEST_F(ImageDecoderFixtureTest, InvalidImageResultsError) { TestIOManager manager(runners.GetIOTaskRunner()); Settings settings; auto decoder = ImageDecoder::Make(settings, runners, loop->GetTaskRunner(), - manager.GetWeakIOManager()); + manager.GetWeakIOManager(), + std::make_shared()); auto data = OpenFixtureAsSkData("ThisDoesNotExist.jpg"); ASSERT_FALSE(data); @@ -368,9 +373,9 @@ TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) { }; auto decode_image = [&]() { Settings settings; - std::unique_ptr image_decoder = - ImageDecoder::Make(settings, runners, loop->GetTaskRunner(), - io_manager->GetWeakIOManager()); + std::unique_ptr image_decoder = ImageDecoder::Make( + settings, runners, loop->GetTaskRunner(), + io_manager->GetWeakIOManager(), std::make_shared()); auto data = OpenFixtureAsSkData("DashInNooglerHat.jpg"); @@ -424,6 +429,34 @@ float HalfToFloat(uint16_t half) { } } // namespace +TEST_F(ImageDecoderFixtureTest, ImpellerUploadToSharedNoGpu) { +#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); + + auto result = ImageDecoderImpeller::UploadTextureToPrivate( + no_gpu_access_context, buffer, info, bitmap, gpu_disabled_switch); + ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); + ASSERT_EQ(result.second, ""); + + result = ImageDecoderImpeller::UploadTextureToShared( + no_gpu_access_context, bitmap, gpu_disabled_switch, true); + ASSERT_EQ(no_gpu_access_context->command_buffer_count_, 0ul); + ASSERT_EQ(result.second, ""); +} + TEST_F(ImageDecoderFixtureTest, ImpellerNullColorspace) { auto info = SkImageInfo::Make(10, 10, SkColorType::kRGBA_8888_SkColorType, SkAlphaType::kPremul_SkAlphaType); @@ -640,9 +673,9 @@ TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) { SkISize decoded_size = SkISize::MakeEmpty(); auto decode_image = [&]() { Settings settings; - std::unique_ptr image_decoder = - ImageDecoder::Make(settings, runners, loop->GetTaskRunner(), - io_manager->GetWeakIOManager()); + std::unique_ptr image_decoder = ImageDecoder::Make( + settings, runners, loop->GetTaskRunner(), + io_manager->GetWeakIOManager(), std::make_shared()); auto data = OpenFixtureAsSkData("Horizontal.jpg"); @@ -701,9 +734,9 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithoutAGPUContext) { auto decode_image = [&]() { Settings settings; - std::unique_ptr image_decoder = - ImageDecoder::Make(settings, runners, loop->GetTaskRunner(), - io_manager->GetWeakIOManager()); + std::unique_ptr image_decoder = ImageDecoder::Make( + settings, runners, loop->GetTaskRunner(), + io_manager->GetWeakIOManager(), std::make_shared()); auto data = OpenFixtureAsSkData("DashInNooglerHat.jpg"); @@ -769,7 +802,8 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithResizes) { PostTaskSync(runners.GetUITaskRunner(), [&]() { Settings settings; image_decoder = ImageDecoder::Make(settings, runners, loop->GetTaskRunner(), - io_manager->GetWeakIOManager()); + io_manager->GetWeakIOManager(), + std::make_shared()); }); // Setup a generic decoding utility that gives us the final decoded size. diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 1470b357e6ce6..21ac7b375c082 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -158,6 +158,7 @@ MultiFrameCodec::State::GetNextFrameImage( // without mipmap creation there is no command buffer encoding done. return ImageDecoderImpeller::UploadTextureToShared( impeller_context, std::make_shared(bitmap), + std::make_shared(), /*create_mips=*/false); } #endif // IMPELLER_SUPPORTS_RENDERING diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 522ee89a52a7b..100449bfd78ce 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -45,7 +45,8 @@ Engine::Engine( std::unique_ptr animator, fml::WeakPtr io_manager, const std::shared_ptr& font_collection, - std::unique_ptr runtime_controller) + std::unique_ptr runtime_controller, + const std::shared_ptr& gpu_disabled_switch) : delegate_(delegate), settings_(settings), animator_(std::move(animator)), @@ -54,7 +55,8 @@ Engine::Engine( image_decoder_(ImageDecoder::Make(settings_, task_runners, std::move(image_decoder_task_runner), - std::move(io_manager))), + std::move(io_manager), + gpu_disabled_switch)), task_runners_(task_runners), weak_factory_(this) { pointer_data_dispatcher_ = dispatcher_maker(*this); @@ -71,7 +73,8 @@ Engine::Engine(Delegate& delegate, fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::TaskRunnerAffineWeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker) + std::shared_ptr volatile_path_tracker, + const std::shared_ptr& gpu_disabled_switch) : Engine(delegate, dispatcher_maker, vm.GetConcurrentWorkerTaskRunner(), @@ -80,7 +83,8 @@ Engine::Engine(Delegate& delegate, std::move(animator), io_manager, std::make_shared(), - nullptr) { + nullptr, + gpu_disabled_switch) { runtime_controller_ = std::make_unique( *this, // runtime delegate &vm, // VM @@ -112,7 +116,8 @@ std::unique_ptr Engine::Spawn( std::unique_ptr animator, const std::string& initial_route, const fml::WeakPtr& io_manager, - fml::TaskRunnerAffineWeakPtr snapshot_delegate) const { + fml::TaskRunnerAffineWeakPtr snapshot_delegate, + const std::shared_ptr& gpu_disabled_switch) const { auto result = std::make_unique( /*delegate=*/delegate, /*dispatcher_maker=*/dispatcher_maker, @@ -123,7 +128,8 @@ std::unique_ptr Engine::Spawn( /*animator=*/std::move(animator), /*io_manager=*/io_manager, /*font_collection=*/font_collection_, - /*runtime_controller=*/nullptr); + /*runtime_controller=*/nullptr, + /*gpu_disabled_switch=*/gpu_disabled_switch); result->runtime_controller_ = runtime_controller_->Spawn( /*p_client=*/*result, /*advisory_script_uri=*/settings.advisory_script_uri, diff --git a/shell/common/engine.h b/shell/common/engine.h index c6ff37eae37a5..f45ddd280ca97 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -309,7 +309,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { std::unique_ptr animator, fml::WeakPtr io_manager, const std::shared_ptr& font_collection, - std::unique_ptr runtime_controller); + std::unique_ptr runtime_controller, + const std::shared_ptr& gpu_disabled_switch); //---------------------------------------------------------------------------- /// @brief Creates an instance of the engine. This is done by the Shell @@ -364,7 +365,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::TaskRunnerAffineWeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker); + std::shared_ptr volatile_path_tracker, + const std::shared_ptr& gpu_disabled_switch); //---------------------------------------------------------------------------- /// @brief Create a Engine that shares as many resources as @@ -382,7 +384,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { std::unique_ptr animator, const std::string& initial_route, const fml::WeakPtr& io_manager, - fml::TaskRunnerAffineWeakPtr snapshot_delegate) const; + fml::TaskRunnerAffineWeakPtr snapshot_delegate, + const std::shared_ptr& gpu_disabled_switch) const; //---------------------------------------------------------------------------- /// @brief Destroys the engine engine. Called by the shell on the UI task diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 3bfdd000c37eb..0848d79da155e 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -162,7 +162,8 @@ TEST_F(EngineTest, Create) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(runtime_controller_)); + /*runtime_controller=*/std::move(runtime_controller_), + /*gpu_disabled_switch=*/std::make_shared()); EXPECT_TRUE(engine); }); } @@ -183,7 +184,8 @@ TEST_F(EngineTest, DispatchPlatformMessageUnknown) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); fml::RefPtr response = fml::MakeRefCounted(); @@ -209,7 +211,8 @@ TEST_F(EngineTest, DispatchPlatformMessageInitialRoute) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); fml::RefPtr response = fml::MakeRefCounted(); @@ -242,7 +245,8 @@ TEST_F(EngineTest, DispatchPlatformMessageInitialRouteIgnored) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); fml::RefPtr response = fml::MakeRefCounted(); @@ -274,10 +278,12 @@ TEST_F(EngineTest, SpawnSharesFontLibrary) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); - auto spawn = engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, - std::string(), io_manager_, snapshot_delegate_); + auto spawn = + engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, + std::string(), io_manager_, snapshot_delegate_, nullptr); EXPECT_TRUE(spawn != nullptr); EXPECT_EQ(&engine->GetFontCollection(), &spawn->GetFontCollection()); }); @@ -300,10 +306,12 @@ TEST_F(EngineTest, SpawnWithCustomInitialRoute) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); - auto spawn = engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, - "/foo", io_manager_, snapshot_delegate_); + auto spawn = + engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, "/foo", + io_manager_, snapshot_delegate_, nullptr); EXPECT_TRUE(spawn != nullptr); ASSERT_EQ("/foo", spawn->InitialRoute()); }); @@ -332,14 +340,16 @@ TEST_F(EngineTest, SpawnResetsViewportMetrics) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); auto& old_platform_data = engine->GetRuntimeController()->GetPlatformData(); EXPECT_EQ(old_platform_data.viewport_metrics.physical_width, kViewWidth); EXPECT_EQ(old_platform_data.viewport_metrics.physical_height, kViewHeight); - auto spawn = engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, - std::string(), io_manager_, snapshot_delegate_); + auto spawn = + engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, + std::string(), io_manager_, snapshot_delegate_, nullptr); EXPECT_TRUE(spawn != nullptr); auto& new_viewport_metrics = spawn->GetRuntimeController()->GetPlatformData().viewport_metrics; @@ -365,14 +375,15 @@ TEST_F(EngineTest, SpawnWithCustomSettings) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); Settings custom_settings = settings_; custom_settings.persistent_isolate_data = std::make_shared("foo"); auto spawn = engine->Spawn(delegate_, dispatcher_maker_, custom_settings, nullptr, - std::string(), io_manager_, snapshot_delegate_); + std::string(), io_manager_, snapshot_delegate_, nullptr); EXPECT_TRUE(spawn != nullptr); auto new_persistent_isolate_data = const_cast(spawn->GetRuntimeController()) @@ -405,7 +416,8 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) { /*animator=*/std::move(animator_), /*io_manager=*/io_manager_, /*font_collection=*/std::make_shared(), - /*runtime_controller=*/std::move(mock_runtime_controller)); + /*runtime_controller=*/std::move(mock_runtime_controller), + /*gpu_disabled_switch=*/std::make_shared()); engine->LoadDartDeferredLibraryError(error_id, error_message, true); }); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 214b3b384569c..a6ad507f8db76 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -54,19 +54,21 @@ std::unique_ptr CreateEngine( const fml::WeakPtr& io_manager, const fml::RefPtr& unref_queue, const fml::TaskRunnerAffineWeakPtr& snapshot_delegate, - const std::shared_ptr& volatile_path_tracker) { - return std::make_unique(delegate, // - dispatcher_maker, // - vm, // - isolate_snapshot, // - task_runners, // - platform_data, // - settings, // - std::move(animator), // - io_manager, // - unref_queue, // - snapshot_delegate, // - volatile_path_tracker); + const std::shared_ptr& volatile_path_tracker, + const std::shared_ptr& gpu_disabled_switch) { + return std::make_unique(delegate, // + dispatcher_maker, // + vm, // + isolate_snapshot, // + task_runners, // + platform_data, // + settings, // + std::move(animator), // + io_manager, // + unref_queue, // + snapshot_delegate, // + volatile_path_tracker, // + gpu_disabled_switch); } // Though there can be multiple shells, some settings apply to all components in @@ -301,7 +303,8 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( weak_io_manager_future.get(), // unref_queue_future.get(), // snapshot_delegate_future.get(), // - shell->volatile_path_tracker_)); + shell->volatile_path_tracker_, + shell->is_gpu_disabled_sync_switch_)); })); if (!shell->Setup(std::move(platform_view), // @@ -538,7 +541,8 @@ std::unique_ptr Shell::Spawn( const fml::WeakPtr& io_manager, const fml::RefPtr& unref_queue, fml::TaskRunnerAffineWeakPtr snapshot_delegate, - const std::shared_ptr& volatile_path_tracker) { + const std::shared_ptr& volatile_path_tracker, + const std::shared_ptr& is_gpu_disabled_sync_switch) { return engine->Spawn( /*delegate=*/delegate, /*dispatcher_maker=*/dispatcher_maker, @@ -546,7 +550,8 @@ std::unique_ptr Shell::Spawn( /*animator=*/std::move(animator), /*initial_route=*/initial_route, /*io_manager=*/io_manager, - /*snapshot_delegate=*/std::move(snapshot_delegate)); + /*snapshot_delegate=*/std::move(snapshot_delegate), + /*gpu_disabled_switch=*/is_gpu_disabled_sync_switch); }, is_gpu_disabled); result->RunEngine(std::move(run_configuration)); diff --git a/shell/common/shell.h b/shell/common/shell.h index 2ecf71dac6559..1b5d1bd2925fe 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -126,7 +126,8 @@ class Shell final : public PlatformView::Delegate, fml::WeakPtr io_manager, fml::RefPtr unref_queue, fml::TaskRunnerAffineWeakPtr snapshot_delegate, - std::shared_ptr volatile_path_tracker)> + std::shared_ptr volatile_path_tracker, + const std::shared_ptr& gpu_disabled_switch)> EngineCreateCallback; //---------------------------------------------------------------------------- @@ -356,6 +357,7 @@ class Shell final : public PlatformView::Delegate, //---------------------------------------------------------------------------- /// @brief Accessor for the disable GPU SyncSwitch. + // |Rasterizer::Delegate| std::shared_ptr GetIsGpuDisabledSyncSwitch() const override; From 6d02503a860022ff724aa8f5bc88f3ba79152adf Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 26 May 2023 14:39:54 -0700 Subject: [PATCH 3/4] fix bad merge --- lib/ui/painting/image_decoder_impeller.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 6e4f14242d16b..9b87fec9c23e6 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -124,7 +124,6 @@ static std::optional ToPixelFormat(SkColorType type) { return std::nullopt; } -std::optional ImageDecoderImpeller::DecompressTexture( DecompressResult ImageDecoderImpeller::DecompressTexture( ImageDescriptor* descriptor, SkISize target_size, From 92d403bccff828d46ead60eeb83e75d8c2a92ff5 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 26 May 2023 21:18:18 -0700 Subject: [PATCH 4/4] Stray line from differnet patch --- lib/ui/painting/image_decoder_impeller.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 9b87fec9c23e6..fdaf81a7a7ac0 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -442,7 +442,6 @@ ImageDecoderImpeller::UploadTextureToShared( decode_error = "Failed to submit blit pass command buffer."; return; } - command_buffer->WaitUntilScheduled(); })); if (decode_error.has_value()) { FML_DLOG(ERROR) << decode_error.value();