Clarify how IN_BASECOLOR_METALNESS is used

There is IN_BASECOLOR_METALNESS input, which is said in the c++ comments to be used if CommonSettings::isBaseColorMetalnessAvailable = true, however, there is no detail on:

which exact denoisers can benefit from it
what requirements to the data there are.
For example, has base color to be in linear or in log space? The sample seems to be writing log without explanation.

It would be nice if the readme mentioned IN_BASECOLOR_METALNESS somewhere with some details on it.

By the way a nitpick: in [NRD] The number of accumulated frames section of the readme you've probably meant to say "bear in mind".

Hi! Thanks. I fixed this in my local version. In the next update (hope this month):

ResourceType will provide more clear separation into NON-NOISY / NOISY inputs

    enum class ResourceType : uint32_t
    {
        //=======================
        // NON-NOISY INPUTS
        //=======================
        ...
        //=======================
        // NOISY INPUTS
        //=======================
        ...
    };

clarified IN_BASECOLOR_METALNESS usage:

        // (Optional) Base color (can be decoupled to diffuse and specular albedo based on metalness) and metalness (RGBA8+)
        // Used only if "CommonSettings::isBaseColorMetalnessAvailable = true". Currently used only by REBLUR (if temporal
        // stabilization pass is available and "stabilizationStrength != 0") to patch MV if specular (virtual) motion prevails
        // on diffuse (surface) motion

By the way a nitpick: in [NRD] The number of accumulated frames section of the readme you've probably meant to say "bear in mind"

In this section I mean to say what I wrote:

[NRD] The number of accumulated frames in the fast history needs to be carefully tuned to avoid introducing significant bias and dirt. Initial integration should be done by setting `maxFastAccumulatedFrameNum` to `maxAccumulatedFrameNum`. Bare in mind the following recommendation:
maxAccumulatedFrameNum > maxFastAccumulatedFrameNum > historyFixFrameNum

Please, elaborate more on how you think it can be improved?

[EDIT] The typo "bare" => "bear" is fixed.

clarified IN_BASECOLOR_METALNESS usage

Thanks, exactly what I was lookig for. So I should look at areas with specular-like motion after REBLUR to see the effect. Any reqirements on the base color (linear/gamma) or it doesn't care?

The typo "bare" => "bear" is fixed.

Yep, just that; the sentence itself is perfectly fine in technical sense.

But come to think of it, the readme never gives details on what "fast history" exaclty is. It mentions fast history in context of RELAX specifically, to signify its reduced lag, but that's it. May be that's common knowledge I am lacking, but what exactly makes fast history "fast"?

[EDIT[ formatting.

So I should look at areas with specular-like motion after REBLUR to see the effect

Yes, if TAA/DLSS/FSR are active.

Any requirements on the base color (linear/gamma) or it doesn't care?

Linear. But since SRV is coming from your side and it might auto perform "sRGB to linear" conversion - this is fine too. Actually, this is a good practice.

Fast history

Thanks. I adjusted the README a bit:

Simplified intro:

- *REBLUR* - recurrent blur based denoiser
- *RELAX* - A-trous based denoiser
- *SIGMA* - shadow-only denoiser

Added more explicit performance section:

Performance on RTX 4080 @ 1440p (native resolution, default denoiser settings):
- `REBLUR_DIFFUSE_SPECULAR` - 2.45 ms
- `RELAX_DIFFUSE_SPECULAR` - 2.90 ms
- `SIGMA_DIFFUSE_SPECULAR` - 0.30 ms

Clarified fast history:

[NRD] Fast history is the input signal, accumulated for a few frames. Fast history helps to minimize
lags in the main history, which is accumulated for more frames. The number of accumulated frames
in the fast history needs to be carefully tuned to avoid introducing significant bias and dirt. Initial
integration should be done with default settings.

Bear in mind the following recommendation:
maxAccumulatedFrameNum > maxFastAccumulatedFrameNum > historyFixFrameNum

Great, thank you for these explanations, they answer my questions.

On topic of documentation: comments for CommonSettings::enableValidation refer to InstanceCreationDesc::allowValidation which is no longer there:

RayTracingDenoiser/Include/NRDSettings.h

Lines 150 to 151 in 067755e

    
           // Enables debug overlay in OUT_VALIDATION, requires "InstanceCreationDesc::allowValidation = true" 
        
           bool enableValidation = false;

RayTracingDenoiser/Include/NRDDescs.h

Lines 441 to 446 in 067755e

    
           struct InstanceCreationDesc 
        
           { 
        
               MemoryAllocatorInterface memoryAllocatorInterface; 
        
               const DenoiserDesc* denoisers; 
        
               uint32_t denoisersNum; 
        
           };

Thanks. I removed:

, requires "InstanceCreationDesc::allowValidation = true"

It's a leftover...

All discussed changes are included in v4.4.0.

	// Enables debug overlay in OUT_VALIDATION, requires "InstanceCreationDesc::allowValidation = true"
	bool enableValidation = false;

	struct InstanceCreationDesc
	{
	MemoryAllocatorInterface memoryAllocatorInterface;
	const DenoiserDesc* denoisers;
	uint32_t denoisersNum;
	};