Compositor: Unify Denoise node between CPU and GPU #118553

Merged

Omar Emara merged 1 commits from OmarEmaraDev/blender:unify-denoise-gpu-cpu into main

2024-02-21 13:23:14 +01:00

Omar Emara commented

2024-02-21 11:07:10 +01:00

Member

The CPU compositor always adds a filter for normal and albedo even if
they don't contain any data, that is, when they are not connected to
anything, by inflating a buffer with the editor value, which is not even
accesible to the user. In anycase, such a solid color will not help do
anything in the denoise operation.

This patch aligns the implementation by only adding the abeldo and
normal filter if they contain actual data, speeding up the operation in
the process.

The CPU compositor always adds a filter for normal and albedo even if they don't contain any data, that is, when they are not connected to anything, by inflating a buffer with the editor value, which is not even accesible to the user. In anycase, such a solid color will not help do anything in the denoise operation. This patch aligns the implementation by only adding the abeldo and normal filter if they contain actual data, speeding up the operation in the process.

Omar Emara added the

labels 2024-02-21 11:07:11 +01:00

Omar Emara added 1 commit 2024-02-21 11:07:19 +01:00

ae76a6349d Compositor: Unify Denoise node between CPU and GPU

The CPU compositor always adds a filter for normal and albedo even if
they don't contain any data, that is, when they are not connected to
anything, by inflating a buffer with the editor value, which is not even
accesible to the user. In anycase, such a solid color will not help do
anything in the denoise operation.

This patch aligns the implementation by only adding the abeldo and
normal filter if they contain actual data, speeding up the operation in
the process.

Omar Emara requested review from Sergey Sharybin 2024-02-21 11:07:47 +01:00

Omar Emara referenced this pull request

2024-02-21 11:08:24 +01:00

Compositor: CPU vs GPU Differences #118548

Sergey Sharybin reviewed 2024-02-21 11:56:12 +01:00

source/blender/compositor/operations/COM_DenoiseOperation.cc

						
				@ -227,0 +217,4 @@

				  filter.set_image("color", input_color);

				  if (!input_albedo->is_a_single_elem()) {

				    filter.set_image("albedo", input_albedo);

				    if (!input_normal->is_a_single_elem()) {

Sergey Sharybin commented

2024-02-21 11:53:16 +01:00

Owner

I believe there is no coupling between albedo and normal guiding passes for OIDN. It is only in the Cycles interface we do not allow all permutations because they are not supported by OptiX, and having different permutations depending on the denoiser used is not that easy. So it probably should be

  if (!input_albedo->is_a_single_elem()) {
    filter.set_image("albedo", input_albedo);
  }
  if (!input_normal->is_a_single_elem()) {
    filter.set_image("normal", input_normal);
  }

I believe there is no coupling between albedo and normal guiding passes for OIDN. It is only in the Cycles interface we do not allow all permutations because they are not supported by OptiX, and having different permutations depending on the denoiser used is not that easy. So it probably should be ``` if (!input_albedo->is_a_single_elem()) { filter.set_image("albedo", input_albedo); } if (!input_normal->is_a_single_elem()) { filter.set_image("normal", input_normal); } ```

Omar Emara commented

2024-02-21 12:22:00 +01:00

Author

Member

@Sergey Are you sure? Because if only normal is added, with logging enabled, I get Error: unsupported combination of input features. There is also this issue, which explains the limitation:

https://github.com/OpenImageDenoise/oidn/issues/9

@Sergey Are you sure? Because if only normal is added, with logging enabled, I get `Error: unsupported combination of input features`. There is also this issue, which explains the limitation: https://github.com/OpenImageDenoise/oidn/issues/9

Sergey Sharybin commented

2024-02-21 12:29:16 +01:00

Owner

Is from memories working on the denoiser for Cycles-X, but maybe something was just silently ignoring unsupported configuration. Anyway, if Attila confirms it is unsupported combination we probably should make adjustment to Cycles code to guard against usupported configuration in the code side.

Is from memories working on the denoiser for Cycles-X, but maybe something was just silently ignoring unsupported configuration. Anyway, if Attila confirms it is unsupported combination we probably should make adjustment to Cycles code to guard against usupported configuration in the code side.

Sergey Sharybin approved these changes 2024-02-21 12:29:27 +01:00

Omar Emara merged commit 44778bdf41 into main

2024-02-21 13:23:14 +01:00

Omar Emara referenced this issue from a commit

2024-02-21 13:23:15 +01:00

Compositor: Unify Denoise node between CPU and GPU

Omar Emara deleted branch unify-denoise-gpu-cpu

2024-02-21 13:23:16 +01:00

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#118553

No description provided.