search

google / filament

Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WebGL2
https://google.github.io/filament/
Apache License 2.0
17.8k stars 1.89k forks source link

Out-of-gamut colors produced by inverse tone mapping cause hue shifts #3909

Open grassydragon opened 3 years ago

grassydragon commented 3 years ago

Hello! As stated in this issue in the maintained Sceneform repository (https://github.com/ThomasGorisse/sceneform-android-sdk/issues/60) the inverseTonemapSRGB function makes the colors with the full pixel intensity appear incorrectly. Is it because the inverseTonemapSRGB function is an approximation and isn't suitable for converting colors from the sRGB space to the linear space (I think that is why it is used in the ViewRenderable material in Sceneform)?

romainguy commented 3 years ago

Can you define "incorrectly" please? What's the expected output and the actual output?

This function will be exact if you choose the FILMIC tone mapper in the ColorGrading API. It will be an approximation with other tone mappers.

grassydragon commented 3 years ago

Here is what happens when I set the color in the hello-triangle example to #0000ff. Without inverseTonemapSRGB: Screenshot_20210507-184859 With inverseTonemapSRGB: Screenshot_20210507-184354 I expect the color to be close to blue.

romainguy commented 3 years ago

It should be blue, the approximation should not cause this.

romainguy commented 3 years ago

Oh never mind I understand what's going on. It's a hue skew caused by out of gamut colors. You can workaround the problem for now by using the FILMIC tone mapping operator instead of the default ACES_LEGACY.

grassydragon commented 3 years ago

Thank you for the quick reply!

alexey-pelykh commented 3 years ago

@romainguy any ETA on this issue?

pixelflinger commented 3 years ago

No ETA yet. It's probably going to be a while.

ThomasGorisse commented 3 years ago

Sorry for asking it again but we are updating to the last 1.10.6 and we'd like to know if we can suppress our workaround:

filamentView.setColorGrading(
    new ColorGrading.Builder()
        .toneMapping(ColorGrading.ToneMapping.FILMIC)
        .build(EngineInstance.getEngine().getFilamentEngine())
);