Fix RTHandles.Alloc() failing when called inside ScriptableRendererFeature.Create()

This PR addresses https://jira.unity3d.com/browse/UUM-44048

Slack discussion: https://unity.slack.com/archives/C89KFUUCT/p1706102897599419

**The issue:**
Currently, `ScriptableRendererFeature.Create()` is called from `OnEnable()` and `OnValidate()` event functions, both of which regularly occur when `UniversalRenderPipeline` has not yet been constructed, for example any time a script change triggers domain reload, or user enters playmode. This is a problem because ScriptableRendererFeature is logically a subsystem of the render pipeline, so it is expected that you are able initialize rendering resources in `ScriptableRendererFeature.Create()`. Since `RTHandleSystem` is only initialized by URP constructor, calling `RTHandle.Alloc()` here will result in errors.

**The fix:**
Turns out `ScriptableRendererFeature.Create()` is called from 3 places:
1. `ScriptableRendererFeature.OnEnable()` (event function)
2. `ScriptableRendererFeature.OnValidate()` (event function)
3. `ScriptableRenderer.ctor()`

The callsite 3) is the one that actually takes the feature from `ScriptableRendererData`, calls `Create()` and adds it to another list inside the runtime `ScriptableRenderer` instance, where it can be accessed during rendering.

All of this means that `ScriptableRendererFeature.Create()` gets called **at least 3 times** for example whenever you enter play mode. This seems kind of wasteful but maybe there's a reason for it that I'm not seeing.

The fix I'm making is just adding a check to callsites 1) and 2) so that they don't call `Create()` if URP has not yet been constructed. `ScriptableRenderer` constructor still guarantees that the feature gets created before rendering.

Author: arttu-peltonen

Packages/com.unity.render-pipelines.universal/Runtime/ScriptableRendererFeature.cs

