From 2d72413127c7966ed92b629dd1078c1b00ae3a7e Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Thu, 3 Sep 2020 17:03:00 +0200 Subject: [PATCH 1/7] Sky lighting is no longer updated when the hash changed and the update mode is set to OnDemand or Realtime and hasn't passed the time threshold. --- .../Runtime/Sky/SkyManager.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs index b6c1199677d..4d7baac3fa1 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs @@ -735,6 +735,14 @@ public void UpdateEnvironment( HDCamera hdCamera, m_BuiltinParameters.skySettings = skyContext.skySettings; m_BuiltinParameters.cloudLayer = skyContext.cloudLayer; + if (IsCachedContextValid(skyContext) && !updateRequired && !staticSky) + { + if (skyContext.skySettings.updateMode == EnvironmentUpdateMode.OnDemand) + return; + else if (skyContext.skySettings.updateMode == EnvironmentUpdateMode.Realtime && skyContext.currentUpdateTime < skyContext.skySettings.updatePeriod.value) + return; + } + int skyHash = ComputeSkyHash(hdCamera, skyContext, sunLight, ambientMode, staticSky); bool forceUpdate = updateRequired; From 2cb89cc4e4de6183ab1187248f30acacdf54ed50 Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Thu, 3 Sep 2020 17:12:33 +0200 Subject: [PATCH 2/7] Added comment and fixed behavior of static sky. --- .../Runtime/Sky/SkyManager.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs index 4d7baac3fa1..2ec0821455f 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs @@ -735,7 +735,11 @@ public void UpdateEnvironment( HDCamera hdCamera, m_BuiltinParameters.skySettings = skyContext.skySettings; m_BuiltinParameters.cloudLayer = skyContext.cloudLayer; - if (IsCachedContextValid(skyContext) && !updateRequired && !staticSky) + // When update is not requested and the context is already valid (ie: already computed at least once), + // we need to early out in two cases: + // - updateMode is "OnDemand" in which case we never update unless explicitly requested + // - updateMode is "Realtime" in which case we only update if the time threshold for realtime update is passed. + if (IsCachedContextValid(skyContext) && !updateRequired) { if (skyContext.skySettings.updateMode == EnvironmentUpdateMode.OnDemand) return; @@ -761,6 +765,7 @@ public void UpdateEnvironment( HDCamera hdCamera, { using (new ProfilingScope(cmd, ProfilingSampler.Get(HDProfileId.UpdateSkyEnvironment))) { + Debug.Log("Update Sky Lighting"); RenderSkyToCubemap(skyContext); if (updateAmbientProbe) @@ -840,7 +845,7 @@ public void UpdateEnvironment(HDCamera hdCamera, ScriptableRenderContext renderC { m_StaticLightingSky.skySettings = staticLightingSky != null ? staticLightingSky.skySettings : null; m_StaticLightingSky.cloudLayer = staticLightingSky != null ? staticLightingSky.cloudLayer : null; - UpdateEnvironment(hdCamera, renderContext, m_StaticLightingSky, sunLight, m_StaticSkyUpdateRequired, true, true, SkyAmbientMode.Static, frameIndex, cmd); + UpdateEnvironment(hdCamera, renderContext, m_StaticLightingSky, sunLight, m_StaticSkyUpdateRequired || m_UpdateRequired, true, true, SkyAmbientMode.Static, frameIndex, cmd); m_StaticSkyUpdateRequired = false; } From ca6681ccb37e301c794637e2abf398918e773981 Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Fri, 4 Sep 2020 14:30:07 +0200 Subject: [PATCH 3/7] Fixed ambient flickering when a planar is in the scene and a preview is rendered (case https://fogbugz.unity3d.com/f/cases/1269557/) --- .../Runtime/Lighting/Reflection/HDProbe.cs | 3 +-- .../Runtime/RenderPipeline/Settings/FrameSettings.cs | 7 +++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/Runtime/Lighting/Reflection/HDProbe.cs b/com.unity.render-pipelines.high-definition/Runtime/Lighting/Reflection/HDProbe.cs index 6690a5a6d7d..111bf299026 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Lighting/Reflection/HDProbe.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Lighting/Reflection/HDProbe.cs @@ -497,8 +497,7 @@ internal void ForceRenderingNextUpdate() void UpdateProbeName() { - // TODO: ask if this is ok: - if (settings.type == ProbeSettings.ProbeType.PlanarProbe) + if (settings.type == ProbeSettings.ProbeType.ReflectionProbe) { for (int i = 0; i < 6; i++) probeName[i] = $"Reflection Probe RenderCamera ({name}: {(CubemapFace)i})"; diff --git a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs index 6f4de1759e7..3188cf68c95 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs @@ -393,7 +393,7 @@ partial struct FrameSettings (uint)FrameSettingsField.MotionVectors, // Enable/disable whole motion vectors pass (Camera + Object). (uint)FrameSettingsField.ObjectMotionVectors, (uint)FrameSettingsField.Decals, - (uint)FrameSettingsField.DecalLayers, + (uint)FrameSettingsField.DecalLayers, (uint)FrameSettingsField.Refraction, // Depends on DepthPyramid - If not enable, just do a copy of the scene color (?) - how to disable refraction ? (uint)FrameSettingsField.Distortion, (uint)FrameSettingsField.Postprocess, @@ -467,7 +467,7 @@ partial struct FrameSettings (uint)FrameSettingsField.MotionVectors, // Enable/disable whole motion vectors pass (Camera + Object). (uint)FrameSettingsField.ObjectMotionVectors, (uint)FrameSettingsField.Decals, - (uint)FrameSettingsField.DecalLayers, + (uint)FrameSettingsField.DecalLayers, //(uint)FrameSettingsField.Refraction, // Depends on DepthPyramid - If not enable, just do a copy of the scene color (?) - how to disable refraction ? //(uint)FrameSettingsField.Distortion, //(uint)FrameSettingsField.Postprocess, @@ -797,6 +797,9 @@ internal static void Sanitize(ref FrameSettings sanitizedFrameSettings, Camera c sanitizedFrameSettings.bitDatas[(int)FrameSettingsField.FPTLForForwardOpaque] &= !msaa; sanitizedFrameSettings.bitDatas[(int)FrameSettingsField.ProbeVolume] &= renderPipelineSettings.supportProbeVolume && (ShaderConfig.s_ProbeVolumesEvaluationMode != ProbeVolumesEvaluationModes.Disabled); + + sanitizedFrameSettings.bitDatas[(int)FrameSettingsField.ReflectionProbe] &= !preview; + sanitizedFrameSettings.bitDatas[(int)FrameSettingsField.PlanarProbe] &= !preview; } /// Aggregation is default with override of the renderer then sanitized depending on supported features of hdrpasset. From bfa685110c9fa2b714d3a0ef1956757e59cb3c86 Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Fri, 4 Sep 2020 15:44:51 +0200 Subject: [PATCH 4/7] Added comment --- .../Runtime/RenderPipeline/Settings/FrameSettings.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs index 3188cf68c95..b9950eedb66 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Settings/FrameSettings.cs @@ -798,6 +798,10 @@ internal static void Sanitize(ref FrameSettings sanitizedFrameSettings, Camera c sanitizedFrameSettings.bitDatas[(int)FrameSettingsField.ProbeVolume] &= renderPipelineSettings.supportProbeVolume && (ShaderConfig.s_ProbeVolumesEvaluationMode != ProbeVolumesEvaluationModes.Disabled); + // We disable reflection probes and planar reflections in regular preview rendering for two reasons. + // - Performance: Realtime reflection are 99% not necessary in previews + // - Static lighting consistency: When rendering a planar probe from a preview camera it may induce a recomputing of the static lighting + // but with the preview lights which are different from the ones in the scene and will change the result inducing flickering. sanitizedFrameSettings.bitDatas[(int)FrameSettingsField.ReflectionProbe] &= !preview; sanitizedFrameSettings.bitDatas[(int)FrameSettingsField.PlanarProbe] &= !preview; } From 84b0a5428c78ef544f24276aa2ca25496586ce1b Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Fri, 4 Sep 2020 16:08:11 +0200 Subject: [PATCH 5/7] Update changelog --- com.unity.render-pipelines.high-definition/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/com.unity.render-pipelines.high-definition/CHANGELOG.md b/com.unity.render-pipelines.high-definition/CHANGELOG.md index 84f5991dac9..b772ff06f44 100644 --- a/com.unity.render-pipelines.high-definition/CHANGELOG.md +++ b/com.unity.render-pipelines.high-definition/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fix several issues with physically-based DoF (TAA ghosting of the CoC buffer, smooth layer transitions, etc) - Fixed GPU hang on D3D12 on xbox. - Fix Amplitude -> Min/Max parametrization conversion +- Fixed a static lighting flickering issue caused by having an active planar probe in the scene while rendering inspector preview. +- Fixed an issue where even when set to OnDemand, the sky lighting would still be updated when changing sky parameters. ### Changed - Preparation pass for RTSSShadows to be supported by render graph. From 13659524cac18f809a6098b387831df058332faf Mon Sep 17 00:00:00 2001 From: Julien Ignace Date: Fri, 4 Sep 2020 16:20:05 +0200 Subject: [PATCH 6/7] Fixed GCAlloc --- .../Runtime/Sky/SkyManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs index 2ec0821455f..2896a26aa53 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs @@ -741,9 +741,9 @@ public void UpdateEnvironment( HDCamera hdCamera, // - updateMode is "Realtime" in which case we only update if the time threshold for realtime update is passed. if (IsCachedContextValid(skyContext) && !updateRequired) { - if (skyContext.skySettings.updateMode == EnvironmentUpdateMode.OnDemand) + if (skyContext.skySettings.updateMode.value == EnvironmentUpdateMode.OnDemand) return; - else if (skyContext.skySettings.updateMode == EnvironmentUpdateMode.Realtime && skyContext.currentUpdateTime < skyContext.skySettings.updatePeriod.value) + else if (skyContext.skySettings.updateMode.value == EnvironmentUpdateMode.Realtime && skyContext.currentUpdateTime < skyContext.skySettings.updatePeriod.value) return; } From 7fb86f4e9a959ef63d7d27e69e1d1c2d4a0f51fc Mon Sep 17 00:00:00 2001 From: sebastienlagarde Date: Tue, 8 Sep 2020 11:06:04 +0200 Subject: [PATCH 7/7] Update SkyManager.cs --- .../Runtime/Sky/SkyManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs index 2896a26aa53..cef35c4cedd 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs @@ -765,7 +765,7 @@ public void UpdateEnvironment( HDCamera hdCamera, { using (new ProfilingScope(cmd, ProfilingSampler.Get(HDProfileId.UpdateSkyEnvironment))) { - Debug.Log("Update Sky Lighting"); + // Debug.Log("Update Sky Lighting"); RenderSkyToCubemap(skyContext); if (updateAmbientProbe)