Skip to content

Commit 0b79608

Browse files
raquelpecesEvergreen
authored andcommitted
NativeRenderPass in URP can reach a hard-coded limit that breaks the rendering when using Compatibility Mode
When extending URP with custom renderer features, and Native RenderPass is enabled in the non-RenderGraph pass, the renderer can crash because of a hard-coded value `kRenderPassMaxCount = 20` used by an static array to set his size. That static array is saving in the index of the render Pass the hash of the render pass. That is causing that when the index of the pass is higher or equal than 20, the array has no place to save the hash of the pass. In fact, most of the places of the array are empty, because the hash is only allocated when the pass is active. With this change, the static array is changed to a Dictionary with the key as the index of the Pass, and the value as the hash of the Pass, so when the index of the RenderPass is higher than 20 this will no cause an `IndexOutOfBounds` error. The rest of structures used in the class could still be working without any problem Jira ticket: [UUM-69943](https://jira.unity3d.com/browse/UUM-69943)
1 parent aa21871 commit 0b79608

File tree

3 files changed

+145
-8
lines changed

3 files changed

+145
-8
lines changed

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
using System.Linq;
44
using Unity.Collections;
55
using UnityEngine.Experimental.Rendering;
6-
using UnityEngine.Rendering;
76

87
namespace UnityEngine.Rendering.Universal
98
{
109
public partial class ScriptableRenderer
1110
{
12-
private const int kRenderPassMapSize = 10;
13-
private const int kRenderPassMaxCount = 20;
11+
// Used "internal" only for testing. This should be private.
12+
internal const int kRenderPassMapSize = 10;
13+
internal const int kRenderPassMaxCount = 20;
1414

1515
// used to keep track of the index of the last pass when we called BeginSubpass
1616
private int m_LastBeginSubpassPassIndex = 0;
@@ -96,7 +96,6 @@ internal void SetupNativeRenderPassFrameData(UniversalCameraData cameraData, boo
9696
int lastPassIndex = m_ActiveRenderPassQueue.Count - 1;
9797

9898
// Make sure the list is already sorted!
99-
10099
m_MergeableRenderPassesMap.Clear();
101100
m_RenderPassesAttachmentCount.Clear();
102101
uint currentHashIndex = 0;
@@ -105,15 +104,22 @@ internal void SetupNativeRenderPassFrameData(UniversalCameraData cameraData, boo
105104
{
106105
var renderPass = m_ActiveRenderPassQueue[i];
107106

108-
renderPass.renderPassQueueIndex = i;
109-
110107
// Disable obsolete warning for internal usage
111108
#pragma warning disable CS0618
112109
bool RPEnabled = IsRenderPassEnabled(renderPass);
113110
#pragma warning restore CS0618
114111
if (!RPEnabled)
115112
continue;
116113

114+
// Check if current index pass is higher than the maximum number of passes
115+
if (i >= kRenderPassMaxCount)
116+
{
117+
Debug.LogError($"Exceeded the maximum number of Render Passes (${kRenderPassMaxCount}). Please consider using Render Graph to support a higher number of render passes with Native RenderPass, note support will be enabled by default.");
118+
return;
119+
}
120+
121+
renderPass.renderPassQueueIndex = i;
122+
117123
var rpDesc = InitializeRenderPassDescriptor(cameraData, renderPass);
118124

119125
Hash128 hash = CreateRenderPassHash(rpDesc, currentHashIndex);
@@ -129,10 +135,8 @@ internal void SetupNativeRenderPassFrameData(UniversalCameraData cameraData, boo
129135
else if (m_MergeableRenderPassesMap[hash][GetValidPassIndexCount(m_MergeableRenderPassesMap[hash]) - 1] != (i - 1))
130136
{
131137
// if the passes are not sequential we want to split the current mergeable passes list. So we increment the hashIndex and update the hash
132-
133138
currentHashIndex++;
134139
hash = CreateRenderPassHash(rpDesc, currentHashIndex);
135-
136140
m_PassIndexToPassHash[i] = hash;
137141

138142
m_MergeableRenderPassesMap.Add(hash, m_MergeableRenderPassesMapArrays[m_MergeableRenderPassesMap.Count]);
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
using System;
2+
using NUnit.Framework;
3+
using UnityEngine.TestTools;
4+
using UnityEngine;
5+
using UnityEngine.Rendering;
6+
using UnityEngine.Rendering.Universal;
7+
8+
9+
namespace UnityEditor.Rendering.Universal.Tests
10+
{
11+
[TestFixture]
12+
class NativeRenderPassTests
13+
{
14+
internal class TestHelper
15+
{
16+
internal UniversalRendererData rendererData;
17+
internal UniversalCameraData cameraData;
18+
internal UniversalRenderPipelineAsset urpAsset;
19+
internal ScriptableRenderer scriptableRenderer;
20+
21+
public TestHelper()
22+
{
23+
try
24+
{
25+
rendererData = ScriptableObject.CreateInstance<UniversalRendererData>();
26+
27+
urpAsset = UniversalRenderPipelineAsset.Create(rendererData);
28+
urpAsset.name = "TestHelper_URPAsset";
29+
GraphicsSettings.defaultRenderPipeline = urpAsset;
30+
31+
scriptableRenderer = urpAsset.GetRenderer(0);
32+
33+
cameraData = new UniversalCameraData();
34+
35+
ResetData();
36+
}
37+
catch (Exception e)
38+
{
39+
Debug.LogError(e.StackTrace);
40+
Cleanup();
41+
}
42+
}
43+
44+
internal void ResetData()
45+
{
46+
scriptableRenderer.useRenderPassEnabled = true;
47+
}
48+
49+
internal void Cleanup()
50+
{
51+
ScriptableObject.DestroyImmediate(urpAsset);
52+
ScriptableObject.DestroyImmediate(rendererData);
53+
}
54+
}
55+
56+
private TestHelper m_TestHelper;
57+
58+
private RenderPipelineAsset m_PreviousRenderPipelineAssetGraphicsSettings;
59+
private RenderPipelineAsset m_PreviousRenderPipelineAssetQualitySettings;
60+
public class TestRenderPassUseNRP : ScriptableRenderPass
61+
{
62+
public TestRenderPassUseNRP()
63+
{
64+
// Initialize with this argument to true, to avoid other unrelated errors
65+
overrideCameraTarget = true;
66+
// Enable the use of Native Render Pass. This is set to true by defalult, but we want to make it explicit
67+
useNativeRenderPass = true;
68+
}
69+
}
70+
71+
[OneTimeSetUp]
72+
public void OneTimeSetup()
73+
{
74+
m_PreviousRenderPipelineAssetGraphicsSettings = GraphicsSettings.defaultRenderPipeline;
75+
m_PreviousRenderPipelineAssetQualitySettings = QualitySettings.renderPipeline;
76+
GraphicsSettings.defaultRenderPipeline = null;
77+
QualitySettings.renderPipeline = null;
78+
}
79+
80+
[SetUp]
81+
public void Setup()
82+
{
83+
m_TestHelper = new();
84+
m_TestHelper.ResetData();
85+
}
86+
87+
[TearDown]
88+
public void TearDown()
89+
{
90+
m_TestHelper.Cleanup();
91+
}
92+
93+
[OneTimeTearDown]
94+
public void OneTimeTearDown()
95+
{
96+
GraphicsSettings.defaultRenderPipeline = m_PreviousRenderPipelineAssetGraphicsSettings;
97+
QualitySettings.renderPipeline = m_PreviousRenderPipelineAssetQualitySettings;
98+
}
99+
100+
public void InitializeRenderPassQueue(ScriptableRenderer renderer, int count)
101+
{
102+
for (int i = 0; i < count; i++)
103+
{
104+
renderer.EnqueuePass(new TestRenderPassUseNRP());
105+
}
106+
}
107+
108+
[Test]
109+
public void UnderLimitRenderPassInNRP()
110+
{
111+
// Use kRenderPassMaxCount so this is the maximun allowed
112+
InitializeRenderPassQueue(m_TestHelper.scriptableRenderer, ScriptableRenderer.kRenderPassMaxCount);
113+
// Check that no exception is thrown.
114+
Assert.DoesNotThrow(() => m_TestHelper.scriptableRenderer.SetupNativeRenderPassFrameData(m_TestHelper.cameraData, true));
115+
}
116+
117+
[Test]
118+
public void OverLimitRenderPassInNRP()
119+
{
120+
// Increase by one the maximum allowed render passes
121+
InitializeRenderPassQueue(m_TestHelper.scriptableRenderer, ScriptableRenderer.kRenderPassMaxCount+1);
122+
// Check that a logError is thrown, but no other errors are thrown.
123+
m_TestHelper.scriptableRenderer.SetupNativeRenderPassFrameData( m_TestHelper.cameraData, true );
124+
LogAssert.Expect($"Exceeded the maximum number of Render Passes (${ScriptableRenderer.kRenderPassMaxCount}). Please consider using Render Graph to support a higher number of render passes with Native RenderPass, note support will be enabled by default.");
125+
}
126+
127+
}
128+
129+
130+
131+
}

Packages/com.unity.render-pipelines.universal/Tests/Editor/NativeRenderPassTests.cs.meta

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)