Do not block raster thread when delaying present

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

@@ -18,6 +18,10 @@
 
 namespace flutter {
 
+class PlatformViewLayer;
+
+typedef std::pair<PlatformViewLayer, size_t> PlatformViewLayerWithIndex;
+
 // FlutterCompositor creates and manages the backing stores used for
 // rendering Flutter content and presents Flutter content and Platform views.
 // Platform views are not yet supported.
@@ -57,15 +61,14 @@ class FlutterCompositor {
 
  private:
   void PresentPlatformViews(FlutterView* default_base_view,
-                            const FlutterLayer** layers,
-                            size_t layers_count);
+                            const std::vector<PlatformViewLayerWithIndex>& platform_views_layers);
 
   // Presents the platform view layer represented by `layer`. `layer_index` is
   // used to position the layer in the z-axis. If the layer does not have a
   // superview, it will become subview of `default_base_view`.
   FlutterMutatorView* PresentPlatformView(FlutterView* default_base_view,
-                                          const FlutterLayer* layer,
-                                          size_t layer_position);
+                                          const PlatformViewLayer& layer,
+                                          size_t index);
 
   // Where the compositor can query FlutterViews. Must not be null.
   id<FlutterViewProvider> const view_provider_;

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

@@ -9,6 +9,19 @@
 
 namespace flutter {
 
+namespace {
+std::vector<PlatformViewLayerWithIndex> CopyPlatformViewLayers(const FlutterLayer** layers,
+                                                               size_t layer_count) {
+  std::vector<PlatformViewLayerWithIndex> platform_views;
+  for (size_t i = 0; i < layer_count; i++) {
+    if (layers[i]->type == kFlutterLayerContentTypePlatformView) {
+      platform_views.push_back(std::make_pair(PlatformViewLayer(layers[i]), i));
+    }
+  }
+  return platform_views;
+}
+}  // namespace
+
 FlutterCompositor::FlutterCompositor(id<FlutterViewProvider> view_provider,
                                      FlutterTimeConverter* time_converter,
                                      FlutterPlatformViewController* platform_view_controller)
@@ -77,29 +90,32 @@
     presentation_time = [time_converter_ engineTimeToCAMediaTime:layers[0]->presentation_time];
   }
 
+  // Notify block below may be called asynchronously, hence the need to copy
+  // the layer information instead of passing the original pointers from embedder.
+  auto platform_views_layers = std::make_shared<std::vector<PlatformViewLayerWithIndex>>(
+      CopyPlatformViewLayers(layers, layers_count));
+
   [view.surfaceManager presentSurfaces:surfaces
                                 atTime:presentation_time
                                 notify:^{
-                                  PresentPlatformViews(view, layers, layers_count);
+                                  PresentPlatformViews(view, *platform_views_layers);
                                 }];
 
   return true;
 }
 
