only clip when necessary

tests

fix tests

format

review

fix

Author: Chris Yang

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

@@ -32,6 +32,22 @@ - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
 }
 @end
 
+// Determines if the final `clipBounds` from a clipRect/clipRRect/clipPath mutator contains the
+// `platformview_boundingrect`.
+//
+// `clip_bounds` is the bounding rect of the rect/rrect/path in the clipRect/clipRRect/clipPath
+// mutator. This rect is in its own coordinate space. The rect needs to be transformed by
+// `transform_matrix` to be in the coordinate space where the PlatformView is displayed.
+//
+// `platformview_boundingrect` is the final bounding rect of the PlatformView in the coordinate
+// space where the PlatformView is displayed.
+static bool ClipBoundsContainsPlatformViewBoundingRect(const SkRect& clip_bounds,
+                                                       const SkRect& platformview_boundingrect,
+                                                       const SkMatrix& transform_matrix) {
+  SkRect transforme_clip_bounds = transform_matrix.mapRect(clip_bounds);
+  return transforme_clip_bounds.contains(platformview_boundingrect);
+}
+
 namespace flutter {
 // Becomes NO if Apple's API changes and blurred backdrop filters cannot be applied.
 BOOL canApplyBlurBackdrop = YES;
@@ -404,47 +420,62 @@ - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
 }
 
 void FlutterPlatformViewsController::ApplyMutators(const MutatorsStack& mutators_stack,
-                                                   UIView* embedded_view) {
+                                                   UIView* embedded_view,
+                                                   const SkRect& bounding_rect) {
   if (flutter_view_ == nullptr) {
     return;
   }
   FML_DCHECK(CATransform3DEqualToTransform(embedded_view.layer.transform, CATransform3DIdentity));
   ResetAnchor(embedded_view.layer);
   ChildClippingView* clipView = (ChildClippingView*)embedded_view.superview;
 
-  // The UIKit frame is set based on the logical resolution 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. And until this point, we did all the calculation based on the flow
-  // resolution. So we need to scale down to match UIKit's logical resolution.
   CGFloat screenScale = [UIScreen mainScreen].scale;
-  CATransform3D finalTransform = CATransform3DMakeScale(1 / screenScale, 1 / screenScale, 1);
 
   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))] autorelease];
+                               CGRectGetHeight(flutter_view.bounds))
+        screenScale:screenScale] autorelease];
 
+  SkMatrix transformMatrix;
   NSMutableArray* blurFilters = [[[NSMutableArray alloc] init] autorelease];
 
+  clipView.maskView = nil;
   auto iter = mutators_stack.Begin();
   while (iter != mutators_stack.End()) {
     switch ((*iter)->GetType()) {
       case kTransform: {
-        CATransform3D transform = GetCATransform3DFromSkMatrix((*iter)->GetMatrix());
-        finalTransform = CATransform3DConcat(transform, finalTransform);
+        transformMatrix.preConcat((*iter)->GetMatrix());
         break;
       }
-      case kClipRect:
-        [maskView clipRect:(*iter)->GetRect() matrix:finalTransform];
+      case kClipRect: {
+        if (ClipBoundsContainsPlatformViewBoundingRect((*iter)->GetRect(), bounding_rect,
+                                                       transformMatrix)) {
+          break;
+        }
+        [maskView clipRect:(*iter)->GetRect() matrix:transformMatrix];
+        clipView.maskView = maskView;
         break;
-      case kClipRRect:
-        [maskView clipRRect:(*iter)->GetRRect() matrix:finalTransform];
+      }
+      case kClipRRect: {
+        if (ClipBoundsContainsPlatformViewBoundingRect((*iter)->GetRRect().getBounds(),
+                                                       bounding_rect, transformMatrix)) {
+          break;
+        }
+        [maskView clipRRect:(*iter)->GetRRect() matrix:transformMatrix];
+        clipView.maskView = maskView;
         break;
-      case kClipPath:
-        [maskView clipPath:(*iter)->GetPath() matrix:finalTransform];
+      }
+      case kClipPath: {
+        if (ClipBoundsContainsPlatformViewBoundingRect((*iter)->GetPath().getBounds(),
+                                                       bounding_rect, transformMatrix)) {
+          break;
+        }
+        [maskView clipPath:(*iter)->GetPath() matrix:transformMatrix];
+        clipView.maskView = maskView;
         break;
+      }
       case kOpacity:
         embedded_view.alpha = (*iter)->GetAlphaFloat() * embedded_view.alpha;
         break;
@@ -489,17 +520,23 @@ - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
     [clipView applyBlurBackdropFilters:blurFilters];
   }
 
+  // The UIKit frame is set based on the logical resolution 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. And until this point, we did all the calculation based on the flow
+  // resolution. So we need to scale down to match UIKit's logical resolution.
+  transformMatrix.postScale(1 / screenScale, 1 / screenScale);
+
   // Reverse the offset of the clipView.
   // The clipView's frame includes the final translate of the final transform matrix.
   // 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
   // the mask view, whose origin is always (0,0) to the flutter_view.
-  CATransform3D reverseTranslate =
-      CATransform3DMakeTranslation(-clipView.frame.origin.x, -clipView.frame.origin.y, 0);
-  embedded_view.layer.transform = CATransform3DConcat(finalTransform, reverseTranslate);
-  clipView.maskView = maskView;
+  transformMatrix.postTranslate(-clipView.frame.origin.x, -clipView.frame.origin.y);
+
+  embedded_view.layer.transform = flutter::GetCATransform3DFromSkMatrix(transformMatrix);
 }
 
 void FlutterPlatformViewsController::CompositeWithParams(int view_id,
@@ -538,7 +575,7 @@ - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
   CGFloat screenScale = [UIScreen mainScreen].scale;
   clippingView.frame = CGRectMake(rect.x() / screenScale, rect.y() / screenScale,
                                   rect.width() / screenScale, rect.height() / screenScale);
-  ApplyMutators(mutatorStack, touchInterceptor);
+  ApplyMutators(mutatorStack, touchInterceptor, rect);
 }
 
 EmbedderPaintContext FlutterPlatformViewsController::CompositeEmbeddedView(int view_id) {

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

@@ -1478,6 +1478,74 @@ - (void)testChildClippingViewShouldBeTheBoundingRectOfPlatformView {
       kFloatCompareEpsilon);
 }
 
