Cycles: Add Intel GPU support for OpenImageDenoise #108314

OpenImageDenoise V2 comes with GPU support for various backends. This adds a new class, OIDN2Denoiser, in order to add this functionality into the existing Cycles post processing pipeline without having to change it much. OptiX and OIDN CPU denoising remain as they are, there is now a new user option to select OIDN GPU denoising when a compatible device is detected.

Device support is initially limited to the oneAPI devices that are supported by Cycles already, but can be extended.

OpenImageDenoise V2 comes with GPU support for various backends. This adds a new class, OIDN2Denoiser, in order to add this functionality into the existing Cycles post processing pipeline without having to change it much. OptiX and OIDN CPU denoising remain as they are, there is now a new user option to select OIDN GPU denoising when a compatible device is detected. Device support is initially limited to the oneAPI devices that are supported by Cycles already, but can be extended.

on the deps, something similar as in 66b4e426cc needs to be done, esp using ninja on Windows (CMAKE_GENERATOR ${PLATFORM_ALT_GENERATOR})
https://github.com/OpenImageDenoise/oidn/blob/master/doc/compilation.md also recommends using ocloc 101.4314 but it has known issues with Blender, updating ocloc to a newer version such as 101.4369 will be needed.

on the deps, something similar as in https://projects.blender.org/blender/blender/commit/66b4e426cc3d8a12adca585bea217f2013568393 needs to be done, esp using ninja on Windows (`CMAKE_GENERATOR ${PLATFORM_ALT_GENERATOR}`) https://github.com/OpenImageDenoise/oidn/blob/master/doc/compilation.md also recommends using ocloc 101.4314 but it has known issues with Blender, updating ocloc to a newer version such as [101.4369](https://www.intel.com/content/www/us/en/developer/articles/tool/oneapi-standalone-components.html) will be needed.

I'll figure it out, not overly worried, I'll bring up the Windows/CPU support first, and we'll branch out from there.

Just to clarify: i'm working the deps builder part, best not to duplicate this work

I'll figure it out, not overly worried, I'll bring up the Windows/CPU support first, and we'll branch out from there. Just to clarify: i'm working the deps builder part, best not to duplicate this work

What is currently holding up this PR?

the WIP label

Please follow the Commit Message Guidelines and present the PR accordingly to what it is.

If it is just an update of a library without user-level changes then I am not really sure about deeper changes in the OIDNDenoiser and DenoiserGPU .

If this change actually brings GPU support for the OIDN it should be presented as such, and mentioned what are supported GPUs are, what are the limitations (user-level and technical), some major architectural considerations, etc.

Please follow the [Commit Message Guidelines](https://wiki.blender.org/wiki/Style_Guide/Commit_Messages) and present the PR accordingly to what it is. If it is just an update of a library without user-level changes then I am not really sure about deeper changes in the `OIDNDenoiser` and `DenoiserGPU `. If this change actually brings GPU support for the OIDN it should be presented as such, and mentioned what are supported GPUs are, what are the limitations (user-level and technical), some major architectural considerations, etc.

Initial pass of review.

@Sergey Thanks for the quick review. I think it'll be best if I make the first pass to support SYCL devices only, and then add CUDA device support, if possible, in a second PR.

~~git refuses to push to this diff, keeps insisting it has commits I don't have locally even after a fresh fetch.. .soo ... that's fun...~~

~~anyhow here's the changes needed for windows please commit them your self~~

edit: poor timing, we were working at the same time, all good now.

We're not aot'ing for the intel GPU's? when I previously scripted this I also had a OpenImageDenoise_device_sycl_xehpg.dll and OpenImageDenoise_device_sycl_xelp.dll ?

~~git refuses to push to this diff, keeps insisting it has commits I don't have locally even after a fresh fetch.. .soo ... that's fun...~~ ~~anyhow here's the changes needed for windows please commit them your self~~ edit: poor timing, we were working at the same time, all good now. We're not aot'ing for the intel GPU's? when I previously scripted this I also had a `OpenImageDenoise_device_sycl_xehpg.dll` and ` OpenImageDenoise_device_sycl_xelp.dll` ?

oidn2_windows.diff

11 KiB

Please update the PR description following Commit Message Guidelines. It helps both reviewers to know what are they looking at instead of figuring out what parts of the code are supposed to be related to.

Also, is it possible to split the library update to a separate PR which will not functional changes yet? This will help the review process and followup maintenance a lot.

Please update the PR description following [Commit Message Guidelines](https://wiki.blender.org/wiki/Style_Guide/Commit_Messages). It helps both reviewers to know what are they looking at instead of figuring out what parts of the code are supposed to be related to. Also, is it possible to split the library update to a separate PR which will not functional changes yet? This will help the review process and followup maintenance a lot.

Something seems have gone wrong with the latest update. It includes 400+ changes, which made the PR to have 1.5k files changed.

Not again. I don't know why or how git does that, I've had this before.

Should be fixed now. My apologies for the force push, I don't know of a better way to do this.

This PR still contains the changes from #112143. Once that has been merged, this one will be smaller.

This PR still contains the changes from https://projects.blender.org/blender/blender/pulls/112143. Once that has been merged, this one will be smaller.

This branch can now build against OIDN V1 and V2. The changes for build/deps are now entirely contained in #112143 and no longer included in this patch.

To test the new functionality in this branch, #112143 needs to be merged or applied locally first.

This branch can now build against OIDN V1 and V2. The changes for build/deps are now entirely contained in #112143 and no longer included in this patch. To test the new functionality in this branch, #112143 needs to be merged or applied locally first.

Is good to see an OIDN denoiser which follows the typical flow of the GPU denoiser so closely! Makes it clear to see the difference.

The small comment I have is the naming. Can't say the OIDN2Denoiser is the clearest: the existing/CPU denoiser uses the same library. This is why I was proposing to use OIDNDenoiserGPU to kind of put it into a scope of flow-based naming. As in, hey, this denoiser implements the GPU denoiser flow. Maybe there are better names. Or, maybe, if we can unify the CPU and GPU sides it will simply a OIDNDenoiser, as it is now.

On a user level I don't think we should have separate options for CPU and GPU OIDN: "just" choose the best fitting device. Similar to the OptiX denoiser which uses the same device for denoising as for path tracing whenever possible. We can even decide to choose available GPU device for denoising even when path tracing happens on a CPU. If a more granular control is needed, it is better to be implemented as an explicit device selection for denoising, which then can be used for both OIDN and OptiX. But such option is not something I'd start with.

When I was mentioning separation of CPU and GPU OIDN denoiser I mainly meant the code-side separation, as it is quite tricky to follow code which has a lot of top-level if-else statements. The Denoiser::create can choose which implementation to choose based on the device selection.

One thing I am did not have time to fully understand yet is the peak memory comparison. It seems that now when pre-filter is used memory for albedo and normal is always allocated, while previously it was allowed to modify data in-place. For example, when rendering happens on multiple devices and denoiser is used a temporary RenderBuffers is created with the result gathered from all the devices, denoised, and copied back to individual devices. It is only the denoised passes which are copied, which makes it possible to modify guiding passes in-place, without extra full-frame buffer allocated. This is something that should be supported by the new denoiser implementation, to keep memory usage under control as much as possible.

The CPU/GPU unification is very interesting story! Sounds like there are ways to avoid separate CPU and GPU implementations for OIDN, so that would need to happen. Thing I am not sure is whether it's better to do such unification from the get-go, or allow some time of both implementations to co-exist for the cross-referencing purposes. Think I feel that allowing some time to co-exist is a better and more achievable step. But I am open to be convinced otherwise :)

For the CPU queue I am not really sure is the best approach. If the queue is only needed to schedule kernels we can take care of that by abstracting the call/synchronization. We can make functions like denoise_filter_color_preprocess virtual and let the OIDN denoiser implementation to take care of whether using queue or a direct kernel call. Somehow it seems like a better idea than opening a topic of introduction of CPU queue.

Is good to see an OIDN denoiser which follows the typical flow of the GPU denoiser so closely! Makes it clear to see the difference. The small comment I have is the naming. Can't say the `OIDN2Denoiser` is the clearest: the existing/CPU denoiser uses the same library. This is why I was proposing to use `OIDNDenoiserGPU` to kind of put it into a scope of flow-based naming. As in, hey, this denoiser implements the GPU denoiser flow. Maybe there are better names. Or, maybe, if we can unify the CPU and GPU sides it will simply a `OIDNDenoiser`, as it is now. On a user level I don't think we should have separate options for CPU and GPU OIDN: "just" choose the best fitting device. Similar to the OptiX denoiser which uses the same device for denoising as for path tracing whenever possible. We can even decide to choose available GPU device for denoising even when path tracing happens on a CPU. If a more granular control is needed, it is better to be implemented as an explicit device selection for denoising, which then can be used for both OIDN and OptiX. But such option is not something I'd start with. When I was mentioning separation of CPU and GPU OIDN denoiser I mainly meant the code-side separation, as it is quite tricky to follow code which has a lot of top-level if-else statements. The `Denoiser::create` can choose which implementation to choose based on the device selection. One thing I am did not have time to fully understand yet is the peak memory comparison. It seems that now when pre-filter is used memory for albedo and normal is always allocated, while previously it was allowed to modify data in-place. For example, when rendering happens on multiple devices and denoiser is used a temporary RenderBuffers is created with the result gathered from all the devices, denoised, and copied back to individual devices. It is only the denoised passes which are copied, which makes it possible to modify guiding passes in-place, without extra full-frame buffer allocated. This is something that should be supported by the new denoiser implementation, to keep memory usage under control as much as possible. The CPU/GPU unification is very interesting story! Sounds like there are ways to avoid separate CPU and GPU implementations for OIDN, so that would need to happen. Thing I am not sure is whether it's better to do such unification from the get-go, or allow some time of both implementations to co-exist for the cross-referencing purposes. Think I feel that allowing some time to co-exist is a better and more achievable step. But I am open to be convinced otherwise :) For the CPU queue I am not really sure is the best approach. If the queue is only needed to schedule kernels we can take care of that by abstracting the call/synchronization. We can make functions like `denoise_filter_color_preprocess` virtual and let the OIDN denoiser implementation to take care of whether using queue or a direct kernel call. Somehow it seems like a better idea than opening a topic of introduction of CPU queue.

I think some user control over using CPU or GPU denoising would be helpful.

The safe would be to use GPU denoising only for GPU rendering by default, maybe also for CPU viewport rendering it's fine. But final and especially background CPU renders should probably not use the GPU by default, and be something that a user has to enable. Because you wouldn't want to automatically use whatever GPU may exist on a render farm node, when it may be a simple integrated or low memory CPU not meant for that purpose.

Perhaps this could be configured in the preferences rather than the scene?

Even better would be a way to configure in the preferences which GPU (if any) to use for denoising.

I think some user control over using CPU or GPU denoising would be helpful. The safe would be to use GPU denoising only for GPU rendering by default, maybe also for CPU viewport rendering it's fine. But final and especially background CPU renders should probably not use the GPU by default, and be something that a user has to enable. Because you wouldn't want to automatically use whatever GPU may exist on a render farm node, when it may be a simple integrated or low memory CPU not meant for that purpose. Perhaps this could be configured in the preferences rather than the scene? Even better would be a way to configure in the preferences which GPU (if any) to use for denoising.

That is all valid points, Brecht. I do agree such control is needed. Is just to me implementing it as a user preference or as a denoiser option in scene seems more flexible, more future proof, and reusable for OptiX than implementing such functionality via denoiser type.

I am not sure how OIDN implements device selection. Implementing the device used for OptiX denoiser seems straightforward: add a code to the find_best_denoiser_device_info which will return DeviceInfo for the selected in user preferences device.

One thing which would be nice to agree on is the breakdown in terms what gets implemented when.
The way i see it is:

Initial OIDN2 implementation follows simple rule: when rendering on oneAPI use oneAPI denoiser, otherwise use CPU denoiser. I think all OIDN capable devices are powerful enough to run denoiser.
Add a device selection
Unify code paths for CPU and GPU OIDN implementations.

How does it sound to you?

That is all valid points, Brecht. I do agree such control is needed. Is just to me implementing it as a user preference or as a denoiser option in scene seems more flexible, more future proof, and reusable for OptiX than implementing such functionality via denoiser type. I am not sure how OIDN implements device selection. Implementing the device used for OptiX denoiser seems straightforward: add a code to the `find_best_denoiser_device_info` which will return `DeviceInfo` for the selected in user preferences device. One thing which would be nice to agree on is the breakdown in terms what gets implemented when. The way i see it is: - Initial OIDN2 implementation follows simple rule: when rendering on oneAPI use oneAPI denoiser, otherwise use CPU denoiser. I think all OIDN capable devices are powerful enough to run denoiser. - Add a device selection - Unify code paths for CPU and GPU OIDN implementations. How does it sound to you?

Sounds fine, it's good to keep this PR simple and then add CPU render + GPU denoising support as a second step with a proper UI.

All currently supported (and planned) oneAPI GPU that can run Cycles can also run OIDN2. However, a user may want to render on the GPU and denoise on the CPU if memory is tight - I've seen a few scenes where that made a difference between rendering and out of memory errors. Likewise, someone with a scene too big for the GPU may still want to use GPU viewport denoising for performance reasons.

As a side note, denoising on a oneAPI GPU while rendering on an Nvidia GPU worked for me too. Since the reverse scenario, render on oneAPI and denoise in OptiX, was already working, this was not unexpected, still something I was happy to see.

@Sergey I'll take a look at memory consumption, if there are any excess allocations that can be skipped.

All currently supported (and planned) oneAPI GPU that can run Cycles can also run OIDN2. However, a user may want to render on the GPU and denoise on the CPU if memory is tight - I've seen a few scenes where that made a difference between rendering and out of memory errors. Likewise, someone with a scene too big for the GPU may still want to use GPU viewport denoising for performance reasons. As a side note, denoising on a oneAPI GPU while rendering on an Nvidia GPU worked for me too. Since the reverse scenario, render on oneAPI and denoise in OptiX, was already working, this was not unexpected, still something I was happy to see. @Sergey I'll take a look at memory consumption, if there are any excess allocations that can be skipped.

If someone has multiple GPUs, especially if they are not equal in performance it does not feel enough to have just CPU and GPU separation, and a more granular control seems to be a good idea. Having it could also benefit OptiX denoiser.

Is it OK if I make a suggestion as a user?

To keep it as simple as possible, In the preferences have checkboxes for:

denoising devices (same as cycles render devices). Allow user to exclude slow devices. If any GPU selected then assume user always wants to favour GPU denoising.

if CPU and GPU are ticked in the denoising devices list, always favour GPU unless multiple compositor denoise nodes, in which case assign one GPU to each node, and one node to the CPU. When no nodes remaining, have the GPU restart the CPU's node if time remaining on it > GPU's average denoise time per completed node.

Beyond that I think everything should be automatic based on a quick calculation prior to execution to ensure only eligible GPU's are used based on them having sufficient VRAM to complete the task (for example if VRAM can't be emptied before denoising/moving onto the next frame due to persistent data being enabled (unless it could be briefly moved to system ram during denoising?))

