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

FXAA exhibits artifacts not present in NVIDIA's reference implementation #64985

Closed
mrjustaguy opened this issue Aug 28, 2022 · 9 comments · Fixed by #66466
Closed

FXAA exhibits artifacts not present in NVIDIA's reference implementation #64985

mrjustaguy opened this issue Aug 28, 2022 · 9 comments · Fixed by #66466

Comments

@mrjustaguy
Copy link
Contributor

mrjustaguy commented Aug 28, 2022

Godot version

4.0 Alpha 14

System information

Windows 11, Vulkan, Nvidia GTX 1050 Ti, 512.15

Issue description

Godot's FXAA implementation creates unusual wave-like patterns on the image.
0 25 FXAA
FXAA Test

Comparison (Native res rendering (1.0 render scale), taken in the current dx12 branch for Nvidia Driver FXAA to work):

No FXAA
No FXAA

Godot FXAA
Godot FXAA

Nvidia Driver FXAA
Nvidia FXAA

Steps to reproduce

Add some geometry to look at and enable FXAA.
Lowering the Resolution Scale makes the artifacts easier to see, but they also happen on smaller (screen space) more complex objects at full res

Minimal reproduction project

No response

@Calinou
Copy link
Member

Calinou commented Aug 28, 2022

Godot uses a different implementation of FXAA compared to the reference one for simplicity reasons. The official NVIDIA implementation is open source, but it's much more complex as it exposes several quality knobs (which are usually not relevant anymore in 2022).

This leads to a slightly different appearance compared to the official implementation. If you have time, look into adapting the official implementation to work with GLSL and always use the Medium quality preset (which is the default). High and Ultra quality presets are not only more demanding, but also more blurry (unlike SMAA's Ultra quality preset which just provides better antialiasing).

Note that this issue also occurs in 3.x, as it uses the same simplified FXAA implementation.

Some comparison screenshots in Tesseract SVN at 2560×1440 between FXAA/SMAA levels (with and without temporal AA). This game follows the reference FXAA implementation more closely:

@Calinou Calinou changed the title FXAA Artifacts FXAA exhibits artifacts not present in NVIDIA's reference implementation Aug 28, 2022
@clayjohn
Copy link
Member

The screenshots make it look like the sampling coordinates are slightly off leading to some wrapping artifacts. I note especially how in corners like in the "r" and "W" you can see floating pixels.

@mrjustaguy
Copy link
Contributor Author

#37819 screen shot from implementation also has this issue it seems.. at the top, there's a floating red pixel next to the red box

@mrjustaguy
Copy link
Contributor Author

I've been doing more testing, and 3.5 also suffers from this issue.

@mrjustaguy
Copy link
Contributor Author

mrjustaguy commented Sep 26, 2022

I've made a project where I've made a Post Processing FXAA Step, and ported the Godot FXAA to the GDShaders to play around with, and I think I've found what the issue is with Godot's FXAA Implementation.

The Direction Vector is messed up, which would explain why there seem to be sampling issues

dir.x = -((lumaNW + lumaNE) - (lumaSW + lumaSE)); should only determine the direction based on brightness along the X axis, but if you simplify the terms to dir.x = -(lumaN-lumas) you see that if north is brighter, you get a negative value, if the south is brighter, you get a positive value. North and South are on the Y axis, not X axis.
The term should be dir.x = lumaE-lumaW (term is simplified) so that when the East is brighter, you get a positive value (pointing right) and when the West is brighter you get a negative value (pointing left), or Inverted, I'm still unclear if dir should point to brighter or darker side.
The same holds true for the Y axis
dir.y = ((lumaNW + lumaSW) - (lumaNE + lumaSE)); is simplified to dir.y = lumaW-lumaE which is wrong for the y axis, and should be dir.y = lumaS-lumaN

From testing it doesn't seem like it matters if the direction is pointing from brighter to darker or the other way around, so here's the corrected directions
dir.x = (lumaNE + lumaSE) - (lumaNW + lumaSW); (x axis where west is negative, east is positive)
dir.y = (lumaSW + lumaSE) - (lumaNW + lumaNE); (y axis where north is negative, south is positive)

There are still some issues with FXAA when compared to Nvidia's reference, however they seem to be there due to missing steps in Godot's FXAA implementation

@clayjohn
Copy link
Member

@mrjustaguy Great work! Will you be making a PR?

@mrjustaguy
Copy link
Contributor Author

I don't really use Git so it'd probably be a better idea for someone else to make a PR.

@clayjohn
Copy link
Member

I wasn't able to get improved visuals with the code changes above. So I looked into the original source to see if there is an explanation (there isnt) but I noticed we sample the wrong pixel positions.

(Zoom in to see differences)
Test scene:
Screenshot from 2022-09-26 10-57-25

Test scene with FXAA in master:
Screenshot from 2022-09-26 10-57-50

Test scene with fix proposed here:
Screenshot from 2022-09-26 11-03-14

Test scene without fix from here, but with improved sample positions:

Screenshot from 2022-09-26 11-30-56

@mrjustaguy
Copy link
Contributor Author

Yeah my bad, the ghosting introduced looked like AA in the case I was testing on and the solution seemed to make sense on the surface..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants