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

Add option to enable HDR rendering in 2D #80215

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Aug 3, 2023

Fixes: #75153
Fixes: #62110
Fixes: #54122
Fixes: #74785

This is needed to allow 2D to fully make use of 3D effects (e.g. glow), and can be used to substantially improve quality of 2D rendering at the cost of performance

Additionally, the 2D rendering pipeline is done in linear space (we skip linear_to_srgb conversion in 3D tonemapping) so the entire Viewport can be kept linear. This is necessary for proper HDR screen support in the future.

Implementation notes

  1. I did not do the additional optimization of combing the color render buffer with the render target when possible. I want to do that in a follow up PR as it will be a high risk change and this isn't
  2. I separated p_use_linear_color into two flags p_use_linear_color and p_3d_material so that 2D materials with linear don't get improperly flagged as 3D materials and have their textures compressed. A side benefit of this is textures used in particles shaders won't get flagged as 3D anymore.
  3. I made hdr_2d a property of viewports in the end as the visual difference between rendering in linear made a surprisingly small change to visuals.
  4. One risk factor I foresee is users enabling HDR on subviewports when the Viewport attached to the window isn't in HDR. In those cases, they need to convert to SDR in the shader. This PR doesn't add a warning in such cases (I don't think it is possible). On the other hand, if all viewports in a chain are in HDR this just works out of the box and looks fine.

Glow working in 2D, notice how the viewport preview is in linear space?
The green icon is in its own subviewport and still contributes to glow in the main viewport
Screenshot from 2023-08-03 12-01-32

This highlights that ViewportTextures can now be used for values above 1
Screenshot from 2023-08-03 12-05-17

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation / code style pass.

doc/classes/RenderingServer.xml Outdated Show resolved Hide resolved
doc/classes/Viewport.xml Outdated Show resolved Hide resolved
scene/main/viewport.h Outdated Show resolved Hide resolved
servers/rendering/renderer_rd/effects/tone_mapper.cpp Outdated Show resolved Hide resolved
servers/rendering/renderer_rd/effects/tone_mapper.cpp Outdated Show resolved Hide resolved
servers/rendering/renderer_rd/shaders/blit.glsl Outdated Show resolved Hide resolved
servers/rendering/renderer_rd/shaders/blit.glsl Outdated Show resolved Hide resolved
servers/rendering/renderer_rd/shaders/effects/tonemap.glsl Outdated Show resolved Hide resolved
drivers/gles3/shaders/canvas_uniforms_inc.glsl Outdated Show resolved Hide resolved
This is needed to allow 2D to fully make use of 3D effects (e.g. glow), and can be used to substantially improve quality of 2D rendering at the cost of performance

Additionally, the 2D rendering pipeline is done in linear space (we skip linear_to_srgb conversion in 3D tonemapping) so the entire Viewport can be kept linear.
This is necessary for proper HDR screen support in the future.
@clayjohn
Copy link
Member Author

clayjohn commented Aug 7, 2023

Updated with all the suggestions from review!

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style wise it looks good.

CC @BastiaanOlij for technical review.

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, exciting change!

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

Some comments:

  • The HDR 2D toggle in the project settings works instantly (despite the warning), but the clear color is too dark until you restart the editor.
  • Clear color is lighter on mobile renderer #79931 affects 2D rendering when using the Mobile rendering method too (which is expected).
  • HDR in 2D is not available when using the Compatibility rendering method, and the maximum brightness is more limited when using Mobile. This should be noted in the class reference description.
  • Should the project setting be moved to Rendering > 2D? This way, we don't have to create yet another section in the Project Settings.

HDR 2D disabled

Screenshot_20230808_150148

HDR 2D enabled (before restarting the editor)

Screenshot_20230808_150154

@akien-mga akien-mga merged commit 1163dac into godotengine:master Aug 8, 2023
15 checks passed
@akien-mga
Copy link
Member

akien-mga commented Aug 8, 2023

Thanks!

Merged already to get wider testing, but @Calinou's comments may still be worth addressing.

@Carbonateb
Copy link

Seems like 2D HDR on the Mobile renderer has some significant problems. I've detailed what I found below, if it helps!

2D HDR Issues

There are some colour banding issues when HDR is turned on.
It seems to affect colours below about #252525.

To demonstrate, I created a gradient in paint.net between #000 and #252525 and loaded it into Godot.
Here's how it looks with HDR turned off:

image

And here it is with HDR turned on:

image

Here's a visualization of the difference between the two image (Image 1 XOR Image 2). Black pixels mean the two images perfectly match, while coloured pixels show where they differ.

image


Bloom is also quite affected. Here are three squares, I've used an extremely simple shader to set their colour.

image

  • The middle one is pure white but not overbright, so it should not be glowing at all.
  • The right one should definitely be glowing, and be much brighter than the middle one.
  • The left square also has a major issue - the square should be #7f7f7f, but it is being completely overexposed, rendering as pure white #ffffff.

You can also see the colour banding issues around the edges of the bloom.

I'd suggest limiting 2D HDR to only support the Forward+ renderer to avoid confusion for those using the Mobile renderer.

I'm running Windows 11, with an RTX 2070 on the latest graphic drivers.

Everything looks great on the Forward+ renderer though, nice work 👍

@clayjohn clayjohn deleted the HDR-2D branch October 25, 2023 12:21
@clayjohn
Copy link
Member Author

@Carbonateb Those issues have nothing to do with 2D rendering.

The Mobile render uses much less precision than the Forward+ renderer for 3D rendering which is where those banding issues are coming from (RGB10_A2 vs RGBA16). Just because 2D is using high precision, doesn't mean that 3D has changed. The Mobile renderer also uses a limited dynamic range of 0-2 (which is where the glow issues are coming from). When using the Mobile renderer you unfortunately have to keep the limitations of the Mobile renderer in mind.

Most of the important limitations are listed here: https://docs.godotengine.org/en/latest/contributing/development/core_and_modules/internal_rendering_architecture.html#forward-mobile

@Carbonateb
Copy link

Forgive me for potentially missing something obvious, but I'm not sure what the 3D renderer has to do with this? The banding examples I gave occurred purely on the 2D renderer, I did not render them in a 3D viewport.

I understand that the mobile renderer uses less precision, but I would not expect that to change when the glow starts to appear. E.g. a pure white object should not glow when the cut-off is set to 1. Also, it does not explain why a shader outputting (0.5, 0.5, 0.5) displays as white when HDR is enabled (the left square).

I'm actually a bit puzzled about what's going on here - I would have expected smaller numbers to have more precision as more bits are "allocated" to the fractional part, with precision decreasing as the colour gets brighter. I'd appreciate a quick rundown if you have time!

@Carbonateb
Copy link

Also, I've switched to the Forward+ renderer on my project, I noticed one small issue.

Certain Control nodes appear washed out when HDR is enabled:

image

The left side has HDR on, the right side doesn't. The top thing is a TextureRect with a shader that I wrote, the middle one is just a TextureRect with no material assigned, and there's a Label at the bottom. The only thing unaffected here is the middle one.

Even the origin & viewport guides seems to be affected

@clayjohn
Copy link
Member Author

clayjohn commented Oct 25, 2023

@Carbonateb Glow is a 3D feature. If you have Glow turned on you are running the 3D renderer. My guess from your screenshots is that you are using the "Canvas" background mode in order to render a 2D canvas as the background in 3D so you can use Glow and other 3D effects. When you do that, you accept the tradeoffs that come with using a 3D renderer.

The design of the mobile renderer is explained in detail here: https://docs.godotengine.org/en/latest/contributing/development/core_and_modules/internal_rendering_architecture.html#forward-mobile

I understand that the mobile renderer uses less precision, but I would not expect that to change when the glow starts to appear. E.g. a pure white object should not glow when the cut-off is set to 1. Also, it does not explain why a shader outputting (0.5, 0.5, 0.5) displays as white when HDR is enabled (the left square).

No, the square shouldn't appear white with a color of (0.5, 0.5, 0.5), that may be a consequence of that 0-2 range I mentioned before (the renderer has to do some scaling as technically we can only use numbers between 0-1). I'll see if I can reproduce the issue on my end and investigate a fix if I can.

edit: I tried to repro this issue, but here are my results. If you want to send me the scene you made above, I'd be happy to take a look

image

I'm actually a bit puzzled about what's going on here - I would have expected smaller numbers to have more precision as more bits are "allocated" to the fractional part, with precision decreasing as the colour gets brighter. I'd appreciate a quick rundown if you have time!

The way the "Canvas" background works is you first render 2D to the 2D buffer (using HDR you have 16 bits per channel and you use floats). Then you copy the 2D buffer to the 3D buffer (in the mobile renderer this is 10 bits per channel, but not using floats, it uses fixed point numbers). When in the 3D buffer you can do whatever operations you need to (glow tonemapping etc.) all of which may change your final color, then you render to the screen (or back to the 2D buffer if you have more canvas layers).

Because you go through the 3D stage after doing 2D, you are limited by the precision of the 3D renderer and all the limitations that apply when using the 3D renderer will also apply to you.

The left side has HDR on, the right side doesn't. The top thing is a TextureRect with a shader that I wrote, the middle one is just a TextureRect with no material assigned, and there's a Label at the bottom. The only thing unaffected here is the middle one.

Did you restart the editor after enabling HDR? The one on the left hand side looks like HDR has been enabled, but the editor hasn't restarted yet.

@Carbonateb
Copy link

@clayjohn thanks for taking the time to reply!

I managed to reproduce the issue with the squares in a clean empty project
minimal-repro.zip

I found that the issue arises as soon as you set the WorldEnvironment background mode to Canvas.
Bloom does not seem to have an effect, but I left it on for consistency.

I never suspected that using a WorldEnvironment node would invoke the entire 3D renderer! That explanation does make sense though, I can see how the conversion from 2D -> 3D -> 2D would be prone to have inconsistencies. I wonder if I can achieve those effects (tonemapping, bloom) via custom shaders instead of using WorldEnvironment, which should keep everything in the 2D renderer. This would surely also reduce the exported game size as it wouldn't have to ship the whole 3D renderer.

Did you restart the editor after enabling HDR? The one on the left hand side looks like HDR has been enabled, but the editor hasn't restarted yet.

Yes I did! Before I restarted the editor, it actually becomes darker and more contrast-y. It's as if the editor is over-compensating to return things back to normal 🤔

You can observe this in the minimal repro project I sent - if you delete the WorldEnvironment node, the leftmost square has a colour of #bbbbbb which is roughly 75% grey. Once you disable HDR 2D and restart the editor, the colour darkens to #7F7F7F which is 50% grey - the expected result considering the shader is outputting RGB(0.5, 0.5, 0.5).

@Calinou
Copy link
Member

Calinou commented Oct 27, 2023

This would surely also reduce the exported game size as it wouldn't have to ship the whole 3D renderer.

Export templates will always contain all features, even those you don't use in the project (unless you build custom export templates with certain features disabled). It's not feasible to exclude features dynamically without building custom export templates, as the engine is a single statically linked binary.

@zhagsenkk
Copy link
Contributor

the Forward+ renderer:
1
With the same option of the WorldEnvironment Node, it will be like this on the Mobile renderer, and I cannt realize the same result by adjust the parameters:
2

I never suspected that using a WorldEnvironment node would invoke the entire 3D renderer! That explanation does make sense though, I can see how the conversion from 2D -> 3D -> 2D would be prone to have inconsistencies. I wonder if I can achieve those effects (tonemapping, bloom) via custom shaders instead of using WorldEnvironment, which should keep everything in the 2D renderer. This would surely also reduce the exported game size as it wouldn't have to ship the whole 3D renderer.

I agree this. So is there any way to realize this effect? Boom with beautiful light. Like in the StarSector Game.
3
@Calinou @clayjohn

@Calinou
Copy link
Member

Calinou commented Nov 18, 2023

With the same option of the WorldEnvironment Node, it will be like this on the Mobile renderer, and I cannt realize the same result by adjust the parameters:

The Mobile rendering method has lower dynamic range and precision. A pixel can't get brighter than Color(2.0, 2.0, 2.0) when using Mobile, while it can get much brighter than that in Forward+ (Color(1000.0, 1000.0, 1000.0) is achievable there). This will affect glow appearance a lot, and tweaking parameters will not always get you a similar enough appearance simply because a lot of dynamic range is lost.

Given your use case, I suggest you use glow sprites in your project – that is, a sprite parented to your bullet with a CanvasItemMaterial that uses additive blending. You can assign a GradientTexture2D to this glow sprite to procedurally generate the texture. This is also faster than enabling HDR and glow in 2D, and will give you more artistic control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants