Page MenuHome

Compositor: Added denoising node
Needs ReviewPublic

Authored by Stefan Werner (swerner) on Feb 4 2019, 10:28 PM.
Tokens
"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
Branch
openimagedenoise
Build Status
Buildable 3403
Build 3403: arc lint + arc unit

Event Timeline

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

OpenImageDenoise is adding about ~35MB to the binary, and the current version builds as shared library only (static library should be coming soon). We can debate whether adding 35MB for a single feature is worth it or not (or if we want to keep it as a compile time option that's off by default).

make deps is working for macOS and Linux, haven't tested Windows. install_deps.sh isn't updated yet.

Only COM_DenoiseOperation.cpp is dependent on OpenImageDenoise, we could reuse this patch to integrate other denoisers into the compositor, for example the Cycles denoiser by @Lukas Stockner (lukasstockner97).

Sergey Sharybin (sergey) requested changes to this revision.Feb 5 2019, 9:59 AM

I'm not sure whether it's just an experiment or what, but i am not convinced in this.

Even keeping specific implementation aside, the OpenImagheDenoise fails in a simple scenes, and does almost nothing on our production shots.

I also don't think we should be having more denoising algorithm is a great idea. Better to make Cycles' denoiser available in compositor. So then we can always replicate final F12 result when only having multilayer EXR.

source/blender/blenkernel/intern/node.c
3520–3522

You shouldn't be conditionally registering nodes. That will cause data loss for no reason when opening files in Blender compiled without specific feature.

Not even mentioning that WITH_OPENIMAGEDENOISE is never defined here.

source/blender/compositor/operations/COM_DenoiseOperation.cpp
115–118

Attempts of such a threaded copying almost always ending up a reason of performance drop.
Is better to leave threads ready for an actual work.

source/blender/compositor/operations/COM_DenoiseOperation.h
29

Within the current compositor setup nodes are supposed to be tileable. I do not see big issue why this specific node can't be tiled, and having it as a single threaded operation will only cause issues (in terms of how instant the result is appearing in the interface).

Another thing here is that it seems you're relying on OpenImageDenoise to do threading. That is a huge design flaw: nobody should be summoning working threads from a working thread,

source/blender/editors/space_node/drawnode.c
2684–2685

Missing break statement.

This revision now requires changes to proceed.Feb 5 2019, 9:59 AM

I'm not sure whether it's just an experiment or what, but i am not convinced in this.
Even keeping specific implementation aside, the OpenImagheDenoise fails in a simple scenes, and does almost nothing on our production shots.
I also don't think we should be having more denoising algorithm is a great idea. Better to make Cycles' denoiser available in compositor. So then we can always replicate final F12 result when only having multilayer EXR.

I am not sure if that's error in the implementation or your testing methodology, but I can assure you that OpenImageDenoise is vastly superior to current Cycles denoiser. Tests done by ChaosGroup and Corona confirm it's very capable denosier, and both of the render engines plan to implement it based on the surprisingly good results.

Could you please provide the test results on which you base your conclusion? The tests done by pretty much everyone else suggest the exact opposite.

It's the job of the developer submitting the new code or the users to demonstrate the new denoiser is better, not the other way around. So I would love to see some test renders with this patch.

As I understand it this denoiser is similar to the one in Optix: designed to work fast and give good results for low sample counts, and suited for interactive viewport rendering. This then comes at the cost of blurring out too much detail at high sample counts and more complex materials, which makes it not great for final frames.

  • Fixed code style.
  • Cleanup in denoiser compositing node.
Stefan Werner (swerner) marked 3 inline comments as done.Feb 5 2019, 4:08 PM

Please check this thread with several tests, expand the post to show the comparisons. Other test and video are submitted across the topic.

https://blenderartists.org/t/intel-open-image-denoise-released-on-github/1144893/2

Right, the comparisons in that thread show that OpenImageDenoise is better at low sample counts and that makes it valuable.

What I didn't see is a comparison at high sample counts, which is what the current denoiser is aimed at. Comparing it for 1 sample is interesting but doesn't extrapolate that easily.

Ideally we'd have a single denoiser that can do everything, but it seems a bit unlikely we'd get this anytime soon. So until then, multiple denoisers is acceptable in my opinion.

Regarding integrating the Cycles denoiser into the Compositor node: I'm not sure how we would implement that - the animation denoiser patch adds support for standalone denoising, but we'd still need to call that from Blender somehow, and adding a function to the Renderer API specifically for that seems far from ideal as well.

Instead, the way I'd like to implement denoising after rendering is in the form of a general way of continuing a render after it finished, generating an updated result. This is something that has been requested for a long time, an option to only denoise would fit perfectly into the design and code for reading an Image from Blender back into Cycles already exists in D3108.

On a different note, it seems that this denoiser is far on the blurring side in the noise vs. blurring tradeoff, so I think it might be worth looking into using its output as either another feature pass for the Cycles denoiser or as a clean base on which to compute the NLM weights. That way, we could get the detail preservation of the Cycles denoiser combined with the smooth low-spp results of OIDN.

You need to be careful when looking at test posted in various forums - naturally, OIDN shows a big quality difference between with and without normal/albedo passes. The HDR option also makes a significant difference, when it's turned off it is much more aggressive at blurring bright areas.

This patch was born out of curiosity, I simply wanted to test OIDN, and turning it into a compositing node seemed like a natural choice. Uploading it here should allow others to experiment with it too, and should we find it useful, we can go on merging it to master from here. I don't expect anyone to add 35MB of payload just because I say so.

One thing no one is bringing up is the built in cycles denoiser isn't worth turning on half the time because the amount of time and overhead it adds you can do twice as many samples. We had a 4k render recently take over a minute longer with the denoiser on vs 4x the samples.

We build highly optimized cycles scenes and the cycles denoiser is not highly optimized. The Intel denoiser adds at most 6 seconds to our renders.

Sometimes you gotta ask if it's worth making people suffer for a long time so you can add the perfect feature to blender or just pull the trigger and make everyone happy and give them usable and features that are needed while you can polish them later. If you added the same functionality and speed to cycles denoiser, good job now you can remove the Intel denoiser. No one uses blender because it's stable lol

Here some comparisons at higher sample counts, with hair, SSS, depth of field and motion blur.
11_01_A.lighting.flamenco, 50% resolution, 1024 AA samples.

Raw image without denoising:

Cycles denoiser (default settings):

OpenImageDenoise:

Here some comparisons at higher sample counts, with hair, SSS, depth of field and motion blur.
11_01_A.lighting.flamenco, 50% resolution, 1024 AA samples.

Could you please also post 256 sample results, or something like that? It will be much better at showing how inferior Cycles denoiser is to OIDN :)

