fix: synchronise Vulkan attachment reads, buffer barriers and swapchain acquisition

sources/engine/Stride.Graphics.Tests.11_0/Assets/BufferBarrierReadTestShader.sdsl

@@ -0,0 +1,19 @@
+// Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net)
+// Distributed under the MIT license. See the LICENSE.md file in the project root for more information.
+
+namespace Stride.Graphics.Tests
+{
+    /// <summary>
+    /// Reads a structured buffer written by a previous dispatch and increments each element.
+    /// </summary>
+    internal shader BufferBarrierReadTestShader : ComputeShaderBase
+    {
+        stage StructuredBuffer<uint> Input;
+        stage RWStructuredBuffer<uint> Result;
+
+        override void Compute()
+        {
+            Result[streams.DispatchThreadId.x] = Input[streams.DispatchThreadId.x] + 1;
+        }
+    };
+}

sources/engine/Stride.Graphics.Tests.11_0/Assets/BufferBarrierTestShader.sdsl

@@ -0,0 +1,18 @@
+// Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net)
+// Distributed under the MIT license. See the LICENSE.md file in the project root for more information.
+
+namespace Stride.Graphics.Tests
+{
+    /// <summary>
+    /// Writes a value derived from each thread's dispatch index into a structured buffer.
+    /// </summary>
+    internal shader BufferBarrierTestShader : ComputeShaderBase
+    {
+        stage RWStructuredBuffer<uint> Output;
+
+        override void Compute()
+        {
+            Output[streams.DispatchThreadId.x] = streams.DispatchThreadId.x * 3;
+        }
+    };
+}

sources/engine/Stride.Graphics.Tests.11_0/TestBufferBarrier.cs

@@ -0,0 +1,94 @@
+// Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net)
+// Distributed under the MIT license. See the LICENSE.md file in the project root for more information.
+
+using System.Threading.Tasks;
+
+using Xunit;
+
+using Stride.Core;
+using Stride.Core.Mathematics;
+using Stride.Rendering;
+using Stride.Rendering.ComputeEffect;
+
+namespace Stride.Graphics.Tests;
+
+/// <summary>
+/// Covers a compute write handed to a second dispatch that reads it as a shader resource.
+/// </summary>
+/// <remarks>
+/// The consumer is a dispatch rather than a readback: a readback copy emits its own barrier from
+/// the buffer's access and stage masks, which would synchronise the write even with no transition
+/// and leave the test unable to fail. Run with STRIDE_VULKAN_SYNC_VALIDATION=1 to have Vulkan
+/// report the hazard when the transition is missing.
+/// </remarks>
+public class TestBufferBarrier : GraphicTestGameBase
+{
+    private const int ElementCount = 64;
+
+    private ComputeEffectShader writeEffect;
+    private ComputeEffectShader readEffect;
+    private Buffer sharedBuffer;
+    private Buffer resultBuffer;
+
+    protected override void RegisterTests()
+    {
+        base.RegisterTests();
+
+        FrameGameSystem.Draw(ComputeWriteIsVisibleToASubsequentDispatch);
+    }
+
+    protected override async Task LoadContent()
+    {
+        await base.LoadContent();
+
+        sharedBuffer = Buffer.Structured.New<uint>(GraphicsDevice, ElementCount, unorderedAccess: true).DisposeBy(this);
+        resultBuffer = Buffer.Structured.New<uint>(GraphicsDevice, ElementCount, unorderedAccess: true).DisposeBy(this);
+
+        var renderContext = RenderContext.GetShared(Services);
+        writeEffect = new ComputeEffectShader(renderContext)
+        {
+            ShaderSourceName = "BufferBarrierTestShader",
+            ThreadNumbers = new Int3(ElementCount, 1, 1),
+            ThreadGroupCounts = new Int3(1, 1, 1),
+        };
+        writeEffect.DisposeBy(this);
+
+        readEffect = new ComputeEffectShader(renderContext)
+        {
+            ShaderSourceName = "BufferBarrierReadTestShader",
+            ThreadNumbers = new Int3(ElementCount, 1, 1),
+            ThreadGroupCounts = new Int3(1, 1, 1),
+        };
+        readEffect.DisposeBy(this);
+    }
+
+    private void ComputeWriteIsVisibleToASubsequentDispatch()
+    {
+        var commandList = GraphicsContext.CommandList;
+        var renderDrawContext = new RenderDrawContext(Services, RenderContext.GetShared(Services), GraphicsContext);
+
+        commandList.ResourceBarrierTransition(sharedBuffer, BarrierLayout.UnorderedAccess);
+        writeEffect.Parameters.Set(BufferBarrierTestShaderKeys.Output, sharedBuffer);
+        ((RendererBase)writeEffect).Draw(renderDrawContext);
+
+        commandList.ResourceBarrierTransition(sharedBuffer, BarrierLayout.ShaderResource);
+        commandList.ResourceBarrierTransition(resultBuffer, BarrierLayout.UnorderedAccess);
+        readEffect.Parameters.Set(BufferBarrierReadTestShaderKeys.Input, sharedBuffer);
+        readEffect.Parameters.Set(BufferBarrierReadTestShaderKeys.Result, resultBuffer);
+        ((RendererBase)readEffect).Draw(renderDrawContext);
+
+        var values = resultBuffer.GetData<uint>(commandList);
+
+        Assert.Equal(ElementCount, values.Length);
+        for (uint i = 0; i < ElementCount; i++)
+        {
+            Assert.Equal((i * 3) + 1, values[i]);
+        }
+    }
+
+    [SkippableFact]
+    public void ComputeWriteIsVisibleToASubsequentRead()
+    {
+        RunGameTest(new TestBufferBarrier());
+    }
+}

