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

Remove high quality glow as it is not any higher quality than regular glow #70009

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

clayjohn
Copy link
Member

While working on #70003 I noticed that the high quality glow mode is actually no longer needed since we switched to this optimized compute shader-based glow. The low quality mode already samples all the pixels from the previous mip level. The high quality mode actually made the effect look worse and take 2X as long since it doubled the pixel samples, but just read the extra pixels from a fixed offset.

I originally tried to "fix" high quality mode, but there is no way to make it not broken its just needlessly taking extra samples.

Marked as breaking compatibility because users may be using the functions to set the quality manually. I would prefer to remove everything as I have, but if we want to avoid breaking compatibility, I can mark the setting and functions as deprecated and just make them do nothing.

"Low quality" mode
Screenshot from 2022-12-12 22-48-58

"High Quality" enabled: notice how the glow looks lopsided
Screenshot from 2022-12-12 22-49-07

These next images are pretty much worse case scenario for glow as they contain one pixel aliased lines. Capturing fine features like this was the reason we added the high quality mode in the first place.

"Low quality" mode
Screenshot from 2022-12-12 22-51-27

"High Quality" enabled: There is a very slight shift of strength to the upper left, but it is almost impossible to see without an image diff tool
Screenshot from 2022-12-12 22-51-33

@akien-mga akien-mga merged commit 465d4c1 into godotengine:master Dec 13, 2022
@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.

2 participants