EQMG / Acid

A high speed C++17 Vulkan game engine
https://equilibrium.games
MIT License
1.75k stars 153 forks source link

Skybox shader corruption on linux #77

Open SpookySkeletons opened 5 years ago

SpookySkeletons commented 5 years ago

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.

mattparks commented 5 years ago

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: https://github.com/EQMG/Acid/blob/45e1b3114e8c0ced1027d628444d31ea6378614d/Sources/Graphics/Images/ImageDepth.cpp#L8

If your GPU only supports VK_FORMAT_D16_UNORM then it does not support depth stencils: https://github.com/EQMG/Acid/blob/45e1b3114e8c0ced1027d628444d31ea6378614d/Sources/Graphics/Images/Image.cpp#L164

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.

SpookySkeletons commented 5 years ago

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

SpookySkeletons commented 5 years ago

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.

mattparks commented 5 years ago

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?

SpookySkeletons commented 5 years ago

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!

SpookySkeletons commented 5 years ago

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.

mattparks commented 5 years ago

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.

SpookySkeletons commented 5 years ago

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

SpookySkeletons commented 5 years ago

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.

SpookySkeletons commented 4 years ago

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.

SpookySkeletons commented 4 years ago

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.

SpookySkeletons commented 4 years ago

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

SpookySkeletons commented 4 years ago

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.

SpookySkeletons commented 4 years ago

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.