Add support for decoding RLE24 Bitmaps (#939)

* Add support for decoding RLE24

* Simplified determining colorMapSize, OS/2 always has 3 bytes for each palette entry

* Enum value for RLE24 is remapped to a different value, to be clearly separate from valid windows values.

src/ImageSharp/Formats/Bmp/BmpCompression.cs

@@ -1,10 +1,10 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 namespace SixLabors.ImageSharp.Formats.Bmp
 {
     /// <summary>
-    /// Defines how the compression type of the image data
+    /// Defines the compression type of the image data
     /// in the bitmap file.
     /// </summary>
     internal enum BmpCompression : int
@@ -21,7 +21,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
 
         /// <summary>
         /// Two bytes are one data record. If the first byte is not zero, the
-        /// next two half bytes will be repeated as much as the value of the first byte.
+        /// next byte will be repeated as much as the value of the first byte.
         /// If the first byte is zero, the record has different meanings, depending
         /// on the second byte. If the second byte is zero, it is the end of the row,
         /// if it is one, it is the end of the image.
@@ -30,7 +30,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
 
         /// <summary>
         /// Two bytes are one data record. If the first byte is not zero, the
-        /// next byte will be repeated as much as the value of the first byte.
+        /// next two half bytes will be repeated as much as the value of the first byte.
         /// If the first byte is zero, the record has different meanings, depending
         /// on the second byte. If the second byte is zero, it is the end of the row,
         /// if it is one, it is the end of the image.
@@ -60,6 +60,17 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// Specifies that the bitmap is not compressed and that the color table consists of four DWORD color
         /// masks that specify the red, green, blue, and alpha components of each pixel.
         /// </summary>
-        BI_ALPHABITFIELDS = 6
+        BI_ALPHABITFIELDS = 6,
+
+        /// <summary>
+        /// OS/2 specific compression type.
+        /// Similar to run length encoding of 4 and 8 bit.
+        /// The only difference is that run values encoded are three bytes in size (one byte per RGB color component),
+        /// rather than four or eight bits in size.
+        ///
+        /// Note: Because compression value of 4 is ambiguous for BI_RGB for windows and RLE24 for OS/2, the enum value is remapped
+        /// to a different value.
+        /// </summary>
+        RLE24 = 100,
     }
-}
+}

src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs

@@ -39,22 +39,22 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         private const int DefaultRgb16BMask = 0x1F;
 
         /// <summary>
-        /// RLE8 flag value that indicates following byte has special meaning.
+        /// RLE flag value that indicates following byte has special meaning.
         /// </summary>
         private const int RleCommand = 0x00;
 
         /// <summary>
-        /// RLE8 flag value marking end of a scan line.
+        /// RLE flag value marking end of a scan line.
         /// </summary>
         private const int RleEndOfLine = 0x00;
 
         /// <summary>
-        /// RLE8 flag value marking end of bitmap data.
+        /// RLE flag value marking end of bitmap data.
         /// </summary>
         private const int RleEndOfBitmap = 0x01;
 
         /// <summary>
-        /// RLE8 flag value marking the start of [x,y] offset instruction.
+        /// RLE flag value marking the start of [x,y] offset instruction.
         /// </summary>
         private const int RleDelta = 0x02;
 
@@ -78,6 +78,11 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// </summary>
         private BmpFileHeader fileHeader;
 
+        /// <summary>
+        /// Indicates which bitmap file marker was read.
+        /// </summary>
+        private BmpFileMarkerType fileMarkerType;
+
         /// <summary>
         /// The info header containing detailed information about the bitmap.
         /// </summary>
@@ -167,6 +172,12 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                         }
 
                         break;
+
+                    case BmpCompression.RLE24:
+                        this.ReadRle24(pixels, this.infoHeader.Width, this.infoHeader.Height, inverted);
+
+                        break;
+
                     case BmpCompression.RLE8:
                     case BmpCompression.RLE4:
                         this.ReadRle(this.infoHeader.Compression, pixels, palette, this.infoHeader.Width, this.infoHeader.Height, inverted);
@@ -349,6 +360,75 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             }
         }
 
