From 0fde6867e552d5ecc45fdc391062b2c2765289d2 Mon Sep 17 00:00:00 2001
From: woutware <35376607+woutware@users.noreply.github.com>
Date: Thu, 19 Apr 2018 16:58:39 +0200
Subject: [PATCH] Fixed color component order (should be reversed) for Argb32,
 Rgba32, Bgr24, Rgb24, Bgra32. Tests still need to be updated.

---
 src/ImageSharp/PixelFormats/Argb32.cs         | 34 ++++++++++++++-----
 src/ImageSharp/PixelFormats/Bgr24.cs          |  8 ++---
 src/ImageSharp/PixelFormats/Bgra32.cs         | 16 ++++-----
 src/ImageSharp/PixelFormats/Rgb24.cs          | 24 ++++++-------
 src/ImageSharp/PixelFormats/Rgba32.cs         | 24 +++++++------
 .../PixelFormats/PackedPixelTests.cs          |  4 +--
 .../PixelFormats/Rgb24Tests.cs                |  4 +--
 7 files changed, 67 insertions(+), 47 deletions(-)

diff --git a/src/ImageSharp/PixelFormats/Argb32.cs b/src/ImageSharp/PixelFormats/Argb32.cs
index 6038214100..05a3a9c747 100644
--- a/src/ImageSharp/PixelFormats/Argb32.cs
+++ b/src/ImageSharp/PixelFormats/Argb32.cs
@@ -22,24 +22,24 @@ namespace SixLabors.ImageSharp.PixelFormats
     public struct Argb32 : IPixel<Argb32>, IPackedVector<uint>
     {
         /// <summary>
-        /// Gets or sets the alpha component.
+        /// Gets or sets the blue component.
         /// </summary>
-        public byte A;
+        public byte B;
 
         /// <summary>
-        /// Gets or sets the red component.
+        /// Gets or sets the green component.
         /// </summary>
-        public byte R;
+        public byte G;
 
         /// <summary>
-        /// Gets or sets the green component.
+        /// Gets or sets the red component.
         /// </summary>
-        public byte G;
+        public byte R;
 
         /// <summary>
-        /// Gets or sets the blue component.
+        /// Gets or sets the alpha component.
         /// </summary>
-        public byte B;
+        public byte A;
 
         /// <summary>
         /// The maximum byte value.
@@ -153,6 +153,24 @@ namespace SixLabors.ImageSharp.PixelFormats
             }
         }
 
