From 817761ba03e243de1e999daac8914abe3da6eb9e Mon Sep 17 00:00:00 2001 From: Pavlos Mavridis Date: Tue, 12 May 2020 14:03:50 +0200 Subject: [PATCH 1/5] Fix issue when switching back to custom sensor type in physical camera --- .../CHANGELOG.md | 1 + .../Camera/HDCameraUI.Drawers.cs | 38 +++++++++++++++---- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/CHANGELOG.md b/com.unity.render-pipelines.high-definition/CHANGELOG.md index e6d59ed0021..c7984a6ad58 100644 --- a/com.unity.render-pipelines.high-definition/CHANGELOG.md +++ b/com.unity.render-pipelines.high-definition/CHANGELOG.md @@ -581,6 +581,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed flickering of the game/scene view when lookdev is running. - Fixed issue with reflection probes in realtime time mode with OnEnable baking having wrong lighting with sky set to dynamic (case 1238047). - Fixed transparent motion vectors not working when in MSAA. +- Fixed issue when switching back to custom sensor type in physical camera settings (case 1244350). ### Changed - Improve MIP selection for decals on Transparents diff --git a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs index f2db83088ee..ed58467ac25 100644 --- a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs +++ b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs @@ -62,6 +62,8 @@ enum ShutterSpeedUnit "Custom" }; + static readonly int k_CustomPresetIndex = k_ApertureFormatNames.Length - 1; + static readonly Vector2[] k_ApertureFormatValues = { new Vector2(4.8f, 3.5f), @@ -76,6 +78,9 @@ enum ShutterSpeedUnit new Vector2(70.41f, 52.63f) }; + // Saves the value of the sensor size when the user switches from "custom" size to a preset. + static Vector2 s_LastCustomSensorSizeValue; + static bool s_FovChanged; static float s_FovLastValue; @@ -298,14 +303,33 @@ static void Drawer_PhysicalCamera(SerializedHDCamera p, Editor owner) using (new EditorGUI.IndentLevelScope()) { EditorGUI.BeginChangeCheck(); - int filmGateIndex = Array.IndexOf(k_ApertureFormatValues, new Vector2((float)Math.Round(cam.sensorSize.vector2Value.x, 3), (float)Math.Round(cam.sensorSize.vector2Value.y, 3))); - if (filmGateIndex == -1) - filmGateIndex = EditorGUILayout.Popup(cameraTypeContent, k_ApertureFormatNames.Length - 1, k_ApertureFormatNames); - else - filmGateIndex = EditorGUILayout.Popup(cameraTypeContent, filmGateIndex, k_ApertureFormatNames); - if (EditorGUI.EndChangeCheck() && filmGateIndex < k_ApertureFormatValues.Length) - cam.sensorSize.vector2Value = k_ApertureFormatValues[filmGateIndex]; + int oldFilmGateIndex = Array.IndexOf(k_ApertureFormatValues, new Vector2((float)Math.Round(cam.sensorSize.vector2Value.x, 3), (float)Math.Round(cam.sensorSize.vector2Value.y, 3))); + + // If it is not one of the preset sizes, set it to custom + oldFilmGateIndex = (oldFilmGateIndex == -1) ? k_CustomPresetIndex: oldFilmGateIndex; + + // Get the new user selection + int newFilmGateIndex = EditorGUILayout.Popup(cameraTypeContent, oldFilmGateIndex, k_ApertureFormatNames); + + if (EditorGUI.EndChangeCheck()) + { + // When switching from custom to a preset, save the last custom value (to display again, in case the user switches back to custom) + if (oldFilmGateIndex == k_CustomPresetIndex) + { + s_LastCustomSensorSizeValue = cam.sensorSize.vector2Value; + } + + if (newFilmGateIndex < k_ApertureFormatValues.Length) + { + cam.sensorSize.vector2Value = k_ApertureFormatValues[newFilmGateIndex]; + } + else + { + // The user switched back to custom, so display the old value + cam.sensorSize.vector2Value = s_LastCustomSensorSizeValue; + } + } EditorGUILayout.PropertyField(cam.sensorSize, sensorSizeContent); EditorGUILayout.PropertyField(p.iso, isoContent); From 555c387a6734f0cb5a0e2e2b2c9c4b68eb3bf8ff Mon Sep 17 00:00:00 2001 From: Pavlos Mavridis Date: Tue, 12 May 2020 15:07:46 +0200 Subject: [PATCH 2/5] make if comparison more explicit --- .../Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs index ed58467ac25..01656c2d11c 100644 --- a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs +++ b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs @@ -320,7 +320,7 @@ static void Drawer_PhysicalCamera(SerializedHDCamera p, Editor owner) s_LastCustomSensorSizeValue = cam.sensorSize.vector2Value; } - if (newFilmGateIndex < k_ApertureFormatValues.Length) + if (newFilmGateIndex != k_CustomPresetIndex) { cam.sensorSize.vector2Value = k_ApertureFormatValues[newFilmGateIndex]; } From 4904304dd5aa2cf92c544707d05115d10f1a2fc7 Mon Sep 17 00:00:00 2001 From: Pavlos Mavridis Date: Tue, 12 May 2020 18:08:24 +0200 Subject: [PATCH 3/5] make if comparison more safe --- .../Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs index 01656c2d11c..e871f4eb8aa 100644 --- a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs +++ b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs @@ -320,7 +320,7 @@ static void Drawer_PhysicalCamera(SerializedHDCamera p, Editor owner) s_LastCustomSensorSizeValue = cam.sensorSize.vector2Value; } - if (newFilmGateIndex != k_CustomPresetIndex) + if (newFilmGateIndex < k_CustomPresetIndex) { cam.sensorSize.vector2Value = k_ApertureFormatValues[newFilmGateIndex]; } From 125c218a6e994cc5f4036289f6da557be1aaac4b Mon Sep 17 00:00:00 2001 From: Pavlos Mavridis Date: Tue, 12 May 2020 19:05:45 +0200 Subject: [PATCH 4/5] Per-camera history for the custom settings using a static dictionary --- .../Camera/HDCameraUI.Drawers.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs index e871f4eb8aa..235434eeceb 100644 --- a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs +++ b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Reflection; +using System.Collections.Generic; using UnityEngine; using UnityEngine.Rendering.HighDefinition; using UnityEngine.Rendering; @@ -78,8 +79,8 @@ enum ShutterSpeedUnit new Vector2(70.41f, 52.63f) }; - // Saves the value of the sensor size when the user switches from "custom" size to a preset. - static Vector2 s_LastCustomSensorSizeValue; + // Saves the value of the sensor size when the user switches from "custom" size to a preset per camera. + static Dictionary s_PerCameraSensorSizeHistory = new Dictionary(); static bool s_FovChanged; static float s_FovLastValue; @@ -317,7 +318,7 @@ static void Drawer_PhysicalCamera(SerializedHDCamera p, Editor owner) // When switching from custom to a preset, save the last custom value (to display again, in case the user switches back to custom) if (oldFilmGateIndex == k_CustomPresetIndex) { - s_LastCustomSensorSizeValue = cam.sensorSize.vector2Value; + s_PerCameraSensorSizeHistory[(Camera)p.serializedObject.targetObject] = cam.sensorSize.vector2Value; } if (newFilmGateIndex < k_CustomPresetIndex) @@ -326,8 +327,16 @@ static void Drawer_PhysicalCamera(SerializedHDCamera p, Editor owner) } else { - // The user switched back to custom, so display the old value - cam.sensorSize.vector2Value = s_LastCustomSensorSizeValue; + // The user switched back to custom, so display the old value, if we have a valid history for this camera + Vector2 defaultValue; + if (s_PerCameraSensorSizeHistory.TryGetValue((Camera)p.serializedObject.targetObject, out defaultValue)) + { + cam.sensorSize.vector2Value = defaultValue; + } + else + { + cam.sensorSize.vector2Value = new Vector2(36.0f, 24.0f); // this is the value new cameras are created with + } } } From 098f94da147c43983d49cf9076c7d1e0a33d7411 Mon Sep 17 00:00:00 2001 From: Pavlos Mavridis Date: Tue, 12 May 2020 21:16:54 +0200 Subject: [PATCH 5/5] Use ConditionalWeakTable instead of a Dictionary to avoid keeping alive deleted cameras --- .../Camera/HDCameraUI.Drawers.cs | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs index 235434eeceb..c895f09020c 100644 --- a/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs +++ b/com.unity.render-pipelines.high-definition/Editor/RenderPipeline/Camera/HDCameraUI.Drawers.cs @@ -1,7 +1,7 @@ using System; using System.Linq; using System.Reflection; -using System.Collections.Generic; +using System.Runtime.CompilerServices; using UnityEngine; using UnityEngine.Rendering.HighDefinition; using UnityEngine.Rendering; @@ -80,7 +80,8 @@ enum ShutterSpeedUnit }; // Saves the value of the sensor size when the user switches from "custom" size to a preset per camera. - static Dictionary s_PerCameraSensorSizeHistory = new Dictionary(); + // We use a ConditionalWeakTable instead of a Dictionary to avoid keeping alive (with strong references) deleted cameras + static ConditionalWeakTable s_PerCameraSensorSizeHistory = new ConditionalWeakTable(); static bool s_FovChanged; static float s_FovLastValue; @@ -315,10 +316,21 @@ static void Drawer_PhysicalCamera(SerializedHDCamera p, Editor owner) if (EditorGUI.EndChangeCheck()) { - // When switching from custom to a preset, save the last custom value (to display again, in case the user switches back to custom) + // Retrieve the previous custom size value, if one exists for this camera + object previousCustomValue; + s_PerCameraSensorSizeHistory.TryGetValue((Camera)p.serializedObject.targetObject, out previousCustomValue); + + // When switching from custom to a preset, update the last custom value (to display again, in case the user switches back to custom) if (oldFilmGateIndex == k_CustomPresetIndex) { - s_PerCameraSensorSizeHistory[(Camera)p.serializedObject.targetObject] = cam.sensorSize.vector2Value; + if (previousCustomValue == null) + { + s_PerCameraSensorSizeHistory.Add((Camera)p.serializedObject.targetObject, cam.sensorSize.vector2Value); + } + else + { + previousCustomValue = cam.sensorSize.vector2Value; + } } if (newFilmGateIndex < k_CustomPresetIndex) @@ -327,11 +339,10 @@ static void Drawer_PhysicalCamera(SerializedHDCamera p, Editor owner) } else { - // The user switched back to custom, so display the old value, if we have a valid history for this camera - Vector2 defaultValue; - if (s_PerCameraSensorSizeHistory.TryGetValue((Camera)p.serializedObject.targetObject, out defaultValue)) + // The user switched back to custom, so display by deafulr the previous custom value + if (previousCustomValue != null) { - cam.sensorSize.vector2Value = defaultValue; + cam.sensorSize.vector2Value = (Vector2)previousCustomValue; } else {