Shader Graph memory leak plumbing (#5553)

* 2 down * Unregister some delegates in GraphEditorView.Dispose. * fix more leeks * Update change log * PR Feedback - lots of using statements instead of explicit Dispose. * PR Feedback - a couple more using statements instead of explicit Dispose. * Move GC finalize registration calls into #if * Update CHANGELOG.md Co-authored-by: Marc Templin <marctem@users.noreply.github.com> Co-authored-by: Kerry Turner <mskerryturner@gmail.com>
Unity-Technologies · Jan 27, 2020 · 728baa8 · 728baa8
1 parent 15db9d6
commit 728baa8
Show file tree

Hide file tree

Showing 16 changed files with 603 additions and 483 deletions.
diff --git a/com.unity.shadergraph/CHANGELOG.md b/com.unity.shadergraph/CHANGELOG.md
@@ -49,6 +49,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fixed an issue where boolean keywords in a Shader Graph caused HDRP Material features to fail. [1204827](https://issuetracker.unity3d.com/issues/hdrp-shadergraph-adding-a-boolean-keyword-to-an-hdrp-lit-shader-makes-material-features-not-work)
 - Fixed a bug where Object space normals scaled with Object Scale. 
 - Documentation links on nodes now point to the correct URLs and package versions.
+- Fixed a number of memory leaks that caused Shader Graph assets to stay in memory after closing the Shader Graph window.
 - You can now smoothly edit controls on the `Dielectric Specular` node.
 - Fixed Blackboard Properties to support scientific notation.
 

diff --git a/com.unity.shadergraph/Editor/Data/Implementation/GraphObject.cs b/com.unity.shadergraph/Editor/Data/Implementation/GraphObject.cs
@@ -104,5 +104,10 @@ void OnEnable()
         {
             Validate();
         }
+
+        void OnDestroy()
+        {
+            graph?.OnDisable();
+        }
     }
 }
diff --git a/com.unity.shadergraph/Editor/Data/Nodes/AbstractMaterialNode.cs b/com.unity.shadergraph/Editor/Data/Nodes/AbstractMaterialNode.cs
diff --git a/com.unity.shadergraph/Editor/Data/Nodes/CodeFunctionNode.cs b/com.unity.shadergraph/Editor/Data/Nodes/CodeFunctionNode.cs
@@ -343,34 +343,36 @@ private static MaterialSlot CreateBoundSlot(Binding attributeBinding, int slotId
 
         public void GenerateNodeCode(ShaderStringBuilder sb, GenerationMode generationMode)
         {
-            s_TempSlots.Clear();
-            GetOutputSlots(s_TempSlots);
-            foreach (var outSlot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                sb.AppendLine(outSlot.concreteValueType.ToShaderString() + " " + GetVariableNameForSlot(outSlot.id) + ";");
-            }
+                GetOutputSlots(tempSlots);
+                foreach (var outSlot in tempSlots)
+                {
+                    sb.AppendLine(outSlot.concreteValueType.ToShaderString() + " " + GetVariableNameForSlot(outSlot.id) + ";");
+                }
 
-            string call = GetFunctionName() + "(";
-            bool first = true;
-            s_TempSlots.Clear();
-            GetSlots(s_TempSlots);
-            s_TempSlots.Sort((slot1, slot2) => slot1.id.CompareTo(slot2.id));
-            foreach (var slot in s_TempSlots)
-            {
-                if (!first)
+                string call = GetFunctionName() + "(";
+                bool first = true;
+                tempSlots.Clear();
+                GetSlots(tempSlots);
+                tempSlots.Sort((slot1, slot2) => slot1.id.CompareTo(slot2.id));
+                foreach (var slot in tempSlots)
                 {
-                    call += ", ";
+                    if (!first)
+                    {
+                        call += ", ";
+                    }
+                    first = false;
+
+                    if (slot.isInputSlot)
+                        call += GetSlotValue(slot.id, generationMode);
+                    else
+                        call += GetVariableNameForSlot(slot.id);
                 }
-                first = false;
+                call += ");";
 
-                if (slot.isInputSlot)
-                    call += GetSlotValue(slot.id, generationMode);
-                else
-                    call += GetVariableNameForSlot(slot.id);
+                sb.AppendLine(call);
             }
-            call += ");";
-
-            sb.AppendLine(call);
         }
 
         private string GetFunctionName()
@@ -385,24 +387,27 @@ private string GetFunctionHeader()
         {
             string header = "void " + GetFunctionName() + "(";
 
-            s_TempSlots.Clear();
-            GetSlots(s_TempSlots);
-            s_TempSlots.Sort((slot1, slot2) => slot1.id.CompareTo(slot2.id));
-            var first = true;
-            foreach (var slot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                if (!first)
-                    header += ", ";
+                GetSlots(tempSlots);
+                tempSlots.Sort((slot1, slot2) => slot1.id.CompareTo(slot2.id));
+                var first = true;
+                foreach (var slot in tempSlots)
+                {
+                    if (!first)
+                        header += ", ";
 
-                first = false;
+                    first = false;
 
-                if (slot.isOutputSlot)
-                    header += "out ";
+                    if (slot.isOutputSlot)
+                        header += "out ";
+
+                    header += slot.concreteValueType.ToShaderString() + " " + slot.shaderOutputName;
+                }
 
-                header += slot.concreteValueType.ToShaderString() + " " + slot.shaderOutputName;
+                header += ")";
             }
 
-            header += ")";
             return header;
         }
 
@@ -422,14 +427,17 @@ private string GetFunctionBody(MethodInfo info)
             if (string.IsNullOrEmpty(result))
                 return string.Empty;
 
-            s_TempSlots.Clear();
-            GetSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                var toReplace = string.Format("{{slot{0}dimension}}", slot.id);
-                var replacement = NodeUtils.GetSlotDimension(slot.concreteValueType);
-                result = result.Replace(toReplace, replacement);
+                GetSlots(tempSlots);
+                foreach (var slot in tempSlots)
+                {
+                    var toReplace = string.Format("{{slot{0}dimension}}", slot.id);
+                    var replacement = NodeUtils.GetSlotDimension(slot.concreteValueType);
+                    result = result.Replace(toReplace, replacement);
+                }
             }
