recycle maskView at the begining of the frame

remove format

revert format change

fix typo

draft

rework recycle pool

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

@@ -18,8 +18,6 @@
 #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h"
 #import "flutter/shell/platform/darwin/ios/ios_surface.h"
 
-static const NSUInteger kFlutterClippingMaskViewPoolCapacity = 5;
-
 @implementation UIView (FirstResponder)
 - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
   if (self.isFirstResponder) {
@@ -461,12 +459,10 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
   NSMutableArray* blurFilters = [[[NSMutableArray alloc] init] autorelease];
   FML_DCHECK(!clipView.maskView ||
              [clipView.maskView isKindOfClass:[FlutterClippingMaskView class]]);
-  if (mask_view_pool_.get() == nil) {
-    mask_view_pool_.reset([[FlutterClippingMaskViewPool alloc]
-        initWithCapacity:kFlutterClippingMaskViewPoolCapacity]);
+  if (clipView.maskView) {
+    [mask_view_pool_.get() insertViewToPool:(FlutterClippingMaskView*)(clipView.maskView)];
+    clipView.maskView = nil;
   }
-  [mask_view_pool_.get() recycleMaskViews];
-  clipView.maskView = nil;
   CGFloat screenScale = [UIScreen mainScreen].scale;
   auto iter = mutators_stack.Begin();
   while (iter != mutators_stack.End()) {

shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm

@@ -2649,7 +2649,8 @@ - (void)testFlutterClippingMaskViewPoolReuseViewsAfterRecycle {
       [[[FlutterClippingMaskViewPool alloc] initWithCapacity:2] autorelease];
   FlutterClippingMaskView* view1 = [pool getMaskViewWithFrame:CGRectZero];
   FlutterClippingMaskView* view2 = [pool getMaskViewWithFrame:CGRectZero];
-  [pool recycleMaskViews];
+  [pool insertViewToPool:view1];
+  [pool insertViewToPool:view2];
   CGRect newRect = CGRectMake(0, 0, 10, 10);
   FlutterClippingMaskView* view3 = [pool getMaskViewWithFrame:newRect];
   FlutterClippingMaskView* view4 = [pool getMaskViewWithFrame:newRect];
@@ -2727,17 +2728,17 @@ - (void)testClipMaskViewIsReused {
   auto embeddedViewParams1 = std::make_unique<flutter::EmbeddedViewParams>(
       screenScaleMatrix, SkSize::Make(10, 10), stack1);
 
-  flutter::MutatorsStack stack2;
-  auto embeddedViewParams2 = std::make_unique<flutter::EmbeddedViewParams>(
-      screenScaleMatrix, SkSize::Make(10, 10), stack2);
-
   flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams1));
   flutterPlatformViewsController->CompositeEmbeddedView(1);
   UIView* childClippingView1 = gMockPlatformView.superview.superview;
   UIView* maskView1 = childClippingView1.maskView;
   XCTAssertNotNil(maskView1);
 
   // Composite a new frame.
+  flutterPlatformViewsController->BeginFrame(SkISize::Make(100, 100));
+  flutter::MutatorsStack stack2;
+  auto embeddedViewParams2 = std::make_unique<flutter::EmbeddedViewParams>(
+      screenScaleMatrix, SkSize::Make(10, 10), stack2);
   auto embeddedViewParams3 = std::make_unique<flutter::EmbeddedViewParams>(
       screenScaleMatrix, SkSize::Make(10, 10), stack2);
   flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams3));
@@ -2763,6 +2764,77 @@ - (void)testClipMaskViewIsReused {
   XCTAssertNil(childClippingView1.maskView);
 }
 
+- (void)testDifferentClipMaskViewIsUsedForEachView {
+  flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
+  auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");
+  flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
+                               /*platform=*/thread_task_runner,
+                               /*raster=*/thread_task_runner,
+                               /*ui=*/thread_task_runner,
+                               /*io=*/thread_task_runner);
+  auto flutterPlatformViewsController = std::make_shared<flutter::FlutterPlatformViewsController>();
+  auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
+      /*delegate=*/mock_delegate,
+      /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
+      /*platform_views_controller=*/flutterPlatformViewsController,
+      /*task_runners=*/runners);
+
+  FlutterPlatformViewsTestMockFlutterPlatformFactory* factory =
+      [[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease];
+  flutterPlatformViewsController->RegisterViewFactory(
+      factory, @"MockFlutterPlatformView",
+      FlutterPlatformViewGestureRecognizersBlockingPolicyEager);
+  FlutterResult result = ^(id result) {
+  };
+
+  flutterPlatformViewsController->OnMethodCall(
+      [FlutterMethodCall
+          methodCallWithMethodName:@"create"
+                         arguments:@{@"id" : @1, @"viewType" : @"MockFlutterPlatformView"}],
+      result);
+  UIView* view1 = gMockPlatformView;
+
+  // This overwrites `gMockPlatformView` to another view.
+  flutterPlatformViewsController->OnMethodCall(
+      [FlutterMethodCall
+          methodCallWithMethodName:@"create"
+                         arguments:@{@"id" : @2, @"viewType" : @"MockFlutterPlatformView"}],
+      result);
+  UIView* view2 = gMockPlatformView;
+
+  XCTAssertNotNil(gMockPlatformView);
+  UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 10, 10)] autorelease];
+  flutterPlatformViewsController->SetFlutterView(mockFlutterView);
+  // Create embedded view params
+  flutter::MutatorsStack stack1;
+  // Layer tree always pushes a screen scale factor to the stack
+  SkMatrix screenScaleMatrix =
+      SkMatrix::Scale([UIScreen mainScreen].scale, [UIScreen mainScreen].scale);
+  stack1.PushTransform(screenScaleMatrix);
+  // Push a clip rect
+  SkRect rect = SkRect::MakeXYWH(2, 2, 3, 3);
+  stack1.PushClipRect(rect);
+
+  auto embeddedViewParams1 = std::make_unique<flutter::EmbeddedViewParams>(
+      screenScaleMatrix, SkSize::Make(10, 10), stack1);
+
+  flutter::MutatorsStack stack2;
+  stack2.PushClipRect(rect);
+  auto embeddedViewParams2 = std::make_unique<flutter::EmbeddedViewParams>(
+      screenScaleMatrix, SkSize::Make(10, 10), stack2);
+
+  flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams1));
+  flutterPlatformViewsController->CompositeEmbeddedView(1);
+  UIView* childClippingView1 = view1.superview.superview;
+
+  flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams2));
+  flutterPlatformViewsController->CompositeEmbeddedView(2);
+  UIView* childClippingView2 = view2.superview.superview;
+  UIView* maskView1 = childClippingView1.maskView;
+  UIView* maskView2 = childClippingView2.maskView;
+  XCTAssertNotEqual(maskView1, maskView2);
+}
+
 // Return true if a correct visual effect view is found. It also implies all the validation in this
 // method passes.
 //

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h

