Blend glow before tonemapping and change default to screen.#110671
Conversation
f3b8f80 to
7ff6a57
Compare
6d1e33b to
8ad4e6c
Compare
Environment glow more consistent between rendering configurationsEnvironment glow more consistent between rendering configurations
Environment glow more consistent between rendering configurationsEnvironment glow blending more consistent between rendering configurations
8ad4e6c to
8dc30a6
Compare
Environment glow blending more consistent between rendering configurationsEnvironment glow blending more consistent between rendering configurations and add legacy mode.
|
This PR is now ready for review. |
b0d5a5f to
f900a14
Compare
|
I noticed a mistake in I've pushed a fix to correct this. |
ea557ac to
319bf81
Compare
|
In yesterday's rendering meeting we discussed my proposed "Legacy mode" project setting for
While the last point is not ideal for some projects, our hope is that the new environment effects behaviour will be as capable as the existing behaviour. This means that there will be a pain point from upgrading because parameters will need to be readjusted to match the new behaviour. But our expectation is visuals that are as good, if not better, should be achievable with the new behaviour. I will update the PR and proposal description to not include the original "Legacy mode" proposal. I have pushed a new version of the PR and completed my work removing the "Legacy mode" from the code, so the PR is now again ready for review and testing. |
|
Multiple reviewers is standard practice—something that implies compatibility breakage even moreso. Will wait for more rendering opinions first |
df547d3 to
87b4f24
Compare
|
I've made a correction to the soft light blend mode documentation: old (incorrectly referred to scene values):
new (correctly refers to post-tonemapped image values):
|
87b4f24 to
37ae144
Compare
Environment glow blending more consistent between rendering configurations and add legacy mode.Environment glow blending more consistent between rendering configurations.
Environment glow blending more consistent between rendering configurations.37ae144 to
2aa6937
Compare
|
In response to the discussion in last night's rendering meeting, I've updated this PR's text and commit text to better describe what this PR actually changes. |
…mode to screen. Additionally, change the minimum `tonemap_white` parameter to `1.0`; users can increase `tonemap_exposure` for a similar effect to decreasing `tonemap_white` below `1.0`. Co-authored-by: Hei <40064911+Lielay9@users.noreply.github.com> Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
2aa6937 to
cafc012
Compare
clayjohn
left a comment
There was a problem hiding this comment.
Looks good! Let's go ahead with this now since so many PRs are waiting for it.
As a note for other maintainers. This is a breaking change. It changes the default glow settings and changes the visual appearance of glow with all settings. This is necessary for 2 reasons:
- It simply looks better. Calculating glow before tonemapping just looks better when working with HDR content. Especially when using the new AgX tonemapper
- It makes glow behave consistently when using an HDR screen vs. an SDR screen. In 4.5, glow will look very different if you switch between the two. In the long run, making this change now, before adding HDR screen support will be worth it.
This has been discussed extensively with rendering contributors and we don't make this change lightly. Accordingly, we should call this out specifically in the dev release blog post, the beta blog post, and the release blog post. I will also call it out in the rendering update this fall.
|
Thank you! |
|
Does this conflict with #52227 by any chance? |
|
I don't think so because the PR is from 2021 & still hasn't been merged
|
|
Hello @allenwp I retrieved two images from your PR to illustrate this new feature on the release page. Unfortunately, the images on GitHub are not in the correct format and aspect ratio (HD 16:9). Do you happen to have any higher-quality images illustrating your PR? Preferably, by presenting a clear side-by-side comparison, as you did with the street lamp.
For reference: |
|
Sure, I can try and whip something up in the next week or two. I'm not the best for art/related stuff... so if you decide you'd like to make your own comparison, please message me on RocketChat and I can give you the exact configurations to use for screenshots that will highlight the changes! |
|
Hello! I am trying 4.6.rc1 and the glow intensity has totally increased compared to 4.5: Do you know what is the factor by which I need to decrease the glow to get back to the original intensity? or should I just eyeball it? EDIT: if I understand well, I just need to change the glow blending to softlight if I want the same effect as before? |
|
If you have HDR 2D enabled, then yes, changing to soft light and changing the levels and intensity back to their 4.5.1 values will give you the same effect. Changes to default values:
If HDR 2D is disabled (the default for new projects), then there is no way to get the exact soft light behaviour from 4.5.1 and you will need to eyeball it. I recommend using the new default screen blend mode instead of soft light because soft light removes the glow effect when blending against dark backgrounds, which is typically not a desirable behaviour for a glow effect, even though it was previously the default. |



Upgrading to Godot 4.6
This PR changed default values for glow effects. To restore similar behaviour of Godot 4.5 and earlier, please refer to the migration guide for the old glow default values.
white > 1.0#110670This PR supersedes:
This PR is related to:
Environmentadjustments (favor performance). #110630Rationale and screenshots
Rationale and additional screenshots for this PR can be found in godotengine/godot-proposals#13181.
Summary
Mobile and Forward+
Mobile and Forward+ now apply glow before the tonemap function, just like the Compatibility renderer, for all blend modes except for soft light. The
whiteparameter is now used to normalize screen blend mode.Notice how blending before tonemapping gives a softer, more realistic blending of the glow effect without hue shifts and harder edges introduced by blending after tonemapping:
Compatibility
As described in the proposal, this PR applies glow to nonlinear sRGB encoded values in the Compatibility renderer.
Although blending is performed consistently between HDR 2D enabled and disabled, the result is substantially different. I believe this is related to other glow functions and is independent of the purpose of this PR (glow blending).
As a side effect of the new behaviour, #110670 is fixed.
Defaults
Previously the screen blend mode was the default for Compatibility and the soft light blend mode was used for Mobile and Forward+. This PR changes the default to be screen for all renderers.
I have also included defaults that were suggested in #52227, so this PR fully supersedes this original PR. (co-author added)
Future work
With other upcoming glow changes it might make sense to revisit these to ensure default level values are chosen to match Compatibility behaviour if Compatibility behaviour can be made to look acceptable. Or, if Compatibility can not be made to look as good as the other renderers, new defaults may be needed to better match new glow behaviour in Mobile and Forward+.
Performance
Blending glow before tonemapping improves performance by removing the need for a second tonemapping operation on glow values. Because glow is blended before tonemapping, tonemapping is applied only once to the final scene that includes the glow effect.
I've added mathematical simplifications and optimizations to the screen and soft light blend functions. Please let me know if you believe these should be in a separate PR instead. The soft light optimization comes from #110239. (co-author added)
To help with the performance impact of branching, specialization constants are likely needed in a followup PR for Mobile and Forward+ renderers.
Notes for reviewers
whiteparameter has been restricted to1.0and higher! Previously, you could setwhiteto be less than1.0, but that does not work with this new glow screen blend mode.whitethat is less than1.0, they should increase tonemap exposure instead. Decreasing tonemapwhitebelow1.0simply doesn't make sense.Comparisons
Screen blend mode
Soft light blend mode