-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
[Impeller] Adds impeller display list golden tests #52690
Conversation
537b6df
to
b1ec721
Compare
INSTANTIATE_PLAYGROUND_SUITE(DlGoldenTest); | ||
|
||
TEST_P(DlGoldenTest, CanDrawPaint) { | ||
auto draw = [](DlCanvas* canvas, |
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.
I considered making an OpenPlaygroundHere
that takes in this lambda, but decided to stick with the existing API. The lambda API would remove boilerplate, but this makes the golden tests more accessible to old tests if we want.
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.
Can we make sure that the goldens are registered with the Aiks prefix, so when we migrate tests the existing goldens stay valid?
If we follow out the plan in flutter/flutter#146941 that's not going to be an issue since when the tests are migrated they will be in the exact same location and the names come from the location. They'll just be routed through the display lists instead. |
I'd like to lift and sift the existing tests without any shims that need to be maintained |
However we do it we can keep the old names. If one wanted to manually migrate a test they'd just put it in a test suite that uses the |
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
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. |
…148137) flutter/engine@c0917b1...1ccd0c3 2024-05-10 30870216+gaaclarke@users.noreply.github.com Fixed constness of display list storage. (flutter/engine#52705) 2024-05-10 zanderso@users.noreply.github.com Revert "Various documentation improvements (#52600)" (flutter/engine#52709) 2024-05-10 skia-flutter-autoroll@skia.org Manual roll Dart SDK from b7cad2edae4b to 01121c008f4d (3 revisions) (flutter/engine#52706) 2024-05-10 gray@tacck.net [iOS] Fix App crash when use WebView with iOS VoiceOver (flutter/engine#52484) 2024-05-09 ian@hixie.ch Various documentation improvements (#52600) (flutter/engine#52623) 2024-05-09 yjbanov@google.com [web] scale semantic text elements to match the desired focus ring size (flutter/engine#52586) 2024-05-09 30870216+gaaclarke@users.noreply.github.com [Impeller] Adds impeller display list golden tests (flutter/engine#52690) 2024-05-09 zanderso@users.noreply.github.com Roll buildroot to 70a42312a688 (flutter/engine#52675) 2024-05-09 matanlurey@users.noreply.github.com When `et` is not attached to a terminal, still split lines for status updates. (flutter/engine#52681) 2024-05-09 30870216+gaaclarke@users.noreply.github.com updated analysis exclusion (flutter/engine#52699) 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 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
issue flutter/flutter#146941
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.