diff --git a/com.unity.shadergraph/CHANGELOG.md b/com.unity.shadergraph/CHANGELOG.md index 6ccf9b85ab3..d3e158bd3ed 100644 --- a/com.unity.shadergraph/CHANGELOG.md +++ b/com.unity.shadergraph/CHANGELOG.md @@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed the ViewDirection Node in Tangent space's calculation to match how the transform node works. - Boolean keywords now have no longer require their reference name to end in _ON to show up in the Material inspector [1306820] (https://issuetracker.unity3d.com/product/unity/issues/guid/1306820/) - Newly created properties and keywords will no longer use obfuscated GUID-based reference names in the shader code [1300484] +- Fixed issue with SRP Batcher compatibility [1310624] +- Fixed issue with Hybrid renderer compatibility [1296776] ## [10.3.0] - 2020-11-03 diff --git a/com.unity.shadergraph/Editor/Data/Implementation/NodeUtils.cs b/com.unity.shadergraph/Editor/Data/Implementation/NodeUtils.cs index 3c760710fb5..3f323cd69e6 100644 --- a/com.unity.shadergraph/Editor/Data/Implementation/NodeUtils.cs +++ b/com.unity.shadergraph/Editor/Data/Implementation/NodeUtils.cs @@ -102,7 +102,7 @@ public static SlotReference DepthFirstCollectRedirectNodeFromNode(RedirectNodeDa } public static void DepthFirstCollectNodesFromNode(List nodeList, AbstractMaterialNode node, - IncludeSelf includeSelf = IncludeSelf.Include, List> keywordPermutation = null) + IncludeSelf includeSelf = IncludeSelf.Include, List> keywordPermutation = null, bool ignoreActiveState = false) { // no where to start if (node == null) @@ -132,11 +132,11 @@ public static void DepthFirstCollectNodesFromNode(List nod { var outputNode = edge.outputSlot.node; if (outputNode != null) - DepthFirstCollectNodesFromNode(nodeList, outputNode, keywordPermutation: keywordPermutation); + DepthFirstCollectNodesFromNode(nodeList, outputNode, keywordPermutation: keywordPermutation, ignoreActiveState: ignoreActiveState); } } - if (includeSelf == IncludeSelf.Include && node.isActive) + if (includeSelf == IncludeSelf.Include && (node.isActive || ignoreActiveState)) nodeList.Add(node); } diff --git a/com.unity.shadergraph/Editor/Drawing/PreviewManager.cs b/com.unity.shadergraph/Editor/Drawing/PreviewManager.cs index 08330b6ce90..3046ad175a2 100644 --- a/com.unity.shadergraph/Editor/Drawing/PreviewManager.cs +++ b/com.unity.shadergraph/Editor/Drawing/PreviewManager.cs @@ -217,7 +217,7 @@ void OnNodeModified(AbstractMaterialNode node, ModificationScope scope) static HashSet m_TempAddedToNodeWave = new HashSet(); // cache the Action to avoid GC - Action AddNextLevelNodesToWave = + static Action AddNextLevelNodesToWave = nextLevelNode => { if (!m_TempAddedToNodeWave.Contains(nextLevelNode)) @@ -227,7 +227,7 @@ void OnNodeModified(AbstractMaterialNode node, ModificationScope scope) } }; - enum PropagationDirection + internal enum PropagationDirection { Upstream, Downstream @@ -236,7 +236,7 @@ enum PropagationDirection // ADDs all nodes in sources, and all nodes in the given direction relative to them, into result // sources and result can be the same HashSet private static readonly ProfilerMarker PropagateNodesMarker = new ProfilerMarker("PropagateNodes"); - void PropagateNodes(HashSet sources, PropagationDirection dir, HashSet result) + internal static void PropagateNodes(HashSet sources, PropagationDirection dir, HashSet result) { using (PropagateNodesMarker.Auto()) if (sources.Count > 0) diff --git a/com.unity.shadergraph/Editor/Generation/Processors/Generator.cs b/com.unity.shadergraph/Editor/Generation/Processors/Generator.cs index 52375b77432..306ceca5979 100644 --- a/com.unity.shadergraph/Editor/Generation/Processors/Generator.cs +++ b/com.unity.shadergraph/Editor/Generation/Processors/Generator.cs @@ -88,6 +88,7 @@ public ActiveFields GatherActiveFieldsFromNode(AbstractMaterialNode outputNode, void BuildShader() { var activeNodeList = Pool.ListPool.Get(); + bool ignoreActiveState = (m_Mode == GenerationMode.Preview); // for previews, we ignore node active state if (m_OutputNode == null) { foreach (var block in m_Blocks) @@ -97,12 +98,12 @@ void BuildShader() if (!block.isActive) continue; - NodeUtils.DepthFirstCollectNodesFromNode(activeNodeList, block, NodeUtils.IncludeSelf.Include); + NodeUtils.DepthFirstCollectNodesFromNode(activeNodeList, block, NodeUtils.IncludeSelf.Include, ignoreActiveState: ignoreActiveState); } } else { - NodeUtils.DepthFirstCollectNodesFromNode(activeNodeList, m_OutputNode); + NodeUtils.DepthFirstCollectNodesFromNode(activeNodeList, m_OutputNode, ignoreActiveState: ignoreActiveState); } var shaderProperties = new PropertyCollector(); @@ -132,6 +133,10 @@ void BuildShader() target.CollectShaderProperties(shaderProperties, m_Mode); } + // set the property collector to read only + // (to ensure no rogue target or pass starts adding more properties later..) + shaderProperties.SetReadOnly(); + m_Builder.AppendLine(@"Shader ""{0}""", m_Name); using (m_Builder.BlockScope()) { @@ -144,9 +149,11 @@ void BuildShader() // Instead of setup target, we can also just do get context m_Targets[i].Setup(ref context); + var subShaderProperties = GetSubShaderPropertiesForTarget(m_Targets[i], m_GraphData, m_Mode, m_OutputNode); + foreach (var subShader in context.subShaders) { - GenerateSubShader(i, subShader); + GenerateSubShader(i, subShader, subShaderProperties); } var customEditor = context.defaultShaderGUI; @@ -167,7 +174,7 @@ void BuildShader() m_ConfiguredTextures = shaderProperties.GetConfiguredTexutres(); } - void GenerateSubShader(int targetIndex, SubShaderDescriptor descriptor) + void GenerateSubShader(int targetIndex, SubShaderDescriptor descriptor, PropertyCollector subShaderProperties) { if (descriptor.passes == null) return; @@ -195,12 +202,70 @@ void GenerateSubShader(int targetIndex, SubShaderDescriptor descriptor) // Check masternode fields for valid passes if (pass.TestActive(activeFields)) - GenerateShaderPass(targetIndex, pass.descriptor, activeFields, currentBlockDescriptors.Select(x => x.descriptor).ToList()); + GenerateShaderPass(targetIndex, pass.descriptor, activeFields, currentBlockDescriptors.Select(x => x.descriptor).ToList(), subShaderProperties); + } + } + } + + // this builds the list of properties for a Target / Graph combination + static PropertyCollector GetSubShaderPropertiesForTarget(Target target, GraphData graph, GenerationMode generationMode, AbstractMaterialNode outputNode) + { + PropertyCollector subshaderProperties = new PropertyCollector(); + + // Collect shader properties declared by active nodes + using (var activeNodes = PooledHashSet.Get()) + { + if (outputNode == null) + { + // shader graph builds active nodes starting from the set of active blocks + var currentBlocks = graph.GetNodes(); + var activeBlockContext = new TargetActiveBlockContext(currentBlocks.Select(x => x.descriptor).ToList(), null); + target.GetActiveBlocks(ref activeBlockContext); + + foreach (var blockFieldDesc in activeBlockContext.activeBlocks) + { + // attempt to get BlockNode(s) from the stack + var vertBlockNode = graph.vertexContext.blocks.FirstOrDefault(x => x.value.descriptor == blockFieldDesc).value; + if (vertBlockNode != null) + activeNodes.Add(vertBlockNode); + + var fragBlockNode = graph.fragmentContext.blocks.FirstOrDefault(x => x.value.descriptor == blockFieldDesc).value; + if (fragBlockNode != null) + activeNodes.Add(fragBlockNode); + } + } + else + { + // preview and/or subgraphs build their active node set based on the single output node + activeNodes.Add(outputNode); } + + PreviewManager.PropagateNodes(activeNodes, PreviewManager.PropagationDirection.Upstream, activeNodes); + + // NOTE: this is NOT a deterministic ordering + foreach (var node in activeNodes) + node.CollectShaderProperties(subshaderProperties, generationMode); + + // So we sort the properties after + subshaderProperties.Sort(); + } + + // Collect graph properties + { + graph.CollectShaderProperties(subshaderProperties, generationMode); + } + + // Collect shader properties declared by the Target + { + target.CollectShaderProperties(subshaderProperties, generationMode); } + + subshaderProperties.SetReadOnly(); + + return subshaderProperties; } - void GenerateShaderPass(int targetIndex, PassDescriptor pass, ActiveFields activeFields, List currentBlockDescriptors) + void GenerateShaderPass(int targetIndex, PassDescriptor pass, ActiveFields activeFields, List currentBlockDescriptors, PropertyCollector subShaderProperties) { // Early exit if pass is not used in preview if (m_Mode == GenerationMode.Preview && !pass.useInPreview) @@ -653,7 +718,7 @@ void ProcessStackForPass(ContextData contextData, BlockFieldDescriptor[] passBlo using (var propertyBuilder = new ShaderStringBuilder()) { - propertyCollector.GetPropertiesDeclaration(propertyBuilder, m_Mode, m_GraphData.concretePrecision); + subShaderProperties.GetPropertiesDeclaration(propertyBuilder, m_Mode, m_GraphData.concretePrecision); if (propertyBuilder.length == 0) propertyBuilder.AppendLine("// GraphProperties: "); spliceCommands.Add("GraphProperties", propertyBuilder.ToCodeBlock()); @@ -662,17 +727,12 @@ void ProcessStackForPass(ContextData contextData, BlockFieldDescriptor[] passBlo // -------------------------------------------------- // Dots Instanced Graph Properties - bool hasDotsProperties = false; - m_GraphData.ForeachHLSLProperty(h => - { - if (h.declaration == HLSLDeclaration.HybridPerInstance) - hasDotsProperties = true; - }); + bool hasDotsProperties = subShaderProperties.HasDotsProperties(); using (var dotsInstancedPropertyBuilder = new ShaderStringBuilder()) { if (hasDotsProperties) - dotsInstancedPropertyBuilder.AppendLines(propertyCollector.GetDotsInstancingPropertiesDeclaration(m_Mode)); + dotsInstancedPropertyBuilder.AppendLines(subShaderProperties.GetDotsInstancingPropertiesDeclaration(m_Mode)); else dotsInstancedPropertyBuilder.AppendLine("// HybridV1InjectedBuiltinProperties: "); spliceCommands.Add("HybridV1InjectedBuiltinProperties", dotsInstancedPropertyBuilder.ToCodeBlock()); diff --git a/com.unity.shadergraph/Editor/Generation/Processors/PropertyCollector.cs b/com.unity.shadergraph/Editor/Generation/Processors/PropertyCollector.cs index fa46a5f501a..55de7e78f86 100644 --- a/com.unity.shadergraph/Editor/Generation/Processors/PropertyCollector.cs +++ b/com.unity.shadergraph/Editor/Generation/Processors/PropertyCollector.cs @@ -1,7 +1,9 @@ +using System; using System.Collections.Generic; using System.Linq; using System.Text; using UnityEditor.ShaderGraph.Internal; +using UnityEngine; namespace UnityEditor.ShaderGraph { @@ -14,13 +16,117 @@ public struct TextureInfo public bool modifiable; } - public readonly List properties = new List(); + bool m_ReadOnly; + List m_HLSLProperties = null; - public void AddShaderProperty(AbstractShaderProperty chunk) + // reference name ==> property index in list + Dictionary m_ReferenceNames = new Dictionary(); + + // list of properties (kept in a list to maintain deterministic declaration order) + List m_Properties = new List(); + + public int propertyCount => m_Properties.Count; + public IEnumerable properties => m_Properties; + public AbstractShaderProperty GetProperty(int index) { return m_Properties[index]; } + + public void Sort() { - if (properties.Any(x => x.referenceName == chunk.referenceName)) + if (m_ReadOnly) + { + Debug.LogError("Cannot sort the properties when the PropertyCollector is already marked ReadOnly"); return; - properties.Add(chunk); + } + + m_Properties.Sort((a, b) => String.CompareOrdinal(a.referenceName, b.referenceName)); + } + + public void SetReadOnly() + { + m_ReadOnly = true; + } + + private static bool EquivalentHLSLProperties(AbstractShaderProperty a, AbstractShaderProperty b) + { + bool equivalent = true; + var bHLSLProps = new List(); + b.ForeachHLSLProperty(bh => bHLSLProps.Add(bh)); + a.ForeachHLSLProperty(ah => + { + var i = bHLSLProps.FindIndex(bh => bh.name == ah.name); + if (i < 0) + equivalent = false; + else + { + var bh = bHLSLProps[i]; + if ((ah.name != bh.name) || + (ah.type != bh.type) || + (ah.precision != bh.precision) || + (ah.declaration != bh.declaration) || + ((ah.customDeclaration == null) != (bh.customDeclaration == null))) + { + equivalent = false; + } + else if (ah.customDeclaration != null) + { + var ssba = new ShaderStringBuilder(); + var ssbb = new ShaderStringBuilder(); + ah.customDeclaration(ssba); + bh.customDeclaration(ssbb); + if (ssba.ToCodeBlock() != ssbb.ToCodeBlock()) + equivalent = false; + } + bHLSLProps.RemoveAt(i); + } + }); + return equivalent && (bHLSLProps.Count == 0); + } + + public void AddShaderProperty(AbstractShaderProperty prop) + { + if (m_ReadOnly) + { + Debug.LogError("ERROR attempting to add property to readonly collection"); + return; + } + + int propIndex = -1; + if (m_ReferenceNames.TryGetValue(prop.referenceName, out propIndex)) + { + // existing referenceName + var existingProp = m_Properties[propIndex]; + if (existingProp != prop) + { + // duplicate reference name, but different property instances + if (existingProp.GetType() != prop.GetType()) + { + Debug.LogError("Two properties with the same reference name (" + prop.referenceName + ") using different types"); + } + else + { + if (!EquivalentHLSLProperties(existingProp, prop)) + Debug.LogError("Two properties with the same reference name (" + prop.referenceName + ") produce different HLSL properties"); + } + } + } + else + { + // new referenceName, new property + propIndex = m_Properties.Count; + m_Properties.Add(prop); + m_ReferenceNames.Add(prop.referenceName, propIndex); + } + } + + private List BuildHLSLPropertyList() + { + SetReadOnly(); + if (m_HLSLProperties == null) + { + m_HLSLProperties = new List(); + foreach (var p in m_Properties) + p.ForeachHLSLProperty(h => m_HLSLProperties.Add(h)); + } + return m_HLSLProperties; } public void GetPropertiesDeclaration(ShaderStringBuilder builder, GenerationMode mode, ConcretePrecision inheritedPrecision) @@ -31,8 +137,7 @@ public void GetPropertiesDeclaration(ShaderStringBuilder builder, GenerationMode } // build a list of all HLSL properties - var hlslProps = new List(); - properties.ForEach(p => p.ForeachHLSLProperty(h => hlslProps.Add(h))); + var hlslProps = BuildHLSLPropertyList(); if (mode == GenerationMode.Preview) { @@ -150,6 +255,18 @@ public void GetPropertiesDeclaration(ShaderStringBuilder builder, GenerationMode h.AppendTo(builder); } + public bool HasDotsProperties() + { + var hlslProps = BuildHLSLPropertyList(); + bool hasDotsProperties = false; + foreach (var h in hlslProps) + { + if (h.declaration == HLSLDeclaration.HybridPerInstance) + hasDotsProperties = true; + } + return hasDotsProperties; + } + public string GetDotsInstancingPropertiesDeclaration(GenerationMode mode) { // Hybrid V1 needs to declare a special macro to that is injected into @@ -160,12 +277,7 @@ public string GetDotsInstancingPropertiesDeclaration(GenerationMode mode) var batchAll = (mode == GenerationMode.Preview); // build a list of all HLSL properties - var hybridHLSLProps = new List(); - properties.ForEach(p => p.ForeachHLSLProperty(h => - { - if (h.declaration == HLSLDeclaration.HybridPerInstance) - hybridHLSLProps.Add(h); - })); + var hybridHLSLProps = BuildHLSLPropertyList().Where(h => h.declaration == HLSLDeclaration.HybridPerInstance); if (hybridHLSLProps.Any()) { diff --git a/com.unity.shadergraph/Editor/Importers/ShaderGraphImporter.cs b/com.unity.shadergraph/Editor/Importers/ShaderGraphImporter.cs index 4ed0197f1b0..fcd5d7bed0c 100644 --- a/com.unity.shadergraph/Editor/Importers/ShaderGraphImporter.cs +++ b/com.unity.shadergraph/Editor/Importers/ShaderGraphImporter.cs @@ -24,9 +24,9 @@ namespace UnityEditor.ShaderGraph // sure that all shader graphs get re-imported. Re-importing is required, // because the shader graph codegen is different for V2. // This ifdef can be removed once V2 is the only option. - [ScriptedImporter(109, Extension, -902)] + [ScriptedImporter(110, Extension, -902)] #else - [ScriptedImporter(41, Extension, -902)] + [ScriptedImporter(42, Extension, -902)] #endif class ShaderGraphImporter : ScriptedImporter diff --git a/com.unity.shadergraph/Editor/Importers/ShaderSubGraphImporter.cs b/com.unity.shadergraph/Editor/Importers/ShaderSubGraphImporter.cs index da96791362c..3123cd3cb19 100644 --- a/com.unity.shadergraph/Editor/Importers/ShaderSubGraphImporter.cs +++ b/com.unity.shadergraph/Editor/Importers/ShaderSubGraphImporter.cs @@ -19,7 +19,7 @@ namespace UnityEditor.ShaderGraph { [ExcludeFromPreset] - [ScriptedImporter(20, Extension, -905)] + [ScriptedImporter(21, Extension, -905)] class ShaderSubGraphImporter : ScriptedImporter { public const string Extension = "shadersubgraph"; @@ -299,16 +299,16 @@ static void ProcessSubGraph(SubGraphAsset asset, GraphData graph) var collector = new PropertyCollector(); foreach (var node in nodes) { - int previousPropertyCount = Math.Max(0, collector.properties.Count - 1); + int previousPropertyCount = Math.Max(0, collector.propertyCount - 1); node.CollectShaderProperties(collector, GenerationMode.ForReals); // This is a stop-gap to prevent the autogenerated values from JsonObject and ShaderInput from // resulting in non-deterministic import data. While we should move to local ids in the future, // this will prevent cascading shader recompilations. - for (int i = previousPropertyCount; i < collector.properties.Count; ++i) + for (int i = previousPropertyCount; i < collector.propertyCount; ++i) { - var prop = collector.properties[i]; + var prop = collector.GetProperty(i); var namespaceId = node.objectId; var nameId = prop.referenceName;