[Fogbugz 1352407] Fixing TAAU and DLSS resolutions on custom post process #5329
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 SRP Core 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. |
kecho
requested review from
JulienIgnace-Unity,
FrancescoC-unity,
alelievr and
sebastienlagarde
kecho
force-pushed
the
HDRP/CustomPostProcessFixWithDLSSAndTAAU
branch
from
4c6fb99
to
38c8956
Compare
| newProps.rtHandleScale = passData.postProcessScales; | ||
| newProps.currentRenderTargetSize = passData.postProcessViewportSize; | ||
| newProps.previousRenderTargetSize = passData.postProcessViewportSize; | ||
| newProps.currentViewportSize = passData.postProcessViewportSize; |
There was a problem hiding this comment.
Uhm waiting for @alelievr as I don't know, but can custom PP have source that is not full size? If so, will override src RTHandle stuff create problems?
There was a problem hiding this comment.
Also can user use anything but the HD Blit that creates problems? (Not that I have a solution, but worth knowing a bit more the problem space)
There was a problem hiding this comment.
yes. The documentation we explain that the color must use m_PostProcessSize. Apart from that, the user could just use the rtHandleProperties now blindly and anything would work.
There was a problem hiding this comment.
@FrancescoC-unity Custom post-processing in shaders can sample the normal or depth buffer which is not full size (but it's not possible to access these buffers in C#).
Also, with the latest changes, custom post-processes are now using cmd.Blit() instead of HDUtils.DrawFullscreen which has different behavior.
kecho
force-pushed
the
HDRP/CustomPostProcessFixWithDLSSAndTAAU
branch
from
38c8956
to
f38a966
Compare
|
Uhm @alelievr if cmd.Blit() is used, won't this change do virtually nothing? |
|
@FrancescoC-unity technically yes |
…stom state * Adding missing flag for dynamic res target * Formatting * Documentation * More docs * changelog * Formatting
…ng in a GCAlloc call
kecho
force-pushed
the
HDRP/CustomPostProcessFixWithDLSSAndTAAU
branch
from
1f8c1ab
to
6f2a21f
Compare
sebastienlagarde
deleted the
HDRP/CustomPostProcessFixWithDLSSAndTAAU
branch
|
@kecho I'm not sure why but this PR doesn't fix a custom PP using HDUtils.DrawFullScreen at AfterOpaqueAndSky injection point, in that specific case source.rtHandleProperties.rtHandleScale seems to be wrong. Other injection points are fine and I made sure to call ClampAndScaleUVForBilinearPostProcessTexture for correct UVs. |
Purpose of this PR
Fixing TAAU and DLSS resolution by passing a custom resolution to custom post processes.
The problem is that RTHandle system has a monolithic rtHandleProperties. Thus, we cant just give a texture its specific resoltuion. For this to work with the blitter and the ecosystem, I am introducing a small API that allows to have a custom handle properties into the desired target.
Fogbugz: https://fogbugz.unity3d.com/f/cases/1352407/
Testing status
Tested with custom post process from example dx11, also hardware DRS on dx12.
Comments to reviewers
I tried many ways of fixing it, all of them incurring into heavy complexity on HDRP. The real solution should be implementing proper support of multi resolutions in RTHandle.