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

Fix bug in normal map decompression #44293

Merged
merged 1 commit into from
Dec 11, 2020

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Dec 11, 2020

Not clamping the range here leads to dithering artifacts.

Fixes #43309

This seemed obviously wrong in the canvas shader which was leading to artifacts, see the full discussion in #44285. Although the latter issue shows a problem in the sprite 3d, my very quick look suggested the scene shaders were not suffering from the same flaw.

There may be another fix that can be applied for the scene shaders. I'm just doing this as a separate PR as I have to rush off.

Notes

  • The current version in the canvas shader is incorrect and this fixes that bug
  • There is still some dither, I'm not clear as to whether this is a cost of compression, or there is something else we could do to improve this
  • If the dither is a cost of compression, we should investigate having uncompressed normals option. Especially as this may be cheaper on mobile anyway.

Example before / after

Screenshot from 2020-12-11 09-34-56
Screenshot from 2020-12-11 09-51-36

@lawnjelly lawnjelly requested a review from reduz as a code owner December 11, 2020 10:27
Not clamping the range here leads to dithering artifacts.
@lawnjelly lawnjelly requested a review from clayjohn December 11, 2020 10:35
@Chaosus Chaosus added this to the 3.2 milestone Dec 11, 2020
@akien-mga akien-mga merged commit 00d9ffe into godotengine:3.2 Dec 11, 2020
@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.

3 participants