EQMG/Acid

Skybox shader corruption on linux

SpookySkeletons opened this issue · 14 comments

Describe the bug
The skybox of my test builds has major corruption that is cut through by the on screen models.
The corruption seems to pulsate in color.

To Reproduce
Not sure what's the cause yet...

Expected behaviour
A clear view into the skybox without the slowly pulsating color.

Screenshots
Screenshot_20190829_013120

Hardware:
Gentoo linux kernel 5.2.10, AMD WX 7100, RADV

Additional context

I am using Mesa ACO git but I have tested with the LLVM compiler and ACO both and I get the same odd behavior.

I believe this is a depth stencil or depth buffer related issue, you may need to use a Vulkan capabilities tool or debug mode to find out what mode Acid is selecting:

static const std::vector<VkFormat> TRY_FORMATS{ VK_FORMAT_D32_SFLOAT_S8_UINT, VK_FORMAT_D32_SFLOAT, VK_FORMAT_D24_UNORM_S8_UINT, VK_FORMAT_D16_UNORM_S8_UINT,

If your GPU only supports VK_FORMAT_D16_UNORM then it does not support depth stencils:

static const std::vector<VkFormat> STENCIL_FORMATS{ VK_FORMAT_S8_UINT, VK_FORMAT_D16_UNORM_S8_UINT, VK_FORMAT_D24_UNORM_S8_UINT, VK_FORMAT_D32_SFLOAT_S8_UINT };

I don't know if a stencil aspect bit is optional with VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, if it's not then this is probably the cause. If you can confirm that VK_FORMAT_D16_UNORM is the selected format then I can move forward with a fix.

Sure thing I'll rig together a test in a few hours yet! Thank you for the quick followup.

I checked the mesa source code for RADV, all depth stencils listed are supported. I believe the stencil aspect bit is required in this case.
You can check https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/amd/vulkan/vk_format.h
To confirm if you need to.

VK_FORMAT_D16_UNORM , Acids last resort option, does not have VK_IMAGE_ASPECT_STENCIL_BIT from what that mesa source lists. If that is the only available format the depth texture will not have a stencil bit.

VK_FORMAT_D32_SFLOAT does not have a stencil bit either, maybe the solution would be to list that format before VK_FORMAT_D16_UNORM and after the other formats in Graphics/Images/ImageDepth.cpp in TRY_FORMATS?

I set VK_FORMAT_D32_SFLOAT_S8_UINT as the only try format and the background still presents corruption. I ensured that it was actually set as VK_FORMAT_D32_SFLOAT_S8_UINT by printing the expected and found enumerated values.

Funny thing is
if (formatProperties.optimalTilingFeatures & VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT)
Still breaks the loop despite the fact that I can set the format to VK_FORMAT_D16_UNORM which does not have a stencil attachment bit.

It terminates as expected when I put VK_FORMAT_D24_UNORM_S8_UINT but not VK_FORMAT_D16_UNORM or VK_FORMAT_D16_UNORM_S8_UINT, just treats them like they have a stencil bit!

I'm running over it in a RenderDoc capture.

The 2D depth map seems okay (Doesn't seem to "drift" like I'm seeing).
It's baking it into the 2D color texture, perhaps from the irradiance input of the render ? I am not sure exactly what I'm looking at/ for but I can upload my RenderDoc capture if it would help reproduce it.

Sorry if I'm being extremely slow to fix this bug, shadows and skybox severely need a rewrite anyways. A RenderDoc capture would be very helpful, I am getting other kinds of artifacts using Mesa on a laptop with integrated Intel graphics.

Whoa, good to hear at least that you can reproduce, thought my setup was insane. Will post once home, my card is a polaris10.

https://drive.google.com/open?id=1a_xqfcyjaOPzS5Wt2LT_Q2pYTc2d1Ymn

PBR test renderdoc on a wx7100 mobile GPU, polaris10.

I dragged the camera as the trace capture wipes all but the last overlay of the corruption so you can clearly see it.

It's not utterly crippling but the engine's stability under mesa is a bit worrying.
I've studied up on Vulkan a bit, any way I can enable validation layers to help inform/ fix this bug? I'd love to use this project in earnest but it's not too operable on Linux.

Messed around disabling code and it looks like the skybox issue is a problem with fog. Once I disable fog in the physics test render it isn't an issue. Not sure if the shader is borked or the Vulkan compliance but hopefully I can nail it down.

Got schooled up on Vulkan a bit more and enabled standard validation layers and I think I found the problem. Full log attached here:
20200523152548.txt

Fixed!
Sources/Graphics/Renderpass/Renderpass.cpp
Line 67: attachmentReference.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
Changed to
attachmentReference.layout = VK_IMAGE_LAYOUT_GENERAL;

Sources/Graphics/Renderpass/Framebuffers.cpp
Line 18:
imageAttachments.emplace_back(std::make_unique(extent, attachment.GetFormat(), VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
to
imageAttachments.emplace_back(std::make_unique(extent, attachment.GetFormat(), VK_IMAGE_LAYOUT_GENERAL,

Looks like certain formats aren't available to me here? Know why this is the case on linux/mesa?
Allowed layouts are: VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_GENERAL, VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL, VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL.

I now get
Layout for color attachment is GENERAL but should be COLOR_ATTACHMENT_OPTIMAL.
Layout for color attachment is GENERAL but should be COLOR_ATTACHMENT_OPTIMAL.
Layout for color attachment is GENERAL but should be COLOR_ATTACHMENT_OPTIMAL.
Layout for color attachment is GENERAL but should be COLOR_ATTACHMENT_OPTIMAL.
Layout for color attachment is GENERAL but should be COLOR_ATTACHMENT_OPTIMAL.
Layout for color attachment is GENERAL but should be COLOR_ATTACHMENT_OPTIMAL.
Layout for color attachment is GENERAL but should be COLOR_ATTACHMENT_OPTIMAL.
Even though these attachments aren't available, which is odd.

Okay, took a look through the mesa src/amd/vulkan/ tree. Looks like VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL isn't actually available as a format under radv or anv even though its enumerated in the basic vulkan includes. I assume it would be better to add a check here to see if this format is available before use between GENERAL and OPTIMAL.