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

[3.x] Add a per-light shadow blur property #60243

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Apr 14, 2022

Follow-up to #60186 (can be merged independently, I'll rebase as needed).

This backports a feature already available in the master branch: per-light shadow blur adjustments.

This is available for all light types in both GLES3 and GLES2. It only has an effect when the shadow filter is PCF5 or PCF13.

From my testing, performance is identical compared to before in both GLES3 and GLES2.

For DirectionalLight shadows, there is an issue in GLES2 when the shadow filter mode is PCF13 and the shadow blur is not equal to 1.0. The shadow will flicker as the camera rotates and moves, I don't know why. This does not occur with other shadow filter modes, other light types or in GLES3.

Testing project: test_shadow_b_3.x_1.zip

Preview

The shadow filter mode is PCF13 on all screenshots. The Blue OmniLight had its shadow blur increased to 8.0.

GLES3

Before After
2022-04-14_18 07 07 2022-04-14_18 06 51

GLES2

Before After
2022-04-14_18 05 56 2022-04-14_18 05 41

This is available for all light types in both GLES3 and GLES2.
It only has an effect when the shadow filter is PCF5 or PCF13.
@clayjohn
Copy link
Member

I'm not sure about this one. PCF isn't really designed for a variable width shadow blur. The texture sample locations are chosen specifically for the algorithm. By increasing or decreasing the blur, you end up oversampling or undersampling certain pixels which can lead to artifacts.

Overall, the implementation seems fine. But there needs to be significant user demand to merge this.

@eon-s
Copy link
Contributor

eon-s commented May 24, 2022

@clayjohn I don't know if users are aware that this was a possible to do, to demand for it, this looks optional and can improve the looks with little effort.

What can be done to see if users want this? open a discussion?

@clayjohn
Copy link
Member

What can be done to see if users want this? open a discussion?

That is certainly a good option!

To clarify, I am not suggesting that users need to come here and say that this PR is demanded. I just haven't seen demand for uniformly wider shadow penumbra in 3.x. I have seen people ask for PCSS style shadows (blur increases as distance increases), but not uniform blur (as is implemented in this PR).

We added uniform blur in 4.0 as a way to allow users to hide some of the artifacts that come from using PCSS. But in 3.x I'm not sure it adds much value. As you can see, adding blur makes the shadows become blocky and aliased, this is becuase any amount of blur is going to cause undersampling of the shadow map which introduces aliasing.

@Calinou
Copy link
Member Author

Calinou commented May 24, 2022

I have seen people ask for PCSS style shadows (blur increases as distance increases), but not uniform blur (as is implemented in this PR).

I've seen that DPCF (depth-based PCF) could be used to implement a cheaper form of contact-hardening shadows. It could also help make biasing issues less noticeable (since lower blur will tend to do that). It might be worth looking into for 3.x.

DPCF is mentioned on slides 59 and after in this presentation (with a code sample on slide 74).

@akien-mga akien-mga modified the milestones: 3.5, 3.x Jul 2, 2022
@Calinou Calinou changed the title Add a per-light shadow blur property [3.x] Add a per-light shadow blur property Jun 27, 2023
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.

4 participants