Skip to content

Conversation

robcupisz
Copy link
Collaborator

@robcupisz robcupisz commented Mar 30, 2021

DRAFT PR - WORK IN PROGRESS

PCSS sampling improvements - instead of sampling in a flat disk pattern, we now use a visibility cone (or: pyramid), with its edges aimed at the edges of the shadow's near plane. The idea is that anything outside of that cone has no business contributing to the shadow of the currently shaded pixel, both in blocker search and final coverage.

Also makes area lights use PCSS when the shadow filtering quality is set to Very High.

Before
pcss-before0
pcss-before1

After
pcss-after0
pcss-after1

@github-actions github-actions bot added the HDRP label Mar 30, 2021
{
// TODO: implement less messily. Also probably shouldn't even rely on a global quality setting, but on a per-light shadow algorithm switch.
#ifdef SHADOW_HIGH // Do PCSS
return EvalShadow_PunctualDepth(sd, tex, s_linear_clamp_compare_sampler, positionSS, positionWS, normalWS, L, L_dist, perspective);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if we do this, we should move the definition of the algorithm used in the HDShadowAlgorithms.hlsl and do same kind of thing that we do for the other algorithm, so it is configurable

#ifdef SHADOW_ULTRA_LOW
#define PUNCTUAL_FILTER_ALGORITHM(sd, posSS, posTC, tex, samp, bias) SampleShadow_PCF_Tent_3x3(sd.isInCachedAtlas ? _CachedShadowAtlasSize.zwxy : _ShadowAtlasSize.zwxy, posTC, tex, samp, bias)
#define DIRECTIONAL_FILTER_ALGORITHM(sd, posSS, posTC, tex, samp, bias) SampleShadow_Gather_PCF(_CascadeShadowAtlasSize.zwxy, posTC, tex, samp, bias)
#elif defined(SHADOW_LOW)
#define PUNCTUAL_FILTER_ALGORITHM(sd, posSS, posTC, tex, samp, bias) SampleShadow_PCF_Tent_3x3(sd.isInCachedAtlas ? _CachedShadowAtlasSize.zwxy : _ShadowAtlasSize.zwxy, posTC, tex, samp, bias)
...
#endif

Copy link
Contributor

@sebastienlagarde sebastienlagarde Mar 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but on a per-light shadow algorithm switch.

That's will be an issue.
For this, either you add new variant (and combination will explode) or you do a dynamic branch (and VGPR will explode for area lights (PCSS used way more VGPR than EVSM).

So for now we will keep it as global.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if we do this, we should move the definition of the algorithm used in the HDShadowAlgorithms.hlsl and do same kind of thing that we do for the other algorithm, so it is configurable

Yup, it just needs a bit of a refactor, since the entire EvalShadow_AreaDepth() needs to reuse what happens inside EvalShadow_PunctualDepth(). In other words it's not sufficient to define the algo for area lights at the same level as PUNCTUAL_FILTER_ALGORITHM

@FrancescoC-unity
Copy link
Contributor

So the sampling improvements is something we do want provided the fitting for the softness is verified again so ping me when ready to be out of draft.

Regarding the PCSS for area light, we should add a new tier; I will do a PR with a new "Very High" setting that will do that for area lights and link the PR here.

@sebastienlagarde
Copy link
Contributor

Hey, any update on this one?

@Passeridae
Copy link

Is it going to be merged soon? Our team really needs this shadow improvement;)

@robcupisz
Copy link
Collaborator Author

Will be done as soon as I'm back from a break! @sebastienlagarde: I'll DM you

@github-actions
Copy link

github-actions bot commented Sep 7, 2021

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.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_trunk

URP
/.yamato%252Fall-urp.yml%2523PR_URP_trunk

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.

@sebastienlagarde
Copy link
Contributor

Landed in trunk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants