Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PCF25 shadow filtering option in the Compatibility rendering method #92090

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented May 18, 2024

This is a high-quality shadow filter which results in smooth, stable shadows even for dynamic objects.

Thanks to early bailing, PCF25 is faster than the old PCF13 implementation while looking better. It's also not that much slower compared to the new PCF13 implementation.

PCF25 is used when the soft shadow quality setting is set to Ultra.

Preview

Hard Soft Low (PCF5) Soft High (PCF13) Soft Ultra (PCF25)
Screenshot_20240517_142333_hard Screenshot_20240517_142328_pcf5 Screenshot_20240517_142323_pcf13 Screenshot_20240517_142312_pcf25

This is a high-quality shadow filter which results in smooth,
stable shadows even for dynamic objects.

Thanks to early bailing, PCF25 is faster than the old PCF13 implementation
while looking better. It's also not that much slower compared to the
new PCF13 implementation.

PCF25 is used when the soft shadow quality setting is set to Ultra.
@mrjustaguy
Copy link
Contributor

A question, regarding early bail, doesn't it make sense to sample in the x pattern and not + pattern?

@mrjustaguy
Copy link
Contributor

mrjustaguy commented May 29, 2024

Eh, Ok Early Bail causes artifacts on 25, even on 13 just not as often...

These Blocks of sudden shadow on PCF25 are caused by Early Bail
image

I think we should have multiple different versions of this:

  1. Hard - No PCF
  2. Low - PCF5
  3. Medium - PCF13 + early bail
  4. High - PCF13 no bail
  5. Ultra - PCF25 no bail (no point in providing version with bail if it breaks with any shadows remotely more complex like shown above with the fingers)

@lawnjelly
Copy link
Member

Or have early bail as a separate option (e.g. off, medium, high)?

Maybe if we rotated e.g. a disc, the chance of having offensive artifacts from early bail might be reduced.

@mrjustaguy
Copy link
Contributor

Maybe the pattern change from + to x would help, which i asked about earlier..
But yea an on-off option for early bail would be good, however that'd have to be like fully on or off based on how it's working right now.

@Calinou
Copy link
Member Author

Calinou commented May 31, 2024

These Blocks of sudden shadow on PCF25 are caused by Early Bail

Could you upload a minimal reproduction project? It might be possible to tweak the early bailing pattern to reduce artifacts.

I wouldn't remove early bailing entirely though (or provide an option for what is essentially an optimization), as the performance gain is very significant for something like PCF25. The Compatbility rendering method targets low-end hardware after all, so PCF25 needs to be somewhat usable on mobile and recent integrated graphics.

@mrjustaguy
Copy link
Contributor

I don't have the project where I took this, however the basic creation steps are simple, just get something that has high frequency gaps in shadows, like say an Alpha Hash transparency casting shadows, that just wrecks things worse than the above example.

I'm also afraid that this is just a fundamental flaw of Early Bail, and anything that creates small, pixel or so wide gaps between shadows causes the artifact, and just does so much more easily with PCF25 which is likely why it wasn't noticed and reported before for PCF13, due to a lack of any more complex shadowing scenarios being present for them to show up.
This makes it only suitable to deal with Shadows from buildings, cars and the like that Don't have High Frequency Shadow gaps, and a Nightmare for Trees, Hair, and even just more complex geometry such as humans

@clayjohn clayjohn self-requested a review May 31, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants