From 3850057c21bff1f71a68868b6d24d61b3a28cbf9 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Fri, 19 May 2023 14:21:50 -0700 Subject: [PATCH] [Impeller] Return image decoder error messages to the Dart API Fixes https://github.com/flutter/flutter/issues/127061 See https://github.com/flutter/flutter/issues/126768 --- 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 | 161 +++++++++++++-------- lib/ui/painting/image_decoder_impeller.h | 7 +- lib/ui/painting/image_decoder_skia.cc | 2 +- lib/ui/painting/image_decoder_unittests.cc | 37 +++-- 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, 178 insertions(+), 114 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 6658948e422f2..f091211c62958 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 c23bf58167718..5b8f9003cc76c 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2125,9 +2125,12 @@ base class _NativeCodec extends NativeFieldWrapperClass1 implements Codec { @override 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), @@ -2143,7 +2146,7 @@ base class _NativeCodec extends NativeFieldWrapperClass1 implements Codec { /// 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); @override @Native)>(symbol: 'Codec::dispose') 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 5f7c724ed95a9..9238e10ad4553 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -107,7 +107,7 @@ static SkAlphaType ChooseCompatibleAlphaType(SkAlphaType type) { return type; } -std::optional ImageDecoderImpeller::DecompressTexture( +DecompressResult ImageDecoderImpeller::DecompressTexture( ImageDescriptor* descriptor, SkISize target_size, impeller::ISize max_texture_size, @@ -115,8 +115,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), @@ -162,8 +163,11 @@ std::optional ImageDecoderImpeller::DecompressTexture( const auto pixel_format = impeller::skia_conversions::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(); @@ -172,14 +176,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(); @@ -189,9 +195,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(); @@ -200,7 +207,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, @@ -218,9 +225,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(), @@ -231,26 +239,32 @@ 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 = impeller::skia_conversions::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; @@ -263,8 +277,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( @@ -272,15 +287,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); @@ -290,27 +309,35 @@ 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 = impeller::skia_conversions::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; @@ -323,8 +350,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( @@ -334,8 +362,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()); @@ -343,29 +372,34 @@ 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); } command_buffer->WaitUntilScheduled(); } - return impeller::DlImageImpeller::Make(std::move(texture)); + return std::make_pair(impeller::DlImageImpeller::Make(std::move(texture)), + std::string()); } // |ImageDecoder| @@ -382,10 +416,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); }); }; @@ -398,7 +432,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 = @@ -408,21 +442,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. + 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) - result(UploadTextureToPrivate(context, bitmap_result.device_buffer, - bitmap_result.image_info)); + 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 5a54b7fb269f0..6584a1c32f0cf 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 12aceccaef2de..486f1158f89e7 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -340,7 +340,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(); @@ -386,7 +387,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_); @@ -657,7 +659,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(); @@ -717,7 +720,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); @@ -788,12 +792,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(); @@ -860,14 +865,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->width(), 6); - ASSERT_EQ(result_1->sk_bitmap->height(), 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->width(), 10); - ASSERT_EQ(result_2->sk_bitmap->height(), 10); + ASSERT_EQ(result_2.sk_bitmap->width(), 10); + ASSERT_EQ(result_2.sk_bitmap->height(), 10); #endif // IMPELLER_SUPPORTS_RENDERING } @@ -919,14 +924,14 @@ 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)); sk_sp encoded = SkPngEncoder::Encode(nullptr, decoded_image.get(), {}); ASSERT_TRUE(encoded->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 c52b4a72dbc2f..92a1687072c34 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -39,6 +39,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(); @@ -49,7 +50,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 @@ -85,7 +87,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, @@ -97,9 +100,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 = @@ -133,8 +139,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. @@ -177,7 +186,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( @@ -190,7 +200,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) { @@ -204,11 +216,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) { @@ -224,12 +237,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'); }