+
             return result;
         }
 
@@ -453,87 +461,106 @@ private static SlotAttribute GetSlotAttribute([NotNull] ParameterInfo info)
         public NeededCoordinateSpace RequiresNormal(ShaderStageCapability stageCapability)
         {
             var binding = NeededCoordinateSpace.None;
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
-                binding |= slot.RequiresNormal();
-            return binding;
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
+            {
+                GetInputSlots(tempSlots);
+                foreach (var slot in tempSlots)
+                    binding |= slot.RequiresNormal();
+                return binding;
+            }
         }
 
         public NeededCoordinateSpace RequiresViewDirection(ShaderStageCapability stageCapability)
         {
             var binding = NeededCoordinateSpace.None;
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
-                binding |= slot.RequiresViewDirection();
-            return binding;
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
+            {
+                GetInputSlots(tempSlots);
+                foreach (var slot in tempSlots)
+                    binding |= slot.RequiresViewDirection();
+                return binding;
+            }
         }
 
         public NeededCoordinateSpace RequiresPosition(ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            var binding = NeededCoordinateSpace.None;
-            foreach (var slot in s_TempSlots)
-                binding |= slot.RequiresPosition();
-            return binding;
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
+            {
+                GetInputSlots(tempSlots);
+                var binding = NeededCoordinateSpace.None;
+                foreach (var slot in tempSlots)
+                    binding |= slot.RequiresPosition();
+                return binding;
+            }
         }
 
         public NeededCoordinateSpace RequiresTangent(ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            var binding = NeededCoordinateSpace.None;
-            foreach (var slot in s_TempSlots)
-                binding |= slot.RequiresTangent();
-            return binding;
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
+            {
+                GetInputSlots(tempSlots);
+                var binding = NeededCoordinateSpace.None;
+                foreach (var slot in tempSlots)
+                    binding |= slot.RequiresTangent();
+                return binding;
+            }
         }
 
         public NeededCoordinateSpace RequiresBitangent(ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            var binding = NeededCoordinateSpace.None;
-            foreach (var slot in s_TempSlots)
-                binding |= slot.RequiresBitangent();
-            return binding;
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
+            {
+                GetInputSlots(tempSlots);
+                var binding = NeededCoordinateSpace.None;
+                foreach (var slot in tempSlots)
+                    binding |= slot.RequiresBitangent();
+                return binding;
+            }
         }
 
         public bool RequiresMeshUV(UVChannel channel, ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                if (slot.RequiresMeshUV(channel))
-                    return true;
+                GetInputSlots(tempSlots);
+                foreach (var slot in tempSlots)
+                {
+                    if (slot.RequiresMeshUV(channel))
+                        return true;
+                }
+
+                return false;
             }
