Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for encoding 8-bit bitmaps #906

Merged
merged 11 commits into from
May 12, 2019
7 changes: 6 additions & 1 deletion src/ImageSharp/Formats/Bmp/BmpBitsPerPixel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,15 @@
namespace SixLabors.ImageSharp.Formats.Bmp
{
/// <summary>
/// Enumerates the available bits per pixel for bitmap.
/// Enumerates the available bits per pixel the bitmap encoder supports.
/// </summary>
public enum BmpBitsPerPixel : short
{
/// <summary>
/// 8 bits per pixel. Each pixel consists of 1 byte.
/// </summary>
Pixel8 = 8,

/// <summary>
/// 16 bits per pixel. Each pixel consists of 2 bytes.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,8 @@ private void ReadInfoHeader()
this.bmpMetadata.InfoHeaderType = infoHeaderType;

// We can only encode at these bit rates so far.
if (bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel16)
if (bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel8)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there other options?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, 4 bit and 1 bit

|| bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel16)
|| bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel24)
|| bitsPerPixel.Equals((short)BmpBitsPerPixel.Pixel32))
{
Expand Down
1 change: 0 additions & 1 deletion src/ImageSharp/Formats/Bmp/BmpEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace SixLabors.ImageSharp.Formats.Bmp
/// <summary>
/// Image encoder for writing an image to a stream as a Windows bitmap.
/// </summary>
/// <remarks>The encoder can currently only write 24-bit rgb images to streams.</remarks>
public sealed class BmpEncoder : IImageEncoder, IBmpEncoderOptions
{
/// <summary>
Expand Down
60 changes: 59 additions & 1 deletion src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.Metadata;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Processing.Processors.Quantization;
using SixLabors.Memory;

namespace SixLabors.ImageSharp.Formats.Bmp
Expand Down Expand Up @@ -43,6 +44,11 @@ internal sealed class BmpEncoderCore
/// </summary>
private const int Rgba32BlueMask = 0xFF;

/// <summary>
/// The color palette for an 8 bit image will have 256 entry's with 4 bytes for each entry.
/// </summary>
private const int ColorPaletteSize8Bit = 1024;

private readonly MemoryAllocator memoryAllocator;

private Configuration configuration;
Expand Down Expand Up @@ -142,11 +148,13 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
infoHeader.Compression = BmpCompression.BitFields;
}

int colorPaletteSize = this.bitsPerPixel == BmpBitsPerPixel.Pixel8 ? ColorPaletteSize8Bit : 0;

var fileHeader = new BmpFileHeader(
type: BmpConstants.TypeMarkers.Bitmap,
fileSize: BmpFileHeader.Size + infoHeaderSize + infoHeader.ImageSize,
reserved: 0,
offset: BmpFileHeader.Size + infoHeaderSize);
offset: BmpFileHeader.Size + infoHeaderSize + colorPaletteSize);

#if NETCOREAPP2_1
Span<byte> buffer = stackalloc byte[infoHeaderSize];
Expand Down Expand Up @@ -198,6 +206,10 @@ private void WriteImage<TPixel>(Stream stream, ImageFrame<TPixel> image)
case BmpBitsPerPixel.Pixel16:
this.Write16Bit(stream, pixels);
break;

case BmpBitsPerPixel.Pixel8:
this.Write8Bit(stream, image);
break;
}
}

Expand Down Expand Up @@ -276,5 +288,51 @@ private void Write16Bit<TPixel>(Stream stream, Buffer2D<TPixel> pixels)
}
}
}

