Feature: Bitmap RLE undefined pixel handling (#927)

* Add bitmap decoder option, how to treat skipped pixels for RLE

* Refactored bitmap tests into smaller tests, instead of just one test which goes through all bitmap files

* Add another adobe v3 header bitmap testcase

* Using the constant from BmpConstants to Identify bitmaps

* Bitmap decoder now can handle oversized palette's

* Add test for invalid palette size

* Renamed RleUndefinedPixelHandling to RleSkippedPixelHandling

* Explicitly using SystemDrawingReferenceDecoder in some BitmapDecoder tests

* Add test cases for unsupported bitmaps

* Comparing RLE test images to reference decoder only on windows

* Add test case for decoding winv4 fast path

* Add another 8 Bit RLE test with magick reference decoder

* Optimize RLE skipped pixel handling

* Refactor RLE decoding to eliminate code duplication

* Using MagickReferenceDecoder for the 8-Bit RLE test

src/ImageSharp/Formats/Bmp/BmpDecoder.cs

@@ -14,13 +14,18 @@ namespace SixLabors.ImageSharp.Formats.Bmp
     /// <list type="bullet">
     ///    <item>JPG</item>
     ///    <item>PNG</item>
-    ///    <item>RLE4</item>
+    ///    <item>Some OS/2 specific subtypes like: Bitmap Array, Color Icon, Color Pointer, Icon, Pointer.</item>
     /// </list>
     /// Formats will be supported in a later releases. We advise always
     /// to use only 24 Bit Windows bitmaps.
     /// </remarks>
     public sealed class BmpDecoder : IImageDecoder, IBmpDecoderOptions, IImageInfoDetector
     {
+        /// <summary>
+        /// Gets or sets a value indicating how to deal with skipped pixels, which can occur during decoding run length encoded bitmaps.
+        /// </summary>
+        public RleSkippedPixelHandling RleSkippedPixelHandling { get; set; } = RleSkippedPixelHandling.Black;
+
         /// <inheritdoc/>
         public Image<TPixel> Decode<TPixel>(Configuration configuration, Stream stream)
             where TPixel : struct, IPixel<TPixel>

src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 using System;
@@ -69,7 +69,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         private ImageMetadata metadata;
 
         /// <summary>
-        /// The bmp specific metadata.
+        /// The bitmap specific metadata.
         /// </summary>
         private BmpMetadata bmpMetadata;
 
@@ -83,10 +83,21 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// </summary>
         private BmpInfoHeader infoHeader;
 
+        /// <summary>
+        /// The global configuration.
+        /// </summary>
         private readonly Configuration configuration;
 
+        /// <summary>
+        /// Used for allocating memory during processing operations.
+        /// </summary>
         private readonly MemoryAllocator memoryAllocator;
 
+        /// <summary>
+        /// The bitmap decoder options.
+        /// </summary>
+        private readonly IBmpDecoderOptions options;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="BmpDecoderCore"/> class.
         /// </summary>
@@ -96,6 +107,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         {
             this.configuration = configuration;
             this.memoryAllocator = configuration.MemoryAllocator;
+            this.options = options;
         }
 
         /// <summary>
@@ -207,7 +219,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// <param name="width">The image width.</param>
         /// <param name="componentCount">The pixel component count.</param>
         /// <returns>
-        /// The <see cref="int"/>.
+        /// The padding.
         /// </returns>
         private static int CalculatePadding(int width, int componentCount)
         {
@@ -222,7 +234,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         }
 
         /// <summary>
-        /// Decodes a bitmap containing BITFIELDS Compression type. For each color channel, there will be bitmask
+        /// Decodes a bitmap containing the BITFIELDS Compression type. For each color channel, there will be a bitmask
         /// which will be used to determine which bits belong to that channel.
         /// </summary>
         /// <typeparam name="TPixel">The pixel format.</typeparam>
@@ -258,8 +270,8 @@ namespace SixLabors.ImageSharp.Formats.Bmp
 
         /// <summary>
         /// Looks up color values and builds the image from de-compressed RLE8 or RLE4 data.
-        /// Compressed RLE8 stream is uncompressed by <see cref="UncompressRle8(int, Span{byte})"/>
-        /// Compressed RLE4 stream is uncompressed by <see cref="UncompressRle4(int, Span{byte})"/>
+        /// Compressed RLE8 stream is uncompressed by <see cref="UncompressRle8(int, Span{byte}, Span{bool}, Span{bool})"/>
+        /// Compressed RLE4 stream is uncompressed by <see cref="UncompressRle4(int, Span{byte}, Span{bool}, Span{bool})"/>
         /// </summary>
         /// <typeparam name="TPixel">The pixel format.</typeparam>
         /// <param name="compression">The compression type. Either RLE4 or RLE8.</param>
@@ -273,14 +285,17 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         {
             TPixel color = default;
             using (Buffer2D<byte> buffer = this.memoryAllocator.Allocate2D<byte>(width, height, AllocationOptions.Clean))
+            using (Buffer2D<bool> undefinedPixels = this.memoryAllocator.Allocate2D<bool>(width, height, AllocationOptions.Clean))
+            using (IMemoryOwner<bool> rowsWithUndefinedPixels = this.memoryAllocator.Allocate<bool>(height, AllocationOptions.Clean))
             {
+                Span<bool> rowsWithUndefinedPixelsSpan = rowsWithUndefinedPixels.Memory.Span;
                 if (compression == BmpCompression.RLE8)
                 {
-                    this.UncompressRle8(width, buffer.GetSpan());
+                    this.UncompressRle8(width, buffer.GetSpan(), undefinedPixels.GetSpan(), rowsWithUndefinedPixelsSpan);
                 }
                 else
                 {
-                    this.UncompressRle4(width, buffer.GetSpan());
+                    this.UncompressRle4(width, buffer.GetSpan(), undefinedPixels.GetSpan(), rowsWithUndefinedPixelsSpan);
                 }
 
                 for (int y = 0; y < height; y++)
@@ -289,10 +304,46 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                     Span<byte> bufferRow = buffer.GetRowSpan(y);
                     Span<TPixel> pixelRow = pixels.GetRowSpan(newY);
 
-                    for (int x = 0; x < width; x++)
+                    bool rowHasUndefinedPixels = rowsWithUndefinedPixelsSpan[y];
+                    if (rowHasUndefinedPixels)
                     {
-                        color.FromBgr24(Unsafe.As<byte, Bgr24>(ref colors[bufferRow[x] * 4]));
-                        pixelRow[x] = color;
+                        // Slow path with undefined pixels.
+                        for (int x = 0; x < width; x++)
+                        {
+                            byte colorIdx = bufferRow[x];
+                            if (undefinedPixels[x, y])
+                            {
+                                switch (this.options.RleSkippedPixelHandling)
+                                {
+                                    case RleSkippedPixelHandling.FirstColorOfPalette:
+                                        color.FromBgr24(Unsafe.As<byte, Bgr24>(ref colors[colorIdx * 4]));
+                                        break;
+                                    case RleSkippedPixelHandling.Transparent:
+                                        color.FromVector4(Vector4.Zero);
+                                        break;
+
+                                    // Default handling for skipped pixels is black (which is what System.Drawing is also doing).
+                                    default:
+                                        color.FromVector4(new Vector4(0.0f, 0.0f, 0.0f, 1.0f));
+                                        break;
+                                }
+                            }
+                            else
+                            {
+                                color.FromBgr24(Unsafe.As<byte, Bgr24>(ref colors[colorIdx * 4]));
+                            }
+
+                            pixelRow[x] = color;
+                        }
+                    }
+                    else
+                    {
+                        // Fast path without any undefined pixels.
+                        for (int x = 0; x < width; x++)
+                        {
+                            color.FromBgr24(Unsafe.As<byte, Bgr24>(ref colors[bufferRow[x] * 4]));
+                            pixelRow[x] = color;
+                        }
                     }
                 }
             }
@@ -308,7 +359,9 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// </remarks>
         /// <param name="w">The width of the bitmap.</param>
         /// <param name="buffer">Buffer for uncompressed data.</param>
-        private void UncompressRle4(int w, Span<byte> buffer)
+        /// <param name="undefinedPixels">Keeps track over skipped and therefore undefined pixels.</param>
+        /// <param name="rowsWithUndefinedPixels">Keeps track of rows, which have undefined pixels.</param>
+        private void UncompressRle4(int w, Span<byte> buffer, Span<bool> undefinedPixels, Span<bool> rowsWithUndefinedPixels)
         {
 #if NETCOREAPP2_1
             Span<byte> cmd = stackalloc byte[2];
@@ -329,21 +382,20 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                     switch (cmd[1])
                     {
                         case RleEndOfBitmap:
+                            int skipEoB = buffer.Length - count;
+                            RleSkipEndOfBitmap(count, w, skipEoB, undefinedPixels, rowsWithUndefinedPixels);
+
                             return;
 
                         case RleEndOfLine:
-                            int extra = count % w;
-                            if (extra > 0)
-                            {
-                                count += w - extra;
-                            }
+                            count += RleSkipEndOfLine(count, w, undefinedPixels, rowsWithUndefinedPixels);
 
                             break;
 
                         case RleDelta:
                             int dx = this.stream.ReadByte();
                             int dy = this.stream.ReadByte();
-                            count += (w * dy) + dx;
+                            count += RleSkipDelta(count, w, dx, dy, undefinedPixels, rowsWithUndefinedPixels);
 
                             break;
 
@@ -374,7 +426,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                                 }
                             }
 
-                            // Absolute mode data is aligned to two-byte word-boundary
+                            // Absolute mode data is aligned to two-byte word-boundary.
                             int padding = bytesToRead & 1;
 
                             this.stream.Skip(padding);
@@ -418,7 +470,9 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// </remarks>
         /// <param name="w">The width of the bitmap.</param>
         /// <param name="buffer">Buffer for uncompressed data.</param>
-        private void UncompressRle8(int w, Span<byte> buffer)
+        /// <param name="undefinedPixels">Keeps track of skipped and therefore undefined pixels.</param>
+        /// <param name="rowsWithUndefinedPixels">Keeps track of rows, which have undefined pixels.</param>
+        private void UncompressRle8(int w, Span<byte> buffer, Span<bool> undefinedPixels, Span<bool> rowsWithUndefinedPixels)
         {
 #if NETCOREAPP2_1
             Span<byte> cmd = stackalloc byte[2];
@@ -439,27 +493,26 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                     switch (cmd[1])
                     {
                         case RleEndOfBitmap:
+                            int skipEoB = buffer.Length - count;
+                            RleSkipEndOfBitmap(count, w, skipEoB, undefinedPixels, rowsWithUndefinedPixels);
+
                             return;
 
                         case RleEndOfLine:
-                            int extra = count % w;
-                            if (extra > 0)
-                            {
-                                count += w - extra;
-                            }
+                            count += RleSkipEndOfLine(count, w, undefinedPixels, rowsWithUndefinedPixels);
 
                             break;
 
                         case RleDelta:
                             int dx = this.stream.ReadByte();
                             int dy = this.stream.ReadByte();
-                            count += (w * dy) + dx;
+                            count += RleSkipDelta(count, w, dx, dy, undefinedPixels, rowsWithUndefinedPixels);
 
                             break;
 
                         default:
-                            // If the second byte > 2, we are in 'absolute mode'
-                            // Take this number of bytes from the stream as uncompressed data
+                            // If the second byte > 2, we are in 'absolute mode'.
+                            // Take this number of bytes from the stream as uncompressed data.
                             int length = cmd[1];
 
                             byte[] run = new byte[length];
@@ -470,7 +523,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
 
                             count += run.Length;
 
-                            // Absolute mode data is aligned to two-byte word-boundary
+                            // Absolute mode data is aligned to two-byte word-boundary.
                             int padding = length & 1;
 
                             this.stream.Skip(padding);
@@ -481,16 +534,105 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                 else
                 {
                     int max = count + cmd[0]; // as we start at the current count in the following loop, max is count + cmd[0]
-                    byte cmd1 = cmd[1]; // store the value to avoid the repeated indexer access inside the loop
+                    byte colorIdx = cmd[1]; // store the value to avoid the repeated indexer access inside the loop.
 
                     for (; count < max; count++)
                     {
-                        buffer[count] = cmd1;
+                        buffer[count] = colorIdx;
                     }
                 }
             }
         }
 
+        /// <summary>
+        /// Keeps track of skipped / undefined pixels, when EndOfBitmap command occurs.
+        /// </summary>
+        /// <param name="count">The already processed pixel count.</param>
+        /// <param name="w">The width of the image.</param>
+        /// <param name="skipPixelCount">The skipped pixel count.</param>
+        /// <param name="undefinedPixels">The undefined pixels.</param>
+        /// <param name="rowsWithUndefinedPixels">Rows with undefined pixels.</param>
+        private static void RleSkipEndOfBitmap(
+            int count,
+            int w,
+            int skipPixelCount,
+            Span<bool> undefinedPixels,
+            Span<bool> rowsWithUndefinedPixels)
+        {
+            for (int i = count; i < count + skipPixelCount; i++)
+            {
+                undefinedPixels[i] = true;
+            }
+
+            int skippedRowIdx = count / w;
+            int skippedRows = (skipPixelCount / w) - 1;
+            int lastSkippedRow = Math.Min(skippedRowIdx + skippedRows, rowsWithUndefinedPixels.Length - 1);
+            for (int i = skippedRowIdx; i <= lastSkippedRow; i++)
+            {
+                rowsWithUndefinedPixels[i] = true;
+            }
+        }
+
+        /// <summary>
+        /// Keeps track of undefined / skipped pixels, when the EndOfLine command occurs.
+        /// </summary>
+        /// <param name="count">The already processed pixel count.</param>
+        /// <param name="w">The width of image.</param>
+        /// <param name="undefinedPixels">The undefined pixels.</param>
+        /// <param name="rowsWithUndefinedPixels">The rows with undefined pixels.</param>
+        /// <returns>The number of skipped pixels.</returns>
+        private static int RleSkipEndOfLine(int count, int w, Span<bool> undefinedPixels, Span<bool> rowsWithUndefinedPixels)
+        {
+            rowsWithUndefinedPixels[count / w] = true;
+            int remainingPixelsInRow = count % w;
+            if (remainingPixelsInRow > 0)
+            {
+                int skipEoL = w - remainingPixelsInRow;
+                for (int i = count; i < count + skipEoL; i++)
+                {
+                    undefinedPixels[i] = true;
+                }
+
+                return skipEoL;
+            }
+
+            return 0;
+        }
+
+        /// <summary>
+        /// Keeps track of undefined / skipped pixels, when the delta command occurs.
+        /// </summary>
+        /// <param name="count">The count.</param>
+        /// <param name="w">The width of the image.</param>
+        /// <param name="dx">Delta skip in x direction.</param>
+        /// <param name="dy">Delta skip in y direction.</param>
+        /// <param name="undefinedPixels">The undefined pixels.</param>
+        /// <param name="rowsWithUndefinedPixels">The rows with undefined pixels.</param>
+        /// <returns>The number of skipped pixels.</returns>
+        private static int RleSkipDelta(
+            int count,
+            int w,
+            int dx,
+            int dy,
+            Span<bool> undefinedPixels,
+            Span<bool> rowsWithUndefinedPixels)
+        {
+            int skipDelta = (w * dy) + dx;
+            for (int i = count; i < count + skipDelta; i++)
+            {
+                undefinedPixels[i] = true;
+            }
+
+            int skippedRowIdx = count / w;
+            int lastSkippedRow = Math.Min(skippedRowIdx + dy, rowsWithUndefinedPixels.Length - 1);
+            for (int i = skippedRowIdx; i <= lastSkippedRow; i++)
+            {
+                rowsWithUndefinedPixels[i] = true;
+            }
+
+            return skipDelta;
+        }
+
         /// <summary>
         /// Reads the color palette from the stream.
         /// </summary>
@@ -506,7 +648,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         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)
+            // Pixels per byte (bits per pixel).
             int ppb = 8 / bitsPerPixel;
 
             int arrayWidth = (width + ppb - 1) / ppb;
@@ -514,7 +656,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             // Bit mask
             int mask = 0xFF >> (8 - bitsPerPixel);
 
-            // Rows are aligned on 4 byte boundaries
+            // Rows are aligned on 4 byte boundaries.
             int padding = arrayWidth % 4;
             if (padding != 0)
             {
@@ -810,11 +952,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             uint maxValueAlpha = 0xFFFFFFFF >> (32 - bitsAlphaMask);
             float invMaxValueAlpha = 1.0f / maxValueAlpha;
 
-            bool unusualBitMask = false;
-            if (bitsRedMask > 8 || bitsGreenMask > 8 || bitsBlueMask > 8 || invMaxValueAlpha > 8)
-            {
-                unusualBitMask = true;
-            }
+            bool unusualBitMask = bitsRedMask > 8 || bitsGreenMask > 8 || bitsBlueMask > 8 || invMaxValueAlpha > 8;
 
             using (IManagedByteBuffer buffer = this.memoryAllocator.AllocateManagedByteBuffer(stride))
             {
@@ -910,7 +1048,9 @@ namespace SixLabors.ImageSharp.Formats.Bmp
 #else
             byte[] buffer = new byte[BmpInfoHeader.MaxHeaderSize];
 #endif
-            this.stream.Read(buffer, 0, BmpInfoHeader.HeaderSizeSize); // read the header size
+
+            // Read the header size.
+            this.stream.Read(buffer, 0, BmpInfoHeader.HeaderSizeSize);
 
             int headerSize = BinaryPrimitives.ReadInt32LittleEndian(buffer);
             if (headerSize < BmpInfoHeader.CoreSize)
@@ -925,7 +1065,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                 headerSize = BmpInfoHeader.MaxHeaderSize;
             }
 
-            // read the rest of the header
+            // Read the rest of the header.
             this.stream.Read(buffer, BmpInfoHeader.HeaderSizeSize, headerSize - BmpInfoHeader.HeaderSizeSize);
 
             BmpInfoHeaderType infoHeaderType = BmpInfoHeaderType.WinVersion2;
@@ -953,7 +1093,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                 {
                     byte[] bitfieldsBuffer = new byte[12];
                     this.stream.Read(bitfieldsBuffer, 0, 12);
-                    Span<byte> data = bitfieldsBuffer.AsSpan<byte>();
+                    Span<byte> data = bitfieldsBuffer.AsSpan();
                     this.infoHeader.RedMask = BinaryPrimitives.ReadInt32LittleEndian(data.Slice(0, 4));
                     this.infoHeader.GreenMask = BinaryPrimitives.ReadInt32LittleEndian(data.Slice(4, 4));
                     this.infoHeader.BlueMask = BinaryPrimitives.ReadInt32LittleEndian(data.Slice(8, 4));
@@ -962,7 +1102,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                 {
                     byte[] bitfieldsBuffer = new byte[16];
                     this.stream.Read(bitfieldsBuffer, 0, 16);
-                    Span<byte> data = bitfieldsBuffer.AsSpan<byte>();
+                    Span<byte> data = bitfieldsBuffer.AsSpan();
                     this.infoHeader.RedMask = BinaryPrimitives.ReadInt32LittleEndian(data.Slice(0, 4));
                     this.infoHeader.GreenMask = BinaryPrimitives.ReadInt32LittleEndian(data.Slice(4, 4));
                     this.infoHeader.BlueMask = BinaryPrimitives.ReadInt32LittleEndian(data.Slice(8, 4));
@@ -1021,7 +1161,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             this.bmpMetadata = this.metadata.GetFormatMetadata(BmpFormat.Instance);
             this.bmpMetadata.InfoHeaderType = infoHeaderType;
 
-            // We can only encode at these bit rates so far.
+            // We can only encode at these bit rates so far (1 bit and 4 bit are still missing).
             if (bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel8)
                 || bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel16)
                 || bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel24)
@@ -1030,7 +1170,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                 this.bmpMetadata.BitsPerPixel = (BmpBitsPerPixel)bitsPerPixel;
             }
 
-            // skip the remaining header because we can't read those parts
+            // Skip the remaining header because we can't read those parts.
             this.stream.Skip(skipAmount);
         }
 
@@ -1105,10 +1245,12 @@ namespace SixLabors.ImageSharp.Formats.Bmp
 
             if (colorMapSize > 0)
             {
-                // 256 * 4
-                if (colorMapSize > 1024)
+                // Usually the color palette is 1024 byte (256 colors * 4), but the documentation does not mention a size limit.
+                // Make sure, that we will not read pass the bitmap offset (starting position of image data).
+                if ((this.stream.Position + colorMapSize) > this.fileHeader.Offset)
                 {
-                    BmpThrowHelper.ThrowImageFormatException($"Invalid bmp colormap size '{colorMapSize}'");
+                    BmpThrowHelper.ThrowImageFormatException(
+                        $"Reading the color map would read beyond the bitmap offset. Either the color map size of '{colorMapSize}' is invalid or the bitmap offset.");
                 }
 
                 palette = new byte[colorMapSize];
@@ -1121,7 +1263,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             int skipAmount = this.fileHeader.Offset - (int)this.stream.Position;
             if ((skipAmount + (int)this.stream.Position) > this.stream.Length)
             {
-                BmpThrowHelper.ThrowImageFormatException($"Invalid fileheader offset found. Offset is greater than the stream length.");
+                BmpThrowHelper.ThrowImageFormatException("Invalid fileheader offset found. Offset is greater than the stream length.");
             }
 
             if (skipAmount > 0)
@@ -1132,4 +1274,4 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             return bytesPerColorMapEntry;
         }
     }
