Core - Fix color gradient and utility bugs found by the tests (#914)

* Profile adaption - Fix arrow keys section including Page Down

- The arrow keys keyboard section now starts at Arrow Up instead of the key before it (Page Down)
- Updated the keyboard section test to expect only the four arrow keys

* Color quantizer - Let fully saturated colors be vibrant and grays be muted

- The saturation limits of the color types are now inclusive, so 100% saturated colors count as vibrant and 0% saturated colors count as muted
- The lightness limits stay exclusive so pure black and white are still left out
- Updated the quantizer tests to match

* Color gradients - Fix stop placement, seamless toggling and list events

- Stops on an occupied position now move down 0.001 instead of 0.002 (rounding error in the distance check)
- Flip, rotate, spread and interpolate move all stops together, so a stop can take a position another stop is leaving
- Spreading or randomizing a single stop puts it at 0 instead of NaN
- ToggleSeamless does nothing on an empty gradient and caps stretched stops at 1 instead of 100
- RemoveAt reports the removed stop and its index
- Insert links the stop like Add does and raises the add before the move
- Remove, RemoveAt, Clear and replacing a stop through the indexer unlink and stop listening to the actual removed stop
- Copying from a list of stops sorts them by position
- Obsolete GetPositionsArray no longer skips the seamless positions, obsolete GetColor picks the right colors after a repeat
- Updated the gradient tests to match

* Core - Fix small color, enum, string and type utility bugs

- Darken keeps the alpha of the color
- GetAllValuesAndDescriptions(Type) uses the [Description] attribute like the generic version
- UrlFriendly cuts titles at 80 characters instead of 81
- RemapInternationalCharToAscii knows both cases of r, l, d, thorn, sharp s, h and j with accents
- IsGenericType returns false instead of throwing when the base type is not generic
- IsCastableFrom also finds conversion operators declared on the target type
- GetDisplayName keeps the brackets of array types when humanizing
- Updated the utility tests to match

src/Artemis.Core/Models/Profile/AdaptionHints/KeyboardSectionAdaptionHint.cs

@@ -20,7 +20,7 @@ public class KeyboardSectionAdaptionHint : CorePropertyChanged, IAdaptionHint
         {KeyboardSection.FunctionKeys, Enum.GetValues<LedId>().Where(l => l >= LedId.Keyboard_F1 && l <= LedId.Keyboard_F12).ToList()},
         {KeyboardSection.NumberKeys, Enum.GetValues<LedId>().Where(l => l >= LedId.Keyboard_1 && l <= LedId.Keyboard_0).ToList()},
         {KeyboardSection.NumPad, Enum.GetValues<LedId>().Where(l => l >= LedId.Keyboard_NumLock && l <= LedId.Keyboard_NumPeriodAndDelete).ToList()},
-        {KeyboardSection.ArrowKeys, Enum.GetValues<LedId>().Where(l => l >= LedId.Keyboard_PageDown && l <= LedId.Keyboard_ArrowRight).ToList()},
+        {KeyboardSection.ArrowKeys, Enum.GetValues<LedId>().Where(l => l >= LedId.Keyboard_ArrowUp && l <= LedId.Keyboard_ArrowRight).ToList()},
         {KeyboardSection.MediaKeys, Enum.GetValues<LedId>().Where(l => l >= LedId.Keyboard_MediaMute && l <= LedId.Keyboard_MediaNextTrack).ToList()},
     };
 

src/Artemis.Core/Models/Profile/Colors/ColorGradient.cs

@@ -87,6 +87,9 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
             stop.ColorGradient = this;
             _stops.Add(stop);
         }
+
+        // The list may not be in order, the rest of the gradient expects the stops to be sorted by position
+        Update();
     }
 
     /// <summary>
