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 support for soft shadows to the GPU lightmapper #62054

Merged

Conversation

techiepriyansh
Copy link
Contributor

master port of #50184.
Closes #52199.

Results

Directional Light
  • w/o soft shadows
    before_soft_shadows
  • w/ soft shadows (size = 10deg)
    after_soft_shadows_10deg
Spot Light
  • w/o soft shadows
    without_soft_shadows_spotlight

  • w/ soft shadows (size = 1m)
    after_soft_shadows_spotlight_1m

Omni Light
  • w/o soft shadows
    before_soft_shadows_omni

  • w/ soft shadows (size = 1m)
    after_soft_shadows_omni

@techiepriyansh techiepriyansh requested review from a team as code owners June 15, 2022 00:23
@jcostello
Copy link
Contributor

Nice result

image

@clayjohn clayjohn requested a review from JFonS June 15, 2022 04:25
@Calinou
Copy link
Member

Calinou commented Jun 15, 2022

I tested this PR locally, it's looking good 🙂

The PCSS shadow blur between real-time and baked shadows seems to be a close match in the default configuration.

The only issue I could notice is that the Light3D's Shadow Blur property isn't taken into account to increase/decrease the shadow blur (even after baking lightmaps again). Also, constant shadow blur (when size = 0.0 and blur > 0.0) isn't supported, but I suppose this is outside the scope of this PR.

I suggest multiplying the shadow blur in the lightmapper by the Shadow Blur property. This would allow for a better matching between real-time and baked shadows when Shadow Blur is modified from its default value (1.0).

Testing project: test_lightmap_shadow_blur.zip

Shadow Blur = 1.0

2022-06-15_18 06 55

Shadow Blur = 2.0

2022-06-15_18 10 40

@techiepriyansh
Copy link
Contributor Author

Updated the PR to respect the Shadow Blur property. Needs testing.

@jcostello
Copy link
Contributor

Angular Distance and blur taken into consideration. Althought it might have the same result because at edges the shadow is not blur at all. So distance and blur affect each other as a multiply. If Angular Distance is 0, blur has no effect.

Angular Distance 2, Blur 4

image

Angular Distance 0, Blur 4

image

@clayjohn
Copy link
Member

Angular Distance and blur taken into consideration. Althought it might have the same result because at edges the shadow is not blur at all. So distance and blur affect each other as a multiply. If Angular Distance is 0, blur has no effect.

In the regular scene shader, we still take the soft_shadow_scale into account to scale PCF shadows
For Omnilight when light size == 0

} else {
vec4 uv_rect = base_uv_rect;
vec3 shadow_sample = normalize(shadow_dir + normal_bias);
if (shadow_sample.z >= 0.0) {
uv_rect.xy += flip_offset;
flip_offset *= -1.0;
}
shadow_sample.z = 1.0 + abs(shadow_sample.z);
vec2 pos = shadow_sample.xy / shadow_sample.z;
float depth = shadow_len - omni_lights.data[idx].shadow_bias;
depth *= omni_lights.data[idx].inv_radius;
shadow = sample_omni_pcf_shadow(shadow_atlas, omni_lights.data[idx].soft_shadow_scale / shadow_sample.z, pos, uv_rect, flip_offset, depth);
}

For directional light when angular diameter == 0

shadow = sample_directional_pcf_shadow(directional_shadow_atlas, scene_data.directional_shadow_pixel_size * directional_lights.data[i].soft_shadow_scale * blur_factor, pssm_coord);

Overall, I'm not sure we need to have blur when Angular Distance is 0. Blur is present in the default PCF path for shadows in order to hide pixelation artifacts from using low resolution shadowmaps. For lightmaps, we can get proper hard shadows that look really nice, so the blurring is less beneficial

@Calinou
Copy link
Member

Calinou commented Jun 21, 2022

Blur is present in the default PCF path for shadows in order to hide pixelation artifacts from using low resolution shadowmaps. For lightmaps, we can get proper hard shadows that look really nice, so the blurring is less beneficial

This is only true if bicubic sampling is used, which is not the case in master yet. You can't always use high-resolution lightmaps for storage and bake time reasons, so it's important that low-resolution lightmaps look good too.

That said, I agree that contact hardening (PCSS) is preferable to constant blur when using fully baked lights.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for your hard work

@akien-mga akien-mga merged commit 6023822 into godotengine:master Jun 24, 2022
@akien-mga
Copy link
Member

Thanks!

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.

Vulkan: LightmapGI soft shadows not reimplemented yet (unlike BakedLightmap in 3.x)
7 participants