Wrong Colors When 2D HDR Enabled

[3booodpro]

Godot version

4.2.dev5

System information

Windows 11 / Godot 4.2.dev5 / Intel i7-1355U

Issue description

taking a screenshot of the screen when the 2D HDR enabled results with a really weird color shift

Here's the screenshot that I got:

How should it be:

Steps to reproduce

"Take a screenshot / take a video" when the 2D HDR disabled and compare it with the 2D HDR enabled

here's the code to speed the test process :D

extends Node2D

func _physics_process(delta):
    if Input.is_action_just_pressed("ui_accept"):
        var img = get_viewport().get_texture().get_image()
        img.save_png("res://Screenshot.png")
        get_tree().quit()
    pass

Minimal reproduction project

TestRecording.zip just remember to enable & disable the 2D HDR mode from the project settings

[clayjohn]

When using HDR mode in 2D, we render 2D in linear color space (not gamma color space like before). So the color of the image in the Viewport is going to look different if not altered before viewing.

If you treat that PNG like a linear space image in your image viewer, it should look correct. The problem is that image viewers typically assume that they are getting a gamma encoded image.

I can see how this is a bit unexpected from the user perspective as Godot automagically converts your image into gamme/srgb space automatically when drawing to the screen. One option could be to convert to gamma/srgb space when calling get_texture(), but that might not be a good option for users who are using HDR mode because they want a linear image.

[Calinou]

I think we should document linear to sRGB conversion in the get_image() documentation. The Screen Capture demo should also showcase this.

[clayjohn]

Just to add to this issue, we need to discuss a proper fix as a straightforward fix will likely lead to confusion.

First of all, this bug report could be closed with the below code that forces the HDR render target to sRGB space before returning it in get_image().

diff --git a/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp b/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp
index 6e5e8f63e0..6ec7d525f7 100644
--- a/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp
+++ b/servers/rendering/renderer_rd/storage_rd/texture_storage.cpp
@@ -1304,6 +1304,11 @@ Ref<Image> TextureStorage::texture_2d_get(RID p_texture) const {
                image->convert(tex->format);
        }

+       if (tex->is_render_target && tex->render_target->use_hdr) {
+               image->convert(Image::FORMAT_RGBA8);
+               image->linear_to_srgb(); // This function needs to be added.
+       }
+
 #ifdef TOOLS_ENABLED
        if (Engine::get_singleton()->is_editor_hint() && !tex->is_render_target) {
                tex->image_cache_2d = image;

However, this code would force users to always read back linear HDR render targets in LDR sRGB space.

As background, we added the 2D HDR mode for two reasons:

1. To allow users to take advantage of 3D effects like glow
2. To allow users to operate in linear sRGB space for the entire rendering pipeline.

The users who need it for reason number 2, need the data to stay as it is and not be changed arbitrarily by Godot. Making the change I suggest above would break use-case number 2 (only when using get_image(), it would still work when using the texture directly). Additionally, with that change, the data returned by the ViewportTexture when read in a shader would be different from the data when read on the CPU. I think that will need to more confusion in the long run.

I have a feeling the best plan of action will be:

1. Document the fact that the data returned by an HDR Viewport is in linear HDR space (since the linear->sRGB conversion happens when drawing to the screen
2. Implement Image::linear_to_srgb()
3. Point users to Image::linear_to_srgb()
4. Make the movie mode always convert to RGBA8 + sRGB (to solve https://github.com/godotengine/godot/issues/81301)

@Calinou Can you think of any reason why movie mode should use RGBA16 instead of RGBA8? My assumptions is that users of the movie maker mode will want RGBA8 output.

[Calinou]

@Calinou Can you think of any reason why movie mode should use RGBA16 instead of RGBA8? My assumptions is that users of the movie maker mode will want RGBA8 output.

RGBA16 output is relevant when you want MovieWriter to write linear (non-tonemapped) output for later compositing in external software, or when you're writing the depth buffer to the alpha channel (also for compositing purposes). We don't have an easy way to write the depth buffer to the alpha channel yet, but it might be feasible using a custom post-processing shader.

That said, for 99% of use cases, writing RGBA8 is fine. I think it makes sense to always convert to RGBA8 sRGB for now, then we can add a project setting later once users need more advanced use cases (and there is a way to properly write this data[^1]).

[^1]: Writing PNG always writes 8 bpc images right now. It can't write 16 bpc, so the higher-precision data wouldn't be retained when saving images.