[Penumbra] Optimize adding points to a hull

Samples/Sandbox/SandboxGame.cs

@@ -31,7 +31,7 @@ public class SandboxGame : Game
         public SandboxGame()
         {
             var deviceManager = new GraphicsDeviceManager(this);
-            Content.RootDirectory = "Content";            
+            Content.RootDirectory = "Content";
             _penumbra = new PenumbraComponent(this)
             {
                 SpriteBatchTransformEnabled = false,
@@ -49,14 +49,14 @@ public SandboxGame()
             Components.Add(ui);
             _camera = new CameraMovementComponent(this);
             Components.Add(_camera);
-            Components.Add(new FpsGarbageComponent(this));            
+            Components.Add(new FpsGarbageComponent(this));
 
             // There's a bug when trying to change resolution during window resize.
             // https://github.com/mono/MonoGame/issues/3572
             deviceManager.PreferredBackBufferWidth = 1280;
             deviceManager.PreferredBackBufferHeight = 720;
-            Window.AllowUserResizing = false;            
-            IsMouseVisible = true;            
+            Window.AllowUserResizing = false;
+            IsMouseVisible = true;
         }
 
         protected override void LoadContent()

Source/Extensions.cs

@@ -0,0 +1,44 @@
+using Penumbra.Utilities;
+using System.Collections.Generic;
+
+namespace Penumbra
+{
+    /// <summary>
+    /// Provides extension methods for various types.
+    /// </summary>
+    public static class Extensions
+    {
+        /// <summary>
+        /// Adds the elements of the specified collection to the end of the <see cref="IList{T}"/>.
+        /// </summary>
+        /// <typeparam name="T">Type of collection elements.</typeparam>
+        /// <param name="listInterface">The <see cref="IList{T}"/> to add elements to.</param>
+        /// <param name="collection">
+        /// The collection whose elements should be added to the end of the <see cref="IList{T}"/>.
+        /// The collection itself cannot be <c>null</c>, but it can contain elements that are
+        /// <c>null</c>, if type <typeparamref name="T"/> is a reference type.
+        /// </param>
+        public static void AddRange<T>(this IList<T> listInterface, IEnumerable<T> collection)
+        {
+            // Use fast path in case of extended observable collection.
+            var extendedObservableCollection = listInterface as ExtendedObservableCollection<T>;
+            if (extendedObservableCollection != null)
+            {
+                extendedObservableCollection.AddRange(collection);
+                return;
+            }
+
+            // Use fast path in case of list.
+            var list = listInterface as List<T>;
+            if (list != null)
+            {
+                list.AddRange(collection);
+                return;
+            }
+
+            // Fallback to iterative add.
+            foreach (T item in collection)
+                listInterface.Add(item);
+        }
+    }
+}

Source/Hull.cs

@@ -1,6 +1,5 @@
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
-using System.Collections.Specialized;
 using Microsoft.Xna.Framework;
 using Penumbra.Geometry;
 using Penumbra.Graphics;
@@ -55,19 +54,7 @@ public Hull(IEnumerable<Vector2> points = null)
                     ConvertRawLocalPointsToLocalPoints();
             }
 
-            _rawLocalPoints.CollectionChanged += (s, e) =>
-            {
-                ValidateRawLocalPoints();
-                if (Valid)
-                {
-                    ConvertRawLocalPointsToLocalPoints();
-                    if (e.Action == NotifyCollectionChangedAction.Add)
-                        foreach (Vector2 point in e.NewItems)
-                            Logger.Write($"New point at {point}.");
-                    _worldDirty = true;
-                    _pointsDirty = true;
-                }
-            };
+            _rawLocalPoints.CollectionChanged += (s, e) => _pointsDirty = true;
         }
 
         /// <summary>
@@ -176,36 +163,20 @@ public Vector2 Scale
 
         internal void Update()
         {
-            if (_worldDirty)
-            {
+            if (_pointsDirty)
                 UpdatePoints();
 
-                // Calculate local to world transform.
-                Calculate.Transform(ref _position, ref _origin, ref _scale, _rotation, out LocalToWorld);
-
-                // Calculate points in world space.
-                WorldPoints.Clear();
-                int pointCount = LocalPoints.Count;
-                for (int i = 0; i < pointCount; i++)
-                {
-                    Vector2 originalPos = LocalPoints[i];
-                    Vector2 transformedPos;
-                    Vector2.Transform(ref originalPos, ref LocalToWorld, out transformedPos);
-                    WorldPoints.Add(transformedPos);
-                }
-
-                // Calculate bounds.
-                WorldPoints.GetBounds(out Bounds);
-
-                _worldDirty = false;
-                Dirty = true;
-            }
+            if (_worldDirty)
+                UpdateWorld();
         }
 
         private void UpdatePoints()
         {
-            if (_pointsDirty)
+            ValidateRawLocalPoints();
+            if (Valid)
             {
+                ConvertRawLocalPointsToLocalPoints();
+
                 IsConvex = LocalPoints.IsConvex();
                 Indices.Clear();
 
@@ -224,8 +195,32 @@ private void UpdatePoints()
                     Triangulator.Process(LocalPoints, Indices);
                 }
 
-                _pointsDirty = false;
+                _worldDirty = true;
+            }
+            _pointsDirty = false;
+        }
+
+        private void UpdateWorld()
+        {
+            // Calculate local to world transform.
+            Calculate.Transform(ref _position, ref _origin, ref _scale, _rotation, out LocalToWorld);
+
+            // Calculate points in world space.
+            WorldPoints.Clear();
+            int pointCount = LocalPoints.Count;
+            for (int i = 0; i < pointCount; i++)
+            {
+                Vector2 originalPos = LocalPoints[i];
+                Vector2 transformedPos;
+                Vector2.Transform(ref originalPos, ref LocalToWorld, out transformedPos);
+                WorldPoints.Add(transformedPos);
             }
+
+            // Calculate bounds.
+            WorldPoints.GetBounds(out Bounds);
+
+            Dirty = true;
+            _worldDirty = false;
         }
 
         // Raw local points are points unmodified by the system. Local points are always in CCW order.

