[9.x.x] Internal Inspector

com.unity.shadergraph/Editor/Data/Attributes/SGPropertyDrawerAttribute.cs

@@ -0,0 +1,15 @@
+using System;
+
+namespace UnityEditor.ShaderGraph.Drawing
+{
+    [AttributeUsage(AttributeTargets.Class)]
+    public class SGPropertyDrawerAttribute : Attribute
+    {
+        public Type propertyType { get; private set; }
+
+        public SGPropertyDrawerAttribute(Type propertyType)
+        {
+            this.propertyType = propertyType;
+        }
+    }
+}

com.unity.shadergraph/Editor/Data/Interfaces/IInspectable.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Reflection;
+using Data.Interfaces;
 using Drawing.Inspector;
 using UnityEngine.UIElements;
 

com.unity.shadergraph/Editor/Data/Interfaces/IPropertyDrawer.cs

@@ -0,0 +1,15 @@
+using System;
+using System.Reflection;
+using UnityEditor.ShaderGraph.Drawing;
+using UnityEngine.UIElements;
+
+namespace Data.Interfaces
+{
+    // Interface that should be implemented by any property drawer for the inspector view
+    public interface IPropertyDrawer
+    {
+        Action inspectorUpdateDelegate { get; set; }
+
+        VisualElement DrawProperty(PropertyInfo propertyInfo, object actualObject, InspectableAttribute attribute);
+    }
+}

com.unity.shadergraph/Editor/Drawing/Blackboard/BlackboardFieldView.cs

@@ -1,18 +1,16 @@
 using System;
 using System.Linq;
-using System.Collections.Generic;
 using System.Reflection;
-using System.Text.RegularExpressions;
+using Data.Interfaces;
 using Drawing.Inspector;
+using Drawing.Inspector.PropertyDrawers;
 using UnityEngine;
 using UnityEngine.UIElements;
-using UnityEditor.UIElements;
 using UnityEditor.Graphing;
 using UnityEditor.Graphing.Util;
 using UnityEditor.Experimental.GraphView;
 using UnityEditor.ShaderGraph.Drawing.Controls;
 using UnityEditor.ShaderGraph.Internal;
