Removed RenderSettings.ambientProbe assignment that should not exist.

alekseiunity · Evergreen · commit f716ea0f003b · 2024-04-25T13:22:01.000Z
Fixes the issue with accumulating ambient probe lighting when sky light multiplier is not equal to one. https://jira.unity3d.com/browse/UUM-69385
diff --git a/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/GPUResidentBatcher.cs b/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/GPUResidentBatcher.cs
@@ -84,6 +84,19 @@ public void DestroyMeshes(NativeArray<int> destroyedMeshes)
             m_InstanceCullingBatcher.DestroyMeshes(destroyedMeshes);
         }
 
+        internal void FreeRendererGroupInstances(NativeArray<int> rendererGroupIDs)
+        {
+            if (rendererGroupIDs.Length == 0)
+                return;
+
+            var instances = new NativeList<InstanceHandle>(rendererGroupIDs.Length, Allocator.TempJob);
+            m_BatchersContext.ScheduleQueryRendererGroupInstancesJob(rendererGroupIDs, instances).Complete();
+            DestroyInstances(instances.AsArray());
+            instances.Dispose();
+
+            m_BatchersContext.FreeRendererGroupInstances(rendererGroupIDs);
+        }
+
         public void InstanceOcclusionTest(RenderGraph renderGraph, in OcclusionCullingSettings settings, ReadOnlySpan<SubviewOcclusionTest> subviewOcclusionTests)
         {
             if (!m_BatchersContext.hasBoundingSpheres)
@@ -122,13 +135,18 @@ public void UpdateSelectedRenderers(NativeArray<int> renderersID)
 
         public void PostCullBeginCameraRendering(RenderRequestBatcherContext context)
         {
-            RenderSettings.ambientProbe = context.ambientProbe;
-            m_BatchersContext.UpdateAmbientProbeAndGpuBuffer(context.ambientProbe);
             m_InstanceCullingBatcher.PostCullBeginCameraRendering(context);
         }
 
+        public void OnSetupAmbientProbe()
+        {
+            m_BatchersContext.UpdateAmbientProbeAndGpuBuffer(forceUpdate: false);
+        }
+
         private void UpdateRendererData(in GPUDrivenRendererGroupData rendererData, IList<Mesh> meshes, IList<Material> materials)
         {
+            FreeRendererGroupInstances(rendererData.invalidRendererGroupID);
+
             if (rendererData.rendererGroupID.Length == 0)
                 return;
 
diff --git a/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/GPUResidentDrawer.cs b/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/GPUResidentDrawer.cs
@@ -64,6 +64,15 @@ public static void PostCullBeginCameraRendering(RenderRequestBatcherContext cont
             s_Instance?.batcher.PostCullBeginCameraRendering(context);
         }
 
+
+        /// <summary>
+        /// Utility function for updating probe data after global ambient probe is set up
+        /// </summary>
+        public static void OnSetupAmbientProbe()
+        {
+            s_Instance?.batcher.OnSetupAmbientProbe();
+        }
+
         /// <summary>
         /// Utility function to run an occlusion test in compute to update indirect draws.
         /// This function will dispatch compute shaders to run the given occlusion test and
@@ -445,8 +454,9 @@ private void Dispose()
 
         private void OnSceneLoaded(Scene scene, LoadSceneMode mode)
         {
+            // Loaded scene might contain light probes that would affect existing objects. Hence we have to update all probes data.
             if(mode == LoadSceneMode.Additive)
-                m_BatchersContext.UpdateAmbientProbeAndGpuBuffer(RenderSettings.ambientProbe, true);
+                m_BatchersContext.UpdateAmbientProbeAndGpuBuffer(forceUpdate: true);
         }
 
         private static void PostPostLateUpdateStatic()
@@ -540,22 +550,24 @@ private void OnEndCameraRendering(ScriptableRenderContext context, Camera camera
 
         private void PostPostLateUpdate()
         {
+            m_BatchersContext.UpdateAmbientProbeAndGpuBuffer(forceUpdate: false);
+
             Profiler.BeginSample("GPUResidentDrawer.DispatchChanges");
             var lodGroupTransformData = m_Dispatcher.GetTransformChangesAndClear<LODGroup>(TransformTrackingType.GlobalTRS, Allocator.TempJob);
             var lodGroupData = m_Dispatcher.GetTypeChangesAndClear<LODGroup>(Allocator.TempJob, noScriptingArray: true);
             var meshDataSorted = m_Dispatcher.GetTypeChangesAndClear<Mesh>(Allocator.TempJob, sortByInstanceID: true, noScriptingArray: true);
             var materialData = m_Dispatcher.GetTypeChangesAndClear<Material>(Allocator.TempJob, noScriptingArray: true);
             Profiler.EndSample();
 
-            Profiler.BeginSample("GPUResindentDrawer.ProcessMaterials");
+            Profiler.BeginSample("GPUResidentDrawer.ProcessMaterials");
             ProcessMaterials(materialData.destroyedID);
             Profiler.EndSample();
 
-            Profiler.BeginSample("GPUResindentDrawer.ProcessMeshes");
+            Profiler.BeginSample("GPUResidentDrawer.ProcessMeshes");
             ProcessMeshes(meshDataSorted.destroyedID);
             Profiler.EndSample();
 
-            Profiler.BeginSample("GPUResindentDrawer.ProcessLODGroups");
+            Profiler.BeginSample("GPUResidentDrawer.ProcessLODGroups");
             ProcessLODGroups(lodGroupData.changedID, lodGroupData.destroyedID, lodGroupTransformData.transformedID);
             Profiler.EndSample();
 
@@ -564,12 +576,11 @@ private void PostPostLateUpdate()
             meshDataSorted.Dispose();
             materialData.Dispose();
 
-            Profiler.BeginSample("GPUResindentDrawer.ProcessDraws");
+            Profiler.BeginSample("GPUResidentDrawer.ProcessDraws");
             m_MeshRendererDrawer.ProcessDraws();
             // Add more drawers here ...
             Profiler.EndSample();
 
-            m_BatchersContext.UpdateAmbientProbeAndGpuBuffer(RenderSettings.ambientProbe);
             m_BatchersContext.UpdateInstanceMotions();
 
             m_Batcher.UpdateFrame();
@@ -610,7 +621,7 @@ private void ProcessLODGroups(NativeArray<int> changedID, NativeArray<int> destr
 
         internal void ProcessRenderers(NativeArray<int> rendererGroupsID)
         {
-            Profiler.BeginSample("GPUResindentDrawer.ProcessMeshRenderers");
+            Profiler.BeginSample("GPUResidentDrawer.ProcessMeshRenderers");
 
             var changedInstances = new NativeArray<InstanceHandle>(rendererGroupsID.Length, Allocator.TempJob, NativeArrayOptions.UninitializedMemory);
             ScheduleQueryRendererGroupInstancesJob(rendererGroupsID, changedInstances).Complete();
@@ -624,7 +635,7 @@ internal void ProcessRenderers(NativeArray<int> rendererGroupsID)
 
         internal void TransformInstances(NativeArray<InstanceHandle> instances, NativeArray<Matrix4x4> localToWorldMatrices)
         {
-            Profiler.BeginSample("GPUResindentDrawer.TransformInstances");
+            Profiler.BeginSample("GPUResidentDrawer.TransformInstances");
 
             m_BatchersContext.UpdateInstanceTransforms(instances, localToWorldMatrices);
 
@@ -633,7 +644,7 @@ internal void TransformInstances(NativeArray<InstanceHandle> instances, NativeAr
 
         internal void FreeInstances(NativeArray<InstanceHandle> instances)
         {
-            Profiler.BeginSample("GPUResindentDrawer.FreeInstances");
+            Profiler.BeginSample("GPUResidentDrawer.FreeInstances");
 
             m_Batcher.DestroyInstances(instances);
             m_BatchersContext.FreeInstances(instances);
@@ -643,17 +654,9 @@ internal void FreeInstances(NativeArray<InstanceHandle> instances)
 
         internal void FreeRendererGroupInstances(NativeArray<int> rendererGroupIDs)
         {
-            if(rendererGroupIDs.Length == 0)
-                return;
-
-            Profiler.BeginSample("GPUResindentDrawer.FreeRendererGroupInstances");
-
-            var instances = new NativeList<InstanceHandle>(rendererGroupIDs.Length, Allocator.TempJob);
-            ScheduleQueryRendererGroupInstancesJob(rendererGroupIDs, instances).Complete();
-            m_Batcher.DestroyInstances(instances.AsArray());
-            instances.Dispose();
+            Profiler.BeginSample("GPUResidentDrawer.FreeRendererGroupInstances");
 
-            m_BatchersContext.FreeRendererGroupInstances(rendererGroupIDs);
+            m_Batcher.FreeRendererGroupInstances(rendererGroupIDs);
 
             Profiler.EndSample();
         }
diff --git a/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/GPUResidentDrawerTypes.cs b/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/GPUResidentDrawerTypes.cs
@@ -15,11 +15,6 @@ public struct RenderRequestBatcherContext
         /// CommandBuffer that will be used for resulting commands
         /// </summary>
         public CommandBuffer commandBuffer;
-
-        /// <summary>
-        /// Ambient probe to be set
-        /// </summary>
-        public SphericalHarmonicsL2 ambientProbe;
     }
 
     /// <summary>
diff --git a/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/RenderersBatchersContext.cs b/Packages/com.unity.render-pipelines.core/Runtime/GPUDriven/RenderersBatchersContext.cs
@@ -307,11 +307,11 @@ public void UpdateInstanceTransforms(NativeArray<InstanceHandle> instances, Nati
             ChangeInstanceBufferVersion();
         }
 
-        public void UpdateAmbientProbeAndGpuBuffer(SphericalHarmonicsL2 ambientProbe, bool forceUpdate = false)
+        public void UpdateAmbientProbeAndGpuBuffer(bool forceUpdate)
         {
-            if (m_CachedAmbientProbe != ambientProbe || forceUpdate)
+            if (forceUpdate || m_CachedAmbientProbe != RenderSettings.ambientProbe)
             {
-                m_CachedAmbientProbe = ambientProbe;
+                m_CachedAmbientProbe = RenderSettings.ambientProbe;
                 m_InstanceDataSystem.UpdateAllInstanceProbes(m_RenderersParameters, m_InstanceDataBuffer);
                 ChangeInstanceBufferVersion();
             }
diff --git a/Packages/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs b/Packages/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/HDRenderPipeline.cs
@@ -2360,11 +2360,7 @@ protected override void Render(ScriptableRenderContext renderContext, Camera[] c
 
                             var cmd = CommandBufferPool.Get("");
 
-                            GPUResidentDrawer.PostCullBeginCameraRendering(new RenderRequestBatcherContext
-                            {
-                                commandBuffer = cmd,
-                                ambientProbe = renderRequest.hdCamera.cameraFrameCount < 2 ? RenderSettings.ambientProbe : m_SkyManager.GetAmbientProbe(renderRequest.hdCamera)
-                            });
+                            GPUResidentDrawer.PostCullBeginCameraRendering(new RenderRequestBatcherContext { commandBuffer = cmd });
 
                             // The HDProbe store only one RenderData per probe, however RenderData can be view dependent (e.g. planar probes).
                             // To avoid that the render data for the wrong view is used, we previously store a copy of the render data
diff --git a/Packages/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs b/Packages/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs
@@ -708,6 +708,9 @@ internal void SetupAmbientProbe(HDCamera hdCamera)
             RenderSettings.ambientMode = AmbientMode.Custom; // Needed to specify ourselves the ambient probe (this will update internal ambient probe data passed to shaders)
             RenderSettings.ambientProbe = GetAmbientProbe(hdCamera);
 
+            // We need to inform GPUResidentDrawer that the ambient probe has been set up by a camera, so it can refresh the probe instance data on the GPU.
+            GPUResidentDrawer.OnSetupAmbientProbe();
+
             // If a camera just returns from being disabled, sky is not setup yet for it.
             if (hdCamera.lightingSky == null && hdCamera.skyAmbientMode == SkyAmbientMode.Dynamic)
             {
diff --git a/Packages/com.unity.render-pipelines.universal/Runtime/UniversalRenderPipeline.cs b/Packages/com.unity.render-pipelines.universal/Runtime/UniversalRenderPipeline.cs
@@ -778,7 +778,7 @@ static void RenderSingleCamera(ScriptableRenderContext context, UniversalCameraD
                 var data = frameData.Create<UniversalRenderingData>();
                 data.cullResults = context.Cull(ref cullingParameters);
 
-                GPUResidentDrawer.PostCullBeginCameraRendering(new RenderRequestBatcherContext { commandBuffer = cmd, ambientProbe = RenderSettings.ambientProbe });
+                GPUResidentDrawer.PostCullBeginCameraRendering(new RenderRequestBatcherContext { commandBuffer = cmd });
 
                 var isForwardPlus = cameraData.renderer is UniversalRenderer { renderingModeActual: RenderingMode.ForwardPlus };
 

Original file line number	Diff line number	Diff line change
`@@ -307,11 +307,11 @@ public void UpdateInstanceTransforms(NativeArray<InstanceHandle> instances, Nati`
`307`	`307`	`ChangeInstanceBufferVersion();`
`308`	`308`	`}`
`309`	`309`
`310`		`- public void UpdateAmbientProbeAndGpuBuffer(SphericalHarmonicsL2 ambientProbe, bool forceUpdate = false)`
	`310`	`+ public void UpdateAmbientProbeAndGpuBuffer(bool forceUpdate)`
`311`	`311`	`{`
`312`		`- if (m_CachedAmbientProbe != ambientProbe \|\| forceUpdate)`
	`312`	`+ if (forceUpdate \|\| m_CachedAmbientProbe != RenderSettings.ambientProbe)`
`313`	`313`	`{`
`314`		`- m_CachedAmbientProbe = ambientProbe;`
	`314`	`+ m_CachedAmbientProbe = RenderSettings.ambientProbe;`
`315`	`315`	`m_InstanceDataSystem.UpdateAllInstanceProbes(m_RenderersParameters, m_InstanceDataBuffer);`
`316`	`316`	`ChangeInstanceBufferVersion();`
`317`	`317`	`}`
Original file line number	Diff line number	Diff line change
`@@ -708,6 +708,9 @@ internal void SetupAmbientProbe(HDCamera hdCamera)`
`708`	`708`	`RenderSettings.ambientMode = AmbientMode.Custom; // Needed to specify ourselves the ambient probe (this will update internal ambient probe data passed to shaders)`
`709`	`709`	`RenderSettings.ambientProbe = GetAmbientProbe(hdCamera);`
`710`	`710`
	`711`	`+ // We need to inform GPUResidentDrawer that the ambient probe has been set up by a camera, so it can refresh the probe instance data on the GPU.`
	`712`	`+ GPUResidentDrawer.OnSetupAmbientProbe();`
	`713`	`+`
`711`	`714`	`// If a camera just returns from being disabled, sky is not setup yet for it.`
`712`	`715`	`if (hdCamera.lightingSky == null && hdCamera.skyAmbientMode == SkyAmbientMode.Dynamic)`
`713`	`716`	`{`