RenderKit / oidn

Intel® Open Image Denoise library
https://www.openimagedenoise.org/
Apache License 2.0
1.81k stars 164 forks source link

Add proper alpha channel support #140

Open michaelnikelsky opened 2 years ago

michaelnikelsky commented 2 years ago

At the moment only RGB channels are denoised, which means for a full result one has to create an rgb image from the alpha channel, denoise it and copy the result back to the final image. Considering that OpenImageDenoise is already quite slow compared to other denoiser this doubles the amount of computation time making OpenImageDenoise basically useless for realtime (30fps+) applications.

atafra commented 2 years ago

The upcoming release of Open Image Denoise will add GPU support so it will be suitable for real-time applications even if the alpha channel would be denoised separately. Do you denoise the alpha channel separately regardless of the denoiser, or do you need a special code path only for Open Image Denoise? If Open Image Denoise would support denoising the alpha channel together with the RGB image, what would be your expectations regarding performance?

michaelnikelsky commented 2 years ago

For interactive rendering we are using the optix denoiser which just has an option to switch alpha denoising on or off with minimal performance impact (less than 10% if I remember correctly). We have both, the optix denoiser and the open image denoiser encapsulated in their own dll with a common interface so from the outside there is no special handling. Inside our open image denoiser wrapper we are calling the denoiser twice which also means we have do duplicate the alpha channel to a RGB image first, denoiser and then decomposite it again which additionally impacts performance. So basically what I would expect is at most a 25% performance impact due to the additional channel.

atafra commented 2 years ago

There are multiple modes for denoising the alpha channel in OptiX. Are you using OPTIX_DENOISER_ALPHA_MODE_ALPHA_AS_AOV? Thanks!

michaelnikelsky commented 2 years ago

We are using OPTIX_DENOISER_ALPHA_MODE_FULL_DENOISE_PASS since the OPTIX_DENOISER_ALPHA_MODE_ALPHA_AS_AOV mode is buggy and creates transparent areas in solid alpha cases. We are also still using the older HDR denoiser instead of the AOV denoiser since it delivers better quality (still worse than open image denoise though).

atafra commented 2 years ago

This is quite surprising. I just looked up the latest OptiX documentation for OPTIX_DENOISER_ALPHA_MODE_FULL_DENOISE_PASS:

A new denoising mode that is useful where the alpha channel has a substantially different noise profile than the RGB image. If artifacts appear, such as extra halo areas around alpha, this mode may yield better denoised alpha channels. However, this will execute a separate inference pass on alpha, meaning that denoise execution will take twice as long.

Which sounds like what you're doing manually with Open Image Denoise. So I'm wondering how does this have only 10% performance impact for you, instead of 2x? Perhaps the OptiX documentation is out-of-date?

michaelnikelsky commented 2 years ago

Mhhh..interesting. I will have to retest it, not sure if the measurement I did included all the additional setup as well. Will keep you posted.

atafra commented 2 years ago

Thanks, it would be useful to know! Also, could you perhaps share a few representative noisy RGB + alpha images that we could internally use for testing? Feel free to contact me via email if you would prefer to share these privately.

michaelnikelsky commented 2 years ago

Ok, the timing was indeed my mistake. I measured the full denoiser run, including all the setup calls and so on. Additionally I figured out that the HDR-denoiser we use doesn´t support the OPTIX_DENOISER_ALPHA_MODE_ALPHA_AS_AOV. In numbers for a FullHD image: HDR denoiser took 10ms (it always does the Full Denoise Pass for alpha), simple alpha copy took 5ms. AOV denoiser took 12.6ms with Full Alpha Denoise and 7.5ms with the Alpha as AOV denoise and 6ms with simply copying the alpha over.

So if it would just be possible to handle the alpha denoising internally so we won´t have to manually copy the image to an extra buffer and copy the result back to the original image it would be ok if there would be 2 runs internally, especially if the GPU implementation will be a lot faster than the CPU implementation.

About testing images: I will check it internally and contact you per mail. Thanks for your help.

atafra commented 2 years ago

Thanks for the info! We will add alpha channel denoising support to the upcoming release: there will be an option to either denoise the alpha channel or copy it without changes. This will be initially implemented as an additional denoising step, and in future versions we may switch to a faster implementation.

Regarding your current workflow with Open Image Denoise: do you denoise the alpha channel with the hdr and srgb filter parameters both set to false?

michaelnikelsky commented 2 years ago

Thanks, that sounds good. At the moment yes, we have both hdr and srgb set to false. I tried switching hdr to on and didn´t see any difference though. Our color buffer is always denoised in hdr mode though.

atafra commented 2 years ago

Thanks for confirming. The alpha channel should be denoised in LDR mode (HDR off), so what you're currently doing sounds good.

pberto commented 2 years ago

Hello @atafra, regarding this issue. I just sent you privately 3x correlated exr images (beauty with alpha channel, normal and albedo). Further details on the e-mail.

atafra commented 1 year ago

v2.1.0 just introduced grayscale/alpha denoising support but it's a bit different from how OptiX does it. You have to set up a separate filter just for the alpha channel. Using one filter for RGB and another for alpha (with the right buffer offsets/strides) it's possible to denoise both without any extra copying.

michaelnikelsky commented 1 year ago

That sounds great, I will test it out as soon as possible. Thanks for implementing this.

msercheli commented 11 months ago

Hi,

