feat: support shadows for additional lights#756
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds shadow support for additional lights in Unity's Toon Shader for Universal Render Pipeline. Previously, additional lights used a constant value ignoring actual shadow data, while now they properly process shadow attenuation through a new centralized function.
Changes:
- Introduced
TweakShadowfunction to centralize shadow attenuation processing - Modified
GetAdditionalUtsLightto retrieve shadow data from additional lights by passing shadowMask parameter - Replaced inline shadow calculations with calls to the new
TweakShadowfunction across main and additional lights
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| UniversalToonBodyShadingGradeMap.hlsl | Refactored shadow processing for shading grade map technique to use TweakShadow function; updated parameter names from tweakShadows to shadowAtt |
| UniversalToonBodyDoubleShadeWithFeather.hlsl | Refactored shadow processing for double shade technique to use TweakShadow function; removed leading blank line and added section separator |
| UniversalToonBody.hlsl | Added TweakShadow function and modified GetAdditionalUtsLight to support shadow mask for proper shadow retrieval |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| float halfLambert = 0.5 * dot(lerp(vertexNormalWS, perturbedNormalWS, _Is_NormalMapToBase), lightDirection) + 0.5; | ||
|
|
||
| float Set_ShadingGrade = saturate(sgMapLevel) * lerp(halfLambert,(halfLambert * saturate(tweakShadows)), _Set_SystemShadowsToBase); | ||
| float Set_ShadingGrade = saturate(sgMapLevel) * lerp(halfLambert,halfLambert * saturate(shadowAtt), _Set_SystemShadowsToBase); |
There was a problem hiding this comment.
The second argument to lerp is missing parentheses and has no space after the comma. The previous code had (halfLambert * saturate(tweakShadows)) with parentheses for clarity. While the current code is functionally correct due to operator precedence, adding the parentheses and space would improve readability and consistency with the original code.
| float Set_ShadingGrade = saturate(sgMapLevel) * lerp(halfLambert,halfLambert * saturate(shadowAtt), _Set_SystemShadowsToBase); | |
| float Set_ShadingGrade = saturate(sgMapLevel) * lerp(halfLambert, (halfLambert * saturate(shadowAtt)), _Set_SystemShadowsToBase); |
com.unity.toonshader/Runtime/UniversalRP/Shaders/UniversalToonBody.hlsl
Outdated
Show resolved
Hide resolved
sindharta
force-pushed
the
additional-light-shadow
branch
from
d0d08d8 to
f1ad111
Compare
…Body.hlsl Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| inline float TweakShadow(float shadow) { | ||
| return saturate(max((shadow * 0.5) + 0.5 + _Tweak_SystemShadowsLevel, 0.0001)); |
There was a problem hiding this comment.
The minimum clamp value has been changed from 0.0001 (in the original code) to 0.001, which is 10x larger. This may alter shadow appearance. The original code used: (shadowAttenuation * 0.5) + 0.5 + _Tweak_SystemShadowsLevel > 0.001 ? (shadowAttenuation * 0.5) + 0.5 + _Tweak_SystemShadowsLevel : 0.0001. Consider whether 0.001 or 0.0001 is the intended minimum value for consistency with the previous behavior.
| return saturate(max((shadow * 0.5) + 0.5 + _Tweak_SystemShadowsLevel, 0.0001)); | |
| float tweaked = (shadow * 0.5) + 0.5 + _Tweak_SystemShadowsLevel; | |
| return saturate(tweaked > 0.001 ? tweaked : 0.0001); |
No description provided.