From 56020e75701ee6b79dd3c72a1e134f86e204b4c7 Mon Sep 17 00:00:00 2001 From: luckysmg <2539699336@qq.com> Date: Fri, 7 Jul 2023 10:50:54 +0800 Subject: [PATCH 1/3] Revert "Revert "[iOS][Keyboard] Wait vsync on UI thread and update viewport inset to avoid jitter." (#43422)" This reverts commit 3ade212082fa48e9b32f7b6008ef0c9a2a7c5c67. --- .../ios/framework/Source/FlutterEngine.mm | 7 +- .../framework/Source/FlutterEngine_Internal.h | 3 +- .../framework/Source/FlutterViewController.mm | 115 +++++++++++------- .../Source/FlutterViewControllerTest.mm | 58 +++++---- .../Source/FlutterViewControllerTest_mrc.mm | 31 ++++- .../Source/FlutterViewController_Internal.h | 2 + 6 files changed, 149 insertions(+), 67 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 85e1f7d4d5e8e..52e6af116b079 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -333,7 +333,12 @@ - (void)dispatchPointerDataPacket:(std::unique_ptr)p return _shell->GetTaskRunners().GetPlatformTaskRunner(); } -- (fml::RefPtr)RasterTaskRunner { +- (fml::RefPtr)uiTaskRunner { + FML_DCHECK(_shell); + return _shell->GetTaskRunners().GetUITaskRunner(); +} + +- (fml::RefPtr)rasterTaskRunner { FML_DCHECK(_shell); return _shell->GetTaskRunners().GetRasterTaskRunner(); } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h index 6fe25e406af6f..2d476e93ebc3c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h @@ -39,7 +39,8 @@ extern NSString* const kFlutterEngineWillDealloc; - (void)dispatchPointerDataPacket:(std::unique_ptr)packet; - (fml::RefPtr)platformTaskRunner; -- (fml::RefPtr)RasterTaskRunner; +- (fml::RefPtr)uiTaskRunner; +- (fml::RefPtr)rasterTaskRunner; - (fml::WeakPtr)platformView; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 5f565b7cf0739..2828ea16814ce 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -69,11 +69,11 @@ @interface FlutterViewController () SetNextFrameCallback([weakSelf = [self getWeakPtr], platformTaskRunner = [_engine.get() platformTaskRunner], - RasterTaskRunner = [_engine.get() RasterTaskRunner]]() { - FML_DCHECK(RasterTaskRunner->RunsTasksOnCurrentThread()); + rasterTaskRunner = [_engine.get() rasterTaskRunner]]() { + FML_DCHECK(rasterTaskRunner->RunsTasksOnCurrentThread()); // Get callback on raster thread and jump back to platform thread. platformTaskRunner->PostTask([weakSelf]() { if (weakSelf) { @@ -1605,7 +1605,55 @@ - (void)startKeyBoardAnimation:(NSTimeInterval)duration { // Invalidate old vsync client if old animation is not completed. [self invalidateKeyboardAnimationVSyncClient]; - [self setupKeyboardAnimationVsyncClient]; + + fml::WeakPtr weakSelf = [self getWeakPtr]; + FlutterKeyboardAnimationCallback keyboardAnimationCallback = ^( + fml::TimePoint keyboardAnimationTargetTime) { + if (!weakSelf) { + return; + } + fml::scoped_nsobject flutterViewController( + [(FlutterViewController*)weakSelf.get() retain]); + if (!flutterViewController) { + return; + } + + // If the view controller's view is not loaded, bail out. + if (!flutterViewController.get().isViewLoaded) { + return; + } + // If the view for tracking keyboard animation is nil, means it is not + // created, bail out. + if ([flutterViewController keyboardAnimationView] == nil) { + return; + } + // If keyboardAnimationVSyncClient is nil, means the animation ends. + // And should bail out. + if (flutterViewController.get().keyboardAnimationVSyncClient == nil) { + return; + } + + if ([flutterViewController keyboardAnimationView].superview == nil) { + // Ensure the keyboardAnimationView is in view hierarchy when animation running. + [flutterViewController.get().view addSubview:[flutterViewController keyboardAnimationView]]; + } + + if ([flutterViewController keyboardSpringAnimation] == nil) { + if (flutterViewController.get().keyboardAnimationView.layer.presentationLayer) { + flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = + flutterViewController.get() + .keyboardAnimationView.layer.presentationLayer.frame.origin.y; + [flutterViewController updateViewportMetricsIfNeeded]; + } + } else { + fml::TimeDelta timeElapsed = + keyboardAnimationTargetTime - flutterViewController.get().keyboardAnimationStartTime; + flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = + [[flutterViewController keyboardSpringAnimation] curveFunction:timeElapsed.ToSecondsF()]; + [flutterViewController updateViewportMetricsIfNeeded]; + } + }; + [self setupKeyboardAnimationVsyncClient:keyboardAnimationCallback]; VSyncClient* currentVsyncClient = _keyboardAnimationVSyncClient; [UIView animateWithDuration:duration @@ -1648,45 +1696,28 @@ - (void)setupKeyboardSpringAnimationIfNeeded:(CAAnimation*)keyboardAnimation { toValue:self.targetViewInsetBottom]); } -- (void)setupKeyboardAnimationVsyncClient { - auto callback = [weakSelf = - [self getWeakPtr]](std::unique_ptr recorder) { - if (!weakSelf) { - return; - } - fml::scoped_nsobject flutterViewController( - [(FlutterViewController*)weakSelf.get() retain]); - if (!flutterViewController) { - return; - } - - if ([flutterViewController keyboardAnimationView].superview == nil) { - // Ensure the keyboardAnimationView is in view hierarchy when animation running. - [flutterViewController.get().view addSubview:[flutterViewController keyboardAnimationView]]; - } - - if ([flutterViewController keyboardSpringAnimation] == nil) { - if (flutterViewController.get().keyboardAnimationView.layer.presentationLayer) { - flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = - flutterViewController.get() - .keyboardAnimationView.layer.presentationLayer.frame.origin.y; - [flutterViewController updateViewportMetricsIfNeeded]; - } - } else { - fml::TimeDelta timeElapsed = recorder.get()->GetVsyncTargetTime() - - flutterViewController.get().keyboardAnimationStartTime; - - flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = - [[flutterViewController keyboardSpringAnimation] curveFunction:timeElapsed.ToSecondsF()]; - [flutterViewController updateViewportMetricsIfNeeded]; - } - }; - flutter::Shell& shell = [_engine.get() shell]; +- (void)setupKeyboardAnimationVsyncClient: + (FlutterKeyboardAnimationCallback)keyboardAnimationCallback { + if (!keyboardAnimationCallback) { + return; + } NSAssert(_keyboardAnimationVSyncClient == nil, @"_keyboardAnimationVSyncClient must be nil when setup"); - _keyboardAnimationVSyncClient = - [[VSyncClient alloc] initWithTaskRunner:shell.GetTaskRunners().GetPlatformTaskRunner() - callback:callback]; + + // Make sure the new viewport metrics get sent after the begin frame event has processed. + fml::scoped_nsprotocol animationCallback( + [keyboardAnimationCallback copy]); + auto uiCallback = [animationCallback, + engine = _engine](std::unique_ptr recorder) { + fml::TimeDelta frameInterval = recorder->GetVsyncTargetTime() - recorder->GetVsyncStartTime(); + fml::TimePoint keyboardAnimationTargetTime = recorder->GetVsyncTargetTime() + frameInterval; + [engine platformTaskRunner]->PostTask([animationCallback, keyboardAnimationTargetTime] { + animationCallback.get()(keyboardAnimationTargetTime); + }); + }; + + _keyboardAnimationVSyncClient = [[VSyncClient alloc] initWithTaskRunner:[_engine uiTaskRunner] + callback:uiCallback]; _keyboardAnimationVSyncClient.allowPauseAfterVsync = NO; [_keyboardAnimationVSyncClient await]; } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm index f2b8d15ee4d87..54bca88f57e0f 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm @@ -26,6 +26,7 @@ - (FlutterTextInputPlugin*)textInputPlugin; - (void)sendKeyEvent:(const FlutterKeyEvent&)event callback:(nullable FlutterKeyEventCallback)callback userData:(nullable void*)userData; +- (fml::RefPtr)uiTaskRunner; @end /// Sometimes we have to use a custom mock to avoid retain cycles in OCMock. @@ -135,10 +136,11 @@ - (BOOL)shouldIgnoreKeyboardNotification:(NSNotification*)notification; - (FlutterKeyboardMode)calculateKeyboardAttachMode:(NSNotification*)notification; - (CGFloat)calculateMultitaskingAdjustment:(CGRect)screenRect keyboardFrame:(CGRect)keyboardFrame; - (void)startKeyBoardAnimation:(NSTimeInterval)duration; -- (void)setupKeyboardAnimationVsyncClient; - (UIView*)keyboardAnimationView; - (SpringAnimation*)keyboardSpringAnimation; - (void)setupKeyboardSpringAnimationIfNeeded:(CAAnimation*)keyboardAnimation; +- (void)setupKeyboardAnimationVsyncClient: + (FlutterKeyboardAnimationCallback)keyboardAnimationCallback; - (void)ensureViewportMetricsIsCorrect; - (void)invalidateKeyboardAnimationVSyncClient; - (void)addInternalPlugins; @@ -197,18 +199,6 @@ - (void)testViewDidLoadWillInvokeCreateTouchRateCorrectionVSyncClient { OCMVerify([viewControllerMock createTouchRateCorrectionVSyncClientIfNeeded]); } -- (void)testStartKeyboardAnimationWillInvokeSetupKeyboardAnimationVsyncClient { - FlutterEngine* engine = [[FlutterEngine alloc] init]; - [engine runWithEntrypoint:nil]; - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine - nibName:nil - bundle:nil]; - FlutterViewController* viewControllerMock = OCMPartialMock(viewController); - viewControllerMock.targetViewInsetBottom = 100; - [viewControllerMock startKeyBoardAnimation:0.25]; - OCMVerify([viewControllerMock setupKeyboardAnimationVsyncClient]); -} - - (void)testStartKeyboardAnimationWillInvokeSetupKeyboardSpringAnimationIfNeeded { FlutterEngine* engine = [[FlutterEngine alloc] init]; [engine runWithEntrypoint:nil]; @@ -451,6 +441,34 @@ - (void)testShouldIgnoreKeyboardNotification { } } +- (void)testKeyboardAnimationWillWaitUIThreadVsync { + // We need to make sure the new viewport metrics get sent after the + // begin frame event has processed. And this test is to expect that the callback + // will sync with UI thread. So just simulate a lot of works on UI thread and + // test the keyboard animation callback will execute until UI task completed. + // Related issue: https://github.com/flutter/flutter/issues/120555. + + FlutterEngine* engine = [[FlutterEngine alloc] init]; + [engine runWithEntrypoint:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + // Post a task to UI thread to block the thread. + const int delayTime = 1; + [engine uiTaskRunner]->PostTask([] { sleep(delayTime); }); + XCTestExpectation* expectation = [self expectationWithDescription:@"keyboard animation callback"]; + + __block CFTimeInterval fulfillTime; + FlutterKeyboardAnimationCallback callback = ^(fml::TimePoint targetTime) { + fulfillTime = CACurrentMediaTime(); + [expectation fulfill]; + }; + CFTimeInterval startTime = CACurrentMediaTime(); + [viewController setupKeyboardAnimationVsyncClient:callback]; + [self waitForExpectationsWithTimeout:5.0 handler:nil]; + XCTAssertTrue(fulfillTime - startTime > delayTime); +} + - (void)testCalculateKeyboardAttachMode { FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]); [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil]; @@ -629,9 +647,9 @@ - (void)testCalculateKeyboardInset { } - (void)testHandleKeyboardNotification { - FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]); - [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil]; - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine + FlutterEngine* engine = [[FlutterEngine alloc] init]; + [engine runWithEntrypoint:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; // keyboard is empty @@ -652,11 +670,9 @@ - (void)testHandleKeyboardNotification { [self setupMockMainScreenAndView:viewControllerMock viewFrame:viewFrame convertedFrame:viewFrame]; viewControllerMock.targetViewInsetBottom = 0; XCTestExpectation* expectation = [self expectationWithDescription:@"update viewport"]; - OCMStub([mockEngine updateViewportMetrics:flutter::ViewportMetrics()]) - .ignoringNonObjectArgs() - .andDo(^(NSInvocation* invocation) { - [expectation fulfill]; - }); + OCMStub([viewControllerMock updateViewportMetricsIfNeeded]).andDo(^(NSInvocation* invocation) { + [expectation fulfill]; + }); [viewControllerMock handleKeyboardNotification:notification]; XCTAssertTrue(viewControllerMock.targetViewInsetBottom == 320 * UIScreen.mainScreen.scale); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm index e18b0b55c9335..c19486f0a9be1 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm @@ -7,6 +7,7 @@ #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" #import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h" +#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h" #import "flutter/shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.h" FLUTTER_ASSERT_NOT_ARC @@ -31,7 +32,9 @@ @interface FlutterViewController (Testing) @property(nonatomic, retain) VSyncClient* touchRateCorrectionVSyncClient; - (void)createTouchRateCorrectionVSyncClientIfNeeded; -- (void)setupKeyboardAnimationVsyncClient; +- (void)setupKeyboardAnimationVsyncClient: + (FlutterKeyboardAnimationCallback)keyboardAnimationCallback; +- (void)startKeyBoardAnimation:(NSTimeInterval)duration; - (void)triggerTouchRateCorrectionIfNeeded:(NSSet*)touches; @end @@ -53,7 +56,9 @@ - (void)testSetupKeyboardAnimationVsyncClientWillCreateNewVsyncClientForFlutterV FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - [viewController setupKeyboardAnimationVsyncClient]; + FlutterKeyboardAnimationCallback callback = ^(fml::TimePoint targetTime) { + }; + [viewController setupKeyboardAnimationVsyncClient:callback]; XCTAssertNotNil(viewController.keyboardAnimationVSyncClient); CADisplayLink* link = [viewController.keyboardAnimationVSyncClient getDisplayLink]; XCTAssertNotNil(link); @@ -173,4 +178,26 @@ - (void)testTriggerTouchRateCorrectionVSyncClientCorrectly { XCTAssertFalse(link.isPaused); } +- (void)testFlutterViewControllerStartKeyboardAnimationWillCreateVsyncClientCorrectly { + FlutterEngine* engine = [[FlutterEngine alloc] init]; + [engine runWithEntrypoint:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + viewController.targetViewInsetBottom = 100; + [viewController startKeyBoardAnimation:0.25]; + XCTAssertNotNil(viewController.keyboardAnimationVSyncClient); +} + +- (void) + testSetupKeyboardAnimationVsyncClientWillNotCreateNewVsyncClientWhenKeyboardAnimationCallbackIsNil { + FlutterEngine* engine = [[FlutterEngine alloc] init]; + [engine runWithEntrypoint:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + [viewController setupKeyboardAnimationVsyncClient:nil]; + XCTAssertNil(viewController.keyboardAnimationVSyncClient); +} + @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h index 052cf3323b30c..09cb7c455446d 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h @@ -33,6 +33,8 @@ typedef NS_ENUM(NSInteger, FlutterKeyboardMode) { FlutterKeyboardModeFloating = 2, }; +typedef void (^FlutterKeyboardAnimationCallback)(fml::TimePoint); + @interface FlutterViewController () @property(class, nonatomic, readonly) BOOL accessibilityIsOnOffSwitchLabelsEnabled; From f515bf70ac847d4a636c25877433a5d1751a8afd Mon Sep 17 00:00:00 2001 From: luckysmg <2539699336@qq.com> Date: Fri, 7 Jul 2023 16:20:12 +0800 Subject: [PATCH 2/3] ++ --- .../ios/framework/Source/FlutterViewController.mm | 5 ++--- .../ios/framework/Source/FlutterViewControllerTest.mm | 11 +++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 2828ea16814ce..38d5f66c3c335 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -1707,11 +1707,10 @@ - (void)setupKeyboardAnimationVsyncClient: // Make sure the new viewport metrics get sent after the begin frame event has processed. fml::scoped_nsprotocol animationCallback( [keyboardAnimationCallback copy]); - auto uiCallback = [animationCallback, - engine = _engine](std::unique_ptr recorder) { + auto uiCallback = [animationCallback](std::unique_ptr recorder) { fml::TimeDelta frameInterval = recorder->GetVsyncTargetTime() - recorder->GetVsyncStartTime(); fml::TimePoint keyboardAnimationTargetTime = recorder->GetVsyncTargetTime() + frameInterval; - [engine platformTaskRunner]->PostTask([animationCallback, keyboardAnimationTargetTime] { + dispatch_async(dispatch_get_main_queue(), ^(void) { animationCallback.get()(keyboardAnimationTargetTime); }); }; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm index 54bca88f57e0f..274180f1b9a32 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm @@ -187,6 +187,17 @@ - (id)setupMockMainScreenAndView:(FlutterViewController*)viewControllerMock return mockView; } +- (void)testKeyboardAnimationWillNotCrashWhenEngineDestroyed { + FlutterEngine* engine = [[FlutterEngine alloc] init]; + [engine runWithEntrypoint:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + [viewController setupKeyboardAnimationVsyncClient:^(fml::TimePoint){ + }]; + [engine destroyContext]; +} + - (void)testViewDidLoadWillInvokeCreateTouchRateCorrectionVSyncClient { FlutterEngine* engine = [[FlutterEngine alloc] init]; [engine runWithEntrypoint:nil]; From dd6cd30da4c1f122319246832b72b290d9f6e4a0 Mon Sep 17 00:00:00 2001 From: luckysmg <2539699336@qq.com> Date: Fri, 7 Jul 2023 16:22:18 +0800 Subject: [PATCH 3/3] ++ --- .../Source/FlutterViewControllerTest.mm | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm index 274180f1b9a32..b3c5f078e6772 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm @@ -187,17 +187,6 @@ - (id)setupMockMainScreenAndView:(FlutterViewController*)viewControllerMock return mockView; } -- (void)testKeyboardAnimationWillNotCrashWhenEngineDestroyed { - FlutterEngine* engine = [[FlutterEngine alloc] init]; - [engine runWithEntrypoint:nil]; - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine - nibName:nil - bundle:nil]; - [viewController setupKeyboardAnimationVsyncClient:^(fml::TimePoint){ - }]; - [engine destroyContext]; -} - - (void)testViewDidLoadWillInvokeCreateTouchRateCorrectionVSyncClient { FlutterEngine* engine = [[FlutterEngine alloc] init]; [engine runWithEntrypoint:nil]; @@ -451,6 +440,16 @@ - (void)testShouldIgnoreKeyboardNotification { XCTAssertTrue(shouldIgnore == YES); } } +- (void)testKeyboardAnimationWillNotCrashWhenEngineDestroyed { + FlutterEngine* engine = [[FlutterEngine alloc] init]; + [engine runWithEntrypoint:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + [viewController setupKeyboardAnimationVsyncClient:^(fml::TimePoint){ + }]; + [engine destroyContext]; +} - (void)testKeyboardAnimationWillWaitUIThreadVsync { // We need to make sure the new viewport metrics get sent after the