Page MenuHome

Compositor: Added denoising node
ClosedPublic

Authored by Stefan Werner (swerner) on Feb 4 2019, 10:28 PM.
Tokens
"Mountain of Wealth" token, awarded by finnb."Love" token, awarded by slumber."Like" token, awarded by yeus."Like" token, awarded by YAFU."Love" token, awarded by ace_dragon."Love" token, awarded by vitos1k."Love" token, awarded by LukeD."Love" token, awarded by juang3d."Love" token, awarded by lordodin."Like" token, awarded by 5k1n2."Love" token, awarded by marcog."Like" token, awarded by TheRedWaxPolice.

Details

Summary

This node is built on Intel's OpenImageDenoise library.
Other denoisers could be integrated, for example Lukas' Cycles denoiser.

Compositor: Made OpenImageDenoise optional, added CMake and build_env files to find OIDN

Compositor: Fixed some warnings in the denoising operator

build_environment: Updated OpenImageDenoise to 0.8.1

build_environment: Updated OpenImageDenoise in make deps for macOS

Diff Detail

Repository
rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Ive been testing with denoising the light passes instead of the images and it does a significantly better job keeping the texture data.

Look two posts above. There's a bug that causes textures to get too blurry.

Ive been testing with denoising the light passes instead of the images and it does a significantly better job keeping the texture data.

Look two posts above. There's a bug that causes textures to get too blurry.

If you look closely in the picutres, is not just that the textures are blurry or not, it's also that there are places were the plain denoiser does not work, and denoising it in passes the denoise is perfect and full.
May be related to the textures bug, I don't know, but the texurtes thing is not the only thing denoising in a per-pass fashion solves.

Cheers!

Ive been testing with denoising the light passes instead of the images and it does a significantly better job keeping the texture data.

The second, directly denoised image looks obviously wrong. If the albedo image is indeed wrong as others reported, that could cause not just blurring but any other kind of artifact too. The resulting behavior is pretty much undefined. Separately denoising the light passes likely results in somewhat better quality but the difference shouldn't be nearly as noticeable as on these images.

BTW, when comparing OIDN vs the Cycles denoiser, please keep in mind that the Cycles denoiser has a lot more auxiliary information to work with (variance, depth, etc.). Currently OIDN supports only albedo and normal buffers, but we're planning to add support for more soon, so the quality will improve. We're also planning to extend the training dataset, so the denoiser will get better and better.

OIDN does work with Animations and it also works with high sample renders. The bug I showed earlier was rendered with just 4 samples just to make the point very clear.
This setup is a simple fix for the wrong “Denoising Albedo” pass:


@derek barker (lordodin) You should try this setup. It should fix the areas where no denoising is happening and improves the texture sharpness.

Here are Images to prove that this Denoiser converges to the ground truth.
This is some Snow I've built this week, it has a complex shape and the Shader uses subsurface scattering.
128spp:

128spp Denoised:

1024spp:

1024spp Denoised:

And here is a video to show the temporal stability 128spp:

After all my tests, I don’t see any reason to not include it in master after the Bug was Fixed.

Nice tests @Malte S (pandrodor), i also had issues with Albedo pass being in the wrong range for some objects in my scenes and they turned white , I used the same fix as you did but isn't the Mix node unnecessary? I got same results with just the Greater Than node and the Color Subtract.

In my tests OIDN gives quite impressive results, I can't share renders at the moment since i played with my commercial projects. I noticed the amount of memory used is quite lower compared to standard Cycles denoiser, that could be a benefit too.
I hope it could be polished and implemented soon, and considering what @Attila Afra (aafra) said about them working to improve it even further...!

@Stefan Werner (swerner) Do you plan to work on it further? They updated the Documentation about Albedo and Normal https://openimagedenoise.github.io/documentation.html

What are you doing to get Albedo > 1? I didn't see that happen in my test scenes. Adding clamping to the albedo input will be easy though.

What are you doing to get Albedo > 1? I didn't see that happen in my test scenes. Adding clamping to the albedo input will be easy though.

Hi @Stefan Werner (swerner)
I figured out in my tests, the Albedo > 1 appears when a material (Pricipled BSDF), has Specular > 1.

If you look here, first image shows the brown material with Specular = 0.0, second image has same settings but a slight Specular value was added (about 0.1 or so)
When adding Specular, Albedo denoise pass goes above 1.0, thus resulting in pure white and then not preserving details in case a textured material is used.

What are you doing to get Albedo > 1? I didn't see that happen in my test scenes. Adding clamping to the albedo input will be easy though.

Hi @Stefan Werner (swerner)
I figured out in my tests, the Albedo > 1 appears when a material (Pricipled BSDF), has Specular > 1.
If you look here, first image shows the brown material with Specular = 0.0, second image has same settings but a slight Specular value was added (about 0.1 or so)
When adding Specular, Albedo denoise pass goes above 1.0, thus resulting in pure white and then not preserving details in case a textured material is used.