-}
+}

src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs

@@ -51,10 +51,19 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// </summary>
         private const int ColorPaletteSize8Bit = 1024;
 
+        /// <summary>
+        /// Used for allocating memory during processing operations.
+        /// </summary>
         private readonly MemoryAllocator memoryAllocator;
 
+        /// <summary>
+        /// The global configuration.
+        /// </summary>
         private Configuration configuration;
 
+        /// <summary>
+        /// The color depth, in number of bits per pixel.
+        /// </summary>
         private BmpBitsPerPixel? bitsPerPixel;
 
         /// <summary>

src/ImageSharp/Formats/Bmp/BmpImageFormatDetector.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0.
 
 using System;
+using System.Buffers.Binary;
 
 namespace SixLabors.ImageSharp.Formats.Bmp
 {
@@ -21,10 +22,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
 
         private bool IsSupportedFileFormat(ReadOnlySpan<byte> header)
         {
-            // TODO: This should be in constants
-            return header.Length >= this.HeaderSize
-                && header[0] == 0x42 // B
-                && header[1] == 0x4D; // M
+            return header.Length >= this.HeaderSize && BinaryPrimitives.ReadInt16LittleEndian(header) == BmpConstants.TypeMarkers.Bitmap;
         }
     }
 }

src/ImageSharp/Formats/Bmp/IBmpDecoderOptions.cs

