-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Using initExpensiveAndroidView for Android Hybrid Composition Mode
#142399
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
Conversation
…instead of `initSurfaceAndroidView` `initSurfaceAndroidView` attempts to use the newest and most efficient platform view implementation when possible. In cases where that is not supported, it falls back to using Hybrid Composition, which is the mode used by `initExpensiveAndroidView`.
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
In the future please include someone from the Flutter Android team on PRs like this. |
|
SkiaPerf recorded a large regression on this PR in |
|
Time to revert pull request flutter/flutter/142399 has elapsed. |
|
@zanderso Based on the benchmark name this benchmark is supposed to be testing the hybrid composition path. It was testing the TLHC path before this PR. Not sure what we want to do. This change invalidates historical data as it is now measuring something completely different. I do think this change brings the benchmark in line with what it claims to be measuring. WDYT? |
|
@zanderso I sent a revert PR your way, feel free to submit it. |
I don't think we should revert this. I approved the PR because the benchmark had clearly become incorrect when Emmanuel changed what this function did and added a new, different way of getting the old behavior (HC). The benchmark clearly should have been changed at that time, but wasn't, and has therefore been wrong for quite some time. |
flutter/flutter@ace9181...75a2e5b 2024-01-30 [email protected] Reset framesEnabled to default value at the end of each test (flutter/flutter#141844) 2024-01-30 [email protected] Fix SliverMainAxisGroup geometry cacheExtent (flutter/flutter#142482) 2024-01-30 [email protected] Roll Flutter Engine from 0e586d1c28c8 to f02a4a80a77e (3 revisions) (flutter/flutter#142528) 2024-01-30 [email protected] Roll Packages from 516648a to 25abb5d (6 revisions) (flutter/flutter#142527) 2024-01-30 [email protected] Roll Flutter Engine from 438e9b4d7d4e to 0e586d1c28c8 (4 revisions) (flutter/flutter#142515) 2024-01-30 [email protected] Using `initExpensiveAndroidView` for Android Hybrid Composition Mode (flutter/flutter#142399) 2024-01-30 [email protected] Roll Flutter Engine from ed73d40a8c93 to 438e9b4d7d4e (1 revision) (flutter/flutter#142508) 2024-01-30 [email protected] Organize leak tracking TODOs. (flutter/flutter#142460) 2024-01-30 [email protected] Roll Flutter Engine from 5584a78a439b to ed73d40a8c93 (1 revision) (flutter/flutter#142504) 2024-01-30 [email protected] Roll Flutter Engine from df5f1afd4991 to 5584a78a439b (2 revisions) (flutter/flutter#142503) 2024-01-30 [email protected] Roll Flutter Engine from 65bf8b1db4d1 to df5f1afd4991 (1 revision) (flutter/flutter#142501) 2024-01-30 [email protected] Roll Flutter Engine from c9268c7db03c to 65bf8b1db4d1 (1 revision) (flutter/flutter#142496) 2024-01-30 [email protected] Roll Flutter Engine from e21208583956 to c9268c7db03c (1 revision) (flutter/flutter#142492) 2024-01-29 [email protected] Roll Flutter Engine from bedafa8794b6 to e21208583956 (1 revision) (flutter/flutter#142483) 2024-01-29 [email protected] Revert "Reland: "Fix how Gradle resolves Android plugin" (#137115)" (flutter/flutter#142464) 2024-01-29 [email protected] Marks Mac_pixel_7pro native_assets_android to be unflaky (flutter/flutter#141675) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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
|
@zanderso Given we've decided to keep this change, how do we reset the benchmark history? |
|
To answer the question, renaming the benchmark is the only way I know of to do that. Unfortunately, that's kind of a big hammer since the benchmark with the old name will persist in the database and clutter the UI. If we do nothing here but clear the triage bit, future regressions against the new baseline will still be flagged and come up as needing triage, so that's the path I would recommend. |
After #100990, we should use
initExpensiveAndroidViewfor Android Hybrid Composition mode instead ofinitSurfaceAndroidView.initSurfaceAndroidViewattempts to useTLHCwhen possible. In cases where that is not supported, it falls back to using Hybrid Composition.flutter/engine#49414
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.