Avoid Helix crash due to edge double values (#11031)

mmisol · web-flow · commit 1a1addf31985 · 2020-08-24T11:11:50.000-04:00
This prevents the display of geometry located in positions considered
invalid by Helix. This includes the special values PositiveInfinity,
NegativeInfinity and NaN.

Also NaN is checked as a special value when unmarhsalling data from an
FFI. Special values PositiveInfinity and NegativeInfinity were already
handled. This allows to generate a warning over the node with an
appropriate message.
diff --git a/src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs b/src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs
@@ -1607,6 +1607,8 @@ private void ToggleTreeViewItemHighlighting(string path, bool isSelected)
         /// <param name="packages">An <see cref="IEnumerable"/> of <see cref="HelixRenderPackage"/>.</param>
         internal virtual void AggregateRenderPackages(IEnumerable<HelixRenderPackage> packages)
         {
+            packages = FilterOutInvalidPackages(packages);
+
             IEnumerable<string> customNodeIdents = null;
             if (InCustomNode())
             {
@@ -1809,6 +1811,29 @@ internal virtual void AggregateRenderPackages(IEnumerable<HelixRenderPackage> pa
             }
         }
 
+        /// <summary>
+        /// Filters out packages that are considered invalid by Helix. This includes any package
+        /// that has a coordinate with a special value like NaN or Infinite.
+        /// </summary>
+        /// <param name="packages">Original packages to render</param>
+        /// <returns>List of packages considered valid</returns>
+        private IEnumerable<HelixRenderPackage> FilterOutInvalidPackages(IEnumerable<HelixRenderPackage> packages)
+        {
+            List<HelixRenderPackage> result = new List<HelixRenderPackage>();
+
+            foreach (var package in packages)
+            {
+                if (!package.Points.Positions.Any(v => v.IsInvalid()) &&
+                    !package.Lines.Positions.Any(v => v.IsInvalid()) &&
+                    !package.Mesh.Positions.Any(v => v.IsInvalid()))
+                {
+                    result.Add(package);
+                }
+            }
+
+            return result;
+        }
+
         private void AddLabelPlace(string nodeId, Vector3 pos, IRenderPackage rp)
         {
             if (!labelPlaces.ContainsKey(nodeId))
@@ -2481,6 +2506,13 @@ internal static double DistanceToPlane(this Vector3 point, Vector3 planeOrigin,
         {
             return Vector3.Dot(planeNormal, (point - planeOrigin));
         }
+
+        internal static bool IsInvalid(this Vector3 point)
+        {
+            return float.IsNaN(point.X) || float.IsInfinity(point.X) ||
+                float.IsNaN(point.Y) || float.IsInfinity(point.Y) ||
+                float.IsNaN(point.Z) || float.IsInfinity(point.Z);
+        }
     }
 
     internal static class DoubleExtensions
diff --git a/src/Engine/ProtoCore/FFI/CLRObjectMarshaler.cs b/src/Engine/ProtoCore/FFI/CLRObjectMarshaler.cs
@@ -159,7 +159,7 @@ public override StackValue Marshal(object obj, ProtoCore.Runtime.Context context
 
         public override object UnMarshal(StackValue dsObject, ProtoCore.Runtime.Context context, Interpreter dsi, Type type)
         {
-            if (dsObject.DoubleValue > MaxValue || dsObject.DoubleValue < MinValue)
+            if (dsObject.DoubleValue > MaxValue || dsObject.DoubleValue < MinValue || double.IsNaN(dsObject.DoubleValue))
             {
                 string message = String.Format(Resources.kFFIInvalidCast, dsObject.DoubleValue, type.Name, MinValue, MaxValue);
                 dsi.LogWarning(ProtoCore.Runtime.WarningID.TypeMismatch, message);
diff --git a/src/VisualizationTests/CustomGraphicItemVisualizationTest.cs b/src/VisualizationTests/CustomGraphicItemVisualizationTest.cs
@@ -0,0 +1,37 @@
+﻿using System.Collections.Generic;
+using System.Linq;
+using Dynamo.Graph.Nodes;
+using NUnit.Framework;
+
+namespace WpfVisualizationTests
+{
+    [TestFixture]
+    public class CustomGraphicItemVisualizationTest : VisualizationTest
+    {
+        protected override void GetLibrariesToPreload(List<string> libraries)
+        {
+            base.GetLibrariesToPreload(libraries);
+            libraries.Add("FFITarget.dll");
+        }
+
+        [Test]
+        public void TestInvalidGeometryIsFilteredOut()
+        {
+            OpenVisualizationTest("NaN.dyn");
+
+            // Only the default scene items should be present
+            Assert.AreEqual(3, BackgroundPreviewGeometry.Count());
+
+            // Nodes should be set to warning status with an appropriate message
+            var nanNode = Model.CurrentWorkspace.Nodes.First(n => n.Name == "NaN");
+            Assert.AreEqual(ElementState.Warning, nanNode.State);
+            StringAssert.Contains("'NaN' is being cast to 'Double'", nanNode.ToolTipText);
+            var negInfNode = Model.CurrentWorkspace.Nodes.First(n => n.Name == "NegativeInfinity");
+            Assert.AreEqual(ElementState.Warning, negInfNode.State);
+            StringAssert.Contains("'-∞' is being cast to 'Double'", negInfNode.ToolTipText);
+            var posInfNode = Model.CurrentWorkspace.Nodes.First(n => n.Name == "PositiveInfinity");
+            Assert.AreEqual(ElementState.Warning, posInfNode.State);
+            StringAssert.Contains("'∞' is being cast to 'Double'", posInfNode.ToolTipText);
+        }
+    }
+}
diff --git a/src/VisualizationTests/PerformanceVisualizationTests.cs b/src/VisualizationTests/PerformanceVisualizationTests.cs
@@ -0,0 +1,40 @@
+﻿using System.Collections;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using NUnit.Framework;
+
+namespace WpfVisualizationTests
+{
+    /// <summary>
+    /// Runs the performance graphs as visualization tests. Their purpose is to provide rendering timings only.
+    /// For that, you should make sure the renderTimer Stopwatch in HelixWatch3DViewModel is enabled and logging to Console.
+    /// You should run test including only the PerformanceVisualization category. Output should be redirected to a file.
+    /// You can extract the timings using grep and output them to a CSV file for ease of postprocessing.
+    /// </summary>
+    [TestFixture]
+    [Category("PerformanceVisualization")]
+    [Category("Failure")] // Not really a failure, we just don't want them to run on a normal build.
+    public class PerformanceVisualizationTests : VisualizationTest
+    {
+        // Determines how many times each performance graph will be run.
+        const int SamplingAmount = 10;
+
+        [Test]
+        [TestCaseSource("PerformanceTestSource")]
+        public void TestVisualizationPerformance(string filePath)
+        {
+            ViewModel.OpenCommand.Execute(filePath);
+            RunCurrentModel();
+        }
+
+        public IEnumerable<string> PerformanceTestSource
+        {
+            get
+            {
+                var path = Path.Combine(GetTestDirectory(ExecutingDirectory), @"..\tools\Performance\DynamoPerformanceTests\graphs");
+                return new DirectoryInfo(path).GetFiles("*.dyn").SelectMany(fileInfo => Enumerable.Repeat(fileInfo.FullName, SamplingAmount));
+            }
+        }
+    }
+}
diff --git a/src/VisualizationTests/WpfVisualizationTests.csproj b/src/VisualizationTests/WpfVisualizationTests.csproj
@@ -99,12 +99,14 @@
       <Link>Properties\AssemblySharedInfo.cs</Link>
     </Compile>
     <Compile Include="CameraTests.cs" />
+    <Compile Include="CustomGraphicItemVisualizationTest.cs" />
     <Compile Include="CustomNodeTests.cs" />
     <Compile Include="HelixImageComparisonTests.cs" />
     <Compile Include="HelixRenderPackageTests.cs" />
     <Compile Include="HelixWatch3dViewModelPrimitiveTests.cs" />
     <Compile Include="HelixWatch3DViewModelTests.cs" />
     <Compile Include="HelixWatch3DViewModelUnitTests.cs" />
+    <Compile Include="PerformanceVisualizationTests.cs" />
     <Compile Include="Preview3DMemoryOutageTest.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Watch3DViewModelTests.cs" />
diff --git a/test/Engine/FFITarget/CustomLabel.cs b/test/Engine/FFITarget/CustomLabel.cs
@@ -0,0 +1,33 @@
+﻿using Autodesk.DesignScript.Interfaces;
+using Autodesk.DesignScript.Runtime;
+
+namespace FFITarget
+{
+    public class CustomLabel : IGraphicItem
+    {
+        private readonly double x;
+        private readonly double y;
+        private readonly double z;
+        private readonly string label;
+
+        private CustomLabel(double x, double y, double z, string label)
+        {
+            this.x = x;
+            this.y = y;
+            this.z = z;
+            this.label = label;
+        }
+
+        public static CustomLabel ByCoordinatesAndString(double x = 0, double y = 0, double z = 0, string label = "EMPTY_LABEL")
+        {
+            return new CustomLabel(x, y, z, label);
+        }
+
+        [IsVisibleInDynamoLibrary(false)]
+        public void Tessellate(IRenderPackage package, TessellationParameters parameters)
+        {
+            package.AddPointVertex(x, y, z);
+            package.Description = label;
+        }
+    }
+}
diff --git a/test/Engine/FFITarget/FFITarget.csproj b/test/Engine/FFITarget/FFITarget.csproj
@@ -53,6 +53,7 @@
     <Compile Include="ClassFunctionality.cs" />
     <Compile Include="ClassWithRefParams.cs" />
     <Compile Include="CodeCompletionClass.cs" />
+    <Compile Include="CustomLabel.cs" />
     <Compile Include="DefaultArguments.cs" />
     <Compile Include="DisposeTest.cs" />
     <Compile Include="DummyMath.cs" />
diff --git a/test/core/visualization/NaN.dyn b/test/core/visualization/NaN.dyn

Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,7 @@ public override StackValue Marshal(object obj, ProtoCore.Runtime.Context context`
`159`	`159`
`160`	`160`	`public override object UnMarshal(StackValue dsObject, ProtoCore.Runtime.Context context, Interpreter dsi, Type type)`
`161`	`161`	`{`
`162`		`- if (dsObject.DoubleValue > MaxValue \|\| dsObject.DoubleValue < MinValue)`
	`162`	`+ if (dsObject.DoubleValue > MaxValue \|\| dsObject.DoubleValue < MinValue \|\| double.IsNaN(dsObject.DoubleValue))`
`163`	`163`	`{`
`164`	`164`	`string message = String.Format(Resources.kFFIInvalidCast, dsObject.DoubleValue, type.Name, MinValue, MaxValue);`
`165`	`165`	`dsi.LogWarning(ProtoCore.Runtime.WarningID.TypeMismatch, message);`