-            return false;
         }
 
         public bool RequiresScreenPosition(ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                if (slot.RequiresScreenPosition())
-                    return true;
+                GetInputSlots(tempSlots);
+                foreach (var slot in tempSlots)
+                {
+                    if (slot.RequiresScreenPosition())
+                        return true;
+                }
+
+                return false;
             }
-            return false;
         }
 
         public bool RequiresVertexColor(ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                if (slot.RequiresVertexColor())
-                    return true;
+                GetInputSlots(tempSlots);
+                foreach (var slot in tempSlots)
+                {
+                    if (slot.RequiresVertexColor())
+                        return true;
+                }
+
+                return false;
             }
-            return false;
         }
     }
 }
diff --git a/com.unity.shadergraph/Editor/Data/Nodes/Input/Texture/SampleTexture2DArrayNode.cs b/com.unity.shadergraph/Editor/Data/Nodes/Input/Texture/SampleTexture2DArrayNode.cs
@@ -80,14 +80,22 @@ public virtual void GenerateNodeCode(ShaderStringBuilder sb, GenerationMode gene
 
         public bool RequiresMeshUV(UVChannel channel, ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                if (slot.RequiresMeshUV(channel))
-                    return true;
+                GetInputSlots(tempSlots);
+                var result = false;
+                foreach (var slot in tempSlots)
+                {
+                    if (slot.RequiresMeshUV(channel))
+                    {
+                        result = true;
+                        break;
+                    }
+                }
+
+                tempSlots.Clear();
+                return result;
             }
-            return false;
         }
     }
 }
diff --git a/com.unity.shadergraph/Editor/Data/Nodes/Input/Texture/SampleTexture2DLODNode.cs b/com.unity.shadergraph/Editor/Data/Nodes/Input/Texture/SampleTexture2DLODNode.cs
@@ -137,14 +137,21 @@ public virtual void GenerateNodeCode(ShaderStringBuilder sb, GenerationMode gene
 
         public bool RequiresMeshUV(UVChannel channel, ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
+            var result = false;
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                if (slot.RequiresMeshUV(channel))
-                    return true;
+                GetInputSlots(tempSlots);
+                foreach (var slot in tempSlots)
+                {
+                    if (slot.RequiresMeshUV(channel))
+                    {
+                        result = true;
+                        break;
+                    }
+                }
             }
-            return false;
+
+            return result;
         }
     }
 }
diff --git a/com.unity.shadergraph/Editor/Data/Nodes/Input/Texture/SampleTexture2DNode.cs b/com.unity.shadergraph/Editor/Data/Nodes/Input/Texture/SampleTexture2DNode.cs
@@ -138,14 +138,22 @@ public virtual void GenerateNodeCode(ShaderStringBuilder sb, GenerationMode gene
 
         public bool RequiresMeshUV(UVChannel channel, ShaderStageCapability stageCapability)
         {
-            s_TempSlots.Clear();
-            GetInputSlots(s_TempSlots);
-            foreach (var slot in s_TempSlots)
+            using (var tempSlots = PooledList<MaterialSlot>.Get())
             {
-                if (slot.RequiresMeshUV(channel))
-                    return true;
+                GetInputSlots(tempSlots);
+                var result = false;
+                foreach (var slot in tempSlots)
+                {
+                    if (slot.RequiresMeshUV(channel))
+                    {
+                        result = true;
+                        break;
+                    }
+                }
+
+                tempSlots.Clear();
+                return result;
             }
-            return false;
         }
     }
 }