-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Add EXPOSURE built in to spatial shaders #71364
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Needs rebase, otherwise should be good to go if nobody has any comment :) |
clayjohn
force-pushed
the
exposure-builtin
branch
from
April 12, 2023 15:14
3974a2e
to
72d859d
Compare
Chaosus
reviewed
Apr 12, 2023
Chaosus
requested changes
Apr 12, 2023
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.
Seems good except few typos in visual shader.
This allows users to restore light values to pre-pre-exposure amounts
clayjohn
force-pushed
the
exposure-builtin
branch
from
April 12, 2023 17:35
72d859d
to
9be0a73
Compare
Thanks @Chaosus! I pushed a new version with your suggested changes |
Chaosus
approved these changes
Apr 13, 2023
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows users to restore light values to pre-prebaked-exposure amounts
Fixes: #69847
Supersedes: #71304
When using Physical Light Units or
exposure_multiplier
in aCameraAttributes***
resource all light sources are pre-baked by the exposure amounts in theCameraAttributes
. This ensures that the screen texture values stay within a manageable range.A side-effect of this pre-baking is that SCREEN_TEXTURE returns the post-pre-baked exposure amount. I.e. the final brightness after applying exposure from CameraAttributes, but before applying exposure from the tonemapper. If this value is fed back in as an ALBEDO, everything works fine because ALBEDO expects a non exposed color. However, if you feed the SCREEN_TEXTURE result in EMISSION, it appears black because EMISSION is in physical light units, so it expects un-pre-baked exposed values (it will apply the pre-baked exposure internally after the user shader has ended). The end result is that we can either have SCREEN_TEXTURE properly exposed for ALBEDO (including baked exposure) or for EMISSION (including baked exposure).
#71304 falls short as all it does is revert the default so that SCREEN_TEXTURE works for EMISSION, but not for ALBEDO (i.e. it returns the un-pre-baked exposure value).
This PR exposes an
EXPOSURE
(open to alternative naming suggestions) builtin to spatial shaders that allows us to account for the pre-baked exposure. Simply multiplyingEXPOSURE
by values that already have the exposure baked in will undo the pre-baked exposure and return the original value. For example, multiplying this by a light's energy will give the original energy before pre-baking the CameraAttributes exposure. Or alternatively, multiplying by the result of SCREEN_TEXTURE will give you what the screen texture pixel would have been if the pre-baking had not been applied.Tagging as 4.x as there is no rush on this. It is a pain for people using Physical Light units, but this is an addition to the shader API so I would also like more time for this to be tested and discussed, accordingly, I would prefer to merge after 4.0 stable release.