[iOS] remove arbitrary framerate cap. #51663
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| // Pressure tested on iPhone 13 pro, the oldest iPhone that supports refresh rate greater than | ||
| // 60fps. A flutter app can handle fast scrolling on 80 fps with 6 PlatformViews in the scene | ||
| // at the same time. | ||
| new_max_refresh_rate = 80; |
There was a problem hiding this comment.
Ask to learn: Does this mean that we wanted to cap to 80FPS when there's platform view on screen (but if no platform view, it's capped at 120FPS)? By removing this, we remove the 80FPS cap, and let OS to decide what refresh rate is.
There was a problem hiding this comment.
When there were 5 or more platform views, we set the cap to 80. Now we always set to whatever the maximum rate is, in this case 120. AFAIK, we don't fully support adjusting the frame rate.
There was a problem hiding this comment.
For background, check out flutter/flutter#116640 #39172 (and #39172), there's a lot of info in that issue.
jonahwilliams
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Mar 27, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Mar 27, 2024
…145801) flutter/engine@92ebd47...d872d50 2024-03-27 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from uu8lffXkeJQ9PC96I... to Lk8KBU-c97ROj-YHm... (flutter/engine#51690) 2024-03-26 zanderso@users.noreply.github.com Turn off internal retries for Android scenario app tests (flutter/engine#51689) 2024-03-26 skia-flutter-autoroll@skia.org Roll Skia from 1808016c7a6d to 23a5617e7f47 (1 revision) (flutter/engine#51688) 2024-03-26 chris@bracken.jp [macOS] Consolidate FlutterViewController static types/data (flutter/engine#51486) 2024-03-26 skia-flutter-autoroll@skia.org Roll Skia from 0590062821dc to 1808016c7a6d (3 revisions) (flutter/engine#51686) 2024-03-26 jonahwilliams@google.com [iOS] remove arbitrary framerate cap. (flutter/engine#51663) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from uu8lffXkeJQ9 to Lk8KBU-c97RO If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This doesn't actually seem to be helping. For the most part, the frame time of the banners is steady with occassional spikes. Adding the framerate cap doesn't help the spikes, since those are in the 30+ ms frame time, and just results in everything else looking janky as well (especially if we dance around the cap, then we go 120 - 80)
Since this is based on an arbitrary number, from an arbitrary app, on an arbitrary point in time we should remove it and instead try to fix the performance problems