+- (void)testClipsDoNotInterceptWithPlatformViewShouldNotAddMaskView {
+  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" : @2, @"viewType" : @"MockFlutterPlatformView"}],
+      result);
+
+  XCTAssertNotNil(gMockPlatformView);
+
+  UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 30, 30)] autorelease];
+  flutterPlatformViewsController->SetFlutterView(mockFlutterView);
+  // Create embedded view params
+  flutter::MutatorsStack stack;
+  // Layer tree always pushes a screen scale factor to the stack
+  SkMatrix screenScaleMatrix =
+      SkMatrix::Scale([UIScreen mainScreen].scale, [UIScreen mainScreen].scale);
+  stack.PushTransform(screenScaleMatrix);
+  SkMatrix translateMatrix = SkMatrix::Translate(5, 5);
+  // The platform view's rect for this test will be (5, 5, 10, 10)
+  stack.PushTransform(translateMatrix);
+  // Push a clip rect, big enough to contain the entire platform view bound
+  SkRect rect = SkRect::MakeXYWH(0, 0, 25, 25);
+  stack.PushClipRect(rect);
+  // Push a clip rrect, big enough to contain the entire platform view bound
+  SkRect rect_for_rrect = SkRect::MakeXYWH(0, 0, 24, 24);
+  SkRRect rrect = SkRRect::MakeRectXY(rect_for_rrect, 1, 1);
+  stack.PushClipRRect(rrect);
+  // Push a clip path, big enough to contain the entire platform view bound
+  SkPath path = SkPath::RRect(SkRect::MakeXYWH(0, 0, 23, 23), 1, 1);
+  stack.PushClipPath(path);
+
+  auto embeddedViewParams = std::make_unique<flutter::EmbeddedViewParams>(
+      SkMatrix::Concat(screenScaleMatrix, translateMatrix), SkSize::Make(5, 5), stack);
+
+  flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams));
+  flutterPlatformViewsController->CompositeEmbeddedView(2);
+  gMockPlatformView.backgroundColor = UIColor.redColor;
+  XCTAssertTrue([gMockPlatformView.superview.superview isKindOfClass:ChildClippingView.class]);
+  ChildClippingView* childClippingView = (ChildClippingView*)gMockPlatformView.superview.superview;
+  [mockFlutterView addSubview:childClippingView];
+
+  [mockFlutterView setNeedsLayout];
+  [mockFlutterView layoutIfNeeded];
+
+  XCTAssertNil(childClippingView.maskView);
+}
+
 - (void)testClipRect {
   flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
   auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");

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

@@ -28,20 +28,22 @@
 // is replaced with the alpha channel of the |FlutterClippingMaskView|.
 @interface FlutterClippingMaskView : UIView
 
+- (instancetype)initWithFrame:(CGRect)frame screenScale:(CGFloat)screenScale;
+
 // Adds a clip rect operation to the queue.
 //
 // The `clipSkRect` is transformed with the `matrix` before adding to the queue.
-- (void)clipRect:(const SkRect&)clipSkRect matrix:(const CATransform3D&)matrix;
+- (void)clipRect:(const SkRect&)clipSkRect matrix:(const SkMatrix&)matrix;
 
 // Adds a clip rrect operation to the queue.
 //
 // The `clipSkRRect` is transformed with the `matrix` before adding to the queue.
-- (void)clipRRect:(const SkRRect&)clipSkRRect matrix:(const CATransform3D&)matrix;
+- (void)clipRRect:(const SkRRect&)clipSkRRect matrix:(const SkMatrix&)matrix;
 
 // Adds a clip path operation to the queue.
 //
 // The `path` is transformed with the `matrix` before adding to the queue.
-- (void)clipPath:(const SkPath&)path matrix:(const CATransform3D&)matrix;
+- (void)clipPath:(const SkPath&)path matrix:(const SkMatrix&)matrix;
 
 @end
 
@@ -280,7 +282,9 @@ class FlutterPlatformViewsController {
   // T_1 is applied to C_2, T_3 and T_4 are applied to C_5, and T_6 is applied to PLATFORM_VIEW.
   //
   // After each clip operation, we update the head to the super view of the current head.
-  void ApplyMutators(const MutatorsStack& mutators_stack, UIView* embedded_view);
+  void ApplyMutators(const MutatorsStack& mutators_stack,
+                     UIView* embedded_view,
+                     const SkRect& bounding_rect);
   void CompositeWithParams(int view_id, const EmbeddedViewParams& params);
 
   // Allocates a new FlutterPlatformViewLayer if needed, draws the pixels within the rect from

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

@@ -241,6 +241,8 @@ - (NSMutableArray*)backdropFilterSubviews {
 
 @interface FlutterClippingMaskView ()
 
+@property(nonatomic) CATransform3D reverseScreenScale;
+
 - (fml::CFRef<CGPathRef>)getTransformedPath:(CGPathRef)path matrix:(CATransform3D)matrix;
 
 @end
@@ -250,8 +252,13 @@ @implementation FlutterClippingMaskView {
 }
 
 - (instancetype)initWithFrame:(CGRect)frame {
+  return [self initWithFrame:frame screenScale:[UIScreen mainScreen].scale];
+}
+
+- (instancetype)initWithFrame:(CGRect)frame screenScale:(CGFloat)screenScale {
   if (self = [super initWithFrame:frame]) {
     self.backgroundColor = UIColor.clearColor;
+    _reverseScreenScale = CATransform3DMakeScale(1 / screenScale, 1 / screenScale, 1);
   }
   return self;
 }
@@ -280,13 +287,16 @@ - (void)drawRect:(CGRect)rect {
   CGContextRestoreGState(context);
 }
 
-- (void)clipRect:(const SkRect&)clipSkRect matrix:(const CATransform3D&)matrix {
+- (void)clipRect:(const SkRect&)clipSkRect matrix:(const SkMatrix&)matrix {
   CGRect clipRect = flutter::GetCGRectFromSkRect(clipSkRect);
   CGPathRef path = CGPathCreateWithRect(clipRect, nil);
-  paths_.push_back([self getTransformedPath:path matrix:matrix]);
+  // The `matrix` is based on the physical pixels, convert it to UIKit points.
+  CATransform3D reverseScaledMatrix =
+      CATransform3DConcat(flutter::GetCATransform3DFromSkMatrix(matrix), _reverseScreenScale);
+  paths_.push_back([self getTransformedPath:path matrix:reverseScaledMatrix]);
 }
 
-- (void)clipRRect:(const SkRRect&)clipSkRRect matrix:(const CATransform3D&)matrix {
+- (void)clipRRect:(const SkRRect&)clipSkRRect matrix:(const SkMatrix&)matrix {
   CGPathRef pathRef = nullptr;
   switch (clipSkRRect.getType()) {
     case SkRRect::kEmpty_Type: {
@@ -346,13 +356,16 @@ - (void)clipRRect:(const SkRRect&)clipSkRRect matrix:(const CATransform3D&)matri
       break;
     }
   }
+  // The `matrix` is based on the physical pixels, convert it to UIKit points.
+  CATransform3D reverseScaledMatrix =
+      CATransform3DConcat(flutter::GetCATransform3DFromSkMatrix(matrix), _reverseScreenScale);
   // 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:matrix]);
+  paths_.push_back([self getTransformedPath:pathRef matrix:reverseScaledMatrix]);
 }
 
-- (void)clipPath:(const SkPath&)path matrix:(const CATransform3D&)matrix {
+- (void)clipPath:(const SkPath&)path matrix:(const SkMatrix&)matrix {
   if (!path.isValid()) {
     return;
   }
@@ -411,7 +424,10 @@ - (void)clipPath:(const SkPath&)path matrix:(const CATransform3D&)matrix {
     }
     verb = iter.next(pts);
   }
-  paths_.push_back([self getTransformedPath:pathRef matrix:matrix]);
+  // The `matrix` is based on the physical pixels, convert it to UIKit points.
+  CATransform3D reverseScaledMatrix =
+      CATransform3DConcat(flutter::GetCATransform3DFromSkMatrix(matrix), _reverseScreenScale);
+  paths_.push_back([self getTransformedPath:pathRef matrix:reverseScaledMatrix]);
 }
 
 - (fml::CFRef<CGPathRef>)getTransformedPath:(CGPathRef)path matrix:(CATransform3D)matrix {