From 5128634a5eed8caa33cb0d2922a093d26c110a66 Mon Sep 17 00:00:00 2001
From: Eric Mellino <erme@microsoft.com>
Date: Fri, 4 Nov 2016 17:09:20 -0700
Subject: [PATCH] Ensure Color structure stores elements in RGBA order

---
 src/ImageSharp/Colors/Color.cs              | 37 +++++++++++++--------
 tests/ImageSharp.Tests/Colors/ColorTests.cs | 14 ++++++++
 tests/ImageSharp.Tests/project.json         |  3 +-
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/ImageSharp/Colors/Color.cs b/src/ImageSharp/Colors/Color.cs
index 633a1c0144..edd0fd0f09 100644
--- a/src/ImageSharp/Colors/Color.cs
+++ b/src/ImageSharp/Colors/Color.cs
@@ -19,6 +19,11 @@ namespace ImageSharp
     /// </remarks>
     public partial struct Color : IPackedPixel<uint>, IEquatable<Color>
     {
+        private const int RedShift = 0;
+        private const int GreenShift = 8;
+        private const int BlueShift = 16;
+        private const int AlphaShift = 24;
+
         /// <summary>
         /// The maximum byte value.
         /// </summary>
@@ -63,6 +68,9 @@ namespace ImageSharp
             {
                 throw new ArgumentException("Hexadecimal string is not in the correct format.", nameof(hex));
             }
+
+            // Order parsed from hex string will be backwards, so reverse it.
+            packedValue = Pack(A, B, G, R);
         }
 
         /// <summary>
@@ -106,12 +114,12 @@ namespace ImageSharp
         {
             get
             {
-                return (byte)(this.packedValue >> 24);
+                return (byte)(this.packedValue >> RedShift);
             }
 
             set
             {
-                this.packedValue = this.packedValue & 0x00FFFFFF | (uint)value << 24;
+                this.packedValue = this.packedValue & 0xFFFFFF00 | (uint)value << RedShift;
             }
         }
 
@@ -122,12 +130,12 @@ namespace ImageSharp
         {
             get
             {
-                return (byte)(this.packedValue >> 16);
+                return (byte)(this.packedValue >> GreenShift);
             }
 
             set
             {
-                this.packedValue = this.packedValue & 0xFF00FFFF | (uint)value << 16;
+                this.packedValue = this.packedValue & 0xFFFF00FF | (uint)value << GreenShift;
             }
         }
 
@@ -138,12 +146,12 @@ namespace ImageSharp
         {
             get
             {
-                return (byte)(this.packedValue >> 8);
+                return (byte)(this.packedValue >> BlueShift);
             }
 
             set
             {
-                this.packedValue = this.packedValue & 0xFFFF00FF | (uint)value << 8;
+                this.packedValue = this.packedValue & 0xFF00FFFF | (uint)value << BlueShift;
             }
         }
 
@@ -154,12 +162,12 @@ namespace ImageSharp
         {
             get
             {
-                return (byte)this.packedValue;
+                return (byte)(this.packedValue >> AlphaShift);
             }
 
             set
             {
-                this.packedValue = this.packedValue & 0xFFFFFF00 | value;
+                this.packedValue = this.packedValue & 0x00FFFFFF | (uint)value << AlphaShift;
             }
         }
 
@@ -234,7 +242,8 @@ namespace ImageSharp
         /// <returns>A hexadecimal string representation of the value.</returns>
         public string ToHex()
         {
-            return this.PackedValue.ToString("X8");
+            uint hexOrder = Pack(A, B, G, R);
+            return hexOrder.ToString("X8");
         }
 
         /// <inheritdoc/>
@@ -318,10 +327,10 @@ namespace ImageSharp
             vector = Vector4.Clamp(vector, Vector4.Zero, Vector4.One);
             vector *= MaxBytes;
             vector += Half;
-            return (uint)(((byte)vector.X << 24)
-                        | ((byte)vector.Y << 16)
-                        | ((byte)vector.Z << 8)
-                        | (byte)vector.W);
+            return (uint)(((byte)vector.X << RedShift)
+                        | ((byte)vector.Y << GreenShift)
+                        | ((byte)vector.Z << BlueShift)
+                        | (byte)vector.W << AlphaShift);
         }
 
         /// <summary>
@@ -359,7 +368,7 @@ namespace ImageSharp
         /// <returns>The <see cref="uint"/></returns>
         private static uint Pack(byte x, byte y, byte z, byte w)
         {
-            return (uint)(x << 24 | y << 16 | z << 8 | w);
+            return (uint)(x << RedShift | y << GreenShift | z << BlueShift | w << AlphaShift);
         }
 
         /// <summary>
diff --git a/tests/ImageSharp.Tests/Colors/ColorTests.cs b/tests/ImageSharp.Tests/Colors/ColorTests.cs
index ac7ef81237..23ebbec986 100644
--- a/tests/ImageSharp.Tests/Colors/ColorTests.cs
+++ b/tests/ImageSharp.Tests/Colors/ColorTests.cs
@@ -117,5 +117,19 @@ namespace ImageSharp.Tests
 
             Assert.Equal("00CCBBFF", color.ToHex());
         }
+
+        /// <summary>
+        /// Tests that the individual byte elements are layed out in RGBA order.
+        /// </summary>
+        [Fact]
+        public unsafe void ByteLayout()
+        {
+            Color color = new Color(1, 2, 3, 4);
+            byte* colorBase = (byte*)&color;
+            Assert.Equal(1, colorBase[0]);
+            Assert.Equal(2, colorBase[1]);
+            Assert.Equal(3, colorBase[2]);
+            Assert.Equal(4, colorBase[3]);
+        }
     }
 }
\ No newline at end of file
diff --git a/tests/ImageSharp.Tests/project.json b/tests/ImageSharp.Tests/project.json
index f81cbfe824..b9b7b50ab6 100644
--- a/tests/ImageSharp.Tests/project.json
+++ b/tests/ImageSharp.Tests/project.json
@@ -10,7 +10,8 @@
     ]
   },
   "buildOptions": {
-    "debugType": "portable"
+    "debugType": "portable",
+    "allowUnsafe": true
   },
   "dependencies": {
     "ImageSharp": "1.0.0-*",