From 5efa6ba4892aa03f56215719a347be2f1629e206 Mon Sep 17 00:00:00 2001 From: Rie Date: Sat, 15 Jun 2024 23:27:00 +0200 Subject: [PATCH] Fix incorrect Reinhard tonemap operator --- doc/classes/Environment.xml | 2 +- doc/classes/RenderingServer.xml | 2 +- drivers/gles3/shaders/tonemap_inc.glsl | 8 ++++++-- .../rendering/renderer_rd/shaders/effects/tonemap.glsl | 8 ++++++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/doc/classes/Environment.xml b/doc/classes/Environment.xml index 8c265098126..de3295fbe04 100644 --- a/doc/classes/Environment.xml +++ b/doc/classes/Environment.xml @@ -414,7 +414,7 @@ Linear tonemapper operator. Reads the linear data and passes it on unmodified. This can cause bright lighting to look blown out, with noticeable clipping in the output colors. - Reinhardt tonemapper operator. Performs a variation on rendered pixels' colors by this formula: [code]color = color / (1 + color)[/code]. This avoids clipping bright highlights, but the resulting image can look a bit dull. + Reinhard tonemapper operator. Performs a variation on rendered pixels' colors by this formula: [code]color = color * (1 + color / (white * white)) / (1 + color)[/code]. This avoids clipping bright highlights, but the resulting image can look a bit dull. When [member tonemap_white] is left at the default value of [code]1.0[/code] this is identical to [constant TONE_MAPPER_LINEAR] while also being slightly less performant. Filmic tonemapper operator. This avoids clipping bright highlights, with a resulting image that usually looks more vivid than [constant TONE_MAPPER_REINHARDT]. diff --git a/doc/classes/RenderingServer.xml b/doc/classes/RenderingServer.xml index 144f78349f4..4b2ce6f45ee 100644 --- a/doc/classes/RenderingServer.xml +++ b/doc/classes/RenderingServer.xml @@ -5219,7 +5219,7 @@ Output color as they came in. This can cause bright lighting to look blown out, with noticeable clipping in the output colors. - Use the Reinhard tonemapper. Performs a variation on rendered pixels' colors by this formula: [code]color = color / (1 + color)[/code]. This avoids clipping bright highlights, but the resulting image can look a bit dull. + Use the Reinhard tonemapper. Performs a variation on rendered pixels' colors by this formula: [code]color = color * (1 + color / (white * white)) / (1 + color)[/code]. This avoids clipping bright highlights, but the resulting image can look a bit dull. When [member Environment.tonemap_white] is left at the default value of [code]1.0[/code] this is identical to [constant ENV_TONE_MAPPER_LINEAR] while also being slightly less performant. Use the filmic tonemapper. This avoids clipping bright highlights, with a resulting image that usually looks more vivid than [constant ENV_TONE_MAPPER_REINHARD]. diff --git a/drivers/gles3/shaders/tonemap_inc.glsl b/drivers/gles3/shaders/tonemap_inc.glsl index fb915aeb383..6738bdf748c 100644 --- a/drivers/gles3/shaders/tonemap_inc.glsl +++ b/drivers/gles3/shaders/tonemap_inc.glsl @@ -76,8 +76,12 @@ vec3 tonemap_aces(vec3 color, float p_white) { return color_tonemapped / p_white_tonemapped; } +// Based on Reinhard's extended formula, see equation 4 in https://doi.org/cjbgrt vec3 tonemap_reinhard(vec3 color, float p_white) { - return (p_white * color + color) / (color * p_white + p_white); + float white_squared = p_white * p_white; + vec3 white_squared_color = white_squared * color; + // Equivalent to color * (1 + color / white_squared) / (1 + color) + return (white_squared_color + color * color) / (white_squared_color + white_squared); } #define TONEMAPPER_LINEAR 0 @@ -85,7 +89,7 @@ vec3 tonemap_reinhard(vec3 color, float p_white) { #define TONEMAPPER_FILMIC 2 #define TONEMAPPER_ACES 3 -vec3 apply_tonemapping(vec3 color, float p_white) { // inputs are LINEAR, always outputs clamped [0;1] color +vec3 apply_tonemapping(vec3 color, float p_white) { // inputs are LINEAR // Ensure color values passed to tonemappers are positive. // They can be negative in the case of negative lights, which leads to undesired behavior. if (tonemapper == TONEMAPPER_LINEAR) { diff --git a/servers/rendering/renderer_rd/shaders/effects/tonemap.glsl b/servers/rendering/renderer_rd/shaders/effects/tonemap.glsl index 841f02f673e..fa3b45a9627 100644 --- a/servers/rendering/renderer_rd/shaders/effects/tonemap.glsl +++ b/servers/rendering/renderer_rd/shaders/effects/tonemap.glsl @@ -256,8 +256,12 @@ vec3 tonemap_aces(vec3 color, float white) { return color_tonemapped / white_tonemapped; } +// Based on Reinhard's extended formula, see equation 4 in https://doi.org/cjbgrt vec3 tonemap_reinhard(vec3 color, float white) { - return (white * color + color) / (color * white + white); + float white_squared = white * white; + vec3 white_squared_color = white_squared * color; + // Equivalent to color * (1 + color / white_squared) / (1 + color) + return (white_squared_color + color * color) / (white_squared_color + white_squared); } vec3 linear_to_srgb(vec3 color) { @@ -272,7 +276,7 @@ vec3 linear_to_srgb(vec3 color) { #define TONEMAPPER_FILMIC 2 #define TONEMAPPER_ACES 3 -vec3 apply_tonemapping(vec3 color, float white) { // inputs are LINEAR, always outputs clamped [0;1] color +vec3 apply_tonemapping(vec3 color, float white) { // inputs are LINEAR // Ensure color values passed to tonemappers are positive. // They can be negative in the case of negative lights, which leads to undesired behavior. if (params.tonemapper == TONEMAPPER_LINEAR) {