Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skybox shader corruption on linux #77

Open
SpookySkeletons opened this issue Aug 29, 2019 · 14 comments
Open

Skybox shader corruption on linux #77

SpookySkeletons opened this issue Aug 29, 2019 · 14 comments
Projects

Comments

@SpookySkeletons
Copy link

SpookySkeletons commented Aug 29, 2019

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
Copy link
Member

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.

@SpookySkeletons
Copy link
Author

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

@SpookySkeletons
Copy link
Author

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
Copy link
Member

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
Copy link
Author

SpookySkeletons commented Aug 29, 2019

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
Copy link
Author

SpookySkeletons commented Sep 2, 2019

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

SpookySkeletons commented May 23, 2020

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
Copy link
Author

SpookySkeletons commented May 23, 2020

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
Copy link
Author

SpookySkeletons commented May 25, 2020

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.

@mattparks mattparks added this to Needs triage in Issues May 28, 2020
@mattparks mattparks moved this from Needs triage to High priority in Issues May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Issues
  
High priority
Development

No branches or pull requests

2 participants