bmp: ignore alpha in Decode when the alpha channel is not supported
When the alpha channel is not supported, the alpha component in the
original image should be ignored in the decoded image data (that is,
set to 0xFF in the decoded data).
testdata/yellow_rose-small.png has been modified to pass the test.
No new test is added because testdata/yellow_rose-small.bmp has
covered this case.
Fixes golang/go#54368.
Change-Id: Ib46947ec0448340e4313773f3c8534f4dc26bc36
GitHub-Last-Rev: 1eeb39647c0873ae47aab1e3934394e5180de59a
GitHub-Pull-Request: golang/image#10
Reviewed-on: https://go-review.googlesource.com/c/image/+/425418
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
diff --git a/bmp/reader.go b/bmp/reader.go
index 52e2520..e165c2e 100644
--- a/bmp/reader.go
+++ b/bmp/reader.go
@@ -85,7 +85,7 @@
// decodeNRGBA reads a 32 bit-per-pixel BMP image from r.
// If topDown is false, the image rows will be read bottom-up.
-func decodeNRGBA(r io.Reader, c image.Config, topDown bool) (image.Image, error) {
+func decodeNRGBA(r io.Reader, c image.Config, topDown, allowAlpha bool) (image.Image, error) {
rgba := image.NewNRGBA(image.Rect(0, 0, c.Width, c.Height))
if c.Width == 0 || c.Height == 0 {
return rgba, nil
@@ -102,6 +102,9 @@
for i := 0; i < len(p); i += 4 {
// BMP images are stored in BGRA order rather than RGBA order.
p[i+0], p[i+2] = p[i+2], p[i+0]
+ if !allowAlpha {
+ p[i+3] = 0xFF
+ }
}
}
return rgba, nil
@@ -110,7 +113,7 @@
// Decode reads a BMP image from r and returns it as an image.Image.
// Limitation: The file must be 8, 24 or 32 bits per pixel.
func Decode(r io.Reader) (image.Image, error) {
- c, bpp, topDown, err := decodeConfig(r)
+ c, bpp, topDown, allowAlpha, err := decodeConfig(r)
if err != nil {
return nil, err
}
@@ -120,7 +123,7 @@
case 24:
return decodeRGB(r, c, topDown)
case 32:
- return decodeNRGBA(r, c, topDown)
+ return decodeNRGBA(r, c, topDown, allowAlpha)
}
panic("unreachable")
}
@@ -129,13 +132,15 @@
// decoding the entire image.
// Limitation: The file must be 8, 24 or 32 bits per pixel.
func DecodeConfig(r io.Reader) (image.Config, error) {
- config, _, _, err := decodeConfig(r)
+ config, _, _, _, err := decodeConfig(r)
return config, err
}
-func decodeConfig(r io.Reader) (config image.Config, bitsPerPixel int, topDown bool, err error) {
- // We only support those BMP images that are a BITMAPFILEHEADER
- // immediately followed by a BITMAPINFOHEADER.
+func decodeConfig(r io.Reader) (config image.Config, bitsPerPixel int, topDown bool, allowAlpha bool, err error) {
+ // We only support those BMP images with one of the following DIB headers:
+ // - BITMAPINFOHEADER (40 bytes)
+ // - BITMAPV4HEADER (108 bytes)
+ // - BITMAPV5HEADER (124 bytes)
const (
fileHeaderLen = 14
infoHeaderLen = 40
@@ -147,21 +152,21 @@
if err == io.EOF {
err = io.ErrUnexpectedEOF
}
- return image.Config{}, 0, false, err
+ return image.Config{}, 0, false, false, err
}
if string(b[:2]) != "BM" {
- return image.Config{}, 0, false, errors.New("bmp: invalid format")
+ return image.Config{}, 0, false, false, errors.New("bmp: invalid format")
}
offset := readUint32(b[10:14])
infoLen := readUint32(b[14:18])
if infoLen != infoHeaderLen && infoLen != v4InfoHeaderLen && infoLen != v5InfoHeaderLen {
- return image.Config{}, 0, false, ErrUnsupported
+ return image.Config{}, 0, false, false, ErrUnsupported
}
if _, err := io.ReadFull(r, b[fileHeaderLen+4:fileHeaderLen+infoLen]); err != nil {
if err == io.EOF {
err = io.ErrUnexpectedEOF
}
- return image.Config{}, 0, false, err
+ return image.Config{}, 0, false, false, err
}
width := int(int32(readUint32(b[18:22])))
height := int(int32(readUint32(b[22:26])))
@@ -169,12 +174,12 @@
height, topDown = -height, true
}
if width < 0 || height < 0 {
- return image.Config{}, 0, false, ErrUnsupported
+ return image.Config{}, 0, false, false, ErrUnsupported
}
// We only support 1 plane and 8, 24 or 32 bits per pixel and no
// compression.
planes, bpp, compression := readUint16(b[26:28]), readUint16(b[28:30]), readUint32(b[30:34])
- // if compression is set to BITFIELDS, but the bitmask is set to the default bitmask
+ // if compression is set to BI_BITFIELDS, but the bitmask is set to the default bitmask
// that would be used if compression was set to 0, we can continue as if compression was 0
if compression == 3 && infoLen > infoHeaderLen &&
readUint32(b[54:58]) == 0xff0000 && readUint32(b[58:62]) == 0xff00 &&
@@ -182,16 +187,16 @@
compression = 0
}
if planes != 1 || compression != 0 {
- return image.Config{}, 0, false, ErrUnsupported
+ return image.Config{}, 0, false, false, ErrUnsupported
}
switch bpp {
case 8:
if offset != fileHeaderLen+infoLen+256*4 {
- return image.Config{}, 0, false, ErrUnsupported
+ return image.Config{}, 0, false, false, ErrUnsupported
}
_, err = io.ReadFull(r, b[:256*4])
if err != nil {
- return image.Config{}, 0, false, err
+ return image.Config{}, 0, false, false, err
}
pcm := make(color.Palette, 256)
for i := range pcm {
@@ -199,19 +204,40 @@
// Every 4th byte is padding.
pcm[i] = color.RGBA{b[4*i+2], b[4*i+1], b[4*i+0], 0xFF}
}
- return image.Config{ColorModel: pcm, Width: width, Height: height}, 8, topDown, nil
+ return image.Config{ColorModel: pcm, Width: width, Height: height}, 8, topDown, false, nil
case 24:
if offset != fileHeaderLen+infoLen {
- return image.Config{}, 0, false, ErrUnsupported
+ return image.Config{}, 0, false, false, ErrUnsupported
}
- return image.Config{ColorModel: color.RGBAModel, Width: width, Height: height}, 24, topDown, nil
+ return image.Config{ColorModel: color.RGBAModel, Width: width, Height: height}, 24, topDown, false, nil
case 32:
if offset != fileHeaderLen+infoLen {
- return image.Config{}, 0, false, ErrUnsupported
+ return image.Config{}, 0, false, false, ErrUnsupported
}
- return image.Config{ColorModel: color.RGBAModel, Width: width, Height: height}, 32, topDown, nil
+ // 32 bits per pixel is possibly RGBX (X is padding) or RGBA (A is
+ // alpha transparency). However, for BMP images, "Alpha is a
+ // poorly-documented and inconsistently-used feature" says
+ // https://source.chromium.org/chromium/chromium/src/+/bc0a792d7ebc587190d1a62ccddba10abeea274b:third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc;l=621
+ //
+ // That goes on to say "BITMAPV3HEADER+ have an alpha bitmask in the
+ // info header... so we respect it at all times... [For earlier
+ // (smaller) headers we] ignore alpha in Windows V3 BMPs except inside
+ // ICO files".
+ //
+ // "Ignore" means to always set alpha to 0xFF (fully opaque):
+ // https://source.chromium.org/chromium/chromium/src/+/bc0a792d7ebc587190d1a62ccddba10abeea274b:third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.h;l=272
+ //
+ // Confusingly, "Windows V3" does not correspond to BITMAPV3HEADER, but
+ // instead corresponds to the earlier (smaller) BITMAPINFOHEADER:
+ // https://source.chromium.org/chromium/chromium/src/+/bc0a792d7ebc587190d1a62ccddba10abeea274b:third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc;l=258
+ //
+ // This Go package does not support ICO files and the (infoLen >
+ // infoHeaderLen) condition distinguishes BITMAPINFOHEADER (40 bytes)
+ // vs later (larger) headers.
+ allowAlpha = infoLen > infoHeaderLen
+ return image.Config{ColorModel: color.RGBAModel, Width: width, Height: height}, 32, topDown, allowAlpha, nil
}
- return image.Config{}, 0, false, ErrUnsupported
+ return image.Config{}, 0, false, false, ErrUnsupported
}
func init() {
diff --git a/testdata/yellow_rose-small.png b/testdata/yellow_rose-small.png
index 772c239..57ce9b8 100644
--- a/testdata/yellow_rose-small.png
+++ b/testdata/yellow_rose-small.png
Binary files differ