+        /// <summary>
+        /// Looks up color values and builds the image from de-compressed RLE24.
+        /// </summary>
+        /// <typeparam name="TPixel">The pixel format.</typeparam>
+        /// <param name="pixels">The <see cref="Buffer2D{TPixel}"/> to assign the palette to.</param>
+        /// <param name="width">The width of the bitmap.</param>
+        /// <param name="height">The height of the bitmap.</param>
+        /// <param name="inverted">Whether the bitmap is inverted.</param>
+        private void ReadRle24<TPixel>(Buffer2D<TPixel> pixels, int width, int height, bool inverted)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            TPixel color = default;
+            using (IMemoryOwner<byte> buffer = this.memoryAllocator.Allocate<byte>(width * height * 3, 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;
+                Span<byte> bufferSpan = buffer.GetSpan();
+                this.UncompressRle24(width, bufferSpan, undefinedPixels.GetSpan(), rowsWithUndefinedPixelsSpan);
+                for (int y = 0; y < height; y++)
+                {
+                    int newY = Invert(y, height, inverted);
+                    Span<TPixel> pixelRow = pixels.GetRowSpan(newY);
+                    bool rowHasUndefinedPixels = rowsWithUndefinedPixelsSpan[y];
+                    if (rowHasUndefinedPixels)
+                    {
+                        // Slow path with undefined pixels.
+                        for (int x = 0; x < width; x++)
+                        {
+                            int idx = (y * width * 3) + (x * 3);
+                            if (undefinedPixels[x, y])
+                            {
+                                switch (this.options.RleSkippedPixelHandling)
+                                {
+                                    case RleSkippedPixelHandling.FirstColorOfPalette:
+                                        color.FromBgr24(Unsafe.As<byte, Bgr24>(ref bufferSpan[idx]));
+                                        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 bufferSpan[idx]));
+                            }
+
+                            pixelRow[x] = color;
+                        }
+                    }
+                    else
+                    {
+                        // Fast path without any undefined pixels.
+                        for (int x = 0; x < width; x++)
+                        {
+                            int idx = (y * width * 3) + (x * 3);
+                            color.FromBgr24(Unsafe.As<byte, Bgr24>(ref bufferSpan[idx]));
+                            pixelRow[x] = color;
+                        }
+                    }
+                }
+            }
+        }
+
         /// <summary>
         /// Produce uncompressed bitmap data from a RLE4 stream.
         /// </summary>
@@ -545,7 +625,95 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         }
 
         /// <summary>
