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

Bitmap encoder writes V3 header as default #889

Merged
5 changes: 5 additions & 0 deletions src/ImageSharp/Formats/Bmp/BmpEncoder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ public sealed class BmpEncoder : IImageEncoder, IBmpEncoderOptions
/// </summary>
public BmpBitsPerPixel? BitsPerPixel { get; set; }

/// <summary>
/// Gets or sets a value indicating whether the encoder should support transparency.
/// </summary>
public bool SupportTransparency { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't BmpBitsPerPixel.Pixel32 implicitly mean this?


/// <inheritdoc/>
public void Encode<TPixel>(Image<TPixel> image, Stream stream)
where TPixel : struct, IPixel<TPixel>
Expand Down
45 changes: 29 additions & 16 deletions src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,22 @@ internal sealed class BmpEncoderCore
private int padding;

/// <summary>
/// The mask for the alpha channel of the color for a 32 bit rgba bitmaps.
/// The mask for the alpha channel of the color for 32 bit rgba bitmaps.
/// </summary>
private const int Rgba32AlphaMask = 0xFF << 24;

/// <summary>
/// The mask for the red part of the color for a 32 bit rgba bitmaps.
/// The mask for the red part of the color for 32 bit rgba bitmaps.
/// </summary>
private const int Rgba32RedMask = 0xFF << 16;

/// <summary>
/// The mask for the green part of the color for a 32 bit rgba bitmaps.
/// The mask for the green part of the color for 32 bit rgba bitmaps.
/// </summary>
private const int Rgba32GreenMask = 0xFF << 8;

/// <summary>
/// The mask for the blue part of the color for a 32 bit rgba bitmaps.
/// The mask for the blue part of the color for 32 bit rgba bitmaps.
/// </summary>
private const int Rgba32BlueMask = 0xFF;

Expand All @@ -49,15 +49,23 @@ internal sealed class BmpEncoderCore

private BmpBitsPerPixel? bitsPerPixel;

/// <summary>
/// A bitmap v4 header will only be written, if the user explicitly wants support for transparency.
/// In this case the compression type BITFIELDS will be used.
/// Otherwise a bitmap v3 header will be written, which is supported by almost all decoders.
/// </summary>
private readonly bool writeV4Header;

/// <summary>
/// Initializes a new instance of the <see cref="BmpEncoderCore"/> class.
/// </summary>
/// <param name="options">The encoder options</param>
/// <param name="memoryAllocator">The memory manager</param>
/// <param name="options">The encoder options.</param>
/// <param name="memoryAllocator">The memory manager.</param>
public BmpEncoderCore(IBmpEncoderOptions options, MemoryAllocator memoryAllocator)
{
this.memoryAllocator = memoryAllocator;
this.bitsPerPixel = options.BitsPerPixel;
this.writeV4Header = options.SupportTransparency;
}

/// <summary>
Expand Down Expand Up @@ -112,7 +120,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
}
}

int infoHeaderSize = BmpInfoHeader.SizeV4;
int infoHeaderSize = this.writeV4Header ? BmpInfoHeader.SizeV4 : BmpInfoHeader.SizeV3;
var infoHeader = new BmpInfoHeader(
headerSize: infoHeaderSize,
height: image.Height,
Expand All @@ -123,17 +131,15 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
clrUsed: 0,
clrImportant: 0,
xPelsPerMeter: hResolution,
yPelsPerMeter: vResolution)
{
RedMask = Rgba32RedMask,
GreenMask = Rgba32GreenMask,
BlueMask = Rgba32BlueMask,
Compression = BmpCompression.BitFields
};
yPelsPerMeter: vResolution);

if (this.bitsPerPixel == BmpBitsPerPixel.Pixel32)
if (this.writeV4Header && this.bitsPerPixel == BmpBitsPerPixel.Pixel32)
{
infoHeader.AlphaMask = Rgba32AlphaMask;
infoHeader.RedMask = Rgba32RedMask;
infoHeader.GreenMask = Rgba32GreenMask;
infoHeader.BlueMask = Rgba32BlueMask;
infoHeader.Compression = BmpCompression.BitFields;
}

var fileHeader = new BmpFileHeader(
Expand All @@ -151,7 +157,14 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)

stream.Write(buffer, 0, BmpFileHeader.Size);

infoHeader.WriteV4Header(buffer);
if (this.writeV4Header)
{
infoHeader.WriteV4Header(buffer);
}
else
{
infoHeader.WriteV3Header(buffer);
}

stream.Write(buffer, 0, infoHeaderSize);

Expand Down
7 changes: 6 additions & 1 deletion src/ImageSharp/Formats/Bmp/IBmpEncoderOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ namespace SixLabors.ImageSharp.Formats.Bmp
/// <summary>
/// Configuration options for use during bmp encoding
/// </summary>
/// <remarks>The encoder can currently only write 24-bit rgb images to streams.</remarks>
/// <remarks>The encoder can currently only write 24-bit and 32-bit rgb images to streams.</remarks>
internal interface IBmpEncoderOptions
{
/// <summary>
/// Gets the number of bits per pixel.
/// </summary>
BmpBitsPerPixel? BitsPerPixel { get; }

/// <summary>
/// Gets a value indicating whether the encoder should support transparency.
/// </summary>
bool SupportTransparency { get; }
}
}
38 changes: 34 additions & 4 deletions tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace SixLabors.ImageSharp.Tests
{
using static TestImages.Bmp;

public class BmpEncoderTests : FileTestBase
{
public static readonly TheoryData<BmpBitsPerPixel> BitsPerPixel =
Expand Down Expand Up @@ -102,15 +104,43 @@ public void Encode_IsNotBoundToSinglePixelType<TPixel>(TestImageProvider<TPixel>
public void Encode_WorksWithDifferentSizes<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel);

private static void TestBmpEncoderCore<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
[Theory]
[WithFile(Bit32Rgb, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[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)]
Copy link
Member

Choose a reason for hiding this comment

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

👍 these tests are great! 💯

public void Encode_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);

[Theory]
[WithFile(Bit32Rgb, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(Bit32Rgba, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(WinBmpv4, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(WinBmpv5, PixelTypes.Rgba32 | PixelTypes.Rgb24, BmpBitsPerPixel.Pixel32)]
[WithFile(WinBmpv3, PixelTypes.Rgb24, BmpBitsPerPixel.Pixel24)]
public void Encode_WithV4Header_Works<TPixel>(TestImageProvider<TPixel> provider, BmpBitsPerPixel bitsPerPixel)
where TPixel : struct, IPixel<TPixel> => TestBmpEncoderCore(provider, bitsPerPixel, supportTransparency: true);

[Theory]
[WithFile(TestImages.Png.GrayAlpha2BitInterlaced, 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)
where TPixel : struct, IPixel<TPixel>
{
using (Image<TPixel> image = provider.GetImage())
{
// there is no alpha in bmp!
image.Mutate(c => c.MakeOpaque());
// There is no alpha in bmp with 24 bits per pixels, so the reference image will be made opaque.
if (bitsPerPixel == BmpBitsPerPixel.Pixel24)
{
image.Mutate(c => c.MakeOpaque());
}

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

// Does DebugSave & load reference CompareToReferenceInput():
image.VerifyEncoder(provider, "bmp", bitsPerPixel, encoder);
Expand Down