Source/MonoGame.Penumbra.DesktopGL.csproj

@@ -58,6 +58,7 @@
     </Reference>
     <Reference Include="System" />
     <Reference Include="System.Core" />
+    <Compile Include="Extensions.cs" />
     <Compile Include="Graphics\GraphicsExtensions.cs" />
     <Compile Include="Graphics\Vao.cs" />
     <Compile Include="Graphics\Renderers\LightRenderer.cs" />

Source/MonoGame.Penumbra.WindowsDX.csproj

@@ -58,6 +58,7 @@
     </Reference>
     <Reference Include="System" />
     <Reference Include="System.Core" />
+    <Compile Include="Extensions.cs" />
     <Compile Include="Graphics\GraphicsExtensions.cs" />
     <Compile Include="Graphics\Vao.cs" />
     <Compile Include="Graphics\Renderers\LightRenderer.cs" />

Source/Utilities/ExtendedObservableCollection.cs

@@ -2,43 +2,40 @@
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
 using System.ComponentModel;
-using System.Linq;
 
 namespace Penumbra.Utilities
 {
-    // Adds an AddRange functionality to <see cref="ObservableCollection{T}"/>. We want this so that we don't get
+    // Adds an AddRange functionality to ObservableCollection. We want this so that we don't get
     // collection changed events raised after adding each item from a sequence.
     // Ref: http://blogs.msdn.com/b/nathannesbit/archive/2009/04/20/addrange-and-observablecollection.aspx
     internal class ExtendedObservableCollection<T> : ObservableCollection<T>
     {
+        private static readonly PropertyChangedEventArgs CountPropertyChangedEventArgs = new PropertyChangedEventArgs(nameof(Count));
+        private static readonly PropertyChangedEventArgs ItemsPropertyChangedEventArgs = new PropertyChangedEventArgs(nameof(Items));
+
+        // We must not use NULL for changedItem param as ObservableCollection will throw for that.
+        private static object _dummy = new object();
+        private static readonly NotifyCollectionChangedEventArgs DefaultNotifyCollectionChangedEventArgs
+            = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, _dummy, 0);
+
         public void AddRange(IEnumerable<T> items)
         {
             CheckReentrancy();
-            //
+
             // We need the starting index later.
-            //
             int startingIndex = Count;
 
-            //
-            // Add the items directly to the inner collection.
-            //
-            var changedItems = items.ToList();
-            foreach (var data in changedItems)
-            {
-                Items.Add(data);
-            }
+            // Add the items directly to the inner collection. In case the framework's inner
+            // implementation uses List{T} type, use its add range instead for better performance.
+            Items.AddRange(items);
 
-            //
             // Now raise the changed events.
-            //
-            OnPropertyChanged(new PropertyChangedEventArgs(nameof(Count)));
-            OnPropertyChanged(new PropertyChangedEventArgs(nameof(Items)));
+            OnPropertyChanged(CountPropertyChangedEventArgs);
+            OnPropertyChanged(ItemsPropertyChangedEventArgs);
 
-            //
-            // We have to change our input of new items into an IList since that is what the
-            // event args require.
-            //
-            OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, changedItems, startingIndex));
+            // The event args require a list of changed items and a starting index. Since the type is
+            // internal and the library does not care about args, we pass default args instead.
+            OnCollectionChanged(DefaultNotifyCollectionChangedEventArgs);
         }
     }
 }

Source/Utilities/FastList.cs

@@ -6,7 +6,7 @@
 namespace Penumbra.Utilities
 {
     // Differs from List{T} by allowing direct access to the underlying array.
-    // ref: https://github.com/SiliconStudio/paradox/blob/master/sources/common/core/SiliconStudio.Core/Collections/FastList.cs
+    // Ref: https://github.com/SiliconStudio/paradox/blob/master/sources/common/core/SiliconStudio.Core/Collections/FastList.cs
     internal class FastList<T> : IList<T>
     {
         // Fields
@@ -34,9 +34,7 @@ public FastList(IEnumerable<T> collection)
                 using (IEnumerator<T> enumerator = collection.GetEnumerator())
                 {
                     while (enumerator.MoveNext())
-                    {
                         Add(enumerator.Current);
-                    }
                 }
             }
         }