-        /// Keeps track of skipped / undefined pixels, when EndOfBitmap command occurs.
+        /// Produce uncompressed bitmap data from a RLE24 stream.
+        /// </summary>
+        /// <remarks>
+        /// <br/>If first byte is 0, the second byte may have special meaning.
+        /// <br/>Otherwise, the first byte is the length of the run and following three bytes are the color for the run.
+        /// </remarks>
+        /// <param name="w">The width of the bitmap.</param>
+        /// <param name="buffer">Buffer for uncompressed data.</param>
+        /// <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 UncompressRle24(int w, Span<byte> buffer, Span<bool> undefinedPixels, Span<bool> rowsWithUndefinedPixels)
+        {
+#if NETCOREAPP2_1
+            Span<byte> cmd = stackalloc byte[2];
+#else
+            byte[] cmd = new byte[2];
+#endif
+            int uncompressedPixels = 0;
+
+            while (uncompressedPixels < buffer.Length)
+            {
+                if (this.stream.Read(cmd, 0, cmd.Length) != 2)
+                {
+                    BmpThrowHelper.ThrowImageFormatException("Failed to read 2 bytes from stream while uncompressing RLE24 bitmap.");
+                }
+
+                if (cmd[0] == RleCommand)
+                {
+                    switch (cmd[1])
+                    {
+                        case RleEndOfBitmap:
+                            int skipEoB = (buffer.Length - (uncompressedPixels * 3)) / 3;
+                            RleSkipEndOfBitmap(uncompressedPixels, w, skipEoB, undefinedPixels, rowsWithUndefinedPixels);
+
+                            return;
+
+                        case RleEndOfLine:
+                            uncompressedPixels += RleSkipEndOfLine(uncompressedPixels, w, undefinedPixels, rowsWithUndefinedPixels);
+
+                            break;
+
+                        case RleDelta:
+                            int dx = this.stream.ReadByte();
+                            int dy = this.stream.ReadByte();
+                            uncompressedPixels += RleSkipDelta(uncompressedPixels, 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.
+                            int length = cmd[1];
+
+                            byte[] run = new byte[length * 3];
+
+                            this.stream.Read(run, 0, run.Length);
+
+                            run.AsSpan().CopyTo(buffer.Slice(start: uncompressedPixels * 3));
+
+                            uncompressedPixels += length;
+
+                            // Absolute mode data is aligned to two-byte word-boundary.
+                            int padding = run.Length & 1;
+
+                            this.stream.Skip(padding);
+
+                            break;
+                    }
+                }
+                else
+                {
+                    int max = uncompressedPixels + cmd[0];
+                    byte blueIdx = cmd[1];
+                    byte greenIdx = (byte)this.stream.ReadByte();
+                    byte redIdx = (byte)this.stream.ReadByte();
+
+                    int bufferIdx = uncompressedPixels * 3;
+                    for (; uncompressedPixels < max; uncompressedPixels++)
+                    {
+                        buffer[bufferIdx++] = blueIdx;
+                        buffer[bufferIdx++] = greenIdx;
+                        buffer[bufferIdx++] = redIdx;
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Keeps track of skipped / undefined pixels, when the EndOfBitmap command occurs.
         /// </summary>
         /// <param name="count">The already processed pixel count.</param>
         /// <param name="w">The width of the image.</param>
@@ -576,7 +744,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// <summary>
         /// Keeps track of undefined / skipped pixels, when the EndOfLine command occurs.
         /// </summary>
-        /// <param name="count">The already processed pixel count.</param>
+        /// <param name="count">The already uncompressed 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>
@@ -1167,8 +1335,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         /// <summary>
         /// Reads the <see cref="BmpFileHeader"/> from the stream.
         /// </summary>
-        /// <returns>The color map size in bytes, if it could be determined by the file header. Otherwise -1.</returns>
-        private int ReadFileHeader()
+        private void ReadFileHeader()
         {
 #if NETCOREAPP2_1
             Span<byte> buffer = stackalloc byte[BmpFileHeader.Size];
@@ -1181,11 +1348,14 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             switch (fileTypeMarker)
             {
                 case BmpConstants.TypeMarkers.Bitmap:
+                    this.fileMarkerType = BmpFileMarkerType.Bitmap;
                     this.fileHeader = BmpFileHeader.Parse(buffer);
                     break;
                 case BmpConstants.TypeMarkers.BitmapArray:
-                    // The Array file header is followed by the bitmap file header of the first image.
-                    var arrayHeader = BmpArrayFileHeader.Parse(buffer);
+                    this.fileMarkerType = BmpFileMarkerType.BitmapArray;
+
+                    // Because we only decode the first bitmap in the array, the array header will be ignored.
+                    // The bitmap file header of the first image follows the array header.
                     this.stream.Read(buffer, 0, BmpFileHeader.Size);
                     this.fileHeader = BmpFileHeader.Parse(buffer);
                     if (this.fileHeader.Type != BmpConstants.TypeMarkers.Bitmap)
@@ -1193,20 +1363,12 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                         BmpThrowHelper.ThrowNotSupportedException($"Unsupported bitmap file inside a BitmapArray file. File header bitmap type marker '{this.fileHeader.Type}'.");
                     }
 
-                    if (arrayHeader.OffsetToNext != 0)
-                    {
-                        int colorMapSizeBytes = arrayHeader.OffsetToNext - arrayHeader.Size;
-                        return colorMapSizeBytes;
-                    }
-
                     break;
 
                 default:
                     BmpThrowHelper.ThrowNotSupportedException($"ImageSharp does not support this BMP file. File header bitmap type marker '{fileTypeMarker}'.");
                     break;
             }
-
-            return -1;
         }
 
         /// <summary>
@@ -1218,7 +1380,7 @@ namespace SixLabors.ImageSharp.Formats.Bmp
         {
             this.stream = stream;
 
-            int colorMapSizeBytes = this.ReadFileHeader();
+            this.ReadFileHeader();
             this.ReadInfoHeader();
 
             // see http://www.drdobbs.com/architecture-and-design/the-bmp-file-format-part-1/184409517
@@ -1234,23 +1396,34 @@ namespace SixLabors.ImageSharp.Formats.Bmp
             }
 
             int bytesPerColorMapEntry = 4;
-
+            int colorMapSizeBytes = -1;
             if (this.infoHeader.ClrUsed == 0)
             {
                 if (this.infoHeader.BitsPerPixel == 1
                     || this.infoHeader.BitsPerPixel == 4
                     || this.infoHeader.BitsPerPixel == 8)
                 {
-                    if (colorMapSizeBytes == -1)
+                    switch (this.fileMarkerType)
                     {
-                        colorMapSizeBytes = this.fileHeader.Offset - BmpFileHeader.Size - this.infoHeader.HeaderSize;
-                    }
+                        case BmpFileMarkerType.Bitmap:
+                            colorMapSizeBytes = this.fileHeader.Offset - BmpFileHeader.Size - this.infoHeader.HeaderSize;
+                            int colorCountForBitDepth = ImageMaths.GetColorCountForBitDepth(this.infoHeader.BitsPerPixel);
+                            bytesPerColorMapEntry = colorMapSizeBytes / colorCountForBitDepth;
 
-                    int colorCountForBitDepth = ImageMaths.GetColorCountForBitDepth(this.infoHeader.BitsPerPixel);
-                    bytesPerColorMapEntry = colorMapSizeBytes / colorCountForBitDepth;
+                            // Edge case for less-than-full-sized palette: bytesPerColorMapEntry should be at least 3.
+                            bytesPerColorMapEntry = Math.Max(bytesPerColorMapEntry, 3);
 
-                    // Edge case for less-than-full-sized palette: bytesPerColorMapEntry should be at least 3.
-                    bytesPerColorMapEntry = Math.Max(bytesPerColorMapEntry, 3);
+                            break;
+                        case BmpFileMarkerType.BitmapArray:
+                        case BmpFileMarkerType.ColorIcon:
+                        case BmpFileMarkerType.ColorPointer:
+                        case BmpFileMarkerType.Icon:
+                        case BmpFileMarkerType.Pointer:
+                            // OS/2 bitmaps always have 3 colors per color palette entry.
+                            bytesPerColorMapEntry = 3;
+                            colorMapSizeBytes = ImageMaths.GetColorCountForBitDepth(this.infoHeader.BitsPerPixel) * bytesPerColorMapEntry;
+                            break;
+                    }
                 }
             }
             else

src/ImageSharp/Formats/Bmp/BmpFileMarkerType.cs

@@ -0,0 +1,41 @@
+// Copyright (c) Six Labors and contributors.
+// Licensed under the Apache License, Version 2.0.
+
+namespace SixLabors.ImageSharp.Formats.Bmp
+{
+    /// <summary>
+    /// Indicates which bitmap file marker was read.
+    /// </summary>
+    public enum BmpFileMarkerType
+    {
+        /// <summary>
+        /// Single-image BMP file that may have been created under Windows or OS/2.
+        /// </summary>
+        Bitmap,
+
+        /// <summary>
+        /// OS/2 Bitmap Array.
+        /// </summary>
+        BitmapArray,
+
+        /// <summary>
+        /// OS/2 Color Icon.
+        /// </summary>
+        ColorIcon,
+
+        /// <summary>
+        /// OS/2 Color Pointer.
+        /// </summary>
+        ColorPointer,
+
+        /// <summary>
+        /// OS/2 Icon.
+        /// </summary>
+        Icon,
+
+        /// <summary>
+        /// OS/2 Pointer.
+        /// </summary>
+        Pointer
+    }
+}

src/ImageSharp/Formats/Bmp/BmpInfoHeader.cs

@@ -388,8 +388,12 @@ namespace SixLabors.ImageSharp.Formats.Bmp
                 case 2:
                     infoHeader.Compression = BmpCompression.RLE4;
                     break;
+                case 4:
+                    infoHeader.Compression = BmpCompression.RLE24;
+                    break;
                 default:
-                    BmpThrowHelper.ThrowImageFormatException($"Compression type is not supported. ImageSharp only supports uncompressed, RLE4 and RLE8.");
+                    // Compression type 3 (1DHuffman) is not supported.
+                    BmpThrowHelper.ThrowImageFormatException("Compression type is not supported. ImageSharp only supports uncompressed, RLE4, RLE8 and RLE24.");
                     break;
             }
 

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

@@ -228,6 +228,22 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
             }
         }
 