-void FlutterCompositor::PresentPlatformViews(FlutterView* default_base_view,
-                                             const FlutterLayer** layers,
-                                             size_t layers_count) {
+void FlutterCompositor::PresentPlatformViews(
+    FlutterView* default_base_view,
+    const std::vector<PlatformViewLayerWithIndex>& platform_views) {
   FML_DCHECK([[NSThread currentThread] isMainThread])
       << "Must be on the main thread to present platform views";
 
   // Active mutator views for this frame.
   NSMutableArray<FlutterMutatorView*>* present_mutators = [NSMutableArray array];
 
-  for (size_t i = 0; i < layers_count; i++) {
-    FlutterLayer* layer = (FlutterLayer*)layers[i];
-    if (layer->type == kFlutterLayerContentTypePlatformView) {
-      [present_mutators addObject:PresentPlatformView(default_base_view, layer, i)];
-    }
+  for (const auto& platform_view : platform_views) {
+    [present_mutators addObject:PresentPlatformView(default_base_view, platform_view.first,
+                                                    platform_view.second)];
   }
 
   NSMutableArray<FlutterMutatorView*>* obsolete_mutators =
@@ -115,12 +131,12 @@
 }
 
 FlutterMutatorView* FlutterCompositor::PresentPlatformView(FlutterView* default_base_view,
-                                                           const FlutterLayer* layer,
-                                                           size_t layer_position) {
+                                                           const PlatformViewLayer& layer,
+                                                           size_t index) {
   FML_DCHECK([[NSThread currentThread] isMainThread])
       << "Must be on the main thread to present platform views";
 
-  int64_t platform_view_id = layer->platform_view->identifier;
+  int64_t platform_view_id = layer.identifier();
   NSView* platform_view = [platform_view_controller_ platformViewWithID:platform_view_id];
 
   FML_DCHECK(platform_view) << "Platform view not found for id: " << platform_view_id;
@@ -133,8 +149,8 @@
     [default_base_view addSubview:container];
   }
 
-  container.layer.zPosition = layer_position;
-  [container applyFlutterLayer:layer];
+  container.layer.zPosition = index;
+  [container applyFlutterLayer:&layer];
 
   return container;
 }

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

@@ -6,9 +6,37 @@
 #define FLUTTER_SHELL_PLATFORM_DARWIN_MACOS_FRAMEWORK_SOURCE_FLUTTERMUTATORVIEW_H_
 
 #import <Cocoa/Cocoa.h>
+#include <vector>
 
 #include "flutter/shell/platform/embedder/embedder.h"
 
+namespace flutter {
+
+/// Represents a platform view layer, including all mutations.
+class PlatformViewLayer {
+ public:
+  /// Creates platform view from provided FlutterLayer, which must be
+  /// of type kFlutterLayerContentTypePlatformView.
+  explicit PlatformViewLayer(const FlutterLayer* _Nonnull layer);
+
+  PlatformViewLayer(FlutterPlatformViewIdentifier identifier,
+                    const std::vector<FlutterPlatformViewMutation>& mutations,
+                    FlutterPoint offset,
+                    FlutterSize size);
+
+  FlutterPlatformViewIdentifier identifier() const { return identifier_; }
+  const std::vector<FlutterPlatformViewMutation>& mutations() const { return mutations_; }
+  FlutterPoint offset() const { return offset_; }
+  FlutterSize size() const { return size_; }
+
+ private:
+  FlutterPlatformViewIdentifier identifier_;
+  std::vector<FlutterPlatformViewMutation> mutations_;
+  FlutterPoint offset_;
+  FlutterSize size_;
+};
+}  // namespace flutter
+
 /// FlutterMutatorView contains platform view and is responsible for applying
 /// FlutterLayer mutations to it.
 @interface FlutterMutatorView : NSView
@@ -22,7 +50,7 @@
 /// Applies mutations from FlutterLayer to the platform view. This may involve
 /// creating or removing intermediate subviews depending on current state and
 /// requested mutations.
-- (void)applyFlutterLayer:(nonnull const FlutterLayer*)layer;
+- (void)applyFlutterLayer:(nonnull const flutter::PlatformViewLayer*)layer;
 
 @end
 

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

@@ -12,6 +12,24 @@
 
 #import "flutter/shell/platform/darwin/macos/framework/Source/NSView+ClipsToBounds.h"
 
+namespace flutter {
+PlatformViewLayer::PlatformViewLayer(const FlutterLayer* layer) {
+  FML_CHECK(layer->type == kFlutterLayerContentTypePlatformView);
+  const auto* platform_view = layer->platform_view;
+  identifier_ = platform_view->identifier;
+  for (size_t i = 0; i < platform_view->mutations_count; i++) {
+    mutations_.push_back(*platform_view->mutations[i]);
+  }
+  offset_ = layer->offset;
+  size_ = layer->size;
+}
+PlatformViewLayer::PlatformViewLayer(FlutterPlatformViewIdentifier identifier,
+                                     const std::vector<FlutterPlatformViewMutation>& mutations,
+                                     FlutterPoint offset,
+                                     FlutterSize size)
+    : identifier_(identifier), mutations_(mutations), offset_(offset), size_(size) {}
+}  // namespace flutter
+
 @interface FlutterMutatorView () {
   // Each of these views clips to a CGPathRef. These views, if present,
   // are nested (first is child of FlutterMutatorView and last is parent of
@@ -230,19 +248,16 @@ CGPathRef PathFromRoundedRect(const FlutterRoundedRect& roundedRect) {
 /// The transforms sent from the engine include a transform from logical to physical coordinates.
 /// Since Cocoa deals only in logical points, this function prepends a scale transform that scales
 /// back from physical to logical coordinates to compensate.
-MutationVector MutationsForPlatformView(const FlutterPlatformView* view, float scale) {
-  MutationVector mutations;
-  mutations.reserve(view->mutations_count + 1);
-  mutations.push_back({
-      .type = kFlutterPlatformViewMutationTypeTransformation,
-      .transformation{
-          .scaleX = 1.0 / scale,
-          .scaleY = 1.0 / scale,
-      },
-  });
-  for (size_t i = 0; i < view->mutations_count; ++i) {
-    mutations.push_back(*view->mutations[i]);
-  }
+MutationVector MutationsForPlatformView(const MutationVector& mutationsIn, float scale) {
+  MutationVector mutations(mutationsIn);
+
+  mutations.insert(mutations.begin(), {
+                                          .type = kFlutterPlatformViewMutationTypeTransformation,
+                                          .transformation{
+                                              .scaleX = 1.0 / scale,
+                                              .scaleY = 1.0 / scale,
+                                          },
+                                      });
   return mutations;
 }
 
@@ -484,18 +499,18 @@ - (void)updatePlatformViewWithBounds:(CGRect)untransformedBounds
 /// Whenever possible view will be clipped using layer bounds.
 /// If clipping to path is needed, CAShapeLayer(s) will be used as mask.
 /// Clipping to round rect only clips to path if round corners are intersected.
-- (void)applyFlutterLayer:(const FlutterLayer*)layer {
+- (void)applyFlutterLayer:(const flutter::PlatformViewLayer*)layer {
   // Compute the untransformed bounding rect for the platform view in logical pixels.
   // FlutterLayer.size is in physical pixels but Cocoa uses logical points.
   CGFloat scale = [self contentsScale];
-  MutationVector mutations = MutationsForPlatformView(layer->platform_view, scale);
+  MutationVector mutations = MutationsForPlatformView(layer->mutations(), scale);
 
   CATransform3D finalTransform = CATransformFromMutations(mutations);
 
   // Compute the untransformed bounding rect for the platform view in logical pixels.
   // FlutterLayer.size is in physical pixels but Cocoa uses logical points.
   CGRect untransformedBoundingRect =
-      CGRectMake(0, 0, layer->size.width / scale, layer->size.height / scale);
+      CGRectMake(0, 0, layer->size().width / scale, layer->size().height / scale);
   CGRect finalBoundingRect = CGRectApplyAffineTransform(
       untransformedBoundingRect, CATransform3DGetAffineTransform(finalTransform));
   self.frame = finalBoundingRect;

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

@@ -21,26 +21,12 @@ @interface FlutterMutatorView (Private)
 void ApplyFlutterLayer(FlutterMutatorView* view,
                        FlutterSize size,
                        const std::vector<FlutterPlatformViewMutation>& mutations) {
-  FlutterLayer layer;
-  layer.struct_size = sizeof(FlutterLayer);
-  layer.type = kFlutterLayerContentTypePlatformView;
-  // Offset is ignored by mutator view, the bounding rect is determined by
-  // width and transform.
-  layer.offset = FlutterPoint{0, 0};
-  layer.size = size;
-
-  FlutterPlatformView flutterPlatformView;
-  flutterPlatformView.struct_size = sizeof(FlutterPlatformView);
-  flutterPlatformView.identifier = 0;
-
-  std::vector<const FlutterPlatformViewMutation*> mutationPointers;
-  mutationPointers.reserve(mutations.size());
-  for (auto& mutation : mutations) {
-    mutationPointers.push_back(&mutation);
-  }
-  flutterPlatformView.mutations = mutationPointers.data();
-  flutterPlatformView.mutations_count = mutationPointers.size();
-  layer.platform_view = &flutterPlatformView;
+  flutter::PlatformViewLayer layer(0,  // identifier
+                                   mutations,
+                                   // Offset is ignored by mutator view, the bounding rect is
+                                   // determined by width and transform.
+                                   FlutterPoint{0, 0},  // offset
+                                   size);
 
   [view applyFlutterLayer:&layer];
 }

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

@@ -222,39 +222,45 @@ - (void)presentSurfaces:(NSArray<FlutterSurfacePresentInfo*>*)surfaces
   [commandBuffer commit];
   [commandBuffer waitUntilScheduled];
 
+  dispatch_block_t presentBlock = ^{
+    // Get the actual dimensions of the frame (relevant for thread synchronizer).
+    CGSize size = GetRequiredFrameSize(surfaces);
+    [_delegate onPresent:size
+               withBlock:^{
+                 _lastPresentationTime = presentationTime;
+                 [self commit:surfaces];
+                 if (notify != nil) {
+                   notify();
+                 }
+               }];
+  };
+
   if (presentationTime > 0) {
     // Enforce frame pacing. It seems that the target timestamp of CVDisplayLink does not
     // exactly correspond to core animation deadline. Especially with 120hz, setting the frame
     // contents too close after previous target timestamp will result in uneven frame pacing.
     // Empirically setting the content in the second half of frame interval seems to work
     // well for both 60hz and 120hz.
     //
-    // The easiest way to ensure that the content is not set too early is to delay raster thread.
-    // At this point raster thread should be idle (the next frame vsync has not been signalled yet).
-    // This will show on a timeline as "FlutterCompositionPresentLayers" but should not cause jank
-    // because the waiting interval is calculated relative to presentation time.
+    // This schedules a timer on current (raster) thread runloop. Raster thread at
+    // this point should be idle (the next frame vsync has not been signalled yet).
     //
-    // Alternative to blocking raster thread would be to copy all presentation info provided by
-    // embedder and schedule a presentation timer. This would require additional coordination with
-    // FlutterThreadSynchronizer.
+    // Alternative could be simply blocking the raster thread, but that would show
+    // as a average_frame_rasterizer_time_millis regresson.
     CFTimeInterval minPresentationTime = (presentationTime + _lastPresentationTime) / 2.0;
     CFTimeInterval now = CACurrentMediaTime();
     if (now < minPresentationTime) {
-      [NSThread sleepForTimeInterval:minPresentationTime - now];
+      // [NSThread sleepForTimeInterval:minPresentationTime - now];
+      NSTimer* timer = [NSTimer timerWithTimeInterval:minPresentationTime - now
+                                              repeats:NO
+                                                block:^(NSTimer* timer) {
+                                                  presentBlock();
+                                                }];
+      [[NSRunLoop currentRunLoop] addTimer:timer forMode:NSDefaultRunLoopMode];
+      return;
     }
   }
-  _lastPresentationTime = presentationTime;
-
-  // Get the actual dimensions of the frame (relevant for thread synchronizer).
-  CGSize size = GetRequiredFrameSize(surfaces);
-
-  [_delegate onPresent:size
-             withBlock:^{
-               [self commit:surfaces];
-               if (notify != nil) {
-                 notify();
-               }
-             }];
+  presentBlock();
 }
 
 @end