Lots of Synchronization Validation Errors (specially with MSAA enabled) #81060
Comments
|
Maybe use vulkan's VK_VALIDATION_FEATURE_ENABLE_SYNCHRONIZATION_VALIDATION_EXT can also help finding this kind of issue too ? |
|
Hi!
Yes, the validation messages I posted were using that extension (but I used vkconfig to force it, rather than modifying Godot to set that bit). However this flag is often disabled because it has a severe performance impact, so it's only enabled on demand and off by default. Btw I forgot to mention on the ticket, I will be handling this bug. I wrote this ticket to explain the problem, order my thoughts, and allow anyone to offer feedback. |
Vulkan Synchronization is hard, I'm looking forward to learn from your PR :) |
This should be documented on Validation layers 🙂 I'm not sure if we should add one more CLI argument (we have a lot of them already, and |
|
I actually find vkconfig more convenient, since I can control what does and doesn't get reported; in order to better focus on specific problems. Otherwise I have to understand Godot's source code to see what is exactly being enabled with each CLI flag. |
|
May I ask is acyclic graph some kind of render graph ? |
|
Yes. I just edited the comment to avoid the confusion. |
|
After some discussion with @clayjohn , the impact of this bug is too large to be depending on acycl. render graphs being completed. I will wrap up what I'm doing now and then focus on fixing this bug even if that means sacrificing performance (since fixing this bug right now means placing inefficient barriers). |
Godot version
4.2.x master [16a9356]
System information
Linux - GCC g++ 9.4.0 - Ubuntu 20.04 LTS
Issue description
Ticket #78497 tracks a good number of validation number and this ticket addresses all of them that start with "READ_AFTER_WRITE" or "WRITE_AFTER READ". This affects #61415
First, I need to explain what these are. Because I see this is a systemic problem.
Vulkan Synchronization isn't easy. Good reads are this/this and this.
We can basically split Vulkan Barriers in 3 things they do:
VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL) now wants to be used for sampling (VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL). Godot gets this right. The default layers will scream at you if you get this wrong, and rendering will often be very corrupt.VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT) depends on the rendering done by Pixel Shader A (VK_PIPELINE_STAGE_FRAGMENT_SHADER_BITorVK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BITdepending on the case), then a barrier must be issued to ensure the Compute Shader doesn't start before rendering is done. Godot gets this more or less right since often a layout change is involved; but at other times it doesn't.VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT), then caches need to be flushed to VRAM before the Compute Shader B can read them (VK_ACCESS_COLOR_ATTACHMENT_READ_BIT). Godot takes the brute force path of almost always askingREAD_BIT|WRITE_BIT, but sometimes forgets to do this.Normally, the 3 things happen at the same time: We change layouts because we need to do something else to a Resource, that means we must wait for a previously issued command and thus enforce command order, and while we are at it we flush the caches.
But unfortunately in Godot, that's not always the case.
Godot asks Render Passes to automatically change layouts
A VkRenderPass requests 3 parameters we care about:
VkAttachmentDescription::initialLayoutVulkan asks us in what layout the texture currently is before starting the rendering pass. Validation will scream at us if we get it wrong.VkAttachmentReference::layout. In which layout the rendering should be done. Unless we're doing subpasses in Mobile, this is almost alwaysVK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMALbecause we want to render into it.VkAttachmentDescription::finalLayoutin what layout do we wish to leave the textures in.Godot "for simplicity" (in quotes because IMHO as things get more complex this adds a lot of problems) almost always assumes the following:
That is, assume the texture was being used for sampling, then temporarily change it to Render Texture, and then resume sampling again. For most simple effects this make sense.
For more complex Compute Shaders, this is counterproductive because if the CS wants to write to it; it needs to change from
VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMALtoVK_IMAGE_LAYOUT_GENERALand when it's done back toVK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL.I talk about this behavior in godotengine/godot-proposals/issues#7366 see "Gaussian Glow performs inefficient pipeline barriers & lack of barrier solver".
However that is not the problem here.
The problem is that VkRenderPass performs the layout change but does not enforce order (and honestly I don't know if it flushes the caches...).
VkRenderPass are guaranteed to be ordered against other VkRenderPass, but not against compute shader dispatches.
Frankly VkRenderPass "conveniently" leaving the layout in the one you want is a disservice that makes more harm than good which is why I hate subpasses with a passion.
If we run a VkRenderPass and then a Compute Shader that uses the RenderTarget(s) from that pass; we must use a barrier. Otherwise it's a data hazard. The CS may start before rendering is done writing to it.
In OgreNext I prefer to leave
finalLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMALso that validation will always scream at me if I try to use the texture again. In very few cases this is slightly less efficient, but once you start doing anything more advanced (literally anything with Compute Shaders) it becomes a blessing.MSAA
If we create an empty a project and turn on MSAA + SSAO and leave everything else off (e.g. SSR, SDFGI) the following happens:
Unfortunately, Step 2 can start before Step 1 finishes. Regular validation doesn't complain because the layout is in the correct state. There is code that doesn't get to be called
which inserts a global barrier. However even if I force the call by commenting out the if(), synchronization validation still complains and rendering on AMD Polaris is still messed up. I haven't yet figured out why though.
Though a simple possibility is that the barrier explicitly needs us to submit the textures involved.
Steps to reproduce
Messages like the following:
Minimal reproduction project
Any
The text was updated successfully, but these errors were encountered: