Expand coverage of android_hardware_smoke_test. Add image, text, blend mode, and blur tests.#187600
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request expands the android_hardware_smoke_test integration test suite by introducing new test cases for text rendering, image texture sampling, advanced blending, and backdrop filter blurring. It refactors the test application to support these scenarios with new widgets and lazy image loading, and adds corresponding widget and driver tests. Feedback focuses on addressing potential runtime exceptions and memory leaks by checking if the widget is mounted before calling setState during image loading, avoiding force-unwrapping on potentially null images, and documenting or privatizing several undocumented public members to comply with the repository's style guide.
- Add 'imageTest' rendering test case. - Add a small 32x32 colored checkerboard test png. - Add image loading behavior which executes only for the relevant test case, and handles load failure. - Support dependency injection for ImageLoader to enable mocking in widget tests. - Add 'captureScreenshot' channel parameter to skip fetching image bytes, which would cause a FakeAsync deadlock in widget tests. - Dispose of native Codec and Image handles in main.dart and goldens.dart to prevent resource leaks which can cause widget tests to hang.
…t so far and any references which informed them
…es missing documentation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request expands the Android hardware smoke test suite by adding new test cases for text rendering, image texture sampling, advanced blending, and backdrop filter blur. It refactors the rendering logic into separate widgets, updates the documentation, and adds corresponding widget and integration tests. Feedback focuses on improving robustness and resource management, including safer buffer conversion and resource cleanup during image loading, disposing of TextPainter to prevent leaks, replacing debug-only asserts with runtime null checks, and fixing a markdown formatting typo in the README.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request expands the Android hardware smoke test suite by adding new test cases for text rendering, image texture sampling, advanced blending, and backdrop filter blurring. It introduces the BackdropFilterBlur and VectorDrawingsCanvas widgets, updates the test driver and native JUnit tests, and adds corresponding widget tests. The review feedback highlights a performance concern in VectorDrawingsCanvas where a TextPainter is instantiated, laid out, and disposed inside the paint method of the custom painter.
…ther vector drawing tests can remain stateless
Fixed in bf888ac |
There was a problem hiding this comment.
Code Review
This pull request expands the Android hardware smoke test suite by adding new visual regression test cases for font rendering, texture sampling, advanced blending, and compositing/blur. It introduces dedicated canvas widgets, updates the Kotlin test suite and test driver, and adds corresponding widget tests. The review feedback identifies opportunities to prevent native memory leaks by ensuring proper disposal of ui.Picture and ui.Image objects across image_drawing_canvas.dart, widget_test.dart, and goldens.dart.
|
BTW this is what the goldens look like for the new tests textTest imageTest advancedBlendTest backdropFilterBlurTest |
gaaclarke
left a comment
There was a problem hiding this comment.
This is looking good. I added one question. I'm not sure if emoji will actually give us some coverage we don't have elsewhere that the gpu might get wrong.
andywolff
left a comment
There was a problem hiding this comment.
Thanks, updated textTest to add some emojis and fix the wrapping. New golden looks like this:
flutter/flutter@8bdce07...b7cb925 2026-06-12 engine-flutter-autoroll@skia.org Roll Skia from cadbde1ec4b7 to 8c89bf2b0ee3 (5 revisions) (flutter/flutter#187926) 2026-06-12 engine-flutter-autoroll@skia.org Roll Packages from 1b56cde to b78ad83 (5 revisions) (flutter/flutter#187928) 2026-06-12 engine-flutter-autoroll@skia.org Roll Dart SDK from f3441f2067ae to f6c31f4c3a63 (17 revisions) (flutter/flutter#187924) 2026-06-12 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 2KosSR4ONUjIB7tP_... to A3eaUn9mQ_EkSNxVI... (flutter/flutter#187923) 2026-06-12 engine-flutter-autoroll@skia.org Roll Skia from a2228b926c68 to cadbde1ec4b7 (9 revisions) (flutter/flutter#187921) 2026-06-12 kustermann@google.com Remove dynamic module loading code in flutter web engine (flutter/flutter#187777) 2026-06-12 matt.kosarek@canonical.com Remove EnableTransparentWindowBackground because it did nothing important and because Windows 10 does not support DWMWA_SYSTEMBACKDROP_TYPE (flutter/flutter#187848) 2026-06-12 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from dQ4PjIJB5kZFU8Y32... to EmfiOMUge_nnNS33B... (flutter/flutter#187912) 2026-06-12 mvincentong@gmail.com Clarify RichText selection docs (flutter/flutter#186844) 2026-06-12 1063596+reidbaker@users.noreply.github.com Custom KGP task and migration to AGP api for geting kgp version (flutter/flutter#182788) 2026-06-12 engine-flutter-autoroll@skia.org Roll Skia from f61acb31edf8 to a2228b926c68 (5 revisions) (flutter/flutter#187896) 2026-06-12 bdero@google.com [Flutter GPU] Expose ASTC HDR texture formats (flutter/flutter#187715) 2026-06-12 awolff@google.com Expand coverage of android_hardware_smoke_test. Add image, text, blend mode, and blur tests. (flutter/flutter#187600) 2026-06-11 bdero@google.com [Flutter GPU] Add blit operations (flutter/flutter#187289) 2026-06-11 bkonyi@google.com [flutter_tools] Fix version cache git fallback performance regression (flutter/flutter#187400) 2026-06-11 nshahan@google.com Rewrite `-d web-server` hot reload/restart tests (flutter/flutter#187453) 2026-06-11 bdero@google.com [Impeller] Allow sampling textures with manually-uploaded mip levels (flutter/flutter#187729) 2026-06-11 154381524+flutteractionsbot@users.noreply.github.com Sync CHANGELOG.md from stable (flutter/flutter#187884) 2026-06-11 47866232+chunhtai@users.noreply.github.com iOS a11y sets header trait based on heading level (flutter/flutter#186916) 2026-06-11 engine-flutter-autoroll@skia.org Roll Skia from 9f02102df298 to f61acb31edf8 (19 revisions) (flutter/flutter#187869) 2026-06-11 engine-flutter-autoroll@skia.org Roll ICU from ee5f27adc28b to d578f2e8b7bd (8 revisions) (flutter/flutter#187829) 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 louisehsu@google.com,stuartmorgan@google.com 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
…d mode, and blur tests. (flutter#187600) Expand test coverage to address more of flutter#182123 Platform view tests will be next after this. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Expand test coverage to address more of #182123
Platform view tests will be next after this.
Pre-launch Checklist
///).