Skip to content

Enhanced Bloom #5431

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Enhanced Bloom #5431

wants to merge 10 commits into from

Conversation

qazwsxal
Copy link
Contributor

Hi there, I've gone and implemented the style of bloom in use in UE4/Unity first introduced by Sledgehammer games in Call of Duty: Advanced Warfare.

There's an extra tuneable parameter here, blur width, i.e. radius, which has been taken from https://www.froyok.fr/blog/2021-12-ue4-custom-bloom/, although I am open to change the implementation of this as it doesn't feel particularly intuitive at the moment, past about 0.5 it just seems to fade away into a flat whole-image update.

The key advantage to this approach is a big reduction in the amount of bloom shimmering/crawling. This is achieved through two methods:
Firstly, there is no longer any thresholding applied to the image before blooming. This is in line with modern practices for HDR renderers, and prevents pixels close to the bloom threshold from jittering between bloomed and not bloomed across frames.
Secondly, the standard mipmap box filter has been replaced with a custom downsampling filter that sledgehammer games designed to help alleviate these issues.

Performance wise, the sledgehammer implementation was targeted at the GTX 760 (recommended spec.). This implementation runs in 0.59ms on a RTX 3070. For comparison, the old bloom implementation runs in 0.37ms.

Draft Status

The PR is in draft status for 3 main reasons, the table entries aren't done yet, there needs to be a discussion on sensible defaults, and the width parameter might need some work.

  • Configuration with tables.
  • Choice of sensible defaults.
  • More intuitive width parameter?

That said, I'd like to get some feedback on how it is as-is, if it's worth changing the bloom_intensity command line parameter name completely rather than just repurposing it, where configuration should go, should it even be user controllable? etc.

@The-E
Copy link
Member

The-E commented May 29, 2023

Regarding defaults:

screen0005

I feel like this is pretty damn ugly. The background starfield suffers the most: all stars get smeared out, and while this would be realistic for a starfield filtered through an atmosphere, it kinda looks bad here.

Reducing the bloom level to 4 fixes this:
screen0006
.... buuuut it also makes the bloom effect pretty unnoticeable.

I think one of the great drawbacks of this effect is that very lack of a brightness threshold. This leads to a high bloom level leading to the bloom shader acting as a full-screen blur:
screen0007

With all of that in mind: I think the defaults for both bloom level and width should be very low. A bloom level of 10 and a bloom width of 0.1 produces pretty good results, IMHO:
screen0008

@qazwsxal
Copy link
Contributor Author

That's exactly the sort of feedback I was looking for, thanks E. And yes, I agree the current defaults aren't suitable. On the implementation side (and why I'm considering a re-name) bloom level now refers to the percentage blend between the original image and bloomed version. At 100% it's entirely the bloom buffer, hence the full screen blur.

@wookieejedi wookieejedi added enhancement A new feature or upgrade of an existing feature to add additional functionality. graphics A feature or issue related to graphics (2d and 3d) labels May 30, 2023
@qazwsxal
Copy link
Contributor Author

qazwsxal commented Jun 4, 2023

Made some changes to how the bloom contribution for each level is calculated, this should help the bloom width slider feel more intuitive.

@The-E
Copy link
Member

The-E commented Jun 4, 2023

Here's a direct comparison between bloom off and bloom 10/bloom width 0.1, which is subtle enough to be noticeable but not distracting:
screen0011
screen0012

I quite like this.

Also, as a random aside: I think this actually has some potential use as a blur-out kind of effect. If we could ramp the two parameters via sexp over time, that could be a neat thing for mission designers to use.
screen0010

@qazwsxal qazwsxal marked this pull request as ready for review August 15, 2024 20:54
@MoerasGrizzly
Copy link

I have some pedantic questions:
How does this work on different tonemappers? The old bloom effect has some weird behaviours on luminance based tone mapping (eg PPC without RGB) and i wonder if this deals with that.

@qazwsxal
Copy link
Contributor Author

So the main difference here is that the old bloom approach has a threshold to it, only pixels above a certain brightness got bloomed. Here, every pixel gets bloomed no matter the brightness (which more closely follows how bloom occurs IRL due to flaws in eye/camera optics). Do you have any examples of how bloom looks weird with luminance tonemappers?

@MoerasGrizzly
Copy link

So the main difference here is that the old bloom approach has a threshold to it, only pixels above a certain brightness got bloomed. Here, every pixel gets bloomed no matter the brightness (which more closely follows how bloom occurs IRL due to flaws in eye/camera optics). Do you have any examples of how bloom looks weird with luminance tonemappers?

I send you examples of this in discord (just mentioning that here for the sake of record keeping) - but based on your description alone I think this might actually fix those issues - the main difference between PPC and PPC RGB is how it handles the colours of bright objects (where PPC RGB will bring everything to white whereas PPC tries its damnest to retain the same colours).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or upgrade of an existing feature to add additional functionality. graphics A feature or issue related to graphics (2d and 3d)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants