diff --git a/com.unity.render-pipelines.high-definition/CHANGELOG.md b/com.unity.render-pipelines.high-definition/CHANGELOG.md index 5e044da921b..15f71fafee8 100644 --- a/com.unity.render-pipelines.high-definition/CHANGELOG.md +++ b/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). diff --git a/com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionManagerEditor.cs b/com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionManagerEditor.cs index 610e834e067..88d8f7f2074 100644 --- a/com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionManagerEditor.cs +++ b/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(); } diff --git a/com.unity.render-pipelines.high-definition/Editor/Compositor/CompositorWindow.cs b/com.unity.render-pipelines.high-definition/Editor/Compositor/CompositorWindow.cs index 3dfc13bed21..c13d8306dbc 100644 --- a/com.unity.render-pipelines.high-definition/Editor/Compositor/CompositorWindow.cs +++ b/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(); } } diff --git a/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionLayer.cs b/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionLayer.cs index 8abee9d7c3e..eb6f8325536 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionLayer.cs +++ b/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionLayer.cs @@ -259,6 +259,7 @@ public void Init(string layerID = "") m_LayerCamera = newCameraGameObject.AddComponent(); newCameraGameObject.AddComponent(); 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; } diff --git a/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionManager.cs b/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionManager.cs index df1272d3647..76fb3cb8e7d 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositionManager.cs +++ b/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); } } + } } diff --git a/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositorCameraRegistry.cs b/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositorCameraRegistry.cs new file mode 100644 index 00000000000..a2b34c584cf --- /dev/null +++ b/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 s_CompositorManagedCameras = new List(); + 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 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(); + 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; + } + } + } +} diff --git a/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositorCameraRegistry.cs.meta b/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositorCameraRegistry.cs.meta new file mode 100644 index 00000000000..522b7596a79 --- /dev/null +++ b/com.unity.render-pipelines.high-definition/Runtime/Compositor/CompositorCameraRegistry.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0decceaaad943db43a6bfa36c3ad43f5 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: