Merge branch 'main' into gpu_tracing_metal

Author: Jonah Williams

DEPS

@@ -18,7 +18,7 @@ vars = {
   'llvm_git': 'https://llvm.googlesource.com',
   # OCMock is for testing only so there is no google clone
   'ocmock_git': 'https://github.com/erikdoe/ocmock.git',
-  'skia_revision': '675f088b9ac40837541db9ee05b17c702097a72b',
+  'skia_revision': '68de6e3525856f9475eb27d3fc808c7e339eead1',
 
   # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY
   # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version.

ci/licenses_golden/licenses_skia

@@ -1,4 +1,4 @@
-Signature: 95b229c06464b5b2380d14193495ab12
+Signature: 717f85ae89c540094a7fba29a8df7720
 
 ====================================================================================================
 LIBRARY: etc1
@@ -8749,6 +8749,7 @@ ORIGIN: ../../../third_party/skia/include/core/SkColorTable.h + ../../../third_p
 ORIGIN: ../../../third_party/skia/include/core/SkPoint.h + ../../../third_party/skia/LICENSE
 ORIGIN: ../../../third_party/skia/include/core/SkTextureCompressionType.h + ../../../third_party/skia/LICENSE
 ORIGIN: ../../../third_party/skia/include/core/SkTiledImageUtils.h + ../../../third_party/skia/LICENSE
+ORIGIN: ../../../third_party/skia/include/docs/SkMultiPictureDocument.h + ../../../third_party/skia/LICENSE
 ORIGIN: ../../../third_party/skia/include/gpu/ganesh/GrExternalTextureGenerator.h + ../../../third_party/skia/LICENSE
 ORIGIN: ../../../third_party/skia/include/gpu/ganesh/SkImageGanesh.h + ../../../third_party/skia/LICENSE
 ORIGIN: ../../../third_party/skia/include/gpu/ganesh/SkMeshGanesh.h + ../../../third_party/skia/LICENSE
@@ -9013,6 +9014,7 @@ FILE: ../../../third_party/skia/include/core/SkColorTable.h
 FILE: ../../../third_party/skia/include/core/SkPoint.h
 FILE: ../../../third_party/skia/include/core/SkTextureCompressionType.h
 FILE: ../../../third_party/skia/include/core/SkTiledImageUtils.h
+FILE: ../../../third_party/skia/include/docs/SkMultiPictureDocument.h
 FILE: ../../../third_party/skia/include/gpu/ganesh/GrExternalTextureGenerator.h
 FILE: ../../../third_party/skia/include/gpu/ganesh/SkImageGanesh.h
 FILE: ../../../third_party/skia/include/gpu/ganesh/SkMeshGanesh.h

fml/synchronization/sync_switch.cc

@@ -32,8 +32,26 @@ void SyncSwitch::Execute(const SyncSwitch::Handlers& handlers) const {
 }
 
 void SyncSwitch::SetSwitch(bool value) {
+  {
+    fml::UniqueLock lock(*mutex_);
+    value_ = value;
+  }
+  for (Observer* observer : observers_) {
+    observer->OnSyncSwitchUpdate(value);
+  }
+}
+
+void SyncSwitch::AddObserver(Observer* observer) const {
   fml::UniqueLock lock(*mutex_);
-  value_ = value;
+  if (std::find(observers_.begin(), observers_.end(), observer) ==
+      observers_.end()) {
+    observers_.push_back(observer);
+  }
 }
 
+void SyncSwitch::RemoveObserver(Observer* observer) const {
+  fml::UniqueLock lock(*mutex_);
+  observers_.erase(std::remove(observers_.begin(), observers_.end(), observer),
+                   observers_.end());
+}
 }  // namespace fml

fml/synchronization/sync_switch.h

