Skip to content

[Universal] Make sure upgrading to Universal RP 2021 does not change Spot Light shadow resolutions #3831

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Apr 9, 2021
Merged

[Universal] Make sure upgrading to Universal RP 2021 does not change Spot Light shadow resolutions #3831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
94393c1
[UniversalRP] Replace some magic numbers by named variable UniversalA…
Wilfrid-Unity Mar 11, 2021
204a9e1
[UniversalRP] Make sure upgrading to Universal RP 2021 does not chang…
Wilfrid-Unity Mar 11, 2021
1f7e953
[UniversalRP] Only upgrade Additional Light Shadow Resolution Tier va…
Wilfrid-Unity Mar 11, 2021
cd1b5e8
[UniversalRP] Move UniversalRenderPipelineAsset.AdditionalLightsDefau…
Wilfrid-Unity Mar 12, 2021
aac184c
[UniversalRP] Fix merge error - Addresses comment https://github.com/…
Wilfrid-Unity Mar 17, 2021
43ed67d
[UniversalRP] Increase UniversalGraphicsTest_Foundation scene 105_Tra…
Wilfrid-Unity Mar 23, 2021
82f939d
Add local variable to make code easier to read (as suggested in https…
Wilfrid-Unity Apr 1, 2021
e6b8db2
Merge branch 'master' into universal/additional-light-shadow-resoluti…
Wilfrid-Unity Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ojects/UniversalGraphicsTest_Foundation/Assets/Scenes/105_TransparentReceiveShadows.unity
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1752,7 +1752,7 @@ MonoBehaviour:
m_Name:
m_EditorClassIdentifier:
m_UsePipelineSettings: 0
m_AdditionalLightsShadowResolutionTier: 1
m_AdditionalLightsShadowResolutionTier: 2
--- !u!1 &558490757
GameObject:
m_ObjectHideFlags: 0
Expand Down
4 changes: 2 additions & 2 deletions com.unity.render-pipelines.universal/Editor/UniversalRenderPipelineAssetEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ internal class Styles
public static GUIContent perObjectLimit = EditorGUIUtility.TrTextContent("Per Object Limit", "Maximum amount of additional lights. These lights are sorted and culled per-object.");
public static GUIContent supportsAdditionalShadowsText = EditorGUIUtility.TrTextContent("Cast Shadows", "If enabled shadows will be supported for spot lights.\n");
public static GUIContent additionalLightsShadowmapResolution = EditorGUIUtility.TrTextContent("Shadow Atlas Resolution", "All additional lights are packed into a single shadowmap atlas. This setting controls the atlas size.");
public static GUIContent additionalLightsShadowResolutionTiers = EditorGUIUtility.TrTextContent("Shadow Resolution Tiers", "Additional Lights Shadow Resolution Tiers. Rounded to the next power of two, and clamped to be at least 128.");
public static GUIContent additionalLightsShadowResolutionTiers = EditorGUIUtility.TrTextContent("Shadow Resolution Tiers", $"Additional Lights Shadow Resolution Tiers. Rounded to the next power of two, and clamped to be at least {UniversalAdditionalLightData.AdditionalLightsShadowMinimumResolution}.");
public static GUIContent[] additionalLightsShadowResolutionTierNames =
{
new GUIContent("Low"),
Expand Down Expand Up @@ -379,7 +379,7 @@ void DrawShadowResolutionTierSettings()
{
var fieldSlot = new Rect(contentRect.x + pixelShift, contentRect.y, num - labelWidth, contentRect.height); // Define the rectangle for the field
int value = EditorGUI.DelayedIntField(fieldSlot, values[index].intValue);
values[index].intValue = Mathf.Max(128, Mathf.NextPowerOfTwo(value));
values[index].intValue = Mathf.Max(UniversalAdditionalLightData.AdditionalLightsShadowMinimumResolution, Mathf.NextPowerOfTwo(value));
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't from this change, but we really need to get away from defaulting to power of two sizes. Most modern GPU architectures support non power of two sizes, and it is only really relevant for mipmaps and compressed formats.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it is a atlas fitting limitation, then it is a bit more understandable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in the case of Additional Light shadows, it is a limitation of the current (simple) atlas fitting.

}
pixelShift += spaceLeft; // Shift by the slot that was left for the field
}
Expand Down
4 changes: 2 additions & 2 deletions com.unity.render-pipelines.universal/Editor/UniversalRenderPipelineLightEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Styles
public static readonly GUIContent ShadowNormalBias = EditorGUIUtility.TrTextContent("Normal", "Controls the distance shadow caster vertices are offset along their normals when rendering shadow maps. Currently ignored for Point Lights.");

// Resolution (default or custom)
public static readonly GUIContent ShadowResolution = EditorGUIUtility.TrTextContent("Resolution", "Sets the rendered resolution of the shadow maps. A higher resolution increases the fidelity of shadows at the cost of GPU performance and memory usage. Rounded to the next power of two, and clamped to be at least 128.");
public static readonly GUIContent ShadowResolution = EditorGUIUtility.TrTextContent("Resolution", $"Sets the rendered resolution of the shadow maps. A higher resolution increases the fidelity of shadows at the cost of GPU performance and memory usage. Rounded to the next power of two, and clamped to be at least {UniversalAdditionalLightData.AdditionalLightsShadowMinimumResolution}.");
public static readonly int[] ShadowResolutionDefaultValues =
{
UniversalAdditionalLightData.AdditionalLightsShadowResolutionTierCustom,
Expand Down Expand Up @@ -376,7 +376,7 @@ void DrawShadowsResolutionGUI()
{
// show the custom value field GUI.
var newResolution = EditorGUI.IntField(fieldRect, settings.shadowsResolution.intValue);
settings.shadowsResolution.intValue = Mathf.Max(128, Mathf.NextPowerOfTwo(newResolution));
settings.shadowsResolution.intValue = Mathf.Max(UniversalAdditionalLightData.AdditionalLightsShadowMinimumResolution, Mathf.NextPowerOfTwo(newResolution));

m_AdditionalLightDataSO.ApplyModifiedProperties();
}
Expand Down
37 changes: 30 additions & 7 deletions com.unity.render-pipelines.universal/Runtime/Data/UniversalRenderPipelineAsset.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ public partial class UniversalRenderPipelineAsset : RenderPipelineAsset, ISerial
ScriptableRenderer[] m_Renderers = new ScriptableRenderer[1];

// Default values set when a new UniversalRenderPipeline asset is created
[SerializeField] int k_AssetVersion = 8;
[SerializeField] int k_AssetPreviousVersion = 8;
[SerializeField] int k_AssetVersion = 9;
[SerializeField] int k_AssetPreviousVersion = 9;

// Deprecated settings for upgrading sakes
[SerializeField] RendererType m_RendererType = RendererType.UniversalRenderer;
Expand Down Expand Up @@ -139,9 +139,9 @@ public partial class UniversalRenderPipelineAsset : RenderPipelineAsset, ISerial
[SerializeField] bool m_AdditionalLightShadowsSupported = false;
[SerializeField] ShadowResolution m_AdditionalLightsShadowmapResolution = ShadowResolution._2048;

[SerializeField] int m_AdditionalLightsShadowResolutionTierLow = 256;
[SerializeField] int m_AdditionalLightsShadowResolutionTierMedium = 512;
[SerializeField] int m_AdditionalLightsShadowResolutionTierHigh = 1024;
[SerializeField] int m_AdditionalLightsShadowResolutionTierLow = AdditionalLightsDefaultShadowResolutionTierLow;
[SerializeField] int m_AdditionalLightsShadowResolutionTierMedium = AdditionalLightsDefaultShadowResolutionTierMedium;
[SerializeField] int m_AdditionalLightsShadowResolutionTierHigh = AdditionalLightsDefaultShadowResolutionTierHigh;

// Shadows Settings
[SerializeField] float m_ShadowDistance = 50.0f;
Expand Down Expand Up @@ -186,6 +186,10 @@ public partial class UniversalRenderPipelineAsset : RenderPipelineAsset, ISerial
internal const int k_ShadowCascadeMinCount = 1;
internal const int k_ShadowCascadeMaxCount = 4;

public static readonly int AdditionalLightsDefaultShadowResolutionTierLow = 256;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if I like constant tiers. It doesn't scale very well. IMO user should just be able to put a number or 2^N for pot. It' depends on the platform and content what's suitable.
This is not a deal breaker though.

public static readonly int AdditionalLightsDefaultShadowResolutionTierMedium = 512;
public static readonly int AdditionalLightsDefaultShadowResolutionTierHigh = 1024;

#if UNITY_EDITOR
[NonSerialized]
internal UniversalRenderPipelineEditorResources m_EditorResourcesAsset;
Expand Down Expand Up @@ -934,6 +938,25 @@ public void OnAfterDeserialize()
k_AssetVersion = 8;
}

if (k_AssetVersion < 9)
{
bool assetContainsCustomAdditionalLightShadowResolutions =
m_AdditionalLightsShadowResolutionTierHigh != AdditionalLightsDefaultShadowResolutionTierHigh ||
m_AdditionalLightsShadowResolutionTierMedium != AdditionalLightsDefaultShadowResolutionTierMedium ||
m_AdditionalLightsShadowResolutionTierLow != AdditionalLightsDefaultShadowResolutionTierLow;

if (!assetContainsCustomAdditionalLightShadowResolutions)
{
// if all resolutions are still the default values, we assume that they have never been customized and that it is safe to upgrade them to fit better the Additional Lights Shadow Atlas size
m_AdditionalLightsShadowResolutionTierHigh = (int)m_AdditionalLightsShadowmapResolution;
m_AdditionalLightsShadowResolutionTierMedium = Mathf.Max(m_AdditionalLightsShadowResolutionTierHigh / 2, UniversalAdditionalLightData.AdditionalLightsShadowMinimumResolution);
m_AdditionalLightsShadowResolutionTierLow = Mathf.Max(m_AdditionalLightsShadowResolutionTierMedium / 2, UniversalAdditionalLightData.AdditionalLightsShadowMinimumResolution);
}

k_AssetPreviousVersion = k_AssetVersion;
k_AssetVersion = 9;
}

#if UNITY_EDITOR
if (k_AssetPreviousVersion != k_AssetVersion)
{
Expand Down Expand Up @@ -964,10 +987,10 @@ static void UpgradeAsset(UniversalRenderPipelineAsset asset)
asset.k_AssetPreviousVersion = 5;
}

if (asset.k_AssetPreviousVersion < 8)
if (asset.k_AssetPreviousVersion < 9)
{
// The added feature was reverted, we keep this version to avoid breakage in case somebody already has version 7
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Comment needs updating. Should the comment be generic to say previous version instead of explicitly saying 7 or 8, etc.

asset.k_AssetPreviousVersion = 8;
asset.k_AssetPreviousVersion = 9;
}

EditorUtility.SetDirty(asset);
Expand Down
3 changes: 2 additions & 1 deletion com.unity.render-pipelines.universal/Runtime/UniversalAdditionalLightData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ public bool usePipelineSettings
public static readonly int AdditionalLightsShadowResolutionTierLow = 0;
public static readonly int AdditionalLightsShadowResolutionTierMedium = 1;
public static readonly int AdditionalLightsShadowResolutionTierHigh = 2;
public static readonly int AdditionalLightsShadowDefaultResolutionTier = AdditionalLightsShadowResolutionTierLow;
public static readonly int AdditionalLightsShadowDefaultResolutionTier = AdditionalLightsShadowResolutionTierHigh;
public static readonly int AdditionalLightsShadowDefaultCustomResolution = 128;
public static readonly int AdditionalLightsShadowMinimumResolution = 128;

[Tooltip("Controls if light shadow resolution uses pipeline settings.")]
[SerializeField] int m_AdditionalLightsShadowResolutionTier = AdditionalLightsShadowDefaultResolutionTier;
Expand Down