Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 2a11023

Browse files
author
Chris Yang
authored
[ios_platform_view] more precision when determine if a clip rrect is necessary (#38965)
* draft fix rename unittest * update scenario * test
1 parent 290636c commit 2a11023

10 files changed

+253
-26
lines changed

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

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,52 @@ - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
3232
}
3333
@end
3434

35-
// Determines if the final `clipBounds` from a clipRect/clipRRect/clipPath mutator contains the
35+
// Determines if the `clip_rect` from a clipRect mutator contains the
3636
// `platformview_boundingrect`.
3737
//
38-
// `clip_bounds` is the bounding rect of the rect/rrect/path in the clipRect/clipRRect/clipPath
39-
// mutator. This rect is in its own coordinate space. The rect needs to be transformed by
38+
// `clip_rect` is in its own coordinate space. The rect needs to be transformed by
4039
// `transform_matrix` to be in the coordinate space where the PlatformView is displayed.
4140
//
4241
// `platformview_boundingrect` is the final bounding rect of the PlatformView in the coordinate
4342
// space where the PlatformView is displayed.
44-
static bool ClipBoundsContainsPlatformViewBoundingRect(const SkRect& clip_bounds,
45-
const SkRect& platformview_boundingrect,
46-
const SkMatrix& transform_matrix) {
47-
SkRect transforme_clip_bounds = transform_matrix.mapRect(clip_bounds);
48-
return transforme_clip_bounds.contains(platformview_boundingrect);
43+
static bool ClipRectContainsPlatformViewBoundingRect(const SkRect& clip_rect,
44+
const SkRect& platformview_boundingrect,
45+
const SkMatrix& transform_matrix) {
46+
SkRect transformed_rect = transform_matrix.mapRect(clip_rect);
47+
return transformed_rect.contains(platformview_boundingrect);
48+
}
49+
50+
// Determines if the `clipRRect` from a clipRRect mutator contains the
51+
// `platformview_boundingrect`.
52+
//
53+
// `clip_rrect` is in its own coordinate space. The rrect needs to be transformed by
54+
// `transform_matrix` to be in the coordinate space where the PlatformView is displayed.
55+
//
56+
// `platformview_boundingrect` is the final bounding rect of the PlatformView in the coordinate
57+
// space where the PlatformView is displayed.
58+
static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
59+
const SkRect& platformview_boundingrect,
60+
const SkMatrix& transform_matrix) {
61+
SkVector upper_left = clip_rrect.radii(SkRRect::Corner::kUpperLeft_Corner);
62+
SkVector upper_right = clip_rrect.radii(SkRRect::Corner::kUpperRight_Corner);
63+
SkVector lower_right = clip_rrect.radii(SkRRect::Corner::kLowerRight_Corner);
64+
SkVector lower_left = clip_rrect.radii(SkRRect::Corner::kLowerLeft_Corner);
65+
SkScalar transformed_upper_left_x = transform_matrix.mapRadius(upper_left.x());
66+
SkScalar transformed_upper_left_y = transform_matrix.mapRadius(upper_left.y());
67+
SkScalar transformed_upper_right_x = transform_matrix.mapRadius(upper_right.x());
68+
SkScalar transformed_upper_right_y = transform_matrix.mapRadius(upper_right.y());
69+
SkScalar transformed_lower_right_x = transform_matrix.mapRadius(lower_right.x());
70+
SkScalar transformed_lower_right_y = transform_matrix.mapRadius(lower_right.y());
71+
SkScalar transformed_lower_left_x = transform_matrix.mapRadius(lower_left.x());
72+
SkScalar transformed_lower_left_y = transform_matrix.mapRadius(lower_left.y());
73+
SkRect transformed_clip_rect = transform_matrix.mapRect(clip_rrect.rect());
74+
SkRRect transformed_rrect;
75+
SkVector corners[] = {{transformed_upper_left_x, transformed_upper_left_y},
76+
{transformed_upper_right_x, transformed_upper_right_y},
77+
{transformed_lower_right_x, transformed_lower_right_y},
78+
{transformed_lower_left_x, transformed_lower_left_y}};
79+
transformed_rrect.setRectRadii(transformed_clip_rect, corners);
80+
return transformed_rrect.contains(platformview_boundingrect);
4981
}
5082

