Repository navigation
Conversation
…rites The render pass's incoming dependency covered only color stages, so a depth/stencil clear wasn't ordered after the attachment's layout transition or earlier depth/stencil writes (SYNC-HAZARD-WRITE-AFTER-WRITE). Add the fragment test stages and depth/stencil access, only when the pass has a depth/stencil attachment.
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Vulkan render pass builder to include depth/stencil stages and access flags in subpass dependencies when a depth/stencil attachment is present, and adds corresponding unit tests. It also updates the Vulkan mock device to track and retrieve render pass dependencies. Feedback on the mock implementation notes that constructing a vector from a potentially null pointer when the dependency count is zero can lead to undefined behavior, and suggests adding a null check.
…vkCreateRenderPass
andywolff
left a comment
andywolff
left a comment
There was a problem hiding this comment.
LGTM, thanks for the fix
|
the ci.yaml validation failure check means you need to sync |
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
Let's rebase this one too and add a validation test to run_tests.py
…y tests flutter#195714 added eColorAttachmentRead to the incoming dependency's destination access, and flutter#195689 landed the depth/stencil masks this PR originally added. Update the expected masks to match.
|
Thanks for landing #195689 (and #195714), and for getting the depth validation tests back on - that covers both the fix and the validation test. I merged main and this PR is now just the RenderPassBuilder unit tests (the mock records the render pass dependencies, and the two tests check the masks, updated for the color attachment read). Totally understand if that's not worth keeping now that the validation shard covers it. Happy to close it if so. |
|
Okay, sounds good, thanks for your help. |
RenderPassBuilderVK's incoming subpass dependency (VK_SUBPASS_EXTERNALto subpass 0) only covers the color attachment output and fragment shader stages. A depth/stencil attachment's load op (clear) runs in the fragment test stages, so it wasn't ordered after the attachment's layout transition or earlier depth/stencil writes.When the render pass has a depth/stencil attachment, this adds the early and late fragment test stages and depth/stencil access to that dependency. Passes without one keep the existing masks, so color-only passes don't wait on more than before.
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.Tests:
RenderPassBuilder.ExternalDependencyCoversDepthStencilClearchecks the exact masks for a pass with depth/stencil. It fails without the fix.RenderPassBuilder.ExternalDependencyOmitsDepthStagesWithoutDepthStencilchecks a color-only pass keeps the existing masks.To read the dependencies, the Vulkan mock now records the ones passed to
vkCreateRenderPass(GetLastRenderPassDependencies).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.