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

ACES tonemapping #2264

Closed
Pauan opened this issue May 28, 2021 · 4 comments
Closed

ACES tonemapping #2264

Pauan opened this issue May 28, 2021 · 4 comments
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible

Comments

@Pauan
Copy link

Pauan commented May 28, 2021

What problem does this solve or what need does it fill?

ACES is the standard for tonemapping HDR colors to sRGB. It is widely used in the film industry, it is used by default in Unreal / Filament, and it can optionally be used in Unity, Godot, BabylonJS, ThreeJS, Blender, and various other software.

Using ACES prevents crushing/clamping bright colors (which is very important when using HDRIs), and it also guarantees consistent results (e.g. if you create a texture in Blender with ACES, it will look the same in an ACES game engine).

What solution would you like?

Implement ACES by default, ideally matching Unreal / Filament. Other tonemappers can be provided as optional extras.

This will require correct HDR support in the entire PBR rendering pipeline (using linear colors and converting into sRGB at the very last moment). It will also affect how assets are loaded (since texture files can be loaded as either Utility - Linear - sRGB, Utility - sRGB - Texture, or Utility - Raw depending on the color space of the texture).

What alternative(s) have you considered?

There are a lot of other tonemappers available (Filmic, Reinhard, Uncharted, Uchimura, etc.) but ACES is a widely used standard.

Using sRGB (i.e. not using tonemapping) isn't really a solution, since that causes a lot of problems with bright lights and HDRIs. sRGB can be provided as an option, but I don't think it should be the default.

Additional context

Note that ACES tonemapping causes colors to appear darker than they do in sRGB. Some engines like Unreal and Filament compensate for that by applying an extra gain which brightens the colors. This extra gain should be configurable (and it should probably default to Unreal's gain).

This is an old but good overview of Unreal's implementation (though it might have changed since then): https://www.youtube.com/watch?v=A-wectYNfRQ

@Pauan Pauan added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels May 28, 2021
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels May 28, 2021
@CptPotato
Copy link
Contributor

CptPotato commented Jun 3, 2021

👍 for providing multiple tonemappers. ACES has very high contrast in my opinion so I think it's probably not the best choice if there's only one fixed tonemapper provided (or as a default). Having an srgb option is also important in my opinion because not all apps neccesarily want tonemapping (at least in non-pbr scenarios).


(I'm going off on a tangent here but these are related things that came to mind:)

This will require correct HDR support in the entire PBR rendering pipeline (using linear colors and converting into sRGB at the very last moment).

To elaborate on that, up until the tonemapping step all color rendertargets need to be able to store linear, "unclipped" color values. The most common formats seem to be R16G16B16A16_FLOAT and R11G11B10_FLOAT. I could imagine the general flow (for PBR) looking somewhat like this:

main_pass -> transparent_pass -> msaa_resolve¹ -> hdr_post_process² -> tonemap -> ldr_post_process² -> ui_pass

¹ optional
² possibly with multiple passes

Tonemapping can also be an interesting use case for injecting user defined shader code so you could provide any custom tonemapping operator and color grading without much limitations.


How HDR in general integrates with the rest of the rendering pipeline could be a bit tricky. I see a few potential pain points as things currently stand:

  • at the moment, tonemapping for PBR is applied in the main_pass object shader
    • this is fine if no post processing needs to be done before tonemapping
    • for physical effects like bloom, depth of field, etc. it needs to be separated though
  • since the render target formats are currently hard-coded in the provided render graphs, using a different format for main_pass requires changes in bevy_pbr and bevy_render
  • MSAA is (as always) a bit of a problem:
    • memory consumption can grow pretty fast when switching to R16G16B16A16_FLOAT render targets
    • resolving MSAA on an HDR target requires some trickery to keep edges with high contrast smooth in the final image (custom resolve step is required)
  • the general ordering of passes is not always clear
    • some users might want their UI being rendered using the HDR path
    • graphics/shader plugins that aim to work within this pipeline can get very tricky in terms of modularity, compositing and "wiring things up"

I could give some more input or help if anyone decides to tackle this or something related (hdr/bloom/color grading/etc).

@CptPotato
Copy link
Contributor

#2876 is related (adds ACES tonemapping)

bors bot pushed a commit that referenced this issue Feb 19, 2023
# Objective

Splits tone mapping from #6677 into a separate PR.
Address #2264.
Adds tone mapping options:
- None: Bypasses tonemapping for instances where users want colors output to match those set.
- Reinhard
- Reinhard Luminance: Bevy's exiting tonemapping
- [ACES](https://github.com/TheRealMJP/BakingLab/blob/master/BakingLab/ACES.hlsl) (Fitted version, based on the same implementation that Godot 4 uses) see #2264
- [AgX](https://github.com/sobotka/AgX)
- SomewhatBoringDisplayTransform
- TonyMcMapface
- Blender Filmic

This PR also adds support for EXR images so they can be used to compare tonemapping options with reference images.

## Migration Guide
- Tonemapping is now an enum with NONE and the various tonemappers.
- The DebandDither is now a separate component.




Co-authored-by: JMS55 <[email protected]>
@inodentry
Copy link
Contributor

Closing this, as I feel it has been addressed by #7594

@alexpanter
Copy link

A bit old now, and probably not relevant anymore, but I did make a small program to compare various tonemapping operators. It's on youtube.

Might be interesting to visually compare them, for anyone curious

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible
Projects
None yet
Development

No branches or pull requests

5 participants