The thing is that Cycles denoiser only works with such a high amount of samples that it almost looks like final quality. OIDN on the other hand can generate relatively artifact free images from quite noisy ones. Cycles denoiser can't.

Luxcore team is experimenting the integration with Intel OIDN. Keep in mind their previous Bayesian denoiser was overblurring everything (and was much inferior compared to Cycles one), though the test shows the OIDN could do well with higher samplecounts too.

https://blenderartists.org/uploads/default/original/4X/9/1/5/9155fe9d7ed8d54014b5e6acd06709b3fb115efc.jpeg

I think I found a Bug in the current Implementation.
The Denoising Albedo pass contains the Texture data in the value range from 1-2, resulting in blurry textures.
If I manually subtract 1 for every channel, the Image is much sharper.
However, it appears that metal colors are stored in the range of 0-1, just subtracting by one may not be the final solution.
Here is an example render with 4 Samples:

This Denoiser is beyond impressive, even high sample renderings benefit from it and don’t lose details.

I tired this on a Theory Studios build and it is amazing!

However I'm trying to apply the patch and I get several erros, please excuse my "noobness" but are this errors important? can they be ignored? or how can I solve them?

patching file CMakeLists.txt
Hunk #1 succeeded at 237 (offset -5 lines).
patching file build_files/build_environment/CMakeLists.txt
patching file build_files/build_environment/cmake/harvest.cmake
Hunk #1 succeeded at 164 (offset 4 lines).
patching file build_files/build_environment/cmake/openimagedenoise.cmake
patching file build_files/build_environment/cmake/tbb.cmake
patching file build_files/build_environment/cmake/versions.cmake
patching file build_files/build_environment/patches/openimagedenoise.diff
patching file build_files/cmake/Modules/FindOpenImageDenoise.cmake
patching file build_files/cmake/macros.cmake
Hunk #1 succeeded at 308 (offset -5 lines).
Hunk #2 succeeded at 426 (offset -5 lines).
patching file build_files/cmake/platform/platform_apple.cmake
**Hunk #1 FAILED at 385.
1 out of 1 hunk FAILED -- saving rejects to file build_files/cmake/platform/platform_apple.cmake.rej**
patching file build_files/cmake/platform/platform_unix.cmake
Hunk #1 succeeded at 364 (offset -3 lines).
patching file release/scripts/startup/nodeitems_builtins.py
patching file source/blender/blenkernel/BKE_node.h
Hunk #1 succeeded at 949 (offset -9 lines).
patching file source/blender/blenkernel/intern/node.c
Hunk #1 succeeded at 3425 (offset -10 lines).
patching file source/blender/compositor/CMakeLists.txt
**Hunk #1 FAILED at 284.
Hunk #2 FAILED at 495.
Hunk #3 succeeded at 552 (offset -5 lines).
2 out of 3 hunks FAILED -- saving rejects to file source/blender/compositor/CMakeLists.txt.rej**
patching file source/blender/compositor/intern/COM_Converter.cpp
Hunk #1 succeeded at 53 (offset -4 lines).
Hunk #2 succeeded at 128 (offset -4 lines).
Hunk #3 succeeded at 408 (offset -4 lines).
patching file source/blender/compositor/nodes/COM_DenoiseNode.h
patching file source/blender/compositor/nodes/COM_DenoiseNode.cpp
patching file source/blender/compositor/operations/COM_DenoiseOperation.h
patching file source/blender/compositor/operations/COM_DenoiseOperation.cpp
patching file source/blender/editors/space_node/drawnode.c
Hunk #1 succeeded at 2499 (offset -9 lines).
Hunk #2 succeeded at 2740 (offset -9 lines).
patching file source/blender/makesdna/DNA_node_types.h
Hunk #1 succeeded at 1019 (offset -9 lines).
patching file source/blender/makesrna/intern/rna_nodetree.c
Hunk #1 succeeded at 6983 (offset -8 lines).
patching file source/blender/nodes/CMakeLists.txt
**Hunk #1 FAILED at 65.
1 out of 1 hunk FAILED -- saving rejects to file source/blender/nodes/CMakeLists.txt.rej**
patching file source/blender/nodes/NOD_composite.h
Hunk #1 succeeded at 74 (offset -9 lines).
patching file source/blender/nodes/NOD_static_types.h
Hunk #1 succeeded at 210 (offset -7 lines).
patching file source/blender/nodes/composite/nodes/node_composite_denoise.c

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

In defense of the built-in Cycles denoiser, one needs to take into account the fact that it produces markedly better results on a noticeably noisy image when Branched Path Tracing is used instead of normal Path Tracing (especially in areas like highlights and reflections). There have also been improvements in recent 2.8 builds as indicated by the pass data.

Another thing to note is that the relative checkbox being ticked will blur reflections because the pass information shows an either/or result when it comes to reflected and refracted data (so no information).

However, the flipside is the built-in denoiser can end up leaving some noisy pixels untouched, the new library-based solutions do not. Though leftover noise is a heck of a lot easier to clean up in post than an overblurred area.

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?