Fix issue when switching back to custom sensor type in physical camera (#417)

* Fix issue when switching back to custom sensor type in physical camera

* make if comparison more explicit

* make if comparison more safe

* Per-camera history for the custom settings using a static dictionary

* Use ConditionalWeakTable instead of a Dictionary to avoid keeping alive deleted cameras

Co-authored-by: sebastienlagarde <[email protected]>

Author: pmavridis

com.unity.render-pipelines.high-definition/CHANGELOG.md

@@ -590,6 +590,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fix conflicts with Handles manipulation when performing a Reset in DecalComponent (case 1238833)
 - Fixed depth prepass and postpass being disabled after changing the shader in the material UI.
 - Fixed issue with sceneview camera settings not being saved after Editor restart.
+- Fixed issue when switching back to custom sensor type in physical camera settings (case 1244350).
 
 ### Changed
 - Improve MIP selection for decals on Transparents

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.Runtime.CompilerServices;
 using UnityEngine;
 using UnityEngine.Rendering.HighDefinition;
 using UnityEngine.Rendering;
@@ -62,6 +63,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 +79,10 @@ 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 per camera.
+        // We use a ConditionalWeakTable instead of a Dictionary to avoid keeping alive (with strong references) deleted cameras
+        static ConditionalWeakTable<Camera, object> s_PerCameraSensorSizeHistory = new ConditionalWeakTable<Camera, object>();
+
         static bool s_FovChanged;
         static float s_FovLastValue;
 
@@ -298,14 +305,51 @@ 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())
+                {
+                    // 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)
+                    {
+                        if (previousCustomValue == null)
+                        {
+                            s_PerCameraSensorSizeHistory.Add((Camera)p.serializedObject.targetObject, cam.sensorSize.vector2Value);
+                        }
+                        else
+                        {
+                            previousCustomValue = cam.sensorSize.vector2Value;
+                        }
+                    }
+
+                    if (newFilmGateIndex < k_CustomPresetIndex)
+                    {
+                        cam.sensorSize.vector2Value = k_ApertureFormatValues[newFilmGateIndex];
+                    }
+                    else
+                    {
+                        // The user switched back to custom, so display by deafulr the previous custom value
+                        if (previousCustomValue != null)
+                        {
+                            cam.sensorSize.vector2Value = (Vector2)previousCustomValue;
+                        }
+                        else
+                        {
+                            cam.sensorSize.vector2Value = new Vector2(36.0f, 24.0f); // this is the value new cameras are created with
+                        }
+                    }
+                }
 
                 EditorGUILayout.PropertyField(cam.sensorSize, sensorSizeContent);
                 EditorGUILayout.PropertyField(p.iso, isoContent);