Improve bloom and add option to use faster but lower quality bloom #21340
+137
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
Contributor
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
Contributor
|
What's perf like with the low quality version? Afaik, the main perf issues with bloom is CPU usage, not GPU usage. Having so many separate render passes (1 per upsample/downsample step) is quite expensive. |
alice-i-cecile
added
C-Feature
alice-i-cecile
added
S-Needs-Benchmarking
Contributor
|
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
Member
|
I quite like the idea of this, but I want to see benchmarks on low-end machines :) |
alice-i-cecile
removed
the
S-Needs-Review
Contributor
Author
|
Tested the bloom 3d example on Mali G76 android with log diagnostics (debug build, opt-level 3): CPU timeGPU time (high quality bloom)GPU time (low quality bloom)TLDR: Bloom CPU takes ~2.3 ms (can be reduced if building in release mode). |
Contributor
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
alice-i-cecile
added
S-Needs-Review
Contributor
Author
|
I also add a |
Contributor
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
beicause
force-pushed
the
improve-bloom
branch
from
bbd330a to
ea0c52c
Compare
Contributor
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
1 similar comment
Contributor
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
IceSentry
added
the
M-Deliberate-Rendering-Change
IceSentry
reviewed
Oct 6, 2025
Contributor
|
I added the Deliberate-Rendering-Change tag but I'm not sure all of it is correct. There's new text in the example which should cause it to fail but there's also a ton of other changes in the images which is surprising since I would have expected the default values to still look the same. |
tychedelia
suggested changes
Oct 6, 2025
Member
tychedelia
left a comment
tychedelia
left a comment
There was a problem hiding this comment.
Looks good, happy to enable more perf configurability here.
|
|
||
| // Whether to use a high quality bloom implementation (default: true). | ||
| // If false, bloom will use an implementation that significantly reduces the number of texture samples and improves performance, but at the cost of lower quality. | ||
| pub high_quality: bool, |
Member
There was a problem hiding this comment.
Can we make this an enum to allow adding more possible quality settings / configuration in the future?
Contributor
Author
There was a problem hiding this comment.
I'm not sure if we should add an enum. The two bloom implementations seem sufficient, and there are other quality configuration. The performance difference of using moderate samples may not be significant.
Contributor
Author
I |
beicause
force-pushed
the
improve-bloom
branch
from
ea0c52c to
ab01abd
Compare
beicause
force-pushed
the
improve-bloom
branch
from
ab01abd to
4e8ecca
Compare
beicause
force-pushed
the
improve-bloom
branch
from
4e8ecca to
a3f0c5c
Compare
Contributor
Author
|
ClampToBorder is a native only feature so I will not use it. The default values should look the same now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Related to #13109. Bloom is slow, especially on mobile.
Solution
high_qualityoption to Bloom, if false, use blur kernels that only read 4 samples for downsampling and upsampling (from https://www.shadertoy.com/view/mdsyDf), instead of 13 samples and 9 samples.max_mip_countoption to control the used mipmap count.ClampToBorderfor downsampling andClampToEdgefor upsampling. Only usingClampToEdgecauses the bloom too bright and may flicker in the edges in low quality bloom.Testing
Added the option to
bloom_2dandbloom_3dexamples to test it.TODO: benchmark the performance.
Showcase
Click to view showcase