-
Notifications
You must be signed in to change notification settings - Fork 6k
[ios_platform_view] MaskView pool to reuse maskViews. #38989
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,66 @@ - (void)clipPath:(const SkPath&)path matrix:(const SkMatrix&)matrix { | |
} | ||
|
||
@end | ||
|
||
@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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this retain the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well never mind, retaining it after the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will add a test for it! |
||
// The index points to the first available FlutterClippingMaskView in the `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; | ||
} | ||
|
||
- (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame { | ||
FML_DCHECK(self.availableIndex <= self.capacity); | ||
FlutterClippingMaskView* maskView; | ||
if (self.availableIndex == self.capacity) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it looks like the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can probably just compare with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The capacity is a static limit set for the pool. My intention is to limit the memory used by the pool. For example, (it is probably not possible in real life), I don't want the pool to cache 20 views for the whole application session. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. gotcha. I mistakenly thought that this capacity increases like an NSMutableArray. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yeah it could be confusing, I added a comment. |
||
// 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 { | ||
// Reuse a maskView from the pool. | ||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to make this a
scoped_nsobject
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I should change it to properties. I was blindly following the current pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, actually, it is in a c++ class. So I think scoped_nsobject is better that the lifecycle is automatically managed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which class is c++? Your new
FlutterClippingMaskViewPool
is a Obj-C class.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FlutterPlatformViewsController is the c++ class.