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

Author: brianpopow

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

@@ -51,10 +51,19 @@ internal sealed class BmpEncoderCore
         /// </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/BmpEncoderCore.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 @@ public IImageFormat DetectFormat(ReadOnlySpan<byte> header)
 
         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/BmpImageFormatDetector.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/IBmpDecoderOptions.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
+    }
+}

src/ImageSharp/Formats/Bmp/RleSkippedPixelHandling.cs

@@ -28,7 +28,7 @@ public abstract class FileTestBase
         /// <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