dynamic writes to uniform buffers are not synchronized

[timoore]

I'm implementing an LOD fade effect in vsgCs. A fade in/out flag and value in a tile's uniform buffer data is used to threshold against a blue noise texture map. Unfortunately there is a visible flicker when the transfer begins. I haven't been able to capture the flickering frame in RenderDoc; I think the problem is hidden by the recording layer. But, I'm pretty sure that the issue is a data race in the fade values.

The Vulkan sychronization layer produces a spew of errors like:

        [0] 0x220dc70, type: 4, name: NULL
SYNC-HAZARD-WRITE-RACING-READ(ERROR / SPEC): msgNum: -860391127 - Validation Error: [ SYNC-HAZARD-WRITE-RACING-READ ] Object 0: handle = 0x220dc70, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0xccb77929 | vkQueueSubmit: Hazard WRITE_RACING_READ for entry 0, VkCommandBuffer 0x28c6a30[], Recorded access info (recorded_usage: SYNC_COPY_TRANSFER_WRITE, command: vkCmdCopyBuffer, seq_no: 5, reset_no: 1453). Access info (prior_usage: SYNC_FRAGMENT_SHADER_UNIFORM_READ, read_barriers: VK_PIPELINE_STAGE_2_BOTTOM_OF_PIPE_BIT, queue: VkQueue 0x2108000[], submit: 4483, batch: 0, batch_tag: 70343, command: vkCmdDrawIndexed, seq_no: 18, command_buffer: VkCommandBuffer 0x2729650[], reset_no: 1493).
    Objects: 1
        [0] 0x220dc70, type: 4, name: NULL

The source of the bug is pretty obvious (to me 😎 ). In src/vsg/TransferTask.cpp:

        submitInfo.waitSemaphoreCount = 0;
        submitInfo.pWaitSemaphores = nullptr;
        submitInfo.pWaitDstStageMask = nullptr;

The transfer task submission does not wait for the rendering on the graphics queue to complete. The buffers in question are single buffered, so this submission should wait for the the relevant stages to complete.

[robertosfield]

Could you provide links to the relevant code and instructions how to reproduce the error so I can run tests myself and see the code to understand the usage case fully.

[timoore]

The vsgCs bug is evident in the lod-fade branch of vsgCs. No special run-time flags are necessary.

You will probably find it faster to see the problem in vsgExamples using the Vulkan SDK synchronization layer. I used vkconfig to enable synchronization testing for all Vulkan programs, and turned off the option to limit duplicate debug messages. vsgdynamicvertex --modify DamagedHelmet.gltf produces a pretty good spew of messages.

[timoore]

I've updated the lod-fade branch of vsgCs with @robertosfield 's PR to make vsgCs compile and run with vsgCs master. The bug doesn't appear on the hardware I have with me, AMD Radeon Graphics (RADV RENOIR), which is an integrated GPU, nor are there any complaints from the Vulkan synchronization layer. This implementation has only one graphics-capable queue, so it's not surprising that synchronization errors go away.

Given that an important class of hardware (i.e., hardware I own 😎 ) has only one graphics-capable queue, it would be nice sometime to revisit #786; that PR allowed compile traversals to make progress and not block when submitting the compile-generated commands to the main graphics queue.

[robertosfield]

Following @timoore suggestion about the use of a dedicated transfer queue for the TransferTask causing problems I have disabled this in the Viewer::compile(..) implementation by simply disabling the set up of the transfer queue and low and this fixes the visuals with vsgshadow and quietens the synchronization error reported when running vkconfig.

The change is:

$ git diff
diff --git a/src/vsg/app/Viewer.cpp b/src/vsg/app/Viewer.cpp
index 987e7527..9a7332d6 100644
--- a/src/vsg/app/Viewer.cpp
+++ b/src/vsg/app/Viewer.cpp
@@ -491,6 +491,7 @@ void Viewer::assignRecordAndSubmitTaskAndPresentation(CommandGraphs in_commandGr
         ref_ptr<Queue> transferQueue = mainQueue;
 
         VkQueueFlags transferQueueFlags = VK_QUEUE_TRANSFER_BIT | VK_QUEUE_GRAPHICS_BIT; // use VK_QUEUE_GRAPHICS_BIT to ensure we can blit images
+#if 0
         for (auto& queue : device->getQueues())
         {
             if ((queue->queueFlags() & transferQueueFlags) == transferQueueFlags)
@@ -502,6 +503,7 @@ void Viewer::assignRecordAndSubmitTaskAndPresentation(CommandGraphs in_commandGr
                 }
             }
         }
+#endif
 
         if (deviceQueueFamily.presentFamily >= 0)
         {

This isn't a proper fix, but it does look to confirm Tim's suggestion that this is an issue, and may solve the NVidia shadowing issue completely.

What I'll do this morning is review the TransferTask implementation, looking at adding a synchronization primitive to the start of the transfer command buffer submission so it doesn't overwrite the data on the GPU before the previous frame has finished.

[timoore]

I've thought a bit about this issue, and here are some of those random thoughts, with accompanying food for them 😄

• Supporting multiple queues is nice, but the critical issue for me is allowing multiple compile traversals to make progress without blocking. If a compile travesal thread blocks and goes to sleep, that causes real visible latency in database paging. I attacked this for a single command queue in Nonblocking compile #786 in a rather complicated way. The transfer queue does help on NVidia hardware, but given that transfer requires a graphics-capable queue today, we're back to where we started with most integrated GPUs which seem to have a single graphics queue.
• Timeline semaphores are a very powerful new abstraction in Vulkan. This NVidia sample shows a good way to synchronize work in multiple queues. It would generalize to compute queues too. Timeline semaphores require an extension or Vulkan 1.2, but there is an official layer that emulates them. Where even that is not available, I suggest that falling back to a single queue is totally reasonable, as this is likely to be on mobile / embedded hardware that probably only has a single queue. If there are any users of such hardware, please speak up.
• I note that semaphores aren't required at all for synchronization within a single queue. If there is only one command queue available, then they only need to be used to synchronize presentation.

[robertosfield]

Thanks for the links/suggestions. Something for me to read up about, though for the transfer task problem I think I have solution very close at hand just using existing semaphores that the viewer managers.

[robertosfield]

As I have mentioned in Shadow jitter Issue I have a partial solution for the TransferTask synchronization problem - it fixes the issue of the TransferTask task starting before it should, but results in Vulkan debug error with semaphores that are wired up but never waited upon.

I don't know if timeline semaphores might be a solution to this problem, even if they aren't I think it would be good to implement then in vsg::Semaphore. I am wary of heading off on tangents that may or may not help resolve the current problem though so would rather fix the problem with standard binary vk/vsg::Semaphore than later implement the timeline Semaphore.

[timoore]

Would you accept a PR for the VSG representation of timeline semaphores, checking the feature or extension, etc?

[robertosfield]

If you want to tackle it please go ahead. The ability to compile against Vulkan 1.1 is required, so extension specs would need to be added as a fallback to include/vsg/vk/vulkan.h as there other extensions are done.

[robertosfield]

I have now merged the changes to TransferTask/RecordAndSubmitTask that add use Semaphore to sync the start of transfer of data with the completion of the previous frames rendering. The fixes are merged with PR #1033.

@timoore could you test VSG master out with your version of vsgCs that was triggering problems?

[timoore]

Looking good here on an NVidia GTX 1070. My blue-noise LOD transition is pretty sweet.

Thanks!

[timoore]

If you want to tackle it please go ahead. The ability to compile against Vulkan 1.1 is required, so extension specs would need to be added as a fallback to include/vsg/vk/vulkan.h as there other extensions are done.

Do you mean compile against a 1.1.x version of the Vulkan SDK, or be able to run on a 1.1 target? I'm not sure where I'd be able to find such an ancient version... well OK, I'm sure it's possible.

I've discovered all the version tests / extension definitions in vk/vulkan.h, so I guess you really do want to support compiling against SDK 1.1.70.0, but... yuck. I can add the needed definitions to that file.