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

Bloom fixes #1441

Merged
merged 4 commits into from
Nov 26, 2024
Merged

Bloom fixes #1441

merged 4 commits into from
Nov 26, 2024

Conversation

VReaperV
Copy link
Contributor

  • Removed mip-levels from contrast and bloom FBOs, other than the base one since that's the only one that we use.
  • NUKED one of the blur shaders and moved everything into just one of them, since the difference was only the components of a vec2 being switched, which is better done with a uniform.
  • Fixes Bloom has no effect #1438.

@slipher
Copy link
Member

slipher commented Nov 22, 2024

It doesn't seem to do the same thing as in 0.54.0. Here's map "zandronum" -175 -2529 -82 -97 3. I tested this scene since bloom has a very large effect on it. For testing the PR I set r_overbrightDefaultClamp 1 for fair comparison.

0.54.0:
unvanquished-zandronum-bloom-cage

This PR:
unvanquished-zandronum-bloom-cage

There is a lot of added brightness around the top left corner of the photograph that shouldn't be there.

While reviewing the first commit, I noticed that in the old code a "contrast" shader is the first one executed, while the commit changes it so that a "blur" shader is first.

@VReaperV
Copy link
Contributor Author

VReaperV commented Nov 22, 2024

It's still executed first, at lines #3075-3090. Both use the blur shader(s) after the contrast one.

There is a lot of added brightness around the top left corner of the photograph that shouldn't be there.

Could it be due to framebuffer format changes?

@slipher
Copy link
Member

slipher commented Nov 22, 2024

It's still executed first, at lines #3075-3090. Both use the blur shader(s) after the contrast one.

I see.

Could it be due to framebuffer format changes?

I tried reverting 483d83d and setting r_highPrecisionRendering 0 and there wasn't any difference.

@VReaperV
Copy link
Contributor Author

Hmm, that's weird. I'll look at what the issue is later. On my end on the same map + viewpos the effect is not as strong as on the 2nd screenshot, but still brighter than the 1st one.

The only difference between the 2 shaders was switching the 2 components of a vec2, which is better done by using a uniform instead of switching shaders. Simplifies the code as well.
@VReaperV
Copy link
Contributor Author

It doesn't seem to do the same thing as in 0.54.0. Here's map "zandronum" -175 -2529 -82 -97 3. I tested this scene since bloom has a very large effect on it. For testing the PR I set r_overbrightDefaultClamp 1 for fair comparison.

0.54.0: unvanquished-zandronum-bloom-cage

This PR: unvanquished-zandronum-bloom-cage

There is a lot of added brightness around the top left corner of the photograph that shouldn't be there.

While reviewing the first commit, I noticed that in the old code a "contrast" shader is the first one executed, while the commit changes it so that a "blur" shader is first.

Fixed, both look the same now:
0.54.1:
unvanquished_2024-11-24_131644_000
This pr:
unvanquished_2024-11-24_131543_000

@slipher
Copy link
Member

slipher commented Nov 26, 2024

LGTM

@VReaperV VReaperV merged commit 9364a8f into DaemonEngine:master Nov 26, 2024
9 checks passed
@VReaperV VReaperV deleted the fix-bloom branch November 26, 2024 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bloom has no effect
2 participants