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

Vulkan: Artifacts when using a HDRI with PanoramaSkyMaterial (only with HighQuality update mode, not RealTime) #43763

Closed
fracteed opened this issue Nov 22, 2020 · 10 comments · Fixed by #58177

Comments

@fracteed
Copy link

fracteed commented Nov 22, 2020

Godot version:
current master 4.0

OS/device including version:
Win10 960M

Issue description:
When using a HDRI in .hdr format with the panorama sky material in the World Environment node, artifacts are exhibited on curved surfaces.

Steps to reproduce:
Add a panormama sky material to the World Environment node and add in a texture in .hdr format.
As can be seen in this image, there are artifacts in the curved surfaces of the sphere, cylinder and capsule. The planar surfaces of the cube and cylinder look fine.

I have tried this on imported meshes and the same issues are seen. This hdri from hdri haven works fine in 3.2.4. All hdri's I have tested in 4.0 vulkan have these issues. As far as I know, the vulkan renderer has always had this problem, as I saw this same issue at the start of the year when doing tests. Changing the radiance size setting seems to have no effect.

hdri_issue

Minimal reproduction project:
hdri_issue.zip

@clayjohn
Copy link
Member

clayjohn commented Nov 22, 2020

This is the result of using a high frequency, high range panorama sky. You have two options, you can increase ggx_samples or you can switch your sky to use the "Realtime" update mode.

Related: #36171 and #32125

@fracteed
Copy link
Author

fracteed commented Nov 22, 2020

Thanks, the realtime mode shows none of those artifacts, but does make colors appear duller. Still looks fine though. Is this the equivalent of the way it was in 3.2 and presumably this is the most performant? Not sure if ggx samples affect this mode or not?

The best I could get with the high quality mode was to have high ggx samples at 4096 and low radiance size at 32. The artifacts are still slightly there but you probably wouldn't see them with texture mapping.

hdri_issue2

and with realtime mode:

hdri_issue3

Another anomaly, is trying to rotate the sky using the rotation vector. Just trying to rotate on the y axis, but getting very bizarre results. Presumably this is a bug?

@clayjohn
Copy link
Member

Realtime mode doesn't look as high quality HighQuality mode, but it doesn't suffer from the "fireflies" look that the HighQuality mode gets from too few samples. The Realtime mode isn't based on the 3.2 style, it uses a very different approach, the HighQuality mode is the same basic method as in 3.2, but 3.2 has some improvements. Realtime is fast enough to change the sky every frame. HighQuality will take a few to a few dozen milliseconds to compute, so is not suitable for realtime.

We can probably port some of the improvements from 3.2 to 4.0.

Another anomaly, is trying to rotate the sky using the rotation vector. Just trying to rotate on the y axis, but getting very bizarre results. Presumably this is a bug?

Sounds like a bug worth reporting in another issue.

@fracteed
Copy link
Author

So after the initial time for the radiance map to be computed, the high quality and realtime modes have the same gameplay rendering performance after that? Sounds like you would only use the realtime when using an animated sky.

Is that normal to have the big difference in the albedo value between the realtime and high quality modes, as can be seen in the 2 images I posted?

@clayjohn
Copy link
Member

So after the initial time for the radiance map to be computed, the high quality and realtime modes have the same gameplay rendering performance after that? Sounds like you would only use the realtime when using an animated sky.

Yep, Realtime is also nice because it never has fireflies. So it's a trade-off. But in general, you should always be okay with high-quality when using panorama skies.

Is that normal to have the big difference in the albedo value between the realtime and high quality modes, as can be seen in the 2 images I posted?

I've never seen any HDRIs with such a large difference. But it's pretty clear that your using an HDRI with an unusually high frequency and dynamic range. So the difference isn't really surprising.

@Calinou
Copy link
Member

Calinou commented Nov 22, 2020

I've never seen any HDRIs with such a large difference. But it's pretty clear that your using an HDRI with an unusually high frequency and dynamic range. So the difference isn't really surprising.

Could we add a "reflection intensity" slider to compensate this in such scenarios? Or is there another way this could be done?

@clayjohn
Copy link
Member

@Calinou a "reflection intensity" slider would just be the roughness parameter. The issue here is in generating the radiance map from the HDRI. So changing the amount of reflection won't really help. What we need to do is port of the improvement mentioned in this article I already implemented it in 3.x in #33668

My understanding is that you would typically just author the HDRI with a lower dynamic range. Unfortunately, you can't just capture and HDRI and plug it in and expect it to look perfect. There is an art to designing HDRIs for games.

@lyuma
Copy link
Contributor

lyuma commented Apr 20, 2021

So my problem with this is that a maximally rough object shouldn't be sampling a texture with 256x256 size, or even anything bigger than 32x32. Indeed, changing radiance down to 32x32 solves the issue. But really I want my rough objects to be sampling irradiance or at least a maximum mipmap on the radiance texture..

Basically, wouldn't heavily blurring the radiance texture solve this?

@Calinou Calinou changed the title Vulkan: artifacts when using a hdri with panorama sky material Vulkan: Artifacts when using a HDRI with PanoramaSkyMaterial (only with HighQuality update mode, not RealTime) Apr 20, 2021
@clayjohn
Copy link
Member

@lyuma Right now we use a high mipmap of the radiance map as a stand in for irradiance:

ambient_out.rgb = textureLod(samplerCubeArray(reflection_atlas, material_samplers[SAMPLER_LINEAR_WITH_MIPMAPS_CLAMP]), vec4(local_amb_vec, reflections.data[ref_index].index), MAX_ROUGHNESS_LOD).rgb;

In the future, we should indeed add a proper irradiance map (or more likely, spherical harmonics or something similar)

Basically, wouldn't heavily blurring the radiance texture solve this?

For irradiance, probably, it wouldn't be physically correct, but it would be very plausible.

@clayjohn
Copy link
Member

clayjohn commented Feb 14, 2022

To properly fix this bug we need to implement a few improvements to our current environment map filtering code:

  1. Use "Filtered Importance Sampling" (described here here, and here) (Implemented in 3.x already in Fix issues with environment mapping #33668) Filtered Importance Sampling will allow us to get away with far fewer samples so it will also improve performance
  2. Add various optimizations from here (optimize shader code to reduce work)
  3. Add random rotation to cubemap roughness sampling #58086
  4. When using compute, we can also process all 6 faces at once which should provide a small performance boost

We should also consider:

  1. Tonemapping before filtering the environment map to reduce the weight of very high intensity spots(we can use a reverse tonemapper as described by Brian Karis here)
  2. Allow users to clamp HDRI to a specific range on import (useful to ensure that bright pixels don't ruin the entire image during filtering) (also useful to effectively remove the sun from HDRI so it doesn't conflict with the sun from the DirectionalLight)
  3. If the above isn't enough to remove fireflies, we should optionally blur the generated radiance map (with Add random rotation to cubemap roughness sampling #58086, a small radius blur should be enough)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants