[HDRP][Compositor] Fix issues with compositor's undo

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

@@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fixed light gizmo showing shadow near plane when shadows are disabled.
 - Fixed path tracing alpha channel support (case 1304187).
 - Fixed shadow matte not working with ambient occlusion when MSAA is enabled
+- Fixed issues with compositor's undo (cases 1305633, 1307170).
 
 ### Changed
 - Change the source value for the ray tracing frame index iterator from m_FrameCount to the camera frame count (case 1301356).

com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionManagerEditor.cs

@@ -49,13 +49,22 @@ static partial class Styles
         public bool isDirty => m_IsEditorDirty;
 
         public int defaultSelection = -1;
-        public int selectionIndex => m_layerList != null ? m_layerList.index : -1;
+        public int selectionIndex
+        {
+            get => m_layerList != null ? m_layerList.index : -1;
+            set
+            {
+                if (m_layerList != null) m_layerList.index = Math.Min(value, m_layerList.count - 1);
+            }
+        }
+
 
         void AddLayerOfTypeCallback(object type)
         {
             Undo.RecordObject(m_compositionManager, "Add compositor sublayer");
             m_compositionManager.AddNewLayer(m_layerList.index + 1, (CompositorLayer.LayerType)type);
             m_SerializedProperties.layerList.serializedObject.Update();
+            m_compositionManager.DeleteLayerRTs();
             m_compositionManager.UpdateLayerSetup();
         }
 

com.unity.render-pipelines.high-definition/Editor/Compositor/CompositorWindow.cs

@@ -114,13 +114,13 @@ void OnGUI()
                 Undo.RegisterCreatedObjectUndo(compositor.outputCamera.gameObject, "Create Compositor");
                 Undo.RegisterCreatedObjectUndo(go, "Create Compositor");
             }
-            else if (compositor)
+            else if (compositor && (compositor.enabled != enableCompositor))
             {
                 string message = enableCompositor ? "Enable Compositor" : "Disable Compositor";
                 Undo.RecordObject(compositor, message);
                 compositor.enabled = enableCompositor;
             }
-            else
+            else if (!compositor)
             {
                 return;
             }
@@ -178,6 +178,9 @@ void OnGUI()
                 if (m_Editor)
                 {
                     m_Editor.OnInspectorGUI();
+
+                    // Remember which layer was selected / drawn in the last draw call
+                    s_SelectionIndex = m_Editor.selectionIndex;
                 }
             }
             GUILayout.EndScrollView();
@@ -214,7 +217,11 @@ void UndoCallback()
 
             m_Editor.CacheSerializedObjects();
             m_RequiresRedraw = true;
-            s_SelectionIndex = m_Editor.selectionIndex;
+
+            // After undo, set the selection index to the last shown layer, because the Unity Editor resets the value to the last layer in the list
+            m_Editor.defaultSelection = s_SelectionIndex;
+            m_Editor.selectionIndex = s_SelectionIndex;
+
 
             CompositionManager compositor = CompositionManager.GetInstance();
             // The compositor might be null even if the CompositionManagerEditor is not (in case the user switches from a scene with a compositor to a scene without one)
@@ -223,6 +230,10 @@ void UndoCallback()
                 // Some properties were changed, mark the profile as dirty so it can be saved if the user saves the scene
                 EditorUtility.SetDirty(compositor);
                 EditorUtility.SetDirty(compositor.profile);
+
+                // Clean-up existing cameras after undo, we will re-allocate the layer resources
+                CompositorCameraRegistry.GetInstance().CleanUpCameraOrphans(compositor.layers);
+                compositor.DeleteLayerRTs();
                 compositor.UpdateLayerSetup();
             }
         }

com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionLayer.cs

@@ -259,6 +259,7 @@ public void Init(string layerID = "")
                     m_LayerCamera = newCameraGameObject.AddComponent<Camera>();
                     newCameraGameObject.AddComponent<HDAdditionalCameraData>();
                     CopyInternalCameraData();
+                    CompositorCameraRegistry.GetInstance().RegisterInternalCamera(m_LayerCamera);
 
                     m_LayerCamera.name = "Compositor" + layerID;
                     m_LayerCamera.gameObject.hideFlags = HideFlags.HideInInspector | HideFlags.HideInHierarchy | HideFlags.HideAndDontSave;
@@ -421,6 +422,7 @@ public void DestroyCameras()
                         CoreUtils.Destroy(cameraData);
                     }
                     m_LayerCamera.targetTexture = null;
+                    CompositorCameraRegistry.GetInstance().UnregisterInternalCamera(m_LayerCamera);
                     CoreUtils.Destroy(m_LayerCamera);
                     m_LayerCamera = null;
                 }

com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionManager.cs

@@ -555,6 +555,9 @@ void OnDestroy()
             // We don't need the custom passes anymore
             var hdPipeline = RenderPipelineManager.currentPipeline as HDRenderPipeline;
             UnRegisterCustomPasses(hdPipeline);
+
+            // By now the s_CompositorManagedCameras should be empty, but clear it just to be safe
+            CompositorCameraRegistry.GetInstance().CleanUpCameraOrphans();
         }
 
         public void AddInputFilterAtLayer(CompositionFilter filter, int index)
@@ -962,5 +965,6 @@ static internal void UnRegisterCustomPasses(HDRenderPipeline hdPipeline)
                 hdPipeline.asset.beforePostProcessCustomPostProcesses.Remove(typeof(AlphaInjection).AssemblyQualifiedName);
             }
         }
+
     }
 }

com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositorCameraRegistry.cs

@@ -0,0 +1,80 @@
+using System.Collections.Generic;
+using UnityEngine.Rendering;
+using UnityEngine.Rendering.HighDefinition;
+
+namespace UnityEngine.Rendering.HighDefinition.Compositor
+{
+    // Internal class to keep track of compositor allocated cameras.
+    // Required to properly manage cameras that are deleted or "ressurected" by undo/redo operations.
+    class CompositorCameraRegistry
+    {
+        static List<Camera> s_CompositorManagedCameras = new List<Camera>();
+        static private CompositorCameraRegistry s_CompositorCameraRegistry;
+        static public CompositorCameraRegistry GetInstance() =>
+            s_CompositorCameraRegistry ?? (s_CompositorCameraRegistry = new CompositorCameraRegistry());
+
+        // Keeps track of compositor allocated cameras
+        internal void RegisterInternalCamera(Camera camera)
+        {
+            s_CompositorManagedCameras.Add(camera);
+        }
+        internal void UnregisterInternalCamera(Camera camera)
+        {
+            s_CompositorManagedCameras.Remove(camera);
+        }
+
+        // Checks for any compositor allocated cameras that are now unused and frees their resources. 
+        internal void CleanUpCameraOrphans(List<CompositorLayer> layers = null)
+        {
+            s_CompositorManagedCameras.RemoveAll(x => x == null);
+
+            for (int i = s_CompositorManagedCameras.Count - 1; i >= 0; i--)
+            {
+                bool found = false;
+                if (layers != null)
+                {
+                    foreach (var layer in layers)
+                    {
+                        if (s_CompositorManagedCameras[i].Equals(layer.camera))
+                        {
+                            found = true;
+                            break;
+                        }
+                    }
+                }
+
+                // If the camera is not used by any layer anymore, then destroy it
+                if (found == false && s_CompositorManagedCameras[i] != null)
+                {
+                    var cameraData = s_CompositorManagedCameras[i].GetComponent<HDAdditionalCameraData>();
+                    if (cameraData)
+                    {
+                        CoreUtils.Destroy(cameraData);
+                    }
+                    s_CompositorManagedCameras[i].targetTexture = null;
+                    CoreUtils.Destroy(s_CompositorManagedCameras[i]);
+                    s_CompositorManagedCameras.RemoveAt(i);
+                }
+            }
+
+            if (layers != null)
+            {
+                foreach (var layer in layers)
+                {
+                    if (layer != null && !s_CompositorManagedCameras.Contains(layer.camera))
+                    {
+                        s_CompositorManagedCameras.Add(layer.camera);
+                    }
+                }
+            }
+        }
+
+        internal void PrinCameraIDs()
+        {
+            for (int i = s_CompositorManagedCameras.Count - 1; i >= 0; i--)
+            {
+                var id = s_CompositorManagedCameras[i] ? s_CompositorManagedCameras[i].GetInstanceID() : 0;
+            }
+        }
+    }
+}