+        /// <summary>
+        /// Gets or sets the RGB components of this struct as <see cref="Rgb24"/>
+        /// </summary>
+        public Rgb24 Rgb
+        {
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            get
+            {
+                return Unsafe.As<Argb32, Rgb24>(ref this);
+            }
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            set
+            {
+                Unsafe.As<Argb32, Rgb24>(ref this) = value;
+            }
+        }
+
         /// <inheritdoc/>
         public uint PackedValue
         {
diff --git a/src/ImageSharp/PixelFormats/Bgr24.cs b/src/ImageSharp/PixelFormats/Bgr24.cs
index 5c1845768a..d33c481616 100644
--- a/src/ImageSharp/PixelFormats/Bgr24.cs
+++ b/src/ImageSharp/PixelFormats/Bgr24.cs
@@ -18,9 +18,9 @@ namespace SixLabors.ImageSharp.PixelFormats
     public struct Bgr24 : IPixel<Bgr24>
     {
         /// <summary>
-        /// The blue component.
+        /// The red component.
         /// </summary>
-        public byte B;
+        public byte R;
 
         /// <summary>
         /// The green component.
@@ -28,9 +28,9 @@ namespace SixLabors.ImageSharp.PixelFormats
         public byte G;
 
         /// <summary>
-        /// The red component.
+        /// The blue component.
         /// </summary>
-        public byte R;
+        public byte B;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Bgr24"/> struct.
diff --git a/src/ImageSharp/PixelFormats/Bgra32.cs b/src/ImageSharp/PixelFormats/Bgra32.cs
index 91875671a9..83c69f5b10 100644
--- a/src/ImageSharp/PixelFormats/Bgra32.cs
+++ b/src/ImageSharp/PixelFormats/Bgra32.cs
@@ -19,24 +19,24 @@ namespace SixLabors.ImageSharp.PixelFormats
     public struct Bgra32 : IPixel<Bgra32>, IPackedVector<uint>
     {
         /// <summary>
-        /// Gets or sets the blue component.
+        /// Gets or sets the alpha component.
         /// </summary>
-        public byte B;
+        public byte A;
 
         /// <summary>
-        /// Gets or sets the green component.
+        /// Gets or sets the red component.
         /// </summary>
-        public byte G;
+        public byte R;
 
         /// <summary>
-        /// Gets or sets the red component.
+        /// Gets or sets the green component.
         /// </summary>
-        public byte R;
+        public byte G;
 
         /// <summary>
-        /// Gets or sets the alpha component.
+        /// Gets or sets the blue component.
         /// </summary>
-        public byte A;
+        public byte B;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Bgra32"/> struct.
diff --git a/src/ImageSharp/PixelFormats/Rgb24.cs b/src/ImageSharp/PixelFormats/Rgb24.cs
index db798e053c..94a503e928 100644
--- a/src/ImageSharp/PixelFormats/Rgb24.cs
+++ b/src/ImageSharp/PixelFormats/Rgb24.cs
@@ -19,9 +19,9 @@ namespace SixLabors.ImageSharp.PixelFormats
     public struct Rgb24 : IPixel<Rgb24>
     {
         /// <summary>
-        /// The red component.
+        /// The blue component.
         /// </summary>
-        public byte R;
+        public byte B;
 
         /// <summary>
         /// The green component.
@@ -29,9 +29,9 @@ namespace SixLabors.ImageSharp.PixelFormats
         public byte G;
 
         /// <summary>
-        /// The blue component.
+        /// The red component.
         /// </summary>
-        public byte B;
+        public byte R;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Rgb24"/> struct.
@@ -80,16 +80,16 @@ namespace SixLabors.ImageSharp.PixelFormats
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public void PackFromRgba32(Rgba32 source)
         {
-            this = Unsafe.As<Rgba32, Rgb24>(ref source);
+            this.R = source.R;
+            this.G = source.G;
+            this.B = source.B;
         }
 
         /// <inheritdoc/>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public void PackFromArgb32(Argb32 source)
         {
-            this.R = source.R;
-            this.G = source.G;
-            this.B = source.B;
+            this = Unsafe.As<Argb32, Rgb24>(ref source);
         }
 
         /// <inheritdoc/>
@@ -132,15 +132,15 @@ namespace SixLabors.ImageSharp.PixelFormats
         /// <inheritdoc/>
         public void ToRgba32(ref Rgba32 dest)
         {
-            dest.Rgb = this;
+            dest.R = this.R;
+            dest.G = this.G;
+            dest.B = this.B;
             dest.A = 255;
         }
 
         /// <inheritdoc/>
         public void ToArgb32(ref Argb32 dest) {
-            dest.R = this.R;
-            dest.G = this.G;
-            dest.B = this.B;
+            dest.Rgb = this;
             dest.A = 255;
         }
 
diff --git a/src/ImageSharp/PixelFormats/Rgba32.cs b/src/ImageSharp/PixelFormats/Rgba32.cs
index 220f835b93..a06cd7d2f6 100644
--- a/src/ImageSharp/PixelFormats/Rgba32.cs
+++ b/src/ImageSharp/PixelFormats/Rgba32.cs
@@ -22,24 +22,24 @@ namespace SixLabors.ImageSharp.PixelFormats
     public partial struct Rgba32 : IPixel<Rgba32>, IPackedVector<uint>
     {
         /// <summary>
-        /// Gets or sets the red component.
+        /// Gets or sets the alpha component.
         /// </summary>
-        public byte R;
+        public byte A;
 
         /// <summary>
-        /// Gets or sets the green component.
+        /// Gets or sets the blue component.
         /// </summary>
-        public byte G;
+        public byte B;
 
         /// <summary>
-        /// Gets or sets the blue component.
+        /// Gets or sets the green component.
         /// </summary>
-        public byte B;
+        public byte G;
 
         /// <summary>
-        /// Gets or sets the alpha component.
+        /// Gets or sets the red component.
         /// </summary>
-        public byte A;
+        public byte R;
 
         /// <summary>
         /// The shift count for the red component
@@ -174,20 +174,22 @@ namespace SixLabors.ImageSharp.PixelFormats
         }
 
         /// <summary>
-        /// Gets or sets the RGB components of this struct as <see cref="Rgb24"/>
+        /// Gets or sets the RGB components of this struct as <see cref="Rgb24"/>.
         /// </summary>
         public Rgb24 Rgb
         {
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             get
             {
-                return Unsafe.As<Rgba32, Rgb24>(ref this);
+                return new Rgb24(this.R, this.G, this.B);
             }
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             set
             {
-                Unsafe.As<Rgba32, Rgb24>(ref this) = value;
+                this.R = value.R;
+                this.G = value.G;
+                this.B = value.B;
             }
         }
 
diff --git a/tests/ImageSharp.Tests/PixelFormats/PackedPixelTests.cs b/tests/ImageSharp.Tests/PixelFormats/PackedPixelTests.cs
index 0281531420..95e8239986 100644
--- a/tests/ImageSharp.Tests/PixelFormats/PackedPixelTests.cs
+++ b/tests/ImageSharp.Tests/PixelFormats/PackedPixelTests.cs
@@ -106,7 +106,7 @@ namespace SixLabors.ImageSharp.Tests.Colors
             float z = +0.5f;
             float w = -0.7f;
             var argb = new Argb32(x, y, z, w);
-            Assert.Equal(0x80001a00u, argb.PackedValue);
+            Assert.Equal(0x001a0080u, argb.PackedValue);
 
             // Test ordering
             var rgb = default(Rgb24);
@@ -925,7 +925,7 @@ namespace SixLabors.ImageSharp.Tests.Colors
             float z = +0.5f;
             float w = -0.7f;
             var rgba32 = new Rgba32(x, y, z, w);
-            Assert.Equal(0x80001Au, rgba32.PackedValue);
+            Assert.Equal(0x1a008000u, rgba32.PackedValue);
 
             // Test ordering
             var rgb = default(Rgb24);
diff --git a/tests/ImageSharp.Tests/PixelFormats/Rgb24Tests.cs b/tests/ImageSharp.Tests/PixelFormats/Rgb24Tests.cs
index 4e85fe7e32..29de303d81 100644
--- a/tests/ImageSharp.Tests/PixelFormats/Rgb24Tests.cs
+++ b/tests/ImageSharp.Tests/PixelFormats/Rgb24Tests.cs
@@ -30,9 +30,9 @@ namespace SixLabors.ImageSharp.Tests
             var color = new Rgb24(1, 2, 3);
             byte* ptr = (byte*)&color;
 
-            Assert.Equal(1, ptr[0]);
+            Assert.Equal(3, ptr[0]);
             Assert.Equal(2, ptr[1]);
-            Assert.Equal(3, ptr[2]);
+            Assert.Equal(1, ptr[2]);
         }
 
         [Theory]