Ive been testing with denoising the light passes instead of the images and it does a significantly better job keeping the texture data.



Hi where can i get the cycles denoice version,can you share link.thank you.

FYI, I'm waiting for Intel to release the next version of OIDN that can build as a static library.

That is quite interesting results, and clearly there is a niche where OIDN clearly shines. (probably, i was testing it on too complex or too simple file).
SO me being skeptical in the beginning is more like feeling that is something useful to have. Probably, also as an option for Cycles itself.

@Stefan Werner (swerner) Intel said that they just uploaded 0.8.2 with that you said fixed!

This comment was removed by Kalyan (coder.kalyan).
  • Merge remote-tracking branch 'origin' into openimagedenoise
  • Merge branch 'master' of git.blender.org:blender into openimagedenoise
  • Fixed padding for OIDN node.
  • build_environemnt: Updated OpenImageDenoise to static 0.8.2
  • build_environment: OpenImageDenoise fixes for Linux and Windows.
  • build_environment: Fixed FindOpenImageDenoise.cmake.
  • build_environment: Fixed Windows build with static OpenImageDenoise.
  • Compositor: More fixes for OIDN build on Windows.
  • Added support for debug versions of OpenImageDenoise Windows DLLs.
Harbormaster completed remote builds in B3257: Diff 14485.

I can't get this branch to compile properly. At the end of compilation, I get the following errors (there are more, but these are the first):

[100%] Built target bf_compositor
Scanning dependencies of target blender
[100%] Building C object source/creator/CMakeFiles/blender.dir/buildinfo.c.o
[100%] Linking CXX executable ../../bin/blender
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(device.cpp.o):device.cpp:function oidn::Device::commit(): error: undefined reference to 'tbb::interface7::internal::task_arena_base::internal_max_concurrency(tbb::interface7::task_arena const*)'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(device.cpp.o):device.cpp:function std::_Sp_counted_ptr_inplace<tbb::interface7::task_arena, std::allocator<tbb::interface7::task_arena>, (__gnu_cxx::_Lock_policy)2>::_M_dispose(): error: undefined reference to 'tbb::interface7::internal::task_arena_base::internal_terminate()'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::AutoencoderFilter::execute(): error: undefined reference to 'tbb::interface7::internal::task_arena_base::internal_initialize()'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::AutoencoderFilter::execute(): error: undefined reference to 'tbb::interface7::internal::task_arena_base::internal_execute(tbb::interface7::internal::delegate_base&) const'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::AutoencoderFilter::commit(): error: undefined reference to 'tbb::interface7::internal::task_arena_base::internal_initialize()'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::AutoencoderFilter::commit(): error: undefined reference to 'tbb::interface7::internal::task_arena_base::internal_execute(tbb::interface7::internal::delegate_base&) const'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::InputReorderNode<16, oidn::SRGBTransferFunc>::execute(): error: undefined reference to 'tbb::task_group_context::init()'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::InputReorderNode<16, oidn::SRGBTransferFunc>::execute(): error: undefined reference to 'tbb::internal::allocate_root_with_context_proxy::allocate(unsigned long) const'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::InputReorderNode<16, oidn::SRGBTransferFunc>::execute(): error: undefined reference to 'tbb::internal::get_initial_auto_partitioner_divisor()'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::InputReorderNode<16, oidn::SRGBTransferFunc>::execute(): error: undefined reference to 'tbb::interface7::internal::task_arena_base::internal_current_slot()'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::InputReorderNode<16, oidn::SRGBTransferFunc>::execute(): error: undefined reference to 'tbb::task_group_context::~task_group_context()'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::InputReorderNode<16, oidn::SRGBTransferFunc>::execute(): error: undefined reference to 'tbb::internal::allocate_root_with_context_proxy::free(tbb::task&) const'
/home/aliasguru/blender-git/lib/linux_x86_64/openimagedenoise/lib/libOpenImageDenoise.a(autoencoder.cpp.o):autoencoder.cpp:function oidn::InputReorderNode<16, oidn::SRGBTransferFunc>::execute(): error: undefined reference to 'tbb::task_group_context::~task_group_context()'

and so on and so forth

master compiles just fine. I have created the static libs using make deps command. Any recommendations?

  • Merge 'master' into 'openimagedenoise'
  • Merge 'master' into 'openimagedenoise'
  • Merge 'master' into 'openimagedenoise'

Will this branch make it into stable 2.80? Is there anything else that needs to be done - maybe I could help?

  • build_environment: Updated to OIDN 1.0.0.
Brecht Van Lommel (brecht) requested changes to this revision.EditedMon, Aug 5, 4:26 PM

I think we can merge this into master quickly, I don't really have any issues with the implementation as it is.

It can be committed disabled by default, and then we can commit precompiled libraries, set up the buildbot and enable it by default soon after.

I would like to unify this with our existing denoiser (or other future denoisers) at some point, preferably before the 2.81. The current Cycles denoiser could be made to work at the end of rendering, on the entire image buffer. This would take some extra CPU memory, but it requires fewer passed than it used to and we are loading those into memory at the end anyway.

Then OpenImageDenoise could then be integrated outside the compositor as well, and the Cycles denoiser could become available in the compositor.

CMakeLists.txt
240

To commit, this should be disabled by default until we have the precompiled libraries.

This option should also be added to blender_full.cmake (ON), blender_lite.cmake (OFF) and blender_release.cmake (ON).

build_files/cmake/platform/platform_unix.cmake
372

This should not be a required library, and instead automatically turn off with a message if the library is not found. See for example OpenColorIO right above this.

source/blender/makesrna/intern/rna_nodetree.c
7609–7617

I'm not sure we need an sRGB option. Images in the compositor are in scene linear color space by design.

Is there a practical use case for this, or was it more a matter of just exposing all available options?

source/blender/nodes/composite/nodes/node_composite_denoise.c
45–46

Use true and false.

This revision now requires changes to proceed.Mon, Aug 5, 4:26 PM

I think we can merge this into master quickly, I don't really have any issues with the implementation as it is.
It can be committed disabled by default, and then we can commit precompiled libraries, set up the buildbot and enable it by default soon after.
I would like to unify this with our existing denoiser (or other future denoisers) at some point, preferably before the 2.81. The current Cycles denoiser could be made to work at the end of rendering, on the entire image buffer. This would take some extra CPU memory, but it requires fewer passed than it used to and we are loading those into memory at the end anyway.
Then OpenImageDenoise could then be integrated outside the compositor as well, and the Cycles denoiser could become available in the compositor.

Doing denoising aftermath is a good idea (at least as an option). My tests show a much faster denoising (not more than +10% of render time) when run from cmd, rather than while rendering (+30% of render time on a full frame). Plus the memory consumption. You can run out of memory if the scene is close to the vram limit with active denoising. Running the denoising process after the fact frees the render time memory.

  • Compositor: Addressed Brecht's requests for OIDN node.

I guess the remaining issue is the static vs. dynamic library issue?

Static would be more consistent with what we do already, and avoids conflicts in case external renderers ship with their own OpenImageDenoise. So I think there's preferable.

If it must be a shared library, we need code to install it along with the executable, and I guess set an rpath to find the library on Linux and macOS.

make deps and everything else should be set up for static builds, unless I missed something.

It appears that OIDN_STATIC_LIB requires CMake 3.13, which I didn't have. Without that it builds a dynamic library

Normally we don't require a CMake version that recent, it should check the CMake version in openimagedenoise.cmake and abort with an error if needed.

  • Support building OpenImageDenoise with older CMake versions
  • Change Normal socket to vector type
  • Fix use of use removed property in UI
  • Show warning in node for builds without OpenImageDenoise

This is now ready to commit as far as I'm concerned.

Once that is done we can do a call on bf-committers for platform maintainers to compile the libraries.

LazyDodo (LazyDodo) added inline comments.
CMakeLists.txt
240

For commit all of them (full/lite/release) should be off until we have libs i assume?

YAFU (YAFU) added a subscriber: YAFU (YAFU).EditedMon, Aug 12, 6:36 PM

Hi.
Will the compilation and installation of OIDN dependencies through install_deps.sh be automated?

Turn off OpenImageDenoise for full/release builds until we have libraries.

CMakeLists.txt
240

You're right, patch updated.

Hi.
Will the compilation and installation of OIDN dependencies through install_deps.sh be automated?

Working on it, but it isn't that easy. Here's a WIP patch:

The problem is that OIDN depends on fairly recent versions of CMake and TBB, which not all distributions may provide in their package managers.

Stefan Werner (swerner) marked 2 inline comments as done.Wed, Aug 14, 12:46 PM

@Brecht Van Lommel (brecht) should I land the patch in its current state (OIDN disabled by default) or do we want to wait for install_deps.sh to be fully working first?

The windows libs have been added to SVN.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.

I think it's potentially confusing that we have a "Denoise" Cycles feature configurable through View Layer settings and an identically named "Denoise" node in the Compositor which is a completely different denoising system.

I think most users would expect these to be the same thing based on the naming. The new Compositing node really needs a "Method" setting (even if "Intel OpenImage" is currently the only option), or it should be named more specifically like "OpenImage Denoise" so people know what they're actually getting.

Any eta. when this denoiser will be in daily builds that are aviable on blender site? (linux)

+ i do agree that it should have prefix like. Intel denoise, Optix denoise, Blender denoise.

Never mind todays build is working.