-using UnityEditorInternal;
 
 namespace UnityEditor.ShaderGraph.Drawing
 {
@@ -36,8 +34,11 @@ private void DirtyNodes(ModificationScope modificationScope = ModificationScope.
             switch(m_Input)
             {
                 case AbstractShaderProperty property:
-                    var colorManager = GetFirstAncestorOfType<GraphEditorView>().colorManager;
-                    var nodes = GetFirstAncestorOfType<GraphEditorView>().graphView.Query<MaterialNodeView>().ToList();
+                    var graphEditorView = GetFirstAncestorOfType<GraphEditorView>();
+                    if(graphEditorView == null)
+                        return;
+                    var colorManager = graphEditorView.colorManager;
+                    var nodes = graphEditorView.graphView.Query<MaterialNodeView>().ToList();
 
                     colorManager.SetNodesDirty(nodes);
                     colorManager.UpdateNodeViews(nodes);
@@ -116,7 +117,7 @@ void BuildContextualMenu(ContextualMenuPopulateEvent evt)
             evt.menu.AppendAction("Reset Reference", e =>
                 {
                     m_Input.overrideReferenceName = null;
-                    this.m_resetReferenceNameTrigger(shaderInput.referenceName);
+                    m_resetReferenceNameTrigger(shaderInput.referenceName);
                     DirtyNodes(ModificationScope.Graph);
                 }, DropdownMenuAction.AlwaysEnabled);
         }
@@ -127,22 +128,22 @@ public void SupplyDataToPropertyDrawer(IPropertyDrawer propertyDrawer, Action in
             if(propertyDrawer is ShaderInputPropertyDrawer shaderInputPropertyDrawer)
             {
                 shaderInputPropertyDrawer.GetPropertyData(m_Graph.isSubGraph,
-                    this.ChangeExposedField,
-                    this.ChangeReferenceNameField,
+                    ChangeExposedField,
+                    ChangeReferenceNameField,
                     () => m_Graph.ValidateGraph(),
                     () => m_Graph.OnKeywordChanged(),
-                    this.ChangePropertyValue,
-                    this.RegisterPropertyChangeUndo,
-                    this.MarkNodesAsDirty);
+                    ChangePropertyValue,
+                    RegisterPropertyChangeUndo,
+                    MarkNodesAsDirty);
 
-                this.m_propertyViewUpdateTrigger = inspectorUpdateDelegate;
-                this.m_resetReferenceNameTrigger = shaderInputPropertyDrawer._resetReferenceNameCallback;
+                m_propertyViewUpdateTrigger = inspectorUpdateDelegate;
+                m_resetReferenceNameTrigger = shaderInputPropertyDrawer._resetReferenceNameCallback;
             }
         }
 
         public PropertyInfo[] GetPropertyInfo()
         {
-            return this.GetType().GetProperties();
+            return GetType().GetProperties();
         }
 
         void ChangeExposedField(bool newValue)
@@ -168,7 +169,7 @@ void MarkNodesAsDirty(bool triggerPropertyViewUpdate = false, ModificationScope
         {
             DirtyNodes(modificationScope);
             if(triggerPropertyViewUpdate)
-                this.m_propertyViewUpdateTrigger();
+                m_propertyViewUpdateTrigger();
         }
 
         void ChangePropertyValue(object newValue)
@@ -228,7 +229,7 @@ void ChangePropertyValue(object newValue)
                     throw new ArgumentOutOfRangeException();
             }
 
-            this.MarkDirtyRepaint();
+            MarkDirtyRepaint();
         }
 #endregion
     }

com.unity.shadergraph/Editor/Drawing/Blackboard/BlackboardProvider.cs

@@ -267,16 +267,24 @@ public void HandleGraphChanges(bool wasUndoRedoPerformed)
                 }
             }
 
-            foreach (var input in m_Graph.addedInputs)
+            // This tries to maintain the selection the user had before the undo/redo was performed,
+            // if the user hasn't added or removed any inputs
+            if (wasUndoRedoPerformed)
             {
-                AddInputRow(input, index: m_Graph.GetGraphInputIndex(input));
+                oldSelectionPersistenceData.Clear();
+                foreach (var item in selection)
+                {
+                    if (item is BlackboardFieldView blackboardFieldView)
+                    {
+                        var guid = blackboardFieldView.shaderInput.referenceName;
+                        oldSelectionPersistenceData.Add(guid, blackboardFieldView.viewDataKey);
+                    }
+                }
             }
 
-            // This tries to maintain the selection the user had before the undo/redo was performed,
-            // if the user hasn't added or removed any inputs
-            if (wasUndoRedoPerformed && m_Graph.addedInputs.Count() == m_Graph.removedInputs.Count())
+            foreach (var input in m_Graph.addedInputs)
             {
-                oldSelection = selection;
+                AddInputRow(input, index: m_Graph.GetGraphInputIndex(input));
             }
 
             if (m_Graph.movedInputs.Any())
@@ -292,7 +300,9 @@ public void HandleGraphChanges(bool wasUndoRedoPerformed)
             }
         }
 
