[Impeller] Avoid encoding metal commands while the GPU is unavailable when decoding images. (#42349)

Fixes flutter/flutter#126878

This disables device private upload on iOS when backgrounded, and disables mipmap generation when backgrounded.

We don't have a good way to test the core problem in this repo because it only reproduces on physical iOS hardware - simulators don't really care if you do this stuff in the background.

I'll write a devicelab test in the framework to capture this. In the mean time it can be reviewed.

We could consider making the IOManager a shared_ptr instead of having an fml::WeakPtr and that'd clean up some of the extra arguments to engine construction - or we could consider vending the sync switch from impeller::Context unconditionally, but it's pretty iOS specific...

Author: dnfield

impeller/renderer/backend/metal/render_pass_mtl.mm

@@ -385,10 +385,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,

lib/ui/painting/image_decoder.cc

@@ -16,14 +16,16 @@ std::unique_ptr<ImageDecoder> ImageDecoder::Make(
     const Settings& settings,
     const TaskRunners& runners,
     std::shared_ptr<fml::ConcurrentTaskRunner> concurrent_task_runner,
-    fml::WeakPtr<IOManager> io_manager) {
+    fml::WeakPtr<IOManager> io_manager,
+    const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch) {
 #if IMPELLER_SUPPORTS_RENDERING
   if (settings.enable_impeller) {
     return std::make_unique<ImageDecoderImpeller>(
         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<ImageDecoderSkia>(

lib/ui/painting/image_decoder.h

@@ -27,7 +27,8 @@ class ImageDecoder {
       const Settings& settings,
       const TaskRunners& runners,
       std::shared_ptr<fml::ConcurrentTaskRunner> concurrent_task_runner,
-      fml::WeakPtr<IOManager> io_manager);
+      fml::WeakPtr<IOManager> io_manager,
+      const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch);
 
   virtual ~ImageDecoder();
 

lib/ui/painting/image_decoder_impeller.cc

@@ -80,9 +80,11 @@ ImageDecoderImpeller::ImageDecoderImpeller(
     const TaskRunners& runners,
     std::shared_ptr<fml::ConcurrentTaskRunner> concurrent_task_runner,
     const fml::WeakPtr<IOManager>& io_manager,
-    bool supports_wide_gamut)
+    bool supports_wide_gamut,
+    const std::shared_ptr<fml::SyncSwitch>& 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<std::shared_ptr<impeller::Context>> context_promise;
   context_ = context_promise.get_future();
   runners_.GetIOTaskRunner()->PostTask(fml::MakeCopyable(
@@ -246,18 +248,11 @@ DecompressResult ImageDecoderImpeller::DecompressTexture(
                           .image_info = scaled_bitmap->info()};
 }
 
-std::pair<sk_sp<DlImage>, std::string>
-ImageDecoderImpeller::UploadTextureToPrivate(
+/// Only call this method if the GPU is available.
+static std::pair<sk_sp<DlImage>, std::string> UnsafeUploadTextureToPrivate(
     const std::shared_ptr<impeller::Context>& context,
     const std::shared_ptr<impeller::DeviceBuffer>& buffer,
     const SkImageInfo& image_info) {
-  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");
-  }
   const auto pixel_format =
       impeller::skia_conversions::ToPixelFormat(image_info.colorType());
   if (!pixel_format) {
@@ -318,10 +313,40 @@ ImageDecoderImpeller::UploadTextureToPrivate(
       impeller::DlImageImpeller::Make(std::move(dest_texture)), std::string());
 }
 
+std::pair<sk_sp<DlImage>, std::string>
+ImageDecoderImpeller::UploadTextureToPrivate(
+    const std::shared_ptr<impeller::Context>& context,
+    const std::shared_ptr<impeller::DeviceBuffer>& buffer,
+    const SkImageInfo& image_info,
+    std::shared_ptr<SkBitmap> bitmap,
+    const std::shared_ptr<fml::SyncSwitch>& 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<sk_sp<DlImage>, 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<sk_sp<DlImage>, std::string>
 ImageDecoderImpeller::UploadTextureToShared(
     const std::shared_ptr<impeller::Context>& context,
     std::shared_ptr<SkBitmap> bitmap,
+    const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch,
     bool create_mips) {
   TRACE_EVENT0("impeller", __FUNCTION__);
   if (!context) {
@@ -370,32 +395,40 @@ 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);
+    std::optional<std::string> 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());
     }
-    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);
-    }
-    command_buffer->WaitUntilScheduled();
   }
 
   return std::make_pair(impeller::DlImageImpeller::Make(std::move(texture)),
@@ -429,8 +462,8 @@ void ImageDecoderImpeller::Decode(fml::RefPtr<ImageDescriptor> 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;
@@ -446,24 +479,28 @@ void ImageDecoderImpeller::Decode(fml::RefPtr<ImageDescriptor> 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<DlImage> image;
           std::string decode_error;
+
 #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);
       });

lib/ui/painting/image_decoder_impeller.h

@@ -50,7 +50,8 @@ class ImageDecoderImpeller final : public ImageDecoder {
       const TaskRunners& runners,
       std::shared_ptr<fml::ConcurrentTaskRunner> concurrent_task_runner,
       const fml::WeakPtr<IOManager>& io_manager,
-      bool supports_wide_gamut);
+      bool supports_wide_gamut,
+      const std::shared_ptr<fml::SyncSwitch>& 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<sk_sp<DlImage>, std::string> UploadTextureToPrivate(
       const std::shared_ptr<impeller::Context>& context,
       const std::shared_ptr<impeller::DeviceBuffer>& buffer,
-      const SkImageInfo& image_info);
+      const SkImageInfo& image_info,
+      std::shared_ptr<SkBitmap> bitmap,
+      const std::shared_ptr<fml::SyncSwitch>& 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<sk_sp<DlImage>, std::string> UploadTextureToShared(
       const std::shared_ptr<impeller::Context>& context,
       std::shared_ptr<SkBitmap> bitmap,
+      const std::shared_ptr<fml::SyncSwitch>& gpu_disabled_switch,
       bool create_mips = true);
 
  private:
   using FutureContext = std::shared_future<std::shared_ptr<impeller::Context>>;
   FutureContext context_;
   const bool supports_wide_gamut_;
+  std::shared_ptr<fml::SyncSwitch> gpu_disabled_switch_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoderImpeller);
 };

lib/ui/painting/image_decoder_unittests.cc

@@ -138,9 +138,12 @@ class TestImpellerContext : public impeller::Context {
   }
 
   std::shared_ptr<CommandBuffer> CreateCommandBuffer() const override {
+    command_buffer_count_ += 1;
     return nullptr;
   }
 
+  mutable size_t command_buffer_count_ = 0;
+
  private:
   std::shared_ptr<const Capabilities> capabilities_;
 };
@@ -280,7 +283,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<fml::SyncSwitch>());
     ASSERT_NE(decoder, nullptr);
   });
 }