/// <summary>
/// Writes an 8 Bit image with a color palette. The color palette has 256 entry's with 4 bytes for each entry.
/// </summary>
/// <typeparam name="TPixel">The type of the pixel.</typeparam>
/// <param name="stream">The <see cref="Stream"/> to write to.</param>
/// <param name="image"> The <see cref="ImageFrame{TPixel}"/> containing pixel data.</param>
private void Write8Bit<TPixel>(Stream stream, ImageFrame<TPixel> image)
where TPixel : struct, IPixel<TPixel>
{
#if NETCOREAPP2_1
Span<byte> colorPalette = stackalloc byte[ColorPaletteSize8Bit];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1024 byte is too much for stackallock, we can remove this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the limit? I can never find an explicit instruction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've read something in Joe Duffy's original material suggesting 128 bytes, based on their benchmarks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok. I use 257 bytes in the HuffmanTable but I'm gonna leave that as-is.

#else
byte[] colorPalette = new byte[ColorPaletteSize8Bit];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to use MemoryAllocator.AllocateManagedByteBuffer() for this size.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't actually need the #ifdef anymore. It's supported in latest c# versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean replacing GC array allocation.

#endif

var quantizer = new OctreeQuantizer(dither: true, maxColors: 256);
QuantizedFrame<TPixel> quantized = quantizer.CreateFrameQuantizer<TPixel>(this.configuration).QuantizeFrame(image);
Copy link
Member

@antonfirsov antonfirsov May 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FrameQuantizer<T> and QuantizedFrame<T> are IDisposable!

Copy link
Member

@antonfirsov antonfirsov May 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also:
IQuantizer should be configuratble by the user as an Encoder parameter, like we do it in PngEncoder!


int idx = 0;
var color = default(Rgba32);
foreach (TPixel quantizedColor in quantized.Palette)
{
quantizedColor.ToRgba32(ref color);
Copy link
Member

@antonfirsov antonfirsov May 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be a stupid idea and lack of understanding, but:
What if we convert to Rgba32 first, and quantize in Rgba32 space?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, in Octree we're working in the Rgba32 space then converting to/from so I see where you are coming from. However, by converting before quantization we end up allocating a new buffer to house the image. That's probably a lot more costly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally forgot that it will lead to another allocation, so let's keep it for now.

(An idea is starting to formulate in my mind, but of course it would break IQuantizer API again 😄.)

colorPalette[idx] = color.B;
colorPalette[idx + 1] = color.G;
colorPalette[idx + 2] = color.R;

// Padding byte, always 0
colorPalette[idx + 3] = 0;
idx += 4;
}

stream.Write(colorPalette, 0, ColorPaletteSize8Bit);

for (int y = image.Height - 1; y >= 0; y--)
{
Span<byte> pixelSpan = quantized.GetRowSpan(y);
stream.Write(pixelSpan);

for (int i = 0; i < this.padding; i++)
{
stream.WriteByte(0);
}
}
}
}
}
1 change: 0 additions & 1 deletion src/ImageSharp/Formats/Bmp/IBmpEncoderOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ namespace SixLabors.ImageSharp.Formats.Bmp
/// <summary>
/// Configuration options for use during bmp encoding
/// </summary>
/// <remarks>The encoder can currently only write 16-bit, 24-bit and 32-bit rgb images to streams.</remarks>
internal interface IBmpEncoderOptions
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public OctreeQuantizer(int maxColors)
/// <summary>
/// Initializes a new instance of the <see cref="OctreeQuantizer"/> class.
/// </summary>
/// <param name="dither">Whether to apply dithering to the output image</param>
/// <param name="dither">Whether to apply dithering to the output image.</param>
public OctreeQuantizer(bool dither)
: this(GetDiffuser(dither), QuantizerConstants.MaxColors)
{
Expand All @@ -44,7 +44,17 @@ public OctreeQuantizer(bool dither)
/// <summary>
/// Initializes a new instance of the <see cref="OctreeQuantizer"/> class.
/// </summary>
/// <param name="diffuser">The error diffusion algorithm, if any, to apply to the output image</param>
/// <param name="maxColors">The maximum number of colors to hold in the color palette.</param>
/// <param name="dither">Whether to apply dithering to the output image.</param>
public OctreeQuantizer(bool dither, int maxColors)
: this(GetDiffuser(dither), maxColors)
{
}

/// <summary>
/// Initializes a new instance of the <see cref="OctreeQuantizer"/> class.
/// </summary>
/// <param name="diffuser">The error diffusion algorithm, if any, to apply to the output image.</param>
public OctreeQuantizer(IErrorDiffuser diffuser)
: this(diffuser, QuantizerConstants.MaxColors)
{
Expand All @@ -53,8 +63,8 @@ public OctreeQuantizer(IErrorDiffuser diffuser)
/// <summary>
/// Initializes a new instance of the <see cref="OctreeQuantizer"/> class.
/// </summary>
/// <param name="diffuser">The error diffusion algorithm, if any, to apply to the output image</param>
/// <param name="maxColors">The maximum number of colors to hold in the color palette</param>
/// <param name="diffuser">The error diffusion algorithm, if any, to apply to the output image.</param>
/// <param name="maxColors">The maximum number of colors to hold in the color palette.</param>
public OctreeQuantizer(IErrorDiffuser diffuser, int maxColors)
{
this.Diffuser = diffuser;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public QuantizedFrame(MemoryAllocator memoryAllocator, int width, int height, TP

/// <summary>
/// Gets the representation of the pixels as a <see cref="Span{T}"/> of contiguous memory
/// at row <paramref name="rowIndex"/> beginning from the the first pixel on that row.
/// at row <paramref name="rowIndex"/> beginning from the first pixel on that row.
/// </summary>
/// <param name="rowIndex">The row.</param>
/// <returns>The <see cref="Span{T}"/></returns>
Expand Down
78 changes: 68 additions & 10 deletions tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
// Licensed under the Apache License, Version 2.0.

using System.IO;

using SixLabors.ImageSharp.Formats.Bmp;
using SixLabors.ImageSharp.Metadata;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Processing;
using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison;

using Xunit;
using Xunit.Abstractions;
Expand Down Expand Up @@ -110,11 +112,7 @@ public void Encode_WorksWithDifferentSizes<TPixel>(TestImageProvider<TPixel> pro
[WithFile(Bit32Rgba, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(WinBmpv4, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(WinBmpv5, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
// WinBmpv3 is a 24 bits per pixel image
[WithFile(WinBmpv3, PixelTypes.Rgb24, BmpBitsPerPixel.Pixel24)]
[WithFile(Rgb16, PixelTypes.Bgra5551, BmpBitsPerPixel.Pixel16)]
[WithFile(Bit16, PixelTypes.Bgra5551, BmpBitsPerPixel.Pixel16)]
public void Encode_WithV3Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
public void Encode_32Bit_WithV3Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
// if supportTransparency is false, a v3 bitmap header will be written
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: false);

Expand All @@ -123,32 +121,92 @@ public void Encode_WithV3Header_Works<TPixel>(TestImageProvider<TPixel> provider
[WithFile(Bit32Rgba, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(WinBmpv4, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(WinBmpv5, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
public void Encode_32Bit_WithV4Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: true);

[Theory]
// WinBmpv3 is a 24 bits per pixel image
[WithFile(WinBmpv3, PixelTypes.Rgb24, BmpBitsPerPixel.Pixel24)]
[WithFile(F, PixelTypes.Rgb24, BmpBitsPerPixel.Pixel24)]
public void Encode_24Bit_WithV3Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: false);

[Theory]
[WithFile(WinBmpv3, PixelTypes.Rgb24, BmpBitsPerPixel.Pixel24)]
[WithFile(F, PixelTypes.Rgb24, BmpBitsPerPixel.Pixel24)]
public void Encode_24Bit_WithV4Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: true);


[Theory]
[WithFile(Rgb16, PixelTypes.Bgra5551, BmpBitsPerPixel.Pixel16)]
[WithFile(Bit16, PixelTypes.Bgra5551, BmpBitsPerPixel.Pixel16)]
public void Encode_16Bit_WithV3Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: false);

[Theory]
[WithFile(Rgb16, PixelTypes.Bgra5551, BmpBitsPerPixel.Pixel16)]
[WithFile(Bit16, PixelTypes.Bgra5551, BmpBitsPerPixel.Pixel16)]
public void Encode_WithV4Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
public void Encode_16Bit_WithV4Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: true);

[Theory]
[WithFile(WinBmpv5, PixelTypes.Rgba32, BmpBitsPerPixel.Pixel8)]
[WithFile(Bit8Palette4, PixelTypes.Rgba32, BmpBitsPerPixel.Pixel8)]
public void Encode_8Bit_WithV3Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: false);

[Theory]
[WithFile(WinBmpv5, PixelTypes.Rgba32, BmpBitsPerPixel.Pixel8)]
[WithFile(Bit8Palette4, PixelTypes.Rgba32, BmpBitsPerPixel.Pixel8)]
public void Encode_8Bit_WithV4Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: true);

[Theory]
[WithFile(Bit8Gs, PixelTypes.Gray8, BmpBitsPerPixel.Pixel8)]
public void Encode_8BitGray_WithV3Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say enough times, how much I ❤️ this kind of extensive and explanatory test coverage! This is an important enabler for us to do optimizations and refactors.
For #907 stuff I'm literally spending days to add missing tests, so I can change the code safely.

where TPixel : struct, IPixel<TPixel> =>
TestBmpEncoderCore(
provider,
bitsPerPixel,
supportTransparency: false,
ImageComparer.TolerantPercentage(0.01f));

[Theory]
[WithFile(Bit8Gs, PixelTypes.Gray8, BmpBitsPerPixel.Pixel8)]
public void Encode_8BitGray_WithV4Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> =>
TestBmpEncoderCore(
provider,
bitsPerPixel,
supportTransparency: true,
ImageComparer.TolerantPercentage(0.01f));

[Theory]
[WithFile(TestImages.Png.GrayAlpha2BitInterlaced, PixelTypes.Rgba32, BmpBitsPerPixel.Pixel32)]
[WithFile(Bit32Rgba, PixelTypes.Rgba32, BmpBitsPerPixel.Pixel32)]
public void Encode_PreservesAlpha<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: true);

private static void TestBmpEncoderCore<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel, bool supportTransparency = true)
private static void TestBmpEncoderCore<TPixel>(
TestImageProvider<TPixel> provider,
BmpBitsPerPixel bitsPerPixel,
bool supportTransparency = true,
ImageComparer customComparer = null)
where TPixel : struct, IPixel<TPixel>
{
using (Image<TPixel> image = provider.GetImage())
{
// There is no alpha in bmp with 24 bits per pixels, so the reference image will be made opaque.
if (bitsPerPixel == BmpBitsPerPixel.Pixel24)
// There is no alpha in bmp with less then 32 bits per pixels, so the reference image will be made opaque.
if (bitsPerPixel != BmpBitsPerPixel.Pixel32)
{
image.Mutate(c => c.MakeOpaque());
}

var encoder = new BmpEncoder { BitsPerPixel = bitsPerPixel, SupportTransparency = supportTransparency };

// Does DebugSave & load reference CompareToReferenceInput():
image.VerifyEncoder(provider, "bmp", bitsPerPixel, encoder);
image.VerifyEncoder(provider, "bmp", bitsPerPixel, encoder, customComparer);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/ImageSharp.Tests/TestImages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ public static class Bmp
public const string Bit1Pal1 = "Bmp/pal1p1.bmp";
public const string Bit4 = "Bmp/pal4.bmp";
public const string Bit8 = "Bmp/test8.bmp";
public const string Bit8Gs = "Bmp/pal8gs.bmp";
public const string Bit8Inverted = "Bmp/test8-inverted.bmp";
public const string Bit16 = "Bmp/test16.bmp";
public const string Bit16Inverted = "Bmp/test16-inverted.bmp";
Expand Down
2 changes: 2 additions & 0 deletions tests/ImageSharp.Tests/TestUtilities/PixelTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public enum PixelTypes

Bgra5551 = 1 << 22,

Gray8 = 1 << 23,

// TODO: Add multi-flag entries by rules defined in PackedPixelConverterHelper

// "All" is handled as a separate, individual case instead of using bitwise OR
Expand Down
Binary file added tests/Images/Input/Bmp/pal8gs.bmp
Binary file not shown.