Always perform color correction and debanding on nonlinear sRGB values.

Fixes #107730

Co-authored-by: LuoZhihao <[email protected]>

doc/classes/ProjectSettings.xml

@@ -2744,7 +2744,7 @@
 			[b]Note:[/b] This property is only read when the project starts. There is currently no way to change this setting at run-time.
 		</member>
 		<member name="rendering/anti_aliasing/quality/use_debanding" type="bool" setter="" getter="" default="false">
-			If [code]true[/code], uses a fast post-processing filter to make banding significantly less visible in 3D. 2D rendering is [i]not[/i] affected by debanding unless the [member Environment.background_mode] is [constant Environment.BG_CANVAS].
+			If [code]true[/code], uses a fast post-processing filter to make banding significantly less visible. If [member rendering/viewport/hdr_2d] is [code]false[/code], 2D rendering is [i]not[/i] affected by debanding unless the [member Environment.background_mode] is [constant Environment.BG_CANVAS]. If [member rendering/viewport/hdr_2d] is [code]true[/code], debanding will affect all 2D and 3D rendering, including canvas items.
 			In some cases, debanding may introduce a slightly noticeable dithering pattern. It's recommended to enable debanding only when actually needed since the dithering pattern will make lossless-compressed screenshots larger.
 			[b]Note:[/b] This property is only read when the project starts. To set debanding at runtime, set [member Viewport.use_debanding] on the root [Viewport] instead, or use [method RenderingServer.viewport_set_use_debanding].
 		</member>

doc/classes/RenderingServer.xml

@@ -4232,7 +4232,7 @@
 			<param index="0" name="viewport" type="RID" />
 			<param index="1" name="enable" type="bool" />
 			<description>
-				If [code]true[/code], enables debanding on the specified viewport. Equivalent to [member ProjectSettings.rendering/anti_aliasing/quality/use_debanding] or [member Viewport.use_debanding].
+				Equivalent to [member Viewport.use_debanding]. See also [member ProjectSettings.rendering/anti_aliasing/quality/use_debanding].
 			</description>
 		</method>
 		<method name="viewport_set_use_hdr_2d">

doc/classes/Viewport.xml

@@ -444,7 +444,7 @@
 			If [code]true[/code], the viewport should render its background as transparent.
 		</member>
 		<member name="use_debanding" type="bool" setter="set_use_debanding" getter="is_using_debanding" default="false">
-			If [code]true[/code], uses a fast post-processing filter to make banding significantly less visible in 3D. 2D rendering is [i]not[/i] affected by debanding unless the [member Environment.background_mode] is [constant Environment.BG_CANVAS].
+			If [code]true[/code], uses a fast post-processing filter to make banding significantly less visible. If [member use_hdr_2d] is [code]false[/code], 2D rendering is [i]not[/i] affected by debanding unless the [member Environment.background_mode] is [constant Environment.BG_CANVAS]. If [member use_hdr_2d] is [code]true[/code], debanding will only be applied if this is the root [Viewport] and will affect all 2D and 3D rendering, including canvas items.
 			In some cases, debanding may introduce a slightly noticeable dithering pattern. It's recommended to enable debanding only when actually needed since the dithering pattern will make lossless-compressed screenshots larger.
 			See also [member ProjectSettings.rendering/anti_aliasing/quality/use_debanding] and [method RenderingServer.viewport_set_use_debanding].
 		</member>

drivers/gles3/storage/texture_storage.h

@@ -647,6 +647,8 @@ public:
 	virtual void render_target_do_msaa_resolve(RID p_render_target) override {}
 	virtual void render_target_set_use_hdr(RID p_render_target, bool p_use_hdr_2d) override;
 	virtual bool render_target_is_using_hdr(RID p_render_target) const override;
+	virtual void render_target_set_use_debanding(RID p_render_target, bool p_use_debanding) override {}
+	virtual bool render_target_is_using_debanding(RID p_render_target) const override { return false; }
 
 	// new
 	void render_target_set_as_unused(RID p_render_target) override {

servers/rendering/dummy/storage/texture_storage.h

@@ -180,6 +180,8 @@ public:
 	virtual void render_target_do_msaa_resolve(RID p_render_target) override {}
 	virtual void render_target_set_use_hdr(RID p_render_target, bool p_use_hdr_2d) override {}
 	virtual bool render_target_is_using_hdr(RID p_render_target) const override { return false; }
+	virtual void render_target_set_use_debanding(RID p_render_target, bool p_use_debanding) override {}
+	virtual bool render_target_is_using_debanding(RID p_render_target) const override { return false; }
 
 	virtual void render_target_request_clear(RID p_render_target, const Color &p_clear_color) override {}
 	virtual bool render_target_is_clear_requested(RID p_render_target) override { return false; }

servers/rendering/renderer_rd/effects/tone_mapper.cpp

@@ -123,7 +123,8 @@ void ToneMapper::tonemapper(RID p_source_color, RID p_dst_framebuffer, const Ton
 	tonemap.push_constant.flags |= p_settings.use_color_correction ? TONEMAP_FLAG_USE_COLOR_CORRECTION : 0;
 
 	tonemap.push_constant.flags |= p_settings.use_fxaa ? TONEMAP_FLAG_USE_FXAA : 0;
-	tonemap.push_constant.flags |= p_settings.use_debanding ? TONEMAP_FLAG_USE_DEBANDING : 0;
+	// When convert_to_srgb is false: postpone debanding until convert_to_srgb is true (usually during blit).
+	tonemap.push_constant.flags |= (p_settings.use_debanding && p_settings.convert_to_srgb) ? TONEMAP_FLAG_USE_DEBANDING : 0;
 	tonemap.push_constant.pixel_size[0] = 1.0 / p_settings.texture_size.x;
 	tonemap.push_constant.pixel_size[1] = 1.0 / p_settings.texture_size.y;
 
@@ -207,8 +208,8 @@ void ToneMapper::tonemapper(RD::DrawListID p_subpass_draw_list, RID p_source_col
 	tonemap.push_constant.auto_exposure_scale = p_settings.auto_exposure_scale;
 
 	tonemap.push_constant.flags |= p_settings.use_color_correction ? TONEMAP_FLAG_USE_COLOR_CORRECTION : 0;
-
-	tonemap.push_constant.flags |= p_settings.use_debanding ? TONEMAP_FLAG_USE_DEBANDING : 0;
+	// When convert_to_srgb is false: postpone debanding until convert_to_srgb is true (usually during blit).
+	tonemap.push_constant.flags |= (p_settings.use_debanding && p_settings.convert_to_srgb) ? TONEMAP_FLAG_USE_DEBANDING : 0;
 	tonemap.push_constant.luminance_multiplier = p_settings.luminance_multiplier;
 
 	tonemap.push_constant.flags |= p_settings.convert_to_srgb ? TONEMAP_FLAG_CONVERT_TO_SRGB : 0;

servers/rendering/renderer_rd/renderer_compositor_rd.cpp

@@ -96,6 +96,8 @@ void RendererCompositorRD::blit_render_targets_to_screen(DisplayServer::WindowID
 		blit.push_constant.upscale = p_render_targets[i].lens_distortion.upscale;
 		blit.push_constant.aspect_ratio = p_render_targets[i].lens_distortion.aspect_ratio;
 		blit.push_constant.convert_to_srgb = texture_storage->render_target_is_using_hdr(p_render_targets[i].render_target);
+		// If convert_to_srgb is false, debanding was applied earlier (usually in tonemapping).
+		blit.push_constant.use_debanding = uint32_t(blit.push_constant.convert_to_srgb && texture_storage->render_target_is_using_debanding(p_render_targets[i].render_target));
 
 		RD::get_singleton()->draw_list_set_push_constant(draw_list, &blit.push_constant, sizeof(BlitPushConstant));
 		RD::get_singleton()->draw_list_draw(draw_list, true);
@@ -257,6 +259,7 @@ void RendererCompositorRD::set_boot_image(const Ref<Image> &p_image, const Color
 	blit.push_constant.upscale = 1.0;
 	blit.push_constant.aspect_ratio = 1.0;
 	blit.push_constant.convert_to_srgb = false;
+	blit.push_constant.use_debanding = false;
 
 	RD::get_singleton()->draw_list_set_push_constant(draw_list, &blit.push_constant, sizeof(BlitPushConstant));
 	RD::get_singleton()->draw_list_draw(draw_list, true);

servers/rendering/renderer_rd/renderer_compositor_rd.h

@@ -73,7 +73,6 @@ protected:
 
 		float rotation_sin;
 		float rotation_cos;
-		float pad[2];
 
 		float eye_center[2];
 		float k1;
@@ -83,6 +82,8 @@ protected:
 		float aspect_ratio;
 		uint32_t layer;
 		uint32_t convert_to_srgb;
+		uint32_t use_debanding;
+		float pad;
 	};
 
 	struct Blit {

servers/rendering/renderer_rd/renderer_scene_render_rd.cpp

@@ -665,6 +665,7 @@ void RendererSceneRenderRD::_render_buffers_post_process_and_tonemap(const Rende
 		tonemap.use_color_correction = false;
 		tonemap.use_1d_color_correction = false;
 		tonemap.color_correction_texture = texture_storage->texture_rd_get_default(RendererRD::TextureStorage::DEFAULT_RD_TEXTURE_3D_WHITE);
+		tonemap.convert_to_srgb = !texture_storage->render_target_is_using_hdr(render_target);
 
 		if (can_use_effects && p_render_data->environment.is_valid()) {
 			tonemap.use_bcs = environment_get_adjustments_enabled(p_render_data->environment);
@@ -674,15 +675,13 @@ void RendererSceneRenderRD::_render_buffers_post_process_and_tonemap(const Rende
 			if (environment_get_adjustments_enabled(p_render_data->environment) && environment_get_color_correction(p_render_data->environment).is_valid()) {
 				tonemap.use_color_correction = true;
 				tonemap.use_1d_color_correction = environment_get_use_1d_color_correction(p_render_data->environment);
-				tonemap.color_correction_texture = texture_storage->texture_get_rd_texture(environment_get_color_correction(p_render_data->environment));
+				tonemap.color_correction_texture = texture_storage->texture_get_rd_texture(environment_get_color_correction(p_render_data->environment), !tonemap.convert_to_srgb);
 			}
 		}
 
 		tonemap.luminance_multiplier = _render_buffers_get_luminance_multiplier();
 		tonemap.view_count = rb->get_view_count();
 
-		tonemap.convert_to_srgb = !texture_storage->render_target_is_using_hdr(render_target);
-
 		RID dest_fb;
 		if (spatial_upscaler != nullptr || use_smaa) {
 			// If we use a spatial upscaler to upscale or SMAA to antialias we need to write our result into an intermediate buffer.
@@ -824,6 +823,7 @@ void RendererSceneRenderRD::_post_process_subpass(RID p_source_texture, RID p_fr
 	tonemap.use_color_correction = false;
 	tonemap.use_1d_color_correction = false;
 	tonemap.color_correction_texture = texture_storage->texture_rd_get_default(RendererRD::TextureStorage::DEFAULT_RD_TEXTURE_3D_WHITE);
+	tonemap.convert_to_srgb = !texture_storage->render_target_is_using_hdr(rb->get_render_target());
 
 	if (can_use_effects && p_render_data->environment.is_valid()) {
 		tonemap.use_bcs = environment_get_adjustments_enabled(p_render_data->environment);
@@ -833,7 +833,7 @@ void RendererSceneRenderRD::_post_process_subpass(RID p_source_texture, RID p_fr
 		if (environment_get_adjustments_enabled(p_render_data->environment) && environment_get_color_correction(p_render_data->environment).is_valid()) {
 			tonemap.use_color_correction = true;
 			tonemap.use_1d_color_correction = environment_get_use_1d_color_correction(p_render_data->environment);
-			tonemap.color_correction_texture = texture_storage->texture_get_rd_texture(environment_get_color_correction(p_render_data->environment));
+			tonemap.color_correction_texture = texture_storage->texture_get_rd_texture(environment_get_color_correction(p_render_data->environment), !tonemap.convert_to_srgb);
 		}
 	}
 
@@ -843,8 +843,6 @@ void RendererSceneRenderRD::_post_process_subpass(RID p_source_texture, RID p_fr
 	tonemap.luminance_multiplier = _render_buffers_get_luminance_multiplier();
 	tonemap.view_count = rb->get_view_count();
 
-	tonemap.convert_to_srgb = !texture_storage->render_target_is_using_hdr(rb->get_render_target());
-
 	tone_mapper->tonemapper(draw_list, p_source_texture, RD::get_singleton()->framebuffer_get_format(p_framebuffer), tonemap);
 
 	RD::get_singleton()->draw_command_end_label();

servers/rendering/renderer_rd/shaders/blit.glsl

@@ -10,7 +10,6 @@ layout(push_constant, std140) uniform Pos {
 
 	float rotation_sin;
 	float rotation_cos;
-	vec2 pad;
 
 	vec2 eye_center;
 	float k1;
@@ -20,6 +19,8 @@ layout(push_constant, std140) uniform Pos {
 	float aspect_ratio;
 	uint layer;
 	bool convert_to_srgb;
+	bool use_debanding;
+	float pad;
 }
 data;
 
@@ -50,7 +51,6 @@ layout(push_constant, std140) uniform Pos {
 
 	float rotation_sin;
 	float rotation_cos;
-	vec2 pad;
 
 	vec2 eye_center;
 	float k1;
@@ -60,6 +60,8 @@ layout(push_constant, std140) uniform Pos {
 	float aspect_ratio;
 	uint layer;
 	bool convert_to_srgb;
+	bool use_debanding;
+	float pad;
 }
 data;
 
@@ -74,12 +76,27 @@ layout(binding = 0) uniform sampler2D src_rt;
 #endif
 
 vec3 linear_to_srgb(vec3 color) {
-	// If going to srgb, clamp from 0 to 1.
-	color = clamp(color, vec3(0.0), vec3(1.0));
 	const vec3 a = vec3(0.055f);
 	return mix((vec3(1.0f) + a) * pow(color.rgb, vec3(1.0f / 2.4f)) - a, 12.92f * color.rgb, lessThan(color.rgb, vec3(0.0031308f)));
 }
 
+// From https://alex.vlachos.com/graphics/Alex_Vlachos_Advanced_VR_Rendering_GDC2015.pdf
+// and https://www.shadertoy.com/view/MslGR8 (5th one starting from the bottom)
+// NOTE: `frag_coord` is in pixels (i.e. not normalized UV).
+// This dithering must be applied after encoding changes (linear/nonlinear) have been applied
+// as the final step before quantization from floating point to integer values.
+vec3 screen_space_dither(vec2 frag_coord) {
+	// Iestyn's RGB dither (7 asm instructions) from Portal 2 X360, slightly modified for VR.
+	// Removed the time component to avoid passing time into this shader.
+	vec3 dither = vec3(dot(vec2(171.0, 231.0), frag_coord));
+	dither.rgb = fract(dither.rgb / vec3(103.0, 71.0, 97.0));
+
+	// Subtract 0.5 to avoid slightly brightening the whole viewport.
+	// Use a dither strength of 100% rather than the 37.5% suggested by the original source.
+	// Divide by 255 to align to 8-bit quantization.
+	return (dither.rgb - 0.5) / 255.0;
+}
+
 void main() {
 #ifdef APPLY_LENS_DISTORTION
 	vec2 coords = uv * 2.0 - 1.0;
@@ -118,5 +135,10 @@ void main() {
 
 	if (data.convert_to_srgb) {
 		color.rgb = linear_to_srgb(color.rgb); // Regular linear -> SRGB conversion.
+		// When convert_to_srgb is true, debanding was skipped in tonemap.glsl.
+		if (data.use_debanding) {
+			color.rgb += screen_space_dither(gl_FragCoord.xy);
+		}
+		color.rgb = clamp(color.rgb, vec3(0.0), vec3(1.0));
 	}
 }

servers/rendering/renderer_rd/shaders/effects/tonemap.glsl

@@ -328,7 +328,8 @@ vec3 tonemap_agx(vec3 color) {
 }
 
 vec3 linear_to_srgb(vec3 color) {
-	//if going to srgb, clamp from 0 to 1.
+	// Clamping is not strictly necessary for floating point nonlinear sRGB encoding,
+	// but many cases that call this function need the result clamped.
 	color = clamp(color, vec3(0.0), vec3(1.0));
 	const vec3 a = vec3(0.055f);
 	return mix((vec3(1.0f) + a) * pow(color.rgb, vec3(1.0f / 2.4f)) - a, 12.92f * color.rgb, lessThan(color.rgb, vec3(0.0031308f)));
@@ -816,12 +817,17 @@ vec3 do_fxaa(vec3 color, float exposure, vec2 uv_interp) {
 // From https://alex.vlachos.com/graphics/Alex_Vlachos_Advanced_VR_Rendering_GDC2015.pdf
 // and https://www.shadertoy.com/view/MslGR8 (5th one starting from the bottom)
 // NOTE: `frag_coord` is in pixels (i.e. not normalized UV).
+// This dithering must be applied after encoding changes (linear/nonlinear) have been applied
+// as the final step before quantization from floating point to integer values.
 vec3 screen_space_dither(vec2 frag_coord) {
 	// Iestyn's RGB dither (7 asm instructions) from Portal 2 X360, slightly modified for VR.
+	// Removed the time component to avoid passing time into this shader.
 	vec3 dither = vec3(dot(vec2(171.0, 231.0), frag_coord));
 	dither.rgb = fract(dither.rgb / vec3(103.0, 71.0, 97.0));
 
 	// Subtract 0.5 to avoid slightly brightening the whole viewport.
+	// Use a dither strength of 100% rather than the 37.5% suggested by the original source.
+	// Divide by 255 to align to 8-bit quantization.
 	return (dither.rgb - 0.5) / 255.0;
 }
 
@@ -866,7 +872,8 @@ void main() {
 
 	color.rgb = apply_tonemapping(color.rgb, params.white);
 
-	if (bool(params.flags & FLAG_CONVERT_TO_SRGB)) {
+	bool convert_to_srgb = bool(params.flags & FLAG_CONVERT_TO_SRGB);
+	if (convert_to_srgb) {
 		color.rgb = linear_to_srgb(color.rgb); // Regular linear -> SRGB conversion.
 	}
 #ifndef SUBPASS
@@ -879,7 +886,7 @@ void main() {
 
 		// high dynamic range -> SRGB
 		glow = apply_tonemapping(glow, params.white);
-		if (bool(params.flags & FLAG_CONVERT_TO_SRGB)) {
+		if (convert_to_srgb) {
 			glow = linear_to_srgb(glow);
 		}
 
@@ -894,7 +901,13 @@ void main() {
 	}
 
 	if (bool(params.flags & FLAG_USE_COLOR_CORRECTION)) {
+		// apply_color_correction requires nonlinear sRGB encoding
+		if (!convert_to_srgb) {
+			color.rgb = linear_to_srgb(color.rgb);
+		}
 		color.rgb = apply_color_correction(color.rgb);
+		// When convert_to_srgb is false, there is no need to convert back to
+		// linear because the color correction texture sampling does this for us.
 	}
 
 	if (bool(params.flags & FLAG_USE_DEBANDING)) {

servers/rendering/renderer_rd/storage_rd/texture_storage.cpp

@@ -3709,6 +3709,20 @@ bool TextureStorage::render_target_is_using_hdr(RID p_render_target) const {
 	return rt->use_hdr;
 }
 
+void TextureStorage::render_target_set_use_debanding(RID p_render_target, bool p_use_debanding) {
+	RenderTarget *rt = render_target_owner.get_or_null(p_render_target);
+	ERR_FAIL_NULL(rt);
+
+	rt->use_debanding = p_use_debanding;
+}
+
+bool TextureStorage::render_target_is_using_debanding(RID p_render_target) const {
+	RenderTarget *rt = render_target_owner.get_or_null(p_render_target);
+	ERR_FAIL_NULL_V(rt, false);
+
+	return rt->use_debanding;
+}
+
 RID TextureStorage::render_target_get_rd_framebuffer(RID p_render_target) {
 	RenderTarget *rt = render_target_owner.get_or_null(p_render_target);
 	ERR_FAIL_NULL_V(rt, RID());

servers/rendering/renderer_rd/storage_rd/texture_storage.h

@@ -366,6 +366,7 @@ private:
 
 		bool is_transparent = false;
 		bool use_hdr = false;
+		bool use_debanding = false;
 
 		bool sdf_enabled = false;
 
@@ -759,6 +760,8 @@ public:
 	virtual void render_target_do_msaa_resolve(RID p_render_target) override;
 	virtual void render_target_set_use_hdr(RID p_render_target, bool p_use_hdr) override;
 	virtual bool render_target_is_using_hdr(RID p_render_target) const override;
+	virtual void render_target_set_use_debanding(RID p_render_target, bool p_use_debanding) override;
+	virtual bool render_target_is_using_debanding(RID p_render_target) const override;
 
 	void render_target_copy_to_back_buffer(RID p_render_target, const Rect2i &p_region, bool p_gen_mipmaps);
 	void render_target_clear_back_buffer(RID p_render_target, const Rect2i &p_region, const Color &p_color);

servers/rendering/renderer_viewport.cpp

@@ -1401,6 +1401,7 @@ void RendererViewport::viewport_set_use_debanding(RID p_viewport, bool p_use_deb
 		return;
 	}
 	viewport->use_debanding = p_use_debanding;
+	RSG::texture_storage->render_target_set_use_debanding(viewport->render_target, p_use_debanding);
 	_configure_3d_render_buffers(viewport);
 }
 

servers/rendering/storage/texture_storage.h

@@ -158,6 +158,8 @@ public:
 	virtual void render_target_do_msaa_resolve(RID p_render_target) = 0;
 	virtual void render_target_set_use_hdr(RID p_render_target, bool p_use_hdr) = 0;
 	virtual bool render_target_is_using_hdr(RID p_render_target) const = 0;
+	virtual void render_target_set_use_debanding(RID p_render_target, bool p_use_debanding) = 0;
+	virtual bool render_target_is_using_debanding(RID p_render_target) const = 0;
 
 	virtual void render_target_request_clear(RID p_render_target, const Color &p_clear_color) = 0;
 	virtual bool render_target_is_clear_requested(RID p_render_target) = 0;