-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Fix bloom luma calc in sRGB #22565
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 bloom luma calc in sRGB #22565
Conversation
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
IQuick143
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. (Pedantic: srgb is not logarithmic.)
|
I figured the two changed screenshots are an intentional consequence of the code. |
|
I don't have an opinion: do we want to alter the examples to have a similar bloom as before or not? |
|
It looks like your PR is a breaking change, but you didn't provide a migration guide. Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes. |
alice-i-cecile
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like a small migration guide for this, even though it's a bug fix. Tracking down the cause of changes to bloom intensity and how to correct it will be a huge pain otherwise.
superdump
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me
|
Linter is mad at you but this looks good :) |
Objective
Solution