@@ -56,7 +56,7 @@
 // in the pool. If there are none available, a new FlutterClippingMaskView is constructed. If the
 // capacity is reached, the newly constructed FlutterClippingMaskView is not added to the pool.
 //
-// Call |recycleMaskViews| to mark all the FlutterClippingMaskViews in the pool available.
+// Call |insertViewToPool:| to return a maskView to the pool.
 @interface FlutterClippingMaskViewPool : NSObject
 
 // Initialize the pool with `capacity`. When the `capacity` is reached, a FlutterClippingMaskView is
@@ -67,7 +67,7 @@
 - (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame;
 
 // Mark all the maskViews available.
-- (void)recycleMaskViews;
+- (void)insertViewToPool:(FlutterClippingMaskView*)maskView;
 
 @end
 

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm

@@ -9,6 +9,7 @@
 #import "flutter/shell/platform/darwin/ios/ios_surface.h"
 
 static int kMaxPointsInVerb = 4;
+static const NSUInteger kFlutterClippingMaskViewPoolCapacity = 5;
 
 namespace flutter {
 
@@ -26,7 +27,10 @@
 
 FlutterPlatformViewsController::FlutterPlatformViewsController()
     : layer_pool_(std::make_unique<FlutterPlatformViewLayerPool>()),
-      weak_factory_(std::make_unique<fml::WeakPtrFactory<FlutterPlatformViewsController>>(this)){};
+      weak_factory_(std::make_unique<fml::WeakPtrFactory<FlutterPlatformViewsController>>(this)) {
+  mask_view_pool_.reset(
+      [[FlutterClippingMaskViewPool alloc] initWithCapacity:kFlutterClippingMaskViewPoolCapacity]);
+};
 
 FlutterPlatformViewsController::~FlutterPlatformViewsController() = default;
 
@@ -458,58 +462,45 @@ @interface FlutterClippingMaskViewPool ()
 // The maximum number of `FlutterClippingMaskView` the pool can contain.
 // This prevents the pool to grow infinately and limits the maximum memory a pool can use.
 @property(assign, nonatomic) NSUInteger capacity;
-@property(retain, nonatomic) NSMutableArray<FlutterClippingMaskView*>* pool;
-// The index points to the first available FlutterClippingMaskView in the `pool`.
-@property(assign, nonatomic) NSUInteger availableIndex;
+
+@property(retain, nonatomic) NSMutableSet<FlutterClippingMaskView*>* pool;
 
 @end
 
 @implementation FlutterClippingMaskViewPool : NSObject
 
 - (instancetype)initWithCapacity:(NSInteger)capacity {
   if (self = [super init]) {
-    _pool = [[NSMutableArray alloc] initWithCapacity:capacity];
+    _pool = [[NSMutableSet alloc] init];
     _capacity = capacity;
-    _availableIndex = 0;
   }
   return self;
 }
 
 - (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame {
-  FML_DCHECK(self.availableIndex <= self.capacity);
+  FML_DCHECK(self.pool.count <= self.capacity);
   FlutterClippingMaskView* maskView;
-  if (self.availableIndex == self.capacity) {
-    // The pool is full, alloc a new one.
+  if (self.pool.count == 0) {
+    // The pool is empty, alloc a new one.
     maskView =
         [[[FlutterClippingMaskView alloc] initWithFrame:frame
                                             screenScale:[UIScreen mainScreen].scale] autorelease];
     return maskView;
   }
-
-  if (self.availableIndex >= self.pool.count) {
-    // The pool doesn't have enough maskViews, alloc a new one and add to the pool.
-    maskView =
-        [[[FlutterClippingMaskView alloc] initWithFrame:frame
-                                            screenScale:[UIScreen mainScreen].scale] autorelease];
-    [self.pool addObject:maskView];
-    FML_DCHECK(self.pool.count <= self.capacity);
-  } else {
-    // Reuse a maskView from the pool.
-    maskView = [self.pool objectAtIndex:self.availableIndex];
-    maskView.frame = frame;
-    [maskView reset];
-  }
-  self.availableIndex++;
-  return maskView;
+  maskView = [self.pool anyObject];
+  maskView.frame = frame;
+  [maskView reset];
+  [self.pool removeObject:maskView];
+  return [[maskView retain] autorelease];
 }
 
-- (void)recycleMaskViews {
-  self.availableIndex = 0;
+- (void)insertViewToPool:(FlutterClippingMaskView*)maskView {
+  FML_DCHECK(![self.pool containsObject:maskView]);
+  [self.pool addObject:maskView];
 }
 
 - (void)dealloc {
   [_pool release];
-  _pool = nil;
 
   [super dealloc];
 }

testing/scenario_app/ios/Scenarios/Scenarios.xcodeproj/project.pbxproj

@@ -57,6 +57,9 @@
 		684FFF8D29F9C10700281002 /* golden_platform_view_large_cliprrect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 684FFF7929F9C10600281002 /* golden_platform_view_large_cliprrect_iPhone SE (3rd generation)_16.2_simulator.png */; };
 		684FFF8E29F9C10700281002 /* golden_spawn_engine_works_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 684FFF7A29F9C10700281002 /* golden_spawn_engine_works_iPhone SE (3rd generation)_16.2_simulator.png */; };
 		684FFF8F29F9C10700281002 /* golden_platform_view_with_other_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 684FFF7B29F9C10700281002 /* golden_platform_view_with_other_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png */; };
+		6860CE252A01B2FF00B68EC5 /* golden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 6860CE222A01B2FF00B68EC5 /* golden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_16.2_simulator.png */; };
+		6860CE262A01B2FF00B68EC5 /* golden_two_platform_view_clip_rect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 6860CE232A01B2FF00B68EC5 /* golden_two_platform_view_clip_rect_iPhone SE (3rd generation)_16.2_simulator.png */; };
+		6860CE272A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 6860CE242A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png */; };
 		68A5B63423EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */; };
 		68D4017D2564859300ECD91A /* ContinuousTexture.m in Sources */ = {isa = PBXBuildFile; fileRef = 68D4017C2564859300ECD91A /* ContinuousTexture.m */; };
 		F26F15B8268B6B5600EC54D3 /* iPadGestureTests.m in Sources */ = {isa = PBXBuildFile; fileRef = F26F15B7268B6B5500EC54D3 /* iPadGestureTests.m */; };
@@ -174,6 +177,9 @@
 		684FFF7929F9C10600281002 /* golden_platform_view_large_cliprrect_iPhone SE (3rd generation)_16.2_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_large_cliprrect_iPhone SE (3rd generation)_16.2_simulator.png"; sourceTree = "<group>"; };
 		684FFF7A29F9C10700281002 /* golden_spawn_engine_works_iPhone SE (3rd generation)_16.2_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_spawn_engine_works_iPhone SE (3rd generation)_16.2_simulator.png"; sourceTree = "<group>"; };
 		684FFF7B29F9C10700281002 /* golden_platform_view_with_other_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_with_other_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png"; sourceTree = "<group>"; };
+		6860CE222A01B2FF00B68EC5 /* golden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_16.2_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_16.2_simulator.png"; sourceTree = "<group>"; };
+		6860CE232A01B2FF00B68EC5 /* golden_two_platform_view_clip_rect_iPhone SE (3rd generation)_16.2_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_two_platform_view_clip_rect_iPhone SE (3rd generation)_16.2_simulator.png"; sourceTree = "<group>"; };
+		6860CE242A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png"; sourceTree = "<group>"; };
 		68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = PlatformViewGestureRecognizerTests.m; sourceTree = "<group>"; };
 		68D4017B2564859300ECD91A /* ContinuousTexture.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ContinuousTexture.h; sourceTree = "<group>"; };
 		68D4017C2564859300ECD91A /* ContinuousTexture.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ContinuousTexture.m; sourceTree = "<group>"; };
@@ -302,6 +308,9 @@
 		F7B464DC2759D02B00079189 /* Goldens */ = {
 			isa = PBXGroup;
 			children = (
+				6860CE242A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png */,
+				6860CE232A01B2FF00B68EC5 /* golden_two_platform_view_clip_rect_iPhone SE (3rd generation)_16.2_simulator.png */,
+				6860CE222A01B2FF00B68EC5 /* golden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_16.2_simulator.png */,
 				684FFF7229F9C10500281002 /* golden_bogus_font_text_iPhone SE (3rd generation)_16.2_simulator.png */,
 				684FFF7029F9C10500281002 /* golden_non_full_screen_flutter_view_platform_view_iPhone SE (3rd generation)_16.2_simulator.png */,
 				684FFF7729F9C10600281002 /* golden_platform_view_clippath_iPhone SE (3rd generation)_16.2_simulator.png */,
@@ -456,7 +465,9 @@
 				684FFF7D29F9C10700281002 /* golden_platform_view_cliprect_with_transform_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8829F9C10700281002 /* golden_platform_view_cliprect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8529F9C10700281002 /* golden_platform_view_cliprrect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
+				6860CE252A01B2FF00B68EC5 /* golden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8F29F9C10700281002 /* golden_platform_view_with_other_backdrop_filter_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
+				6860CE262A01B2FF00B68EC5 /* golden_two_platform_view_clip_rect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8A29F9C10700281002 /* golden_platform_view_clippath_with_transform_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8429F9C10700281002 /* golden_non_full_screen_flutter_view_platform_view_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF7C29F9C10700281002 /* golden_platform_view_cliprrect_with_transform_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
@@ -469,6 +480,7 @@
 				684FFF8D29F9C10700281002 /* golden_platform_view_large_cliprrect_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8329F9C10700281002 /* golden_platform_view_transform_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8B29F9C10700281002 /* golden_platform_view_clippath_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
+				6860CE272A01B2FF00B68EC5 /* golden_two_platform_view_clip_path_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 				684FFF8129F9C10700281002 /* golden_platform_view_large_cliprrect_with_transform_iPhone SE (3rd generation)_16.2_simulator.png in Resources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;

testing/scenario_app/ios/Scenarios/Scenarios/AppDelegate.m

@@ -69,7 +69,10 @@ - (BOOL)application:(UIApplication*)application
     @"--spawn-engine-works" : @"spawn_engine_works",
     @"--pointer-events" : @"pointer_events",
     @"--platform-view-scrolling-under-widget" : @"platform_view_scrolling_under_widget",
-    @"--platform-view-cliprect-after-moved" : @"platform_view_cliprect_after_moved"
+    @"--platform-view-cliprect-after-moved" : @"platform_view_cliprect_after_moved",
+    @"--two-platform-view-clip-rect" : @"two_platform_view_clip_rect",
+    @"--two-platform-view-clip-rrect" : @"two_platform_view_clip_rrect",
+    @"--two-platform-view-clip-path" : @"two_platform_view_clip_path",
   };
   __block NSString* flutterViewControllerTestName = nil;
   [launchArgsMap

testing/scenario_app/ios/Scenarios/ScenariosUITests/GoldenTestManager.m

@@ -47,6 +47,9 @@ - (instancetype)initWithLaunchArg:(NSString*)launchArg {
         @"--bogus-font-text" : @"bogus_font_text",
         @"--spawn-engine-works" : @"spawn_engine_works",
         @"--platform-view-cliprect-after-moved" : @"platform_view_cliprect_after_moved",
+        @"--two-platform-view-clip-rect" : @"two_platform_view_clip_rect",
+        @"--two-platform-view-clip-rrect" : @"two_platform_view_clip_rrect",
+        @"--two-platform-view-clip-path" : @"two_platform_view_clip_path",
       };
     });
     _identifier = launchArgsMap[launchArg];