diff --git a/com.unity.shadergraph/CHANGELOG.md b/com.unity.shadergraph/CHANGELOG.md index e79c9ff6eef..b50b88979d9 100644 --- a/com.unity.shadergraph/CHANGELOG.md +++ b/com.unity.shadergraph/CHANGELOG.md @@ -108,6 +108,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed a bug where nodes dealing with matricies would sometimes display a preview, sometimes not. - Optimized loading a large Shader Graph. [1209047](https://issuetracker.unity3d.com/issues/shader-graph-unresponsive-editor-when-using-large-graphs) - Fixed NaN issue in triplanar SG node when blend goes to 0. +- Fixed a recurring bug where node inputs would get misaligned from their ports. [1224480] - Fixed an issue where Blackboard properties would not duplicate with `Precision` or `Hybrid Instancing` options. - Fixed an issue where `Texture` properties on the Blackboard would not duplicate with the same `Mode` settings. - Fixed an issue where `Keywords` on the Blackboard would not duplicate with the same `Default` value. diff --git a/com.unity.shadergraph/Editor/Data/Nodes/Utility/RedirectNodeView.cs b/com.unity.shadergraph/Editor/Data/Nodes/Utility/RedirectNodeView.cs index 873da20fdf9..df3800456d3 100644 --- a/com.unity.shadergraph/Editor/Data/Nodes/Utility/RedirectNodeView.cs +++ b/com.unity.shadergraph/Editor/Data/Nodes/Utility/RedirectNodeView.cs @@ -129,6 +129,12 @@ public void OnModified(ModificationScope scope) } } + public bool FindPort(SlotReference slot, out ShaderPort port) + { + port = contentContainer.Q("top")?.Query().Where(p => p.slot.slotReference.Equals(slot)).First(); + return port != null; + } + public void AttachMessage(string errString, ShaderCompilerMessageSeverity severity) { ClearMessage(); diff --git a/com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs b/com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs index c039ec43eaf..96308036688 100644 --- a/com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs +++ b/com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs @@ -825,14 +825,11 @@ void AddNode(AbstractMaterialNode node, bool usePrebuiltVisualGroupMap = false) m_SearchWindowProvider.targetSlotReference.node == node) { m_SearchWindowProvider.nodeNeedsRepositioning = false; - foreach (var element in nodeView.inputContainer.Children().Union(nodeView.outputContainer.Children())) + if (nodeView is IShaderNodeView shaderView && + shaderView.FindPort(m_SearchWindowProvider.targetSlotReference, out var port)) { - var port = (ShaderPort) element; - if (port.slot.slotReference.Equals(m_SearchWindowProvider.targetSlotReference)) - { - port.RegisterCallback(RepositionNode); - return; - } + port.RegisterCallback(RepositionNode); + return; } } @@ -1016,7 +1013,7 @@ Edge AddEdge(IEdge edge, bool useVisualNodeMap = false, bool updateNodePorts = t if (sourceNodeView != null) { - var sourceAnchor = sourceNodeView.gvNode.outputContainer.Children().OfType().First(x => x.slot.Equals(sourceSlot)); + sourceNodeView.FindPort(sourceSlot.slotReference, out var sourceAnchor); IShaderNodeView targetNodeView; if (useVisualNodeMap) @@ -1024,7 +1021,7 @@ Edge AddEdge(IEdge edge, bool useVisualNodeMap = false, bool updateNodePorts = t else targetNodeView = m_GraphView.nodes.ToList().OfType().First(x => x.node == targetNode); - var targetAnchor = targetNodeView.gvNode.inputContainer.Children().OfType().First(x => x.slot.Equals(targetSlot)); + targetNodeView.FindPort(targetSlot.slotReference, out var targetAnchor); var edgeView = new Edge { @@ -1097,7 +1094,7 @@ void UpdateEdgeColors(HashSet nodeViews) } } - foreach (var anchorView in nodeView.inputContainer.Children().OfType()) + foreach (var anchorView in nodeView.inputContainer.Query().ToList()) { var targetSlot = anchorView.GetSlot(); if (targetSlot.valueType != SlotValueType.DynamicVector) diff --git a/com.unity.shadergraph/Editor/Drawing/Views/IShaderNodeView.cs b/com.unity.shadergraph/Editor/Drawing/Views/IShaderNodeView.cs index e920973d6cd..72466039d3e 100644 --- a/com.unity.shadergraph/Editor/Drawing/Views/IShaderNodeView.cs +++ b/com.unity.shadergraph/Editor/Drawing/Views/IShaderNodeView.cs @@ -2,6 +2,7 @@ using UnityEditor.Experimental.GraphView; using UnityEditor.Graphing; using UnityEditor.Rendering; +using UnityEditor.ShaderGraph.Drawing; using UnityEngine; using UnityEngine.UIElements; @@ -18,5 +19,8 @@ interface IShaderNodeView : IDisposable void OnModified(ModificationScope scope); void AttachMessage(string errString, ShaderCompilerMessageSeverity severity); void ClearMessage(); + // Searches the ports on this node for one that matches the given slot. + // Returns true if found, false if not. + bool FindPort(SlotReference slot, out ShaderPort port); } } diff --git a/com.unity.shadergraph/Editor/Drawing/Views/MaterialNodeView.cs b/com.unity.shadergraph/Editor/Drawing/Views/MaterialNodeView.cs index 2aead237eaf..e379fcbd1be 100644 --- a/com.unity.shadergraph/Editor/Drawing/Views/MaterialNodeView.cs +++ b/com.unity.shadergraph/Editor/Drawing/Views/MaterialNodeView.cs @@ -31,7 +31,6 @@ sealed class MaterialNodeView : Node, IShaderNodeView, IInspectable VisualElement m_PreviewFiller; VisualElement m_ControlsDivider; IEdgeConnectorListener m_ConnectorListener; - VisualElement m_PortInputContainer; VisualElement m_SettingsContainer; bool m_ShowSettings = false; VisualElement m_SettingsButton; @@ -53,7 +52,7 @@ public void Initialize(AbstractMaterialNode inNode, PreviewManager previewManage var contents = this.Q("contents"); m_GraphView = graphView; - + mainContainer.style.overflow = StyleKeyword.None; // Override explicit style set in base class m_ConnectorListener = connectorListener; node = inNode; viewDataKey = node.objectId; @@ -129,20 +128,8 @@ public void Initialize(AbstractMaterialNode inNode, PreviewManager previewManage UpdatePreviewExpandedState(node.previewExpanded); } - // Add port input container, which acts as a pixel cache for all port inputs - m_PortInputContainer = new VisualElement - { - name = "portInputContainer", - style = { overflow = Overflow.Hidden }, - pickingMode = PickingMode.Ignore - }; - Add(m_PortInputContainer); - - AddSlots(node.GetSlots()); - UpdatePortInputs(); base.expanded = node.drawState.expanded; - RefreshExpandedState(); //This should not be needed. GraphView needs to improve the extension api here - UpdatePortInputVisibilities(); + AddSlots(node.GetSlots()); SetPosition(new Rect(node.drawState.position.x, node.drawState.position.y, 0, 0)); @@ -151,12 +138,9 @@ public void Initialize(AbstractMaterialNode inNode, PreviewManager previewManage RegisterCallback(OnSubGraphDoubleClick); } - m_PortInputContainer.SendToBack(); - m_TitleContainer = this.Q("title"); - var masterNode = node as IMasterNode; - if (masterNode != null) + if (node is IMasterNode) { AddToClassList("master"); bool validTarget = false; @@ -217,6 +201,15 @@ public void Initialize(AbstractMaterialNode inNode, PreviewManager previewManage RegisterCallback(OnMouseHover); } + public bool FindPort(SlotReference slotRef, out ShaderPort port) + { + port = inputContainer.Query().ToList() + .Concat(outputContainer.Query().ToList()) + .First(p => p.slot.slotReference.Equals(slotRef)); + + return port != null; + } + public void AttachMessage(string errString, ShaderCompilerMessageSeverity severity) { ClearMessage(); @@ -290,11 +283,13 @@ void OnSubGraphDoubleClick(MouseDownEvent evt) public override bool expanded { - get { return base.expanded; } + get => base.expanded; set { - if (base.expanded != value) - base.expanded = value; + if (base.expanded == value) + return; + + base.expanded = value; if (node.drawState.expanded != value) { @@ -303,8 +298,12 @@ public override bool expanded node.drawState = ds; } - RefreshExpandedState(); //This should not be needed. GraphView needs to improve the extension api here - UpdatePortInputVisibilities(); + foreach (var inputPort in inputContainer.Query().ToList()) + { + inputPort.parent.style.visibility = inputPort.style.visibility; + } + + RefreshExpandedState(); // Necessary b/c we can't override enough Node.cs functions to update only what's needed } } @@ -542,76 +541,66 @@ public void OnModified(ModificationScope scope) base.expanded = node.drawState.expanded; - // Update slots to match node modification - if (scope == ModificationScope.Topological) + switch (scope) { - RecreateSettings(); + // Update slots to match node modification + case ModificationScope.Topological: + { + RecreateSettings(); - var slots = node.GetSlots().ToList(); + var slots = node.GetSlots().ToList(); - var inputPorts = inputContainer.Children().OfType().ToList(); - foreach (var port in inputPorts) - { - var currentSlot = port.slot; - var newSlot = slots.FirstOrDefault(s => s.id == currentSlot.id); - if (newSlot == null) + var inputPorts = inputContainer.Query().ToList(); + foreach (var port in inputPorts) { - // Slot doesn't exist anymore, remove it - inputContainer.Remove(port); + var currentSlot = port.slot; + var newSlot = slots.FirstOrDefault(s => s.id == currentSlot.id); + if (newSlot == null) + { + // Slot doesn't exist anymore, remove it + inputContainer.Remove(port.parent); + } + else + { + port.slot = newSlot; + UpdatePortInput(port); - // We also need to remove the inline input - var portInputView = m_PortInputContainer.Children().OfType().FirstOrDefault(v => Equals(v.slot, port.slot)); - if (portInputView != null) - portInputView.RemoveFromHierarchy(); + slots.Remove(newSlot); + } } - else + + var outputPorts = outputContainer.Children().OfType().ToList(); + foreach (var port in outputPorts) { - port.slot = newSlot; - var portInputView = m_PortInputContainer.Children().OfType().FirstOrDefault(x => x.slot.id == currentSlot.id); - if (newSlot.isConnected) + var currentSlot = port.slot; + var newSlot = slots.FirstOrDefault(s => s.id == currentSlot.id); + if (newSlot == null) { - portInputView?.RemoveFromHierarchy(); + outputContainer.Remove(port); } else { - portInputView?.UpdateSlot(newSlot); + port.slot = newSlot; + slots.Remove(newSlot); } - - slots.Remove(newSlot); } - } - var outputPorts = outputContainer.Children().OfType().ToList(); - foreach (var port in outputPorts) - { - var currentSlot = port.slot; - var newSlot = slots.FirstOrDefault(s => s.id == currentSlot.id); - if (newSlot == null) - { - outputContainer.Remove(port); - } - else - { - port.slot = newSlot; - slots.Remove(newSlot); - } - } + AddSlots(slots); - AddSlots(slots); + slots.Clear(); + slots.AddRange(node.GetSlots()); - slots.Clear(); - slots.AddRange(node.GetSlots()); + if (inputContainer.childCount > 0) + inputContainer.Sort((x, y) => slots.IndexOf(x.Q().slot) - slots.IndexOf(y.Q().slot)); + if (outputContainer.childCount > 0) + outputContainer.Sort((x, y) => slots.IndexOf(((ShaderPort)x).slot) - slots.IndexOf(((ShaderPort)y).slot)); - if (inputContainer.childCount > 0) - inputContainer.Sort((x, y) => slots.IndexOf(((ShaderPort)x).slot) - slots.IndexOf(((ShaderPort)y).slot)); - if (outputContainer.childCount > 0) - outputContainer.Sort((x, y) => slots.IndexOf(((ShaderPort)x).slot) - slots.IndexOf(((ShaderPort)y).slot)); + break; - UpdatePortInputs(); - UpdatePortInputVisibilities(); + } } - RefreshExpandedState(); //This should not be needed. GraphView needs to improve the extension api here + RefreshExpandedState(); // Necessary b/c we can't override enough Node.cs functions to update only what's needed foreach (var listener in m_ControlItems.Children().OfType()) { @@ -629,97 +618,85 @@ void AddSlots(IEnumerable slots) var port = ShaderPort.Create(slot, m_ConnectorListener); if (slot.isOutputSlot) - outputContainer.Add(port); - else - inputContainer.Add(port); - } - } - - void UpdatePortInputs() - { - foreach (var port in inputContainer.Children().OfType()) - { - if (port.slot.isConnected) { - continue; + outputContainer.Add(port); } - - var portInputView = m_PortInputContainer.Children().OfType().FirstOrDefault(a => Equals(a.slot, port.slot)); - if (portInputView == null) + else { - portInputView = new PortInputView(port.slot) { style = { position = Position.Absolute } }; - m_PortInputContainer.Add(portInputView); - SetPortInputPosition(port, portInputView); + var portContainer = new VisualElement(); + portContainer.style.flexDirection = FlexDirection.Row; + var portInputView = new PortInputView(slot) {style = {position = Position.Absolute}}; + portContainer.Add(portInputView); + portContainer.Add(port); + inputContainer.Add(portContainer); } - - port.RegisterCallback(UpdatePortInput); - } - } - - void UpdatePortInput(GeometryChangedEvent evt) - { - var port = (ShaderPort)evt.target; - var inputViews = m_PortInputContainer.Children().OfType().Where(x => Equals(x.slot, port.slot)); - - // Ensure PortInputViews are initialized correctly - // Dynamic port lists require one update to validate before init - if(inputViews.Count() != 0) - { - var inputView = inputViews.First(); - SetPortInputPosition(port, inputView); + port.OnDisconnect = OnEdgeDisconnected; } - - port.UnregisterCallback(UpdatePortInput); } - void SetPortInputPosition(ShaderPort port, PortInputView inputView) + void OnEdgeDisconnected(Port obj) { - inputView.style.top = port.layout.y; - inputView.parent.style.height = inputContainer.layout.height; + RefreshExpandedState(); } - void UpdatePortInputVisibilities() + static bool GetPortInputView(ShaderPort port, out PortInputView view) { - if (expanded) - { - m_PortInputContainer.style.display = StyleKeyword.Null; - } - else - { - m_PortInputContainer.style.display = DisplayStyle.None; - } + view = port.parent.Q(); + return view != null; } public void UpdatePortInputTypes() { - foreach (var anchor in inputContainer.Children().Concat(outputContainer.Children()).OfType()) + foreach (var anchor in inputContainer.Query().ToList()) { var slot = anchor.slot; anchor.portName = slot.displayName; anchor.visualClass = slot.concreteValueType.ToClassName(); + if (GetPortInputView(anchor, out var portInputView)) + { + portInputView.UpdateSlotType(); + UpdatePortInputVisibility(portInputView, anchor); + } } - foreach (var portInputView in m_PortInputContainer.Children().OfType()) - portInputView.UpdateSlotType(); - foreach (var control in m_ControlItems.Children()) { - var listener = control as AbstractMaterialNodeModificationListener; - if (listener != null) + if (control is AbstractMaterialNodeModificationListener listener) listener.OnNodeModified(ModificationScope.Graph); } } - void OnResize(Vector2 deltaSize) + void UpdatePortInput(ShaderPort port) + { + if (GetPortInputView(port, out var portInputView)) + { + portInputView.UpdateSlot(port.slot); + UpdatePortInputVisibility(portInputView, port); + } + } + + void UpdatePortInputVisibility(PortInputView portInputView, ShaderPort port) + { + SetElementVisible(portInputView, !port.slot.isConnected); + port.parent.style.visibility = port.style.visibility; + portInputView.MarkDirtyRepaint(); + } + + void SetElementVisible(VisualElement element, bool isVisible) { - var updatedWidth = topContainer.layout.width + deltaSize.x; - var updatedHeight = m_PreviewImage.layout.height + deltaSize.y; + const string k_HiddenClassList = "hidden"; - var previewNode = node as PreviewNode; - if (previewNode != null) + if (isVisible) { - previewNode.SetDimensions(updatedWidth, updatedHeight); - UpdateSize(); + // Restore default value for visibility by setting it to StyleKeyword.Null. + // Setting it to Visibility.Visible would make it visible even if parent is hidden. + element.style.visibility = StyleKeyword.Null; + element.RemoveFromClassList(k_HiddenClassList); + } + else + { + element.style.visibility = Visibility.Hidden; + element.AddToClassList(k_HiddenClassList); } } @@ -776,27 +753,13 @@ void UpdatePreviewTexture() } } - void UpdateSize() - { - var previewNode = node as PreviewNode; - - if (previewNode == null) - return; - - var width = previewNode.width; - var height = previewNode.height; - - m_PreviewImage.style.height = height; - m_PreviewImage.style.width = width; - } - public void Dispose() { - foreach (var portInputView in m_PortInputContainer.Children().OfType()) + foreach (var portInputView in inputContainer.Query().ToList()) portInputView.Dispose(); node = null; - ((VisualElement)this).userData = null; + userData = null; if (m_PreviewRenderData != null) { m_PreviewRenderData.onPreviewChanged -= UpdatePreviewTexture; diff --git a/com.unity.shadergraph/Editor/Drawing/Views/PortInputView.cs b/com.unity.shadergraph/Editor/Drawing/Views/PortInputView.cs index e5f7f5ddaeb..95fed789b1e 100644 --- a/com.unity.shadergraph/Editor/Drawing/Views/PortInputView.cs +++ b/com.unity.shadergraph/Editor/Drawing/Views/PortInputView.cs @@ -2,7 +2,6 @@ using UnityEngine; using UnityEditor.Experimental.GraphView; using UnityEngine.UIElements; -using UnityEngine.UIElements.StyleSheets; namespace UnityEditor.ShaderGraph.Drawing { @@ -48,9 +47,7 @@ public PortInputView(MaterialSlot slot) m_Container = new VisualElement { name = "container" }; { - m_Control = this.slot.InstantiateControl(); - if (m_Control != null) - m_Container.Add(m_Control); + CreateControl(); var slotElement = new VisualElement { name = "slot" }; { @@ -60,12 +57,10 @@ public PortInputView(MaterialSlot slot) } Add(m_Container); - m_Container.visible = m_EdgeControl.visible = m_Control != null; - RegisterCallback(OnCustomStyleResolved); } - private void OnCustomStyleResolved(CustomStyleResolvedEvent e) + void OnCustomStyleResolved(CustomStyleResolvedEvent e) { Color colorValue; @@ -96,22 +91,30 @@ void Recreate() AddToClassList("type" + m_SlotType); if (m_Control != null) { - var disposable = m_Control as IDisposable; - if (disposable != null) + if (m_Control is IDisposable disposable) disposable.Dispose(); m_Container.Remove(m_Control); } + CreateControl(); + } + + void CreateControl() + { m_Control = slot.InstantiateControl(); if (m_Control != null) + { m_Container.Insert(0, m_Control); - - m_Container.visible = m_EdgeControl.visible = m_Control != null; + } + else + { + // Some slot types don't support an input control, so hide this + m_Container.visible = m_EdgeControl.visible = false; + } } public void Dispose() { - var disposable = m_Control as IDisposable; - if (disposable != null) + if (m_Control is IDisposable disposable) disposable.Dispose(); } } diff --git a/com.unity.shadergraph/Editor/Drawing/Views/PropertyNodeView.cs b/com.unity.shadergraph/Editor/Drawing/Views/PropertyNodeView.cs index b08e5079d62..dd708db63ee 100644 --- a/com.unity.shadergraph/Editor/Drawing/Views/PropertyNodeView.cs +++ b/com.unity.shadergraph/Editor/Drawing/Views/PropertyNodeView.cs @@ -233,6 +233,12 @@ public void UpdatePortInputTypes() { } + public bool FindPort(SlotReference slot, out ShaderPort port) + { + port = output as ShaderPort; + return port != null && port.slot.slotReference.Equals(slot); + } + public void OnModified(ModificationScope scope) { if (scope == ModificationScope.Graph) diff --git a/com.unity.shadergraph/Editor/Drawing/Views/ShaderPort.cs b/com.unity.shadergraph/Editor/Drawing/Views/ShaderPort.cs index 86c014b0612..fab6566f7bd 100644 --- a/com.unity.shadergraph/Editor/Drawing/Views/ShaderPort.cs +++ b/com.unity.shadergraph/Editor/Drawing/Views/ShaderPort.cs @@ -15,7 +15,7 @@ sealed class ShaderPort : Port MaterialSlot m_Slot; - public static Port Create(MaterialSlot slot, IEdgeConnectorListener connectorListener) + public static ShaderPort Create(MaterialSlot slot, IEdgeConnectorListener connectorListener) { var port = new ShaderPort(Orientation.Horizontal, slot.isInputSlot ? Direction.Input : Direction.Output, slot.isInputSlot ? Capacity.Single : Capacity.Multi, null) @@ -45,6 +45,14 @@ public MaterialSlot slot visualClass = slot.concreteValueType.ToClassName(); } } + + public Action OnDisconnect; + + public override void Disconnect(Edge edge) + { + base.Disconnect(edge); + OnDisconnect?.Invoke(this); + } } static class ShaderPortExtensions diff --git a/com.unity.shadergraph/Editor/Resources/Styles/MaterialNodeView.uss b/com.unity.shadergraph/Editor/Resources/Styles/MaterialNodeView.uss index e035ffc7ccc..39a4b9671e6 100644 --- a/com.unity.shadergraph/Editor/Resources/Styles/MaterialNodeView.uss +++ b/com.unity.shadergraph/Editor/Resources/Styles/MaterialNodeView.uss @@ -1,3 +1,7 @@ +MaterialNodeView { + overflow: visible; +} + MaterialNodeView.graphElement.node.MaterialNode { margin-top: 0; margin-bottom: 0; @@ -113,11 +117,9 @@ MaterialNodeView > #resize { cursor: resize-up-left; } -MaterialNodeView > #portInputContainer { +MaterialNodeView PortInputView { position: absolute; - width: 232px; - left: -220px; - top: 42px; + left: -224px; } MaterialNodeView > #settings-container {