Improved shadow fade #3089
Improved shadow fade #3089
Conversation
…anging shadow fade to use last border property for calculating shadow fade distance instead of using hard coded value. Improving shadow fade cliping for main light. Adding ShadowCascadeGUI class into renderpipeline core.
…er functionality. Adding focus keyboard functionality with handles.
… height it looks a bit better this way.
…nside internal class no deprecation is needed.
…e calculation to use capsule shape instead of sphere shape.
| static float[] splitTwo = new float[2]; | ||
| static float[] splitThree = new float[3]; | ||
| static int splitCount; | ||
| public static void DrawCascadeSplitGUI<T>(ref SerializedProperty shadowCascadeSplit, float distance, int cascadeCount, Unit unit) |
There was a problem hiding this comment.
As this is inside internal class, I am removing it
| static partial class ShadowCascadeSplitGUI | ||
| { | ||
| [Obsolete("This is obsolete, please use HandleCascadeSliderGUI(ref float[] normalizedCascadePartitions, float distance, EditorUtils.Unit unit) instead.", false)] | ||
| public static void HandleCascadeSliderGUI(ref float[] normalizedCascadePartitions) |
There was a problem hiding this comment.
As this is inside internal class, I am removing it
| @@ -15,207 +15,5 @@ namespace UnityEditor.Rendering.Universal | |||
| { | |||
| static partial class EditorUtils | |||
| { | |||
| [Obsolete("This is obsolete, please use DrawCascadeSplitGUI<T>(ref SerializedProperty shadowCascadeSplit, float distance, int cascadeCount, Unit unit) instead.", false)] | |||
| public static void DrawCascadeSplitGUI<T>(ref SerializedProperty shadowCascadeSplit) | |||
There was a problem hiding this comment.
As this is inside internal class, I am removing it
|
|
||
| cmd.SetGlobalTexture(m_MainLightShadowmap.id, m_MainLightShadowmapTexture); | ||
| cmd.SetGlobalMatrixArray(MainLightShadowConstantBuffer._WorldToShadow, m_MainLightShadowMatrices); | ||
| ShadowUtils.SetupShadowReceiverConstantBuffer(cmd, m_MainLightShadowParams); |
There was a problem hiding this comment.
This is no longer needed, as additional light calculation no longer uses main light
| // it is needed when realtime shadows gets cut to early during fade and causes disconnect between baked shadow | ||
| shadowFade = shadowCoord.w == 4 ? 1.0h : shadowFade; | ||
| #endif | ||
|
|
There was a problem hiding this comment.
I tested, it is no longer needed as we are calculating shadow fade from cascade
This was the problem.
Co-authored-by: Felipe Lira <felipedrl@gmail.com>
…ge as it was triggering on correct values
There was a problem hiding this comment.
Issues with popping when the shadows lie in the blend range, seems like the overall shadow casting distance is lowered when the blend is there which means things are popping out of being rendered in the shadow map.
wnaOa5bcUd.mp4
UX wise is quite nice, switching between percentage and metric is not 'undo-able' which owuld be nice even though it's purely UI, also I think the end blend shouldnt change when changing in-between cascades, this is kind of annoying:
Other than that I like it very much and want it :D
…ra sphere. It is proved not to be good solution for current stable fit solution as last cascade in some cases might get outside shadow max distance.
…-Technologies/Graphics into universal/shadow-fade-improvements
This should be fixed in latest commit.
|
|
I just reverted shadow fading from last cascade, because it can not really be done for current stable fit solution without sacrficing performance. I will leave this to nigle stable fit fix. |
ernestasKupciunas
requested review from
ernestasKupciunas
and removed request for
a team
There was a problem hiding this comment.
Issues I found seem to have been fixed, and there appear to be no performance issues on PC.
Test doc here
…universal/shadow-fade-improvements
There was a problem hiding this comment.
Looks good on the mobile side.
I have found some bugs, but none of them were related to the changes (reproducible on Trunk and Master branches).
Testing doc: https://docs.google.com/document/d/12ZSHCzSR4j5ZW59EzOSyEZCSRSmPrsmYPjZvTWIs0Uo/edit#
Performance sheet: https://docs.google.com/spreadsheets/d/1Xp_OZcZsY14dpY5P9pAviGCrhzKOT6lzI17bT9KL8yg/edit#gid=0
Devices under testing:
VLNQA00217, Razer Phone 2, 9.0.0, Snapdragon 845 SDM845, Adreno 630
VLNQA00268, Samsung Galaxy S10+, 10.0.0, Exynos 9 9820, Mali-G76
OnePlus Nord, 10, Snapdragon 765G SM7250-AB, Adreno 620
VLNQA00108, LGE LG Leon 4G LTE, 5.0.1, Snapdragon 410 MSM8916, Adreno 306
VLNQA00126, Samsung Galaxy S5 Mini, 6.0.1, Exynos 3 Quad 3470, Mali-400 MP
VLNQA00292, iPad Air3, SoC: A12, iOS: 13.2.3
iPad Air4, SoC: 14, iOS: 14.0
iPhone 12Pro, SoC: 14, iOS: 14.1
iPhone XS, SoC: A12, iOS: 14.3
…universal/shadow-fade-improvements
|
I guess you already noticed but this merge request is leading to a missing |
|
^ PR: #3480 |
Purpose of this PR
Fix issues
Shadow fade distance is very short.
Extend with new functionality:
Controlable shadow fade distance.
Also I decided to refactor shadow cascade GUI for HDRP and URP. For several reasons:
Partition sizes inconsistant
Last cascade had extra pixel at end inconsistantly
Any resizing was causing pixel jumping
Did not wanted to increase code duplication as extending URP cascade would resulted in very similar code of HDRP one.
I could not used HDRP solution as I had to move it to renderpipeline public core. HDRP solution was not written to be public code.
New shadow cascade GUI
Includes:
(New urp upper, old urp bottom)
(New hdrp upper, old hdrp bottom)More info https://docs.google.com/document/d/1MAFk4AINTloYLgw8vBzhpah031LismehPQqVxiUhSx0.
Testing status
TODO
Comments to reviewers
I am mostly concerned about UI side if I did not screw up there with images, paths and so on (As rarely touch UI code).