NVIDIAGameWorks / Falcor

Real-Time Rendering Framework
https://developer.nvidia.com/falcor
Other
2.65k stars 487 forks source link

NRD ReBLUR : Slang Compiler issue #369

Open SirKero opened 1 year ago

SirKero commented 1 year ago

The current included version of NRD (3.1.0) creates a slang compiler error when switching to the ReBLUR mode. It is caused by two cases of an ambiguous call to "&" (bool,int) in the Shader "nrd/Shaders/Include/REBLUR/REBLUR_DiffuseSpecular_TemporalAccumulation.hlsli".

The affected lines are: 241 isCatRomAllowedForSurfaceMotion = isCatRomAllowedForSurfaceMotion & REBLUR_USE_CATROM_FOR_SURFACE_MOTION_IN_TA; 405 isCatRomAllowedForVirtualMotion = isCatRomAllowedForVirtualMotion & REBLUR_USE_CATROM_FOR_VIRTUAL_MOTION_IN_TA;

For those affected, this can be easily fixed (change logical AND "&" with a bitwise AND "&&" or casting both defines to bool).

As this is an external package, is an update to the current version of NRD (4.2.2) planned? The type ambiguity seems to be fixed in that version.

skallweitNV commented 1 year ago

Updating NRD is not high up the priority list, but if there are no conflicts and things run out of the box, we should. Have you tried updating NRD?

SirKero commented 1 year ago

I've tried it and it sadly does not run out of the box. It seems some functions and settings changed between NDR versions.

I would look into it in more detail sometime next week and update this thread as the newer NDR version would be nice to have due to performance and quality updates (and hopefully no more slang compiler issues).

skallweitNV commented 1 year ago

If it helps, I packaged a new version of NRD available under the packman version tag 4.2.2-falcor-windows-x86_64. You can just update the dependencies.xml and run setup.bat to get the new version. But yeah, looks like there are some changes in the settings etc. not sure how hard it is to integrate it.

SirKero commented 1 year ago

I've got the current version of NRD to run, but the 4.2.2 version contains more instances of slang compiler errors (~3 parts ) due to the type ambiguity. Additionally, they changed their internal shader compile system, causing all includes to not work with slang (which can be fixed by putting all shaders in the same folder).

Nonetheless, here is a .zip containing the changes to the Falcor NRD-Pass to make it work, except for the errors described above, in case it is needed: NRDPass.zip

Some additional errors/TODOs remaining:

skallweitNV commented 1 year ago

Thanks for your effort! I was off on vacation and cannot take a look right now, but I will put this on my TODO to look at.