I am trying denoise the RGB and alpha from an EXR using version v2.1.0, but I am getting an error. My oidn build has oiio available. I am using (just for test) a simplified CLI implementation, in which I pass the EXR as 'colorFilename' and an output 'outputFilename' pointing to a EXR filepath. No albedo or normal.

Like @atafra mentioned above, in v2.1.0, all I need to do is to run 2 filters with the proper bytePixelStride to process the alpha channel. I am probably doing something wrong. Could you point me to the error?

Here is my output:

Intel(R) Open Image Denoise 2.1.0
  Compiler  : GCC 6.3.1 20170216 (Red Hat 6.3.1-3)
  Build     : Release
  OS        : Linux (64-bit)
  Device    : Intel(R) Core(TM) i9-10980XE CPU @ 3.00GHz
    Type    : CPU
    ISA     : AVX512
  Tasking   : TBB2019.0 TBB_header_interface_11006 TBB_lib_interface_11006
    Threads : 18 (affinitized)

Filter: RT
Quality: high
Inputs: hdr:h3
Output: h3
Memory usage: 2561711424
Image size: 3840x2160
Tile size : 2016x2160
Tile count: 2x1
In-place  : true
Filter: RT
Error: buffer region is out of range
Error: buffer region is out of range
Error: input image not specified
Error: changes to the filter are not committed
Saving output

Here is a simplified CLI code that I am using:

int main(int argc, char* argv[])
{
  std::string colorFilename, outputFilename;

  ArgParser args(argc, argv);
  while (args.hasNext())
  {
    std::string opt = args.getNextOpt();
    if (opt == "i" || opt == "in" || opt == "input")
      colorFilename = args.getNextValue();
    else if (opt == "o" || opt == "out" || opt == "output")
      outputFilename = args.getNextValue();
    else
      throw std::invalid_argument("invalid argument '" + opt + "'");
  }

  // Enable the FTZ and DAZ flags to maximize performance
  _MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON);
  _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_ON);

  // Device
  int verbose = 2;
  DeviceRef device = newDevice(DeviceType::Default);
  device.set("verbose", verbose);
  device.commit();

  // Input and ouput
  std::shared_ptr<ImageBuffer> input, output, color;

  input = color = loadImage(device, colorFilename, false, DataType::Void);
  output = std::make_shared<ImageBuffer>(device, input->getW(), input->getH(), input->getC(), input->getDataType());

  // Filter RGB (outputs to the same 'color' input)
  FilterRef filterRGB = device.newFilter("RT");

  filterRGB.set("hdr", true);
  filterRGB.setImage("color", color->getBuffer(), color->getFormat(), color->getW(), color->getH());
  filterRGB.setImage("output", color->getBuffer(), color->getFormat(), color->getW(), color->getH());

  filterRGB.commit();
  filterRGB.execute();

  // Filter alpha
  size_t bytePixelStride = 3*sizeof(half);
  size_t byteRowStride = color->getW()*bytePixelStride;

  FilterRef filterAlpha = device.newFilter("RT");

  filterAlpha.set("hdr", false);
  filterAlpha.setImage("color", color->getBuffer(), color->getFormat(), color->getW(), color->getH(), bytePixelStride, byteRowStride);
  filterAlpha.setImage("output", output->getBuffer(), output->getFormat(), output->getW(), output->getH(), bytePixelStride, byteRowStride);

  filterAlpha.commit();
  filterAlpha.execute();

  // Save output image
  std::cout << "Saving output" << std::endl;
  saveImage(outputFilename, *output, false);

  return 0;
}
atafra commented 11 months ago

Are you sure that input contains all 4 channels? It seems you're using OIDN's example framework to load images, which will only load up to 3 channels.

msercheli commented 11 months ago

Hi,

Yes, my input has all 4 channels and yes, I wrote that implementation based on the app oidnDenoise.cpp. How should I load the input image if not using filter.setImage(), to guarantee all 4 channels are loaded?

Thanks

atafra commented 11 months ago

You would need to modify the example app to load all 4 channels. The issue isn't with filter.setImage(). I think the error reported by OIDN is correct.

atafra commented 11 months ago

Yes, my input has all 4 channels

I meant what is actually loaded in the input variable, not in the file. Did you check that?

msercheli commented 11 months ago

Yes, after checking that, the format of the input was Half3. I modified const int numChannels = std::min(spec.nchannels, 3); at apps/utils/image_io.cpp to it loads spec.nchannels.

New output:

Filter: RT
Quality: high
Inputs: hdr:h4
Output: h4
Memory usage: 2578300224
Image size: 3840x2160
Tile size : 2016x2160
Tile count: 2x1
In-place  : true
Error: unsupported number of channels for image accessor
Filter: RT
Error: buffer region is out of range
Error: buffer region is out of range
Error: input image not specified
Error: changes to the filter are not committed
Saving output
atafra commented 11 months ago

It seems you're using Half4 for both the RGB input and output. OIDN doesn't support 4 channel input/outputs, that's why there's a need for separate RGB and alpha filters.

msercheli commented 11 months ago

You mentioned that You would need to modify the example app to load all 4 channels, but if OIDN doesn't support 4 channels, how I would go about it? Can I still load into the input all 4 channels but have the output only 3 channels? If so, how to preserve the alpha channel in the output?

Sorry for so many questions.

atafra commented 11 months ago

You already shared the code how to do it, by denoising RGB and alpha separately. The RGB input should have 3 channels but the verbose output says that it's 4 channels, which is wrong.

msercheli commented 11 months ago

Ok. Let me try more here. I will give you some feedback soon. Thanks for the pacience.