@@ -8,6 +8,9 @@ namespace SixLabors.ImageSharp.Formats.Bmp
     /// </summary>
     internal interface IBmpDecoderOptions
     {
-        // added this for consistency so we can add stuff as required, no options currently available
+        /// <summary>
+        /// Gets the value indicating how to deal with skipped pixels, which can occur during decoding run length encoded bitmaps.
+        /// </summary>
+        RleSkippedPixelHandling RleSkippedPixelHandling { get; }
     }
 }

src/ImageSharp/Formats/Bmp/RleSkippedPixelHandling.cs

@@ -0,0 +1,26 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+namespace SixLabors.ImageSharp.Formats.Bmp
+{
+    /// <summary>
+    /// Defines possible options, how skipped pixels during decoding of run length encoded bitmaps should be treated.
+    /// </summary>
+    public enum RleSkippedPixelHandling : int
+    {
+        /// <summary>
+        /// Undefined pixels should be black. This is the default behavior and equal to how System.Drawing handles undefined pixels.
+        /// </summary>
+        Black = 0,
+
+        /// <summary>
+        /// Undefined pixels should be transparent.
+        /// </summary>
+        Transparent = 1,
+
+        /// <summary>
+        /// Undefined pixels should have the first color of the palette.
+        /// </summary>
+        FirstColorOfPalette = 2
+    }
+}

tests/ImageSharp.Tests/FileTestBase.cs

@@ -28,7 +28,7 @@ namespace SixLabors.ImageSharp.Tests
         /// <summary>
         /// A collection of all the bmp test images
         /// </summary>
-        public static IEnumerable<string> AllBmpFiles = TestImages.Bmp.All;
+        public static IEnumerable<string> AllBmpFiles = TestImages.Bmp.Benchmark;
 
         /// <summary>
         /// A collection of all the jpeg test images

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

@@ -1,6 +1,7 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
+using System;
 using System.IO;
 using SixLabors.ImageSharp.Formats.Bmp;
 using SixLabors.ImageSharp.PixelFormats;
@@ -20,27 +21,26 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
     {
         public const PixelTypes CommonNonDefaultPixelTypes = PixelTypes.Rgba32 | PixelTypes.Bgra32 | PixelTypes.RgbaVector;
 
-        public static readonly string[] AllBmpFiles = All;
+        public static readonly string[] MiscBmpFiles = Miscellaneous;
 
         public static readonly string[] BitfieldsBmpFiles = BitFields;
 
         public static readonly TheoryData<string, int, int, PixelResolutionUnit> RatioFiles =
         new TheoryData<string, int, int, PixelResolutionUnit>
         {
-            { TestImages.Bmp.Car, 3780, 3780 , PixelResolutionUnit.PixelsPerMeter },
-            { TestImages.Bmp.V5Header, 3780, 3780 , PixelResolutionUnit.PixelsPerMeter },
-            { TestImages.Bmp.RLE8, 2835, 2835, PixelResolutionUnit.PixelsPerMeter }
+            { Car, 3780, 3780 , PixelResolutionUnit.PixelsPerMeter },
+            { V5Header, 3780, 3780 , PixelResolutionUnit.PixelsPerMeter },
+            { RLE8, 2835, 2835, PixelResolutionUnit.PixelsPerMeter }
         };
 
         [Theory]
-        [WithFileCollection(nameof(AllBmpFiles), PixelTypes.Rgba32)]
-        public void DecodeBmp<TPixel>(TestImageProvider<TPixel> provider)
+        [WithFileCollection(nameof(MiscBmpFiles), PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_MiscellaneousBitmaps<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : struct, IPixel<TPixel>
         {
             using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
             {
                 image.DebugSave(provider);
-
                 if (TestEnvironment.IsWindows)
                 {
                     image.CompareToOriginal(provider);
@@ -60,6 +60,174 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
             }
         }
 
+        [Theory]
+        [WithFile(Bit16Inverted, PixelTypes.Rgba32)]
+        [WithFile(Bit8Inverted, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_Inverted<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider);
+            }
+        }
+
+        [Theory]
+        [WithFile(Bit1, PixelTypes.Rgba32)]
+        [WithFile(Bit1Pal1, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_1Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider, new SystemDrawingReferenceDecoder());
+            }
+        }
+
+        [Theory]
+        [WithFile(Bit4, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_4Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                // The Magick Reference Decoder can not decode 4-Bit bitmaps, so only execute this on windows.
+                if (TestEnvironment.IsWindows)
+                {
+                    image.CompareToOriginal(provider);
+                }
+            }
+        }
+
+        [Theory]
+        [WithFile(Bit8, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_8Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider);
+            }
+        }
+
+        [Theory]
+        [WithFile(Bit16, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_16Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider);
+            }
+        }
+
+        [Theory]
+        [WithFile(Bit32Rgb, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_32Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider);
+            }
+        }
+
+        [Theory]
+        [WithFile(Rgba32v4, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_32BitV4Header_Fast<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider);
+            }
+        }
+
+        [Theory]
+        [WithFile(RLE4Cut, PixelTypes.Rgba32)]
+        [WithFile(RLE4Delta, PixelTypes.Rgba32)]
+        [WithFile(Rle4Delta320240, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_RunLengthEncoded_4Bit_WithDelta<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder() { RleSkippedPixelHandling = RleSkippedPixelHandling.Black }))
+            {
+                image.DebugSave(provider);
+                // The Magick Reference Decoder can not decode 4-Bit bitmaps, so only execute this on windows.
+                if (TestEnvironment.IsWindows)
+                {
+                    image.CompareToOriginal(provider);
+                }
+            }
+        }
+
+        [Theory]
+        [WithFile(RLE4, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_RunLengthEncoded_4Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder() { RleSkippedPixelHandling = RleSkippedPixelHandling.Black }))
+            {
+                image.DebugSave(provider);
+                // The Magick Reference Decoder can not decode 4-Bit bitmaps, so only execute this on windows.
+                if (TestEnvironment.IsWindows)
+                {
+                    image.CompareToOriginal(provider);
+                }
+            }
+        }
+
+        [Theory]
+        [WithFile(RLE8Cut, PixelTypes.Rgba32)]
+        [WithFile(RLE8Delta, PixelTypes.Rgba32)]
+        [WithFile(Rle8Delta320240, PixelTypes.Rgba32)]
+        [WithFile(Rle8Blank160120, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_RunLengthEncoded_8Bit_WithDelta_SystemDrawingRefDecoder<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder() { RleSkippedPixelHandling = RleSkippedPixelHandling.Black }))
+            {
+                image.DebugSave(provider);
+                if (TestEnvironment.IsWindows)
+                {
+                    image.CompareToOriginal(provider, new SystemDrawingReferenceDecoder());
+                }
+            }
+        }
+
+        [Theory]
+        [WithFile(RLE8Cut, PixelTypes.Rgba32)]
+        [WithFile(RLE8Delta, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_RunLengthEncoded_8Bit_WithDelta_MagickRefDecoder<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder() { RleSkippedPixelHandling = RleSkippedPixelHandling.FirstColorOfPalette }))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider, new MagickReferenceDecoder());
+            }
+        }
+
+        [Theory]
+        [WithFile(RLE8, PixelTypes.Rgba32)]
+        [WithFile(RLE8Inverted, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_RunLengthEncoded_8Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder() { RleSkippedPixelHandling = RleSkippedPixelHandling.FirstColorOfPalette }))
+            {
+                image.DebugSave(provider);
+                image.CompareToOriginal(provider, new MagickReferenceDecoder());
+            }
+        }
+
         [Theory]
         [WithFile(RgbaAlphaBitfields, PixelTypes.Rgba32)]
         public void BmpDecoder_CanDecodeAlphaBitfields<TPixel>(TestImageProvider<TPixel> provider)