5183
namespace flutter {
@@ -450,28 +482,27 @@ static bool ClipBoundsContainsPlatformViewBoundingRect(const SkRect& clip_bounds
450482
break;
451483
}
452484
case kClipRect: {
453-
if (ClipBoundsContainsPlatformViewBoundingRect((*iter)->GetRect(), bounding_rect,
454-
transformMatrix)) {
485+
if (ClipRectContainsPlatformViewBoundingRect((*iter)->GetRect(), bounding_rect,
486+
transformMatrix)) {
455487
break;
456488
}
457489
[maskView clipRect:(*iter)->GetRect() matrix:transformMatrix];
458490
clipView.maskView = maskView;
459491
break;
460492
}
461493
case kClipRRect: {
462-
if (ClipBoundsContainsPlatformViewBoundingRect((*iter)->GetRRect().getBounds(),
463-
bounding_rect, transformMatrix)) {
494+
if (ClipRRectContainsPlatformViewBoundingRect((*iter)->GetRRect(), bounding_rect,
495+
transformMatrix)) {
464496
break;
465497
}
466498
[maskView clipRRect:(*iter)->GetRRect() matrix:transformMatrix];
467499
clipView.maskView = maskView;
468500
break;
469501
}
470502
case kClipPath: {
471-
if (ClipBoundsContainsPlatformViewBoundingRect((*iter)->GetPath().getBounds(),
472-
bounding_rect, transformMatrix)) {
473-
break;
474-
}
503+
// TODO(cyanglaz): Find a way to pre-determine if path contains the PlatformView boudning
504+
// rect. See `ClipRRectContainsPlatformViewBoundingRect`.
505+
// https://github.com/flutter/flutter/issues/118650
475506
[maskView clipPath:(*iter)->GetPath() matrix:transformMatrix];
476507
clipView.maskView = maskView;
477508
break;

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

Lines changed: 71 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,25 +1510,23 @@ - (void)testClipsDoNotInterceptWithPlatformViewShouldNotAddMaskView {
15101510

15111511
UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 30, 30)] autorelease];
15121512
flutterPlatformViewsController->SetFlutterView(mockFlutterView);
1513-
// Create embedded view params
1513+
// Create embedded view params.
15141514
flutter::MutatorsStack stack;
1515-
// Layer tree always pushes a screen scale factor to the stack
1515+
// Layer tree always pushes a screen scale factor to the stack.
15161516
SkMatrix screenScaleMatrix =
15171517
SkMatrix::Scale([UIScreen mainScreen].scale, [UIScreen mainScreen].scale);
15181518
stack.PushTransform(screenScaleMatrix);
15191519
SkMatrix translateMatrix = SkMatrix::Translate(5, 5);
1520-
// The platform view's rect for this test will be (5, 5, 10, 10)
1520+
// The platform view's rect for this test will be (5, 5, 10, 10).
15211521
stack.PushTransform(translateMatrix);
1522-
// Push a clip rect, big enough to contain the entire platform view bound
1522+
// Push a clip rect, big enough to contain the entire platform view bound.
15231523
SkRect rect = SkRect::MakeXYWH(0, 0, 25, 25);
15241524
stack.PushClipRect(rect);
1525-
// Push a clip rrect, big enough to contain the entire platform view bound
1526-
SkRect rect_for_rrect = SkRect::MakeXYWH(0, 0, 24, 24);
1525+
// Push a clip rrect, big enough to contain the entire platform view bound without clipping it.
1526+
// Make the origin (-1, -1) so that the top left rounded corner isn't clipping the PlatformView.
1527+
SkRect rect_for_rrect = SkRect::MakeXYWH(-1, -1, 25, 25);
15271528
SkRRect rrect = SkRRect::MakeRectXY(rect_for_rrect, 1, 1);
15281529
stack.PushClipRRect(rrect);
1529-
// Push a clip path, big enough to contain the entire platform view bound
1530-
SkPath path = SkPath::RRect(SkRect::MakeXYWH(0, 0, 23, 23), 1, 1);
1531-
stack.PushClipPath(path);
15321530

15331531
auto embeddedViewParams = std::make_unique<flutter::EmbeddedViewParams>(
15341532
SkMatrix::Concat(screenScaleMatrix, translateMatrix), SkSize::Make(5, 5), stack);
@@ -1542,10 +1540,73 @@ - (void)testClipsDoNotInterceptWithPlatformViewShouldNotAddMaskView {
15421540

15431541
[mockFlutterView setNeedsLayout];
15441542
[mockFlutterView layoutIfNeeded];
1545-
15461543
XCTAssertNil(childClippingView.maskView);
15471544
}
15481545

1546+
- (void)testClipRRectOnlyHasCornersInterceptWithPlatformViewShouldAddMaskView {
1547+
flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
1548+
auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");
1549+
flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
1550+
/*platform=*/thread_task_runner,
1551+
/*raster=*/thread_task_runner,
1552+
/*ui=*/thread_task_runner,
1553+
/*io=*/thread_task_runner);
1554+
auto flutterPlatformViewsController = std::make_shared<flutter::FlutterPlatformViewsController>();
1555+
auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
1556+
/*delegate=*/mock_delegate,
1557+
/*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
1558+
/*platform_views_controller=*/flutterPlatformViewsController,
1559+
/*task_runners=*/runners);
1560+
1561+
FlutterPlatformViewsTestMockFlutterPlatformFactory* factory =
1562+
[[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease];
1563+
flutterPlatformViewsController->RegisterViewFactory(
1564+
factory, @"MockFlutterPlatformView",
1565+
FlutterPlatformViewGestureRecognizersBlockingPolicyEager);
1566+
FlutterResult result = ^(id result) {
1567+
};
1568+
flutterPlatformViewsController->OnMethodCall(
1569+
[FlutterMethodCall
1570+
methodCallWithMethodName:@"create"
1571+
arguments:@{@"id" : @2, @"viewType" : @"MockFlutterPlatformView"}],
1572+
result);
1573+
1574+
XCTAssertNotNil(gMockPlatformView);
1575+
1576+
UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 30, 30)] autorelease];
1577+
flutterPlatformViewsController->SetFlutterView(mockFlutterView);
1578+
// Create embedded view params
1579+
flutter::MutatorsStack stack;
1580+
// Layer tree always pushes a screen scale factor to the stack.
1581+
SkMatrix screenScaleMatrix =
1582+
SkMatrix::Scale([UIScreen mainScreen].scale, [UIScreen mainScreen].scale);
1583+
stack.PushTransform(screenScaleMatrix);
1584+
SkMatrix translateMatrix = SkMatrix::Translate(5, 5);
1585+
// The platform view's rect for this test will be (5, 5, 10, 10).
1586+
stack.PushTransform(translateMatrix);
1587+
1588+
// Push a clip rrect, the rect of the rrect is the same as the PlatformView of the corner should.
1589+
// clip the PlatformView.
1590+
SkRect rect_for_rrect = SkRect::MakeXYWH(0, 0, 10, 10);
1591+
SkRRect rrect = SkRRect::MakeRectXY(rect_for_rrect, 1, 1);
1592+
stack.PushClipRRect(rrect);
1593+
1594+
auto embeddedViewParams = std::make_unique<flutter::EmbeddedViewParams>(
1595+
SkMatrix::Concat(screenScaleMatrix, translateMatrix), SkSize::Make(5, 5), stack);
1596+
1597+
flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams));
1598+
flutterPlatformViewsController->CompositeEmbeddedView(2);
1599+
gMockPlatformView.backgroundColor = UIColor.redColor;
1600+
XCTAssertTrue([gMockPlatformView.superview.superview isKindOfClass:ChildClippingView.class]);
1601+
ChildClippingView* childClippingView = (ChildClippingView*)gMockPlatformView.superview.superview;
1602+
[mockFlutterView addSubview:childClippingView];
1603+
1604+
[mockFlutterView setNeedsLayout];
1605+
[mockFlutterView layoutIfNeeded];
1606+
1607+
XCTAssertNotNil(childClippingView.maskView);
1608+
}
1609+
15491610
- (void)testClipRect {
15501611
flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
15511612
auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
6816DB9E231750ED00A51400 /* GoldenPlatformViewTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 6816DB9D231750ED00A51400 /* GoldenPlatformViewTests.m */; };
3838
6816DBA12317573300A51400 /* GoldenImage.m in Sources */ = {isa = PBXBuildFile; fileRef = 6816DBA02317573300A51400 /* GoldenImage.m */; };
3939
6816DBA42318358200A51400 /* GoldenTestManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 6816DBA32318358200A51400 /* GoldenTestManager.m */; };
40+
685B9F392977B73100B45442 /* golden_platform_view_large_cliprrect_with_transform_iPhone 8_16.0_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 685B9F372977B73100B45442 /* golden_platform_view_large_cliprrect_with_transform_iPhone 8_16.0_simulator.png */; };
41+
685B9F3A2977B73100B45442 /* golden_platform_view_large_cliprrect_iPhone 8_16.0_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 685B9F382977B73100B45442 /* golden_platform_view_large_cliprrect_iPhone 8_16.0_simulator.png */; };
4042
687AF8E9291EBDE0003912C7 /* golden_platform_view_clippath_with_transform_iPhone 8_16.0_simulator.png in Resources */ = {isa = PBXBuildFile; fileRef = 687AF8E8291EBDE0003912C7 /* golden_platform_view_clippath_with_transform_iPhone 8_16.0_simulator.png */; };
4143
68A5B63423EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */; };
4244
68D4017D2564859300ECD91A /* ContinuousTexture.m in Sources */ = {isa = PBXBuildFile; fileRef = 68D4017C2564859300ECD91A /* ContinuousTexture.m */; };
@@ -151,6 +153,8 @@
151153
6816DBA02317573300A51400 /* GoldenImage.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = GoldenImage.m; sourceTree = "<group>"; };
152154
6816DBA22318358200A51400 /* GoldenTestManager.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GoldenTestManager.h; sourceTree = "<group>"; };
153155
6816DBA32318358200A51400 /* GoldenTestManager.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = GoldenTestManager.m; sourceTree = "<group>"; };
156+
685B9F372977B73100B45442 /* golden_platform_view_large_cliprrect_with_transform_iPhone 8_16.0_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_large_cliprrect_with_transform_iPhone 8_16.0_simulator.png"; sourceTree = "<group>"; };
157+
685B9F382977B73100B45442 /* golden_platform_view_large_cliprrect_iPhone 8_16.0_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_large_cliprrect_iPhone 8_16.0_simulator.png"; sourceTree = "<group>"; };
154158
687AF8E8291EBDE0003912C7 /* golden_platform_view_clippath_with_transform_iPhone 8_16.0_simulator.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "golden_platform_view_clippath_with_transform_iPhone 8_16.0_simulator.png"; sourceTree = "<group>"; };
155159
68A5B63323EB71D300BDBCDB /* PlatformViewGestureRecognizerTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = PlatformViewGestureRecognizerTests.m; sourceTree = "<group>"; };
156160
68D4017B2564859300ECD91A /* ContinuousTexture.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ContinuousTexture.h; sourceTree = "<group>"; };
@@ -296,6 +300,8 @@
296300
F7B464DC2759D02B00079189 /* Goldens */ = {
297301
isa = PBXGroup;
298302
children = (
303+
685B9F382977B73100B45442 /* golden_platform_view_large_cliprrect_iPhone 8_16.0_simulator.png */,
304+
685B9F372977B73100B45442 /* golden_platform_view_large_cliprrect_with_transform_iPhone 8_16.0_simulator.png */,
299305
68D50041291ED8CD001ACFE1 /* golden_platform_view_cliprect_with_transform_iPhone 8_16.0_simulator.png */,
300306
68D5003D291ED645001ACFE1 /* golden_platform_view_cliprrect_with_transform_iPhone 8_16.0_simulator.png */,
301307
687AF8E8291EBDE0003912C7 /* golden_platform_view_clippath_with_transform_iPhone 8_16.0_simulator.png */,
@@ -452,10 +458,12 @@
452458
F7B464F32759D0A900079189 /* golden_platform_view_multiple_background_foreground_iPhone 8_16.0_simulator.png in Resources */,
453459
F7B464F72759D0A900079189 /* golden_platform_view_rotate_iPhone 8_16.0_simulator.png in Resources */,
454460
F7B464ED2759D0A900079189 /* golden_platform_view_cliprrect_iPhone 8_16.0_simulator.png in Resources */,
461+
685B9F3A2977B73100B45442 /* golden_platform_view_large_cliprrect_iPhone 8_16.0_simulator.png in Resources */,
455462
68D5003F291ED645001ACFE1 /* golden_platform_view_cliprrect_with_transform_iPhone 8_16.0_simulator.png in Resources */,
456463
F7B464EB2759D0A900079189 /* golden_two_platform_views_with_other_backdrop_filter_iPhone 8_16.0_simulator.png in Resources */,
457464
F7B464F42759D0A900079189 /* golden_platform_view_with_other_backdrop_filter_iPhone 8_16.0_simulator.png in Resources */,
458465
687AF8E9291EBDE0003912C7 /* golden_platform_view_clippath_with_transform_iPhone 8_16.0_simulator.png in Resources */,
466+
685B9F392977B73100B45442 /* golden_platform_view_large_cliprrect_with_transform_iPhone 8_16.0_simulator.png in Resources */,
459467
F769EB53276312BB007AC10F /* golden_platform_view_cliprect_iPhone 8_16.0_simulator.png in Resources */,
460468
F7B464EF2759D0A900079189 /* golden_platform_view_multiple_iPhone 8_16.0_simulator.png in Resources */,
461469
);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,11 @@ - (BOOL)application:(UIApplication*)application
4343
@"platform_view_multiple_background_foreground",
4444
@"--platform-view-cliprect" : @"platform_view_cliprect",
4545
@"--platform-view-cliprrect" : @"platform_view_cliprrect",
46+
@"--platform-view-large-cliprrect" : @"platform_view_large_cliprrect",
4647
@"--platform-view-clippath" : @"platform_view_clippath",
4748
@"--platform-view-cliprrect-with-transform" : @"platform_view_cliprrect_with_transform",
49+
@"--platform-view-large-cliprrect-with-transform" :
50+
@"platform_view_large_cliprrect_with_transform",
4851
@"--platform-view-cliprect-with-transform" : @"platform_view_cliprect_with_transform",
4952
@"--platform-view-clippath-with-transform" : @"platform_view_clippath_with_transform",
5053
@"--platform-view-transform" : @"platform_view_transform",

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,11 @@ - (instancetype)initWithLaunchArg:(NSString*)launchArg {
2929
@"platform_view_multiple_background_foreground",
3030
@"--platform-view-cliprect" : @"platform_view_cliprect",
3131
@"--platform-view-cliprrect" : @"platform_view_cliprrect",
32+
@"--platform-view-large-cliprrect" : @"platform_view_large_cliprrect",
3233
@"--platform-view-clippath" : @"platform_view_clippath",
3334
@"--platform-view-cliprrect-with-transform" : @"platform_view_cliprrect_with_transform",
35+
@"--platform-view-large-cliprrect-with-transform" :
36+
@"platform_view_large_cliprrect_with_transform",
3437
@"--platform-view-cliprect-with-transform" : @"platform_view_cliprect_with_transform",
3538
@"--platform-view-clippath-with-transform" : @"platform_view_clippath_with_transform",
3639
@"--platform-view-transform" : @"platform_view_transform",

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,24 @@ - (void)testPlatformView {
116116

117117
@end
118118

119+
@interface PlatformViewMutationLargeClipRRectTests : GoldenPlatformViewTests
120+
121+
@end
122+
123+
@implementation PlatformViewMutationLargeClipRRectTests
124+
125+
- (instancetype)initWithInvocation:(NSInvocation*)invocation {
126+
GoldenTestManager* manager =
127+
[[GoldenTestManager alloc] initWithLaunchArg:@"--platform-view-large-cliprrect"];
128+
return [super initWithManager:manager invocation:invocation];
129+
}
130+
131+
- (void)testPlatformView {
132+
[self checkPlatformViewGolden];
133+
}
134+
135+
@end
136+
119137
@interface PlatformViewMutationClipPathTests : GoldenPlatformViewTests
120138

121139
@end
@@ -170,6 +188,24 @@ - (void)testPlatformView {
170188

171189
@end
172190

191+
@interface PlatformViewMutationLargeClipRRectWithTransformTests : GoldenPlatformViewTests
192+
193+
@end
194+
195+
@implementation PlatformViewMutationLargeClipRRectWithTransformTests
196+
197+
- (instancetype)initWithInvocation:(NSInvocation*)invocation {
198+
GoldenTestManager* manager = [[GoldenTestManager alloc]
199+
initWithLaunchArg:@"--platform-view-large-cliprrect-with-transform"];
200+
return [super initWithManager:manager invocation:invocation];
201+
}
202+
203+
- (void)testPlatformView {
204+
[self checkPlatformViewGolden];
205+
}
206+
207+
@end
208+
173209
@interface PlatformViewMutationClipPathWithTransformTests : GoldenPlatformViewTests
174210

175211
@end

0 commit comments

Comments
 (0)