@@ -129,7 +132,7 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     [Obsolete("Use the Positions property instead", true)]
     public float[] GetPositionsArray(int timesToRepeat = 0, bool seamless = false)
     {
-        if (timesToRepeat == 0 && seamless)
+        if (timesToRepeat == 0 && !seamless)
             return Positions;
 
         List<float> result = [];
@@ -190,31 +193,34 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
 
         // Walk through the stops until we find the one at or after the requested position, that becomes the right stop
         // The left stop is the previous stop before the right one was found.
-        float left = stops[0];
-        float? right = null;
-        foreach (float stop in stops)
+        // Keep track of indexes rather than positions, repeats share positions where one ends and the next begins
+        int leftIndex = 0;
+        int rightIndex = -1;
+        for (int i = 0; i < stops.Length; i++)
         {
-            if (stop >= position)
+            if (stops[i] >= position)
             {
-                right = stop;
+                rightIndex = i;
                 break;
             }
 
-            left = stop;
+            leftIndex = i;
         }
 
         // Get the left stop's color
-        SKColor leftColor = colors[Array.IndexOf(stops, left)];
+        float left = stops[leftIndex];
+        SKColor leftColor = colors[leftIndex];
 
         // If no right stop was found or the left and right stops are on the same spot, return the left stop's color
-        if (right == null || left == right)
+        if (rightIndex == -1 || left == stops[rightIndex])
             return leftColor;
 
         // Get the right stop's color
-        SKColor rightColor = colors[Array.IndexOf(stops, right)];
+        float right = stops[rightIndex];
+        SKColor rightColor = colors[rightIndex];
 
         // Interpolate the position between the left and right color
-        position = MathF.Round((position - left) / (right.Value - left), 2);
+        position = MathF.Round((position - left) / (right - left), 2);
         byte a = (byte) ((rightColor.Alpha - leftColor.Alpha) * position + leftColor.Alpha);
         byte r = (byte) ((rightColor.Red - leftColor.Red) * position + leftColor.Red);
         byte g = (byte) ((rightColor.Green - leftColor.Green) * position + leftColor.Green);
@@ -284,17 +290,12 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     /// </summary>
     public void SpreadStops()
     {
-        try
+        MoveStops(() =>
         {
-            _updating = true;
+            // A single stop has nothing to spread across and stays at the start
             for (int i = 0; i < Count; i++)
-                this[i].Position = MathF.Round(i / ((float) Count - 1), 3, MidpointRounding.AwayFromZero);
-        }
-        finally
-        {
-            _updating = false;
-            Update();
-        }
+                this[i].Position = Count > 1 ? MathF.Round(i / ((float) Count - 1), 3, MidpointRounding.AwayFromZero) : 0;
+        });
     }
 
     /// <summary>
@@ -307,6 +308,10 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     /// </summary>
     public void ToggleSeamless()
     {
+        // An empty gradient has no stops to repeat or remove
+        if (Count == 0)
+            return;
+
         try
         {
             _updating = true;
@@ -321,7 +326,7 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
                 {
                     float multiplier = Count / (Count - 1f);
                     foreach (ColorGradientStop stop in this)
-                        stop.Position = MathF.Round(Math.Min(stop.Position * multiplier, 100f), 3, MidpointRounding.AwayFromZero);
+                        stop.Position = MathF.Round(Math.Min(stop.Position * multiplier, 1f), 3, MidpointRounding.AwayFromZero);
                 }
             }
             else
@@ -348,17 +353,11 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     /// </summary>
     public void FlipStops()
     {
-        try
+        MoveStops(() =>
         {
-            _updating = true;
             foreach (ColorGradientStop stop in this)
                 stop.Position = 1 - stop.Position;
-        }
-        finally
-        {
-            _updating = false;
-            Update();
-        }
+        });
     }
 
     /// <summary>
@@ -368,9 +367,8 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     /// <param name="inverse">A boolean indicating whether or not the invert the rotation.</param>
     public void RotateStops(bool inverse)
     {
-        try
+        MoveStops(() =>
         {
-            _updating = true;
             List<ColorGradientStop> stops = inverse
                 ? this.OrderBy(s => s.Position).ToList()
                 : this.OrderByDescending(s => s.Position).ToList();
@@ -378,12 +376,7 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
             float lastStopPosition = stops.Last().Position;
             foreach (ColorGradientStop stop in stops)
                 (stop.Position, lastStopPosition) = (lastStopPosition, stop.Position);
-        }
-        finally
-        {
-            _updating = false;
-            Update();
-        }
+        });
     }
 
     /// <summary>
@@ -401,7 +394,7 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
             for (int index = 0; index < stops; index++)
             {
                 SKColor skColor = SKColor.FromHsv(random.NextSingle() * 360, 100, 100);
-                float position = 1f / (stops - 1f) * index;
+                float position = stops > 1 ? 1f / (stops - 1f) * index : 0;
                 Add(new ColorGradientStop(skColor, position));
             }
         }
@@ -431,22 +424,25 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
         }
 
         // Interpolate stops
-        int index = 0;
-        foreach (ColorGradientStop stop in interpolated.ToList())
+        interpolated.MoveStops(() =>
         {
-            if (index < targetValue.Count)
+            int index = 0;
+            foreach (ColorGradientStop stop in interpolated)
             {
-                ColorGradientStop targetStop = targetValue[index];
-                stop.Interpolate(targetStop, progress);
-            }
-            // Interpolate stops not on the target gradient
-            else
-            {
-                stop.Color = stop.Color.Interpolate(targetValue.GetColor(stop.Position), progress);
-            }
+                if (index < targetValue.Count)
+                {
+                    ColorGradientStop targetStop = targetValue[index];
+                    stop.Interpolate(targetStop, progress);
+                }
+                // Interpolate stops not on the target gradient
+                else
+                {
+                    stop.Color = stop.Color.Interpolate(targetValue.GetColor(stop.Position), progress);
+                }
 
-            index++;
-        }
+                index++;
+            }
+        });
 
         return interpolated;
     }
@@ -494,6 +490,28 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
         StopChanged?.Invoke(this, EventArgs.Empty);
     }
 
