diff --git a/com.unity.render-pipelines.high-definition/CHANGELOG.md b/com.unity.render-pipelines.high-definition/CHANGELOG.md index b6cb72b59b3..f83cd64187a 100644 --- a/com.unity.render-pipelines.high-definition/CHANGELOG.md +++ b/com.unity.render-pipelines.high-definition/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed an exception when opening the color picker in the material UI (case 1307143). - Fixed lights shadow frustum near and far planes. - Fixed various issues with non-temporal SSAO and rendergraph. +- Fix screen being over-exposed when changing very different skies. ### Changed - Removed the material pass probe volumes evaluation mode. diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/HDRISky/HDRISky.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/HDRISky/HDRISky.cs index 049d153968e..675e2dc6d82 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/HDRISky/HDRISky.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/HDRISky/HDRISky.cs @@ -169,6 +169,21 @@ public override int GetHashCode() return hash; } + /// + /// Determines if the SkySettings is significantly divergent from another. This is going to be used to determine whether + /// to reset completely the ambient probe instead of using previous one when waiting for current data upon changes. + /// In addition to the checks done with the base function, this HDRISky override checks whether the cubemap parameter + /// has changed if both settings are HDRISky. + /// + /// The settings to compare with. + /// Whether the settings are deemed very different. + public override bool SignificantlyDivergesFrom(SkySettings otherSettings) + { + HDRISky otherHdriSkySettings = otherSettings as HDRISky; + + return base.SignificantlyDivergesFrom(otherSettings) || hdriSky.value != otherHdriSkySettings.hdriSky.value; + } + /// /// Returns HDRISkyRenderer type. /// 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 a4fb5ab9d55..a01f822f74f 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyManager.cs @@ -617,6 +617,11 @@ void AllocateNewRenderingContext(SkyUpdateContext skyContext, int slot, int newH if (context.renderingContext == null) context.renderingContext = new SkyRenderingContext(m_Resolution, m_IBLFilterArray.Length, supportConvolution, previousAmbientProbe, name); + // If we detected a big difference with previous settings, then carrying over the previous ambient probe is probably going to lead to unexpected result. + // Instead we at least fallback to a neutral one until async readback has finished. + if (skyContext.settingsHadBigDifferenceWithPrev) + context.renderingContext.ClearAmbientProbe(); + skyContext.cachedSkyRenderingContextId = slot; } diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyRenderer.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyRenderer.cs index 2e7d3fb760a..203ea852c1b 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyRenderer.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyRenderer.cs @@ -70,24 +70,7 @@ public virtual void PreRenderSky(BuiltinSkyParameters builtinParams) {} /// Returns SkySetting exposure. protected static float GetSkyIntensity(SkySettings skySettings, DebugDisplaySettings debugSettings) { - float skyIntensity = 1.0f; - - switch (skySettings.skyIntensityMode.value) - { - case SkyIntensityMode.Exposure: - // Note: Here we use EV100 of sky as a multiplier, so it is the opposite of when use with a Camera - // because for sky/light, higher EV mean brighter, but for camera higher EV mean darker scene - skyIntensity *= ColorUtils.ConvertEV100ToExposure(-skySettings.exposure.value); - break; - case SkyIntensityMode.Multiplier: - skyIntensity *= skySettings.multiplier.value; - break; - case SkyIntensityMode.Lux: - skyIntensity *= skySettings.desiredLuxValue.value / skySettings.upperHemisphereLuxValue.value; - break; - } - - return skyIntensity; + return skySettings.GetIntensityFromSettings(); } /// diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkySettings.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkySettings.cs index bd2980b3ba7..7cc23422ac1 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkySettings.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkySettings.cs @@ -217,6 +217,51 @@ public static int GetUniqueID(Type type) return uniqueID; } + /// + /// Returns the sky intensity as determined by this SkySetting. + /// + /// The sky intensity. + public float GetIntensityFromSettings() + { + float skyIntensity = 1.0f; + switch (skyIntensityMode.value) + { + case SkyIntensityMode.Exposure: + // Note: Here we use EV100 of sky as a multiplier, so it is the opposite of when use with a Camera + // because for sky/light, higher EV mean brighter, but for camera higher EV mean darker scene + skyIntensity *= ColorUtils.ConvertEV100ToExposure(-exposure.value); + break; + case SkyIntensityMode.Multiplier: + skyIntensity *= multiplier.value; + break; + case SkyIntensityMode.Lux: + skyIntensity *= desiredLuxValue.value / upperHemisphereLuxValue.value; + break; + } + return skyIntensity; + } + + /// + /// Determines if the SkySettings is significantly divergent from another. This is going to be used to determine whether + /// to reset completely the ambient probe instead of using previous one when waiting for current data upon changes. + /// Override this to have a per-sky specific heuristic. + /// + /// The settings to compare with. + /// Whether the settings are deemed very different. + public virtual bool SignificantlyDivergesFrom(SkySettings otherSettings) + { + if (otherSettings.GetSkyRendererType() != GetSkyRendererType()) + return true; + + float thisIntensity = GetIntensityFromSettings(); + float otherIntensity = otherSettings.GetIntensityFromSettings(); + + // This is an arbitrary difference threshold. This needs to be re-evaluated in case it is proven problematic + float intensityRatio = thisIntensity > otherIntensity ? (thisIntensity / otherIntensity) : (otherIntensity / thisIntensity); + const float ratioThreshold = 3.0f; + return intensityRatio > ratioThreshold; + } + /// /// Returns the class type of the SkyRenderer associated with this Sky Settings. /// diff --git a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyUpdateContext.cs b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyUpdateContext.cs index a8b57c6c2f9..ab87d228197 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyUpdateContext.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Sky/SkyUpdateContext.cs @@ -14,6 +14,8 @@ internal class SkyUpdateContext public int skyParametersHash = -1; public float currentUpdateTime = 0.0f; + public bool settingsHadBigDifferenceWithPrev { get; private set; } + public SkySettings skySettings { get { return m_SkySettings; } @@ -28,6 +30,11 @@ public SkySettings skySettings skyRenderer = null; } + if (m_SkySettings == null) + settingsHadBigDifferenceWithPrev = true; + else + settingsHadBigDifferenceWithPrev = m_SkySettings.SignificantlyDivergesFrom(value); + if (m_SkySettings == value) return;