[VFX] Indeterminate state not updated and label size changing

julienamsellem · Evergreen · commit b7ee57ccc888 · 2024-04-06T04:29:27.000Z
### Jira UUM-68044 Steps to reproduce: 1. Import (or reproduce) attached VFX 2. Observe first computed value in UX of Add => Expected intermediate status with `-` Actual results <img src="https://jira.unity3d.com/secure/attachment/1415346/1415346_image-2024-03-26-17-42-08-042.png" width="500" /> Expected results <img src="https://jira.unity3d.com/secure/attachment/1415345/1415345_image-2024-03-26-17-42-55-937.png" width="500" /> ### Jira UUM-68140 Steps to reproduce: 1. Open VFX 2. Create Update Context 3. Create block Set Axis X 4. Open Inspector change axis to Axis Z 5. Observe weird alignment of UI components Actual results <img src="https://jira.unity3d.com/secure/attachment/1416292/1416292_image-2024-03-27-17-28-31-096.png" width="500" /> Expected results <img src="https://jira.unity3d.com/secure/attachment/1416298/1416298_image-2024-03-27-17-39-57-915.png" width="500" /> ![Video](https://jira.unity3d.com/secure/attachment/1416293/_moving_alignement.mp4) In this same bug another issue is raised: When unlinking an edge on a collapsed port, the previously linked port is not hidden ![Unity_rcHEhtGQOe](https://media.github.cds.internal.unity3d.com/user/4003/files/4afec272-efce-4a83-8341-a5a6ddc7c323)
diff --git a/Packages/com.unity.visualeffectgraph/Editor/GraphView/Elements/VFXNodeUI.cs b/Packages/com.unity.visualeffectgraph/Editor/GraphView/Elements/VFXNodeUI.cs
@@ -166,9 +166,10 @@ public virtual void OnControllerChanged(ref ControllerChangedEvent e)
 
         protected virtual bool HasPosition() => true;
 
-        protected void SyncSettings()
+        private bool SyncSettings()
         {
             Profiler.BeginSample("VFXNodeUI.SyncSettings");
+            var hasChanged = false;
             var settings = controller.settings;
             var graphSettings = controller.model.GetSettings(false, VFXSettingAttribute.VisibleFlags.InGraph).ToArray();
 
@@ -179,6 +180,7 @@ protected void SyncSettings()
                 {
                     propertyRM.RemoveFromHierarchy();
                     m_Settings.Remove(propertyRM);
+                    hasChanged = true;
                 }
             }
 
@@ -191,6 +193,7 @@ protected void SyncSettings()
                     var setting = settings.Single(x => string.Compare(x.name, vfxSetting.field.Name, StringComparison.OrdinalIgnoreCase) == 0);
                     var propertyRM = AddSetting(setting);
                     settingsContainer.Insert(i, propertyRM);
+                    hasChanged = true;
                 }
             }
 
@@ -215,18 +218,17 @@ protected void SyncSettings()
             }
 
             Profiler.EndSample();
+            return hasChanged;
         }
 
-        void SyncAnchors()
+        bool SyncAnchors()
         {
             Profiler.BeginSample("VFXNodeUI.SyncAnchors");
             var hasResync = SyncAnchors(controller.inputPorts, inputContainer, controller.HasActivationAnchor);
             hasResync |= SyncAnchors(controller.outputPorts, outputContainer, false);
-            if (hasResync)
-            {
-                RefreshLayout();
-            }
             Profiler.EndSample();
+
+            return hasResync;
         }
 
         bool SyncAnchors(ReadOnlyCollection<VFXDataAnchorController> ports, VisualElement container, bool hasActivationPort)
@@ -373,15 +375,18 @@ protected virtual void SelfChange()
 
             base.expanded = controller.expanded;
 
-            SyncSettings();
-            SyncAnchors();
+            var needRefresh = SyncSettings();
+            needRefresh |= SyncAnchors();
             Profiler.BeginSample("VFXNodeUI.SelfChange The Rest");
             RefreshExpandedState();
             Profiler.EndSample();
             Profiler.EndSample();
 
-
             UpdateCollapse();
+            if (needRefresh)
+            {
+                EditorApplication.delayCall += RefreshLayout;
+            }
         }
 
         protected virtual VFXDataAnchor InstantiateDataAnchor(VFXDataAnchorController ctrl, VFXNodeUI node)
