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

[feature] Upgrade tone mapping function #84

Closed
Derpius opened this issue Sep 25, 2022 · 1 comment
Closed

[feature] Upgrade tone mapping function #84

Derpius opened this issue Sep 25, 2022 · 1 comment
Labels
binary module Issues relating to the binary module enhancement New feature or request
Milestone

Comments

@Derpius
Copy link
Owner

Derpius commented Sep 25, 2022

Is your feature request related to a problem? Please describe.
The existing tone mapping implementation has two issues:

  • Looking at other post processing stacks it appears that simply gamma correcting the linear output with 2.2 is not correct, so making the user convert linear to sRGB isn't viable
  • Without the user implementing a full exposure system themselves (or using an extension), the tone mapping function is effectively useless for good quality renders

Describe the solution you'd like
When researching post processing stacks for VisCam I decided to validate the implementation of converting the linear HDR rendered image to LDR, revisiting the repository I got the ACES tone mapping fit from: https://github.com/TheRealMJP/BakingLab/blob/master/BakingLab/ToneMapping.hlsl#L105

As you can see the ACES fit output is converted to sRGB using the full function which the tone mapper should also do

As for exposure, the function should have a boolean input, defaulting to true, that toggles full image luminance auto exposure. This means anyone can get good renders without needing to implement their own exposure function

Additional context
True sRGB vs gamma 2.2: https://www.shadertoy.com/view/7sjBWD

@Derpius Derpius added enhancement New feature or request binary module Issues relating to the binary module labels Sep 25, 2022
@Derpius Derpius added this to the v0.13.0 milestone Sep 25, 2022
@Derpius
Copy link
Owner Author

Derpius commented Sep 25, 2022

Should probably default auto exposure to false to avoid breaking existing renderers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binary module Issues relating to the binary module enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

1 participant