From f3f2e410013bdce49bd3c8f50b02f392007b0cc3 Mon Sep 17 00:00:00 2001
From: James Jackson-South <james_south@hotmail.com>
Date: Thu, 12 Oct 2017 16:49:35 +1100
Subject: [PATCH] Fix 16bit decode and add tests

---
 src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs | 43 ++++++++++++--------
 tests/ImageSharp.Tests/TestImages.cs         |  4 +-
 tests/Images/Input/Bmp/test16-inverted.bmp   |  3 ++
 tests/Images/Input/Bmp/test16.bmp            |  3 ++
 4 files changed, 35 insertions(+), 18 deletions(-)
 create mode 100644 tests/Images/Input/Bmp/test16-inverted.bmp
 create mode 100644 tests/Images/Input/Bmp/test16.bmp

diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs
index 39bab442f..04176e033 100644
--- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs
+++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs
@@ -12,22 +12,25 @@ namespace SixLabors.ImageSharp.Formats.Bmp
     /// <summary>
     /// Performs the bmp decoding operation.
     /// </summary>
+    /// <remarks>
+    /// A useful decoding source example can be found at <see href="https://dxr.mozilla.org/mozilla-central/source/image/decoders/nsBMPDecoder.cpp"/>
+    /// </remarks>
     internal sealed class BmpDecoderCore
     {
         /// <summary>
         /// The mask for the red part of the color for 16 bit rgb bitmaps.
         /// </summary>
-        private const int Rgb16RMask = 0x00007C00;
+        private const int Rgb16RMask = 0x7C00;
 
         /// <summary>
         /// The mask for the green part of the color for 16 bit rgb bitmaps.
         /// </summary>
-        private const int Rgb16GMask = 0x000003E0;
+        private const int Rgb16GMask = 0x3E0;
 
         /// <summary>
         /// The mask for the blue part of the color for 16 bit rgb bitmaps.
         /// </summary>
-        private const int Rgb16BMask = 0x0000001F;
+        private const int Rgb16BMask = 0x1F;
 
         /// <summary>
         /// RLE8 flag value that indicates following byte has special meaning
@@ -233,6 +236,17 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             return padding;
         }
 
+        /// <summary>
+        /// Performs final shifting from a 5bit value to an 8bit one.
+        /// </summary>
+        /// <param name="value">The masked and shifted value</param>
+        /// <returns>The <see cref="byte"/></returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static byte GetBytesFrom5BitValue(int value)
+        {
+            return (byte)((value << 3) | (value >> 2));
+        }
+
         /// <summary>
         /// Looks up color values and builds the image from de-compressed RLE8 data.
         /// Compresssed RLE8 stream is uncompressed by <see cref="UncompressRle8(int, Span{byte})"/>
@@ -416,36 +430,31 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         private void ReadRgb16<TPixel>(PixelAccessor<TPixel> pixels, int width, int height, bool inverted)
             where TPixel : struct, IPixel<TPixel>
         {
-            // We divide here as we will store the colors in our floating point format.
-            const int ScaleR = 8; // 256/32
-            const int ScaleG = 4; // 256/64
-            const int ComponentCount = 2;
-
+            int padding = CalculatePadding(width, 2);
+            int stride = (width * 2) + padding;
             var color = default(TPixel);
             var rgba = new Rgba32(0, 0, 0, 255);
 
-            using (var row = new PixelArea<TPixel>(width, ComponentOrder.Xyz))
+            using (var buffer = new Buffer<byte>(stride))
             {
                 for (int y = 0; y < height; y++)
                 {
-                    row.Read(this.currentStream);
-
+                    this.currentStream.Read(buffer.Array, 0, stride);
                     int newY = Invert(y, height, inverted);
-
                     Span<TPixel> pixelRow = pixels.GetRowSpan(newY);
 
                     int offset = 0;
                     for (int x = 0; x < width; x++)
                     {
-                        short temp = BitConverter.ToInt16(row.Bytes, offset);
+                        short temp = BitConverter.ToInt16(buffer.Array, offset);
 
-                        rgba.R = (byte)(((temp & Rgb16RMask) >> 11) * ScaleR);
-                        rgba.G = (byte)(((temp & Rgb16GMask) >> 5) * ScaleG);
-                        rgba.B = (byte)((temp & Rgb16BMask) * ScaleR);
+                        rgba.R = GetBytesFrom5BitValue((temp & Rgb16RMask) >> 10);
+                        rgba.G = GetBytesFrom5BitValue((temp & Rgb16GMask) >> 5);
+                        rgba.B = GetBytesFrom5BitValue(temp & Rgb16BMask);
 
                         color.PackFromRgba32(rgba);
                         pixelRow[x] = color;
-                        offset += ComponentCount;
+                        offset += 2;
                     }
                 }
             }
diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs
index 8be892233..9137049ee 100644
--- a/tests/ImageSharp.Tests/TestImages.cs
+++ b/tests/ImageSharp.Tests/TestImages.cs
@@ -140,8 +140,10 @@ namespace SixLabors.ImageSharp.Tests
             public const string RLEInverted = "Bmp/RunLengthEncoded-inverted.bmp";
             public const string Bit8 = "Bmp/test8.bmp";
             public const string Bit8Inverted = "Bmp/test8-inverted.bmp";
+            public const string Bit16 = "Bmp/test16.bmp";
+            public const string Bit16Inverted = "Bmp/test16-inverted.bmp";
 
-            public static readonly string[] All = { Car, F, NegHeight, CoreHeader, V5Header, RLE, RLEInverted, Bit8, Bit8Inverted };
+            public static readonly string[] All = { Car, F, NegHeight, CoreHeader, V5Header, RLE, RLEInverted, Bit8, Bit8Inverted, Bit16, Bit16Inverted };
         }
 
         public static class Gif
diff --git a/tests/Images/Input/Bmp/test16-inverted.bmp b/tests/Images/Input/Bmp/test16-inverted.bmp
new file mode 100644
index 000000000..551de69b3
--- /dev/null
+++ b/tests/Images/Input/Bmp/test16-inverted.bmp
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:474f93277f764b75ec91446fdffb8f290a6073236aed468a65cba5a333707c7d
+size 16438
diff --git a/tests/Images/Input/Bmp/test16.bmp b/tests/Images/Input/Bmp/test16.bmp
new file mode 100644
index 000000000..d6c5a67a6
--- /dev/null
+++ b/tests/Images/Input/Bmp/test16.bmp
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:472193f2506d768c45ec167518cb7da56e0e8707887431db20d1cfa5f61bd235
+size 16438