+        [Theory]
+        [WithFile(RLE24, PixelTypes.Rgba32)]
+        [WithFile(RLE24Cut, PixelTypes.Rgba32)]
+        [WithFile(RLE24Delta, PixelTypes.Rgba32)]
+        public void BmpDecoder_CanDecode_RunLengthEncoded_24Bit<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage(new BmpDecoder() { RleSkippedPixelHandling = RleSkippedPixelHandling.Black }))
+            {
+                image.DebugSave(provider);
+
+                // TODO: Neither System.Drawing nor MagickReferenceDecoder decode this file.
+                // image.CompareToOriginal(provider);
+            }
+        }
+
         [Theory]
         [WithFile(RgbaAlphaBitfields, PixelTypes.Rgba32)]
         public void BmpDecoder_CanDecodeAlphaBitfields<TPixel>(TestImageProvider<TPixel> provider)
@@ -521,6 +537,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Bmp
         }
 
         [Theory]
+        [WithFile(Os2BitmapArray, PixelTypes.Rgba32)]
         [WithFile(Os2BitmapArray9s, PixelTypes.Rgba32)]
         [WithFile(Os2BitmapArrayDiamond, PixelTypes.Rgba32)]
         [WithFile(Os2BitmapArraySkater, PixelTypes.Rgba32)]

