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

Improve the look of radiance map in Compatibility backend #97676

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Oct 1, 2024

Fixes: #95091

There were 4 problems I found while digging deeper into this code:

  1. The "linear" roughness was used instead of perceptual roughness
  2. The first mip level had a weird artifact that was sampled incorrectly when the tangent frame was aligned with the z axis (as you can see in this comment: Compatibility renderer has lower-quality (ir)radiance map generation than Forward+ and Mobile #95091 (comment))
  3. The faces of the cubemap were y-flipped causing filtering errors
  4. The higher roughness layers looked much lower quality than the lower roughness

Weirdly the first two issues were solved when I mapped the roughness properly. The tangent frame just has less of an impact when the sampling cone is larger.

The third issue is fixed by changing the order of the vertices.

The last issue is fixed by taking more samples at the higher mip levels. While 4x as many samples seems like a lot, its not bad as it only happens at mip levels with few pixels (e.g. for a 256x256 texture, it only changes the 16x16 mip levels and lower. Further rougher mip levels have fewer samples that succeed, and since we pre-calculate samples on the CPU and reject failed samples, we don't end up sampling 4x as much on the GPU.

The big difference in quality between the RD renderers and the compatibility renderer comes from the fact that the RD renderers use a texture array instead a single texture. In practice that means that the low mip levels use the same resolution as the highest mip level (e.g. 256x256). In the compatibility renderer we put rougher levels in higher mips, so effectively you are taking fewer samples. E.g. for mip level 5 in the RD renderers you would do 32 samples x 256256 (2,097,152 samples) for each face, while in the compatibility renderer you would only have 32 samples x 1616 (8,192 samples).

Slightly off topic, but still interesting despite taking 256 times as many samples, the RD renderer is not much slower because the GPU doesn't get saturated with only 8,192 samples. The real benefit of the compatibility renderer is lower memory usage, and fewer necessary passes.

Before
Screenshot from 2024-09-30 21-48-58

After
Screenshot from 2024-09-30 21-48-38

@clayjohn clayjohn added bug topic:rendering cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Oct 1, 2024
@clayjohn clayjohn added this to the 4.4 milestone Oct 1, 2024
@clayjohn clayjohn requested a review from a team as a code owner October 1, 2024 05:01
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

Testing project: test_pr_97676.zip

Before

Using Filmic tonemapping with whitepoint 1.0. (It's hard to avoid this overly contrasted look with this HDRI when using Compatibility, even when HDR Clamp Exposure is enabled in the Import dock.)

Screenshot_20241001_195256 png webp

After

Screenshot_20241001_195247 png webp

Forward+ (for reference)

Using ACES tonemapping with whitepoint 16.0.

Screenshot_20241001_195838 png webp

Mobile (for reference)

Screenshot_20241001_195854 png webp

@F3der1co
Copy link

F3der1co commented Oct 1, 2024

Maybe I am getting confused by the extreme contrast of the HDRI, but in comparison (only looking at the images Calinou posted) it almost lookss like Forward+ and Mobile are using linear instead of perceptive roughness.
Also the quality of compatibility looks better than mobile at high roughness now.

@clayjohn
Copy link
Member Author

clayjohn commented Oct 1, 2024

Maybe I am getting confused by the extreme contrast of the HDRI, but in comparison (only looking at the images Calinou posted) it almost lookss like Forward+ and Mobile are using linear instead of perceptive roughness. Also the quality of compatability looks better than mobile at high roughness now.

It is indeed hard to compare because of the difference in HDRIs (its a known bug #83788)

I think with that HDRI the sun just washes out everything else, while in the compatibility renderer it gets clipped. I did very careful back and forths to ensure that the roughness looked as consistent as possible. There is some expected difference because the compatibility renderer uses fewer mipmap levels

These are some screenshots from when I was working on this. roughness: 0.25. I can't remember which is Forward+ and which is compatibility
Screenshot from 2024-09-29 01-30-42
Screenshot from 2024-09-29 01-30-26

@akien-mga akien-mga merged commit 2bd0fd8 into godotengine:master Oct 2, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:rendering topic:3d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility renderer has lower-quality (ir)radiance map generation than Forward+ and Mobile
4 participants