diff --git a/Packages/com.unity.visualeffectgraph/Editor/GraphView/Elements/VFXOperatorUI.cs b/Packages/com.unity.visualeffectgraph/Editor/GraphView/Elements/VFXOperatorUI.cs
@@ -6,7 +6,7 @@ namespace UnityEditor.VFX.UI
 {
     class VFXOperatorUI : VFXNodeUI
     {
-        private const float defaultOperatorLabelWidth = 30f;
+        private const float defaultOperatorLabelWidth = 10f;
 
         VisualElement m_EditButton;
         VisualElement m_EditContainer;
diff --git a/Packages/com.unity.visualeffectgraph/Editor/GraphView/Views/Controller/VFXViewController.cs b/Packages/com.unity.visualeffectgraph/Editor/GraphView/Views/Controller/VFXViewController.cs
@@ -390,6 +390,8 @@ private bool RecreateNodeEdges()
             foreach (var node in nodeToUpdate)
             {
                 node.UpdateAllEditable();
+                // Mark model as changed since it's connections have changed
+                OnObjectModified(node.model, false);
             }
 
             return nodeToUpdate.Any();
diff --git a/Packages/com.unity.visualeffectgraph/Editor/GraphView/Views/Properties/NumericPropertiesRM.cs b/Packages/com.unity.visualeffectgraph/Editor/GraphView/Views/Properties/NumericPropertiesRM.cs
@@ -21,13 +21,9 @@ public override float GetPreferredControlWidth()
 
         protected override void UpdateIndeterminate()
         {
-            if (field is BaseField<float> floatField)
+            if (field is IMixedValueSupport mixedValueSupport)
             {
-                floatField.showMixedValue = indeterminate;
-            }
-            else if (field is BaseField<int> intField)
-            {
-                intField.showMixedValue = indeterminate;
+                mixedValueSupport.showMixedValue = indeterminate;
             }
 
             if (m_Slider != null)
@@ -54,7 +50,7 @@ public override INotifyValueChanged<U> CreateField()
             VisualElement createdField;
             if (!HasValidRange(range))
             {
-                createdField = CreateSimpleField(ObjectNames.NicifyVariableName(provider.name));
+                createdField = CreateSimpleField(string.IsNullOrEmpty(provider.name) ? "  " : ObjectNames.NicifyVariableName(provider.name));
                 m_TextField = createdField.Q<TextElement>(null, "unity-text-element__selectable");
                 if (m_TextField != null)
                 {
diff --git a/Packages/com.unity.visualeffectgraph/Editor/UIResources/uss/PropertyRM.uss b/Packages/com.unity.visualeffectgraph/Editor/UIResources/uss/PropertyRM.uss
@@ -133,11 +133,6 @@
     -unity-text-align: middle-left;
 }
 
-/*.propertyrm .fieldContainer > *
-{
-    flex:1 1 auto;
-}*/
-
 .propertyrm:disabled
 {
     opacity: 1;
@@ -156,10 +151,10 @@
     padding-left:0;
 }
 
-.propertyrm .fieldContainer, .propertyrm .unity-float-field
+.propertyrm .fieldContainer
 {
     flex-direction:row;
-    flex:1 1 auto;
+    flex:1 0 auto;
     height: auto;
     margin-top: 0;
     margin-bottom: 0;
@@ -244,12 +239,6 @@ ColorPropertyRM .unity-float-field
     flex: 1 0 auto;
 }
 
-FloatPropertyRM .unity-float-field
-{
-    align-self:flex-end;
-    flex:1 0 auto;
-}
-
 IntPropertyRM .unity-integer-field
 {
     align-self:flex-end;
diff --git a/Packages/com.unity.visualeffectgraph/Editor/UIResources/uss/VFXDataAnchor.uss b/Packages/com.unity.visualeffectgraph/Editor/UIResources/uss/VFXDataAnchor.uss
@@ -85,7 +85,6 @@ VFXContextUI #output .VFXDataAnchor.port #connector
     padding-right: 0;
     margin-left: 0;
     flex-direction: row;
-    /*flex-direction: row-reverse;*/
 }
 
 .VFXDataAnchor.Output #type {

Original file line number	Diff line number	Diff line change
`@@ -166,9 +166,10 @@ public virtual void OnControllerChanged(ref ControllerChangedEvent e)`
`166`	`166`
`167`	`167`	`protected virtual bool HasPosition() => true;`
`168`	`168`
`169`		`- protected void SyncSettings()`
	`169`	`+ private bool SyncSettings()`
`170`	`170`	`{`
`171`	`171`	`Profiler.BeginSample("VFXNodeUI.SyncSettings");`
	`172`	`+ var hasChanged = false;`
`172`	`173`	`var settings = controller.settings;`
`173`	`174`	`var graphSettings = controller.model.GetSettings(false, VFXSettingAttribute.VisibleFlags.InGraph).ToArray();`
`174`	`175`
`@@ -179,6 +180,7 @@ protected void SyncSettings()`
`179`	`180`	`{`
`180`	`181`	`propertyRM.RemoveFromHierarchy();`
`181`	`182`	`m_Settings.Remove(propertyRM);`
	`183`	`+ hasChanged = true;`
`182`	`184`	`}`
`183`	`185`	`}`
`184`	`186`
`@@ -191,6 +193,7 @@ protected void SyncSettings()`
`191`	`193`	`var setting = settings.Single(x => string.Compare(x.name, vfxSetting.field.Name, StringComparison.OrdinalIgnoreCase) == 0);`
`192`	`194`	`var propertyRM = AddSetting(setting);`
`193`	`195`	`settingsContainer.Insert(i, propertyRM);`
	`196`	`+ hasChanged = true;`
`194`	`197`	`}`
`195`	`198`	`}`
`196`	`199`
`@@ -215,18 +218,17 @@ protected void SyncSettings()`
`215`	`218`	`}`
`216`	`219`
`217`	`220`	`Profiler.EndSample();`
	`221`	`+ return hasChanged;`
`218`	`222`	`}`
`219`	`223`
`220`		`- void SyncAnchors()`
	`224`	`+ bool SyncAnchors()`
`221`	`225`	`{`
`222`	`226`	`Profiler.BeginSample("VFXNodeUI.SyncAnchors");`
`223`	`227`	`var hasResync = SyncAnchors(controller.inputPorts, inputContainer, controller.HasActivationAnchor);`
`224`	`228`	`hasResync \|= SyncAnchors(controller.outputPorts, outputContainer, false);`
`225`		`- if (hasResync)`
`226`		`- {`
`227`		`- RefreshLayout();`
`228`		`- }`
`229`	`229`	`Profiler.EndSample();`
	`230`	`+`
	`231`	`+ return hasResync;`
`230`	`232`	`}`
`231`	`233`
`232`	`234`	`bool SyncAnchors(ReadOnlyCollection<VFXDataAnchorController> ports, VisualElement container, bool hasActivationPort)`
`@@ -373,15 +375,18 @@ protected virtual void SelfChange()`
`373`	`375`
`374`	`376`	`base.expanded = controller.expanded;`
`375`	`377`
`376`		`- SyncSettings();`
`377`		`- SyncAnchors();`
	`378`	`+ var needRefresh = SyncSettings();`
	`379`	`+ needRefresh \|= SyncAnchors();`
`378`	`380`	`Profiler.BeginSample("VFXNodeUI.SelfChange The Rest");`
`379`	`381`	`RefreshExpandedState();`
`380`	`382`	`Profiler.EndSample();`
`381`	`383`	`Profiler.EndSample();`
`382`	`384`
`383`		`-`
`384`	`385`	`UpdateCollapse();`
	`386`	`+ if (needRefresh)`
	`387`	`+ {`
	`388`	`+ EditorApplication.delayCall += RefreshLayout;`
	`389`	`+ }`
`385`	`390`	`}`
`386`	`391`
`387`	`392`	`protected virtual VFXDataAnchor InstantiateDataAnchor(VFXDataAnchorController ctrl, VFXNodeUI node)`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ namespace UnityEditor.VFX.UI`
`6`	`6`	`{`
`7`	`7`	`class VFXOperatorUI : VFXNodeUI`
`8`	`8`	`{`
`9`		`- private const float defaultOperatorLabelWidth = 30f;`
	`9`	`+ private const float defaultOperatorLabelWidth = 10f;`
`10`	`10`
`11`	`11`	`VisualElement m_EditButton;`
`12`	`12`	`VisualElement m_EditContainer;`
Original file line number	Diff line number	Diff line change
`@@ -390,6 +390,8 @@ private bool RecreateNodeEdges()`
`390`	`390`	`foreach (var node in nodeToUpdate)`
`391`	`391`	`{`
`392`	`392`	`node.UpdateAllEditable();`
	`393`	`+ // Mark model as changed since it's connections have changed`
	`394`	`+ OnObjectModified(node.model, false);`
`393`	`395`	`}`
`394`	`396`
`395`	`397`	`return nodeToUpdate.Any();`
Original file line number	Diff line number	Diff line change
`@@ -21,13 +21,9 @@ public override float GetPreferredControlWidth()`
`21`	`21`
`22`	`22`	`protected override void UpdateIndeterminate()`
`23`	`23`	`{`
`24`		`- if (field is BaseField<float> floatField)`
	`24`	`+ if (field is IMixedValueSupport mixedValueSupport)`
`25`	`25`	`{`
`26`		`- floatField.showMixedValue = indeterminate;`
`27`		`- }`
`28`		`- else if (field is BaseField<int> intField)`
`29`		`- {`
`30`		`- intField.showMixedValue = indeterminate;`
	`26`	`+ mixedValueSupport.showMixedValue = indeterminate;`
`31`	`27`	`}`
`32`	`28`
`33`	`29`	`if (m_Slider != null)`
`@@ -54,7 +50,7 @@ public override INotifyValueChanged<U> CreateField()`
`54`	`50`	`VisualElement createdField;`
`55`	`51`	`if (!HasValidRange(range))`
`56`	`52`	`{`
`57`		`- createdField = CreateSimpleField(ObjectNames.NicifyVariableName(provider.name));`
	`53`	`+ createdField = CreateSimpleField(string.IsNullOrEmpty(provider.name) ? " " : ObjectNames.NicifyVariableName(provider.name));`
`58`	`54`	`m_TextField = createdField.Q<TextElement>(null, "unity-text-element__selectable");`
`59`	`55`	`if (m_TextField != null)`
`60`	`56`	`{`
Original file line number	Diff line number	Diff line change
`@@ -133,11 +133,6 @@`
`133`	`133`	`-unity-text-align: middle-left;`
`134`	`134`	`}`
`135`	`135`
`136`		`-/.propertyrm .fieldContainer > `
`137`		`-{`
`138`		`- flex:1 1 auto;`
`139`		`-}*/`
`140`		`-`
`141`	`136`	`.propertyrm:disabled`
`142`	`137`	`{`
`143`	`138`	`opacity: 1;`
`@@ -156,10 +151,10 @@`
`156`	`151`	`padding-left:0;`
`157`	`152`	`}`
`158`	`153`
`159`		`-.propertyrm .fieldContainer, .propertyrm .unity-float-field`
	`154`	`+.propertyrm .fieldContainer`
`160`	`155`	`{`
`161`	`156`	`flex-direction:row;`
`162`		`- flex:1 1 auto;`
	`157`	`+ flex:1 0 auto;`
`163`	`158`	`height: auto;`
`164`	`159`	`margin-top: 0;`
`165`	`160`	`margin-bottom: 0;`
`@@ -244,12 +239,6 @@ ColorPropertyRM .unity-float-field`
`244`	`239`	`flex: 1 0 auto;`
`245`	`240`	`}`
`246`	`241`
`247`		`-FloatPropertyRM .unity-float-field`
`248`		`-{`
`249`		`- align-self:flex-end;`
`250`		`- flex:1 0 auto;`
`251`		`-}`
`252`		`-`
`253`	`242`	`IntPropertyRM .unity-integer-field`
`254`	`243`	`{`
`255`	`244`	`align-self:flex-end;`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,6 @@ VFXContextUI #output .VFXDataAnchor.port #connector`
`85`	`85`	`padding-right: 0;`
`86`	`86`	`margin-left: 0;`
`87`	`87`	`flex-direction: row;`
`88`		`- /flex-direction: row-reverse;/`
`89`	`88`	`}`
`90`	`89`
`91`	`90`	`.VFXDataAnchor.Output #type {`