Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rotation on iOS does not look right #16322

Closed
DaveShuckerow opened this issue Apr 6, 2018 · 54 comments · Fixed by flutter/engine#40730
Closed

Rotation on iOS does not look right #16322

DaveShuckerow opened this issue Apr 6, 2018 · 54 comments · Fixed by flutter/engine#40730
Assignees
Labels
a: fidelity Matching the OEM platforms better a: layout SystemChrome and Framework's Layout Issues customer: amplify customer: crowd Affects or could affect many people, though not necessarily a specific customer. customer: google Various Google teams customer: splashbulb engine flutter/engine repository. See also e: labels. found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P1 High-priority issues at the top of the work list platform-ios iOS applications specifically r: fixed Issue is closed as already fixed in a newer version

Comments

@DaveShuckerow
Copy link
Contributor

DaveShuckerow commented Apr 6, 2018

Internal: b/77275812

From an internal user trying out Flutter on the external flow for iOS:

I'm trying out flutter for the first time to gauge the quality of the 'native look and feel' to see if it's a platform I should invest my time in learning.

Using the example hellowworld app from https://flutter.io/get-started/test-drive/#terminal

See video. I've slowed it down to show more clearly.

The layout flashes, the text stretches.

@DaveShuckerow DaveShuckerow added platform-ios iOS applications specifically a: first hour The first hour of using Flutter labels Apr 6, 2018
@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2018

bff

@tvolkert tvolkert added customer: splashbulb engine flutter/engine repository. See also e: labels. a: fidelity Matching the OEM platforms better and removed a: first hour The first hour of using Flutter labels Apr 6, 2018
@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2018

Cross-posting from #13300 and consolidating on this issue:

Rotate iOS device running flutter from portrait to landscape and back. Today we use the default behaviour, which results in the view repainting contents for the new orientation before the rotation starts, then transforming the view (rotation, scale) to the new orientation.

Alternatives include Safari's rotation behaviour or allowing some degree of developer control.

@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2018

/cc @chickenblood

@tvolkert tvolkert added this to the 4: Next milestone milestone Apr 6, 2018
@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2018

the stretching of the aspect ratio is what looks worst here. We could potentially also cross-fade from a fixed rotated background so that we don't incur the black edges.

@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2018

/cc @cbracken as the creator of #13300

@tvolkert
Copy link
Contributor

Per @cbracken in #6995

Looks like we need to expose better support for viewWillTransitionToSize:withTransitionCoordinator: in FlutterViewController.

@tvolkert tvolkert added this to Scenario Backlog in iOS Framework Oct 23, 2018
@tvolkert tvolkert moved this from Scenario Backlog to Structural Backlog in iOS Framework Oct 23, 2018
@xster xster moved this from Structural Backlog to Easy fix in iOS Framework Apr 9, 2019
@adriancmurray
Copy link

@tvolkert

Alternatives include ... allowing some degree of developer control.

I would love some control here. Specifically it would be nice to allow for the device to rotate without utilizing SystemChrome.setPreferredOrientations but not forcing the main widget to repaint/size. I could see instead passing a callback to the WidgetsApp with an orientation change. From a developer's perspective this callback could be passed to something like a ChangeNotifierProvider and allow us to have very granule control of this process. Similar to how the Camera app functions on iOS where the main view doesn't actually switch between states but instead the icons rotate to their appropriate view. This skewing is really really annoying from a UX point of view.

@tvolkert
Copy link
Contributor

/cc @goderbauer

@goderbauer goderbauer added the framework flutter/packages/flutter repository. See also f: labels. label Sep 12, 2019
@dannyvalentesonos
Copy link
Contributor

@dnfield Any update on this? We are running into this as well, and on iPads it looks pretty bad.
Thanks.

@dnfield
Copy link
Contributor

dnfield commented Nov 27, 2019

I'm not aware of anyone working on this at the moment - it might take a little bit between holidays coming up and Flutter Interact, but it is being tracked. @xster may know more.

@chickenblood
Copy link
Contributor

