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

Use best fit normals for storing screen space normals #86316

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Dec 18, 2023

Fixes: #74633
Fixes: #84184

Needed for: #86267

This uses an approach described in https://advances.realtimerendering.com/s2010/Kaplanyan-CryEngine3(SIGGRAPH%202010%20Advanced%20RealTime%20Rendering%20Course).pdf to store normals in a way that minimizes error. Essentially, we pre-generate an LUT that describes the length of a vector that (when compressed, uncompressed, then normalized) most closely resembles the original vector.

Thanks to Anton Kaplanyan for pioneering the technique, sharing it, and ultimately helping me fine-tune it for use in Godot.

Before After
Screenshot from 2023-12-21 14-36-29 Screenshot from 2023-12-21 14-36-45

Error visualization, darker is better

Before After
Screenshot from 2023-12-21 14-37-34 Screenshot from 2023-12-21 14-37-48

Bonus section. Using full float normals:

Full Float This PR
Screenshot from 2023-12-21 14-36-17 Screenshot from 2023-12-21 14-36-45

Bonus section. Using octahedral instead clayjohn@62d234f. (pros: better quality, better encode speed; cons: worse decode speed, harder to debug)

Reflection Error visualization
Screenshot from 2023-12-21 14-37-11 Screenshot from 2023-12-21 14-37-56

@jcostello
Copy link
Contributor

Does this affect the look of the normal map in materials outside VoxelGI and HDDAGI?

@clayjohn
Copy link
Member Author

Does this affect the look of the normal map in materials outside VoxelGI and HDDAGI?

No, right now this only improves things that use the normal roughness buffer (so SDFGI, VoxelGI, SSR). It might have a slight benefit for SSAO and SSIL, but likely isn't noticable.

I think a similar change might help LightmapGI though, so I'll experiment with that as well.

@jcostello
Copy link
Contributor

Let me know about the LightmapGI

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. This particularly helps when rendering smooth materials at high resolutions.

Performance is pretty much identical compared to before a on RTX 4090 at 4K.

Preview

Click to view at full size.

SDFGI

Before After
Screenshot_20231219_142933 webp Screenshot_20231219_142955 webp

VoxelGI

Before After
Screenshot_20231219_143041 webp Screenshot_20231219_143051 webp

Normal buffer

Looks glitchy, but the final result looks good either way. clayjohn suggested this was due to this debug view not being normalized before display.

Before After
Screenshot_20231219_151152 webp Screenshot_20231219_151137 webp

Video with slow camera rotation

Using VoxelGI. This was rendered at 4K and encoded in 720p to rule out aliasing from the equation.

Don't mind the flickering at the top – it's caused by me recording with V-Sync disabled.

Before

simplescreenrecorder-2023-12-19_15.16.18.mp4

After

simplescreenrecorder-2023-12-19_15.17.05.mp4

@clayjohn clayjohn force-pushed the RD-BFN-normals branch 2 times, most recently from 9da5e99 to 4295d9f Compare December 20, 2023 23:52
@clayjohn clayjohn modified the milestones: 4.x, 4.3 Dec 20, 2023
@clayjohn clayjohn marked this pull request as ready for review December 20, 2023 23:53
@clayjohn clayjohn requested a review from a team as a code owner December 20, 2023 23:53
@clayjohn
Copy link
Member Author

Okay, just pushed an update doing the following:

  1. Ensure that the normal is being used consistently (SSAO, SSIL, SSR, VoxelGI, SDFGI)
  2. Switch to a signed buffer so we can easily encode static/dynamic in the roughness channel
  3. Added a compatibility function so this change doesn't break compatibility in user shaders

This should be ready for review and merging.

@jcostello In the end it looks like LightmapGI does its own thing with normals. So I didn't touch it.

@reduz
Copy link
Member

reduz commented Dec 21, 2023

@clayjohn it looks good, but (sorry you are going to hate me for this) from what I am reading, in modern OpenGL using 8-bit SNORM, -128 and -127 are exactly the same (both map to -1.0, see the Signed section here) which may be a problem storing the dynamic object flag in roughness.

Sounds like we should probably return to storing UNORM, so we can properly store the roughness range better for dynamic objects. My suggestion given that min roughness (0) is important for sharp reflections, while max roughness changes nothing between say 0.99 and 1.0), is to store as follows:

roughness = roughness * (127.0/255.0);
if (dynamic_object) {
     roughness = 1.0 - roughness;
}
normal_roughness.w = roughness;

This allows us to take advantage of the fact that 0.5 can't be stored on UNORM, given the odd range normalization.

Hence decoding is just:

float roughness = normal_roughness.w;
bool dynamic_object = roughness > 0.5;
if (dynamic_object) {
    roughness = 1.0 - roughness;
}
roughness /= (127.0/255.0);

This way we keep roughness == 0 intact and we can safely retrieve a dynamic object flag.

@clayjohn
Copy link
Member Author

@reduz Updated and switched back to UNORM. Everything works great still. I actually got the error down a little bit during the update, so its slightly higher quality too.

Copy link
Member

@reduz reduz left a comment

Choose a reason for hiding this comment

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

Looks fantastic!

@YuriSizov YuriSizov merged commit 44ded3e into godotengine:master Dec 22, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

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