Skip to content

Commit 0dbb605

Browse files
julienf-unityjulienf-unity
committed
Fix 1154328 - infinite recompile with rand (#93)
1 parent 84ad80d commit 0dbb605

File tree

8 files changed

+86
-21
lines changed

8 files changed

+86
-21
lines changed

‎TestProjects/VisualEffectGraph_HDRP/Assets/AllTests/Editor/Tests/VFXExpressionTests.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,26 @@ public void ProcessExpressionMatrixToVector4s()
236236

237237
Assert.AreEqual(new Vector4(1, 0, 0, 0), reduced.Get<Vector4>());
238238
}
239+
240+
[Test]
241+
public void CheckExpressionRandomEquality()
242+
{
243+
var obj0 = new object();
244+
var obj1 = new object();
245+
246+
var exp0 = new VFXExpressionRandom(true, new RandId(obj0));
247+
var exp1 = new VFXExpressionRandom(true, new RandId(obj0));
248+
var exp2 = new VFXExpressionRandom(false, new RandId(obj0));
249+
var exp3 = new VFXExpressionRandom(true, new RandId(obj1));
250+
var exp4 = new VFXExpressionRandom(true, new RandId(obj0, 1));
251+
252+
Assert.AreEqual(exp0, exp1);
253+
Assert.AreEqual(exp0.GetHashCode(), exp1.GetHashCode());
254+
255+
Assert.AreNotEqual(exp0, exp2);
256+
Assert.AreNotEqual(exp0, exp3);
257+
Assert.AreNotEqual(exp0, exp4);
258+
}
239259
}
240260
}
241261
#endif

‎com.unity.visualeffectgraph/Editor/Expressions/VFXExpressionRandom.cs‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,57 @@
66

77
namespace UnityEditor.VFX
88
{
9+
struct RandId
10+
{
11+
public RandId(object owner, int id = 0)
12+
{
13+
this.owner = new WeakReference(owner);
14+
this.id = id;
15+
}
16+
17+
public override bool Equals(object obj)
18+
{
19+
if (!(obj is RandId))
20+
return false;
21+
22+
var other = (RandId)obj;
23+
return ReferenceEquals(owner.Target, other.owner.Target) && id == other.id;
24+
}
25+
26+
public override int GetHashCode()
27+
{
28+
// This is not good practice as hashcode will mutate when target gets destroyed but in our case we don't care.
29+
// Any entry in cache will just be lost, but it would have never been accessed anyway (as owner is lost)
30+
return (RuntimeHelpers.GetHashCode(owner.Target) * 397) ^ id;
31+
}
32+
33+
WeakReference owner;
34+
int id;
35+
}
36+
937
#pragma warning disable 0659
1038
class VFXExpressionRandom : VFXExpression
1139
{
12-
public VFXExpressionRandom(bool perElement = false) : base(perElement ? VFXExpression.Flags.PerElement : VFXExpression.Flags.None)
13-
{}
40+
public VFXExpressionRandom(bool perElement, RandId id) : base(perElement ? VFXExpression.Flags.PerElement : VFXExpression.Flags.None)
41+
{
42+
m_Id = id;
43+
}
1444

1545
public override bool Equals(object obj)
1646
{
17-
return ReferenceEquals(this, obj);
47+
if (!base.Equals(obj))
48+
return false;
49+
50+
var other = obj as VFXExpressionRandom;
51+
if (other == null)
52+
return false;
53+
54+
return m_Id.Equals(other.m_Id);
1855
}
1956

2057
protected override int GetInnerHashCode()
2158
{
22-
return RuntimeHelpers.GetHashCode(this);
59+
return (base.GetInnerHashCode() * 397) ^ m_Id.GetHashCode();
2360
}
2461

2562
public override VFXExpressionOperation operation { get { return VFXExpressionOperation.GenerateRandom; } }
@@ -39,6 +76,8 @@ public override IEnumerable<VFXAttributeInfo> GetNeededAttributes()
3976
if (Is(Flags.PerElement))
4077
yield return new VFXAttributeInfo(VFXAttribute.Seed, VFXAttributeMode.ReadWrite);
4178
}
79+
80+
private RandId m_Id;
4281
}
4382

4483
class VFXExpressionFixedRandom : VFXExpression

‎com.unity.visualeffectgraph/Editor/Models/Blocks/Implementations/Position/PositionCircle.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ public override IEnumerable<VFXNamedExpression> parameters
4242

4343
VFXExpression theta = null;
4444
if (spawnMode == SpawnMode.Random)
45-
theta = arcCircle_arc * new VFXExpressionRandom(true);
45+
theta = arcCircle_arc * new VFXExpressionRandom(true, new RandId(this,0));
4646
else
4747
theta = arcCircle_arc * arcSequencer;
4848

4949
var one = VFXOperatorUtility.OneExpression[UnityEngine.VFX.VFXValueType.Float];
5050

51-
var rNorm = VFXOperatorUtility.Sqrt(volumeFactor + (one - volumeFactor) * new VFXExpressionRandom(true)) * arcCircleRadius;
51+
var rNorm = VFXOperatorUtility.Sqrt(volumeFactor + (one - volumeFactor) * new VFXExpressionRandom(true, new RandId(this, 1))) * arcCircleRadius;
5252
var sinTheta = new VFXExpressionSin(theta);
5353
var cosTheta = new VFXExpressionCos(theta);
5454

‎com.unity.visualeffectgraph/Editor/Models/Blocks/Implementations/Spawn/VFXSpawnerSetAttribute.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,18 @@ public override IEnumerable<VFXNamedExpression> parameters
7070

7171
if (size == 1)
7272
{
73-
random = new VFXExpressionRandom();
73+
random = new VFXExpressionRandom(false, new RandId(this, 0));
7474
}
7575
else
7676
{
7777
switch (randomMode)
7878
{
7979
default:
8080
case RandomMode.PerComponent:
81-
random = new VFXExpressionCombine(Enumerable.Repeat(0, size).Select(_ => new VFXExpressionRandom()).ToArray());
81+
random = new VFXExpressionCombine(Enumerable.Repeat(0, size).Select(_ => new VFXExpressionRandom(false, new RandId(this, 1))).ToArray());
8282
break;
8383
case RandomMode.Uniform:
84-
random = new VFXExpressionCombine(Enumerable.Repeat(new VFXExpressionRandom(), size).ToArray());
84+
random = new VFXExpressionCombine(Enumerable.Repeat(new VFXExpressionRandom(false, new RandId(this, 2)), size).ToArray());
8585
break;
8686
}
8787
}

‎com.unity.visualeffectgraph/Editor/Models/Contexts/Implementations/VFXBasicSpawner.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,9 @@ protected override IEnumerable<VFXPropertyWithValue> inputProperties
223223
}
224224
}
225225

226-
static VFXExpression RandomFromVector2(VFXExpression input)
226+
static VFXExpression RandomFromVector2(VFXExpression input, RandId randId)
227227
{
228-
return VFXOperatorUtility.Lerp(input.x, input.y, new VFXExpressionRandom());
228+
return VFXOperatorUtility.Lerp(input.x, input.y, new VFXExpressionRandom(false, randId));
229229
}
230230

231231
public override VFXExpressionMapper GetExpressionMapper(VFXDeviceTarget target)
@@ -241,31 +241,31 @@ public override VFXExpressionMapper GetExpressionMapper(VFXDeviceTarget target)
241241
{
242242
var expression = mapperFromContext.FromNameAndId("LoopDuration", -1);
243243
if (loopDuration == LoopMode.Random)
244-
expression = RandomFromVector2(expression);
244+
expression = RandomFromVector2(expression, new RandId(this, 0));
245245
mapper.AddExpression(expression, "LoopDuration", -1);
246246
}
247247

248248
if (loopCount != LoopMode.Infinite)
249249
{
250250
var expression = mapperFromContext.FromNameAndId("LoopCount", -1);
251251
if (loopCount == LoopMode.Random)
252-
expression = new VFXExpressionCastFloatToInt(RandomFromVector2(expression));
252+
expression = new VFXExpressionCastFloatToInt(RandomFromVector2(expression, new RandId(this, 1)));
253253
mapper.AddExpression(expression, "LoopCount", -1);
254254
}
255255

256256
if (delayBeforeLoop != DelayMode.None)
257257
{
258258
var expression = mapperFromContext.FromNameAndId("DelayBeforeLoop", -1);
259259
if (delayBeforeLoop == DelayMode.Random)
260-
expression = RandomFromVector2(expression);
260+
expression = RandomFromVector2(expression, new RandId(this, 2));
261261
mapper.AddExpression(expression, "DelayBeforeLoop", -1);
262262
}
263263

264264
if (delayAfterLoop != DelayMode.None)
265265
{
266266
var expression = mapperFromContext.FromNameAndId("DelayAfterLoop", -1);
267267
if (delayAfterLoop == DelayMode.Random)
268-
expression = RandomFromVector2(expression);
268+
expression = RandomFromVector2(expression, new RandId(this, 3));
269269
mapper.AddExpression(expression, "DelayAfterLoop", -1);
270270
}
271271
return mapper;

‎com.unity.visualeffectgraph/Editor/Models/Operators/Implementations/ProbabilitySampling.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ protected sealed override VFXExpression[] BuildExpression(VFXExpression[] inputE
114114
if (m_Constant)
115115
rand = VFXOperatorUtility.FixedRandom(inputExpression.Last(), m_Seed);
116116
else
117-
rand = new VFXExpressionRandom(m_Seed == VFXSeedMode.PerParticle);
117+
rand = new VFXExpressionRandom(m_Seed == VFXSeedMode.PerParticle, new RandId(this));
118118
}
119119
else
120120
{

‎com.unity.visualeffectgraph/Editor/Models/Operators/Implementations/Random.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,7 @@ protected override IEnumerable<string> filteredOutSettings
6969

7070
protected override sealed VFXExpression[] BuildExpression(VFXExpression[] inputExpression)
7171
{
72-
VFXExpression rand = null;
73-
if (seed == VFXSeedMode.PerParticleStrip || constant)
74-
rand = VFXOperatorUtility.FixedRandom(inputExpression[2], seed);
75-
else
76-
rand = new VFXExpressionRandom(seed == VFXSeedMode.PerParticle);
72+
var rand = VFXOperatorUtility.BuildRandom(seed, constant, new RandId(this), inputExpression.Length > 2 ? inputExpression[2] : null);
7773

7874
return new[] { VFXOperatorUtility.Lerp(inputExpression[0], inputExpression[1], rand) };
7975
}

‎com.unity.visualeffectgraph/Editor/Models/Operators/VFXOperatorUtility.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,16 @@ static public VFXExpression CastFloat(VFXExpression from, VFXValueType toValueTy
529529
return combine;
530530
}
531531

532+
static public VFXExpression BuildRandom(VFXSeedMode seedMode, bool constant, RandId randId, VFXExpression seed = null)
533+
{
534+
if (seedMode == VFXSeedMode.PerParticleStrip || constant)
535+
{
536+
if (seed == null)
537+
throw new ArgumentNullException("seed");
538+
return FixedRandom(seed, seedMode);
539+
}
540+
return new VFXExpressionRandom(seedMode == VFXSeedMode.PerParticle, randId);
541+
}
532542
static public VFXExpression FixedRandom(uint hash, VFXSeedMode mode)
533543
{
534544
return FixedRandom(VFXValue.Constant<uint>(hash), mode);

0 commit comments

Comments
 (0)