Repository navigation
[web] Unskip and strengthen missing glyph tofu test on Wimp - #193426
Conversation
) Unskips `'Renders tab as space instead of tofu'` on Wimp (`isWimp`) in `engine/src/flutter/lib/web_ui/test/ui/text_test.dart` and adds explicit assertions verifying that: - `>\b<` differs from `><` (so `\b` is not silently dropped), and - both control (`\b`) and non-control unassigned (`\u0378`) missing codepoints render non-empty `.notdef` tofu glyph boxes (`50` non-zero pixels at `(0,3)..(5,12)` on both `wimp` and `skwasm`). Root cause of flutter#183944: In the initial draft of PR flutter#183913 (`822dab3491e`), `DlWimpImageFromPicture::GetImpellerTexture` constructed a temporary `impeller::AiksContext(context, nullptr)` with a null `TypographerContext`, causing `picture.toImage()` to drop all text runs and render blank images for `tabImage`, `spaceImage`, and `tofuImage` (making `tabImage == tofuImage`). Later in PR flutter#183913 (`6e48628f47b`), `GetImpellerTexture` was refactored to use the `ImpellerRenderContext`'s `ContentContext` (which owns `TypographerContextSkia::Make()`), fixing `.notdef` glyph rendering in `picture.toImage()`, while the `skip: isWimp` added in `85495319a63` remained. Fixes flutter#183944
|
This pull request targets the Please change the base branch of this pull request to |
…nt inner skip
- Dispose `Paragraph` and `Picture` inside `drawText` and register `addTearDown(image.dispose)`.
- Remove the redundant inner `skip: isSafari || isFirefox` on `Renders tab as space instead of tofu` (already applied on the enclosing `group('Text', ..., skip: isSafari || isFirefox)`).
- Correct historical commit attribution in PR notes (flutter#175442 introduced `DlWimpImageFromPicture::GetImpellerTexture` with a null `TypographerContext`, flutter#183913 added the `!isWimp` skip, and flutter#191747 refactored `GetImpellerTexture` to use `ImpellerRenderContext`).
There was a problem hiding this comment.
Code Review
This pull request refactors the 'Renders tab as space instead of tofu' test in text_test.dart to explicitly dispose of native resources and adds new test cases for unassigned codepoints and empty strings. Feedback recommends wrapping the resource disposal in try/finally blocks to prevent leaks during exceptions and ensuring the paragraph is not disposed of before picture.toImage completes to avoid potential use-after-free issues.
gaaclarke
left a comment
There was a problem hiding this comment.
The bot has a point! Otherwise looks good.
Addressed in 0f21c6d (wrapped paragraph and picture disposal in try/finally after await picture.toImage completes); re-requesting review from @gaaclarke.
|
autosubmit label was removed for flutter/flutter/193426, because - The status or check suite Mac_arm64_verify_binaries has failed. Please fix the issues identified (or deflake) before re-applying this label.
|
…13104) Manual roll requested by [email protected] flutter/flutter@d649d2b...e89fd0a 2026-10-01 [email protected] Manual roll ICU from 6ebb40c59477 to 5aa526207171 (2 revisions) (flutter/flutter#193661) 2026-10-01 [email protected] fix(web): hash source maps before rewriting sourceMappingURL with --web-content-hash (flutter/flutter#193614) 2026-10-01 [email protected] [flutter_tools] Bootstrap DeviceManager in ToolDependencies (flutter/flutter#193524) 2026-10-01 [email protected] [wasm] Set flutter_runtime_mode in tools/gn to_gn_wasm_args (flutter/flutter#193407) 2026-10-01 [email protected] Remove remaining `download_jdk` references (flutter/flutter#193562) 2026-10-01 [email protected] Synchronize dart2wasm feature probes in roll script and flutter_bootstrap.js (flutter/flutter#193271) 2026-10-01 [email protected] [web] Unskip web tests that now pass in gestures, services, and widgets (flutter/flutter#193512) 2026-10-01 [email protected] Roll Fuchsia Test Scripts from whZAV1mfT-Rdc0rQ8... to fCNknap4ZI0AMcvIf... (flutter/flutter#193640) 2026-10-01 [email protected] Roll Skia from 0575a37d0619 to f2d68e0b8863 (2 revisions) (flutter/flutter#193621) 2026-10-01 [email protected] perf(widgets): use const <Never>{} in InheritedModelElement and avoid redundant casts in RenderObjectElement.update (flutter/flutter#192913) 2026-10-01 [email protected] [web] Unskip and strengthen missing glyph tofu test on Wimp (flutter/flutter#193426) 2026-10-01 [email protected] [flutter_tools] Fix `flutter drive -d chrome` hanging on web (flutter/flutter#193459) 2026-10-01 [email protected] [web] Fix hang when Chromium cannot bind remote debugging port (flutter/flutter#193461) 2026-10-01 [email protected] [flutter_tools] Bundle data assets from hooks in flutter test (flutter/flutter#193435) 2026-10-01 [email protected] Roll Packages from 0ba9a82 to d5ec6db (11 revisions) (flutter/flutter#193641) 2026-10-01 [email protected] Revert: [flutter_tools] Fix DTD "Stream already subscribed" error and wait for analysis over DTD in widget preview (flutter/flutter#193637) 2026-10-01 [email protected] Roll Skia from 6182a3cf8f98 to 0575a37d0619 (5 revisions) (flutter/flutter#193613) 2026-10-01 [email protected] [flutter_tools] Fix DTD "Stream already subscribed" error and wait for analysis over DTD in widget preview (flutter/flutter#192930) 2026-09-30 [email protected] [Impeller] Sample the render target rect in offscreen advanced blends (flutter/flutter#192988) 2026-09-30 [email protected] Roll Skia from 01524014dff5 to 6182a3cf8f98 (11 revisions) (flutter/flutter#193594) 2026-09-30 [email protected] [web] Use Xvfb -displayfd and clean up Xvfb after Firefox test suites (flutter/flutter#193510) 2026-09-30 [email protected] Fixes bug where sibling node fail to update size (flutter/flutter#191220) 2026-09-30 [email protected] Roll Skia from c7b323126bf0 to 01524014dff5 (3 revisions) (flutter/flutter#193577) 2026-09-30 [email protected] Remove some Undefined Behavior from run_customer_testing_tests.dart. (flutter/flutter#193582) 2026-09-30 [email protected] Fix pointer id/coordinate mismatch in Android platform view multi-touch (flutter/flutter#191631) 2026-09-30 [email protected] Roll Packages from 3c6ce59 to 0ba9a82 (11 revisions) (flutter/flutter#193578) 2026-09-30 [email protected] Fix unresolved doc comment references in dev/ tooling (flutter/flutter#193501) If this roll has caused a breakage, revert this CL and set the roller to dry run mode 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
…193426) Unskips `Renders tab as space instead of tofu` in `engine/src/flutter/lib/web_ui/test/ui/text_test.dart` on Wimp (`chrome-dart2wasm-wimp-ui`) and strengthens the test assertions: - When `DlWimpImageFromPicture::GetImpellerTexture` was introduced in flutter#175442 (`e0f544bf620`), it constructed a temporary `impeller::AiksContext` with a `nullptr` `TypographerContext`, causing `picture.toImage()` to drop text runs and return blank images (which caused `matchImage(tabImage, tofuImage)` to fail when the `!isWimp` skip was added in flutter#183913 / `2490c22d5be`). - flutter#191747 (`87c02198123`) refactored `DlWimpImageFromPicture::GetImpellerTexture` to render via `ImpellerRenderContext` (which owns the shared `impeller::ContentContext` and `TypographerContextSkia::Make()`), restoring `.notdef` glyph rendering in `picture.toImage()`. - Removes the `!isWimp` guard so `Renders tab as space instead of tofu` runs on both Skwasm and Wimp. - Adds an assertion that unassigned non-control codepoints (`\u{0378}`) also render a `.notdef` tofu box distinct from `''`, and disposes `Paragraph`, `Picture`, and `Image` resources created inside `drawText`. Fixes flutter#183944 ## 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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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 [flutter/website]: https://github.com/flutter/website [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
Unskips
Renders tab as space instead of tofuinengine/src/flutter/lib/web_ui/test/ui/text_test.darton Wimp (chrome-dart2wasm-wimp-ui) and strengthens the test assertions:DlWimpImageFromPicture::GetImpellerTexturewas introduced in [wimp] Initial Impeller on Web implementation. #175442 (e0f544bf620), it constructed a temporaryimpeller::AiksContextwith anullptrTypographerContext, causingpicture.toImage()to drop text runs and return blank images (which causedmatchImage(tabImage, tofuImage)to fail when the!isWimpskip was added in [wimp] Implement images for wimp. #183913 /2490c22d5be).87c02198123) refactoredDlWimpImageFromPicture::GetImpellerTextureto render viaImpellerRenderContext(which owns the sharedimpeller::ContentContextandTypographerContextSkia::Make()), restoring.notdefglyph rendering inpicture.toImage().!isWimpguard soRenders tab as space instead of tofuruns on both Skwasm and Wimp.\u{0378}) also render a.notdeftofu box distinct from'', and disposesParagraph,Picture, andImageresources created insidedrawText.Fixes #183944
Pre-launch Checklist
///).