[DONOTMERGE] Key Optimizations Applied: #680
Conversation
- ✅ Texture sampling cache (eliminates 4N texture fetches per additional light) - ✅ Pre-calculated expensive math operations (exp2/pow) - ✅ Hardware-optimized intrinsics (length() instead of manual sqrt) - ✅ Branchless mirror handling (3 branches eliminated) - ✅ Cached blended normals (6-9 LERP operations saved) - ✅ Pre-calculated UV transforms (20-30 MAD operations saved) [skip-ci]
There was a problem hiding this comment.
Pull request overview
This PR optimizes shader performance in Unity's Toon Shader by eliminating redundant computations and leveraging hardware intrinsics. The changes focus on caching calculated values that are reused across multiple light passes, converting branching operations to branchless alternatives, and replacing manual calculations with optimized built-in functions.
Key Changes:
- Pre-calculated expensive mathematical operations (exp2/pow) and UV transforms to avoid redundant computation in per-light loops
- Cached texture samples and blended normals that are reused multiple times
- Replaced branching mirror handling with branchless ternary operations and manual sqrt calculations with hardware-optimized length() intrinsic
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| UniversalToonBodyShadingGradeMap.hlsl | Added caching for blended normals, UV transforms, texture samples, and expensive math operations; replaced branching mirror logic with branchless alternatives |
| UniversalToonBodyDoubleShadeWithFeather.hlsl | Applied same optimization patterns as ShadingGradeMap including cached calculations, pre-sampled textures, and branchless mirror handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| float mirrorSign = _sign_Mirror < 0 ? -1.0 : 1.0; | ||
| _Right_Axis *= mirrorSign; | ||
| _Rotate_MatCapUV *= mirrorSign; |
There was a problem hiding this comment.
The variable name 'mirrorSign' is ambiguous. Consider renaming it to 'mirrorMultiplier' or 'mirrorFlipSign' to better convey that it's used as a multiplication factor for mirroring.
| float mirrorSign = _sign_Mirror < 0 ? -1.0 : 1.0; | |
| _Right_Axis *= mirrorSign; | |
| _Rotate_MatCapUV *= mirrorSign; | |
| float mirrorMultiplier = _sign_Mirror < 0 ? -1.0 : 1.0; | |
| _Right_Axis *= mirrorMultiplier; | |
| _Rotate_MatCapUV *= mirrorMultiplier; |
| float mirrorSign = _sign_Mirror < 0 ? -1.0 : 1.0; | ||
| _Right_Axis *= mirrorSign; | ||
| _Rotate_MatCapUV *= mirrorSign; |
There was a problem hiding this comment.
The variable name 'mirrorSign' is ambiguous. Consider renaming it to 'mirrorMultiplier' or 'mirrorFlipSign' to better convey that it's used as a multiplication factor for mirroring.
| float mirrorSign = _sign_Mirror < 0 ? -1.0 : 1.0; | |
| _Right_Axis *= mirrorSign; | |
| _Rotate_MatCapUV *= mirrorSign; | |
| float mirrorMultiplier = _sign_Mirror < 0 ? -1.0 : 1.0; | |
| _Right_Axis *= mirrorMultiplier; | |
| _Rotate_MatCapUV *= mirrorMultiplier; |
|
|
||
| float4 _HighColor_Tex_var = tex2D(_HighColor_Tex, TRANSFORM_TEX(Set_UV0, _HighColor_Tex)); | ||
| (1.0 - step(_Specular_var, highColorPowThreshold)), | ||
| pow(_Specular_var, highColorExp), _Is_SpecularToHighColor)); |
There was a problem hiding this comment.
Missing abs() around _Specular_var in pow() call. The original code used pow(abs(_Specular_var), ...) but this optimization removed the abs(). Negative values passed to pow() can cause undefined behavior in shader code.
| pow(_Specular_var, highColorExp), _Is_SpecularToHighColor)); | |
| pow(abs(_Specular_var), highColorExp), _Is_SpecularToHighColor)); |
[skip-ci]