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

[3.x] Add horizon specular occlusion #51416

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Aug 9, 2021

Implements horizon specular occlusion. This helps reduce artifacts where you see the sky on the edges of e.g. tiles bricks etc.

before: GLES3
Screenshot (197)

after: GLES3
Screenshot (198)

before: GLES2
Screenshot (201)

after: GLES2
Screenshot (200)

Note: for some reason GLES2 suffers much less from this particular artifact. With horizon specular occlusion it looks almost perfect. At some point it may be worth exploring what is going on to see if we can improve GLES3 similarly.

@clayjohn clayjohn added this to the 3.4 milestone Aug 9, 2021
@clayjohn clayjohn requested a review from Calinou August 9, 2021 02:36
@clayjohn clayjohn requested a review from a team as a code owner August 9, 2021 02:36
@clayjohn clayjohn changed the title Add horizon specular occlusion [3.x] Add horizon specular occlusion Aug 9, 2021
@Calinou
Copy link
Member

Calinou commented Aug 9, 2021

Looks great! Does this take SSAO into account as well, or does it even rely on AO maps in the first place? Either way, I think this supersedes #50603.

Note: for some reason GLES2 suffers much less from this particular artifact. With horizon specular occlusion it looks almost perfect. At some point it may be worth exploring what is going on to see if we can improve GLES3 similarly.

My guess is that the radiance/irradiance map is less detailed in GLES2 (and possibly darker), due to using a more approximate way to generate it. This would make the artifacts less noticeable overall.

@clayjohn
Copy link
Member Author

clayjohn commented Aug 9, 2021

Looks great! Does this take SSAO into account as well, or does it even rely on AO maps in the first place? Either way, I think this supersedes #50603.

Ah! I forgot your PR included horizon specular occlusion. This is just horizon specular occlusion and does not touch regular AO at all.

The big difference here is that the horizon specular occlusion is only factored in for the IBL and not for other sources of specular lighting.

@akien-mga akien-mga merged commit dad5d09 into godotengine:3.x Aug 10, 2021
@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.

3 participants