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

NRD ReBLUR : Slang Compiler issue #369

Open
SirKero opened this issue Jul 6, 2023 · 5 comments
Open

NRD ReBLUR : Slang Compiler issue #369

SirKero opened this issue Jul 6, 2023 · 5 comments

Comments

@SirKero
Copy link

SirKero commented Jul 6, 2023

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
Copy link
Collaborator

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
Copy link
Author

SirKero commented Jul 7, 2023

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
Copy link
Collaborator

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
Copy link
Author

SirKero commented Jul 14, 2023

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:

  • All unsupported options are just commented out. I did not check for new parameters.
  • Both Delta NRD passes now show ghosting. The radiance buffers in NRD are seemingly not cleared anymore and the guide buffers from the path tracers do not seem to be 0 at non-reflective/transmissive materials.
  • ReBLUR seems not right.

@skallweitNV
Copy link
Collaborator

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.

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

No branches or pull requests

2 participants