fix: synchronise Vulkan attachment reads, buffer barriers and swapchain acquisition - #3324
Draft
sasvdw wants to merge 2 commits into
Draft
Conversation
Contributor
|
🤖 Draft PR — automatic CI is skipped to save runner minutes.
|
BarrierMapping.ToVkAccessFlags granted write-only access to attachments: RenderTarget gave ColorAttachmentWrite and DepthStencilWrite gave DepthStencilAttachmentWrite. A colour attachment is also read, by blending and by a load op preserving existing contents, and a writable depth attachment is read by the depth and stencil tests, so every barrier the engine issued for a render target or depth buffer left the render pass's read unsynchronised. Texture.Vulkan.cs applied the same write-only masks as a texture's initial access state, so correcting the mapping alone was not enough. This aligns Vulkan with the model Direct3D12 already implements, where a single RENDER_TARGET access bit covers both and DEPTH_STENCIL_WRITE permits test reads. Vulkan's access flags are strictly separate, so the same intent has to be spelled out. BarrierLayout never documented which accesses each member covers, which is why UnorderedAccess, the one member whose documentation says "reading and writing", is also the one whose mapping was already correct; the two that were wrong are the two that said nothing. ResourceBarrierTransition also threw for anything that was not a Texture, so a compute write to a structured buffer could not be handed to a later reader. Buffers have no Vulkan layout, so the new branch derives access and stage from BarrierLayout without consulting ToVkImageLayout. It reads the buffer's static access and stage masks as a conservative source but never writes them, because the copy and upload paths rely on those staying a superset of every legal usage. Adds opt-in synchronization validation behind STRIDE_VULKAN_SYNC_VALIDATION=1, which is what surfaced the attachment hazards. Its extension comes from the validation layer rather than the ICD, so it is absent from an instance extension query made with no layer name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CreateBackBuffers transitioned every swapchain image to Present and submitted that command buffer before the first acquisition. Vulkan permits use of a presentable image only between vkAcquireNextImageKHR and vkQueuePresentKHR, and a layout transition is such a use. Validation reported the violation once per image per swapchain creation. The transition existed to give the tracked layout a valid source. AcquireNextImage now sets that state to Undefined instead, which is correct because the contents of a newly acquired image are undefined. This also removes a vkQueueWaitIdle from every swapchain creation and resize. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sasvdw
force-pushed
the
fix/vulkan-attachment-reads-and-buffer-barriers
branch
from
August 9, 2026 11:11
b3b9a75 to
8bf9313
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Details
Vulkan barriers left three classes of access unsynchronized. This PR fixes all three, and adds the switch that reports them.
1. Attachment barriers covered writes but not reads
BarrierMapping.ToVkAccessFlagsgaveRenderTargetonlyColorAttachmentWrite, andDepthStencilWriteonlyDepthStencilAttachmentWrite. A color attachment is also read, by blending and by a load operation that preserves the existing contents. A writable depth attachment is read by the depth and stencil tests. Every barrier the engine issued for a render target or a depth buffer therefore left the render pass read unsynchronized.Texture.Vulkan.csapplied the same write-only masks as a texture's initial access state. A fix to the mapping alone was not enough.Direct3D12 already implements this model. One
RENDER_TARGETaccess bit covers both directions, andDEPTH_STENCIL_WRITEpermits the test reads. Vulkan keeps its access flags strictly separate, so the same intent has to be spelled out.BarrierLayoutnever documented which accesses each member covers.UnorderedAccessis the one member whose documentation said "reading and writing", and it is also the one member whose mapping was already correct. The two members that were wrong are the two that documented nothing. Each member now states what it covers, so a backend maps against a contract instead of a name.Synchronization validation reported 40
READ_AFTER_WRITEhazards across 5 tests before this change, and none after.2.
ResourceBarrierTransitionthrew for buffersThe method handled
Textureand threwNotImplementedExceptionfor everything else. A compute shader could not hand a structured buffer write to a later reader.Buffers have no layout in Vulkan, so the new branch derives access and stage flags from
BarrierLayoutand never callsToVkImageLayout. It records the last access per command buffer, so a later transition names an accurate source instead of the buffer's static usage superset. On first touch it readsNativeAccessMaskandNativePipelineStageMaskas a conservative source, but it never writes them. The copy and upload paths need those to stay a superset of every legal usage.3. Swapchain images were transitioned before acquisition
CreateBackBufferstransitioned every swapchain image to Present and submitted that command buffer before the first acquisition. Vulkan permits use of a presentable image only betweenvkAcquireNextImageKHRandvkQueuePresentKHR, and a layout transition is such a use. Validation reported the violation once per image per swapchain creation.The transition existed to give the tracked layout a valid source.
AcquireNextImagenow sets that state toUndefined, which is correct because the contents of a newly acquired image are undefined. This also removes avkQueueWaitIdlefrom every swapchain creation and resize.This defect needs a real swapchain, so no headless test reaches it. The interactive test runner found it, and the same runner confirms the fix.
Opt-in synchronization validation
STRIDE_VULKAN_SYNC_VALIDATION=1enablesVK_EXT_validation_featureswithSynchronizationValidation. Core validation does not report a missing barrier. This does, and it is what found the first defect above.The switch is opt-in for a reason. Synchronization validation also reports hazards that predate any given change, which would fail unrelated tests. Its extension comes from the validation layer rather than the ICD, so an instance extension query made with no layer name never lists it.
Tests
TestBufferBarrieris new. A compute shader writes a structured buffer, and a second dispatch reads that buffer as a shader resource.The consumer is a dispatch rather than a readback on purpose. A readback copy emits its own barrier from the buffer's access and stage masks. That barrier would synchronize the write even with no transition present, and the test could never fail.
The honest limit of this test: it proves the barrier is emitted and legal, and that the round trip produces the expected values. It does not prove that removing the barrier fails deterministically, because the hazards synchronization validation catches are reported at
vkQueueSubmittime and across command buffers. Run the suite withSTRIDE_VULKAN_SYNC_VALIDATION=1to have Vulkan report the hazard directly.Verification
Debug build, so the validation layers load.
Stride.Graphics.Tests.11_0(7 tests)Stride.Graphics.Tests.10_0(48 tests)Both suites also run clean on Vulkan with
STRIDE_VULKAN_SYNC_VALIDATION=1and report zero hazards. The default path, with the switch unset, is unchanged.The interactive test runner confirms defect 3. Before the change it logged the acquisition error once per image per swapchain, including after a window resize, which recreates the swapchain. After the change the log is clean and the preview renders correctly.
Direct3D is unaffected by design. Every behavioral change is inside the Vulkan backend.
BarrierLayout.csgains documentation only.One caveat, carried over from #3311:
TestHammersleyon Direct3D12 failed twice during this work, both times on the first Direct3D12 run after another backend had run. It then passed 8 consecutive full-suite runs and a run against cleared shader caches, so the trigger is not identified. This PR changes no Direct3D code.Related Issue
Types of changes
Checklist