Revert "[macOS] Use CVDisplayLink to drive repaint (#49159)" #51095
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
This reverts commit 21474ee.
cbracken
added
the
autosubmit
knopp
added
the
waiting for tree to go green
|
@cbracken Will this need a cherry pick to the upcoming beta branch? The branch cutoff was February 28 at 5pm PST. |
cbracken
approved these changes
Feb 29, 2024
|
Failed to create CP due to merge conflicts. |
|
Re: the cherry-pick, yes we'll need to do it, but the release branch won't exist until branch alignment is complete. @itsjustkevin and I will sync on Monday on this. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Feb 29, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Feb 29, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Mar 1, 2024
…144436) flutter/engine@7e8fefe...6f80444 2024-02-29 [email protected] [Impeller] use specific format for bootstrap texture. (flutter/engine#51082) 2024-02-29 [email protected] [Impeller] remove warning about device transient depth+stencil. (flutter/engine#51033) 2024-02-29 [email protected] Refactor GL rendering to prepare for GTK4 (flutter/engine#50754) 2024-02-29 [email protected] Reland "[skwasm] Clip pictures if they go beyond the bounds of the window." (flutter/engine#51077) 2024-02-29 [email protected] Revert "[macOS] Use CVDisplayLink to drive repaint (#49159)" (flutter/engine#51095) 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 [email protected],[email protected],[email protected] 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
8 tasks
knopp
added a commit
to knopp/engine
that referenced
this pull request
Mar 1, 2024
…9159)" (flutter#51095)" This reverts commit dd3383d.
knopp
added a commit
to knopp/engine
that referenced
this pull request
Mar 4, 2024
…9159)" (flutter#51095)" This reverts commit dd3383d.
knopp
added a commit
that referenced
this pull request
Mar 4, 2024
This relands the PR reverted in #51095 Changes since the original PR: - The macOS embedder does not assume particular clock when calling the embedder API, but converts CAMediaTime to engine time and back (`FlutterTimeConverter`) - `FlutterVSyncWaiter` does not wait for displaylink callback during warmup frame. This should prevent `timeToFirstFrameRasterizedMicros` regressions. - When enforcing frame pacing the raster thread is not blocked. This should prevent `average_frame_rasterizer_time_millis` regressions. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
8 tasks
auto-submit bot
added a commit
that referenced
this pull request
Mar 5, 2024
…#51192) Reverts #51126 Initiated by: cbracken Reason for reverting: looks like this regressed Skia benchmarks: https://flutter-flutter-perf.skia.org/e/?keys=Xbf8dce9c233bce34d20e2280e3f1d8db&selected=commit%3D39653%26name%3D%2Carch%3Dintel%2Cbranch%3Dmaster%2Cconfig%3Ddefault%2Cdevice_type%3Dnone%2Cdevice_version%3Dnone%2Chost_type%3Dmac%2Csub_result%3DtimeToFirstFrameRasterizedMicros%2Ctest%3Dflutter_gallery_macos__start_up%2C&xbaroffset=39569 The regression s Original PR Author: knopp Reviewed By: {cbracken} This change reverts the following previous change: Original Description: This relands the PR reverted in #51095 Changes since the original PR: - The macOS embedder does not assume particular clock when calling the embedder API, but converts CAMediaTime to engine time and back (`FlutterTimeConverter`) - `FlutterVSyncWaiter` does not wait for displaylink callback during warmup frame. This should prevent `timeToFirstFrameRasterizedMicros` regressions. - When enforcing frame pacing the raster thread is not blocked. This should prevent `average_frame_rasterizer_time_millis` regressions. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
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.
The original PR assumed that
fml::TimePointhas same base asCACurrentMediaTime(). However due to recent change in Dart SDK that's no longer true. Dart SDK replaced call tomach_absolute_timewithclock_gettime_nsec_np(CLOCK_MONOTONIC_RAW), whileCACurrentMediaTime()corresponds toCLOCK_UPTIME_RAW.This needs to be fixed either in Dart SDK dart-lang/sdk#55071 or the original PR needs to be relanded with appropriate conversions.
This reverts commit 21474ee.
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. You must list at least one issue.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.