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

Properly remap roughness when reading from radiance map #69514

Merged
merged 1 commit into from
Dec 3, 2022

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Dec 3, 2022

This ensures that we consistently use perceptual roughness which matches the behaviour of most other PBR renderers like Blender, Ue4 and Godot 3

Fixes one of the reported issues in #69476

This issue was a regression from #58418. I didn't notice at the time, but the fast filtering path used a non-linear mapping between cubemap mipmap and roughness. I mistakenly thought that the high quality roughness shader took linear roughness as a parameter (linear roughness is perceptual_roughness^2). In fact the high quality roughness shader takes perceptual roughness as a parameter. What I did was square perceptual roughness before passing it in, thus I also created a non-linear mapping between radiance mip maps and roughness. And by chance, a simple square remapping is very close to what is used by the fast filtering algorithm.

The problem is that when reading from the radiance map, we uniformly map perceptual radiance to the radiance map mipmaps. So we end up reading from a much more reflective mipmap than we mean to. The solution is to take the sqrt of perceptual roughness so it uses the same non-linear mapping as the high quality roughness shader.

Before:
Screenshot (301)

After:

Screenshot (298)-godot

Blender:

Screenshot (299)-blender

Ue4:

Screenshot (297)ue4

Implementation details

Getting to this conclusion took a lot of work as I had some mistaken assumptions:

  1. I thought the high quality roughness shader took linear roughness as input (it takes perceptual roughness)
  2. I thought the fast filtering algorithm used a linear mapping of roughness to mipmaps (it doesn't)

In Godot 3.x we linearly map perceptual roughness to mipmap. In other words roughness of 0.5 corresponds to the mip level halfway between 0 and the max mip level. A linear mapping is not an efficient use of resources as above a perceptual roughness of 0.5 there is not a very big difference in each level. While between 0 and 0.25, there is a huge difference. This relationship has been studied by others and various mapping schemes have been proposed (for example, see here for the mapping used by filament).

The fast filtering algorithm bakes the roughness to mipmap mapping into the precalculated coefficients. So we are kind of stuck with their mapping. However, if you take a look at their mapping and the current square mapping we use, you can see they are quite close.

Blue is linear, purple is sqrt(x), red is our square mapping, and black is the fast filtering baked mapping
Screenshot (303)
https://www.desmos.com/calculator/nuh7yakzuq

Notice how the purple and red are a mirror image of each other? Red is what we use to bake the radiance map, and then purple is what we use to read from it. Since red and black are fairly close, it makes sense to treat them both the same.

In a perfect world, we would bake the high quality radiance map using the same mapping function as the fast filter radiance map, however it would be very costly to do required unmapping from that function at run time. So I have opted to just use square as it is pretty close.

With this PR, there is still a small difference between high-quality and fast filter radiance, but they are very close. There is also a small difference between Godot, Blender, and Ue4, but Godot now looks much closer to Blender than Ue4 does, so I think it is within an acceptable range.

This ensures that we consistently use perceptual roughness which matches the behaviour of most other PBR renderers like Blender, Ue4 and Godot 3
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good!

@akien-mga akien-mga merged commit 29ddbfa into godotengine:master Dec 3, 2022
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants