From ef09d5bf92bb347a74ad956eb4f54c9ec930f417 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 23 Dec 2022 22:47:13 -0800 Subject: [PATCH 1/6] reuse maskView reuse mask view format format draft draft fixes test --- .../framework/Source/FlutterPlatformViews.mm | 55 ++++++--- .../Source/FlutterPlatformViewsTest.mm | 105 ++++++++++++++++++ .../Source/FlutterPlatformViews_Internal.h | 18 +++ .../Source/FlutterPlatformViews_Internal.mm | 64 +++++++++++ 4 files changed, 224 insertions(+), 18 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 69a96feeec142..6d1866dd9b1e5 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -18,6 +18,8 @@ #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) { @@ -451,6 +453,17 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, return clipCount; } +void FlutterPlatformViewsController::ClipViewAddMaskView(UIView* clipView) { + if (clipView.maskView) { + return; + } + UIView* flutterView = flutter_view_.get(); + CGRect frame = + CGRectMake(-clipView.frame.origin.x, -clipView.frame.origin.y, + CGRectGetWidth(flutterView.bounds), CGRectGetHeight(flutterView.bounds)); + clipView.maskView = [mask_view_pool_.get() getMaskViewWithFrame:frame]; +} + void FlutterPlatformViewsController::ApplyMutators(const MutatorsStack& mutators_stack, UIView* embedded_view, const SkRect& bounding_rect) { @@ -461,18 +474,15 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, ResetAnchor(embedded_view.layer); ChildClippingView* clipView = (ChildClippingView*)embedded_view.superview; - CGFloat screenScale = [UIScreen mainScreen].scale; - - UIView* flutter_view = flutter_view_.get(); - FlutterClippingMaskView* maskView = [[[FlutterClippingMaskView alloc] - initWithFrame:CGRectMake(-clipView.frame.origin.x, -clipView.frame.origin.y, - CGRectGetWidth(flutter_view.bounds), - CGRectGetHeight(flutter_view.bounds)) - screenScale:screenScale] autorelease]; - SkMatrix transformMatrix; 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]); + } + [mask_view_pool_.get() recycleMaskViews]; clipView.maskView = nil; auto iter = mutators_stack.Begin(); while (iter != mutators_stack.End()) { @@ -486,8 +496,9 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, transformMatrix)) { break; } - [maskView clipRect:(*iter)->GetRect() matrix:transformMatrix]; - clipView.maskView = maskView; + ClipViewAddMaskView(clipView); + [(FlutterClippingMaskView*)clipView.maskView clipRect:(*iter)->GetRect() + matrix:transformMatrix]; break; } case kClipRRect: { @@ -495,16 +506,18 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, transformMatrix)) { break; } - [maskView clipRRect:(*iter)->GetRRect() matrix:transformMatrix]; - clipView.maskView = maskView; + ClipViewAddMaskView(clipView); + [(FlutterClippingMaskView*)clipView.maskView clipRRect:(*iter)->GetRRect() + matrix:transformMatrix]; break; } case kClipPath: { // TODO(cyanglaz): Find a way to pre-determine if path contains the PlatformView boudning // rect. See `ClipRRectContainsPlatformViewBoundingRect`. // https://github.com/flutter/flutter/issues/118650 - [maskView clipPath:(*iter)->GetPath() matrix:transformMatrix]; - clipView.maskView = maskView; + ClipViewAddMaskView(clipView); + [(FlutterClippingMaskView*)clipView.maskView clipPath:(*iter)->GetPath() + matrix:transformMatrix]; break; } case kOpacity: @@ -551,10 +564,13 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, [clipView applyBlurBackdropFilters:blurFilters]; } + CGFloat screenScale = [UIScreen mainScreen].scale; // The UIKit frame is set based on the logical resolution (points) instead of physical. + // // (https://developer.apple.com/library/archive/documentation/DeviceInformation/Reference/iOSDeviceCompatibility/Displays/Displays.html). // However, flow is based on the physical resolution. For example, 1000 pixels in flow equals - // 500 points in UIKit for devices that has screenScale of 2. We need to scale the transformMatrix + // 500 points in UIKit for devices that has screenScale of 2. We need to scale the + // transformMatrix // down to the logical resoltion before applying it to the layer of PlatformView. transformMatrix.postScale(1 / screenScale, 1 / screenScale); @@ -563,11 +579,13 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, // Thus, this translate needs to be reversed so the platform view can layout at the correct // offset. // - // Note that the transforms are not applied to the clipping paths because clipping paths happen on + // Note that the transforms are not applied to the clipping paths because clipping paths happen + // on // the mask view, whose origin is always (0,0) to the flutter_view. transformMatrix.postTranslate(-clipView.frame.origin.x, -clipView.frame.origin.y); embedded_view.layer.transform = flutter::GetCATransform3DFromSkMatrix(transformMatrix); + // embedded_view.frame = clipView.bounds; } void FlutterPlatformViewsController::CompositeWithParams(int view_id, @@ -893,6 +911,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, if (catransaction_added_) { FML_DCHECK([[NSThread currentThread] isMainThread]); [CATransaction commit]; + catransaction_added_ = false; } } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 9b8ee71ead7cd..fa2126432dcc0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -2445,6 +2445,111 @@ - (void)testHasFirstResponderInViewHierarchySubtree_descendantViewBecomesFirstRe XCTAssertFalse(view.flt_hasFirstResponderInViewHierarchySubtree); } +- (void)testFlutterClippingMaskViewPoolReuseViewsAfterRecycle { + FlutterClippingMaskViewPool* pool = + [[[FlutterClippingMaskViewPool alloc] initWithCapacity:2] autorelease]; + FlutterClippingMaskView* view1 = [pool getMaskViewWithFrame:CGRectZero]; + FlutterClippingMaskView* view2 = [pool getMaskViewWithFrame:CGRectZero]; + [pool recycleMaskViews]; + CGRect newRect = CGRectMake(0, 0, 10, 10); + FlutterClippingMaskView* view3 = [pool getMaskViewWithFrame:newRect]; + FlutterClippingMaskView* view4 = [pool getMaskViewWithFrame:newRect]; + XCTAssertEqual(view1, view3); + XCTAssertEqual(view2, view4); + XCTAssertTrue(CGRectEqualToRect(view3.frame, newRect)); + XCTAssertTrue(CGRectEqualToRect(view4.frame, newRect)); +} + +- (void)testFlutterClippingMaskViewPoolAllocsNewMaskViewsAfterReachingCapacity { + FlutterClippingMaskViewPool* pool = + [[[FlutterClippingMaskViewPool alloc] initWithCapacity:2] autorelease]; + FlutterClippingMaskView* view1 = [pool getMaskViewWithFrame:CGRectZero]; + FlutterClippingMaskView* view2 = [pool getMaskViewWithFrame:CGRectZero]; + FlutterClippingMaskView* view3 = [pool getMaskViewWithFrame:CGRectZero]; + XCTAssertNotEqual(view1, view3); + XCTAssertNotEqual(view2, view3); +} + +- (void)testClipMaskViewIsReused { + 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(); + auto platform_view = std::make_unique( + /*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); + + 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( + screenScaleMatrix, SkSize::Make(10, 10), stack1); + + flutter::MutatorsStack stack2; + auto embeddedViewParams2 = std::make_unique( + 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. + auto embeddedViewParams3 = std::make_unique( + screenScaleMatrix, SkSize::Make(10, 10), stack2); + flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams3)); + flutterPlatformViewsController->CompositeEmbeddedView(1); + childClippingView1 = gMockPlatformView.superview.superview; + + // This overrides gMockPlatformView to point to the newly created platform view. + flutterPlatformViewsController->OnMethodCall( + [FlutterMethodCall + methodCallWithMethodName:@"create" + arguments:@{@"id" : @2, @"viewType" : @"MockFlutterPlatformView"}], + result); + + auto embeddedViewParams4 = std::make_unique( + screenScaleMatrix, SkSize::Make(10, 10), stack1); + flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams4)); + flutterPlatformViewsController->CompositeEmbeddedView(2); + UIView* childClippingView2 = gMockPlatformView.superview.superview; + + UIView* maskView2 = childClippingView2.maskView; + XCTAssertEqual(maskView1, maskView2); + XCTAssertNotNil(childClippingView2.maskView); + XCTAssertNil(childClippingView1.maskView); +} + // Return true if a correct visual effect view is found. It also implies all the validation in this // method passes. // diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index 7ec51e3b8ffcf..f32660d9480ff 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -30,6 +30,8 @@ - (instancetype)initWithFrame:(CGRect)frame screenScale:(CGFloat)screenScale; +- (void)reset; + // Adds a clip rect operation to the queue. // // The `clipSkRect` is transformed with the `matrix` before adding to the queue. @@ -47,6 +49,20 @@ @end +// A pool that provides |FlutterClippingMaskView|s. +// +// Allocation and deallocation of |FlutterClippingMaskView| is minimized while using the pool. +@interface FlutterClippingMaskViewPool : NSObject + +- (instancetype)initWithCapacity:(NSInteger)capacity; + +// Reuse a maskView from the pool, or allocate a new one. +- (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame; + +- (void)recycleMaskViews; + +@end + // An object represents a blur filter. // // This object produces a `backdropFilterView`. @@ -268,6 +284,7 @@ class FlutterPlatformViewsController { // Traverse the `mutators_stack` and return the number of clip operations. int CountClips(const MutatorsStack& mutators_stack); + void ClipViewAddMaskView(UIView* clipView); // Applies the mutators in the mutators_stack to the UIView chain that was constructed by // `ReconstructClipViewsChain` // @@ -328,6 +345,7 @@ class FlutterPlatformViewsController { fml::scoped_nsobject channel_; fml::scoped_nsobject flutter_view_; fml::scoped_nsobject flutter_view_controller_; + fml::scoped_nsobject mask_view_pool_; std::map>> factories_; std::map>> views_; std::map> touch_interceptors_; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm index da7f156bafe95..74e421c9e0cf7 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm @@ -271,6 +271,10 @@ - (instancetype)initWithFrame:(CGRect)frame screenScale:(CGFloat)screenScale { return self; } +- (void)reset { + paths_.clear(); +} + // In some scenarios, when we add this view as a maskView of the ChildClippingView, iOS added // this view as a subview of the ChildClippingView. // This results this view blocking touch events on the ChildClippingView. @@ -447,3 +451,63 @@ - (void)clipPath:(const SkPath&)path matrix:(const SkMatrix&)matrix { } @end + +@interface FlutterClippingMaskViewPool () + +@property(assign, nonatomic) NSUInteger capacity; +@property(retain, nonatomic) NSMutableArray* pool; +@property(assign, nonatomic) NSUInteger availableIndex; + +@end + +@implementation FlutterClippingMaskViewPool : NSObject + +- (instancetype)initWithCapacity:(NSInteger)capacity { + if (self = [super init]) { + _pool = [[NSMutableArray alloc] initWithCapacity:capacity]; + _capacity = capacity; + _availableIndex = 0; + } + return self; +} + +// Reuse a maskView from the pool, or allocate a new one. +- (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame { + FML_DCHECK(self.availableIndex <= self.capacity); + FlutterClippingMaskView* maskView; + if (self.availableIndex == self.capacity) { + // The pool is full, 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 { + maskView = [self.pool objectAtIndex:self.availableIndex]; + maskView.frame = frame; + [maskView reset]; + } + self.availableIndex++; + return maskView; +} + +- (void)recycleMaskViews { + self.availableIndex = 0; +} + +- (void)dealloc { + [_pool release]; + _pool = nil; + + [super dealloc]; +} + +@end From afa4f42a191c4c070e00de4ea746012707729bbd Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 18 Jan 2023 15:16:51 -0800 Subject: [PATCH 2/6] comments --- .../ios/framework/Source/FlutterPlatformViews.mm | 9 ++------- .../framework/Source/FlutterPlatformViews_Internal.h | 10 +++++++++- .../framework/Source/FlutterPlatformViews_Internal.mm | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 6d1866dd9b1e5..1c496d85da98e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -566,11 +566,9 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, CGFloat screenScale = [UIScreen mainScreen].scale; // The UIKit frame is set based on the logical resolution (points) instead of physical. - // // (https://developer.apple.com/library/archive/documentation/DeviceInformation/Reference/iOSDeviceCompatibility/Displays/Displays.html). // However, flow is based on the physical resolution. For example, 1000 pixels in flow equals - // 500 points in UIKit for devices that has screenScale of 2. We need to scale the - // transformMatrix + // 500 points in UIKit for devices that has screenScale of 2. We need to scale the transformMatrix // down to the logical resoltion before applying it to the layer of PlatformView. transformMatrix.postScale(1 / screenScale, 1 / screenScale); @@ -579,13 +577,11 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, // Thus, this translate needs to be reversed so the platform view can layout at the correct // offset. // - // Note that the transforms are not applied to the clipping paths because clipping paths happen - // on + // Note that the transforms are not applied to the clipping paths because clipping paths happen on // the mask view, whose origin is always (0,0) to the flutter_view. transformMatrix.postTranslate(-clipView.frame.origin.x, -clipView.frame.origin.y); embedded_view.layer.transform = flutter::GetCATransform3DFromSkMatrix(transformMatrix); - // embedded_view.frame = clipView.bounds; } void FlutterPlatformViewsController::CompositeWithParams(int view_id, @@ -911,7 +907,6 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, if (catransaction_added_) { FML_DCHECK([[NSThread currentThread] isMainThread]); [CATransaction commit]; - catransaction_added_ = false; } } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index f32660d9480ff..ffa8c53e7b45d 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -51,14 +51,22 @@ // A pool that provides |FlutterClippingMaskView|s. // -// Allocation and deallocation of |FlutterClippingMaskView| is minimized while using the pool. +// The pool has a capacity that can be set in the initializer. +// When requesting a FlutterClippingMaskView, the pool will first try to reuse an available maskView +// 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. @interface FlutterClippingMaskViewPool : NSObject +// Initialize the pool with `capacity`. When the `capacity` is reached, a FlutterClippingMaskView is +// constructed when requested, and it is not added to the pool. - (instancetype)initWithCapacity:(NSInteger)capacity; // Reuse a maskView from the pool, or allocate a new one. - (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame; +// Mark all the maskViews available. - (void)recycleMaskViews; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm index 74e421c9e0cf7..e85e9771f0b82 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm @@ -456,6 +456,7 @@ @interface FlutterClippingMaskViewPool () @property(assign, nonatomic) NSUInteger capacity; @property(retain, nonatomic) NSMutableArray* pool; +// The index points to the first available FlutterClippingMaskView in the `pool`. @property(assign, nonatomic) NSUInteger availableIndex; @end @@ -471,7 +472,6 @@ - (instancetype)initWithCapacity:(NSInteger)capacity { return self; } -// Reuse a maskView from the pool, or allocate a new one. - (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame { FML_DCHECK(self.availableIndex <= self.capacity); FlutterClippingMaskView* maskView; From 72b8fa94165ebe82622cfc74f62a7ff9f6771565 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 18 Jan 2023 16:14:11 -0800 Subject: [PATCH 3/6] fix comment --- .../darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm index e85e9771f0b82..28f99a14403a3 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm @@ -491,6 +491,7 @@ - (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame { [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]; From f9b8ad7a7cbf75dd336af95e2ad5bb3d64191a87 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 19 Jan 2023 15:21:11 -0800 Subject: [PATCH 4/6] test for releasing maskView --- .../framework/Source/FlutterPlatformViewsTest.mm | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index fa2126432dcc0..945e5b44536ff 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -2470,6 +2470,20 @@ - (void)testFlutterClippingMaskViewPoolAllocsNewMaskViewsAfterReachingCapacity { XCTAssertNotEqual(view2, view3); } +- (void)testMaskViewsReleasedWhenPoolIsReleased { + UIView* retainedView; + @autoreleasepool { + FlutterClippingMaskViewPool* pool = + [[[FlutterClippingMaskViewPool alloc] initWithCapacity:2] autorelease]; + FlutterClippingMaskView* view = [pool getMaskViewWithFrame:CGRectZero]; + retainedView = [view retain]; + XCTAssertGreaterThan(retainedView.retainCount, 1u); + } + // The only retain left is our manual retain called inside the autorelease pool, meaning the + // maskViews are dealloc'd. + XCTAssertEqual(retainedView.retainCount, 1u); +} + - (void)testClipMaskViewIsReused { flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate; auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest"); From 473bb50d6c44d241690db7f7ff0f74ab1f3ae73a Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 19 Jan 2023 17:45:08 -0800 Subject: [PATCH 5/6] updates --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 8 ++++---- .../ios/framework/Source/FlutterPlatformViews_Internal.mm | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 1c496d85da98e..8df862e4e3d0a 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -453,7 +453,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, return clipCount; } -void FlutterPlatformViewsController::ClipViewAddMaskView(UIView* clipView) { +void FlutterPlatformViewsController::ClipViewSetMaskView(UIView* clipView) { if (clipView.maskView) { return; } @@ -496,7 +496,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, transformMatrix)) { break; } - ClipViewAddMaskView(clipView); + ClipViewSetMaskView(clipView); [(FlutterClippingMaskView*)clipView.maskView clipRect:(*iter)->GetRect() matrix:transformMatrix]; break; @@ -506,7 +506,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, transformMatrix)) { break; } - ClipViewAddMaskView(clipView); + ClipViewSetMaskView(clipView); [(FlutterClippingMaskView*)clipView.maskView clipRRect:(*iter)->GetRRect() matrix:transformMatrix]; break; @@ -515,7 +515,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, // TODO(cyanglaz): Find a way to pre-determine if path contains the PlatformView boudning // rect. See `ClipRRectContainsPlatformViewBoundingRect`. // https://github.com/flutter/flutter/issues/118650 - ClipViewAddMaskView(clipView); + ClipViewSetMaskView(clipView); [(FlutterClippingMaskView*)clipView.maskView clipPath:(*iter)->GetPath() matrix:transformMatrix]; break; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm index 28f99a14403a3..c7df3513e6ee3 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm @@ -454,6 +454,8 @@ - (void)clipPath:(const SkPath&)path matrix:(const SkMatrix&)matrix { @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* pool; // The index points to the first available FlutterClippingMaskView in the `pool`. From 843f9520ef1a1bd42aad992cdff73713e92d15ed Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 20 Jan 2023 09:10:33 -0800 Subject: [PATCH 6/6] fix --- .../darwin/ios/framework/Source/FlutterPlatformViews_Internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index ffa8c53e7b45d..1f767b378e644 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -292,7 +292,7 @@ class FlutterPlatformViewsController { // Traverse the `mutators_stack` and return the number of clip operations. int CountClips(const MutatorsStack& mutators_stack); - void ClipViewAddMaskView(UIView* clipView); + void ClipViewSetMaskView(UIView* clipView); // Applies the mutators in the mutators_stack to the UIView chain that was constructed by // `ReconstructClipViewsChain` //