Skip to content

Commit 8ec2362

Browse files
julienf-unityEvergreen
authored andcommitted
[VFX] Fix and optimize model invalidation
This PR fixes a regression that was introduced by https://github.cds.internal.unity3d.com/unity/unity/pull/41986. The regression caused a change in node settings to not systematically trigger a recompile in auto compile mode. The PR also adds some perf improvements regarding invalidation events: - Don't generate a UI invalidation event every time resync slots is called due to applying property attributes. - Improve the way we determinie whether a context can accept blocks or not.
1 parent 6be3fde commit 8ec2362

12 files changed

Lines changed: 148 additions & 54 deletions

File tree

Packages/com.unity.visualeffectgraph/Editor/GraphView/Elements/Controllers/VFXOperatorController.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ public VFXOperatorController(VFXOperator model, VFXViewController viewController
3434
{
3535
subgraphOperator.RecreateCopy();
3636
model.ResyncSlots(false);
37-
model.UpdateOutputExpressions();
37+
model.MarkOutputExpressionsAsOutOfDate();
38+
model.UpdateOutputExpressionsIfNeeded();
3839
}
3940
catch (Exception e)
4041
{

Packages/com.unity.visualeffectgraph/Editor/GraphView/Elements/VFXContextUI.cs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,7 @@ class VFXContextUI : VFXNodeUI
4040
}
4141
protected override void OnNewController()
4242
{
43-
foreach (var descriptor in VFXLibrary.GetBlocks())
44-
{
45-
var model = descriptor.CreateInstance();
46-
if (controller.model.AcceptChild(model))
47-
{
48-
m_CanHaveBlocks = true;
49-
break;
50-
}
51-
}
43+
m_CanHaveBlocks = controller.model.CanHaveBlocks();
5244
}
5345

5446
public bool canHaveBlocks { get => m_CanHaveBlocks; }

Packages/com.unity.visualeffectgraph/Editor/Models/Contexts/VFXContext.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ enum VFXContextType
2929
InitAndUpdateAndOutput = Init | Update | Output,
3030
UpdateAndOutput = Update | Output,
3131
All = Init | Update | Output | Spawner | Subgraph,
32+
33+
CanHaveBlocks = ~(OutputEvent | Event | SpawnerGPU),
3234
};
3335

3436
[Flags]
@@ -272,6 +274,11 @@ public virtual bool Accept(VFXBlock block, int index = -1)
272274
return ((block.compatibleContexts & contextType) != 0) && ((block.compatibleData & testedType) != 0);
273275
}
274276

277+
public bool CanHaveBlocks()
278+
{
279+
return (contextType & VFXContextType.CanHaveBlocks) != 0;
280+
}
281+
275282
protected override void OnAdded()
276283
{
277284
base.OnAdded();

Packages/com.unity.visualeffectgraph/Editor/Models/Operators/VFXAbstractOperatorNew.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -587,8 +587,11 @@ public void SetOperandType(int index, Type type)
587587
return;
588588
}
589589

590-
m_Operands[index].type = type;
591-
Invalidate(InvalidationCause.kSettingChanged);
590+
if (!m_Operands[index].type.Equals(type))
591+
{
592+
m_Operands[index].type = type;
593+
Invalidate(InvalidationCause.kSettingChanged);
594+
}
592595
}
593596

594597
public void OperandMoved(int movedIndex, int targetIndex)

Packages/com.unity.visualeffectgraph/Editor/Models/Operators/VFXOperator.cs

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,31 +28,6 @@ private static void GetSlotPredicateRecursive(List<VFXSlot> result, IEnumerable<
2828
}
2929
}
3030

31-
// As connections changed can be triggered from ResyncSlots, we need to make sure it is not reentrant
32-
[NonSerialized]
33-
private bool m_ResyncingSlots = false;
34-
35-
public override bool ResyncSlots(bool notify)
36-
{
37-
bool changed = false;
38-
if (!m_ResyncingSlots)
39-
{
40-
m_ResyncingSlots = true;
41-
try
42-
{
43-
changed = base.ResyncSlots(notify);
44-
if (changed && notify)
45-
foreach (var slot in outputSlots) // invalidate expressions on output slots
46-
slot.InvalidateExpressionTree();
47-
}
48-
finally
49-
{
50-
m_ResyncingSlots = false;
51-
}
52-
}
53-
return changed;
54-
}
55-
5631
protected override void OnInvalidate(VFXModel model, InvalidationCause cause)
5732
{
5833
var outputSlotSpaceable = outputSlots.Where(o => o.spaceable);
@@ -79,23 +54,27 @@ protected override void OnInvalidate(VFXModel model, InvalidationCause cause)
7954
}
8055
}
8156

82-
if (cause == InvalidationCause.kConnectionChanged)
83-
{
84-
if (model is VFXSlot slot && slot.direction == VFXSlot.Direction.kInput)
85-
ResyncSlots(true);
86-
}
57+
bool isInputSlot = model is VFXSlot && ((VFXSlot)model).direction == VFXSlot.Direction.kInput;
8758

8859
if (cause == InvalidationCause.kParamChanged ||
8960
cause == InvalidationCause.kExpressionValueInvalidated)
9061
{
91-
if (model is VFXSlot && ((VFXSlot)model).direction == VFXSlot.Direction.kInput)
62+
if (isInputSlot)
9263
{
9364
foreach (var slot in outputSlots)
9465
slot.Invalidate(InvalidationCause.kExpressionValueInvalidated);
9566
}
9667
}
9768

9869
base.OnInvalidate(model, cause);
70+
71+
if (cause == InvalidationCause.kSettingChanged ||
72+
(isInputSlot && (cause == InvalidationCause.kExpressionInvalidated || cause == InvalidationCause.kConnectionChanged)))
73+
{
74+
MarkOutputExpressionsAsOutOfDate();
75+
foreach (var slot in outputSlots) // invalidate expressions on output slots
76+
slot.InvalidateExpressionTree();
77+
}
9978
}
10079

10180
public override VFXSpace GetOutputSpaceFromSlot(VFXSlot outputSlot)
@@ -106,7 +85,7 @@ public override VFXSpace GetOutputSpaceFromSlot(VFXSlot outputSlot)
10685
: VFXSpace.None;
10786
}
10887

109-
public override sealed void UpdateOutputExpressions()
88+
protected override sealed void UpdateOutputExpressions()
11089
{
11190
var outputSlotWithExpression = new List<VFXSlot>();
11291
var inputSlotWithExpression = new List<VFXSlot>();

Packages/com.unity.visualeffectgraph/Editor/Models/Operators/VFXSubgraphOperator.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,8 @@ public override void CheckGraphBeforeImport()
241241
// If the graph is reimported it can be because one of its dependency such as the subgraphs, has been changed.
242242
if (!VFXGraph.explicitCompile)
243243
{
244-
ResyncSlots(true);
244+
MarkOutputExpressionsAsOutOfDate();
245+
ResyncSlots(true);
245246
ResyncCustomAttributes();
246247
}
247248
}

Packages/com.unity.visualeffectgraph/Editor/Models/Parameters/VFXParameter.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ public void ResetOutputValueExpression()
214214
{
215215
if (!isOutput)
216216
{
217+
MarkOutputExpressionsAsOutOfDate();
217218
m_ExprSlots = outputSlots[0].GetVFXValueTypeSlots().ToArray();
218219
m_ValueExpr = m_ExprSlots.Select(t => t.DefaultExpression(valueMode)).ToArray();
219220
}
@@ -421,6 +422,7 @@ protected sealed override void OnInvalidate(VFXModel model, InvalidationCause ca
421422

422423
if (valueExprChanged)
423424
{
425+
MarkOutputExpressionsAsOutOfDate();
424426
m_ValueExpr = valueExpr;
425427
outputSlots[0].InvalidateExpressionTree();
426428
Invalidate(InvalidationCause.kExpressionGraphChanged); // As we need to update exposed list event if not connected to a compilable context
@@ -737,7 +739,7 @@ public void UpdateDefaultExpressionValue()
737739
}
738740
}
739741

740-
public override void UpdateOutputExpressions()
742+
protected override void UpdateOutputExpressions()
741743
{
742744
if (!isOutput)
743745
{

Packages/com.unity.visualeffectgraph/Editor/Models/Slots/VFXSlot.cs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -742,10 +742,12 @@ public void UpdateAttributes(VFXPropertyAttributes attributes, bool notify)
742742
{
743743
if (notify)
744744
{
745+
bool wasInitialized = m_Property.attributes.IsInitialized;
745746
if (!m_Property.attributes.IsEqual(attributes))
746747
{
747748
m_Property.attributes = attributes;
748-
Invalidate(InvalidationCause.kUIChangedTransient); // TODO This will trigger a setDirty while it shouldn't as property attributes are not serialized
749+
if (wasInitialized) // No invalidate at init
750+
Invalidate(InvalidationCause.kUIChangedTransient); // TODO This will trigger a setDirty while it shouldn't as property attributes are not serialized
749751
}
750752
}
751753
else // fast path without comparison
@@ -919,6 +921,7 @@ private void RecomputeExpressionTree()
919921
{
920922
// Start from the top most parent
921923
var masterSlot = GetMasterSlot();
924+
bool wasUpToDate = masterSlot.m_ExpressionTreeUpToDate;
922925

923926
// When deserializing, default expression wont be initialized
924927
if (!m_DefaultExpressionInitialized)
@@ -945,7 +948,7 @@ private void RecomputeExpressionTree()
945948
{
946949
if (owner != null)
947950
{
948-
owner.UpdateOutputExpressions();
951+
owner.UpdateOutputExpressionsIfNeeded();
949952
// Update outputs can trigger an invalidate, it can be reentrant. Just check if we're up to date after that and early out
950953
if (m_ExpressionTreeUpToDate)
951954
return;
@@ -995,8 +998,9 @@ private void RecomputeExpressionTree()
995998
s[i].SetOutExpression(exp != null ? exp[i] : s[i].m_InExpression, toInvalidate, masterSlot.owner != null ? masterSlot.owner.GetOutputSpaceFromSlot(s) : VFXSpace.None);
996999
});
9971000

998-
foreach (var slot in toInvalidate)
999-
slot.InvalidateExpressionTree();
1001+
if (wasUpToDate)
1002+
foreach (var slot in toInvalidate)
1003+
slot.InvalidateExpressionTree();
10001004
}
10011005

10021006
private static VFXExpression ApplySpaceConversion(VFXExpression exp, VFXSlot destSlot, VFXSlot sourceSlot)
@@ -1066,6 +1070,9 @@ private string GetOwnerType()
10661070

10671071
public void InvalidateExpressionTree()
10681072
{
1073+
if (!m_ExpressionTreeUpToDate)
1074+
return;
1075+
10691076
var masterSlot = GetMasterSlot();
10701077

10711078
masterSlot.PropagateToChildren(s =>

Packages/com.unity.visualeffectgraph/Editor/Models/VFXGraph.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ void RecurseSubgraphRecreateCopy(IEnumerable<VFXModel> children)
12741274
{
12751275
operatorChild.RecreateCopy();
12761276
if (operatorChild.ResyncSlots(true))
1277-
operatorChild.UpdateOutputExpressions();
1277+
operatorChild.UpdateOutputExpressionsIfNeeded();
12781278
}
12791279
}
12801280
}
@@ -1309,7 +1309,7 @@ void RecurseSubgraphPatchInputExpression(IEnumerable<VFXModel> children)
13091309
else if (child is VFXSubgraphOperator operatorChild)
13101310
{
13111311
operatorChild.ResyncSlots(false);
1312-
operatorChild.UpdateOutputExpressions();
1312+
operatorChild.UpdateOutputExpressionsIfNeeded();
13131313
}
13141314
}
13151315
foreach (var child in children)

Packages/com.unity.visualeffectgraph/Editor/Models/VFXSlotContainerModel.cs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ interface IVFXSlotContainer
2727

2828
int GetSlotIndex(VFXSlot slot);
2929

30-
void UpdateOutputExpressions();
30+
void UpdateOutputExpressionsIfNeeded();
3131

3232
void ClearSlots();
3333
bool ResyncSlots(bool notify);
@@ -232,6 +232,7 @@ public override void Sanitize(int version)
232232
public override void OnUnknownChange()
233233
{
234234
base.OnUnknownChange();
235+
m_OutputExpressionsUpToDate = false;
235236
ResyncSlots(false);
236237
}
237238

@@ -448,13 +449,37 @@ public bool IsPathExpanded(string fieldPath)
448449
return m_expandedPaths.Contains(fieldPath);
449450
}
450451

451-
public virtual void UpdateOutputExpressions() { }
452+
protected virtual void UpdateOutputExpressions() { }
453+
454+
[NonSerialized]
455+
private bool m_OutputExpressionsUpToDate = false;
456+
public void UpdateOutputExpressionsIfNeeded()
457+
{
458+
if (!m_OutputExpressionsUpToDate)
459+
{
460+
UpdateOutputExpressions();
461+
m_OutputExpressionsUpToDate = true;
462+
}
463+
}
464+
465+
public void MarkOutputExpressionsAsOutOfDate()
466+
{
467+
m_OutputExpressionsUpToDate = false;
468+
}
452469

453470
public virtual VFXSpace GetOutputSpaceFromSlot(VFXSlot slot)
454471
{
455472
return VFXSpace.None;
456473
}
457474

475+
public override void CheckGraphBeforeImport()
476+
{
477+
// The cache is here to avoid multiple output recomputation when updating the expression graph
478+
// So It's marked as out of date before compilation just to minimize risk of cache missed invalidation between different compilation
479+
MarkOutputExpressionsAsOutOfDate();
480+
base.CheckGraphBeforeImport();
481+
}
482+
458483
//[SerializeField]
459484
HashSet<string> m_expandedPaths = new HashSet<string>();
460485

0 commit comments

Comments
 (0)