-        private List<ISelectable> oldSelection { get; set; } = new List<ISelectable>();
+        // A map from shaderInput reference names to the viewDataKey of the blackboardFieldView that used to represent them
+        // This data is used to re-select the shaderInputs in the blackboard after an undo/redo is performed
+        private Dictionary<string, string> oldSelectionPersistenceData { get; set; } = new Dictionary<string, string>();
 
         void AddInputRow(ShaderInput input, bool create = false, int index = -1)
         {
@@ -382,13 +392,15 @@ void AddInputRow(ShaderInput input, bool create = false, int index = -1)
 
         void UpdateSelectionAfterUndoRedo(AttachToPanelEvent evt)
         {
-            foreach (var selection in oldSelection)
+            foreach (var data in oldSelectionPersistenceData)
             {
                 var newFieldView = evt.target as BlackboardFieldView;
-                var oldFieldView = selection as BlackboardFieldView;
-                if (newFieldView?.shaderInput.referenceName == oldFieldView?.shaderInput.referenceName)
+                // If this field view represents a value that was previously selected
+                if (newFieldView?.shaderInput.referenceName == data.Key)
                 {
-                    newFieldView.viewDataKey = oldFieldView.viewDataKey;
+                    // ViewDataKey is how UIElements handles UI state persistence,
+                    // This selects the newly added field view
+                    newFieldView.viewDataKey = data.Value;
                 }
             }
         }

com.unity.shadergraph/Editor/Drawing/Inspector/InspectorView.cs

@@ -2,11 +2,11 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Reflection;
-using Drawing.Views;
- using ICSharpCode.NRefactory.Ast;
+ using Data.Interfaces;
+ using Drawing.Inspector.PropertyDrawers;
+ using Drawing.Views;
  using UnityEditor;
  using UnityEditor.Experimental.GraphView;
- using UnityEditor.Graphing;
  using UnityEditor.ShaderGraph.Drawing;
  using UnityEngine;
  using UnityEngine.UIElements;
@@ -31,7 +31,7 @@ void RegisterPropertyDrawer(Type propertyDrawerType)
             if(typeof(IPropertyDrawer).IsAssignableFrom(propertyDrawerType) == false)
                 Debug.Log("Attempted to register a property drawer that doesn't inherit from IPropertyDrawer!");
 
-            var customAttribute = propertyDrawerType.GetCustomAttribute<SGPropertyDrawer>();
+            var customAttribute = propertyDrawerType.GetCustomAttribute<SGPropertyDrawerAttribute>();
             if(customAttribute != null)
                 m_PropertyDrawerList.Add(propertyDrawerType);
             else
@@ -40,7 +40,7 @@ void RegisterPropertyDrawer(Type propertyDrawerType)
 
         public InspectorView(GraphView graphView, Action updatePreviewDelegate) : base(graphView)
         {
-            this.m_previewUpdateDelegate = updatePreviewDelegate;
+            m_previewUpdateDelegate = updatePreviewDelegate;
 
             var unregisteredPropertyDrawerTypes = TypeCache.GetTypesDerivedFrom<IPropertyDrawer>().ToList();
 
@@ -53,12 +53,7 @@ public InspectorView(GraphView graphView, Action updatePreviewDelegate) : base(g
 #region Selection
         public void Update()
         {
-            // Remove current properties
-            for (int i = 0; i < m_ContentContainer.childCount; ++i)
-            {
-                var child = m_ContentContainer.Children().ElementAt(i);
-                m_ContentContainer.Remove(child);
-            }
+            m_ContentContainer.Clear();
 
             if(selection.Count == 0)
             {
@@ -103,13 +98,13 @@ private void DrawInspectable(
             IInspectable inspectable,
             IPropertyDrawer propertyDrawerToUse = null)
         {
-            InspectorUtils.GatherInspectorContent(m_PropertyDrawerList, outputVisualElement, inspectable, this.TriggerInspectorAndPreviewUpdate, propertyDrawerToUse);
+            InspectorUtils.GatherInspectorContent(m_PropertyDrawerList, outputVisualElement, inspectable, TriggerInspectorAndPreviewUpdate, propertyDrawerToUse);
         }
 
         void TriggerInspectorAndPreviewUpdate()
         {
-            this.m_previewUpdateDelegate();
-            this.Update();
+            m_previewUpdateDelegate();
+            Update();
         }
 
         // This should be implemented by any inspector class that wants to define its own GraphSettings
@@ -175,7 +170,7 @@ private static bool IsPropertyTypeHandled(List<Type> propertyDrawerList, Type ty
             // Check to see if a property drawer has been registered that handles this type
             foreach (var propertyDrawerType in propertyDrawerList)
             {
-                var typeHandledByPropertyDrawer = propertyDrawerType.GetCustomAttribute<SGPropertyDrawer>();
+                var typeHandledByPropertyDrawer = propertyDrawerType.GetCustomAttribute<SGPropertyDrawerAttribute>();
                 // Numeric types and boolean wrapper types like ToggleData handled here
                 if (typeHandledByPropertyDrawer.propertyType == typeOfProperty)
                 {

com.unity.shadergraph/Editor/Drawing/Inspector/PropertyDrawerUtils.cs

@@ -1,4 +1,4 @@
-using UnityEngine;
+using System.Linq;
 using UnityEngine.UIElements;
 
 namespace Drawing.Inspector
@@ -7,18 +7,9 @@ public static class PropertyDrawerUtils
     {
         public static Label CreateLabel(string text, int indentLevel = 0)
         {
-            string label = "";
-            for (var i = 0; i < indentLevel; i++)
-            {
-                label += "    ";
-            }
-
+            string label = new string(' ', indentLevel * 4);
             var labelVisualElement = new Label(label + text);
-            //labelVisualElement.styleSheets.Add(Resources.Load<StyleSheet>("Styles/InspectorLabel"));
             return labelVisualElement;
         }
     }
-
-    // #TODO: Add property raw default instantiation and styling code here that all property drawers should use,
-    // to reduce cost of refactor in future if its ever needed
 }

com.unity.shadergraph/Editor/Drawing/Inspector/PropertyDrawers/BoolPropertyDrawer.cs

@@ -0,0 +1,56 @@
+using System;
+using System.Reflection;
+using Data.Interfaces;
+using UnityEditor.Graphing.Util;
+using UnityEditor.ShaderGraph.Drawing;
+using UnityEngine;
+using UnityEngine.UIElements;
+
+namespace Drawing.Inspector.PropertyDrawers
+{
+    [SGPropertyDrawer(typeof(bool))]
+    class BoolPropertyDrawer : IPropertyDrawer
+    {
+        internal delegate void ValueChangedCallback(bool newValue);
+
+        internal VisualElement CreateGUI(
+            ValueChangedCallback valueChangedCallback,
+            bool fieldToDraw,
+            string labelName,
+            out VisualElement propertyToggle,
+            int indentLevel = 0)
+        {
+            var row = new PropertyRow(PropertyDrawerUtils.CreateLabel(labelName, indentLevel));
+            // Create and assign toggle as out variable here so that callers can also do additional work with enabling/disabling if needed
+            propertyToggle = new Toggle();
+            row.Add((Toggle)propertyToggle, (toggle) =>
+            {
+                toggle.value = fieldToDraw;
+            });
+
+            if (valueChangedCallback != null)
+            {
+                var toggle = (Toggle) propertyToggle;
+                toggle.OnToggleChanged(evt => valueChangedCallback(evt.newValue));
+            }
+
+            row.styleSheets.Add(Resources.Load<StyleSheet>("Styles/PropertyRow"));
+            return row;
+        }
+
+        public Action inspectorUpdateDelegate { get; set; }
+
+        public VisualElement DrawProperty(
+            PropertyInfo propertyInfo,
+            object actualObject,
+            InspectableAttribute attribute)
+        {
+            return this.CreateGUI(
+                // Use the setter from the provided property as the callback
+                newBoolValue => propertyInfo.GetSetMethod(true).Invoke(actualObject, new object[] {newBoolValue}),
+                (bool) propertyInfo.GetValue(actualObject),
+                attribute.labelName,
+                out var propertyVisualElement);
+        }
+    }
+}