open-project-1 icon indicating copy to clipboard operation
open-project-1 copied to clipboard

Add additional light shadow support to Toon lighting model

Open hybridherbst opened this issue 4 years ago • 3 comments

The current custom lighting implementation didn't support additional light shadows. They are turned off on the UniversalRenderPipelineAsset, but since the existing ShaderGraph implementation did contain the relevant keywords I assume they are supposed to work.

This PR adds support for additional light shadows by calling the right GetAdditionalLight overload that takes shadowCoords. There's probably an edge case missing for lightmapped objects that are supposed to also receive mixed lighting with shadows from additional lights, but I think for now it's nice to have shadows when wanted :)

Before / After: 20210608-014912_Unity 20210608-014803_Unity

hybridherbst avatar Jun 07 '21 23:06 hybridherbst

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jun 07 '21 23:06 CLAassistant

Not sure if needed. We generally don't have spotlights, I don't see an use for them in this game, and the point lights (torches, the phoenix chick, etc.) don't cast a shadow. As you know, point light shadows only came in more recent URP versions.

ciro-unity avatar Jun 14 '21 11:06 ciro-unity

I saw that the ShaderGraph shaders defined the ADDITIONAL_LIGHT_SHADOWS keyword so it seems it was used at some point (or someone tried - as it was probably never working so far).

Not sure either if it's needed here, but it doesn't hurt either (if additional light shadows are off the code isn't used at all, conditional compilation for the win) – people are gonna look at Open Project as a reference (many people wait for a proper toon shader for URP) so I think a bit more complete feature support is nice to have.

hybridherbst avatar Jun 14 '21 11:06 hybridherbst