Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulkan: Cannot retrieve HDR texture from Viewport (LDR texture is returned instead) #54122

Closed
thunderk opened this issue Oct 22, 2021 · 13 comments · Fixed by #80215
Closed

Vulkan: Cannot retrieve HDR texture from Viewport (LDR texture is returned instead) #54122

thunderk opened this issue Oct 22, 2021 · 13 comments · Fixed by #80215

Comments

@thunderk
Copy link

Godot version

4.0.dev

System information

Vulkan API 1.2.162 - NVIDIA - GeForce GT 710

Issue description

In Godot 3.x, when a Viewport was configured with hdr flag enabled, the result of get_texture().get_data() was an RGBAHalf image.

In master branch, the hdr flag disappeared (because, as I understand, it's always HDR now), but calling get_texture().get_image() results in an RGB8 image, losing the high dynamic range.

Steps to reproduce

  • Create a SubViewport $vp
  • In GDScript : print($vp.get_texture().get_image().data.format)

Minimal reproduction project

No response

@Calinou Calinou added this to the 4.0 milestone Oct 22, 2021
@Calinou Calinou changed the title Cannot retrieve HDR texture from Viewport Vulkan: Cannot retrieve HDR texture from Viewport (LDR texture is returned instead) Oct 22, 2021
@Anutrix
Copy link
Contributor

Anutrix commented Oct 29, 2021

@thunderk Can you mention exact 3.X version? It'll help.

@thunderk
Copy link
Author

thunderk commented Nov 2, 2021

@Anutrix I can confirm it was working in 3.3.4, but I believe I'm using this feature since at least 3.2

@BastiaanOlij
Copy link
Contributor

In Godot 4 only the 3D rendering is performed in RGBA16F HDR buffers.

In the stage where we apply tone mapping, color correction, exposure and linear -> sRGB conversion the 3D image is resolved into the 2D end result which is a RGBA8 buffer. Then 2D rendering is performed on top of that in sRGB color space. That is the buffer returned, as is, when accessing the image data for a viewport.

Further more, in the clustered renderer the 3D buffers are retained, but in the mobile renderer this result (which is using a R10G10B10A2 buffer) isn't kept, it is only handled in tile memory and never written out (performance reasons). Neither are accessible outside of the rendering engine.

@BastiaanOlij
Copy link
Contributor

Vulkan is able to handle the linear->sRGB and sRGB->linear conversion just like OpenGL but with more control over it. So when you access a buffer in a shader it will do the proper conversions and provide the data correctly. But RGBA8 buffers remain stored in sRGB color space.

@lyuma
Copy link
Contributor

lyuma commented May 29, 2022

I think a suggestion would be to have a "RAW mode" option in Viewport which disables most environment effects ("tone mapping, color correction, exposure and linear -> sRGB conversion") as well as disable 2d rendering (or permit 2d rendering directly to HDR while in this mode).

Generally if you are looking to use HDR output, you also don't want to run exposure or color correction (otherwise, exposure/color grading will be run twice when you output this HDR texture to the screen).

@BastiaanOlij
Copy link
Contributor

i was mentioning that to Fire earlier today, the problem we have with the viewport approach at the moment is that we assume what we render, is the final output we display on screen. It isn't always the case.

The issue we have in accomplishing changing this is ultimately that we're trying to combine a 3D and 2D renderer together so we can overlay UI elements ontop of 3D output, while 2D is handled on the final output.

So there is some restructuring of the renderer needed to have more control over what the output is and thus keeping a 3D viewport that contains an intermediate result as an HDR viewport and thus remove 2D rendering capability.

The same btw for creating a viewport that allows you to render non color data, needed both for 2D and 3D reasons. this should have no conversion at all.

@JonqsGames
Copy link
Contributor

JonqsGames commented Jun 2, 2022

I'm trying to make a quick fix for that to be able to do some post process effect. Using Rendering Server feature i was able to change the texture format of the viewport to RGBAF but this just give me a black output.
I'm having a hard time finding the code where the viewport output is served as an image in the engine. Can someone point to the relevant piece of code ?

@clayjohn
Copy link
Member

clayjohn commented Jun 2, 2022

@JOjox The problem here is not that the wrong format is being returned, the viewport texture is an RGBA8 texture. It is set here:

//until we implement support for HDR monitors (and render target is attached to screen), this is enough.
rt->color_format = RD::DATA_FORMAT_R8G8B8A8_UNORM;
rt->color_format_srgb = RD::DATA_FORMAT_R8G8B8A8_SRGB;
rt->image_format = rt->is_transparent ? Image::FORMAT_RGBA8 : Image::FORMAT_RGB8;

3D rendering is tonemapped into the 0-1 range then converted to sRGB before being copied into the RGB8 buffer. All 2D rendering is done in sRGB space and targetting the RGB8 buffer. That is to say, there is more involved here than just changing a texture format.

@JonqsGames
Copy link
Contributor

JonqsGames commented Jun 2, 2022

Thanks for pointing that piece of code, i have a bit more understanding on how it works.
Now i understand what @BastiaanOlij said about every usage of viewport is made with the idea of rendering directly to the screen. I managed to isolate the case of subviewport (using the Disable 3D flag we could isolate the case where we don't want to enforce format for screen needs).
I changed the render target color and image format for the proper one, the issue i have now is that the image produced by the viewport is still in FORMAT_RGBA8. So i got an error of mismatching size when trying to copy the buffer data.
@clayjohn I understand that even after this i'll still have to disable tone mapping and any convertion, i'm trying to go step by step.

@woodbyte
Copy link

woodbyte commented Dec 5, 2022

I think a suggestion would be to have a "RAW mode" option in Viewport which disables most environment effects ("tone mapping, color correction, exposure and linear -> sRGB conversion") as well as disable 2d rendering (or permit 2d rendering directly to HDR while in this mode).

Generally if you are looking to use HDR output, you also don't want to run exposure or color correction (otherwise, exposure/color grading will be run twice when you output this HDR texture to the screen).

Exactly. If you are rendering 3D stuff in a Viewport and require a linear HDR output from it, that's probably because you need the unclamped data to apply some post-processing to the final composite image (eg. Glow).

This is what I was doing in 3.5 (using HDR and Keep 3D Linear) and now don't see any way to replicate the same in Godot 4. I considered lowering the exposure of the SubViewport scene and then correct it in a Sky shader, but I could only do that if the SubViewport texture was in Format_RGBAF. Trying that with Format_RGBA8 is sure to disappoint.

@sserafimescu
Copy link

In my terraforming game project, I used to make extensive use of Godot 3's HDR viewports to store non-color data, which needs to be high precision and linear (at least 16bit floats, and non post-processed in any way). I am shocked to see that, while Godot 3 is now getting 32bit float color format support for viewport textures, Godot 4 comes with RGBA8 only. Ideally I would want to be able to render to any color format, including R32, RG16, RGBA32, RGBA16 etc, and be able to specify the fallback color formats, not have them autoselected by an opaque algorithm. The assumption that a viewport's output is a color image, which needs to be tonemapped and converted to 8bit sRGB before being served, is very restrictive. I mean, most viewports generate images that are then used as textures in other viewports, so the tonemapping/sRGB conversion will happen again anyway. I will look into pulling one of the PRs which circumvent this issue into my own Godot 4 fork, but I was really hoping I wouldn't need to start maintaining forks of the engine. I'm still hoping this will get addressed soon on master.

@VantaGhost
Copy link

This feature regression is preventing me from implementing post processing effects using either Viewports or screen_texture. I'm in the process of making a screen space lens flare shader, but I'm currently limited to testing in LDR.
I cannot implement my post processing until HDR is added to Viewports and screen_texture.

@JonqsGames
Copy link
Contributor

I'm still trying to have #70970 merged, you can give it a try and it should allow HDR in viewport.

@clayjohn clayjohn modified the milestones: 4.1, 4.x Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment