Fix ApplyAddressingMode : clamp & mirror was overflowing, mirror has also a wrong pattern

Author: PaulDemeulenaere

TestProjects/VisualEffectGraph_HDRP/Assets/AllTests/Editor/Tests/VFXOperatorUtilityTests.cs

@@ -637,7 +637,119 @@ public void CheckBuiltinExpressionListed([ValueSource("CheckAllBuiltinExpression
             var operation = (UnityEngine.VFX.VFXExpressionOperation)Enum.Parse(typeof(UnityEngine.VFX.VFXExpressionOperation), expressionName);
             var referenceExpression = VFXBuiltInExpression.Find(operation);
             Assert.IsTrue(VFXDynamicBuiltInParameter.s_BuiltInInfo.Values.Any(o => o.expression == referenceExpression));
+        }
 
+        public struct ApplyAddressingModeTestCase
+        {
+            public ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode _mode, uint _count)
+            {
+                mode = _mode;
+                count = _count;
+                expectedSequence = new uint[Mathf.Max(50, 7 * (int)_count)];
+
+                //Naive implementation for reference
+                if (mode == VFXOperatorUtility.SequentialAddressingMode.Clamp)
+                {
+                    for (uint i = 0; i < expectedSequence.Length; ++i)
+                    {
+                        expectedSequence[i] = i < count ? i : count - 1;
+                    }
+                }
+                else if (mode == VFXOperatorUtility.SequentialAddressingMode.Wrap)
+                {
+                    uint current = 0u;
+                    for (uint i = 0; i < expectedSequence.Length; ++i)
+                    {
+                        expectedSequence[i] = current;
+                        current++;
+                        if (current >= count)
+                            current = 0u;
+                    }
+                }
+                else if (mode == VFXOperatorUtility.SequentialAddressingMode.Mirror)
+                {
+                    uint current = 0u;
+                    bool increment = true;
+                    for (uint i = 0; i < expectedSequence.Length; ++i)
+                    {
+                        expectedSequence[i] = current;
+                        if (increment)
+                        {
+                            current++;
+                            if (current >= count)
+                            {
+                                increment = false;
+                                current = count > 2u ? count - 2u : 0u;
+                            }
+                        }
+                        else
+                        {
+                            if (current == 0u)
+                            {
+                                increment = true;
+                                current = 1u;
+                            }
+                            else
+                            {
+                                current--;
+                            }
+                        }
+                    }
+                }
+            }
+
+            public VFXOperatorUtility.SequentialAddressingMode mode;
+            public uint count;
+            public uint[] expectedSequence;
+
+            public override string ToString()
+            {
+                return string.Format("{0}_{1}_{2}", mode.ToString(), count, expectedSequence.Length);
+            }
+        }
+
+
+        static readonly ApplyAddressingModeTestCase[] ApplyAddressingModeTestCase_ValueSource =
+        {
+            //The 0 case is always undefined
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Wrap, 1u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Wrap, 4u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Clamp, 1u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Clamp, 4u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 1u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 2u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 3u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 4u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 7u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 8u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 9u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 13u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 15u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 27u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 32u),
+            new ApplyAddressingModeTestCase(VFXOperatorUtility.SequentialAddressingMode.Mirror, 33u),
+        };
+
+        [Test]
+        public void CheckExpectedSequence_ApplyAddressingMode([ValueSource("ApplyAddressingModeTestCase_ValueSource")] ApplyAddressingModeTestCase addressingMode)
+        {
+            var computedSequence = new uint[addressingMode.expectedSequence.Length];
+            for (uint index = 0u; index < computedSequence.Length; ++index)
+            {
+                var indexExpr = VFXValue.Constant(index);
+                var countExpr = VFXValue.Constant(addressingMode.count);
+                var computed = VFXOperatorUtility.ApplyAddressingMode(indexExpr, countExpr, addressingMode.mode);
+
+                var context = new VFXExpression.Context(VFXExpressionContextOption.CPUEvaluation);
+                var result = context.Compile(computed);
+
+                computedSequence[index] = result.Get<uint>();
+            }
+
+            for (uint index = 0u; index < computedSequence.Length; ++index)
+            {
+                Assert.AreEqual(addressingMode.expectedSequence[index], computedSequence[index]);
+            }
         }
     }
 }

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

@@ -570,19 +570,25 @@ public enum SequentialAddressingMode
         static public VFXExpression ApplyAddressingMode(VFXExpression index, VFXExpression count, SequentialAddressingMode mode)
         {
             VFXExpression r = null;
+
             if (mode == SequentialAddressingMode.Wrap)
             {
-                r = VFXOperatorUtility.Modulo(index, count);
+                r = Modulo(index, count);
             }
             else if (mode == SequentialAddressingMode.Clamp)
             {
-                r = VFXOperatorUtility.Clamp(index, ZeroExpression[VFXValueType.Uint32], count, false);
+                var countMinusOne = count - OneExpression[VFXValueType.Uint32];
+                r = new VFXExpressionMin(index, countMinusOne);
             }
             else if (mode == SequentialAddressingMode.Mirror)
             {
-                var direction = VFXOperatorUtility.Modulo(index / count, VFXOperatorUtility.TwoExpression[VFXValueType.Uint32]);
-                var modulo = VFXOperatorUtility.Modulo(index, count);
-                r = VFXOperatorUtility.Lerp(modulo, count - modulo, direction);
+                var two = TwoExpression[VFXValueType.Uint32];
+                var cycle = count * two - two;
+                cycle = new VFXExpressionMax(cycle, OneExpression[VFXValueType.Uint32]);
+                var modulo = Modulo(index, cycle);
+                //TODO: Remove float casting for 10.x.x
+                var compare = new VFXExpressionCondition(VFXCondition.Less, new VFXExpressionCastUintToFloat(modulo), new VFXExpressionCastUintToFloat(count));
+                r = new VFXExpressionBranch(compare, modulo, cycle - modulo);
             }
             return r;
         }