This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
[ios][platform_view] Use CAShapeLayer as the mask to avoid software rendering #53072
Merged
auto-submit
merged 4 commits into
flutter:main
from
hellohuanlin:pv_mask_view_use_cashapelayer_backing
Jun 5, 2024
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1741,17 +1741,15 @@ - (void)testClipRect { | |
| [mockFlutterView setNeedsLayout]; | ||
| [mockFlutterView layoutIfNeeded]; | ||
|
|
||
| CGRect insideClipping = CGRectMake(2, 2, 3, 3); | ||
| for (int i = 0; i < 10; i++) { | ||
| for (int j = 0; j < 10; j++) { | ||
| CGPoint point = CGPointMake(i, j); | ||
| int alpha = [self alphaOfPoint:CGPointMake(i, j) onView:mockFlutterView]; | ||
| // Edges of the clipping might have a semi transparent pixel, we only check the pixels that | ||
| // are fully inside the clipped area. | ||
| CGRect insideClipping = CGRectMake(3, 3, 1, 1); | ||
| if (CGRectContainsPoint(insideClipping, point)) { | ||
| XCTAssertEqual(alpha, 255); | ||
| } else { | ||
| XCTAssertLessThan(alpha, 255); | ||
| XCTAssertEqual(alpha, 0); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -1816,17 +1814,42 @@ - (void)testClipRRect { | |
| [mockFlutterView setNeedsLayout]; | ||
| [mockFlutterView layoutIfNeeded]; | ||
|
|
||
| /* | ||
| ClippingMask outterClipping | ||
| 2 3 4 5 6 7 2 3 4 5 6 7 | ||
| 2 / - - - - \ 2 + - - - - + | ||
| 3 | | 3 | | | ||
| 4 | | 4 | | | ||
| 5 | | 5 | | | ||
| 6 | | 6 | | | ||
| 7 \ - - - - / 7 + - - - - + | ||
|
|
||
| innerClipping1 innerClipping2 | ||
| 2 3 4 5 6 7 2 3 4 5 6 7 | ||
| 2 + - - + 2 | ||
| 3 | | 3 + - - - - + | ||
| 4 | | 4 | | | ||
| 5 | | 5 | | | ||
| 6 | | 6 + - - - - + | ||
| 7 + - - + 7 | ||
| */ | ||
| CGRect innerClipping1 = CGRectMake(3, 2, 4, 6); | ||
| CGRect innerClipping2 = CGRectMake(2, 3, 6, 4); | ||
| CGRect outterClipping = CGRectMake(2, 2, 6, 6); | ||
|
Contributor
Author
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. Again the existing behavior is incorrect (or at least not pixel perfect). In the picture below, the black rounded rect is the mask. Pixels within the red rect ( |
||
| for (int i = 0; i < 10; i++) { | ||
| for (int j = 0; j < 10; j++) { | ||
| CGPoint point = CGPointMake(i, j); | ||
| int alpha = [self alphaOfPoint:CGPointMake(i, j) onView:mockFlutterView]; | ||
| // Edges of the clipping might have a semi transparent pixel, we only check the pixels that | ||
| // are fully inside the clipped area. | ||
| CGRect insideClipping = CGRectMake(3, 3, 4, 4); | ||
| if (CGRectContainsPoint(insideClipping, point)) { | ||
| if (CGRectContainsPoint(innerClipping1, point) || | ||
| CGRectContainsPoint(innerClipping2, point)) { | ||
| // Pixels inside either of the 2 inner clippings should be fully opaque. | ||
| XCTAssertEqual(alpha, 255); | ||
| } else if (CGRectContainsPoint(outterClipping, point)) { | ||
| // Corner pixels (i.e. (2, 2), (2, 7), (7, 2) and (7, 7)) should be partially transparent. | ||
| XCTAssert(0 < alpha && alpha < 255); | ||
| } else { | ||
| XCTAssertLessThan(alpha, 255); | ||
| // Pixels outside outterClipping should be fully transparent. | ||
| XCTAssertEqual(alpha, 0); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -1892,17 +1915,42 @@ - (void)testClipPath { | |
| [mockFlutterView setNeedsLayout]; | ||
| [mockFlutterView layoutIfNeeded]; | ||
|
|
||
| /* | ||
| ClippingMask outterClipping | ||
| 2 3 4 5 6 7 2 3 4 5 6 7 | ||
| 2 / - - - - \ 2 + - - - - + | ||
| 3 | | 3 | | | ||
| 4 | | 4 | | | ||
| 5 | | 5 | | | ||
| 6 | | 6 | | | ||
| 7 \ - - - - / 7 + - - - - + | ||
|
|
||
| innerClipping1 innerClipping2 | ||
| 2 3 4 5 6 7 2 3 4 5 6 7 | ||
| 2 + - - + 2 | ||
| 3 | | 3 + - - - - + | ||
| 4 | | 4 | | | ||
| 5 | | 5 | | | ||
| 6 | | 6 + - - - - + | ||
| 7 + - - + 7 | ||
| */ | ||
| CGRect innerClipping1 = CGRectMake(3, 2, 4, 6); | ||
| CGRect innerClipping2 = CGRectMake(2, 3, 6, 4); | ||
| CGRect outterClipping = CGRectMake(2, 2, 6, 6); | ||
| for (int i = 0; i < 10; i++) { | ||
| for (int j = 0; j < 10; j++) { | ||
| CGPoint point = CGPointMake(i, j); | ||
| int alpha = [self alphaOfPoint:CGPointMake(i, j) onView:mockFlutterView]; | ||
| // Edges of the clipping might have a semi transparent pixel, we only check the pixels that | ||
| // are fully inside the clipped area. | ||
| CGRect insideClipping = CGRectMake(3, 3, 4, 4); | ||
| if (CGRectContainsPoint(insideClipping, point)) { | ||
| if (CGRectContainsPoint(innerClipping1, point) || | ||
| CGRectContainsPoint(innerClipping2, point)) { | ||
| // Pixels inside either of the 2 inner clippings should be fully opaque. | ||
| XCTAssertEqual(alpha, 255); | ||
| } else if (CGRectContainsPoint(outterClipping, point)) { | ||
| // Corner pixels (i.e. (2, 2), (2, 7), (7, 2) and (7, 7)) should be partially transparent. | ||
| XCTAssert(0 < alpha && alpha < 255); | ||
| } else { | ||
| XCTAssertLessThan(alpha, 255); | ||
| // Pixels outside outterClipping should be fully transparent. | ||
| XCTAssertEqual(alpha, 0); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -2957,6 +3005,69 @@ - (void)testDifferentClipMaskViewIsUsedForEachView { | |
| XCTAssertNotEqual(maskView1, maskView2); | ||
| } | ||
|
|
||
| - (void)testMaskViewUsesCAShapeLayerAsTheBackingLayer { | ||
| 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=*/mock_delegate.settings_.enable_impeller | ||
| ? flutter::IOSRenderingAPI::kMetal | ||
| : flutter::IOSRenderingAPI::kSoftware, | ||
| /*platform_views_controller=*/flutterPlatformViewsController, | ||
| /*task_runners=*/runners, | ||
| /*worker_task_runner=*/nil, | ||
| /*is_gpu_disabled_jsync_switch=*/std::make_shared<fml::SyncSwitch>()); | ||
|
|
||
| FlutterPlatformViewsTestMockFlutterPlatformFactory* factory = | ||
| [[FlutterPlatformViewsTestMockFlutterPlatformFactory alloc] init]; | ||
| 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)]; | ||
| 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* childClippingView = gMockPlatformView.superview.superview; | ||
|
|
||
| UIView* maskView = childClippingView.maskView; | ||
| XCTAssert([maskView.layer isKindOfClass:[CAShapeLayer class]], | ||
| @"Mask view must use CAShapeLayer as its backing layer."); | ||
| } | ||
|
|
||
| // Return true if a correct visual effect view is found. It also implies all the validation in this | ||
| // method passes. | ||
| // | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -255,12 +255,12 @@ @interface FlutterClippingMaskView () | |
| // information about screen scale. | ||
| @property(nonatomic) CATransform3D reverseScreenScale; | ||
|
|
||
| - (fml::CFRef<CGPathRef>)getTransformedPath:(CGPathRef)path matrix:(CATransform3D)matrix; | ||
| - (void)addTransformedPath:(CGPathRef)path matrix:(CATransform3D)matrix; | ||
|
|
||
| @end | ||
|
|
||
| @implementation FlutterClippingMaskView { | ||
| std::vector<fml::CFRef<CGPathRef>> paths_; | ||
| CGMutablePathRef pathSoFar_; | ||
| } | ||
|
|
||
| - (instancetype)initWithFrame:(CGRect)frame { | ||
|
|
@@ -271,15 +271,31 @@ - (instancetype)initWithFrame:(CGRect)frame screenScale:(CGFloat)screenScale { | |
| if (self = [super initWithFrame:frame]) { | ||
| self.backgroundColor = UIColor.clearColor; | ||
| _reverseScreenScale = CATransform3DMakeScale(1 / screenScale, 1 / screenScale, 1); | ||
| pathSoFar_ = CGPathCreateMutable(); | ||
| } | ||
| return self; | ||
| } | ||
|
|
||
| + (Class)layerClass { | ||
| return [CAShapeLayer class]; | ||
| } | ||
|
|
||
| - (CAShapeLayer*)shapeLayer { | ||
| return (CAShapeLayer*)self.layer; | ||
| } | ||
|
|
||
| - (void)reset { | ||
| paths_.clear(); | ||
| CGPathRelease(pathSoFar_); | ||
| pathSoFar_ = CGPathCreateMutable(); | ||
| [self shapeLayer].path = nil; | ||
| [self setNeedsDisplay]; | ||
| } | ||
|
|
||
| - (void)dealloc { | ||
| CGPathRelease(pathSoFar_); | ||
| [super dealloc]; | ||
|
Contributor
Author
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. @jmagman fyi potentially code conflict with your ARC migration PRs |
||
| } | ||
|
|
||
| // 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. | ||
|
|
@@ -289,28 +305,13 @@ - (BOOL)pointInside:(CGPoint)point withEvent:(UIEvent*)event { | |
| return NO; | ||
| } | ||
|
|
||
| - (void)drawRect:(CGRect)rect { | ||
| CGContextRef context = UIGraphicsGetCurrentContext(); | ||
| CGContextSaveGState(context); | ||
|
|
||
| // For mask view, only the alpha channel is used. | ||
| CGContextSetAlpha(context, 1); | ||
|
|
||
| for (size_t i = 0; i < paths_.size(); i++) { | ||
| CGContextAddPath(context, paths_.at(i)); | ||
| CGContextClip(context); | ||
| } | ||
| CGContextFillRect(context, rect); | ||
| CGContextRestoreGState(context); | ||
| } | ||
|
|
||
| - (void)clipRect:(const SkRect&)clipSkRect matrix:(const SkMatrix&)matrix { | ||
| CGRect clipRect = flutter::GetCGRectFromSkRect(clipSkRect); | ||
| CGPathRef path = CGPathCreateWithRect(clipRect, nil); | ||
| // The `matrix` is based on the physical pixels, convert it to UIKit points. | ||
| CATransform3D matrixInPoints = | ||
| CATransform3DConcat(flutter::GetCATransform3DFromSkMatrix(matrix), _reverseScreenScale); | ||
| paths_.push_back([self getTransformedPath:path matrix:matrixInPoints]); | ||
| [self addTransformedPath:path matrix:matrixInPoints]; | ||
| } | ||
|
|
||
| - (void)clipRRect:(const SkRRect&)clipSkRRect matrix:(const SkMatrix&)matrix { | ||
|
|
@@ -379,7 +380,7 @@ - (void)clipRRect:(const SkRRect&)clipSkRRect matrix:(const SkMatrix&)matrix { | |
| // TODO(cyanglaz): iOS does not seem to support hard edge on CAShapeLayer. It clearly stated that | ||
| // the CAShaperLayer will be drawn antialiased. Need to figure out a way to do the hard edge | ||
| // clipping on iOS. | ||
| paths_.push_back([self getTransformedPath:pathRef matrix:matrixInPoints]); | ||
| [self addTransformedPath:pathRef matrix:matrixInPoints]; | ||
| } | ||
|
|
||
| - (void)clipPath:(const SkPath&)path matrix:(const SkMatrix&)matrix { | ||
|
|
@@ -444,15 +445,15 @@ - (void)clipPath:(const SkPath&)path matrix:(const SkMatrix&)matrix { | |
| // The `matrix` is based on the physical pixels, convert it to UIKit points. | ||
| CATransform3D matrixInPoints = | ||
| CATransform3DConcat(flutter::GetCATransform3DFromSkMatrix(matrix), _reverseScreenScale); | ||
| paths_.push_back([self getTransformedPath:pathRef matrix:matrixInPoints]); | ||
| [self addTransformedPath:pathRef matrix:matrixInPoints]; | ||
| } | ||
|
|
||
| - (fml::CFRef<CGPathRef>)getTransformedPath:(CGPathRef)path matrix:(CATransform3D)matrix { | ||
| - (void)addTransformedPath:(CGPathRef)path matrix:(CATransform3D)matrix { | ||
| CGAffineTransform affine = | ||
| CGAffineTransformMake(matrix.m11, matrix.m12, matrix.m21, matrix.m22, matrix.m41, matrix.m42); | ||
| CGPathRef transformedPath = CGPathCreateCopyByTransformingPath(path, &affine); | ||
| CGPathAddPath(pathSoFar_, &affine, path); | ||
| [self shapeLayer].path = pathSoFar_; | ||
| CGPathRelease(path); | ||
| return fml::CFRef<CGPathRef>(transformedPath); | ||
| } | ||
|
|
||
| @end | ||
|
|
||
Binary file modified
BIN
+8.64 KB
(140%)
...sts/golden_platform_view_clippath_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+8.19 KB
(140%)
...form_view_clippath_with_transform_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+7.98 KB
(140%)
...latform_view_cliprect_after_moved_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+21 Bytes
(100%)
...form_view_cliprect_with_transform_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+8.27 KB
(140%)
...ts/golden_platform_view_cliprrect_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+21 Bytes
(100%)
...orm_view_cliprrect_with_transform_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+7.87 KB
(130%)
...den_platform_view_large_cliprrect_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
-16 Bytes
(100%)
...ew_large_cliprrect_with_transform_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+7.31 KB
(130%)
...olden_two_platform_view_clip_path_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+7.74 KB
(140%)
...olden_two_platform_view_clip_rect_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+7.14 KB
(120%)
...lden_two_platform_view_clip_rrect_iPhone SE (3rd generation)_17.0_simulator.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.

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.
The existing behavior is incorrect - there shouldn't be any semi-transparent pixels on the edge.
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.
(Introduced in #20050)