Merge pull request #548 from Unity-Technologies/p530-cherry-pick

Cherry picking #530

Author: Chman

CHANGELOG.md

@@ -4,6 +4,15 @@ All notable changes to this package will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Indev]
+
+### Fixed
+- On large scenes, the first object you'd add to a profile could throw a `NullReferenceException`. ([#530](https://github.com/Unity-Technologies/PostProcessing/pull/530))
+
+### Changed
+- Minor scripting API improvements. ([#530](https://github.com/Unity-Technologies/PostProcessing/pull/530))
+- Script-instantiated profiles in volumes are now properly supported in the inspector. ([#530](https://github.com/Unity-Technologies/PostProcessing/pull/530))
+
 ## [2.0.5-preview]
 
 ### Fixed

PostProcessing/Editor/EffectListEditor.cs

@@ -9,9 +9,9 @@ namespace UnityEditor.Rendering.PostProcessing
 {
     public sealed class EffectListEditor
     {
+        public PostProcessProfile asset { get; private set; }
         Editor m_BaseEditor;
 
-        PostProcessProfile m_Asset;
         SerializedObject m_SerializedObject;
         SerializedProperty m_SettingsProperty;
 
@@ -29,7 +29,7 @@ public void Init(PostProcessProfile asset, SerializedObject serializedObject)
             Assert.IsNotNull(asset);
             Assert.IsNotNull(serializedObject);
 
-            m_Asset = asset;
+            this.asset = asset;
             m_SerializedObject = serializedObject;
             m_SettingsProperty = serializedObject.FindProperty("settings");
             Assert.IsNotNull(m_SettingsProperty);
@@ -53,16 +53,16 @@ public void Init(PostProcessProfile asset, SerializedObject serializedObject)
             }
 
             // Create editors for existing settings
-            for (int i = 0; i < m_Asset.settings.Count; i++)
-                CreateEditor(m_Asset.settings[i], m_SettingsProperty.GetArrayElementAtIndex(i));
+            for (int i = 0; i < this.asset.settings.Count; i++)
+                CreateEditor(this.asset.settings[i], m_SettingsProperty.GetArrayElementAtIndex(i));
 
             // Keep track of undo/redo to redraw the inspector when that happens
             Undo.undoRedoPerformed += OnUndoRedoPerformed;
         }
 
         void OnUndoRedoPerformed()
         {
-            m_Asset.isDirty = true;
+            asset.isDirty = true;
 
             // Dumb hack to make sure the serialized object is up to date on undo (else there'll be
             // a state mismatch when this class is used in a GameObject inspector).
@@ -104,8 +104,8 @@ void RefreshEditors()
             m_Editors.Clear();
 
             // Recreate editors for existing settings, if any
-            for (int i = 0; i < m_Asset.settings.Count; i++)
-                CreateEditor(m_Asset.settings[i], m_SettingsProperty.GetArrayElementAtIndex(i));
+            for (int i = 0; i < asset.settings.Count; i++)
+                CreateEditor(asset.settings[i], m_SettingsProperty.GetArrayElementAtIndex(i));
         }
 
         public void Clear()
@@ -124,17 +124,17 @@ public void Clear()
 
         public void OnGUI()
         {
-            if (m_Asset == null)
+            if (asset == null)
                 return;
 
-            if (m_Asset.isDirty)
+            if (asset.isDirty)
             {
                 RefreshEditors();
-                m_Asset.isDirty = false;
+                asset.isDirty = false;
             }
 
             bool isEditable = !VersionControl.Provider.isActive
-                || AssetDatabase.IsOpenForEdit(m_Asset, StatusQueryOptions.UseCachedIfPossible);
+                || AssetDatabase.IsOpenForEdit(asset, StatusQueryOptions.UseCachedIfPossible);
 
             using (new EditorGUI.DisabledScope(!isEditable))
             {
@@ -183,7 +183,7 @@ public void OnGUI()
                     {
                         var type = kvp.Key;
                         var title = EditorUtilities.GetContent(kvp.Value.menuItem);
-                        bool exists = m_Asset.HasSettings(type);
+                        bool exists = asset.HasSettings(type);
 
                         if (!exists)
                             menu.AddItem(title, false, () => AddEffectOverride(type));
@@ -205,22 +205,26 @@ void AddEffectOverride(Type type)
             var effect = CreateNewEffect(type);
             Undo.RegisterCreatedObjectUndo(effect, "Add Effect Override");
 
-            // Store this new effect as a subasset so we can reference it safely afterwards
-            AssetDatabase.AddObjectToAsset(effect, m_Asset);
+            // Store this new effect as a subasset so we can reference it safely afterwards. Only when its not an instantiated profile
+            if (EditorUtility.IsPersistent(asset))
+                AssetDatabase.AddObjectToAsset(effect, asset);
 
             // Grow the list first, then add - that's how serialized lists work in Unity
             m_SettingsProperty.arraySize++;
             var effectProp = m_SettingsProperty.GetArrayElementAtIndex(m_SettingsProperty.arraySize - 1);
             effectProp.objectReferenceValue = effect;
 
-            // Force save / refresh
-            EditorUtility.SetDirty(m_Asset);
-            AssetDatabase.SaveAssets();
-
             // Create & store the internal editor object for this effect
             CreateEditor(effect, effectProp);
 
             m_SerializedObject.ApplyModifiedProperties();
+
+            // Force save / refresh. Important to do this last because SaveAssets can cause effect to become null!
+            if (EditorUtility.IsPersistent(asset))
+            {
+                EditorUtility.SetDirty(asset);
+                AssetDatabase.SaveAssets();
+            }
         }
 
         void RemoveEffectOverride(int id)
@@ -260,7 +264,7 @@ void RemoveEffectOverride(int id)
             Undo.DestroyObjectImmediate(effect);
 
             // Force save / refresh
-            EditorUtility.SetDirty(m_Asset);
+            EditorUtility.SetDirty(asset);
             AssetDatabase.SaveAssets();
         }
 
@@ -285,7 +289,7 @@ void ResetEffectOverride(Type type, int id)
             Undo.RegisterCreatedObjectUndo(newEffect, "Reset Effect Override");
 
             // Store this new effect as a subasset so we can reference it safely afterwards
-            AssetDatabase.AddObjectToAsset(newEffect, m_Asset);
+            AssetDatabase.AddObjectToAsset(newEffect, asset);
 
             // Put it in the reserved space
             property.objectReferenceValue = newEffect;
@@ -300,7 +304,7 @@ void ResetEffectOverride(Type type, int id)
             Undo.DestroyObjectImmediate(prevSettings);
 
             // Force save / refresh
-            EditorUtility.SetDirty(m_Asset);
+            EditorUtility.SetDirty(asset);
             AssetDatabase.SaveAssets();
         }
 

PostProcessing/Editor/PostProcessVolumeEditor.cs

@@ -68,18 +68,26 @@ public override void OnInspectorGUI()
             var buttonNewRect = new Rect(fieldRect.xMax, lineRect.y, buttonWidth, lineRect.height);
             var buttonCopyRect = new Rect(buttonNewRect.xMax, lineRect.y, buttonWidth, lineRect.height);
 
-            EditorGUI.PrefixLabel(labelRect, EditorUtilities.GetContent("Profile|A reference to a profile asset."));
+            EditorGUI.PrefixLabel(labelRect, EditorUtilities.GetContent(m_Target.HasInstantiatedProfile() ? "Profile (Instance)|A copy of a profile asset." : "Profile|A reference to a profile asset."));
 
             using (var scope = new EditorGUI.ChangeCheckScope())
             {
                 EditorGUI.BeginProperty(fieldRect, GUIContent.none, m_Profile);
+                PostProcessProfile profile = null;
 
-                var profile = (PostProcessProfile)EditorGUI.ObjectField(fieldRect, m_Profile.objectReferenceValue, typeof(PostProcessProfile), false);
+                if (m_Target.HasInstantiatedProfile())
+                    profile = (PostProcessProfile)EditorGUI.ObjectField(fieldRect, m_Target.profile, typeof(PostProcessProfile), false);
+                else
+                    profile = (PostProcessProfile)EditorGUI.ObjectField(fieldRect, m_Profile.objectReferenceValue, typeof(PostProcessProfile), false);
 
                 if (scope.changed)
                 {
                     assetHasChanged = true;
+
                     m_Profile.objectReferenceValue = profile;
+
+                    if (m_Target.HasInstantiatedProfile()) // Clear the instantiated profile, from now on we're using shared again.
+                        m_Target.profile = null;
                 }
 
                 EditorGUI.EndProperty();
@@ -95,14 +103,16 @@ public override void OnInspectorGUI()
                     var scene = m_Target.gameObject.scene;
                     var asset = ProfileFactory.CreatePostProcessProfile(scene, targetName);
                     m_Profile.objectReferenceValue = asset;
+                    m_Target.profile = null; // Make sure we're not using an instantiated profile anymore
+
                     assetHasChanged = true;
                 }
 
-                if (showCopy && GUI.Button(buttonCopyRect, EditorUtilities.GetContent("Clone|Create a new profile and copy the content of the currently assigned profile."), EditorStyles.miniButtonRight))
+                if (showCopy && GUI.Button(buttonCopyRect, EditorUtilities.GetContent(m_Target.HasInstantiatedProfile() ? "Save|Save the instantiated profile" : "Clone|Create a new profile and copy the content of the currently assigned profile."), EditorStyles.miniButtonRight))
                 {
                     // Duplicate the currently assigned profile and save it as a new profile
-                    var origin = (PostProcessProfile)m_Profile.objectReferenceValue;
-                    var path = AssetDatabase.GetAssetPath(origin);
+                    var origin = profileRef;
+                    var path = AssetDatabase.GetAssetPath(m_Profile.objectReferenceValue);
                     path = AssetDatabase.GenerateUniqueAssetPath(path);
 
                     var asset = Instantiate(origin);
@@ -122,13 +132,14 @@ public override void OnInspectorGUI()
                     AssetDatabase.Refresh();
 
                     m_Profile.objectReferenceValue = asset;
+                    m_Target.profile = null; // Make sure we're not using an instantiated profile anymore
                     assetHasChanged = true;
                 }
             }
 
             EditorGUILayout.Space();
 
-            if (m_Profile.objectReferenceValue == null)
+            if (m_Profile.objectReferenceValue == null && !m_Target.HasInstantiatedProfile())
             {
                 if (assetHasChanged)
                     m_EffectList.Clear(); // Asset wasn't null before, do some cleanup
@@ -137,14 +148,19 @@ public override void OnInspectorGUI()
             }
             else
             {
-                if (assetHasChanged)
-                    RefreshEffectListEditor((PostProcessProfile)m_Profile.objectReferenceValue);
+                if (assetHasChanged || profileRef != m_EffectList.asset) //Refresh when the user just dragged in a new asset, or when it was instantiated by code.
+                    RefreshEffectListEditor(profileRef);
 
                 if (!multiEdit)
                     m_EffectList.OnGUI();
             }
 
             serializedObject.ApplyModifiedProperties();
         }
+
+        public PostProcessProfile profileRef
+        {
+            get { return m_Target.HasInstantiatedProfile() ? m_Target.profile : m_Target.sharedProfile; }
+        }
     }
 } 

PostProcessing/Runtime/PostProcessProfile.cs

@@ -100,6 +100,16 @@ public bool HasSettings(Type type)
             return false;
         }
 
+        public T GetSetting<T>() where T : PostProcessEffectSettings
+        {
+            foreach (var setting in settings)
+            {
+                if (setting is T)
+                    return setting as T;
+            }
+            return null;
+        }
+
         public bool TryGetSettings<T>(out T outSetting)
             where T : PostProcessEffectSettings
         {

PostProcessing/Runtime/PostProcessVolume.cs

@@ -112,6 +112,11 @@ internal PostProcessProfile profileRef
             }
         }
 
+        public bool HasInstantiatedProfile()
+        {
+            return m_InternalProfile != null;
+        }
+
         int m_PreviousLayer;
         float m_PreviousPriority;
         List<Collider> m_TempColliders;

PostProcessing/Runtime/Utils/PropertySheetFactory.cs

@@ -14,7 +14,12 @@ public PropertySheetFactory()
 
         public PropertySheet Get(string shaderName)
         {
-            return Get(Shader.Find(shaderName));
+            var shader = Shader.Find(shaderName);
+
+            if (shader == null)
+                throw new ArgumentException(string.Format("Invalid shader ({0})", shaderName));
+
+            return Get(shader);
         }
 
         public PropertySheet Get(Shader shader)