-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] adjusted the rotated text test to have an opaque background and account for screen scale #40954
Conversation
and account for screen scale
bdero
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!
| paint.color = Color(0.1, 0.1, 0.1, 1.0); | ||
| canvas.DrawRect( | ||
| Rect::MakeLTRB(0, 0, GetWindowSize().width, GetWindowSize().height), | ||
| paint); |
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.
nit: DrawPaint would also work here.
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 call. I guess we should probably add docstrings. It didn't occur to me that's what it would do, but makes sense.
|
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. |
… background and account for screen scale (flutter/engine#40954)
… background and account for screen scale (flutter/engine#40954)
…ions) (#124300) Manual roll requested by [email protected] flutter/engine@610cdf0...1ec74c6 2023-04-06 [email protected] Revert "[web] Move text editing nodes outside of shadowDOM - reopened" (flutter/engine#40960) 2023-04-05 [email protected] [web] Move text editing nodes outside of shadowDOM - reopened (flutter/engine#40904) 2023-04-05 [email protected] [Impeller] adjusted the rotated text test to have an opaque background and account for screen scale (flutter/engine#40954) 2023-04-05 [email protected] Copy canvaskit files directly into `flutter_web_sdk` (flutter/engine#40951) 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://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
|
I'm seeing this flake still. I think we need to have something in gold that accounts for the GPU model. |
Is this flaking or getting confused since it is so different than the old goldens? It seems like there should be a way to reset the test but I couldn't find a way. |
|
I've added an ignore rule for that test: https://flutter-engine-gold.skia.org/ignores I think this will allow us to observe all the variations and decide what to do. |
…ions) (flutter#124300) Manual roll requested by [email protected] flutter/engine@610cdf0...1ec74c6 2023-04-06 [email protected] Revert "[web] Move text editing nodes outside of shadowDOM - reopened" (flutter/engine#40960) 2023-04-05 [email protected] [web] Move text editing nodes outside of shadowDOM - reopened (flutter/engine#40904) 2023-04-05 [email protected] [Impeller] adjusted the rotated text test to have an opaque background and account for screen scale (flutter/engine#40954) 2023-04-05 [email protected] Copy canvaskit files directly into `flutter_web_sdk` (flutter/engine#40951) 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://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
issue: flutter/flutter#124277
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.