vk_updateDescriptorSet is not thread safe

[yaoyao-cn]

pDescriptorData member of pDescriptorSet->mVulkan make vk_updateDescriptorSet not thead safe see: https://github.com/yaoyao-cn/The-Forge/commit/c44d6cdeb04273531b9850d9a7cd37c8a7bdf18c

i found this while using multi-thread command recording with each thread may update some sub index of a large(mMaxSets is large) descriptor set. because all thead use the same pDescriptorData when update texture, all my mesh with texture will flicker like this:

https://user-images.githubusercontent.com/10138823/172975573-aa66a75e-3fea-45d1-a502-c27370da1487.mp4

[wolfgangfengel]

Just saw this. We will look into this. Is that game running on an Android phone?

[yaoyao-cn]

No, it's running on windows 10, the video you see is recorded by my phone

[manas-kulkarni]

DescriptorSet (like all other mutable objects - Cmd, CmdPool, ...) is not thread-safe when used for writing. You will need one DescriptorSet per thread if you want to update them on separate threads. We could potentially solve it for Vulkan by using a stack array for pDescriptorData. But Metal has a limitation here with argument encoders (solved in Metal 3. Still a long way before the devices using it become a majority)

[yaoyao-cn]

Yes, you are right, I need VkDescriptorSet per thread The problem is: the VkDescriptorSet is stored in The-Forge::DescriptorSet. I update VkDescriptorSet througt The-Forge::DescriptorSet in each command-record thread. it is the pDescriptorData shared by all thread cause the problems

[manas-kulkarni]

No. I meant the DescriptorSet Forge object. It can be written to by only one thread at any point irrespective of what index you pass to updateDescriptorSet. Can make it thread safe by using a stack array but the limitation will stay in Metal as it is limited by the argument encoder API

[yaoyao-cn]

https://developer.nvidia.com/sites/default/files/akamai/gameworks/blog/munich/mschott_vulkan_multi_threading.pdf I am not familiar with Metal, is it possible to use a mutex to achieve thread safe for Metal

[manas-kulkarni]

Metal - Using a mutex would defeat the purpose of multi-threading. But we will add the ability to update the same DescriptorSet from different threads on Vulkan (assuming you use a different index for updateDescriptorSet for each thread) Next release

[yaoyao-cn]

solved in the new release