macOS: Fix platform view threading issues

Author: knopp

shell/platform/darwin/macos/framework/Source/FlutterCompositor.h

@@ -9,6 +9,7 @@
 #include <list>
 
 #include "flutter/fml/macros.h"
+#include "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h"
 #include "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h"
 #include "flutter/shell/platform/embedder/embedder.h"
 
@@ -19,7 +20,9 @@ namespace flutter {
 // Platform views are not yet supported.
 class FlutterCompositor {
  public:
-  explicit FlutterCompositor(FlutterViewController* view_controller);
+  explicit FlutterCompositor(
+      FlutterViewController* view_controller,
+      FlutterPlatformViewController* platform_views_controller);
 
   virtual ~FlutterCompositor() = default;
 
@@ -46,7 +49,9 @@ class FlutterCompositor {
   // Present sets frame_started_ to false.
   virtual bool Present(const FlutterLayer** layers, size_t layers_count) = 0;
 
-  using PresentCallback = std::function<bool(bool has_flutter_content)>;
+  using PresentCallback =
+      std::function<bool(bool has_flutter_content,
+                         dispatch_block_t platform_thread_notify)>;
 
   // PresentCallback is called at the end of the Present function.
   void SetPresentCallback(const PresentCallback& present_callback);
@@ -73,14 +78,21 @@ class FlutterCompositor {
 
   // Calls the present callback and ensures the frame status is updated
   // to frame ended, returning whether the present was successful or not.
-  bool EndFrame(bool has_flutter_content);
+  bool EndFrame(bool has_flutter_content, dispatch_block_t on_notify);
+
+  void RemoveOldLayers();
 
   // Creates a CALayer object which is backed by the supplied IOSurface, and
   // adds it to the root CALayer for this FlutterViewController's view.
   void InsertCALayerForIOSurface(
       const IOSurfaceRef& io_surface,
+      size_t layer_position,
       CATransform3D transform = CATransform3DIdentity);
 
+  // Presents the platform view layer represented by `layer`. `layer_index` is
+  // used to position the layer in the z-axis.
+  void PresentPlatformView(const FlutterLayer* layer, size_t layer_index);
+
  private:
   // A list of the active CALayer objects for the frame that need to be removed.
   std::list<CALayer*> active_ca_layers_;
@@ -92,6 +104,8 @@ class FlutterCompositor {
   // Current frame status.
   FrameStatus frame_status_ = kEnded;
 
+  const FlutterPlatformViewController* platform_views_controller_;
+
   FML_DISALLOW_COPY_AND_ASSIGN(FlutterCompositor);
 };
 

shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm

@@ -7,10 +7,10 @@
 
 namespace flutter {
 
-FlutterCompositor::FlutterCompositor(FlutterViewController* view_controller) {
+FlutterCompositor::FlutterCompositor(FlutterViewController* view_controller,
+                                     FlutterPlatformViewController* platform_views_controller)
+    : view_controller_(view_controller), platform_views_controller_(platform_views_controller) {
   FML_CHECK(view_controller != nullptr) << "FlutterViewController* cannot be nullptr";
-
-  view_controller_ = view_controller;
 }
 
 void FlutterCompositor::SetPresentCallback(
@@ -19,18 +19,21 @@
 }
 
 void FlutterCompositor::StartFrame() {
+  SetFrameStatus(FrameStatus::kStarted);
+}
+
+void FlutterCompositor::RemoveOldLayers() {
   // First remove all CALayers from the superlayer.
   for (auto layer : active_ca_layers_) {
     [layer removeFromSuperlayer];
   }
 
   // Reset active layers.
   active_ca_layers_.clear();
-  SetFrameStatus(FrameStatus::kStarted);
 }
 
-bool FlutterCompositor::EndFrame(bool has_flutter_content) {
-  bool status = present_callback_(has_flutter_content);
+bool FlutterCompositor::EndFrame(bool has_flutter_content, dispatch_block_t on_notify) {
+  bool status = present_callback_(has_flutter_content, on_notify);
   SetFrameStatus(FrameStatus::kEnded);
   return status;
 }
@@ -44,15 +47,34 @@
 }
 
 void FlutterCompositor::InsertCALayerForIOSurface(const IOSurfaceRef& io_surface,
+                                                  size_t layer_position,
                                                   CATransform3D transform) {
   // FlutterCompositor manages the lifecycle of CALayers.
   CALayer* content_layer = [[CALayer alloc] init];
   content_layer.transform = transform;
   content_layer.frame = view_controller_.flutterView.layer.bounds;
   [content_layer setContents:(__bridge id)io_surface];
   [view_controller_.flutterView.layer addSublayer:content_layer];
-
+  content_layer.zPosition = layer_position;
   active_ca_layers_.push_back(content_layer);
 }
 
+void FlutterCompositor::PresentPlatformView(const FlutterLayer* layer, size_t layer_position) {
+  FML_DCHECK([[NSThread currentThread] isMainThread])
+      << "Must be on the main thread to present platform views";
+
+  int64_t platform_view_id = layer->platform_view->identifier;
+  NSView* platform_view = [platform_views_controller_ platformViewWithID:platform_view_id];
+
+  FML_DCHECK(platform_view) << "Platform view not found for id: " << platform_view_id;
+
+  CGFloat scale = [[NSScreen mainScreen] backingScaleFactor];
+  platform_view.frame = CGRectMake(layer->offset.x / scale, layer->offset.y / scale,
+                                   layer->size.width / scale, layer->size.height / scale);
+  if (platform_view.superview == nil) {
+    [view_controller_.flutterView addSubview:platform_view];
+  }
+  platform_view.layer.zPosition = layer_position;
+}
+
 }  // namespace flutter

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm

@@ -438,30 +438,32 @@ - (FlutterCompositor*)createFlutterCompositor {
     FlutterMetalRenderer* metalRenderer = reinterpret_cast<FlutterMetalRenderer*>(_renderer);
     _macOSCompositor = std::make_unique<flutter::FlutterMetalCompositor>(
         _viewController, _platformViewController, metalRenderer.device);
-    _macOSCompositor->SetPresentCallback([weakSelf](bool has_flutter_content) {
-      if (has_flutter_content) {
-        FlutterMetalRenderer* metalRenderer =
-            reinterpret_cast<FlutterMetalRenderer*>(weakSelf.renderer);
-        return [metalRenderer present:0 /*=textureID*/] == YES;
-      } else {
-        return true;
-      }
-    });
+    _macOSCompositor->SetPresentCallback(
+        [weakSelf](bool has_flutter_content, dispatch_block_t platform_thread_notify) {
+          if (has_flutter_content) {
+            FlutterMetalRenderer* metalRenderer =
+                reinterpret_cast<FlutterMetalRenderer*>(weakSelf.renderer);
+            return [metalRenderer present:0 /*=textureID*/ withBlock:platform_thread_notify] == YES;
+          } else {
+            return true;
+          }
+        });
   } else {
     FlutterOpenGLRenderer* openGLRenderer = reinterpret_cast<FlutterOpenGLRenderer*>(_renderer);
     [openGLRenderer.openGLContext makeCurrentContext];
-    _macOSCompositor = std::make_unique<flutter::FlutterGLCompositor>(_viewController,
-                                                                      openGLRenderer.openGLContext);
-
-    _macOSCompositor->SetPresentCallback([weakSelf](bool has_flutter_content) {
-      if (has_flutter_content) {
-        FlutterOpenGLRenderer* openGLRenderer =
-            reinterpret_cast<FlutterOpenGLRenderer*>(weakSelf.renderer);
-        return [openGLRenderer glPresent] == YES;
-      } else {
-        return true;
-      }
-    });
+    _macOSCompositor = std::make_unique<flutter::FlutterGLCompositor>(
+        _viewController, _platformViewController, openGLRenderer.openGLContext);
+
+    _macOSCompositor->SetPresentCallback(
+        [weakSelf](bool has_flutter_content, dispatch_block_t block) {
+          if (has_flutter_content) {
+            FlutterOpenGLRenderer* openGLRenderer =
+                reinterpret_cast<FlutterOpenGLRenderer*>(weakSelf.renderer);
+            return [openGLRenderer glPresentWithBlock:block] == YES;
+          } else {
+            return true;
+          }
+        });
   }
 
   _compositor = {};

shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm

@@ -407,7 +407,7 @@ @interface FlutterEngine (Test)
   // Latch to ensure the entire layer tree has been generated and presented.
   fml::AutoResetWaitableEvent latch;
   auto compositor = engine.macOSCompositor;
-  compositor->SetPresentCallback([&](bool has_flutter_content) {
+  compositor->SetPresentCallback([&](bool has_flutter_content, dispatch_block_t) {
     latch.Signal();
     return true;
   });

shell/platform/darwin/macos/framework/Source/FlutterGLCompositor.h

@@ -20,6 +20,7 @@ namespace flutter {
 class FlutterGLCompositor : public FlutterCompositor {
  public:
   FlutterGLCompositor(FlutterViewController* view_controller,
+                      FlutterPlatformViewController* platform_views_controller,
                       NSOpenGLContext* opengl_context);
 
   virtual ~FlutterGLCompositor() = default;

shell/platform/darwin/macos/framework/Source/FlutterGLCompositor.mm

@@ -20,8 +20,10 @@
 namespace flutter {
 
 FlutterGLCompositor::FlutterGLCompositor(FlutterViewController* view_controller,
+                                         FlutterPlatformViewController* platform_views_controller,
                                          NSOpenGLContext* opengl_context)
-    : FlutterCompositor(view_controller), open_gl_context_(opengl_context) {}
+    : FlutterCompositor(view_controller, platform_views_controller),
+      open_gl_context_(opengl_context) {}
 
 bool FlutterGLCompositor::CreateBackingStore(const FlutterBackingStoreConfig* config,
                                              FlutterBackingStore* backing_store_out) {
@@ -79,6 +81,11 @@
 
   bool has_flutter_content = false;
 
+  std::vector<dispatch_block_t> actions;
+  actions.push_back(^{
+    RemoveOldLayers();
+  });
+
   for (size_t i = 0; i < layers_count; ++i) {
     const auto* layer = layers[i];
     FlutterBackingStore* backing_store = const_cast<FlutterBackingStore*>(layer->backing_store);
@@ -91,21 +98,28 @@
           FlutterIOSurfaceHolder* io_surface_holder = [backing_store_data ioSurfaceHolder];
           IOSurfaceRef io_surface = [io_surface_holder ioSurface];
 
-          // The surface is an OpenGL texture, which means it has origin in bottom left corner
-          // and needs to be flipped vertically
-          InsertCALayerForIOSurface(io_surface, CATransform3DMakeScale(1, -1, 1));
+          actions.push_back(^{
+            // The surface is an OpenGL texture, which means it has origin in bottom left corner
+            // and needs to be flipped vertically
+            InsertCALayerForIOSurface(io_surface, i, CATransform3DMakeScale(1, -1, 1));
+          });
         }
         has_flutter_content = true;
         break;
       }
       case kFlutterLayerContentTypePlatformView:
-        // Add functionality in follow up PR.
-        FML_LOG(WARNING) << "Presenting PlatformViews not yet supported";
+        actions.push_back(^{
+          PresentPlatformView(layer, i);
+        });
         break;
     };
   }
 
-  return EndFrame(has_flutter_content);
+  return EndFrame(has_flutter_content, ^{
+    for (auto& action : actions) {
+      action();
+    }
+  });
 }
 
 }  // namespace flutter

shell/platform/darwin/macos/framework/Source/FlutterGLCompositorUnittests.mm

@@ -14,13 +14,14 @@
   id mockViewController = CreateMockViewController();
 
   std::unique_ptr<flutter::FlutterGLCompositor> macos_compositor =
-      std::make_unique<FlutterGLCompositor>(mockViewController, nullptr);
+      std::make_unique<FlutterGLCompositor>(mockViewController, nullptr, nullptr);
 
   bool flag = false;
-  macos_compositor->SetPresentCallback([f = &flag](bool has_flutter_content) {
-    *f = true;
-    return true;
-  });
+  macos_compositor->SetPresentCallback(
+      [f = &flag](bool has_flutter_content, dispatch_block_t on_notify) {
+        *f = true;
+        return true;
+      });
 
   ASSERT_TRUE(macos_compositor->Present(nil, 0));
   ASSERT_TRUE(flag);

shell/platform/darwin/macos/framework/Source/FlutterMetalCompositor.h

@@ -7,7 +7,6 @@
 
 #include "flutter/fml/macros.h"
 #include "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h"
-#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformViewController.h"
 
 namespace flutter {
 
@@ -45,12 +44,7 @@ class FlutterMetalCompositor : public FlutterCompositor {
   bool Present(const FlutterLayer** layers, size_t layers_count) override;
 
  private:
-  // Presents the platform view layer represented by `layer`. `layer_index` is
-  // used to position the layer in the z-axis.
-  void PresentPlatformView(const FlutterLayer* layer, size_t layer_index);
-
   const id<MTLDevice> mtl_device_;
-  const FlutterPlatformViewController* platform_views_controller_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(FlutterMetalCompositor);
 };

shell/platform/darwin/macos/framework/Source/FlutterMetalCompositor.mm

@@ -14,9 +14,7 @@
     FlutterViewController* view_controller,
     FlutterPlatformViewController* platform_views_controller,
     id<MTLDevice> mtl_device)
-    : FlutterCompositor(view_controller),
-      mtl_device_(mtl_device),
-      platform_views_controller_(platform_views_controller) {}
+    : FlutterCompositor(view_controller, platform_views_controller), mtl_device_(mtl_device) {}
 
 bool FlutterMetalCompositor::CreateBackingStore(const FlutterBackingStoreConfig* config,
                                                 FlutterBackingStore* backing_store_out) {
@@ -80,7 +78,13 @@
 bool FlutterMetalCompositor::Present(const FlutterLayer** layers, size_t layers_count) {
   SetFrameStatus(FrameStatus::kPresenting);
 
+  std::vector<dispatch_block_t> actions;
+  actions.push_back(^{
+    RemoveOldLayers();
+  });
+
   bool has_flutter_content = false;
+
   for (size_t i = 0; i < layers_count; ++i) {
     const auto* layer = layers[i];
     FlutterBackingStore* backing_store = const_cast<FlutterBackingStore*>(layer->backing_store);
@@ -91,38 +95,26 @@
           FlutterIOSurfaceHolder* io_surface_holder =
               (__bridge FlutterIOSurfaceHolder*)backing_store->metal.texture.user_data;
           IOSurfaceRef io_surface = [io_surface_holder ioSurface];
-          InsertCALayerForIOSurface(io_surface);
+          actions.push_back(^{
+            InsertCALayerForIOSurface(io_surface, i);
+          });
         }
         has_flutter_content = true;
         break;
       }
       case kFlutterLayerContentTypePlatformView:
-        PresentPlatformView(layer, i);
+        actions.push_back(^{
+          PresentPlatformView(layer, i);
+        });
         break;
     };
   }
 
-  return EndFrame(has_flutter_content);
-}
-
-void FlutterMetalCompositor::PresentPlatformView(const FlutterLayer* layer, size_t layer_position) {
-  // TODO (https://github.com/flutter/flutter/issues/96668)
-  // once the issue is fixed, this check will pass.
-  FML_DCHECK([[NSThread currentThread] isMainThread])
-      << "Must be on the main thread to present platform views";
-
-  int64_t platform_view_id = layer->platform_view->identifier;
-  NSView* platform_view = [platform_views_controller_ platformViewWithID:platform_view_id];
-
-  FML_DCHECK(platform_view) << "Platform view not found for id: " << platform_view_id;
-
-  CGFloat scale = [[NSScreen mainScreen] backingScaleFactor];
-  platform_view.frame = CGRectMake(layer->offset.x / scale, layer->offset.y / scale,
-                                   layer->size.width / scale, layer->size.height / scale);
-  if (platform_view.superview == nil) {
-    [view_controller_.flutterView addSubview:platform_view];
-  }
-  platform_view.layer.zPosition = layer_position;
+  return EndFrame(has_flutter_content, ^{
+    for (auto& action : actions) {
+      action();
+    }
+  });
 }
 
 }  // namespace flutter

shell/platform/darwin/macos/framework/Source/FlutterMetalCompositorUnittests.mm

@@ -18,10 +18,11 @@
           mockViewController, /*platform_view_controller*/ nullptr, /*mtl_device*/ nullptr);
 
   bool flag = false;
-  macos_compositor->SetPresentCallback([f = &flag](bool has_flutter_content) {
-    *f = true;
-    return true;
-  });
+  macos_compositor->SetPresentCallback(
+      [f = &flag](bool has_flutter_content, dispatch_block_t on_notify) {
+        *f = true;
+        return true;
+      });
 
   ASSERT_TRUE(macos_compositor->Present(nil, 0));
   ASSERT_TRUE(flag);