tests/ImageSharp.Tests/TestImages.cs

@@ -231,6 +231,9 @@ namespace SixLabors.ImageSharp.Tests
             public const string NegHeight = "Bmp/neg_height.bmp";
             public const string CoreHeader = "Bmp/BitmapCoreHeaderQR.bmp";
             public const string V5Header = "Bmp/BITMAPV5HEADER.bmp";
+            public const string RLE24 = "Bmp/rgb24rle24.bmp";
+            public const string RLE24Cut = "Bmp/rle24rlecut.bmp";
+            public const string RLE24Delta = "Bmp/rle24rlecut.bmp";
             public const string RLE8 = "Bmp/RunLengthEncoded.bmp";
             public const string RLE8Cut = "Bmp/pal8rlecut.bmp";
             public const string RLE8Delta = "Bmp/pal8rletrns.bmp";
@@ -262,6 +265,7 @@ namespace SixLabors.ImageSharp.Tests
             public const string Bit8Palette4 = "Bmp/pal8-0.bmp";
             public const string Os2v2Short = "Bmp/pal8os2v2-16.bmp";
             public const string Os2v2 = "Bmp/pal8os2v2.bmp";
+            public const string Os2BitmapArray = "Bmp/ba-bm.bmp";
             public const string Os2BitmapArray9s = "Bmp/9S.BMP";
             public const string Os2BitmapArrayDiamond = "Bmp/DIAMOND.BMP";
             public const string Os2BitmapArrayMarble = "Bmp/GMARBLE.BMP";

tests/Images/Input/Bmp/ba-bm.bmp

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

tests/Images/Input/Bmp/rgb24rle24.bmp

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:351d358824671a79dc63147a78fc555d46cbee357661674e80c898e133e0b5c5
+size 21432

tests/Images/Input/Bmp/rle24rlecut.bmp

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:15b84ee3e41934653939197267758e6719da93d017200a7b9e61820b368af04c
+size 16748

tests/Images/Input/Bmp/rle24rletrns.bmp

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:37caf0742ebc94e4ff73b822052091db543559fa96352b83a3e5f5545999c5f7
+size 20036