Imagesharp: Resized 32-bit BI_RGB BMP is completely black

[juddski]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have verified that I am running the latest version of ImageSharp
• [X] I have verified if the problem exist in both DEBUG and RELEASE mode
• [X] I have searched open and closed issues to ensure it has not already been reported

Description

The file "Test-original.bmp" is loaded, resized to half width and height, and then saved as "Test-resized.bmp". The resulting image is the correct size, but every pixel is black.

"Test-original.bmp" is a 32-bit BI_RGB image where the alpha channel for each pixel is 0. According to MSDN, the alpha channel byte is not used, so I don't _think_ it should be interpreted during the resize if that's what's causing resulting image to be black.

https://msdn.microsoft.com/en-us/library/dd183376%28VS.85%29.aspx
_"The value for blue is in the least significant 8 bits, followed by 8 bits each for green and red. The high byte in each DWORD is not used"_

Steps to Reproduce

using SixLabors.ImageSharp;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Processing;
using System.IO;

namespace TestImageSharp
{
    class Program
    {
        static void Main(string[] args)
        {
            Image<Rgba32> image = Image.Load("Test-original.bmp");
            image.Mutate(img => img.Resize(image.Width / 2, image.Height / 2));
            using (FileStream output = File.OpenWrite("Test-resized.bmp"))
            {
                image.SaveAsBmp(output);
            }
        }
    }
}

Test images attached:
TestImages.zip

System Configuration

• ImageSharp version:
  -- SixLabors.ImageSharp.1.0.0-beta0005
• Other ImageSharp packages and versions:
  -- SixLabors.Core.1.0.0-beta0006
  -- SixLabors.Fonts.1.0.0-beta0007
  -- SixLabors.ImageSharp.Drawing.1.0.0-beta0005
  -- SixLabors.Shapes.1.0.0-beta0007
• Environment (Operating system, version and so on):
  -- Windows 10 Pro 64-bit x64
  -- Microsoft Visual Studio Community 2017 Preview Version 15.9.0 Preview 3.0
• .NET Framework version:
  -- 4.6.1
• Additional information:
  -- None
  

[JimBobSquarePants]

The bug then is in the bmp decoder, not the resize process. We'll need to fix that.

What's the alpha channel for then in a bmp?

[juddski]

Thanks JimBob, you're right of course, it is an issue with the decoder. I hadn't seen the symptoms until performing a resize.

Unfortunately, I don't know why Microsoft have designated the additional byte as unused; perhaps the 32-bit BI_RGB format specified as part of BITMAPINFOHEADER was more for DWORD alignment rather than to provide an alpha channel.
If the biCompression field is set to BI_BITFIELDS instead of BI_RGB, it still doesn't allow the specification of a bit mask for the alpha channel.
https://msdn.microsoft.com/en-us/library/Dd183376(v=VS.85).aspx
_"Specifies that the bitmap is not compressed and that the color table consists of three DWORD color masks that specify the red, green, and blue components, respectively, of each pixel."_

BITMAPV4HEADER with the biCompression set to BI_BITFIELDS allows the bit mask to specify which bits of each 32-bit DWORD are used for red, green, blue, and alpha.
https://docs.microsoft.com/en-us/windows/desktop/api/Wingdi/ns-wingdi-bitmapv4header

  DWORD        bV4RedMask;
  DWORD        bV4GreenMask;
  DWORD        bV4BlueMask;
  DWORD        bV4AlphaMask;

The documentation isn't consistent however:
_"If the bV4Compression member of the BITMAPV4HEADER is BI_BITFIELDS, the bmiColors member contains three DWORD color masks that specify the red, green, and blue components of each pixel. Each DWORD in the bitmap array represents a single pixel."_

In summary, I believe that the _Microsoft_ thing to do when loading a BMP with a BITMAPINFOHEADER is to always set the alpha channel for each pixel to fully opaque. Does that sound right to you?

[JimBobSquarePants]

@juddski Sorry for the slow reply here. I'm currently reading up on bitmap format and studying the mozilla implementation.

https://dxr.mozilla.org/mozilla-central/source/image/decoders/nsBMPDecoder.cpp

[JimBobSquarePants]

If we update the support listed in #320 we can fix this

[dlemstra]

The problem seems to be that are reading an alpha channel but should ignore it because BMP3 (40 bytes header) should not support the alpha channel.

I found this in the comments of the code of ImageMagick:

/*
We should ignore the alpha value in BMP3 files but there have been
reports about 32 bit files with alpha. We do a quick check to see if
the alpha channel contains a value that is not zero (default value).
If we find a non zero value we asume the program that wrote the file
wants to use the alpha channel.
*/

I think we have two options when solving this issue:

1. We add FromBgr32 to the pixel operations where we ignore the alpha channel but still read it.
2. We read the pixels with FromBgra32 and after we read each row we check if we found an alpha pixel that is not zero. And after all the rows have been read we check if we found a non zero value. When this is not the case we change all the alpha pixels to opaque.

I would opt for option two but that could give use a performance degradation. Maybe we should let the user decide between option 1 and 2 and make 1 the default?

[JimBobSquarePants]

I'd have to check the image but is it V4 or V3?

@dlemstra I'm strongly in favor of the second option.

[dlemstra]

This image is V3. Will implement option 2 then and submit a PR.

[juddski]

@JimBobSquarePants The image attached to the original post (Test-original.bmp) has the following header:
https://docs.microsoft.com/en-us/previous-versions/dd183376(v%3Dvs.85)