sources/engine/Stride.Graphics/BarrierLayout.cs

@@ -20,12 +20,14 @@ public enum BarrierLayout
     Common,
 
     /// <summary>
-    ///   The resource is used as a render target.
+    ///   The resource is used as a render target. Covers reads as well as writes, because blending
+    ///   and a load operation that preserves the existing contents both read the attachment.
     /// </summary>
     RenderTarget,
 
     /// <summary>
-    ///   The resource is used as a writable depth-stencil buffer.
+    ///   The resource is used as a writable depth-stencil buffer. Covers reads as well as writes,
+    ///   because the depth and stencil tests read the attachment.
     /// </summary>
     DepthStencilWrite,
 

sources/engine/Stride.Graphics/Vulkan/BarrierMapping.Vulkan.cs

@@ -54,8 +54,8 @@ internal static class BarrierMapping
     /// </summary>
     internal static VkAccessFlags ToVkAccessFlags(BarrierLayout layout) => layout switch
     {
-        BarrierLayout.RenderTarget => VkAccessFlags.ColorAttachmentWrite,
-        BarrierLayout.DepthStencilWrite => VkAccessFlags.DepthStencilAttachmentWrite,
+        BarrierLayout.RenderTarget => VkAccessFlags.ColorAttachmentWrite | VkAccessFlags.ColorAttachmentRead,
+        BarrierLayout.DepthStencilWrite => VkAccessFlags.DepthStencilAttachmentWrite | VkAccessFlags.DepthStencilAttachmentRead,
         BarrierLayout.DepthStencilRead => VkAccessFlags.DepthStencilAttachmentRead,
         BarrierLayout.ShaderResource => VkAccessFlags.ShaderRead,
         BarrierLayout.UnorderedAccess => VkAccessFlags.ShaderRead | VkAccessFlags.ShaderWrite,

sources/engine/Stride.Graphics/Vulkan/CommandList.Vulkan.cs

@@ -36,6 +36,10 @@ public partial class CommandList
         // that's a known limitation to be addressed by adding a "last-submitted layout" tracker.
         private readonly Dictionary<Texture, BarrierLayout> currentCbLayouts = new();
 
+        // Buffers have no Vulkan layout: this records the last access THIS CB synchronised against, so
+        // a later transition can name an accurate source instead of the buffer's static usage superset.
+        private readonly Dictionary<Buffer, BarrierLayout> currentCbBufferLayouts = new();
+
         private readonly Dictionary<FramebufferKey, VkFramebuffer> framebuffers = new();
         private readonly VkImageView[] framebufferAttachments = new VkImageView[9];
         private int framebufferAttachmentCount;
@@ -87,6 +91,7 @@ public unsafe partial void Reset()
             CleanupRenderPass();
             boundDescriptorSets.Clear();
             currentCbLayouts.Clear();
+            currentCbBufferLayouts.Clear();
 
             framebuffers.Clear();
             framebufferDirty = true;
@@ -635,6 +640,45 @@ public unsafe void ResourceBarrierTransition(GraphicsResource resource, BarrierL
                 var memoryBarrier = new VkImageMemoryBarrier(texture.NativeImage, new VkImageSubresourceRange(texture.NativeImageAspect, 0, uint.MaxValue, 0, uint.MaxValue), oldAccessMask, newAccessMask, oldLayout, newVkLayout);
                 GraphicsDevice.NativeDeviceApi.vkCmdPipelineBarrier(currentCommandList.NativeCommandBuffer, sourceStages, newStages, VkDependencyFlags.None, 0, null, 0, null, 1, &memoryBarrier);
             }
+            else if (resource is Buffer buffer)
+            {
+                VkAccessFlags oldAccessMask;
+                VkPipelineStageFlags sourceStages;
+                if (currentCbBufferLayouts.TryGetValue(buffer, out var fromLayout))
+                {
+                    if (fromLayout == newLayout)
+                        return; // already at target in this CB
+
+                    oldAccessMask = BarrierMapping.ToVkAccessFlags(fromLayout);
+                    sourceStages = BarrierMapping.ToVkPipelineStageFlags(fromLayout);
+                }
+                else
+                {
+                    // NativeAccessMask and NativePipelineStageMask are the buffer's static set of every
+                    // legal usage, which the copy and upload paths read to build their restore barriers.
+                    // Use them as a conservative source here, but never write to them.
+                    oldAccessMask = buffer.NativeAccessMask;
+                    sourceStages = buffer.NativePipelineStageMask;
+                }
+
+                var newAccessMask = BarrierMapping.ToVkAccessFlags(newLayout);
+                var newStages = BarrierMapping.ToVkPipelineStageFlags(newLayout);
+
+                sourceStages = FixStagesForAccess(sourceStages, oldAccessMask);
+                newStages = FixStagesForAccess(newStages, newAccessMask);
+
+                if (sourceStages == VkPipelineStageFlags.None)
+                    sourceStages = VkPipelineStageFlags.TopOfPipe;
+                if (newStages == VkPipelineStageFlags.None)
+                    newStages = VkPipelineStageFlags.BottomOfPipe;
+
+                currentCbBufferLayouts[buffer] = newLayout;
+
+                CleanupRenderPass();
+
+                var bufferMemoryBarrier = new VkBufferMemoryBarrier(buffer.NativeBuffer, oldAccessMask, newAccessMask);
+                GraphicsDevice.NativeDeviceApi.vkCmdPipelineBarrier(currentCommandList.NativeCommandBuffer, sourceStages, newStages, VkDependencyFlags.None, 0, null, 1, &bufferMemoryBarrier, 0, null);
+            }
             else
             {
                 throw new NotImplementedException();

sources/engine/Stride.Graphics/Vulkan/GraphicsAdapterFactory.Vulkan.cs

@@ -206,9 +206,10 @@ public unsafe GraphicsAdapterFactoryInstance(bool enableValidation)
                 VK_KHR_XCB_SURFACE_EXTENSION_NAME,
                 VK_EXT_METAL_SURFACE_EXTENSION_NAME,
                 VK_KHR_PORTABILITY_ENUMERATION_EXTENSION_NAME,
-                VK_EXT_DEBUG_UTILS_EXTENSION_NAME
+                VK_EXT_DEBUG_UTILS_EXTENSION_NAME,
+                VK_EXT_VALIDATION_FEATURES_EXTENSION_NAME
             };
-            var supportedExtensions = new Span<VkUtf8String>(supportedExtensionNames, 8);
+            var supportedExtensions = new Span<VkUtf8String>(supportedExtensionNames, 9);
             var availableExtensionNames = GetAvailableExtensionNames(supportedExtensions);
             // Surface extensions are optional at instance creation (not available with headless ICDs).
             // They are validated later when a swapchain is actually created.
@@ -235,6 +236,15 @@ public unsafe GraphicsAdapterFactoryInstance(bool enableValidation)
                 desiredExtensionNames.Add(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
             HasDebugUtilsSupport = enableDebugUtils;
 
+            // Synchronization validation reports missing barriers, which core validation does not. It is
+            // opt-in because it also reports hazards predating any given change, failing unrelated tests.
+            // Its extension comes from the validation layer rather than the ICD, so it is absent from
+            // availableExtensionNames, which queries instance extensions with no layer name.
+            bool enableSyncValidation = enableValidation
+                && Environment.GetEnvironmentVariable("STRIDE_VULKAN_SYNC_VALIDATION") == "1";
+            if (enableSyncValidation)
+                desiredExtensionNames.Add(VK_EXT_VALIDATION_FEATURES_EXTENSION_NAME);
+
             using VkStringArray ppEnabledLayerNames = new(enabledLayerNames);
             using VkStringArray ppEnabledExtensionNames = new(desiredExtensionNames);
 
@@ -249,6 +259,16 @@ public unsafe GraphicsAdapterFactoryInstance(bool enableValidation)
                 ppEnabledExtensionNames = ppEnabledExtensionNames,
             };
 
+            var syncValidationFeature = VkValidationFeatureEnableEXT.SynchronizationValidation;
+            var validationFeatures = new VkValidationFeaturesEXT
+            {
+                sType = VkStructureType.ValidationFeaturesEXT,
+                enabledValidationFeatureCount = 1,
+                pEnabledValidationFeatures = &syncValidationFeature,
+            };
+            if (enableSyncValidation)
+                instanceCreateInfo.pNext = &validationFeatures;
+
             // Silence MoltenVK's per-instance info dump (153-line extension list, device banner).
             // Set via env var instead of VkLayerSettingsCreateInfoEXT — the layer-settings struct
             // requires VK_EXT_layer_settings to be enabled, and MoltenVK acting as ICD without the

sources/engine/Stride.Graphics/Vulkan/SwapChainGraphicsPresenter.Vulkan.cs

@@ -232,6 +232,14 @@ private unsafe void AcquireNextImage()
             // Flip render targets
             backBuffer.SetNativeHandles(swapchainImages[currentBufferIndex].NativeImage, swapchainImages[currentBufferIndex].NativeColorAttachmentView);
 
+            // The contents of a freshly acquired image are undefined, so the first transition of the
+            // frame starts from Undefined. Seeding the images to Present at creation instead would
+            // transition them before they are acquired, which the specification forbids.
+            backBuffer.NativeLayout = VkImageLayout.Undefined;
+            backBuffer.NativeAccessMask = VkAccessFlags.None;
+            backBuffer.NativePipelineStageMask = VkPipelineStageFlags.TopOfPipe;
+            backBuffer.LayoutTracker.Set(uint.MaxValue, BarrierLayout.Undefined);
+
             lock (GraphicsDevice.QueueLock)
             {
                 // Signal vkAcquireNextImageKHR Fence => GraphicsDevice.CommandList (so that next command list will wait for this to complete)
@@ -637,25 +645,6 @@ private unsafe void CreateBackBuffers()
                 viewType = VkImageViewType.Image2D,
             };
 
-            // We initialize swapchain images to PresentSource, since we swap them out while in this layout.
-            backBuffer.NativeAccessMask = VkAccessFlags.MemoryRead;
-            backBuffer.NativeLayout = VkImageLayout.PresentSrcKHR;
-
-            var imageMemoryBarrier = new VkImageMemoryBarrier
-            {
-                sType = VkStructureType.ImageMemoryBarrier,
-                subresourceRange = new VkImageSubresourceRange(VkImageAspectFlags.Color, 0, 1, 0, 1),
-                oldLayout = VkImageLayout.Undefined,
-                newLayout = VkImageLayout.PresentSrcKHR,
-                srcAccessMask = VkAccessFlags.None,
-                dstAccessMask = VkAccessFlags.MemoryRead
-            };
-
-            var commandBuffer = GraphicsDevice.NativeCopyCommandPools.Value.GetObject(0);
-
-            var beginInfo = new VkCommandBufferBeginInfo { sType = VkStructureType.CommandBufferBeginInfo };
-            GraphicsDevice.NativeDeviceApi.vkBeginCommandBuffer(commandBuffer, &beginInfo);
-
             GraphicsDevice.NativeDeviceApi.vkGetSwapchainImagesKHR(GraphicsDevice.NativeDevice, swapChain, out uint swapchainImageCount);
             Span<VkImage> buffers = stackalloc VkImage[(int)swapchainImageCount];
             GraphicsDevice.NativeDeviceApi.vkGetSwapchainImagesKHR(GraphicsDevice.NativeDevice, swapChain, buffers);
@@ -666,29 +655,8 @@ private unsafe void CreateBackBuffers()
                 // Create image views
                 swapchainImages[index].NativeImage = createInfo.image = buffers[index];
                 GraphicsDevice.CheckResult(GraphicsDevice.NativeDeviceApi.vkCreateImageView(GraphicsDevice.NativeDevice, &createInfo, null, out swapchainImages[index].NativeColorAttachmentView));
-
-                // Transition to default layout
-                imageMemoryBarrier.image = buffers[index];
-                GraphicsDevice.NativeDeviceApi.vkCmdPipelineBarrier(commandBuffer, VkPipelineStageFlags.AllCommands, VkPipelineStageFlags.AllCommands, VkDependencyFlags.None, 0, null, 0, null, 1, &imageMemoryBarrier);
-            }
-
-            // Close and submit
-            GraphicsDevice.CheckResult(GraphicsDevice.NativeDeviceApi.vkEndCommandBuffer(commandBuffer));
-
-            lock (GraphicsDevice.QueueLock)
-            {
-                var submitInfo = new VkSubmitInfo
-                {
-                    sType = VkStructureType.SubmitInfo,
-                    commandBufferCount = 1,
-                    pCommandBuffers = &commandBuffer,
-                };
-                GraphicsDevice.CheckResult(GraphicsDevice.NativeDeviceApi.vkQueueSubmit(GraphicsDevice.NativeCommandQueue, 1, &submitInfo, VkFence.Null));
-                GraphicsDevice.CheckResult(GraphicsDevice.NativeDeviceApi.vkQueueWaitIdle(GraphicsDevice.NativeCommandQueue));
             }
 
