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

Commit 1c1dfda

Browse files
hellohuanlinhellohuanlin
authored
[rotation_distortion] Use "delayed swap" solution to reduce rotation distortion (#40730)
The "size interpolation" solution didn't go well (more context [here](#40412 (comment))). Then a new solution came to my mind, and I call it **"delayed swap"**: In the originally behavior, we swap the width/height immediately before the rotation, resulting in roughly ~4x distortion in the beginning. With "delayed swap" solution, we **swap the width/height right in the middle of the rotation** (i.e. delay the swap for half of the transition duration). This new "delayed swap" solution gives us the same benefit as the "snapshot" solution: - reducing ~4x distortion to ~2x - most distorted frames occur in the middle of rotation when it's moving the fastest, making it hard to notice And it fixes the drawback of "snapshot" solution: - it works well with dynamic content like animation or video - it doesn't have a ~0.5 second penalty when taking the snapshot Looks pretty good on flutter gallery: https://user-images.githubusercontent.com/41930132/228383137-7cd09982-89a9-4c83-bf55-9431de708278.mp4 *List which issues are fixed by this PR. You must list at least one issue.* Fixes flutter/flutter#16322 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 2c6a936 commit 1c1dfda

File tree

2 files changed

+150
-20
lines changed

2 files changed

+150
-20
lines changed

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

Lines changed: 67 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ @interface FlutterViewController () <FlutterBinaryMessenger, UIScrollViewDelegat
6161
@property(nonatomic, assign) BOOL isHomeIndicatorHidden;
6262
@property(nonatomic, assign) BOOL isPresentingViewControllerAnimating;
6363

64+
/**
65+
* Whether we should ignore viewport metrics updates during rotation transition.
66+
*/
67+
@property(nonatomic, assign) BOOL shouldIgnoreViewportMetricsUpdatesDuringRotation;
68+
6469
/**
6570
* Keyboard animation properties
6671
*/
@@ -837,6 +842,35 @@ - (void)viewDidDisappear:(BOOL)animated {
837842
[super viewDidDisappear:animated];
838843
}
839844

845+
- (void)viewWillTransitionToSize:(CGSize)size
846+
withTransitionCoordinator:(id<UIViewControllerTransitionCoordinator>)coordinator {
847+
[super viewWillTransitionToSize:size withTransitionCoordinator:coordinator];
848+
849+
// We delay the viewport metrics update for half of rotation transition duration, to address
850+
// a bug with distorted aspect ratio.
851+
// See: https://github.com/flutter/flutter/issues/16322
852+
//
853+
// This approach does not fully resolve all distortion problem. But instead, it reduces the
854+
// rotation distortion roughly from 4x to 2x. The most distorted frames occur in the middle
855+
// of the transition when it is rotating the fastest, making it hard to notice.
856+
857+
NSTimeInterval transitionDuration = coordinator.transitionDuration;
858+
// Do not delay viewport metrics update if zero transition duration.
859+
if (transitionDuration == 0) {
860+
return;
861+
}
862+
863+
_shouldIgnoreViewportMetricsUpdatesDuringRotation = YES;
864+
dispatch_after(dispatch_time(DISPATCH_TIME_NOW,
865+
static_cast<int64_t>(transitionDuration / 2.0 * NSEC_PER_SEC)),
866+
dispatch_get_main_queue(), ^{
867+
// `viewWillTransitionToSize` is only called after the previous rotation is
868+
// complete. So there won't be race condition for this flag.
869+
_shouldIgnoreViewportMetricsUpdatesDuringRotation = NO;
870+
[self updateViewportMetricsIfNeeded];
871+
});
872+
}
873+
840874
- (void)flushOngoingTouches {
841875
if (_engine && _ongoingTouches.get().count > 0) {
842876
auto packet = std::make_unique<flutter::PointerDataPacket>(_ongoingTouches.get().count);
@@ -1226,7 +1260,10 @@ - (void)invalidateTouchRateCorrectionVSyncClient {
12261260

12271261
#pragma mark - Handle view resizing
12281262

1229-
- (void)updateViewportMetrics {
1263+
- (void)updateViewportMetricsIfNeeded {
1264+
if (_shouldIgnoreViewportMetricsUpdatesDuringRotation) {
1265+
return;
1266+
}
12301267
if ([_engine.get() viewController] == self) {
12311268
[_engine.get() updateViewportMetrics:_viewportMetrics];
12321269
}
@@ -1243,11 +1280,9 @@ - (void)viewDidLayoutSubviews {
12431280
// First time since creation that the dimensions of its view is known.
12441281
bool firstViewBoundsUpdate = !_viewportMetrics.physical_width;
12451282
_viewportMetrics.device_pixel_ratio = scale;
1246-
_viewportMetrics.physical_width = viewBounds.size.width * scale;
1247-
_viewportMetrics.physical_height = viewBounds.size.height * scale;
1248-
1249-
[self updateViewportPadding];
1250-
[self updateViewportMetrics];
1283+
[self setViewportMetricsSize];
1284+
[self setViewportMetricsPaddings];
1285+
[self updateViewportMetricsIfNeeded];
12511286

12521287
// There is no guarantee that UIKit will layout subviews when the application is active. Creating
12531288
// the surface when inactive will cause GPU accesses from the background. Only wait for the first
@@ -1276,16 +1311,33 @@ - (void)viewDidLayoutSubviews {
12761311
}
12771312

12781313
- (void)viewSafeAreaInsetsDidChange {
1279-
[self updateViewportPadding];
1280-
[self updateViewportMetrics];
1314+
[self setViewportMetricsPaddings];
1315+
[self updateViewportMetricsIfNeeded];
12811316
[super viewSafeAreaInsetsDidChange];
12821317
}
12831318

1284-
// Updates _viewportMetrics physical padding.
1319+
// Set _viewportMetrics physical size.
1320+
- (void)setViewportMetricsSize {
1321+
UIScreen* mainScreen = [self mainScreenIfViewLoaded];
1322+
if (!mainScreen) {
1323+
return;
1324+
}
1325+
1326+
CGFloat scale = mainScreen.scale;
1327+
_viewportMetrics.physical_width = self.view.bounds.size.width * scale;
1328+
_viewportMetrics.physical_height = self.view.bounds.size.height * scale;
1329+
}
1330+
1331+
// Set _viewportMetrics physical paddings.
12851332
//
1286-
// Viewport padding represents the iOS safe area insets.
1287-
- (void)updateViewportPadding {
1288-
CGFloat scale = [UIScreen mainScreen].scale;
1333+
// Viewport paddings represent the iOS safe area insets.
1334+
- (void)setViewportMetricsPaddings {
1335+
UIScreen* mainScreen = [self mainScreenIfViewLoaded];
1336+
if (!mainScreen) {
1337+
return;
1338+
}
1339+
1340+
CGFloat scale = mainScreen.scale;
12891341
_viewportMetrics.physical_padding_top = self.view.safeAreaInsets.top * scale;
12901342
_viewportMetrics.physical_padding_left = self.view.safeAreaInsets.left * scale;
12911343
_viewportMetrics.physical_padding_right = self.view.safeAreaInsets.right * scale;
@@ -1609,15 +1661,15 @@ - (void)setupKeyboardAnimationVsyncClient {
16091661
flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom =
16101662
flutterViewController.get()
16111663
.keyboardAnimationView.layer.presentationLayer.frame.origin.y;
1612-
[flutterViewController updateViewportMetrics];
1664+
[flutterViewController updateViewportMetricsIfNeeded];
16131665
}
16141666
} else {
16151667
fml::TimeDelta timeElapsed = recorder.get()->GetVsyncTargetTime() -
16161668
flutterViewController.get().keyboardAnimationStartTime;
16171669

16181670
flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom =
16191671
[[flutterViewController keyboardSpringAnimation] curveFunction:timeElapsed.ToSecondsF()];
1620-
[flutterViewController updateViewportMetrics];
1672+
[flutterViewController updateViewportMetricsIfNeeded];
16211673
}
16221674
};
16231675
flutter::Shell& shell = [_engine.get() shell];
@@ -1646,7 +1698,7 @@ - (void)ensureViewportMetricsIsCorrect {
16461698
if (_viewportMetrics.physical_view_inset_bottom != self.targetViewInsetBottom) {
16471699
// Make sure the `physical_view_inset_bottom` is the target value.
16481700
_viewportMetrics.physical_view_inset_bottom = self.targetViewInsetBottom;
1649-
[self updateViewportMetrics];
1701+
[self updateViewportMetricsIfNeeded];
16501702
}
16511703
}
16521704

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

Lines changed: 83 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ - (void)performOrientationUpdate:(UIInterfaceOrientationMask)new_preferences;
124124
- (void)handlePressEvent:(FlutterUIPressProxy*)press
125125
nextAction:(void (^)())next API_AVAILABLE(ios(13.4));
126126
- (void)discreteScrollEvent:(UIPanGestureRecognizer*)recognizer;
127-
- (void)updateViewportMetrics;
127+
- (void)updateViewportMetricsIfNeeded;
128128
- (void)onUserSettingsChanged:(NSNotification*)notification;
129129
- (void)applicationWillTerminate:(NSNotification*)notification;
130130
- (void)goToApplicationLifecycle:(nonnull NSString*)state;
@@ -834,7 +834,7 @@ - (void)testViewDidDisappearDoesPauseEngineWhenIsTheViewController {
834834
OCMReject([lifecycleChannel sendMessage:@"AppLifecycleState.inactive"]);
835835
}
836836

837-
- (void)testUpdateViewportMetricsDoesntInvokeEngineWhenNotTheViewController {
837+
- (void)testUpdateViewportMetricsIfNeeded_DoesntInvokeEngineWhenNotTheViewController {
838838
FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
839839
[mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
840840
FlutterViewController* viewControllerA = [[FlutterViewController alloc] initWithEngine:mockEngine
@@ -845,12 +845,12 @@ - (void)testUpdateViewportMetricsDoesntInvokeEngineWhenNotTheViewController {
845845
nibName:nil
846846
bundle:nil];
847847
mockEngine.viewController = viewControllerB;
848-
[viewControllerA updateViewportMetrics];
848+
[viewControllerA updateViewportMetricsIfNeeded];
849849
flutter::ViewportMetrics viewportMetrics;
850850
OCMVerify(never(), [mockEngine updateViewportMetrics:viewportMetrics]);
851851
}
852852

853-
- (void)testUpdateViewportMetricsDoesInvokeEngineWhenIsTheViewController {
853+
- (void)testUpdateViewportMetricsIfNeeded_DoesInvokeEngineWhenIsTheViewController {
854854
FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
855855
[mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
856856
FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
@@ -859,7 +859,85 @@ - (void)testUpdateViewportMetricsDoesInvokeEngineWhenIsTheViewController {
859859
mockEngine.viewController = viewController;
860860
flutter::ViewportMetrics viewportMetrics;
861861
OCMExpect([mockEngine updateViewportMetrics:viewportMetrics]).ignoringNonObjectArgs();
862-
[viewController updateViewportMetrics];
862+
[viewController updateViewportMetricsIfNeeded];
863+
OCMVerifyAll(mockEngine);
864+
}
865+
866+
- (void)testUpdateViewportMetricsIfNeeded_DoesNotInvokeEngineWhenShouldBeIgnoredDuringRotation {
867+
FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
868+
[mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
869+
FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
870+
nibName:nil
871+
bundle:nil];
872+
FlutterViewController* viewControllerMock = OCMPartialMock(viewController);
873+
OCMStub([viewControllerMock mainScreenIfViewLoaded]).andReturn(UIScreen.mainScreen);
874+
mockEngine.viewController = viewController;
875+
876+
id mockCoordinator = OCMProtocolMock(@protocol(UIViewControllerTransitionCoordinator));
877+
OCMStub([mockCoordinator transitionDuration]).andReturn(0.5);
878+
879+
// Mimic the device rotation.
880+
[viewController viewWillTransitionToSize:CGSizeZero withTransitionCoordinator:mockCoordinator];
881+
// Should not trigger the engine call when during rotation.
882+
[viewController updateViewportMetricsIfNeeded];
883+
884+
OCMVerify(never(), [mockEngine updateViewportMetrics:flutter::ViewportMetrics()]);
885+
}
886+
887+
- (void)testViewWillTransitionToSize_DoesDelayEngineCallIfNonZeroDuration {
888+
FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
889+
[mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
890+
FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
891+
nibName:nil
892+
bundle:nil];
893+
FlutterViewController* viewControllerMock = OCMPartialMock(viewController);
894+
OCMStub([viewControllerMock mainScreenIfViewLoaded]).andReturn(UIScreen.mainScreen);
895+
mockEngine.viewController = viewController;
896+
897+
// Mimic the device rotation with non-zero transition duration.
898+
NSTimeInterval transitionDuration = 0.5;
899+
id mockCoordinator = OCMProtocolMock(@protocol(UIViewControllerTransitionCoordinator));
900+
OCMStub([mockCoordinator transitionDuration]).andReturn(transitionDuration);
901+
902+
flutter::ViewportMetrics viewportMetrics;
903+
OCMExpect([mockEngine updateViewportMetrics:viewportMetrics]).ignoringNonObjectArgs();
904+
905+
[viewController viewWillTransitionToSize:CGSizeZero withTransitionCoordinator:mockCoordinator];
906+
// Should not immediately call the engine (this request should be ignored).
907+
[viewController updateViewportMetricsIfNeeded];
908+
OCMVerify(never(), [mockEngine updateViewportMetrics:flutter::ViewportMetrics()]);
909+
910+
// Should delay the engine call for half of the transition duration.
911+
// Wait for additional transitionDuration to allow updateViewportMetrics calls if any.
912+
XCTWaiterResult result = [XCTWaiter
913+
waitForExpectations:@[ [self expectationWithDescription:@"Waiting for rotation duration"] ]
914+
timeout:transitionDuration];
915+
XCTAssertEqual(result, XCTWaiterResultTimedOut);
916+
917+
OCMVerifyAll(mockEngine);
918+
}
919+
920+
- (void)testViewWillTransitionToSize_DoesNotDelayEngineCallIfZeroDuration {
921+
FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]);
922+
[mockEngine createShell:@"" libraryURI:@"" initialRoute:nil];
923+
FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine
924+
nibName:nil
925+
bundle:nil];
926+
FlutterViewController* viewControllerMock = OCMPartialMock(viewController);
927+
OCMStub([viewControllerMock mainScreenIfViewLoaded]).andReturn(UIScreen.mainScreen);
928+
mockEngine.viewController = viewController;
929+
930+
// Mimic the device rotation with zero transition duration.
931+
id mockCoordinator = OCMProtocolMock(@protocol(UIViewControllerTransitionCoordinator));
932+
OCMStub([mockCoordinator transitionDuration]).andReturn(0);
933+
934+
flutter::ViewportMetrics viewportMetrics;
935+
OCMExpect([mockEngine updateViewportMetrics:viewportMetrics]).ignoringNonObjectArgs();
936+
937+
// Should immediately trigger the engine call, without delay.
938+
[viewController viewWillTransitionToSize:CGSizeZero withTransitionCoordinator:mockCoordinator];
939+
[viewController updateViewportMetricsIfNeeded];
940+
863941
OCMVerifyAll(mockEngine);
864942
}
865943

0 commit comments

Comments
 (0)