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

Ensure post processing happens when adjustments are enabled in the Compatibility renderer #93060

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

clayjohn
Copy link
Member

Fixes: #92853

This issue was caused by how we manage our post-processing effects. Right now we only do post processing when the internal buffers are allocated. We check if the internal FBO exists (i.e. fbo != 0) as our condition for doing post effects.

The internal FBO is only ever allocated when glow or scaling are used. If neither of those are used then no post processing happens. Accordingly, the problem here was that adjustments didn't properly request for the internal FBO to get allocated.

Previously it was requested by Glow each frame. If Glow was enabled or disabled the existing buffers would get thrown out and re-allocated. I have adjusted this logic to simply request that the internal buffer be created. We don't need to free the buffers at run time. They should be allocated if needed, and then kept around to avoid performance penalties if effects are adjusted at run time.

I have fixed a few related issues too:

  1. The old function could leak memory. If the MSAA setting changed, the internal buffers would get re-allocated without freeing the old versions. This would leak them. To fix that, it now checks that the buffers have been freed before creating them.
  2. The documentation said that adjustments aren't supported. I removed that note.

@clayjohn clayjohn added this to the 4.3 milestone Jun 11, 2024
@clayjohn clayjohn requested review from a team as code owners June 11, 2024 21:35
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. Code looks good to me.

@akien-mga akien-mga merged commit f3cb890 into godotengine:master Jun 12, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

Adjustments don't do anything unless glow or 3d_scale is used
3 participants