The way this rotation is implemented on iOS, is that it generates the final frame by applying an animated size change (not transform) to the entire window content (the root view controller's view), so the layout changes dynamically, while applying a 90-degree rotation transform at the same time. At the end of the rotation, the applied transform and layout match what is should look like in landscape.

I'm not familiar with the Flutter codebase yet, but would be willing to take a look. Would what I describe or something like it be possible with Flutter's layout engine?

@dnfield
Copy link
Contributor

dnfield commented Dec 2, 2019

We want to implement -viewWillTransitionToSize:withTransitionCordinator: in FlutterViewController

With that, we should be able to pipe the relevant information down to the framework so it can know to run an animation for rotation and when that animation should end. One thing unclear to me is how long that animation should be, and whether we'll be able to communicate with the framework quickly enough to actually get the animation going in time.

Another option would be to just try to do it all in FlutterViewController.mm and basically just do a size animation on whatever our last frame was unti lwe finish rotating.

@adriancmurray
Copy link

@dnfield in the mean time, is there a known method to hide the transition during the 200 (or so) milliseconds it takes to rotate? I made an attempt by extending WidgetsFlutterBinding, made my own runApp method to pass my own WidgetsBinding, overrode handleMetricsChanged to create a ChangeNotifier that would cover the app with a color and fade out quickly after 200ms (just a rough guess). While it was much prettier, it still wasn't fast enough to cover the first frames of the wretched skew. Am I correct in assuming that without making modifications to the engine that I won't be able to overcome the skew?

@dnfield
Copy link
Contributor

dnfield commented Dec 3, 2019

I believe it will require engine modifications to be able to start drawing on time - not 100% sure though. @chinmaygarde may know about that?

@SoolyChristy

This comment was marked as duplicate.

@SamerCat

This comment was marked as duplicate.

@snoopdoggy322

This comment was marked as duplicate.

@danagbemava-nc
Copy link
Member

Reproducible on the latest versions of flutter

recording
Screen.Recording.2023-01-03.at.10.12.36.mov
updated sample
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

void main() {
  SystemChrome.setPreferredOrientations([
    DeviceOrientation.portraitUp,
    DeviceOrientation.landscapeLeft,
    DeviceOrientation.landscapeRight,
  ]);
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return const Scaffold(
      body: Center(
        child: Text('hello world'),
      ),
    );
  }
}
flutter doctor -v
[✓] Flutter (Channel stable, 3.3.10, on macOS 13.1 22C65 darwin-arm, locale en-GB)
    • Flutter version 3.3.10 on channel stable at /Users/nexus/dev/sdks/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 135454af32 (3 weeks ago), 2022-12-15 07:36:55 -0800
    • Engine revision 3316dd8728
    • Dart version 2.18.6
    • DevTools version 2.15.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14B47b
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] VS Code (version 1.74.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.56.0

[✓] Connected device (3 available)
    • iPhone 14 Pro (mobile) • 4F72110C-F38B-4CF9-93C4-4D6042148D28 • ios            • com.apple.CoreSimulator.SimRuntime.iOS-16-1 (simulator)
    • macOS (desktop)        • macos                                • darwin-arm64   • macOS 13.1 22C65 darwin-arm
    • Chrome (web)           • chrome                               • web-javascript • Google Chrome 108.0.5359.124
    ! Device adb-5dd3be00-22GXB2._adb-tls-connect._tcp. is offline.

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!
[!] Flutter (Channel master, 3.7.0-13.0.pre.138, on macOS 13.1 22C65 darwin-arm64, locale en-GB)
    • Flutter version 3.7.0-13.0.pre.138 on channel master at /Users/nexus/dev/sdks/flutters
    ! Warning: `flutter` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/nexus/dev/sdks/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/nexus/dev/sdks/flutters. Consider adding /Users/nexus/dev/sdks/flutters/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision b938dc13df (23 hours ago), 2023-01-02 05:47:23 -0500
    • Engine revision 472e34cbbc
    • Dart version 3.0.0 (build 3.0.0-76.0.dev)
    • DevTools version 2.20.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/nexus/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14B47b
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[✓] VS Code (version 1.74.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.56.0

[✓] Connected device (3 available)
    • iPhone 14 Pro (mobile) • 4F72110C-F38B-4CF9-93C4-4D6042148D28 • ios            • com.apple.CoreSimulator.SimRuntime.iOS-16-1 (simulator)
    • macOS (desktop)        • macos                                • darwin-arm64   • macOS 13.1 22C65 darwin-arm64
    • Chrome (web)           • chrome                               • web-javascript • Google Chrome 108.0.5359.124
    ! Device adb-5dd3be00-22GXB2._adb-tls-connect._tcp. is offline.

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

@danagbemava-nc danagbemava-nc added found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 and removed found in release: 1.22 Found to occur in 1.22 labels Jan 3, 2023
@nikuz

This comment was marked as duplicate.

@jmagman
Copy link
Member

jmagman commented Feb 6, 2023

Also internally tracked with b/266184881.

@hellohuanlin
Copy link
Contributor

hellohuanlin commented Apr 3, 2023

This is a summary of our research so far, in case it can be helpful for a potential better solution in the future. There are 4 approaches discussed below:

Approach 1. Snapshot trick

This was initially attempted by Chinmay. On device, we add a snapshot during the rotation, and then fade it out after the rotation is complete. This approach simply moves the distortion from the beginning of the rotation to the end of the rotation, so the same amount of distortion is still there, just moved to the end (video here).

Approach 2. Improved snapshot trick

We improved this approach by swapping snapshot in the middle of the rotation (rather than at the end of rotation). The benefits are:
a. it actually reduces the max distortion roughly from ~4x to ~2x
b. the swapping (or "jumping effect") happens in the middle, when it's rotating the fastest, making it harder to notice

However, there are still 2 drawbacks:
a. it doesn't work well with dynamic content (animation or video)
b. it takes about 0.5 second to take the snapshot

Slight pause with dynamic content:

2.snapshot.video.mp4

Approach 3. Size interpolation

The original idea was from Dan's comment. Basically the engine sends interpolated sizes during the rotation. (PR here)

This approach worked pretty well for simple UI - there are almost no distortions at all during the rotation. However, on complex UI (e.g. flutter gallery), we experienced significant frame drops. Reducing the number of interpolations helps with frame drops, but it also resulted in a very un-smooth transition.

Works well with simple UI:

3.size.interpolation.mp4

Frame drop on complex UI:

interpolation.perf.issue.mp4

Approach 4. Delayed swap

This is actually the simplest approach - just delay the swap of width and height until the middle of rotation, rather than at the beginning of the rotation. (PR here)

This approach gives us the same benefit as approach 2, without the 2 drawbacks discussed above. Note that there is still a swapping (or "jumping effect") in the middle of rotation, but it's significantly less noticeable than the existing behavior, which "swaps/jumps" at the beginning of rotation, with larger amount of distortion.

Delayed swap on complex UI:

delayed.swap.mp4

So in conclusion, with the pros and cons discussed above, we will go with approach 4.

@b3nni97
Copy link

b3nni97 commented Apr 5, 2023

@hellohuanlin Couldn't we take a screenshot at the start of the rotation where height and width have already changed and then fade it in?
That's what native iOS does anyway, so the jump in the middle is no longer really noticeable.

@hellohuanlin
Copy link
Contributor

@b3nni97

Couldn't we take a screenshot at the start of the rotation where height and width have already changed and then fade it in?

When you take a screenshot where height and width already changed, the screenshot itself has the max distortion. So the result won't look good.

That's what native iOS does anyway, so the jump in the middle is no longer really noticeable.

Nope. The native iOS actually does size interpolation.

@b3nni97
Copy link

b3nni97 commented Apr 5, 2023

@hellohuanlin
Ok maybe I don't understand something correctly but,

in a native iOS app, it doesn't look to me like the size is interpolated.

IMG_1427 (1)

This screenshot was taken at the beginning of a screen rotation. You can see 2 "screenshots" one of the state before the rotation, and one of the state after the rotation, with the exception that the end state didn't just change width / height, but the height remains (so that the complete height can be filled during the fade in).
During the animation, the width of the clipped part becomes smaller and smaller, but the lower part is clipped.

IMG_1428 (1)
This screenshot was taken shortly before the rotation is finished. Here you can see that the end state is now almost completely visible.

I hope you understand what I mean.

Here is the full video of the rotation

RPReplay_Final1680721048.MP4

@hellohuanlin
Copy link
Contributor

hellohuanlin commented Apr 5, 2023

@b3nni97 I don't think apple is taking a screenshot (otherwise there would be distortion of the screenshot when the window's aspect ratio changes).

It's impossible to take a screenshot of what the screen will look like after the rotation is complete, at the time before the rotation even starts (if that's what you want) - the screenshot shows exactly what the current screen is when the screenshot is taken.

