Skip to content

Commit

Permalink
Fix inverse tone mapping issues (google#4437)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
romainguy authored Aug 3, 2021
1 parent af6f19e commit 91b5cc4
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 23 deletions.
4 changes: 2 additions & 2 deletions docs/Materials.html
Original file line number Diff line number Diff line change
Expand Up @@ -2639,8 +2639,8 @@
<tr><td style="text-align:left"> <strong class="asterisk">getUV0()</strong> </td><td style="text-align:center"> float2 </td><td style="text-align:left"> First interpolated set of UV coordinates, only available if the uv0 attribute is required </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">getUV1()</strong> </td><td style="text-align:center"> float2 </td><td style="text-align:left"> First interpolated set of UV coordinates, only available if the uv1 attribute is required </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">getMaskThreshold()</strong> </td><td style="text-align:center"> float </td><td style="text-align:left"> Returns the mask threshold, only available when <code>blending</code> is set to <code>masked</code> </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">inverseTonemap(float3)</strong> </td><td style="text-align:center"> float3 </td><td style="text-align:left"> Applies the inverse tone mapping operator to the specified linear sRGB color and returns a linear sRGB color. This operation may be an approximation </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">inverseTonemapSRGB(float3)</strong> </td><td style="text-align:center"> float3 </td><td style="text-align:left"> 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 </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">inverseTonemap(float3)</strong> </td><td style="text-align:center"> float3 </td><td style="text-align:left"> 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 </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">inverseTonemapSRGB(float3)</strong> </td><td style="text-align:center"> float3 </td><td style="text-align:left"> 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 </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">luminance(float3)</strong> </td><td style="text-align:center"> float </td><td style="text-align:left"> Computes the luminance of the specified linear sRGB color </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">ycbcrToRgb(float, float2)</strong> </td><td style="text-align:center"> float3 </td><td style="text-align:left"> Converts a luminance and CbCr pair to a sRGB color </td></tr>
<tr><td style="text-align:left"> <strong class="asterisk">uvToRenderTargetUV(float2)</strong> </td><td style="text-align:center"> float2 </td><td style="text-align:left"> Transforms a UV coordinate to allow sampling from a <code>RenderTarget</code> attachment </td></tr>
Expand Down
4 changes: 2 additions & 2 deletions docs/Materials.md.html
Original file line number Diff line number Diff line change
Expand Up @@ -2177,8 +2177,8 @@
**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
Expand Down
15 changes: 7 additions & 8 deletions filament/src/ColorGrading.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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);
Expand All @@ -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);

Expand Down
12 changes: 12 additions & 0 deletions filament/src/ColorSpace.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions filament/src/ToneMapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions samples/gltf_viewer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
9 changes: 4 additions & 5 deletions shaders/src/common_graphics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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)));
}

/**
Expand All @@ -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));
}

//------------------------------------------------------------------------------
Expand Down

0 comments on commit 91b5cc4

Please sign in to comment.