-            GraphicsDevice.NativeCopyCommandPools.Value.RecycleObject(0, commandBuffer);
-
             // Create submit semaphores
             submitSemaphores = new VkSemaphore[buffers.Length];
             var semaphoreCreateInfo = new VkSemaphoreCreateInfo { sType = VkStructureType.SemaphoreCreateInfo };

sources/engine/Stride.Graphics/Vulkan/Texture.Vulkan.cs

@@ -171,18 +171,18 @@ private partial void InitializeFromImpl(DataBox[] dataBoxes = null)
                     NativeAccessMask = VkAccessFlags.TransferRead;
 
                 if (NativeLayout == VkImageLayout.ColorAttachmentOptimal)
-                    NativeAccessMask = VkAccessFlags.ColorAttachmentWrite;
+                    NativeAccessMask = VkAccessFlags.ColorAttachmentWrite | VkAccessFlags.ColorAttachmentRead;
 
                 if (NativeLayout == VkImageLayout.DepthStencilAttachmentOptimal)
-                    NativeAccessMask = VkAccessFlags.DepthStencilAttachmentWrite;
+                    NativeAccessMask = VkAccessFlags.DepthStencilAttachmentWrite | VkAccessFlags.DepthStencilAttachmentRead;
 
                 if (NativeLayout == VkImageLayout.ShaderReadOnlyOptimal)
                     NativeAccessMask = VkAccessFlags.ShaderRead | VkAccessFlags.InputAttachmentRead;
 
                 NativePipelineStageMask =
                     IsRenderTarget ? VkPipelineStageFlags.ColorAttachmentOutput :
                     IsDepthStencil ? VkPipelineStageFlags.ColorAttachmentOutput | VkPipelineStageFlags.EarlyFragmentTests | VkPipelineStageFlags.LateFragmentTests :
-                    IsShaderResource || IsUnorderedAccess ? VkPipelineStageFlags.VertexInput | VkPipelineStageFlags.FragmentShader | VkPipelineStageFlags.ComputeShader :
+                    IsShaderResource || IsUnorderedAccess ? VkPipelineStageFlags.VertexShader | VkPipelineStageFlags.FragmentShader | VkPipelineStageFlags.ComputeShader :
                     VkPipelineStageFlags.None;
 
                 if (ParentTexture != null)