+    private void MoveStops(Action move)
+    {
+        // Unlink the stops while they move so a stop can take a position another stop is about to leave,
+        // otherwise it would avoid that stop and land next to it
+        try
+        {
+            _updating = true;
+            foreach (ColorGradientStop stop in _stops)
+                stop.ColorGradient = null;
+
+            move();
+        }
+        finally
+        {
+            foreach (ColorGradientStop stop in _stops)
+                stop.ColorGradient = this;
+
+            _updating = false;
+            Update();
+        }
+    }
+
     private void Update()
     {
         _dirty = true;
@@ -627,7 +645,11 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     public void Clear()
     {
         foreach (ColorGradientStop item in _stops)
+        {
             item.PropertyChanged -= ItemOnPropertyChanged;
+            item.ColorGradient = null;
+        }
+
         _stops.Clear();
         OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
     }
@@ -673,13 +695,13 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     /// <inheritdoc />
     public bool Remove(ColorGradientStop item)
     {
-        item.PropertyChanged -= ItemOnPropertyChanged;
+        // Stops are compared by color and position, so this may find a different stop that looks the same
         int index = _stops.IndexOf(item);
-        bool removed = _stops.Remove(item);
-        if (removed)
-            OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index));
+        if (index == -1)
+            return false;
 
-        return removed;
+        RemoveAt(index);
+        return true;
     }
 
     /// <inheritdoc />
@@ -720,17 +742,23 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
     public void Insert(int index, ColorGradientStop item)
     {
         _stops.Insert(index, item);
+        item.ColorGradient = this;
+        // Update the position, reapplying the overlap-check in Position's setter
+        item.Position = item.Position;
         item.PropertyChanged += ItemOnPropertyChanged;
+
+        OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index));
         Update();
-        OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, _stops.IndexOf(item)));
     }
 
     /// <inheritdoc cref="IList{T}.RemoveAt" />
     public void RemoveAt(int index)
     {
-        _stops[index].PropertyChanged -= ItemOnPropertyChanged;
+        ColorGradientStop item = _stops[index];
+        item.PropertyChanged -= ItemOnPropertyChanged;
+        item.ColorGradient = null;
         _stops.RemoveAt(index);
-        OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, index));
+        OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index));
     }
 
     /// <inheritdoc />
@@ -744,8 +772,12 @@ public class ColorGradient : IList<ColorGradientStop>, IList, INotifyCollectionC
         {
             ColorGradientStop? oldValue = _stops[index];
             oldValue.PropertyChanged -= ItemOnPropertyChanged;
+            oldValue.ColorGradient = null;
             _stops[index] = value;
-            _stops[index].PropertyChanged += ItemOnPropertyChanged;
+            value.ColorGradient = this;
+            // Update the position, reapplying the overlap-check in Position's setter
+            value.Position = value.Position;
+            value.PropertyChanged += ItemOnPropertyChanged;
             Update();
             OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, value, oldValue));
         }

src/Artemis.Core/Models/Profile/Colors/ColorGradientStop.cs

@@ -9,6 +9,9 @@ namespace Artemis.Core;
 /// </summary>
 public class ColorGradientStop : CorePropertyChanged
 {
+    // The smallest distance kept between two stops in the same gradient
+    private const float MINIMUM_DISTANCE = 0.001f;
+
     private SKColor _color;
     private float _position;
 
@@ -75,23 +78,31 @@ public class ColorGradientStop : CorePropertyChanged
     {
         if (ColorGradient == null)
             return stopPosition;
+
         // Find the first available spot going down
-        while (ColorGradient.Any(s => !ReferenceEquals(s, this) && Math.Abs(s.Position - stopPosition) < 0.001f))
-            stopPosition -= 0.001f;
+        float position = stopPosition;
+        for (int step = 1; IsOccupied(position); step++)
+            position = stopPosition - step * MINIMUM_DISTANCE;
 
         // If we ran out of space, try going up
-        if (stopPosition < 0)
+        if (position < 0)
         {
-            stopPosition = 0;
-            while (ColorGradient.Any(s => !ReferenceEquals(s, this) && Math.Abs(s.Position - stopPosition) < 0.001f))
-                stopPosition += 0.001f;
+            position = 0;
+            for (int step = 1; IsOccupied(position); step++)
+                position = step * MINIMUM_DISTANCE;
         }
 
         // If we ran out of space there too, movement isn't possible
-        if (stopPosition > 1)
-            stopPosition = Position;
+        if (position > 1)
+            position = Position;
 
-        return stopPosition;
+        return position;
+    }
+
+    private bool IsOccupied(float position)
+    {
+        // Allow for a little rounding error, 0.5 - 0.499 comes out a hair under 0.001 in floating point
+        return ColorGradient != null && ColorGradient.Any(s => !ReferenceEquals(s, this) && Math.Abs(s.Position - position) < MINIMUM_DISTANCE - 0.00001f);
     }
 
     /// <summary>