From a2cc7375d78bf302f548e6f305fc287caa70ac9a Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Fri, 2 Feb 2024 14:51:42 -0800 Subject: [PATCH 1/2] Round SkRects in ClipRectContainsPlatformViewBoundingRect --- .../framework/Source/FlutterPlatformViews.mm | 8 +++++++- .../Source/FlutterPlatformViewsTest.mm | 17 ++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 7e8246538231a..aa6129e7e1f9e 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -45,7 +45,13 @@ static bool ClipRectContainsPlatformViewBoundingRect(const SkRect& clip_rect, const SkRect& platformview_boundingrect, const SkMatrix& transform_matrix) { SkRect transformed_rect = transform_matrix.mapRect(clip_rect); - return transformed_rect.contains(platformview_boundingrect); + + // Tolerate floating point errors when comparing flow clipping view bounds + // and quartz platform view bounds. Round to integers. + SkIRect transformed_rect_rounded = transformed_rect.round(); + SkIRect platformview_boundingrect_rounded = platformview_boundingrect.round(); + + return transformed_rect_rounded.contains(platformview_boundingrect_rounded); } // Determines if the `clipRRect` from a clipRRect mutator contains the diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index f8c878f5639a8..18d880575e5e5 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -1507,6 +1507,17 @@ - (void)testChildClippingViewShouldBeTheBoundingRectOfPlatformView { SkMatrix finalMatrix; finalMatrix.setConcat(screenScaleMatrix, rotateMatrix); + // The childclippingview's frame is set based on flow, but the platform view's frame is set based + // on quartz. Although they should be the same, we should tolerate small floating point + // errors. + // Push clip rects the same size as the platform view but faking a floating point error. + SkRect clipRectSmaller = + SkRect::MakeXYWH(0, 0, 300 - kFloatCompareEpsilon, 300 - kFloatCompareEpsilon); + stack.PushClipRect(clipRectSmaller); + SkRect clipRectLarger = + SkRect::MakeXYWH(0, 0, 300 + kFloatCompareEpsilon, 300 + kFloatCompareEpsilon); + stack.PushClipRect(clipRectLarger); + auto embeddedViewParams = std::make_unique(finalMatrix, SkSize::Make(300, 300), stack); @@ -1516,9 +1527,7 @@ - (void)testChildClippingViewShouldBeTheBoundingRectOfPlatformView { toView:mockFlutterView]; XCTAssertTrue([gMockPlatformView.superview.superview isKindOfClass:ChildClippingView.class]); ChildClippingView* childClippingView = (ChildClippingView*)gMockPlatformView.superview.superview; - // The childclippingview's frame is set based on flow, but the platform view's frame is set based - // on quartz. Although they should be the same, but we should tolerate small floating point - // errors. + // See above comment about tolerating small floating point errors. XCTAssertLessThan(fabs(platformViewRectInFlutterView.origin.x - childClippingView.frame.origin.x), kFloatCompareEpsilon); XCTAssertLessThan(fabs(platformViewRectInFlutterView.origin.y - childClippingView.frame.origin.y), @@ -1529,6 +1538,8 @@ - (void)testChildClippingViewShouldBeTheBoundingRectOfPlatformView { XCTAssertLessThan( fabs(platformViewRectInFlutterView.size.height - childClippingView.frame.size.height), kFloatCompareEpsilon); + + XCTAssertNil(childClippingView.maskView); } - (void)testClipsDoNotInterceptWithPlatformViewShouldNotAddMaskView { From dc07ab649dca6bdb5cd5b2e67f588b9f47000d4d Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Fri, 16 Feb 2024 13:40:35 -0800 Subject: [PATCH 2/2] roundOut --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 7 ++----- .../IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj | 4 ---- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index aa6129e7e1f9e..5b21aeaac34ac 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -47,11 +47,8 @@ static bool ClipRectContainsPlatformViewBoundingRect(const SkRect& clip_rect, SkRect transformed_rect = transform_matrix.mapRect(clip_rect); // Tolerate floating point errors when comparing flow clipping view bounds - // and quartz platform view bounds. Round to integers. - SkIRect transformed_rect_rounded = transformed_rect.round(); - SkIRect platformview_boundingrect_rounded = platformview_boundingrect.round(); - - return transformed_rect_rounded.contains(platformview_boundingrect_rounded); + // and quartz platform view bounds. Round platform view to integers. + return transformed_rect.roundOut().contains(platformview_boundingrect); } // Determines if the `clipRRect` from a clipRRect mutator contains the diff --git a/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj b/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj index e67b60cdfcbc3..0ff04e310ac72 100644 --- a/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj +++ b/testing/ios/IosUnitTests/IosUnitTests.xcodeproj/project.pbxproj @@ -50,7 +50,6 @@ 0AC232F724BA71D300A85907 /* FlutterEngineTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterEngineTest.mm; sourceTree = ""; }; 0AC2330324BA71D300A85907 /* accessibility_bridge_test.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = accessibility_bridge_test.mm; sourceTree = ""; }; 0AC2330B24BA71D300A85907 /* FlutterTextInputPluginTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterTextInputPluginTest.mm; sourceTree = ""; }; - 0AC2330F24BA71D300A85907 /* FlutterBinaryMessengerRelayTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterBinaryMessengerRelayTest.mm; sourceTree = ""; }; 0AC2331024BA71D300A85907 /* connection_collection_test.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = connection_collection_test.mm; sourceTree = ""; }; 0AC2331224BA71D300A85907 /* FlutterEnginePlatformViewTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterEnginePlatformViewTest.mm; sourceTree = ""; }; 0AC2331924BA71D300A85907 /* FlutterPluginAppLifeCycleDelegateTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterPluginAppLifeCycleDelegateTest.mm; sourceTree = ""; }; @@ -72,7 +71,6 @@ 3DD7D38C27D2B81000DA365C /* FlutterUndoManagerPluginTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterUndoManagerPluginTest.mm; sourceTree = ""; }; 689EC1E2281B30D3008FEB58 /* FlutterSpellCheckPluginTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterSpellCheckPluginTest.mm; sourceTree = ""; }; 68B6091227F62F990036AC78 /* VsyncWaiterIosTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = VsyncWaiterIosTest.mm; sourceTree = ""; }; - 73F12C22288F92FF00AFC3A6 /* FlutterViewControllerTest_mrc.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterViewControllerTest_mrc.mm; sourceTree = ""; }; D2D361A52B234EAC0018964E /* FlutterMetalLayerTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = FlutterMetalLayerTest.mm; sourceTree = ""; }; F7521D7226BB671E005F15C5 /* libios_test_flutter.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libios_test_flutter.dylib; path = "../../../../out/$(FLUTTER_ENGINE)/libios_test_flutter.dylib"; sourceTree = ""; }; F7521D7526BB673E005F15C5 /* libocmock_shared.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libocmock_shared.dylib; path = "../../../../out/$(FLUTTER_ENGINE)/libocmock_shared.dylib"; sourceTree = ""; }; @@ -109,12 +107,10 @@ 0AC232F724BA71D300A85907 /* FlutterEngineTest.mm */, 0AC2330324BA71D300A85907 /* accessibility_bridge_test.mm */, 0AC2330B24BA71D300A85907 /* FlutterTextInputPluginTest.mm */, - 0AC2330F24BA71D300A85907 /* FlutterBinaryMessengerRelayTest.mm */, 0AC2331024BA71D300A85907 /* connection_collection_test.mm */, 0AC2331224BA71D300A85907 /* FlutterEnginePlatformViewTest.mm */, 0AC2331924BA71D300A85907 /* FlutterPluginAppLifeCycleDelegateTest.mm */, 0AC2332124BA71D300A85907 /* FlutterViewControllerTest.mm */, - 73F12C22288F92FF00AFC3A6 /* FlutterViewControllerTest_mrc.mm */, D2D361A52B234EAC0018964E /* FlutterMetalLayerTest.mm */, ); name = Source;