@@ -331,7 +335,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<fml::SyncSwitch>());
 
     auto data = OpenFixtureAsSkData("ThisDoesNotExist.jpg");
     ASSERT_FALSE(data);
@@ -370,9 +375,9 @@ TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) {
   };
   auto decode_image = [&]() {
     Settings settings;
-    std::unique_ptr<ImageDecoder> image_decoder =
-        ImageDecoder::Make(settings, runners, loop->GetTaskRunner(),
-                           io_manager->GetWeakIOManager());
+    std::unique_ptr<ImageDecoder> image_decoder = ImageDecoder::Make(
+        settings, runners, loop->GetTaskRunner(),
+        io_manager->GetWeakIOManager(), std::make_shared<fml::SyncSwitch>());
 
     auto data = OpenFixtureAsSkData("DashInNooglerHat.jpg");
 
@@ -426,6 +431,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<impeller::TestImpellerContext>();
+  auto gpu_disabled_switch = std::make_shared<fml::SyncSwitch>(true);
+
+  auto info = SkImageInfo::Make(10, 10, SkColorType::kRGBA_8888_SkColorType,
+                                SkAlphaType::kPremul_SkAlphaType);
+  auto bitmap = std::make_shared<SkBitmap>();
+  bitmap->allocPixels(info, 10 * 4);
+  impeller::DeviceBufferDescriptor desc;
+  desc.size = bitmap->computeByteSize();
+  auto buffer = std::make_shared<impeller::TestImpellerDeviceBuffer>(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);
@@ -642,9 +675,9 @@ TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) {
   SkISize decoded_size = SkISize::MakeEmpty();
   auto decode_image = [&]() {
     Settings settings;
-    std::unique_ptr<ImageDecoder> image_decoder =
-        ImageDecoder::Make(settings, runners, loop->GetTaskRunner(),
-                           io_manager->GetWeakIOManager());
+    std::unique_ptr<ImageDecoder> image_decoder = ImageDecoder::Make(
+        settings, runners, loop->GetTaskRunner(),
+        io_manager->GetWeakIOManager(), std::make_shared<fml::SyncSwitch>());
 
     auto data = OpenFixtureAsSkData("Horizontal.jpg");
 
@@ -703,9 +736,9 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithoutAGPUContext) {
 
   auto decode_image = [&]() {
     Settings settings;
-    std::unique_ptr<ImageDecoder> image_decoder =
-        ImageDecoder::Make(settings, runners, loop->GetTaskRunner(),
-                           io_manager->GetWeakIOManager());
+    std::unique_ptr<ImageDecoder> image_decoder = ImageDecoder::Make(
+        settings, runners, loop->GetTaskRunner(),
+        io_manager->GetWeakIOManager(), std::make_shared<fml::SyncSwitch>());
 
     auto data = OpenFixtureAsSkData("DashInNooglerHat.jpg");
 
@@ -771,7 +804,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<fml::SyncSwitch>());
   });
 
   // Setup a generic decoding utility that gives us the final decoded size.