Your particular example is interesting - it changes the entire layout to a split view, in which case I'm not sure what apple is doing under the hood. Without layout change, apple's behavior is size interpolation, as shown below:

swiftui.mp4

@b3nni97
Copy link

b3nni97 commented Apr 5, 2023

@hellohuanlin
Ok then this problem is more complex than thought.

It's impossible to take a screenshot of what the screen will look like after the rotation is complete, at the time before the rotation even starts (if that's what you want) - the screenshot shows exactly what the current screen is when the screenshot is taken.

Hmm, I know too little about the engine, I thought it would be possible to do a complete rebuild with the new width and height, and then display this as a "screenshot".

@hellohuanlin
Copy link
Contributor

Hmm, I know too little about the engine, I thought it would be possible to do a complete rebuild with the new width and height, and then display this as a "screenshot".

The screenshot is not flutter-related tho. It's just UIKit API to take a screenshot of the current screen. So once the end state is displayed, you can't "blend it in", as it's already on the screen.

@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2023

I think @b3nni97 may be suggesting we could build a new tree with the new width & height and paint it to a buffer rather than the screen.

@hellohuanlin
Copy link
Contributor

hellohuanlin commented Apr 6, 2023

@tvolkert Gotcha. That likely requires big changes in both framework and engine. However, a new idea came to me that fading in end-state snapshot should have similar result to fading out begin-state snapshot. The latter is much easier to do, so I did a quick experiment (and I call this "approach 5"):

