[HDRP][DXR] Fix for punctual and directional ray traced shadow broken temporal denoiser #5417
Conversation
|
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. HDRP Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
remi-chapelain
changed the title
remi-chapelain
changed the title
| ctx.cmd.SetComputeTextureParam(data.temporalFilterCS, data.outputHistoryKernel, HDShaderIDs._IntermediateDenoiseOutputTexture, data.intermediateSignalOutput); | ||
| ctx.cmd.SetComputeTextureParam(data.temporalFilterCS, data.outputHistoryKernel, HDShaderIDs._DenoiseOutputArrayTextureRW, data.intermediateSignalOutput); | ||
| ctx.cmd.DispatchCompute(data.temporalFilterCS, data.outputHistoryKernel, numTilesX, numTilesY, data.viewCount); | ||
| CoreUtils.SetKeyword(ctx.cmd, "FULL_RESOLUTION_INPUT", true); |
There was a problem hiding this comment.
note: this code mean FULL_RESOLUTION_INPUT will always be true, even if we take another path later, is it what we want?
There was a problem hiding this comment.
I do not fully grasp the reason why this keyword is set to true after dispatch command. But, If I understand correctly, that's the default behavior (same is happening in the denoise() function above in the same file). Default is full res, unless we explicitly set to false before with the function parameter. The parameter (data.fullResolution) is only here for eventual future use but we always denoise ray traced shadows at full resolution anyway.
|
currently test are green, does it mean that we miss test that cover this cases? should we add one? |
There are tests that cover this and what worries me is that locally all the tests using denoiser for punctual and directional fail (on master) but they seem to pass on Yamato for some reasons... :/
Example for 702_DirectionalShadowFiltering |
remi-chapelain
changed the title
|
Incorrect, i'll change the code |
|
The issue will only be triggered no RTGI, SSGI or RTAO is rendered before the the first shadow. |
…ows temporal denoiser (case 1360132).
anisunity
force-pushed
the
HDRP/fix-1360132
branch
from
f5bf56b to
6d8223a
Compare
|
Fixed |
…n temporal denoiser #5417
5 participants
Purpose of this PR
Fixes this issue.
This recently merged PR added the possibility to use half resolution denoiser for SSGI.
In HDTemporalFilter.cs, in the Denoiser function the new parameter fullResolution was set via the filterParams.
However, the full resolution parameter was not set in the temporal denoiser function leading to rendering the temporal part in half resolution thus breaking Punctual and Directional ray traced shadows denoiser. (Area light were not impacted)
Before fix
After fix
Testing status
Comments to reviewers
Fix seems "trivial" to me but since I'm not familiar with the codebase, I'd love to have some feedback if the fix makes sense to someone who actually knows how it has been written. Might be a more clean / elegant / robust fix.