@@ -21,6 +21,16 @@ namespace fml {
 /// at a time.
 class SyncSwitch {
  public:
+  /// Observes changes to the SyncSwitch.
+  class Observer {
+   public:
+    virtual ~Observer() = default;
+    /// `new_is_disabled` not guaranteed to be the value of the SyncSwitch
+    /// during execution, it should be checked with calls to
+    /// SyncSwitch::Execute.
+    virtual void OnSyncSwitchUpdate(bool new_is_disabled) = 0;
+  };
+
   /// Represents the 2 code paths available when calling |SyncSwitch::Execute|.
   struct Handlers {
     /// Sets the handler that will be executed if the |SyncSwitch| is true.
@@ -53,8 +63,15 @@ class SyncSwitch {
   /// @param[in]  value  New value for the |SyncSwitch|.
   void SetSwitch(bool value);
 
+  /// Threadsafe.
+  void AddObserver(Observer* observer) const;
+
+  /// Threadsafe.
+  void RemoveObserver(Observer* observer) const;
+
  private:
   mutable std::unique_ptr<fml::SharedMutex> mutex_;
+  mutable std::vector<Observer*> observers_;
   bool value_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(SyncSwitch);

impeller/renderer/backend/metal/context_mtl.h

@@ -6,8 +6,8 @@
 
 #include <Metal/Metal.h>
 
+#include <deque>
 #include <string>
-#include <vector>
 
 #include "flutter/fml/concurrent_message_loop.h"
 #include "flutter/fml/macros.h"
@@ -98,7 +98,20 @@ class ContextMTL final : public Context,
   std::shared_ptr<GPUTracerMTL> GetGPUTracer() const;
 #endif  // IMPELLER_DEBUG
 
+  // |Context|
+  void StoreTaskForGPU(std::function<void()> task) override;
+
  private:
+  class SyncSwitchObserver : public fml::SyncSwitch::Observer {
+   public:
+    SyncSwitchObserver(ContextMTL& parent);
+    virtual ~SyncSwitchObserver() = default;
+    void OnSyncSwitchUpdate(bool new_value) override;
+
+   private:
+    ContextMTL& parent_;
+  };
+
   id<MTLDevice> device_ = nullptr;
   id<MTLCommandQueue> command_queue_ = nullptr;
   std::shared_ptr<ShaderLibraryMTL> shader_library_;
@@ -111,6 +124,8 @@ class ContextMTL final : public Context,
 #ifdef IMPELLER_DEBUG
   std::shared_ptr<GPUTracerMTL> gpu_tracer_;
 #endif  // IMPELLER_DEBUG
+  std::deque<std::function<void()>> tasks_awaiting_gpu_;
+  std::unique_ptr<SyncSwitchObserver> sync_switch_observer_;
   bool is_valid_ = false;
 
   ContextMTL(
@@ -122,6 +137,8 @@ class ContextMTL final : public Context,
   std::shared_ptr<CommandBuffer> CreateCommandBufferInQueue(
       id<MTLCommandQueue> queue) const;
 
+  void FlushTasksAwaitingGPU();
+
   FML_DISALLOW_COPY_AND_ASSIGN(ContextMTL);
 };
 

impeller/renderer/backend/metal/context_mtl.mm

@@ -85,6 +85,9 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
     return;
   }
 
+  sync_switch_observer_.reset(new SyncSwitchObserver(*this));
+  is_gpu_disabled_sync_switch_->AddObserver(sync_switch_observer_.get());
+
   // Worker task runner.
   {
     raster_message_loop_ = fml::ConcurrentMessageLoop::Create(
@@ -288,7 +291,9 @@ new ContextMTL(device, command_queue,
   return context;
 }
 
-ContextMTL::~ContextMTL() = default;
+ContextMTL::~ContextMTL() {
+  is_gpu_disabled_sync_switch_->RemoveObserver(sync_switch_observer_.get());
+}
 
 Context::BackendType ContextMTL::GetBackendType() const {
   return Context::BackendType::kMetal;
@@ -386,4 +391,28 @@ new ContextMTL(device, command_queue,
   return buffer;
 }
 
+void ContextMTL::StoreTaskForGPU(std::function<void()> task) {
+  tasks_awaiting_gpu_.emplace_back(std::move(task));
+  while (tasks_awaiting_gpu_.size() > kMaxTasksAwaitingGPU) {
+    tasks_awaiting_gpu_.front()();
+    tasks_awaiting_gpu_.pop_front();
+  }
+}
+
+void ContextMTL::FlushTasksAwaitingGPU() {
+  for (const auto& task : tasks_awaiting_gpu_) {
+    task();
+  }
+  tasks_awaiting_gpu_.clear();
+}
+
+ContextMTL::SyncSwitchObserver::SyncSwitchObserver(ContextMTL& parent)
+    : parent_(parent) {}
+
+void ContextMTL::SyncSwitchObserver::OnSyncSwitchUpdate(bool new_is_disabled) {
+  if (!new_is_disabled) {
+    parent_.FlushTasksAwaitingGPU();
+  }
+}
+
 }  // namespace impeller

impeller/renderer/context.h

@@ -52,6 +52,14 @@ class Context {
     kVulkan,
   };
 
+  /// The maximum number of tasks that should ever be stored for
+  /// `StoreTaskForGPU`.
+  ///
+  /// This number was arbitrarily chosen. The idea is that this is a somewhat
+  /// rare situation where tasks happen to get executed in that tiny amount of
+  /// time while an app is being backgrounded but still executing.
+  static constexpr int32_t kMaxTasksAwaitingGPU = 10;
+
   //----------------------------------------------------------------------------
   /// @brief      Destroys an Impeller context.
   ///
@@ -176,6 +184,19 @@ class Context {
 
   CaptureContext capture;
 
+  /// Stores a task on the `ContextMTL` that is awaiting access for the GPU.
+  ///
+  /// The task will be executed in the event that the GPU access has changed to
+  /// being available or that the task has been canceled. The task should
+  /// operate with the `SyncSwitch` to make sure the GPU is accessible.
+  ///
+  /// Threadsafe.
+  ///
+  /// `task` will be executed on the platform thread.
+  virtual void StoreTaskForGPU(std::function<void()> task) {
+    FML_CHECK(false && "not supported in this context");
+  }
+
  protected:
   Context();
 

lib/ui/fixtures/ui_test.dart

@@ -325,7 +325,11 @@ external void _validateExternal(Uint8List result);
 @pragma('vm:external-name', 'ValidateError')
 external void _validateError(String? error);
 @pragma('vm:external-name', 'TurnOffGPU')
-external void _turnOffGPU();
+external void _turnOffGPU(bool value);
+@pragma('vm:external-name', 'FlushGpuAwaitingTasks')
+external void _flushGpuAwaitingTasks();
+@pragma('vm:external-name', 'ValidateNotNull')
+external void _validateNotNull(Object? object);
 
 @pragma('vm:entry-point')
 Future<void> toByteDataWithoutGPU() async {
@@ -338,12 +342,40 @@ Future<void> toByteDataWithoutGPU() async {
   canvas.drawCircle(c, 25.0, paint);
   final Picture picture = pictureRecorder.endRecording();
   final Image image = await picture.toImage(100, 100);
-  _turnOffGPU();
+  _turnOffGPU(true);
+  Timer flusher = Timer.periodic(Duration(milliseconds: 1), (timer) {
+    _flushGpuAwaitingTasks();
+  });
   try {
     ByteData? byteData = await image.toByteData();
     _validateError(null);
-  } catch (ex) {
-    _validateError(ex.toString());
+  } catch (error) {
+    _validateError(error.toString());
+  } finally {
+    flusher.cancel();
+  }
+}
+
+@pragma('vm:entry-point')
+Future<void> toByteDataRetries() async {
+  final PictureRecorder pictureRecorder = PictureRecorder();
+  final Canvas canvas = Canvas(pictureRecorder);
+  final Paint paint = Paint()
+    ..color = Color.fromRGBO(255, 255, 255, 1.0)
+    ..style = PaintingStyle.fill;
+  final Offset c = Offset(50.0, 50.0);
+  canvas.drawCircle(c, 25.0, paint);
+  final Picture picture = pictureRecorder.endRecording();
+  final Image image = await picture.toImage(100, 100);
+  _turnOffGPU(true);
+  Future<void>.delayed(Duration(milliseconds: 10), () {
+    _turnOffGPU(false);
+  });
+  try {
+    ByteData? byteData = await image.toByteData();
+    _validateNotNull(byteData);
+  } catch (error) {
+    _validateNotNull(null);
   }
 }
 

lib/ui/painting/image_encoding_impeller.cc

@@ -56,21 +56,68 @@ sk_sp<SkImage> ConvertBufferToSkImage(
   return raster_image;
 }
 
-void DoConvertImageToRasterImpeller(
+[[nodiscard]] fml::Status DoConvertImageToRasterImpeller(
     const sk_sp<DlImage>& dl_image,
-    std::function<void(fml::StatusOr<sk_sp<SkImage>>)> encode_task,
+    const std::function<void(fml::StatusOr<sk_sp<SkImage>>)>& encode_task,
     const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch,
     const std::shared_ptr<impeller::Context>& impeller_context) {
+  fml::Status result;
   is_gpu_disabled_sync_switch->Execute(
       fml::SyncSwitch::Handlers()
-          .SetIfTrue([&encode_task] {
-            encode_task(
-                fml::Status(fml::StatusCode::kUnavailable, "GPU unavailable."));
+          .SetIfTrue([&result] {
+            result =
+                fml::Status(fml::StatusCode::kUnavailable, "GPU unavailable.");
           })
           .SetIfFalse([&dl_image, &encode_task, &impeller_context] {
             ImageEncodingImpeller::ConvertDlImageToSkImage(
-                dl_image, std::move(encode_task), impeller_context);
+                dl_image, encode_task, impeller_context);
           }));
+  return result;
+}
+
+/// Same as `DoConvertImageToRasterImpeller` but it will attempt to retry the
+/// operation if `DoConvertImageToRasterImpeller` returns kUnavailable when the
+/// GPU becomes available again.
+void DoConvertImageToRasterImpellerWithRetry(
+    const sk_sp<DlImage>& dl_image,
+    std::function<void(fml::StatusOr<sk_sp<SkImage>>)>&& encode_task,
+    const std::shared_ptr<const fml::SyncSwitch>& is_gpu_disabled_sync_switch,
+    const std::shared_ptr<impeller::Context>& impeller_context,
+    const fml::RefPtr<fml::TaskRunner>& retry_runner) {
+  fml::Status status = DoConvertImageToRasterImpeller(
+      dl_image, encode_task, is_gpu_disabled_sync_switch, impeller_context);
+  if (!status.ok()) {
+    // If the conversion failed because of the GPU is unavailable, store the
+    // task on the Context so it can be executed when the GPU becomes available.
+    if (status.code() == fml::StatusCode::kUnavailable) {
+      impeller_context->StoreTaskForGPU(
+          [dl_image, encode_task = std::move(encode_task),
+           is_gpu_disabled_sync_switch, impeller_context,
+           retry_runner]() mutable {
+            auto retry_task = [dl_image, encode_task = std::move(encode_task),
+                               is_gpu_disabled_sync_switch, impeller_context] {
+              fml::Status retry_status = DoConvertImageToRasterImpeller(
+                  dl_image, encode_task, is_gpu_disabled_sync_switch,
+                  impeller_context);
+              if (!retry_status.ok()) {
+                // The retry failed for some reason, maybe the GPU became
+                // unavailable again. Don't retry again, just fail in this case.
+                encode_task(retry_status);
+              }
+            };
+            // If a `retry_runner` is specified, post the retry to it, otherwise
+            // execute it directly.
+            if (retry_runner) {
+              retry_runner->PostTask(retry_task);
+            } else {
+              retry_task();
+            }
+          });
+    } else {
+      // Pass on errors that are not `kUnavailable`.
+      encode_task(status);
+    }
+  }
 }
 
 }  // namespace
@@ -153,18 +200,20 @@ void ImageEncodingImpeller::ConvertImageToRaster(
   };
 
   if (dl_image->owning_context() != DlImage::OwningContext::kRaster) {
-    DoConvertImageToRasterImpeller(dl_image, std::move(encode_task),
-                                   is_gpu_disabled_sync_switch,
-                                   impeller_context);
+    DoConvertImageToRasterImpellerWithRetry(dl_image, std::move(encode_task),
+                                            is_gpu_disabled_sync_switch,
+                                            impeller_context,
+                                            /*retry_runner=*/nullptr);
     return;
   }
 
   raster_task_runner->PostTask([dl_image, encode_task = std::move(encode_task),
                                 io_task_runner, is_gpu_disabled_sync_switch,
-                                impeller_context]() mutable {
-    DoConvertImageToRasterImpeller(dl_image, std::move(encode_task),
-                                   is_gpu_disabled_sync_switch,
-                                   impeller_context);
+                                impeller_context,
+                                raster_task_runner]() mutable {
+    DoConvertImageToRasterImpellerWithRetry(
+        dl_image, std::move(encode_task), is_gpu_disabled_sync_switch,
+        impeller_context, raster_task_runner);
   });
 }