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

fix fog color inversion if density is set 0 #72329

Closed

Conversation

jainl28patel
Copy link
Contributor

@jainl28patel jainl28patel commented Jan 29, 2023

This PR fixes #66462 and fixes #66459.

Non-zero density

image

Zero density

image

@jainl28patel jainl28patel requested a review from a team as a code owner January 29, 2023 19:06
@clayjohn clayjohn added this to the 4.0 milestone Jan 29, 2023
@clayjohn
Copy link
Member

This also fixes #66459 right?

@jainl28patel
Copy link
Contributor Author

This also fixes #66459 right?

Yess !!

@clayjohn
Copy link
Member

So this fix will work for all cases where the values in fog are between 0 and 1. But it will break outside of that range. The basic usage of fog leaves all the values between 0 and 1, but if sun_scatter is used, it looks like values can be larger than 1. So when using sun_scatter the fog color will get clamped to vec3(1.0) (which may not be a dealbreaker)

@clayjohn
Copy link
Member

clayjohn commented Feb 8, 2023

Superseded by #72914

In the end this bug was fixed by reparing the half2float and float2half functions, so there is no need to work around the bug by limiting to normalized floats as is done here.

Thank you anyway for your investigation and your hard work in fixing this bug!

@clayjohn clayjohn closed this Feb 8, 2023
@clayjohn clayjohn removed this from the 4.0 milestone Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants