-
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] Gpu model information to Skia gold #41216
Conversation
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.
Code lgtm. I don't know how this will manifest in skia gold, but sounds like it should work.
@@ -234,6 +236,7 @@ class SkiaGoldClient { | |||
'--work-dir', _tempPath, | |||
'--test-name', cleanTestName(testName), | |||
'--png-file', goldenFile.path, | |||
'--add-test-key', 'gpu_string:$gpuString', |
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.
This is enough to make skia gold segregate the golden results?
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.
Is this the same as adding to the list of keys provided as initialization for the test? Those are collected in _getKeys
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.
Good point, @Piinks. This should be added to dimensions
, which later gets used in _getKeys()
.
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.
Is this the same as adding to the list of keys provided as initialization for the test? Those are collected in _getKeys
Done
@@ -234,6 +236,7 @@ class SkiaGoldClient { | |||
'--work-dir', _tempPath, | |||
'--test-name', cleanTestName(testName), | |||
'--png-file', goldenFile.path, | |||
'--add-test-key', 'gpu_string:$gpuString', |
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.
Good point, @Piinks. This should be added to dimensions
, which later gets used in _getKeys()
.
In local testing the gpu_string is I'm going to rebase to clear up the validation problem. |
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.
The dimensions change looks good to me!
…125069) flutter/engine@6d263ea...5fcc7b7 2023-04-18 dnfield@google.com [Impeller] Gpu model information to Skia gold (flutter/engine#41216) 2023-04-18 30870216+gaaclarke@users.noreply.github.com [Impeller] faster glyph atlas generation by removing data copies (flutter/engine#41290) 2023-04-18 godofredoc@google.com Migrate android AOT to engine_v2. (flutter/engine#41229) 2023-04-18 skia-flutter-autoroll@skia.org Roll Skia from 5bd4bdc0d8e2 to f80ee1088861 (8 revisions) (flutter/engine#41302) 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 chinmaygarde@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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Does two things:
impeller::Context
This should help reduce noise/flakiness in golden images.