From 1c51164e7bd869aac3f7854b7844cd4544fc8489 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Mon, 2 Aug 2021 18:53:35 -0700 Subject: [PATCH] Fix inverse tone mapping issues (#4437) Bring color grading back into the Rec.709 color space to match previous behaviors. This change also implements an exact inverse tone map function for the "Filmic" operator. --- docs/Materials.html | 4 ++-- docs/Materials.md.html | 4 ++-- filament/src/ColorGrading.cpp | 15 +++++++-------- filament/src/ColorSpace.h | 12 ++++++++++++ filament/src/ToneMapper.cpp | 4 ++-- samples/gltf_viewer.cpp | 4 ---- shaders/src/common_graphics.fs | 9 ++++----- 7 files changed, 29 insertions(+), 23 deletions(-) diff --git a/docs/Materials.html b/docs/Materials.html index 0a7ab5dc87..9cdca300e2 100644 --- a/docs/Materials.html +++ b/docs/Materials.html @@ -2641,8 +2641,8 @@ The following APIs are only available from the fragment block: getUV0() float2 First interpolated set of UV coordinates, only available if the uv0 attribute is required getUV1() float2 First interpolated set of UV coordinates, only available if the uv1 attribute is required getMaskThreshold() float Returns the mask threshold, only available when blending is set to masked - inverseTonemap(float3) float3 Applies the inverse tone mapping operator to the specified linear sRGB color and returns a linear sRGB color. This operation may be an approximation - inverseTonemapSRGB(float3) float3 Applies the inverse tone mapping operator to the specified non-linear sRGB color and returns a linear sRGB color. This operation may be an approximation + inverseTonemap(float3) float3 Applies the inverse tone mapping operator to the specified linear sRGB color and returns a linear sRGB color. This operation may be an approximation and works best with the “Filmic” tone mapping operator + inverseTonemapSRGB(float3) float3 Applies the inverse tone mapping operator to the specified non-linear sRGB color and returns a linear sRGB color. This operation may be an approximation and works best with the “Filmic” tone mapping operator luminance(float3) float Computes the luminance of the specified linear sRGB color ycbcrToRgb(float, float2) float3 Converts a luminance and CbCr pair to a sRGB color uvToRenderTargetUV(float2) float2 Transforms a UV coordinate to allow sampling from a RenderTarget attachment diff --git a/docs/Materials.md.html b/docs/Materials.md.html index a41e1a2c87..5150317f4f 100644 --- a/docs/Materials.md.html +++ b/docs/Materials.md.html @@ -2179,8 +2179,8 @@ The following APIs are only available from the fragment block: **getUV0()** | float2 | First interpolated set of UV coordinates, only available if the uv0 attribute is required **getUV1()** | float2 | First interpolated set of UV coordinates, only available if the uv1 attribute is required **getMaskThreshold()** | float | Returns the mask threshold, only available when `blending` is set to `masked` -**inverseTonemap(float3)** | float3 | Applies the inverse tone mapping operator to the specified linear sRGB color and returns a linear sRGB color. This operation may be an approximation -**inverseTonemapSRGB(float3)** | float3 | Applies the inverse tone mapping operator to the specified non-linear sRGB color and returns a linear sRGB color. This operation may be an approximation +**inverseTonemap(float3)** | float3 | Applies the inverse tone mapping operator to the specified linear sRGB color and returns a linear sRGB color. This operation may be an approximation and works best with the "Filmic" tone mapping operator +**inverseTonemapSRGB(float3)** | float3 | Applies the inverse tone mapping operator to the specified non-linear sRGB color and returns a linear sRGB color. This operation may be an approximation and works best with the "Filmic" tone mapping operator **luminance(float3)** | float | Computes the luminance of the specified linear sRGB color **ycbcrToRgb(float, float2)** | float3 | Converts a luminance and CbCr pair to a sRGB color **uvToRenderTargetUV(float2)** | float2 | Transforms a UV coordinate to allow sampling from a `RenderTarget` attachment diff --git a/filament/src/ColorGrading.cpp b/filament/src/ColorGrading.cpp index 46dd1bc69a..c1dc3eb965 100644 --- a/filament/src/ColorGrading.cpp +++ b/filament/src/ColorGrading.cpp @@ -517,8 +517,7 @@ FColorGrading::FColorGrading(FEngine& engine, const Builder& builder) { v = chromaticAdaptation(v, config.adaptationTransform); } - // Convert to color grading color space - v = sRGB_to_REC2020 * v; + // TODO: We should convert to a color grading color space here if (builder->hasAdjustments) { // Kill negative values before the next transforms @@ -528,7 +527,7 @@ FColorGrading::FColorGrading(FEngine& engine, const Builder& builder) { v = channelMixer(v, builder->outRed, builder->outGreen, builder->outBlue); // Shadows/mid-tones/highlights - v = tonalRanges(v, LUMA_REC2020, + v = tonalRanges(v, LUMA_REC709, builder->shadows, builder->midtones, builder->highlights, builder->tonalRanges); @@ -546,10 +545,10 @@ FColorGrading::FColorGrading(FEngine& engine, const Builder& builder) { v = LogC_to_linear(v); // Vibrance in linear space - v = vibrance(v, LUMA_REC2020, builder->vibrance); + v = vibrance(v, LUMA_REC709, builder->vibrance); // Saturation in linear space - v = saturation(v, LUMA_REC2020, builder->saturation); + v = saturation(v, LUMA_REC709, builder->saturation); // Kill negative values before tone mapping v = max(v, 0.0f); @@ -566,9 +565,9 @@ FColorGrading::FColorGrading(FEngine& engine, const Builder& builder) { v = (*builder->toneMapper)(v); } - // Convert to output color space - // TODO: allow to customize the output color space, - v = REC2020_to_sRGB * v; + // TODO: We should convert to the output color space if we use a working + // color space that's not sRGB + // TODO: Allow the user to customize the output color space v = saturate(v); diff --git a/filament/src/ColorSpace.h b/filament/src/ColorSpace.h index 4dfb73f3fe..976a7ed4ae 100644 --- a/filament/src/ColorSpace.h +++ b/filament/src/ColorSpace.h @@ -112,6 +112,18 @@ constexpr mat3f sRGB_to_AP1{ 0.04737f, 0.01345f, 0.86961f }; +constexpr mat3f AP0_to_sRGB{ + 2.52169f, -0.27648f, -0.01538f, + -1.13413f, 1.37272f, -0.15298f, + -0.38756f, -0.09624f, 1.16835f +}; + +constexpr mat3f sRGB_to_AP0{ + 0.4397010f, 0.0897923f, 0.0175440f, + 0.3829780f, 0.8134230f, 0.1115440f, + 0.1773350f, 0.0967616f, 0.8707040f +}; + constexpr mat3f sRGB_to_REC2020 = XYZ_to_REC2020 * sRGB_to_XYZ; constexpr mat3f REC2020_to_sRGB = XYZ_to_sRGB * REC2020_to_XYZ; diff --git a/filament/src/ToneMapper.cpp b/filament/src/ToneMapper.cpp index 0063bb7ac6..7fdf4eb189 100644 --- a/filament/src/ToneMapper.cpp +++ b/filament/src/ToneMapper.cpp @@ -135,7 +135,7 @@ float3 ACES(float3 color, float brightness) noexcept { constexpr float RRT_SAT_FACTOR = 0.96f; constexpr float ODT_SAT_FACTOR = 0.93f; - float3 ap0 = REC2020_to_AP0 * color; + float3 ap0 = sRGB_to_AP0 * color; // Glow module float saturation = rgb_2_saturation(ap0); @@ -177,7 +177,7 @@ float3 ACES(float3 color, float brightness) noexcept { // Apply desaturation to compensate for luminance difference linearCV = mix(float3(dot(linearCV, LUMA_AP1)), linearCV, ODT_SAT_FACTOR); - return AP1_to_REC2020 * linearCV; + return AP1_to_sRGB * linearCV; } } // namespace aces diff --git a/samples/gltf_viewer.cpp b/samples/gltf_viewer.cpp index 0d4cccb940..05287b9319 100644 --- a/samples/gltf_viewer.cpp +++ b/samples/gltf_viewer.cpp @@ -343,10 +343,6 @@ static void createGroundPlane(Engine* engine, Scene* scene, App& app) { app.scene.groundMaterial = shadowMaterial; } -static LinearColor inverseTonemapSRGB(sRGBColor x) { - return (x * -0.155) / (x - 1.019); -} - static float sGlobalScale = 1.0f; static float sGlobalScaleAnamorphism = 0.0f; diff --git a/shaders/src/common_graphics.fs b/shaders/src/common_graphics.fs index 68387a1aa5..7e342841e7 100644 --- a/shaders/src/common_graphics.fs +++ b/shaders/src/common_graphics.fs @@ -47,8 +47,8 @@ vec3 ycbcrToRgb(float luminance, vec2 cbcr) { /* * The input must be in the [0, 1] range. */ -vec3 Inverse_Tonemap_Unreal(const vec3 x) { - return (x * -0.155) / (x - 1.019); +vec3 Inverse_Tonemap_Filmic(const vec3 x) { + return (0.03 - 0.59 * x - sqrt(0.0009 + 1.3702 * x - 1.0127 * x * x)) / (-5.02 + 4.86 * x); } /** @@ -61,7 +61,7 @@ vec3 Inverse_Tonemap_Unreal(const vec3 x) { vec3 inverseTonemapSRGB(vec3 color) { // sRGB input color = clamp(color, 0.0, 1.0); - return Inverse_Tonemap_Unreal(color); + return Inverse_Tonemap_Filmic(pow(color, vec3(2.2))); } /** @@ -73,8 +73,7 @@ vec3 inverseTonemapSRGB(vec3 color) { */ vec3 inverseTonemap(vec3 linear) { // Linear input - linear = clamp(linear, 0.0, 1.0); - return Inverse_Tonemap_Unreal(pow(linear, vec3(1.0 / 2.2))); + return Inverse_Tonemap_Filmic(clamp(linear, 0.0, 1.0)); } //------------------------------------------------------------------------------