Implement FXAA 3.11 to replace the current broken implementation

[mrjustaguy]

Resolves godotengine/godot-proposals#9300
Fixes #83089
Improves Temporal stability over the old implementation (No Issue on github opened regarding the extremely low temporal stability of FXAA in motion)

Implements FXAA 3.11 to replace the current Godot FXAA implementation which is heavily messed up.

Multiview Hasn't been tested (nor can I test it) and Need to find the exact license text for FXAA 3.11, and where to place it in Godot, so help with these aspects is very welcome.

No AA:

New Implementation:

Old Implementation:

Note: don't mind the performance metrics, for some reason they fluctuate heavily between 4 and 5ms all the time in the scene regardless of version and AA used/not, and these are just the numbers that happened to be at screenshot time, Actual benchmarks are required to determine actual performance differences.

[AThousandShips]

Can someone confirm the license and origins of this code, see the discussion in:

• Replace custom implementation of FXAA with the reference implementation godot-proposals#9300

[mrjustaguy]

I just found the License text in the gist https://gist.github.com/kosua20/0c506b81b3812ac900048059d2383126 which links to the implementation at https://github.com/kosua20/Rendu/blob/master/resources/common/shaders/screens/fxaa.frag that I've used.. Don't know How I've missed it..

[AThousandShips]

It says "cleaner version" but does that mean it's just a copy? The original code is proprietary/closed yes?

COPYRIGHT (C) 2010, 2011 NVIDIA CORPORATION. ALL RIGHTS RESERVED.

Nothing about any ability to share it, are they the author (even if they are, that doesn't mean they can share it if it's for a company)? Otherwise this looks very fishy and not valid

[mrjustaguy]

FXAA 3 is released under a public domain license. A later version, FXAA 3.11, is released under a 3-clause BSD license.(https://en.wikipedia.org/wiki/Fast_approximate_anti-aliasing)

which links to
https://stackoverflow.com/questions/12170575/using-nvidia-fxaa-in-my-code-whats-the-licensing-model/12170857#12170857

[AThousandShips]

The link to the license in that page is dead, as is the link to FXAA 3.11, so that doesn't seem like a confirmation, so all we have right now is explicit stating "it's all rights reserved"

In either case this requires a license inclusion, BSD 3-clause requires that

For my part I'd prefer a properly sourced reference to copy, with a clear license statement, from some original trusted source

[mrjustaguy]

Well all the official Nvidia links regarding FXAA seem to be dead so...

[AThousandShips]

So then there's no original reference to it being open source, so that's a big issue

Even if we take that statement at face value the source you've copied violates that, it doesn't attach a BSD license to the source, the FXAA 3.11 isn't public domain

As said by clayjohn this was already known previously and we've been wanting to implement it but licensing stopped us, so it might be we were wrong before but it doesn't seem that way and that we're still unable to based on the same issues as before

[AThousandShips]

Even that claim about it being open source licensed is far not up to the standards of Wikipedia, a random blog or forum post isn't a valid reference, and the fact that they don't quote a proper source is a huge red flag to me, if it was possible to find a NVIDIA source for it being licensed like that it would be on the Wiki page, trust to Wikipedians to fix that quick if they can

With something like a big company like NVIDIA I wouldn't believe that they've released anything open source without seeing it from them directly

[mrjustaguy]

Well I guess the only way to resolve this is to contact Nvidia, and see if the Wiki information is right or not, and pray they answer.

[AThousandShips]

In either case this source is not compatible with that unless it's under a public domain license, so in that case you'd have to find NVIDIA source code, this code is copied from a page which claims copyright for the code without any attribution to NVIDIA or any original author

I don't think this is safe or realistic as a way to proceed, unless you can find a primary code copy to work with

Edit: Given the phrasing in their comment this might be original code, if that is the case then we'd just need to cite them properly (this code currently violates the license of that code, so you need to fix that), assuming it's a valid implementation

Performs FXAA post-process anti-aliasing as described in the Nvidia FXAA white paper and the associated shader code.

However I don't trust that, they say they have a:

I have a cleaner, probably more readable version here

Which to me reads as a cleaned up version of that source, not their own creation, which wouldn't be valid

Note: The only evidence we have currently for NVIDIA releasing this under any license is:

• A forum post of a
• Claimed screenshot of a
• Person claiming (unverifiable) to be the author claiming that
• It is open source

So it's like very not any evidence lol

[mrjustaguy]

Well, Bevy is using the Gist FXAA implementation - https://github.com/bevyengine/bevy/blob/main/crates/bevy_core_pipeline/src/fxaa/fxaa.wgsl

[AThousandShips]

Well their header claims that it's a cleaned version of the NVIDIA code, which is very clearly not open source unless there's some part in that code I missed, as see above it says "ALL RIGHTS RESERVED", also this one also doesn't give proper attribution and license details 🙃 so it still all hinges on the original NVIDIA code being open source, and if it is that would just mean we need to find how to add proper attribution

The original gist looks like it's a decompilation of the code, given it contains compiled assembly, which to me is a red flag, if this was legitimately released by NVIDIA wouldn't there be actual licensed code by them? Or is the gist just very strangely formatted and laid out?

[mrjustaguy]

This is Blender's FXAA - https://github.com/blender/blender/blob/main/source/blender/draw/intern/shaders/common_fxaa_lib.glsl

[AThousandShips]

That does look more legitimate, and Blender is big enough to get in trouble, but is that actually FXAA 3.11?

However unless that's hugely rewritten in the version you have here that looks nothing like this code

Also it says "BSD-3 AND GPL" which is confusing, and if it is GPL we can't use it

[mrjustaguy]

The original Gist seems like the same thing as the Blender FXAA, and the code I used is a cleaner implementation of the Gist made by the author of the gist...

GPL prob for the blender tweaks stated, IIRC all blender code is GPL

[mrjustaguy]

The cleaned code that I used when porting code to Godot and the source code (both gist and blender versions) seem to be doing the same thing, it's just that the source didn't use things such as iterators which blew the number of lines of code like crazy..

Given Wikipedia states BSD3, and Blender actually confirms BSD3, and the gist is also used by bevy (the same implementation as I've ported) I'm fairly confident if there were Issues, Nvidia would have taken action long ago, especially given the Size of Blender and the fact they clearly know about Blender for quite a long time, and I think for a while they were even Sponsors, though could be wrong on that one, as I don't see them on the Sponsor list for blender..

[AThousandShips]

Given Wikipedia states BSD3

It doesn't cite a remotely reliable source though, so let's not use it as an argument, this sounds like a game of telephone and there's no actual original source of this claim

The Wikipedia source is a screenshot, of a person claiming to be the author of the code, claiming it's open source, that's all we have, if we look at it critically

That is not a valid source (that's not a screenshot by me, that's literally the source Wikipedia uses), it also doesn't say anything about the 3.11 being BSD

The NVIDIAGameWorks gh doesn't even have the samples in question any longer, so the supposed license doesn't exist

Regardless of the legality I feel it's irresponsible to add code for which the original cannot be verified and the claims about it being open source are highly questionable

[fire]

https://github.com/AmplifyCreations/FXAA This website has a copyright notice for FXAA.

[AThousandShips]

Yes the second of those links were discussed above and the issues with that all the references there are dead links

Also that's FXAA 3.9, not 3.11

The first link doesn't have any licensing and all it states is all rights reserved and no basis for sharing it

[fire]

A clear response would be for us to remove FXAA?

I could try asking Nvidia what license it is and they can make a redeclaration?

And this is a gist... https://gist.github.com/kosua20/0c506b81b3812ac900048059d2383126 which isn't that great.

[AThousandShips]

I don't know about the existing implementation, where it's from

Note that the theory behind FXAA is available (and you can't copyright it), so if the FXAA code is original it's fine, it looks relatively original

CC @reduz you added this, where did you get the implementation?

[bikemurt]

At the risk of sounding a little ignorant here... rather than do a simpler refactor, can't the Godot foundation produce their own white paper based on understanding FXAA principles and review of other open source work that sources licenses (such as the NVidia article as well as Blender's FXAA implementation)?

Obviously this requires effort, and if we could simply refactor the library with appropriate licenses, it's a non issue.

I work in machine vision, and it seems to me that the algorithm boils down to: acquiring brightness data from the screen texture, using a high pass filter (subtractively) or presumably use a low pass filter to improve edge detection, apply a sobel filter (small kernel) or heuristic edge detection algorithm, acquire edge direction, then blend the pixel in the direction normal to the edge. Then a ton of tuning constants to dial it in.

And from another viewport - Blender seems comfortable using FXAA3, I feel like Godot should as well.

But I am not a legal expert, so I understand that I could be very wrong here. Just my two cents...

[AThousandShips]

As mentioned above writing our own implementation based on the white paper is fully legal, but that's a lot of work, and having an existing, well tested, implementation is worth a lot, the one linked above with good licensing (it seems, would be good to get it verified) would be a good start, but the current code isn't valid based on the licensing issues (and the current code here doesn't follow the license either)

[bikemurt]

As mentioned above writing our own implementation based on the white paper is fully legal, but that's a lot of work, and having an existing, well tested, implementation is worth a lot, the one linked above with good licensing (it seems, would be good to get it verified) would be a good start, but the current code isn't valid based on the licensing issues (and the current code here doesn't follow the license either)

Practically every game these days (besides pixelated style graphics) ships with FXAA. It will be a big feature for Godot to have nailed in implementation.

So the community, whether by committee or not, probably needs to figure out what the direction forward is. Which sounds like either, engage a legal team, or write a Godot FXAA implementation.

I know I'm asking a lot and so I apologize. I know the engine has 2300 people working so many different things. It's just a feature I would look forward to.

[AThousandShips]

This has already been discussed here and all these options raised, and we're waiting for more details, so this doesn't add anything, please read the full conversation here and on the proposal instead of repeating things :) repeating things isn't going to speed things up and with GDC at the moment a lot of people are away and unable to give their feedback here especially on legal things

[mrjustaguy]

FXAA or SMAA is basically a must for anyone that wants AA and doesn't want TAA for whatever reason (a good reason being that Godot's TAA is Broken, and FSR2 has Super ghosting issues due to Pixel Locking, which is unclear if it's FSR2 issue or Godot issue or a mix, and the reason Why I went into this tbh)

What shocked me actually when looking at the current implementation and thinking about it, and comparing it to reference, I'm not surprised that ours is such garbage, as it's only taking into account a 3x3 pixel grid, whereas 3.11 reference walks on for multiple pixels in a given direction which makes sense, and I see my attempts at making my own AA method were on the right track, though the biggest error I made was blending all those pixels I walk over instead of using the walking over to determine where I sample the end result pixel, though It would have taken ages to figure out how to calculate that position even if I did have that information.

[AThousandShips]

I'm not surprised that ours is such garbage

Please don't talk about the work of others in that way, it's not friendly or constructive :) Calling code "broken" or "not enough" or even "bad" is one thing, those are statements of quality, not value, but "garbage" is a value judgement that is neither helpful nor kind.

[mrjustaguy]

I think I've triggered you a little with the wording and so you missed what was actually being said there.

comparing it to reference, I'm not surprised that ours is such garbage, as it's only taking into account a 3x3 pixel grid, whereas 3.11 reference walks on for multiple pixels in a given direction which makes sense

This is in no way not constructive or hostile when read in it's entirety, and as far as friendliness/kindness goes it's neutral in tone, and there's nothing wrong with that.

To show an example of what's wrong with not taking the whole thing into consideration properly:

Please don't talk about the work of others

The tone of just this statement alone can correctly be interpreted in tones ranging from friendly (your original intention) to neutral (oh I don't care about talking about others' work) to hostile (oh shut up, you're too stupid to understand others' work and thus your input is useless)

Now when you add the extra context in the continuation It's clear that the intent wasn't hostile like it could have been interpreted without it. Hope I've cleared that up now.

On another note, this comment and yours should be marked as off topic, as they're not related to this PR as a whole.

[clayjohn]

I will get in touch with NVidia. It seems that the linked source (https://github.com/hghdev/NVIDIAGameWorks-GraphicsSamples/blob/master/samples/es3-kepler/FXAA/FXAA3_11.h) is a mirror of something that NVidia originally posted

NVidia posted the entire graphics samples to github, but then removed them at some point https://developer.nvidia.com/gameworks-vulkan-and-opengl-samples-30-released (link at end of article). I trust that the mirrors we have seen accurately reproduce the licence (so in theory it should be fine if we do as well), but its best for us to double check with NVidia before doing anything.

More tidbits:

• Unity uses the mirror hosted by hghdev (https://docs.unity.cn/Packages/[email protected]/license/Third%20Party%20Notices.html)
• Wayback machine copy of github page (https://web.archive.org/web/20170723104021/https://github.com/NVIDIAGameWorks/GraphicsSamples)
• The samples are still hosted on Archive.org (https://archive.org/details/nvidiagame-works-graphics-samples-master) (they were a zip file before being open sourced on github) (version 3.00 of FXAA only)
• Falcor used to contain a slang version of FXAA (https://github.com/NVIDIAGameWorks/Falcor/blob/f2b53b1bb9f8433f3c9e2570d2dc90dcd2440415/Framework/Source/Data/Effects/FXAA.slang)
• Miniengine has a compute-optimized version of FXAA (https://github.com/microsoft/DirectX-Graphics-Samples/blob/master/MiniEngine/Core/Shaders/FXAAPass1CS.hlsli) (it cites the original source, but also makes nice improvements) Maybe we should consider using this for desktop and something simpler for mobile
• Filament uses a modified version (via G3D) (https://github.com/google/filament/blob/main/filament/src/materials/antiAliasing/fxaa.fs)

I cannot find a copy of the Gameworks samples SDK that contains FXAA 3.11 anywhere. I have a feeling it might have only been released on the official Github which was then deleted. Seeing as there are a number of sources using the same code, I think it is safe to use

edit: Turns out the version 3.00 of the gameworks SDK (still up on archive.org) contains the source for FXAA 3.11. Which is the same as the hghdev mirror (https://github.com/hghdev/NVIDIAGameWorks-GraphicsSamples/blob/master/samples/es3-kepler/FXAA/FXAA3_11.h)

[Calinou]

Tested locally (rebased on top of master 29b3d9e), it works as expected. This noticeably improves smoothness, especially on near-parallel lines. Image sharpness isn't noticeably affected by this PR.

Testing project: test_pr_89582.zip
This uses the approach described in #89582 (comment) for testing.

No AA	FXAA Before	FXAA After (this PR)

4× zoom:

No AA	FXAA Before	FXAA After (this PR)

Comparisons between the new FXAA and Tesseract's FXAA on Medium and Ultra quality:

FXAA PR	Tesseract FXAA Medium	Tesseract FXAA Ultra

4× zoom:

FXAA PR	Tesseract FXAA Medium	Tesseract FXAA Ultra

[AThousandShips]

Still needs proper attribution, this is not in line with the linked repo's license (i.e. regardless of the source being valid this copy of it isn't)

I'll defer to the legal people on he legality of this, and wash my hands of it, I'm not convinced but I defer to them

Though I'd say to check with Nvidia properly before merging, though I'd still prefer basing our version on one with proper attributation to Nvidia, like the Blender one, as this one isn't compliant with the license seen elsehwere as it doesn't reference the Nvidia license anywhere

[AThousandShips]

To be clear, I won't stand in the way of this being merged with correct attibution, but I'd very much prefer it if we base it on a version that actually follows Nvidia's licensing in the mirrors, i.e. mentions them as originators at all

[mrjustaguy]

Well for the attribution, I just need to know where to place what

[AThousandShips]

You should copy this license text and add it next to the link you have, assuming we're going with this and not the properly sourced one linked above
https://github.com/kosua20/Rendu/blob/master/LICENSE

[mrjustaguy]

isn't there like a place where we put all of the licenses and their texts?

[AThousandShips]

Not for included code, those cases are for licenses in 3rd party code, the license has to be with the code

Accidental approval

[bikemurt]

Any issue with listing the license at the top of this file?
https://github.com/hghdev/NVIDIAGameWorks-GraphicsSamples/blob/master/samples/es3-kepler/FXAA/FXAA3_11.h

Regardless of the fact that this is from a fork of a dead repo, the license attached to the file should still stand:

// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions
// are met:
//  * Redistributions of source code must retain the above copyright
//    notice, this list of conditions and the following disclaimer.
//  * Redistributions in binary form must reproduce the above copyright
//    notice, this list of conditions and the following disclaimer in the
//    documentation and/or other materials provided with the distribution.
//  * Neither the name of NVIDIA CORPORATION nor the names of its
//    contributors may be used to endorse or promote products derived
//    from this software without specific prior written permission.

It stands well enough for Unity to use it (I know, not a great argument, but still, it's something).

[clayjohn]

Revisiting this. I see that I mistakenly thought that the 3.00 version of the Gameworks SDK did not include FXAA 3.11. But it did. Accordingly, I can confirm that the mirror hosted by hghdev contains the most up to date version of FXAA from NVIDIA. Accordingly, we should use that version and we should copy the licence text from that version (https://github.com/hghdev/NVIDIAGameWorks-GraphicsSamples/blob/master/samples/es3-kepler/FXAA/FXAA3_11.h)

This PR uses the wrong licence (it uses the licence for Rendu, not FXAA).

To move forward with this PR, we need to update it to use the correct licence, and then also reproduce the licence in the COPYRIGHT.txt file here (between Bullet and ASSAO):

godot/COPYRIGHT.txt

Lines 131 to 133 in 292e50e

	License: Expat and Zlib
	Files: ./servers/rendering/renderer_rd/shaders/ss_effects_downsample.glsl

Files: ./servers/rendering/renderer_rd/shaders/effects/tonemap.glsl
Comment: FXAA 3.11
Copyright: 2014-2015, NVIDIA CORPORATION
License: BSD-3-clause

[mrjustaguy]

Is there anything else that needs to be done for this to be merged then?

[clayjohn]

Is there anything else that needs to be done for this to be merged then?

Probably not. I still need to do a careful review of the code, but if everything is working well then we can merge this for 4.4!