Vulkan swapchain semaphore reuse issue

[attilaz]

We get validation errors like this with latest Vulkan SDK 1.4.335.0:

D:\Development\github\bgfx\src\renderer_vk.cpp (740): BGFX ---E-           Semaphore, Validation, 0: vkQueueSubmit(): pSubmits[0].pSignalSemaphores[0] (VkSemaphore 0x1d000000001d[m_renderDoneSemaphore[7]]) is being signaled by VkQueue 0x2057f574c30, but it may still be in use by VkSwapchainKHR 0x90000000009[m_swapChain].
Most recently acquired image indices: 0, 0, 0, 0, 0, 0, 0, 0.
(Brackets mark the last use of VkSemaphore 0x1d000000001d[m_renderDoneSemaphore[7]] in a presentation operation.)
Swapchain image 1 was presented but was not re-acquired, so VkSemaphore 0x1d000000001d[m_renderDoneSemaphore[7]] may still be in use and cannot be safely reused with image index 0.
Vulkan insight: See https://docs.vulkan.org/guide/latest/swapchain_semaphore_reuse.html for details on swapchain semaphore reuse. Examples of possible approaches:
   a) Use a separate semaphore per swapchain image. Index these semaphores using the index of the acquired image.
   b) Consider the VK_KHR_swapchain_maintenance1 extension. It allows using a VkFence with the presentation operation.
The Vulkan spec states: Each binary semaphore element of the pSignalSemaphores member of any element of pSubmits must be unsignaled when the semaphore signal operation it defines is executed on the device (https://docs.vulkan.org/spec/latest/chapters/cmdbuffers.html#VUID-vkQueueSubmit-pSignalSemaphores-00067)

I havent't seen this with old sdk, so don't know if this is a false positive or a new validation check.
We get this with our current project, and I can reproduce this with 2 bgfx examples on my machine.
It is somehow related to gpu frametime, maybe when gpu framerate suddenly changes?

In the examples.exe
05-instancing: move the slider grid side size. and the reportcb triggers.
26-occlusion: I simply start this example with combobox, and it triggers error.

This is on windows11 with nvidia geforce gtx 1650, with latest driver (32.0.15.9186)

[attilaz]

https://docs.vulkan.org/guide/latest/swapchain_semaphore_reuse.html
According to this it is a new check

Since Vulkan SDK 1.4.313, the validation layer reports cases where the present wait semaphore is not used safely.

A common mistake is applying a buffering scheme based on the number of frames in flight (such as double or triple buffering) to present wait semaphores.

I think something similar happening here

bgfx/src/renderer_vk.cpp

Lines 8169 to 8172 in 78c3a1f

	m_lastImageAcquiredSemaphore = m_presentDoneSemaphore[m_currentSemaphore];
	m_lastImageRenderedSemaphore = m_renderDoneSemaphore[m_currentSemaphore];
	m_currentSemaphore = (m_currentSemaphore + 1) % kMaxBackBuffers;

It is very simple to fix the above code by doing:

1. Allocate the submit_semaphores array based on the number of swapchain images (instead of the number of in-flight frames)
2. Index this array using the acquired swapchain image index (instead of the current in-flight frame index)

I think bgfx does 1. So that is not a problem.
But 2. is problematic. Bgfx does indexing of submit/signal semaphore with
m_currentSemaphore = (m_currentSemaphore + 1) % kMaxBackBuffers;
instead of the image index returned by vkAcquireNextImageKHR.

Also the "good example" on the web page uses kNumberOfFramesInFlight for acquire semaphores (instead of num_swapchain_images - which I think is kMaxBackBuffers in bgfx), but if kMaxBackBuffers is larger then maxFrameInFlight it is possibly not a problem.

I tried to fix this with

		m_lastImageAcquiredSemaphore = m_presentDoneSemaphore[m_currentSemaphore];
		m_currentSemaphore = (m_currentSemaphore + 1) % kMaxBackBuffers;

		VkResult result;
		{
			BGFX_PROFILER_SCOPE("vkAcquireNextImageKHR", kColorFrame);

			result = vkAcquireNextImageKHR(
				  device
				, m_swapChain
				, UINT64_MAX
				, m_lastImageAcquiredSemaphore
				, VK_NULL_HANDLE
				, &m_backBufferColorIdx
				);
		}

		m_lastImageRenderedSemaphore = m_renderDoneSemaphore[m_backBufferColorIdx];

I do what 2. says. I index image submit semaphore with acquired image index instead of the incrementing value (m_currentSemaphore). I get no validation error with the fix, and see no new problem in any of the examples or in our project.

[bkaradzic]

cc @pezcode

[mcourteaux]

I'm investigating. I have an occasional bad flicker in SilverNode. I'm wondering if this is the reason.

[mcourteaux]

I can't. The number of indirections, copies, and indices sitting in between bgfx's vulkan render backend and what's actually happening in what order is too high, and I just can't follow it. Mapping bgfx's code to the "good example" from the article is not trivial.