DWORD biSize; = 0x00000028 (40)
LONG biWidth; = 0x00000780 (1920)
LONG biHeight; = 0x00000438 (1080)
WORD biPlanes; = 0x0001 (1)
WORD biBitCount; = 0x0020 (32)
DWORD biCompression; = 0x00000000 (0 - BI_RGB)
DWORD biSizeImage; = 0x00000000 (0)
LONG biXPelsPerMeter; = 0x00000b12 (2834)
LONG biYPelsPerMeter; = 0x00000b12 (2834)
DWORD biClrUsed; = 0x00000000 (0)
DWORD biClrImportant; = 0x00000000 (0)

Followed by the pixel values,

Blu..Grn..Red..Alph..Blu..Grn...Red..Alph..etc.
0x33.0x4e.0x6c.0x00..0x2f.0x4b.0x67.0x00 etc.

I don't know if that's what you guys call a V3 or not?

[JimBobSquarePants]

@juddski Bmp headers for the different versions all have different lengths. So 40 in your case is V3.

    WIN_V2 = 12,
    WIN_V3 = 40,
    WIN_V4 = 108,
    WIN_V5 = 124,

    // OS2_V1 is omitted; it's the same as WIN_V2.
    OS2_V2_MIN = 16,    // Minimum allowed value for OS2v2.
    OS2_V2_MAX = 64,    // Maximum allowed value for OS2v2.

    WIN_ICO = WIN_V3,

[juddski]

@dlemstra @JimBobSquarePants Thanks, I see that option 2 also follows the Mozilla approach you referenced earlier so that seems likely to be the most compliant.

[dlemstra]

Not sure what Mozilla is doing. Option two is what ImageMagick does.

[juddski]

@dlemstra When https://dxr.mozilla.org/ comes back online (it appears to be suffering from an Internal Server Error at the moment), I'll quote the lump of code that I found in there.

I'm currently looking at the gimp implementation, just in case there are any other 'interesting' interpretations:
https://gitlab.gnome.org/GNOME/gimp/blob/master/plug-ins/file-bmp/bmp-load.c

The gimp code is a little tricky to follow, but I'll persist. I might have to resort to building it and debugging through it.

[juddski]

Stepped through the following source file during a BMP load of the test image in gimp:
https://gitlab.gnome.org/GNOME/gimp/blob/master/plug-ins/file-bmp/bmp-load.c

When it finds a WIN_V3 32-bit BI_RGB image, it performs the following operations:

Sets a loading mask for Red, Green, and Blue only; nothing for alpha at all:

  switch (biBitCnt)
    {
    case 32:
      masks[0].mask      = 0x00ff0000;
      masks[0].shiftin   = 16;
      masks[0].max_value = (gfloat)255.0;
      masks[1].mask      = 0x0000ff00;
      masks[1].shiftin   = 8;
      masks[1].max_value = (gfloat)255.0;
      masks[2].mask      = 0x000000ff;
      masks[2].shiftin   = 0;
      masks[2].max_value = (gfloat)255.0;
      masks[3].mask      = 0x00000000;
      masks[3].shiftin   = 0;
      masks[3].max_value = (gfloat)0.0;
      break;

When selecting the type and number of channels, it chooses image_type = GIMP_RGB_IMAGE and channels = 3:

  switch (bpp)
    {
    case 32:
    case 24:
    case 16:
      base_type = GIMP_RGB;
      if (masks[3].mask != 0)
        {
          image_type = GIMP_RGBA_IMAGE;
          channels = 4;
        }
      else
        {
          image_type = GIMP_RGB_IMAGE;
          channels = 3;
        }
      break;

When loading the pixels, it completely ignores the alpha because channels == 3:

            for (xpos= 0; xpos < width; ++xpos)
              {
                px32 = ToL(&row_buf[xpos*4]);
                *(temp++) = ((px32 & masks[0].mask) >> masks[0].shiftin) * 255.0 / masks[0].max_value + 0.5;
                *(temp++) = ((px32 & masks[1].mask) >> masks[1].shiftin) * 255.0 / masks[1].max_value + 0.5;
                *(temp++) = ((px32 & masks[2].mask) >> masks[2].shiftin) * 255.0 / masks[2].max_value + 0.5;
                if (channels > 3)
                  *(temp++) = ((px32 & masks[3].mask) >> masks[3].shiftin) * 255.0 / masks[3].max_value + 0.5;
              }

In summary, their interpretation is more aligned with the official Microsoft specification to discard the alpha channel, but at least it's not another variant.

I still think your option 2 is the best one, I just wanted to see how other applications behaved.

[JimBobSquarePants]

@dlemstra Option 2 is the way to go. See what Mozilla do, it looks like as soon as they find a non-zero value they switch to fast processing.

https://dxr.mozilla.org/mozilla-central/source/image/decoders/nsBMPDecoder.cpp#858

case 32:
      if (mH.mCompression == Compression::RGB && mIsWithinICO &&
          mH.mBpp == 32) {
        // This is a special case only used for 32bpp WinBMPv3-ICO files, which
        // could be in either 0RGB or ARGB format. We start by assuming it's
        // an 0RGB image. If we hit a non-zero alpha value, then we know it's
        // actually an ARGB image, and change tack accordingly.
        // (Note: a fully-transparent ARGB image is indistinguishable from a
        // 0RGB image, and we will render such an image as a 0RGB image, i.e.
        // opaquely. This is unlikely to be a problem in practice.)

[brianpopow]

i have just noticed that resizing is indeed important for this to happen. Without resizing the bitmap is correct, so i do not think its a bug in the decoder.

[JimBobSquarePants]

@brianpopow It's definitely a bug in the decoder.

Resizing only highlights the issue as the images are alpha premultiplied as part of that process. All the decoders I have looked at now - Chrome, Mozilla, Gimp do the check described in the code sample above to check each alpha value for V3 32bit during decoding, defaulting to fully opaque when not set.