UniversalRendererData GUID fix #5410
Conversation
|
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. URP Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
| // Core blitter shaders, adapted from HDRP | ||
| // TODO: move to core and share with HDRP | ||
| [Reload("Shaders/Utils/CoreBlit.shader"), SerializeField] | ||
| internal Shader coreBlitPS; | ||
| [Reload("Shaders/Utils/CoreBlitColorAndDepth.shader"), SerializeField] | ||
| internal Shader coreBlitColorAndDepthPS; | ||
|
|
||
|
|
||
| [Reload("Shaders/CameraMotionVectors.shader")] | ||
| public Shader cameraMotionVector; | ||
|
|
||
| [Reload("Shaders/ObjectMotionVectors.shader")] | ||
| public Shader objectMotionVector; |
There was a problem hiding this comment.
Why are these added
And do you have a JIRA for that TODO:?
There was a problem hiding this comment.
These are copied as is from the UniversalRendererData.cs to make sure we don’t loose any user data as the existing UniversalRenderDatas will briefly become ForwardRendererDatas.
| [SerializeField] LayerMask m_OpaqueLayerMask; | ||
| [SerializeField] LayerMask m_TransparentLayerMask; | ||
| [SerializeField] StencilStateData m_DefaultStencilState; // This default state is compatible with deferred renderer. | ||
| [SerializeField] bool m_ShadowTransparentReceive; | ||
| [SerializeField] RenderingMode m_RenderingMode; | ||
| [SerializeField] DepthPrimingMode m_DepthPrimingMode; // Default disabled because there are some outstanding issues with Text Mesh rendering. | ||
| [SerializeField] bool m_AccurateGbufferNormals; | ||
| [SerializeField] bool m_ClusteredRendering; | ||
| [SerializeField] TileSize m_TileSize; |
There was a problem hiding this comment.
Why are you adding these properties?
There was a problem hiding this comment.
Similar to the reason above, these have to be there to be copied over to the new serialized object, this was partially why before the conversion process didn’t retain any data set on the ForwardRenderer, now this list also needs to contain properties that are new to the UniversalRendererData.
This file should only be needed for a brief time, since it is only needed for those taking Alpha/Beta 21.2 projects forward, I imagine we will be able to remove this upgrader in Unity 2022 lifecycle.
There was a problem hiding this comment.
Tested with https://github.com/Unity-Technologies/URP-validation-project:
- if the renderers have been upgraded so the m_Script GUID is: de640fe3d0db1804a85f9fc8f5cadab6
- Has any data been lost, like m_RenderingMode, LayerMasks - I went through all URP assets and renderers, checked settings and confirmed no data was lost; also all scenes were rendering as expected
- Tested with/without Library folder
- Tested if reopening and reimporting project doesn’t break anything
- Tested 2021.2.0a19 > 2022.1.0a5.673
- Tested if upgrading from older version still works (2019.4 > 2022.1.0a5.673)
…es and added an additional warning after all upgrades to recommend user to save project
Purpose of this PR
When we changed the ForwardRendererData to be a UniversalRendererData we did not preserve the original GUID, at the time an upgrader was made to convert older ForwardRendererData assets into the new UniversalRendererData scriptable objects.
Recently it has been discovered that this upgrader has some issues and is not 100% reliable due to various reasons, in this PR we are reverting the GUID back to the original GUID that the file had in 21.1 and earlier.
What this means is that anyone coming from 21.1 or earlier will not notice any difference, and the asset will not need to go through any upgrade path.
What this does mean is any projects that has been opened in the Unity 2021.2 Alpha/Beta and have gone through the existing upgrade path for ForwardRendererData files, will have an incorrect GUID.
In this PR there is also an upgrader for these 21.2 Alpha/Beta projects, this means that users coming from LTS and public release versions of Unity have the safe path and the potentially less safe path is now only affecting Alpha/Beta projects.
Testing status
Describe what manual/automated tests were performed for this PR
Comments to reviewers
Notes for the reviewers you have assigned.