SSGI Improvements #1714
SSGI Improvements #1714
Conversation
anisunity
requested review from
sebastienlagarde,
JulienIgnace-Unity and
remi-chapelain
|
It appears that you made a non-draft PR! |
| @@ -199,7 +207,8 @@ static void Denoise(CommandBuffer cmd, SSGIDenoiserParameters parameters, SSGIDe | |||
| if (parameters.historyNeedsClear) | |||
| { | |||
| // clear it to black if this is the first pass to avoid nans | |||
| CoreUtils.SetRenderTarget(cmd, resources.indirectDiffuseHistory, ClearFlag.Color, clearColor: Color.black); | |||
| CoreUtils.SetRenderTarget(cmd, resources.indirectDiffuseHistory0, ClearFlag.Color, clearColor: Color.black); | |||
There was a problem hiding this comment.
For just clearing buffers it'd be better to use CoreUtils.ClearRenderTarget for the sake of clarity.
| @@ -50,24 +50,6 @@ public int raySteps | |||
| [Tooltip("Controls the number of steps used for ray marching.")] | |||
| private ClampedIntParameter m_RaySteps = new ClampedIntParameter(24, 16, 128); | |||
|
|
|||
| /// <summary> | |||
There was a problem hiding this comment.
This will be considered a breaking change. Not sure if we can use the [Obsolete] attribute on volume parameters. Would need to check that. @sebastienlagarde can we consider doing it anyway if it's for version 10.x.x anyway?
There was a problem hiding this comment.
GlobalIllumination is officially in preview, so i think we can break it
There was a problem hiding this comment.
yes, SSGI was in preview up to now (and have been added in 10.x anyway), btw once we merge this PR we should remove preview from SSGI UI.
anisunity
force-pushed
the
HDRP/SSGI_Improvement
branch
from
2549bd2
to
613980b
Compare
| _IndirectDiffuseTexture1RW[COORD_TEXTURE2D_X(currentCoord)] = float4(outCoCg, invalid ? 0.0 : 1.0, length(motionVectorNDC * 10000.0f)); | ||
| } | ||
|
|
||
| void ConvertYCoCgToRGBUtil(float4 inYSH, float2 inCoCg, float3 inNormal, out float3 outColor) |
There was a problem hiding this comment.
This function should be in color.hlsl (and I guess it already exist there)
There was a problem hiding this comment.
can be done in another PR
|
@anisunity can you merge, move the ycocr convertion funciton to color.hlsls and update the documentation of SSGI? thanks. |
There was a problem hiding this comment.
Looks good to me. No issues found.
Validating that no ghosting issues appear anymore.
PR:
Staging:
SSGI filtering looks better than what was before. Attaching comparisons.
No SSGI:
PR SSGI High Quality:
HDRP staging high quality:
RTGI remains unaffected.
PR performance mode high quality:
HDRP/staging perf mode high quality:
|
@anisunity can you merge latest HDRP/Staging and I will merge this PR (and maybe re run just the DXR test (not the other as they are red on staging). thanks! |
|
Doing it right now |
anisunity
force-pushed
the
HDRP/SSGI_Improvement
branch
from
613980b
to
64ddc4d
Compare
|
Done, had to update a screenshot. Would be great to merge this one as well #1845 |
anisunity
force-pushed
the
HDRP/SSGI_Improvement
branch
from
64ddc4d
to
b1caf12
Compare
This reverts commit 7ac1d1f.
The PR is done on top of this PR: #1697
This PR improves the SSGI filtering code and fixes an issue with the usage of RTGI (bleding was wrong). This PR also removes ghosting issues when changing from full to half res for both effects.
Testing status
The DXR tests were green with and without render graph.