vulkan: Replace fence with semaphore when acquiring surfaces

[udoprog]

Connections
Different way of solving #4946 which uses internal wait semaphores.

#4689, #4775, #4919

Description
This removes the use of a fence in favor of internally using and keeping track of one wait semaphore per swapchain image.

It roughly uses the approach initially attempted at https://github.com/cwfitzgerald/wgpu/tree/vulkan-timing-fixes,

I've defined a generic set in A::SubmitSurfaceTextureSet which can be used to collect whatever information it needs from a surface texture, this is defined as a dummy implementation for backends which does not use it.

Multiple calls to get_current_texture

There is a tricky situation which arises if a user were to perform multiple calls to get_current_texture. The current implementation naively just rotates surface_semaphores. So if we call this function images.len() + 1 times in a row, we will exhaust the semaphores and they will be assigned to a random set of swapchain images by the presentation engine.

This wasn't a problem in #4946 since the user is responsible for ensuring that the number of pending get_current_texture matches the number of semaphores they have (e.g. exactly one per image), but here we have to deal with this scenario somehow.

I think the "correct" way to deal with this is to somehow free the semaphore after a successful call to acquire_next_image has returned a previously used index again. But I'm looking for input.

Testing
Using the vulkan backend for a project I'm working on and dealing with any issues that show up.

Checklist

• Run cargo fmt.
• Run cargo clippy.
• Run cargo xtask test to run tests (still doesn't work).
• Add change to CHANGELOG.md. See simple instructions inside file.

[ids1024]

I think this should also potentially perform better in some circumstances, by not unnecessarily blocking the CPU on a fence and instead just marking this as a dependency of the queue submission. Right? So this seems better in general.

[udoprog]

Looks like a legit validation failure, I'll try to suss it out before unmarking this as a draft:

[2024-01-04T08:20:08Z ERROR wgpu_test::expectations] Unexpected failure due to: ValidationError(Some("Validation Error: [ VUID-VkSubmitInfo-pSignalSemaphores-03242 ] Object 0: handle = 0xf56c9b0000000004, type = VK_OBJECT_TYPE_SEMAPHORE; Object 1: handle = 0x7f09e27c8fd0, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0xdb30ee87 | vkQueueSubmit(): pSubmits[0].pSignalSemaphores[0] signal value (0xffffffffffffffff) in VkQueue 0x7f09e27c8fd0[] must be greater than pending signal timeline semaphore VkSemaphore 0xf56c9b0000000004[] value (0xffffffffffffffff). The Vulkan spec states: For each element of pSignalSemaphores created with a VkSemaphoreType of VK_SEMAPHORE_TYPE_TIMELINE the corresponding element of VkTimelineSemaphoreSubmitInfo::pSignalSemaphoreValues must have a value greater than the current value of the semaphore when the semaphore signal operation is executed (https://vulkan.lunarg.com/doc/view/1.3.268.0/linux/1.3-extensions/vkspec.html#VUID-VkSubmitInfo-pSignalSemaphores-03242)"))

Seems like I wasn't careful when refactoring the timeline semaphores.

[valaphee]

Doesn't fix #4919 but the water example runs 600 fps faster (~1500 fps and now with ~2100 fps)

[udoprog]

That's both unfortunate that it doesn't address your problem but neat about the fps boost. Thanks for trying it out! Personally I can't compare since the unpatched wgpu with the fence doesn't work at all for me right now.

[cwfitzgerald]

@udoprog Haven't had a chance to fully look over the code, but going to send it out to a few people for testing in various environments - could you just resolve the merge conflicts with trunk?

[udoprog]

@cwfitzgerald All right, that should be it. Tell me if there's anything else.

[cwfitzgerald]

Hey! Sorry forgot to follow up on this - sent this to a bunch of people and things look good! Everything seems to work as expected on all platforms. It either has equivalent performance, or significantly higher performance.

Going to do another proper review in the next day or so and we'll get this in!

[cwfitzgerald]

Looks a lot better and performs well based on testing, lets get this out there!

[emilk]

@EmbersArc reports this PR causes 100% CPU usage:

• 100% CPU usage since the update to wgpu 0.20 emilk/egui#5092
• OS: Arch Linux 6.10.8-arch1-1 (64-bit)
• X11 with NVIDIA 560.35.03

[cwfitzgerald]

This got entirely rewritten in the 0.20.1 update (wgpu-hal 0.21), so it's unlikely to be this PR.

Replying upstream