Approach 5

Fade out begin-state snapshot during the rotation. Here's the result:

100.fade.mp4

This doesn't look at good as the original approach 2 when I tried it on device, because:

  1. it still has max distortion in the beginning, just not fully opaque compared with the existing behavior.
  2. the overlapping of the 2 states looks bad

There's another approach that combines approach 2 and 5, and I call this approach 6.

Approach 6:

Show the begin-state snapshot for 40% of rotation, then fade out for 20% of rotation, then show end-state real screen for 40% of rotation (code here, number adjustable, I just find 40%-20%-40% result in the smoothest transition). Here's the result:

20.fade.mp4

Approach 6 is smoother than approach 2. I think it has the best result among approaches 1, 2, 5 and 6.

However, I still think approach 4 is better than any snapshot tricks, since it works for dynamic contents (videos & animations).

@tvolkert
Copy link
Contributor

tvolkert commented Apr 6, 2023

Yeah I think approach 4 still looks the best.

auto-submit bot pushed a commit to flutter/engine that referenced this issue Apr 17, 2023
…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
@danagbemava-nc danagbemava-nc added the r: fixed Issue is closed as already fixed in a newer version label Apr 18, 2023
@github-actions
Copy link

github-actions bot commented May 2, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: fidelity Matching the OEM platforms better a: layout SystemChrome and Framework's Layout Issues customer: amplify customer: crowd Affects or could affect many people, though not necessarily a specific customer. customer: google Various Google teams customer: splashbulb engine flutter/engine repository. See also e: labels. found in release: 3.3 Found to occur in 3.3 found in release: 3.7 Found to occur in 3.7 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P1 High-priority issues at the top of the work list platform-ios iOS applications specifically r: fixed Issue is closed as already fixed in a newer version
Projects
Mobile - iOS fidelity review
  
Engineer Reviewed
iOS Framework
  
Easy fix