Fix missing _THREAD_SAFE_METHOD_ missing from RenderingDeviceVulkan submit and sync

[Zylann]

I use RenderingDeviceVulkan from multiple threads. While one thread is dedicated to execute compute lists, submit and sync, other threads can create or update resources, and that caused crashes inside the Vulkan driver.

Turns out submit and sync did not have _THREAD_SAFE_METHOD_. Adding them made the crashes go away.

---

While working on this, I also found out this call to _buffer_update has a problem:

godot/drivers/vulkan/rendering_device_vulkan.cpp

Line 5867 in a758388

Error err = _buffer_update(buffer, p_offset, (uint8_t *)p_data, p_size, p_post_barrier);

The last sent argument is p_post_barrier, which is a BitField<BarrierMask>, but then the signature of _buffer_update is:

godot/drivers/vulkan/rendering_device_vulkan.h

Line 227 in a758388

Error _buffer_update(Buffer *p_buffer, size_t p_offset, const uint8_t *p_data, size_t p_data_size, bool p_use_draw_command_buffer = false, uint32_t p_required_align = 32);

So p_post_barrier ends up being fed as the p_use_draw_command_buffer parameter, which doesn't seem to be the same meaning.
I don't know if there is a bug associated with this, but it doesn't seems right. I don't know what the code should be though.

cc @RandomShaper

[Zylann]

For some reason Github sees 18 commits... I wonder what I did different between this PR and other PRs I did in the past Oo

[YuriSizov]

@Zylann In that graph it shows that your commit is based on your master branch, but your master branch is not in sync with the upstream master. Instead it has merge commits from the upstream master. Merge commits are new commits. So your master branch is now 17 commits ahead of the upstream master because it has 17 commits that the upstream master doesn't have.

You need to rebase your master instead of merging upstream into it. Or just make (rebase) your feature branch from the upstream master directly.

[clayjohn]

The code looks fine to me. The commits will need to be cleaned up before merging though

[clayjohn]

While working on this, I also found out this call to _buffer_update has a problem:
godot/drivers/vulkan/rendering_device_vulkan.cpp

Line 5867 in a758388

Error err = _buffer_update(buffer, p_offset, (uint8_t *)p_data, p_size, p_post_barrier);

The last sent argument is p_post_barrier, which is a BitField, but then the signature of _buffer_update is:
[godot/drivers/vulkan/rendering_device_vulkan.h]
(

godot/drivers/vulkan/rendering_device_vulkan.h

Line 227 in a758388

Error _buffer_update(Buffer *p_buffer, size_t p_offset, const uint8_t *p_data, size_t p_data_size, bool p_use_draw_command_buffer = false, uint32_t p_required_align = 32);

)

Line 227 in a758388

Error _buffer_update(Buffer *p_buffer, size_t p_offset, const uint8_t *p_data, size_t p_data_size, bool p_use_draw_command_buffer = false, uint32_t p_required_align = 32);

So p_post_barrier ends up being fed as the p_use_draw_command_buffer parameter, which doesn't seem to be the same meaning.

This line has come up a few times. It looks really awkward, but it technically functions as intended. That being said, we do need to clean up the buffer_update function as it now mostly defaults to using the draw_command_buffer which is really bad for performance on mobile

[Zylann]

your master branch is not in sync with the upstream master

It is? (well, apparently not I guess, but it acts like it is) Everytime I do a PR I git pull upstream master in my master branch, I don't know of any other way to sync a fork. I always did that in my previous PRs (also did the same in this one)... hence my confusion about what has happened here, because that didn't happen in my last 17 PRs.

Instead it has merge commits from the upstream master. Merge commits are new commits

This is where I'm very confused, because I intend this fork to be a "clean fork", it only exists to do PRs. I don't modify anything in my master branch, it is meant to reflect Godot's master branch (therefore there is never anything to "merge". I create a branches for each PR). Maybe I've used the wrong command this time, but I dont know which one I did wrong, and not 17 times^^"

I don't know how I should rebase my master branch. git rebase upstream master returns fatal: invalid upstream 'upstream'.
I tried git rebase master inside the PR branch, and nothing happened. I'm quite stuck.

[YuriSizov]

@Zylann I'm not sure this is the correct syntax for git rebase with a remote. Try git pull --rebase upstream master or using git rebase <commit_hash_for_upstream_head> here.

Edit: There is also this git rebase upstream/master syntax that I can find.

[Zylann]

That seems to have done the trick, thanks!

[YuriSizov]

Thanks!