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

Increase precision in linear_to_srgb() and srgb_to_linear() #100659

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

clayjohn
Copy link
Member

Fixes: #100517

This was a combined regression from #93802 and #57703

#57703 forced most color functions to use floating point operations which is nice for performance and generally makes sense. However, in this specific case, double precision is needed.

This PR removes the float literal qualifiers from just enough places to allow the compiler to make a smart decision about how much to keep in floating point and how much to do with doubles. In theory this will be slightly slower, but we have no choice.

The regression became apparent after #93802 because the canvas modulate color is always 1 by default and converting it to linear (which is needed when using a linear viewport) turns it into 0.99999988. Which, when applied over many frames by ping-ponging the buffer, results in a darkening of the viewport. Presumably every other place where these functions are called is tolerant of this slight precision error.

I added tests to ensure that we don't accidentally make the same mistake again. This is a case where is_equal_approx() has too high of a tolerance and we need to use exact floating point comparison with ==.

This avoids the situation where white stops being white after conversion. While maintaining as much floating point ops as possible
@clayjohn clayjohn added bug topic:core regression cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Dec 20, 2024
@clayjohn clayjohn added this to the 4.4 milestone Dec 20, 2024
@clayjohn clayjohn requested review from a team as code owners December 20, 2024 17:57
@akien-mga akien-mga merged commit dac0b67 into godotengine:master Dec 20, 2024
20 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 regression topic:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buffering 2d hdr subviewport leaves image slightly darker (problem for cumulative effects)
2 participants