@@ -45,12 +45,18 @@ public virtual void SetupRenderPasses(ScriptableRenderer renderer, in RenderingD
 
         void OnEnable()
         {
-            Create();
+            // UUM-44048: If the pipeline is not created, don't call Create() as it may allocate RTHandles or do other
+            // things that require the pipeline to be constructed. This is safe because once the pipeline is constructed,
+            // ScriptableRendererFeature.Create() will be called by ScriptableRenderer constructor.
+            if (RenderPipelineManager.currentPipeline is UniversalRenderPipeline)
+                Create();
         }
 
         void OnValidate()
         {
-            Create();
+            // See comment in OnEnable.
+            if (RenderPipelineManager.currentPipeline is UniversalRenderPipeline)
+                Create();
         }
 
         /// <summary>

Tests/SRPTests/Projects/UniversalGraphicsTest_Foundation/Assets/CommonAssets/AllocRTHandleInCreate_ScriptableRendererFeature.cs

@@ -0,0 +1,38 @@
+using UnityEngine;
+using UnityEngine.Rendering;
+using UnityEngine.Rendering.Universal;
+
+public class AllocRTHandleInCreate_ScriptableRendererFeature : ScriptableRendererFeature
+{
+    class AllocRTHandleInCreate_ScriptableRenderPass : ScriptableRenderPass
+    {
+        RTHandle m_RTHandle;
+
+        public AllocRTHandleInCreate_ScriptableRenderPass()
+        {
+            m_RTHandle = RTHandles.Alloc(Vector2.one, name: "DummyScriptableRendererPass RTHandle");
+        }
+
+        public void Dispose()
+        {
+            m_RTHandle.Release();
+        }
+    }
+
+    AllocRTHandleInCreate_ScriptableRenderPass m_Pass;
+
+    public override void Create()
+    {
+        m_Pass = new AllocRTHandleInCreate_ScriptableRenderPass();
+    }
+
+    protected override void Dispose(bool disposing)
+    {
+        m_Pass?.Dispose();
+        m_Pass = null;
+    }
+
+    public override void AddRenderPasses(ScriptableRenderer renderer, ref RenderingData renderingData)
+    {
+    }
+}

Tests/SRPTests/Projects/UniversalGraphicsTest_Foundation/Assets/CommonAssets/AllocRTHandleInCreate_ScriptableRendererFeature.cs.meta

@@ -0,0 +1,67 @@
+%YAML 1.1
+%TAG !u! tag:unity3d.com,2011:
+--- !u!114 &-8472969894007707715
+MonoBehaviour:
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_GameObject: {fileID: 0}
+  m_Enabled: 1
+  m_EditorHideFlags: 0
+  m_Script: {fileID: 11500000, guid: 513627f990bf4d32b40ed586608b5d61, type: 3}
+  m_Name: AllocRTHandleInCreate_ScriptableRendererFeature
+  m_EditorClassIdentifier: 
+  m_Active: 1
+--- !u!114 &11400000
+MonoBehaviour:
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_GameObject: {fileID: 0}
+  m_Enabled: 1
+  m_EditorHideFlags: 0
+  m_Script: {fileID: 11500000, guid: de640fe3d0db1804a85f9fc8f5cadab6, type: 3}
+  m_Name: AllocRTHandleInCreate_UniversalRendererData
+  m_EditorClassIdentifier: 
+  debugShaders:
+    debugReplacementPS: {fileID: 4800000, guid: cf852408f2e174538bcd9b7fda1c5ae7,
+      type: 3}
+    hdrDebugViewPS: {fileID: 4800000, guid: 573620ae32aec764abd4d728906d2587, type: 3}
+    probeVolumeSamplingDebugComputeShader: {fileID: 7200000, guid: 53626a513ea68ce47b59dc1299fe3959,
+      type: 3}
+  probeVolumeResources:
+    probeVolumeDebugShader: {fileID: 0}
+    probeVolumeFragmentationDebugShader: {fileID: 0}
+    probeVolumeOffsetDebugShader: {fileID: 0}
+    probeVolumeSamplingDebugShader: {fileID: 0}
+    probeSamplingDebugMesh: {fileID: 0}
+    probeSamplingDebugTexture: {fileID: 0}
+    probeVolumeBlendStatesCS: {fileID: 0}
+  m_RendererFeatures:
+  - {fileID: -8472969894007707715}
+  m_RendererFeatureMap: bd6717ebc5f6698a
+  m_UseNativeRenderPass: 0
+  xrSystemData: {fileID: 0}
+  postProcessData: {fileID: 0}
+  m_AssetVersion: 2
+  m_OpaqueLayerMask:
+    serializedVersion: 2
+    m_Bits: 4294967295
+  m_TransparentLayerMask:
+    serializedVersion: 2
+    m_Bits: 4294967295
+  m_DefaultStencilState:
+    overrideStencilState: 0
+    stencilReference: 0
+    stencilCompareFunction: 8
+    passOperation: 2
+    failOperation: 0
+    zFailOperation: 0
+  m_ShadowTransparentReceive: 1
+  m_RenderingMode: 0
+  m_DepthPrimingMode: 0
+  m_CopyDepthMode: 1
+  m_AccurateGbufferNormals: 0
+  m_IntermediateTextureMode: 1

Tests/SRPTests/Projects/UniversalGraphicsTest_Foundation/Assets/CommonAssets/AllocRTHandleInCreate_UniversalRendererData.asset

@@ -40,6 +40,7 @@ MonoBehaviour:
   - {fileID: 11400000, guid: 7155e055dd29f9944927bdffff6eb366, type: 2}
   - {fileID: 11400000, guid: 74a701ce533b14adb8f98f35425f3aeb, type: 2}
   - {fileID: 11400000, guid: 846bf2a7afe0943388665d187143c7f6, type: 2}
+  - {fileID: 11400000, guid: 708aca6b4b64e8f41946dabb10fe1586, type: 2}
   m_DefaultRendererIndex: 0
   m_RequireDepthTexture: 1
   m_RequireOpaqueTexture: 1
@@ -58,7 +59,8 @@ MonoBehaviour:
   m_LightProbeSystem: 0
   m_ProbeVolumeMemoryBudget: 1024
   m_ProbeVolumeBlendingMemoryBudget: 128
-  m_SupportProbeVolumeStreaming: 0
+  m_SupportProbeVolumeGPUStreaming: 0
+  m_SupportProbeVolumeDiskStreaming: 0
   m_SupportProbeVolumeScenarios: 0
   m_SupportProbeVolumeScenarioBlending: 0
   m_ProbeVolumeSHBands: 1
@@ -103,26 +105,21 @@ MonoBehaviour:
   m_SupportDataDrivenLensFlare: 0
   m_SupportScreenSpaceLensFlare: 0
   m_GPUResidentDrawerMode: 0
+  m_UseLegacyLightmaps: 0
+  m_SmallMeshScreenPercentage: 0
+  m_GPUResidentDrawerEnableOcclusionCullingInCameras: 0
   m_ShadowType: 0
   m_LocalShadowsSupported: 1
   m_LocalShadowsAtlasResolution: 512
   m_MaxPixelLights: 4
   m_ShadowAtlasResolution: 2048
   m_VolumeFrameworkUpdateMode: 0
   m_VolumeProfile: {fileID: 0}
-  m_Textures:
-    blueNoise64LTex: {fileID: 2800000, guid: e3d24661c1e055f45a7560c033dbb837, type: 3}
-    bayerMatrixTex: {fileID: 2800000, guid: f9ee4ed84c1d10c49aabb9b210b0fc44, type: 3}
   apvScenesData:
-    m_ObsoleteSerializedBakingSets: []
-    sceneToBakingSet:
-      m_Keys: []
-      m_Values: []
-    bakingSets: []
-    sceneBounds:
+    obsoleteSceneBounds:
       m_Keys: []
       m_Values: []
-    hasProbeVolumes:
+    obsoleteHasProbeVolumes:
       m_Keys: []
       m_Values: 
   m_PrefilteringModeMainLightShadows: 3
@@ -153,5 +150,9 @@ MonoBehaviour:
   m_PrefilterSoftShadows: 0
   m_PrefilterScreenCoord: 1
   m_PrefilterNativeRenderPass: 0
+  m_PrefilterUseLegacyLightmaps: 0
   m_ShaderVariantLogLevel: 0
   m_ShadowCascades: 3
+  m_Textures:
+    blueNoise64LTex: {fileID: 2800000, guid: e3d24661c1e055f45a7560c033dbb837, type: 3}
+    bayerMatrixTex: {fileID: 2800000, guid: f9ee4ed84c1d10c49aabb9b210b0fc44, type: 3}

Tests/SRPTests/Projects/UniversalGraphicsTest_Foundation/Assets/CommonAssets/AllocRTHandleInCreate_UniversalRendererData.asset.meta

@@ -0,0 +1,79 @@
+using System;
+using System.Collections;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.Rendering;
+using UnityEngine.Rendering.Universal;
+using UnityEngine.TestTools;
+
+public class ScriptableRendererFeatureTests
+{
+    // Custom log handler is needed because the default one misses LogErrors that happen during RP constructor due to
+    // how domain reload works with the test framework. This handler will notice the errors and fail the test inside
+    // the test body if any were logged (for example if RTHandles.Initialize logs an error).
+    class ErrorLogHandler : ILogHandler, IDisposable
+    {
+        ILogHandler m_OriginalHandler;
+        public string errorMessage { get; private set; }
+        public bool hasError => !string.IsNullOrEmpty(errorMessage);
+
+        public ErrorLogHandler()
+        {
+            m_OriginalHandler = Debug.unityLogger.logHandler;
+            Debug.unityLogger.logHandler = this;
+        }
+
+        public void LogFormat(LogType logType, UnityEngine.Object context, string format, params object[] args)
+        {
+            if (logType == LogType.Error)
+            {
+                // Defer assertion to ensure we are not in the middle of RP constructor or something
+                errorMessage = string.Format(format, args);
+            }
+            else
+            {
+                m_OriginalHandler.LogFormat(logType, context, format, args);
+            }
+        }
+
+        public void LogException(Exception exception, UnityEngine.Object context)
+        {
+            m_OriginalHandler.LogException(exception, context);
+        }
+
+        public void Dispose()
+        {
+            Debug.unityLogger.logHandler = m_OriginalHandler;
+            m_OriginalHandler = null;
+
+            errorMessage = null;
+        }
+    }
+
+    ErrorLogHandler m_ErrorLogHandler;
+
+    [SetUp]
+    public void Setup()
+    {
+        m_ErrorLogHandler = new ErrorLogHandler();
+    }
+
+    [TearDown]
+    public void TearDown()
+    {
+        m_ErrorLogHandler.Dispose();
+    }
+
+    [UnityTest]
+    public IEnumerator AllocRTHandleInCreate_EnterExitPlayModeWithoutErrors()
+    {
+        yield return new EnterPlayMode();
+        yield return null;
+
+        yield return new ExitPlayMode();
+        yield return null;
+
+        if (m_ErrorLogHandler.hasError)
+            Assert.Fail(m_ErrorLogHandler.errorMessage);
+    }
+}

Tests/SRPTests/Projects/UniversalGraphicsTest_Foundation/Assets/CommonAssets/URPAssets/DefaultURPAsset.asset

@@ -12,7 +12,8 @@
         "GUID:15fc0a57446b3144c949da3e2b9737a9",
         "GUID:36d514892e55d4340b470c29fcab8b44",
         "GUID:55aba1c6f9cf8e949aa77aaeccd46083",
-        "GUID:adf6f42bac617413dbcd97c993ab7077"
+        "GUID:adf6f42bac617413dbcd97c993ab7077",
+        "GUID:df380645f10b7bc4b97d4f5eb6303d95"
     ],
     "includePlatforms": [
         "Editor"