[rotation_distortion]delayed swap to reduce rotation distortion

Author: hellohuanlin

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

@@ -63,6 +63,11 @@ @interface FlutterViewController () <FlutterBinaryMessenger,
 @property(nonatomic, assign) BOOL isHomeIndicatorHidden;
 @property(nonatomic, assign) BOOL isPresentingViewControllerAnimating;
 
+/**
+ * Whether we should ignore viewport metrics updates during rotation transition.
+ */
+@property(nonatomic, assign) BOOL shouldIgnoreViewportMetricsUpdatesDuringRotation;
+
 /**
  * Keyboard animation properties
  */
@@ -843,6 +848,35 @@ - (void)viewDidDisappear:(BOOL)animated {
   [super viewDidDisappear:animated];
 }
 
+- (void)viewWillTransitionToSize:(CGSize)size
+       withTransitionCoordinator:(id<UIViewControllerTransitionCoordinator>)coordinator {
+  [super viewWillTransitionToSize:size withTransitionCoordinator:coordinator];
+
+  // We delay the viewport metrics update for half of rotation transition duration, to address
+  // a bug with distorted aspect ratio.
+  // See: https://github.com/flutter/flutter/issues/16322
+  //
+  // This approach does not fully resolve all distortion problem. But instead, it reduces the
+  // rotation distortion roughly from 4x to 2x. The most distorted frames occur in the middle
+  // of the transition when it is rotating the fastest, making it hard to notice.
+
+  NSTimeInterval transitionDuration = coordinator.transitionDuration;
+  // Do not delay viewport metrics update if zero transition duration.
+  if (transitionDuration == 0) {
+    return;
+  }
+
+  _shouldIgnoreViewportMetricsUpdatesDuringRotation = YES;
+  dispatch_after(dispatch_time(DISPATCH_TIME_NOW,
+                               static_cast<int64_t>(transitionDuration / 2.0 * NSEC_PER_SEC)),
+                 dispatch_get_main_queue(), ^{
+                   // `viewWillTransitionToSize` is only called after the previous rotation is
+                   // complete. So there won't be race condition for this flag.
+                   _shouldIgnoreViewportMetricsUpdatesDuringRotation = NO;
+                   [self updateViewportMetricsIfNeeded];
+                 });
+}
+
 - (void)flushOngoingTouches {
   if (_engine && _ongoingTouches.get().count > 0) {
     auto packet = std::make_unique<flutter::PointerDataPacket>(_ongoingTouches.get().count);
@@ -1278,7 +1312,10 @@ - (void)pencilInteractionDidTap:(UIPencilInteraction*)interaction API_AVAILABLE(
 
 #pragma mark - Handle view resizing
 
-- (void)updateViewportMetrics {
+- (void)updateViewportMetricsIfNeeded {
+  if (_shouldIgnoreViewportMetricsUpdatesDuringRotation) {
+    return;
+  }
   if ([_engine.get() viewController] == self) {
     [_engine.get() updateViewportMetrics:_viewportMetrics];
   }
@@ -1295,11 +1332,9 @@ - (void)viewDidLayoutSubviews {
   // First time since creation that the dimensions of its view is known.
   bool firstViewBoundsUpdate = !_viewportMetrics.physical_width;
   _viewportMetrics.device_pixel_ratio = scale;
-  _viewportMetrics.physical_width = viewBounds.size.width * scale;
-  _viewportMetrics.physical_height = viewBounds.size.height * scale;
-
-  [self updateViewportPadding];
-  [self updateViewportMetrics];
+  [self setViewportMetricsSize];
+  [self setViewportMetricsPaddings];
+  [self updateViewportMetricsIfNeeded];
 
   // There is no guarantee that UIKit will layout subviews when the application is active. Creating
   // the surface when inactive will cause GPU accesses from the background. Only wait for the first
@@ -1328,15 +1363,27 @@ - (void)viewDidLayoutSubviews {
 }
 
 - (void)viewSafeAreaInsetsDidChange {
-  [self updateViewportPadding];
-  [self updateViewportMetrics];
+  [self setViewportMetricsPaddings];
+  [self updateViewportMetricsIfNeeded];
   [super viewSafeAreaInsetsDidChange];
 }
 
-// Updates _viewportMetrics physical padding.
+// Set _viewportMetrics physical size.
+- (void)setViewportMetricsSize {
+  // TODO(hellohuanlin): Use [self mainScreenIfViewLoaded] instead of [UIScreen mainScreen].
+  // This requires adding the view to window during unit tests, which calls multiple engine calls
+  // that is hard to mock since they take/return structs. An alternative approach is to partial mock
+  // the FlutterViewController to make view controller life cycle methods no-op, and insert
+  // this mock into the responder chain.
+  CGFloat scale = [UIScreen mainScreen].scale;
+  _viewportMetrics.physical_width = self.view.bounds.size.width * scale;
+  _viewportMetrics.physical_height = self.view.bounds.size.height * scale;
+}
+
+// Set _viewportMetrics physical paddings.
 //
-// Viewport padding represents the iOS safe area insets.
-- (void)updateViewportPadding {
+// Viewport paddings represent the iOS safe area insets.
+- (void)setViewportMetricsPaddings {
   CGFloat scale = [UIScreen mainScreen].scale;
   _viewportMetrics.physical_padding_top = self.view.safeAreaInsets.top * scale;
   _viewportMetrics.physical_padding_left = self.view.safeAreaInsets.left * scale;
@@ -1661,15 +1708,15 @@ - (void)setupKeyboardAnimationVsyncClient {
         flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom =
             flutterViewController.get()
                 .keyboardAnimationView.layer.presentationLayer.frame.origin.y;
-        [flutterViewController updateViewportMetrics];
+        [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 updateViewportMetrics];
+      [flutterViewController updateViewportMetricsIfNeeded];
     }
   };
   flutter::Shell& shell = [_engine.get() shell];
@@ -1698,7 +1745,7 @@ - (void)ensureViewportMetricsIsCorrect {
   if (_viewportMetrics.physical_view_inset_bottom != self.targetViewInsetBottom) {
     // Make sure the `physical_view_inset_bottom` is the target value.
     _viewportMetrics.physical_view_inset_bottom = self.targetViewInsetBottom;
-    [self updateViewportMetrics];
+    [self updateViewportMetricsIfNeeded];
   }
 }
 

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

@@ -126,7 +126,7 @@ - (void)handlePressEvent:(FlutterUIPressProxy*)press
               nextAction:(void (^)())next API_AVAILABLE(ios(13.4));
 - (void)discreteScrollEvent:(UIPanGestureRecognizer*)recognizer;
 - (flutter::PointerData)createAuxillaryStylusActionData;
-- (void)updateViewportMetrics;
+- (void)updateViewportMetricsIfNeeded;
 - (void)onUserSettingsChanged:(NSNotification*)notification;
 - (void)applicationWillTerminate:(NSNotification*)notification;
 - (void)goToApplicationLifecycle:(nonnull NSString*)state;
@@ -836,7 +836,7 @@ - (void)testViewDidDisappearDoesPauseEngineWhenIsTheViewController {
   OCMReject([lifecycleChannel sendMessage:@"AppLifecycleState.inactive"]);
 }
 
-- (void)testUpdateViewportMetricsDoesntInvokeEngineWhenNotTheViewController {
+- (void)testUpdateViewportMetricsIfNeeded_DoesntInvokeEngineWhenNotTheViewController {
   FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
   [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
   FlutterViewController* viewControllerA = [[FlutterViewController alloc] initWithEngine:mockEngine
@@ -847,12 +847,12 @@ - (void)testUpdateViewportMetricsDoesntInvokeEngineWhenNotTheViewController {
                                                                                  nibName:nil
                                                                                   bundle:nil];
   mockEngine.viewController = viewControllerB;
-  [viewControllerA updateViewportMetrics];
+  [viewControllerA updateViewportMetricsIfNeeded];
   flutter::ViewportMetrics viewportMetrics;
   OCMVerify(never(), [mockEngine updateViewportMetrics:viewportMetrics]);
 }
 
-- (void)testUpdateViewportMetricsDoesInvokeEngineWhenIsTheViewController {
+- (void)testUpdateViewportMetricsIfNeeded_DoesInvokeEngineWhenIsTheViewController {
   FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
   [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
   FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
@@ -861,7 +861,79 @@ - (void)testUpdateViewportMetricsDoesInvokeEngineWhenIsTheViewController {
   mockEngine.viewController = viewController;
   flutter::ViewportMetrics viewportMetrics;
   OCMExpect([mockEngine updateViewportMetrics:viewportMetrics]).ignoringNonObjectArgs();
-  [viewController updateViewportMetrics];
+  [viewController updateViewportMetricsIfNeeded];
+  OCMVerifyAll(mockEngine);
+}
+
+- (void)testUpdateViewportMetricsIfNeeded_DoesNotInvokeEngineWhenShouldBeIgnoredDuringRotation {
+  FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
+  [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
+  mockEngine.viewController = viewController;
+
+  id mockCoordinator = OCMProtocolMock(@protocol(UIViewControllerTransitionCoordinator));
+  OCMStub([mockCoordinator transitionDuration]).andReturn(0.5);
+
+  // Mimic the device rotation.
+  [viewController viewWillTransitionToSize:CGSizeZero withTransitionCoordinator:mockCoordinator];
+  // Should not trigger the engine call when during rotation.
+  [viewController updateViewportMetricsIfNeeded];
+
+  OCMVerify(never(), [mockEngine updateViewportMetrics:flutter::ViewportMetrics()]);
+}
+
+- (void)testViewWillTransitionToSize_DoesDelayEngineCallIfNonZeroDuration {
+  FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
+  [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
+  mockEngine.viewController = viewController;
+
+  // Mimic the device rotation with non-zero transition duration.
+  NSTimeInterval transitionDuration = 0.5;
+  id mockCoordinator = OCMProtocolMock(@protocol(UIViewControllerTransitionCoordinator));
+  OCMStub([mockCoordinator transitionDuration]).andReturn(transitionDuration);
+
+  flutter::ViewportMetrics viewportMetrics;
+  OCMExpect([mockEngine updateViewportMetrics:viewportMetrics]).ignoringNonObjectArgs();
+
+  [viewController viewWillTransitionToSize:CGSizeZero withTransitionCoordinator:mockCoordinator];
+  // Should not immediately call the engine (this request should be ignored).
+  [viewController updateViewportMetricsIfNeeded];
+  OCMVerify(never(), [mockEngine updateViewportMetrics:flutter::ViewportMetrics()]);
+
+  // Should delay the engine call for half of the transition duration.
+  // Wait for additional transitionDuration to allow updateViewportMetrics calls if any.
+  XCTWaiterResult result = [XCTWaiter
+      waitForExpectations:@[ [self expectationWithDescription:@"Waiting for rotation duration"] ]
+                  timeout:transitionDuration];
+  XCTAssertEqual(result, XCTWaiterResultTimedOut);
+
+  OCMVerifyAll(mockEngine);
+}
+
+- (void)testViewWillTransitionToSize_DoesNotDelayEngineCallIfZeroDuration {
+  FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
+  [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
+                                                                                nibName:nil
+                                                                                 bundle:nil];
+  mockEngine.viewController = viewController;
+
+  // Mimic the device rotation with zero transition duration.
+  id mockCoordinator = OCMProtocolMock(@protocol(UIViewControllerTransitionCoordinator));
+  OCMStub([mockCoordinator transitionDuration]).andReturn(0);
+
+  flutter::ViewportMetrics viewportMetrics;
+  OCMExpect([mockEngine updateViewportMetrics:viewportMetrics]).ignoringNonObjectArgs();
+
+  // Should immediately trigger the engine call, without delay.
+  [viewController viewWillTransitionToSize:CGSizeZero withTransitionCoordinator:mockCoordinator];
+  [viewController updateViewportMetricsIfNeeded];
+
   OCMVerifyAll(mockEngine);
 }