If no eligible GPUs are found, then revert to CPU (even if not chosen as a denoise device)

I think those options and logic will keep it as simple as possible for the user, whilst ensuring best possible performance, without the user having to mess about trying to calculate if their GPU has sufficient resources for the resolution they wish to denoise (which is tricky because the user often doesn't have a clue how much VRAM is in use/available when cycles finishes sampling and denoising/compositor execution begins).

Is it OK if I make a suggestion as a user? To keep it as simple as possible, In the preferences have checkboxes for: - denoising devices (same as cycles render devices). Allow user to exclude slow devices. If any GPU selected then assume user always wants to favour GPU denoising. if CPU and GPU are ticked in the denoising devices list, always favour GPU unless multiple compositor denoise nodes, in which case assign one GPU to each node, and one node to the CPU. When no nodes remaining, have the GPU restart the CPU's node if time remaining on it > GPU's average denoise time per completed node. Beyond that I think everything should be automatic based on a quick calculation prior to execution to ensure only eligible GPU's are used based on them having sufficient VRAM to complete the task (for example if VRAM can't be emptied before denoising/moving onto the next frame due to persistent data being enabled (unless it could be briefly moved to system ram during denoising?)) If no eligible GPUs are found, then revert to CPU (even if not chosen as a denoise device) I think those options and logic will keep it as simple as possible for the user, whilst ensuring best possible performance, without the user having to mess about trying to calculate if their GPU has sufficient resources for the resolution they wish to denoise (which is tricky because the user often doesn't have a clue how much VRAM is in use/available when cycles finishes sampling and denoising/compositor execution begins).

 @ -139,2 +143,4 @@
     info.has_nanovdb = true;
     info.denoisers = 0;
 #  if defined(WITH_OPENIMAGEDENOISE) && defined(OIDN_DEVICE_CUDA)
     if (major >= 7) {

 @ -57,2 +57,4 @@
   bool denoise_filter_guiding_set_fake_albedo(const DenoiseContext &context);
   /* Read guiding passes from the render buffers, preprocess them in a way which is expected by
    * OptiX and store in the guiding passes memory within the given context.

 @ -577,4 +637,3 @@
       << "OpenImageDenoiser is not supported on this platform or build.";
 #ifdef WITH_OPENIMAGEDENOISE
   thread_scoped_lock lock(mutex_);

 @ -573,12 +628,15 @@ bool OIDNDenoiser::denoise_buffer(const BufferParams &buffer_params,
                                   const int num_samples,
                                   bool allow_inplace_modification)
 {
   if (denoiser_device_->info.type != DEVICE_CPU && !cpu_fallback_) {

 @ -621,9 +679,45 @@ bool OIDNDenoiser::denoise_buffer(const BufferParams &buffer_params,
   return true;
 }
 bool OIDNDenoiser::denoise_buffer(const DenoiseTask &task)

 @ -641,2 +737,3 @@
   return Denoiser::ensure_denoiser_device(progress);
 #  else
   Device *denoiser_device = Denoiser::ensure_denoiser_device(progress);

 @ -643,0 +749,4 @@
 #endif
 }
 bool OIDNDenoiser::denoise_ensure(DenoiseContext &context)

 @ -643,0 +799,4 @@
 #if defined(OIDN_DEVICE_CUDA) && defined(WITH_CUDA)
     case DEVICE_CUDA:
     case DEVICE_OPTIX: {
       int device_id = static_cast<CUDADevice *>(denoiser_queue_->device)->cuDevId;

 @ -643,0 +801,4 @@
     case DEVICE_OPTIX: {
       int device_id = static_cast<CUDADevice *>(denoiser_queue_->device)->cuDevId;
       CUstream stream = nullptr;
       oidn_device_ = oidnNewCUDADevice(&device_id, &stream, 1);

 @ -643,0 +807,4 @@
 #    endif
     /* Devices without explicit support fall through to CPU backend. */
     default:
       oidn_device_ = oidnNewDevice(OIDN_DEVICE_TYPE_CPU);

 @ -643,0 +832,4 @@
     oidnSetFilterInt(oidn_filter_, "cleanAux", true);
   }
   albedo_filter_ = oidnNewFilter(oidn_device_, "RT");

 @ -30,6 +32,7 @@ const NodeEnum *DenoiseParams::get_type_enum()
   if (type_enum.empty()) {
     type_enum.insert("optix", DENOISER_OPTIX);
     type_enum.insert("openimageio", DENOISER_OPENIMAGEDENOISE);

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Cycles: Add Intel GPU support for OpenImageDenoise #108314

Reviewers