@@ -106,6 +274,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
 
         [Theory]
         [WithFile(WinBmpv2, PixelTypes.Rgba32)]
+        [WithFile(CoreHeader, PixelTypes.Rgba32)]
         public void BmpDecoder_CanDecodeBmpv2<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : struct, IPixel<TPixel>
         {
@@ -141,7 +310,41 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
         }
 
         [Theory]
-        [WithFile(Rgba32bf56, PixelTypes.Rgba32)]
+        [WithFile(OversizedPalette, PixelTypes.Rgba32)]
+        [WithFile(Rgb24LargePalette, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecodeOversizedPalette<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder()))
+            {
+                image.DebugSave(provider);
+                if (TestEnvironment.IsWindows)
+                {
+                    image.CompareToOriginal(provider);
+                }
+            }
+        }
+
+        [Theory]
+        [WithFile(InvalidPaletteSize, PixelTypes.Rgba32)]
+        public void BmpDecoder_ThrowsImageFormatException_OnInvalidPaletteSize<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            Assert.Throws<ImageFormatException>( () => { using (Image<TPixel> image = provider.GetImage(new BmpDecoder())) { } });
+        }
+
+        [Theory]
+        [WithFile(Rgb24jpeg, PixelTypes.Rgba32)]
+        [WithFile(Rgb24png, PixelTypes.Rgba32)]
+        public void BmpDecoder_ThrowsNotSupportedException_OnUnsupportedBitmaps<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            Assert.Throws<NotSupportedException>(() => { using (Image<TPixel> image = provider.GetImage(new BmpDecoder())) { } });
+        }
+
+        [Theory]
+        [WithFile(Rgba32bf56AdobeV3, PixelTypes.Rgba32)]
+        [WithFile(Rgb32h52AdobeV3, PixelTypes.Rgba32)]
         public void BmpDecoder_CanDecodeAdobeBmpv3<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : struct, IPixel<TPixel>
         {
@@ -166,6 +369,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
 
         [Theory]
         [WithFile(WinBmpv5, PixelTypes.Rgba32)]
+        [WithFile(V5Header, PixelTypes.Rgba32)]
         public void BmpDecoder_CanDecodeBmpv5<TPixel>(TestImageProvider<TPixel> provider)
             where TPixel : struct, IPixel<TPixel>
         {
@@ -223,6 +427,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
         [InlineData(Bit8, 8)]
         [InlineData(Bit8Inverted, 8)]
         [InlineData(Bit4, 4)]
+        [InlineData(Bit1, 1)]
+        [InlineData(Bit1Pal1, 1)]
         public void Identify(string imagePath, int expectedPixelSize)
         {
             var testFile = TestFile.Create(imagePath);
@@ -281,4 +487,4 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
             }
         }
     }
-}
+}

tests/ImageSharp.Tests/ProfilingBenchmarks/JpegProfilingBenchmarks.cs

@@ -82,7 +82,7 @@ namespace SixLabors.ImageSharp.Tests.ProfilingBenchmarks
                 return;
             }
 
-            string[] testFiles = TestImages.Bmp.All
+            string[] testFiles = TestImages.Bmp.Benchmark
                 .Concat(new[] { TestImages.Jpeg.Baseline.Calliphora, TestImages.Jpeg.Baseline.Cmyk }).ToArray();
 
             Image<Rgba32>[] testImages = testFiles.Select(

tests/ImageSharp.Tests/TestImages.cs

@@ -231,8 +231,15 @@ namespace SixLabors.ImageSharp.Tests
             public const string CoreHeader = "Bmp/BitmapCoreHeaderQR.bmp";
             public const string V5Header = "Bmp/BITMAPV5HEADER.bmp";
             public const string RLE8 = "Bmp/RunLengthEncoded.bmp";
+            public const string RLE8Cut = "Bmp/pal8rlecut.bmp";
+            public const string RLE8Delta = "Bmp/pal8rletrns.bmp";
+            public const string Rle8Delta320240 = "Bmp/rle8-delta-320x240.bmp";
+            public const string Rle8Blank160120 = "Bmp/rle8-blank-160x120.bmp";
+            public const string RLE8Inverted = "Bmp/RunLengthEncoded-inverted.bmp";
             public const string RLE4 = "Bmp/pal4rle.bmp";
-            public const string RLEInverted = "Bmp/RunLengthEncoded-inverted.bmp";
+            public const string RLE4Cut = "Bmp/pal4rlecut.bmp";
+            public const string RLE4Delta = "Bmp/pal4rletrns.bmp";
+            public const string Rle4Delta320240 = "Bmp/rle4-delta-320x240.bmp";
             public const string Bit1 = "Bmp/pal1.bmp";
             public const string Bit1Pal1 = "Bmp/pal1p1.bmp";
             public const string Bit4 = "Bmp/pal4.bmp";
@@ -256,15 +263,22 @@ namespace SixLabors.ImageSharp.Tests
             public const string Os2v2 = "Bmp/pal8os2v2.bmp";
             public const string LessThanFullSizedPalette = "Bmp/pal8os2sp.bmp";
             public const string Pal8Offset = "Bmp/pal8offs.bmp";
+            public const string OversizedPalette = "Bmp/pal8oversizepal.bmp";
+            public const string Rgb24LargePalette = "Bmp/rgb24largepal.bmp";
+            public const string InvalidPaletteSize = "Bmp/invalidPaletteSize.bmp";
+            public const string Rgb24jpeg = "Bmp/rgb24jpeg.bmp";
+            public const string Rgb24png = "Bmp/rgb24png.bmp";
+            public const string Rgba32v4 = "Bmp/rgba32v4.bmp";
 
-            // Bitmap images with compression type BITFIELDS
+            // Bitmap images with compression type BITFIELDS.
             public const string Rgb32bfdef = "Bmp/rgb32bfdef.bmp";
             public const string Rgb32bf = "Bmp/rgb32bf.bmp";
             public const string Rgb16bfdef = "Bmp/rgb16bfdef.bmp";
             public const string Rgb16565 = "Bmp/rgb16-565.bmp";
             public const string Rgb16565pal = "Bmp/rgb16-565pal.bmp";
             public const string Issue735 = "Bmp/issue735.bmp";
-            public const string Rgba32bf56 = "Bmp/rgba32h56.bmp";
+            public const string Rgba32bf56AdobeV3 = "Bmp/rgba32h56.bmp";
+            public const string Rgb32h52AdobeV3 = "Bmp/rgb32h52.bmp";
             public const string Rgba321010102 = "Bmp/rgba32-1010102.bmp";
             public const string RgbaAlphaBitfields = "Bmp/rgba32abf.bmp";
 
@@ -278,25 +292,32 @@ namespace SixLabors.ImageSharp.Tests
                   Issue735,
             };
 
-            public static readonly string[] All
+            public static readonly string[] Miscellaneous
             = {
                 Car,
                 F,
-                NegHeight,
-                CoreHeader,
-                V5Header,
-                RLE4,
-                RLE8,
-                RLEInverted,
-                Bit1,
-                Bit1Pal1,
-                Bit4,
-                Bit8,
-                Bit8Inverted,
-                Bit16,
-                Bit16Inverted,
-                Bit32Rgb
+                NegHeight
             };
+
+            public static readonly string[] Benchmark
+                = {
+                          Car,
+                          F,
+                          NegHeight,
+                          CoreHeader,
+                          V5Header,
+                          RLE4,
+                          RLE8,
+                          RLE8Inverted,
+                          Bit1,
+                          Bit1Pal1,
+                          Bit4,
+                          Bit8,
+                          Bit8Inverted,
+                          Bit16,
+                          Bit16Inverted,
+                          Bit32Rgb
+                      };
         }
 
         public static class Gif