Merge pull request #792 from brianpopow/feature/BMPv2ColorPaletteFix

WIP: Fix for Windows 2.0 or OS/2 1.x bitmaps only use 3 bytes per color palette entry

src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs

@@ -108,7 +108,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         {
             try
             {
-                this.ReadImageHeaders(stream, out bool inverted, out byte[] palette);
+                int bytesPerColorMapEntry = this.ReadImageHeaders(stream, out bool inverted, out byte[] palette);
 
                 var image = new Image<TPixel>(this.configuration, this.infoHeader.Width, this.infoHeader.Height, this.metaData);
 
@@ -137,6 +137,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                                 this.infoHeader.Width,
                                 this.infoHeader.Height,
                                 this.infoHeader.BitsPerPixel,
+                                bytesPerColorMapEntry,
                                 inverted);
                         }
 
@@ -329,18 +330,20 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// <param name="colors">The <see cref="T:byte[]"/> containing the colors.</param>
         /// <param name="width">The width of the bitmap.</param>
         /// <param name="height">The height of the bitmap.</param>
-        /// <param name="bits">The number of bits per pixel.</param>
+        /// <param name="bitsPerPixel">The number of bits per pixel.</param>
+        /// <param name="bytesPerColorMapEntry">Usually 4 bytes, but in case of Windows 2.x bitmaps or OS/2 1.x bitmaps
+        /// the bytes per color palette entry's can be 3 bytes instead of 4.</param>
         /// <param name="inverted">Whether the bitmap is inverted.</param>
-        private void ReadRgbPalette<TPixel>(Buffer2D<TPixel> pixels, byte[] colors, int width, int height, int bits, bool inverted)
+        private void ReadRgbPalette<TPixel>(Buffer2D<TPixel> pixels, byte[] colors, int width, int height, int bitsPerPixel, int bytesPerColorMapEntry, bool inverted)
             where TPixel : struct, IPixel<TPixel>
         {
             // Pixels per byte (bits per pixel)
-            int ppb = 8 / bits;
+            int ppb = 8 / bitsPerPixel;
 
             int arrayWidth = (width + ppb - 1) / ppb;
 
             // Bit mask
-            int mask = 0xFF >> (8 - bits);
+            int mask = 0xFF >> (8 - bitsPerPixel);
 
             // Rows are aligned on 4 byte boundaries
             int padding = arrayWidth % 4;
@@ -366,7 +369,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                         int colOffset = x * ppb;
                         for (int shift = 0, newX = colOffset; shift < ppb && newX < width; shift++, newX++)
                         {
-                            int colorIndex = ((rowSpan[offset] >> (8 - bits - (shift * bits))) & mask) * 4;
+                            int colorIndex = ((rowSpan[offset] >> (8 - bitsPerPixel - (shift * bitsPerPixel))) & mask) * bytesPerColorMapEntry;
 
                             color.FromBgr24(Unsafe.As<byte, Bgr24>(ref colors[colorIndex]));
                             pixelRow[newX] = color;
@@ -576,7 +579,9 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// <summary>
         /// Reads the <see cref="BmpFileHeader"/> and <see cref="BmpInfoHeader"/> from the stream and sets the corresponding fields.
         /// </summary>
-        private void ReadImageHeaders(Stream stream, out bool inverted, out byte[] palette)
+        /// <returns>Bytes per color palette entry. Usually 4 bytes, but in case of Windows 2.x bitmaps or OS/2 1.x bitmaps
+        /// the bytes per color palette entry's can be 3 bytes instead of 4.</returns>
+        private int ReadImageHeaders(Stream stream, out bool inverted, out byte[] palette)
         {
             this.stream = stream;
 
@@ -596,6 +601,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             }
 
             int colorMapSize = -1;
+            int bytesPerColorMapEntry = 4;
 
             if (this.infoHeader.ClrUsed == 0)
             {
@@ -603,12 +609,15 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                     || this.infoHeader.BitsPerPixel == 4
                     || this.infoHeader.BitsPerPixel == 8)
                 {
-                    colorMapSize = ImageMaths.GetColorCountForBitDepth(this.infoHeader.BitsPerPixel) * 4;
+                    int colorMapSizeBytes = this.fileHeader.Offset - BmpFileHeader.Size - this.infoHeader.HeaderSize;
+                    int colorCountForBitDepth = ImageMaths.GetColorCountForBitDepth(this.infoHeader.BitsPerPixel);
+                    bytesPerColorMapEntry = colorMapSizeBytes / colorCountForBitDepth;
+                    colorMapSize = colorMapSizeBytes;
                 }
             }
             else
             {
-                colorMapSize = this.infoHeader.ClrUsed * 4;
+                colorMapSize = this.infoHeader.ClrUsed * bytesPerColorMapEntry;
             }
 
             palette = null;
@@ -627,6 +636,8 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             }
 
             this.infoHeader.VerifyDimensions();
+
+            return bytesPerColorMapEntry;
         }
     }
 }

tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs

@@ -55,6 +55,30 @@ namespace SixLabors.ImageSharp.Tests
             }
         }
 
+        [Theory]
+        [WithFile(WinBmpv2, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecodeBmpv2<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider, "png");
+                image.CompareToOriginal(provider);
+            }
+        }
+
+        [Theory]
+        [WithFile(Bit8Palette4, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode4BytePerEntryPalette<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider, "png");
+                image.CompareToOriginal(provider);
+            }
+        }
+
         [Theory]
         [InlineData(Car, 24)]
         [InlineData(F, 24)]

tests/ImageSharp.Tests/TestImages.cs

@@ -201,6 +201,10 @@ namespace SixLabors.ImageSharp.Tests
             public const string Bit16 = "Bmp/test16.bmp";
             public const string Bit16Inverted = "Bmp/test16-inverted.bmp";
             public const string Bit32Rgb = "Bmp/rgb32.bmp";
+            
+            // Note: This format can be called OS/2 BMPv1, or Windows BMPv2
+            public const string WinBmpv2 = "Bmp/pal8os2v1_winv2.bmp";
+            public const string Bit8Palette4 = "Bmp/pal8-0.bmp";
             public const string Os2v2Short = "Bmp/pal8os2v2-16.bmp";
 
             public static readonly string[] All

tests/ImageSharp.Tests/TestUtilities/TestEnvironment.Formats.cs

@@ -34,8 +34,7 @@ namespace SixLabors.ImageSharp.Tests
         {
             string extension = Path.GetExtension(filePath);
 
-            IImageFormat format = Configuration.ImageFormatsManager.FindFormatByFileExtension(extension);
-            return format;
+            return Configuration.ImageFormatsManager.FindFormatByFileExtension(extension);
         }
 
         private static void ConfigureCodecs(
@@ -69,7 +68,7 @@ namespace SixLabors.ImageSharp.Tests
 
             cfg.ConfigureCodecs(
                 BmpFormat.Instance,
-                SystemDrawingReferenceDecoder.Instance,
+                IsWindows ? (IImageDecoder)SystemDrawingReferenceDecoder.Instance : MagickReferenceDecoder.Instance,
                 bmpEncoder,
                 new BmpImageFormatDetector());
 

tests/ImageSharp.Tests/TestUtilities/Tests/TestEnvironmentTests.cs

@@ -127,7 +127,7 @@ namespace SixLabors.ImageSharp.Tests
 
         [Theory]
         [InlineData("lol/foo.png", typeof(MagickReferenceDecoder))]
-        [InlineData("lol/Rofl.bmp", typeof(SystemDrawingReferenceDecoder))]
+        [InlineData("lol/Rofl.bmp", typeof(MagickReferenceDecoder))]
         [InlineData("lol/Baz.JPG", typeof(JpegDecoder))]
         [InlineData("lol/Baz.gif", typeof(GifDecoder))]
         public void GetReferenceDecoder_ReturnsCorrectDecoders_Linux(string fileName, Type expectedDecoderType)

tests/Images/Input/Bmp/pal8-0.bmp

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:5ae0f808fea31352b8667fdaf897d057c7e02483c43d43cd735f92f4f149fe8d
+size 9270

tests/Images/Input/Bmp/pal8os2v1_winv2.bmp

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:2bfe739377020722872f7fa3ba3ff3ff0daa17f2f202c80e25cf66c4ec30e506
+size 8986