Repository navigation
[Impeller] Use the transfer stage for the Vulkan readback barrier - #194991
Conversation
BlitPassVK::OnCopyTextureToBufferCommand transitions the source image to TransferSrc and then copies it with vkCmdCopyImageToBuffer. The barrier named shader reads in the vertex and fragment stages as its destination, so nothing ordered the copy after the layout transition. Synchronization validation reports SYNC-HAZARD-READ-AFTER-WRITE on the copy. Use transfer reads in the transfer stage, which is what the copy does.
There was a problem hiding this comment.
Code Review
This pull request resolves a Vulkan synchronization hazard (SYNC-HAZARD-READ-AFTER-WRITE) in BlitPassVK::OnCopyTextureToBufferCommand by updating the image memory barrier's destination access mask to eTransferRead and its destination stage to eTransfer. To support testing this change, the mock Vulkan implementation was updated to track and expose the destination stage masks of pipeline barriers. A new unit test was added to verify the barrier configuration, and the Play/RendererTest.CanBlitTextureToBuffer/Vulkan test was re-enabled in the Vulkan validation test suite. There are no review comments, and I have no additional feedback to provide.
gaaclarke
left a comment
There was a problem hiding this comment.
Can you remove the unittests here please and just rely on the validation layers. Otherwise looks good.
Rely on the Vulkan sync validation test (RendererTest.CanBlitTextureToBuffer) instead, per review.
|
Thanks! Removed the unit test and merged main, so it's just the barrier fix plus re-enabling CanBlitTextureToBuffer in the sync validation list. |
|
autosubmit label was removed for flutter/flutter/194991, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
Roll Flutter from 3623d2d34543 to 09175cdeac89 (61 revisions) flutter/flutter@3623d2d...09175cd 2026-11-06 [email protected] Roll Skia from ae35b7b2221f to 793b8ac73d4b (3 revisions) (flutter/flutter#196091) 2026-11-06 [email protected] [web] Rasterize scenes at the size passed to `FlutterView.render` (flutter/flutter#196052) 2026-11-06 [email protected] Roll Skia from 4aa043a81e0c to ae35b7b2221f (1 revision) (flutter/flutter#196086) 2026-11-06 [email protected] Roll Fuchsia Linux SDK from Fqe3UlLwAohsPbsCS... to KIc0XZbhLmvDpTluZ... (flutter/flutter#196085) 2026-11-06 [email protected] Allow a zero scale change in InteractiveViewer (flutter/flutter#194949) 2026-11-06 [email protected] Roll Skia from dd5c2553a856 to 4aa043a81e0c (2 revisions) (flutter/flutter#196079) 2026-11-06 [email protected] [web] Keep ImageFilter.blur bounds in equality and description (flutter/flutter#196013) 2026-11-06 [email protected] iOS: Remove unused PlatformViewIOS setup from FlutterPlatformViewsTest (flutter/flutter#196033) 2026-11-06 [email protected] Serialize LoggerTests that mutate the shared Logger (flutter/flutter#196028) 2026-11-06 [email protected] Use reverseDuration for the AnimatedCrossFade size animation (flutter/flutter#194860) 2026-11-05 [email protected] Roll Skia from a5aacd103e68 to dd5c2553a856 (8 revisions) (flutter/flutter#196073) 2026-11-05 [email protected] Fix ListWheelScrollView null check error when it reads itemIndex before content dimensions are set (flutter/flutter#194859) 2026-11-05 [email protected] Allow zero cross axis extent in SliverGridDelegateWithMaxCrossAxisExtent (flutter/flutter#194858) 2026-11-05 [email protected] Revert: Allow color delta tolerance in in-memory fuzzy image comparator (flutter/flutter#196067) 2026-11-05 [email protected] Revert: Mitigate snapshot race in 3D transform test by reducing loop density (flutter/flutter#196066) 2026-11-05 [email protected] Delegate createStream in ResizeImage and ScrollAwareImageProvider (flutter/flutter#194756) 2026-11-05 [email protected] [iOS] Introduce FlutterShell and move thread host and profiler (flutter/flutter#195388) 2026-11-05 [email protected] [web] Support FragmentShader.getImageSampler on CanvasKit and Skwasm (flutter/flutter#195854) 2026-11-05 [email protected] Deprecates setFloat API (flutter/flutter#195505) 2026-11-05 [email protected] [flutter_tools] Remove public GlobalToolContext and globals from runner (flutter/flutter#195561) 2026-11-05 [email protected] Defer TLHC PlatformViewWrapper frame post and avoid initial 1x1 buffer resize (flutter/flutter#195861) 2026-11-05 [email protected] Roll Skia from b5abed66da86 to a5aacd103e68 (17 revisions) (flutter/flutter#196054) 2026-11-05 [email protected] Made `FlutterWindowControllerSizeTest.SizedToContentNotResizable` more robust (flutter/flutter#195845) 2026-11-05 [email protected] Update docs for contributor ladder (flutter/flutter#196011) 2026-11-05 [email protected] [Impeller] Use the transfer stage for the Vulkan readback barrier (flutter/flutter#194991) 2026-11-05 [email protected] Remove Mac mac_unopt builder (flutter/flutter#196027) 2026-11-05 [email protected] Updated Instructions for Determining Gradle Version for `gradle_dists` in CIPD (flutter/flutter#193310) 2026-11-05 [email protected] Move ios_app_with_extensions_test into the tool integration shard (flutter/flutter#195609) 2026-11-05 [email protected] [web] Respect Uint8List view bounds when detecting animated WebP/GIF (flutter/flutter#196010) 2026-11-05 [email protected] Add platform_views_layout to build_tests so iOS/Android build breaks are caught in presubmit (flutter/flutter#192985) 2026-11-05 [email protected] [flutter_tools] Detect macOS TCC permission crashes and provide diagnostic guidance (flutter/flutter#192432) 2026-11-05 [email protected] Roll Packages from 670b30e to 0478162 (15 revisions) (flutter/flutter#196047) 2026-11-05 [email protected] [web] Support Skwasm rendering without bitmap capture (flutter/flutter#195103) 2026-11-05 [email protected] [flutter_tools] Reland fix for local CanvasKit asset paths on Windows (flutter/flutter#195770) 2026-11-05 [email protected] Roll Skia from f96be51af95d to b5abed66da86 (14 revisions) (flutter/flutter#196017) 2026-11-05 [email protected] Split chrome web engine test drone (flutter/flutter#195873) 2026-11-05 [email protected] Roll Dart SDK from c9153304136a to 6623b84628f3 (1 revision) (flutter/flutter#196026) 2026-11-05 [email protected] Roll ICU from 4fe960221710 to a0ea93436c52 (6 revisions) (flutter/flutter#196025) 2026-11-05 [email protected] Add more guidance in README and AGENTS for contributing to flutter_tools (flutter/flutter#195998) 2026-11-05 [email protected] Roll Fuchsia Linux SDK from p0HDXLISPIElJfSz7... to Fqe3UlLwAohsPbsCS... (flutter/flutter#196019) 2026-11-05 [email protected] Roll Dart SDK from e362a23e9738 to c9153304136a (18 revisions) (flutter/flutter#196014) 2026-11-05 [email protected] [Impeller] Fix dilation input bounds when clipping saveLayers (flutter/flutter#195652) 2026-11-05 [email protected] Add `variantOf` to the M3E migration checklist (flutter/flutter#195099) 2026-11-05 [email protected] [flutter_tools] Eliminate globals from remaining subsystem files (flutter/flutter#195555) 2026-11-05 [email protected] Fix gap flipping when dragging variable height reorderable items (flutter/flutter#194478) 2026-11-05 [email protected] [A11y] Send un-merged SemanticsNodes in the same update that first references them (flutter/flutter#195829) ...
BlitPassVK::OnCopyTextureToBufferCommandtransitions the source image toTransferSrcand then copies it withvkCmdCopyImageToBuffer. The barrier for that transition named shader reads in the vertex and fragment stages as its destination, so nothing ordered the copy after the layout transition.This changes the destination to transfer reads in the transfer stage, which is what the copy does. The barrier after the copy (transfer to host read) was already correct.
Found with
VK_LAYER_KHRONOS_validation1.4.309 and synchronization validation on, on a Raspberry Pi 5 / CM5 (V3D 7.1, Mesa 26.2.1) running Impeller Vulkan through a custom embedder. The message appeared at every app launch and no longer appears with this change. The code changed here is identical onmaster.Test:
BlitPassVKTest.CopyTextureToBufferBarrierTargetsTheCopychecks the barrier's destination access and stage. To check the stage, the Vulkan mock now also records each image barrier'sdstStageMask(GetImageMemoryBarrierDstStages). The test fails without the fix.Fixes: #194987
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.