Refactor SkBmpCodec
I started working on indicating the native encoded formats
and things got really complicated for bmp. I think starting
with this refactor may help a little, and I also think that
this is a good change by itself.
BUG=skia:4133
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1820283002
Review URL: https://codereview.chromium.org/1820283002
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index e327f79..ad6f0dd 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -286,6 +286,18 @@
switch (compression) {
case kNone_BmpCompressionMethod:
inputFormat = kStandard_BmpInputFormat;
+
+ // In addition to more standard pixel compression formats, bmp supports
+ // the use of bit masks to determine pixel components. The standard
+ // format for representing 16-bit colors is 555 (XRRRRRGGGGGBBBBB),
+ // which does not map well to any Skia color formats. For this reason,
+ // we will always enable mask mode with 16 bits per pixel.
+ if (16 == bitsPerPixel) {
+ inputMasks.red = 0x7C00;
+ inputMasks.green = 0x03E0;
+ inputMasks.blue = 0x001F;
+ inputFormat = kBitMask_BmpInputFormat;
+ }
break;
case k8BitRLE_BmpCompressionMethod:
if (bitsPerPixel != 8) {
@@ -331,6 +343,27 @@
inputMasks.red = get_int(iBuffer.get(), 36);
inputMasks.green = get_int(iBuffer.get(), 40);
inputMasks.blue = get_int(iBuffer.get(), 44);
+
+ if (kInfoV2_BmpHeaderType == headerType ||
+ (kInfoV3_BmpHeaderType == headerType && !inIco)) {
+ break;
+ }
+
+ // V3+ bmp files introduce an alpha mask and allow the creator of the image
+ // to use the alpha channels. However, many of these images leave the
+ // alpha channel blank and expect to be rendered as opaque. This is the
+ // case for almost all V3 images, so we ignore the alpha mask. For V4+
+ // images in kMask mode, we will use the alpha mask. Additionally, V3
+ // bmp-in-ico expect us to use the alpha mask.
+ //
+ // skbug.com/4116: We should perhaps also apply the alpha mask in kStandard
+ // mode. We just haven't seen any images that expect this
+ // behavior.
+ //
+ // Header types are matched based on size. If the header is
+ // V3+, we are guaranteed to be able to read at least this size.
+ SkASSERT(infoBytesRemaining > 52);
+ inputMasks.alpha = get_int(iBuffer.get(), 48);
break;
case kOS2VX_BmpHeaderType:
// TODO: Decide if we intend to support this.
@@ -366,101 +399,8 @@
SkCodecPrintf("Error: invalid format for bitmap decoding.\n");
return false;
}
-
- // Most versions of bmps should be rendered as opaque. Either they do
- // not have an alpha channel, or they expect the alpha channel to be
- // ignored. V3+ bmp files introduce an alpha mask and allow the creator
- // of the image to use the alpha channels. However, many of these images
- // leave the alpha channel blank and expect to be rendered as opaque. This
- // is the case for almost all V3 images, so we render these as opaque. For
- // V4+ images in kMask mode, we will use the alpha mask.
- //
- // skbug.com/4116: We should perhaps also apply the alpha mask in kStandard
- // mode. We just haven't seen any images that expect this
- // behavior.
- //
- // Additionally, V3 bmp-in-ico may use the alpha mask.
- SkAlphaType alphaType = kOpaque_SkAlphaType;
- if ((kInfoV3_BmpHeaderType == headerType && inIco) ||
- kInfoV4_BmpHeaderType == headerType ||
- kInfoV5_BmpHeaderType == headerType) {
- // Header types are matched based on size. If the header is
- // V3+, we are guaranteed to be able to read at least this size.
- SkASSERT(infoBytesRemaining > 52);
- inputMasks.alpha = get_int(iBuffer.get(), 48);
- if (inputMasks.alpha != 0) {
- alphaType = kUnpremul_SkAlphaType;
- }
- }
iBuffer.reset();
- // Additionally, 32 bit bmp-in-icos use the alpha channel.
- // FIXME (msarett): Don't all bmp-in-icos use the alpha channel?
- // And, RLE inputs may skip pixels, leaving them as transparent. This
- // is uncommon, but we cannot be certain that an RLE bmp will be opaque.
- if ((inIco && 32 == bitsPerPixel) || (kRLE_BmpInputFormat == inputFormat)) {
- alphaType = kUnpremul_SkAlphaType;
- }
-
- // Check for valid bits per pixel.
- // At the same time, use this information to choose a suggested color type
- // and to set default masks.
- SkColorType colorType = kN32_SkColorType;
- switch (bitsPerPixel) {
- // In addition to more standard pixel compression formats, bmp supports
- // the use of bit masks to determine pixel components. The standard
- // format for representing 16-bit colors is 555 (XRRRRRGGGGGBBBBB),
- // which does not map well to any Skia color formats. For this reason,
- // we will always enable mask mode with 16 bits per pixel.
- case 16:
- if (kBitMask_BmpInputFormat != inputFormat) {
- inputMasks.red = 0x7C00;
- inputMasks.green = 0x03E0;
- inputMasks.blue = 0x001F;
- inputFormat = kBitMask_BmpInputFormat;
- }
- break;
- // We want to decode to kIndex_8 for input formats that are already
- // designed in index format.
- case 1:
- case 2:
- case 4:
- case 8:
- // However, we cannot in RLE format since we may need to leave some
- // pixels as transparent. Similarly, we also cannot for ICO images
- // since we may need to apply a transparent mask.
- if (kRLE_BmpInputFormat != inputFormat && !inIco) {
- colorType = kIndex_8_SkColorType;
- }
-
- // Mask bmps must have 16, 24, or 32 bits per pixel.
- if (kBitMask_BmpInputFormat == inputFormat) {
- SkCodecPrintf("Error: invalid input value of bits per pixel for mask bmp.\n");
- return false;
- }
- case 24:
- case 32:
- break;
- default:
- SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
- return false;
- }
-
- // Check that input bit masks are valid and create the masks object
- SkAutoTDelete<SkMasks>
- masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
- if (nullptr == masks) {
- SkCodecPrintf("Error: invalid input masks.\n");
- return false;
- }
-
- // Check for a valid number of total bytes when in RLE mode
- if (totalBytes <= offset && kRLE_BmpInputFormat == inputFormat) {
- SkCodecPrintf("Error: RLE requires valid input size.\n");
- return false;
- }
- const size_t RLEBytes = totalBytes - offset;
-
// Calculate the number of bytes read so far
const uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes;
if (!inIco && offset < bytesRead) {
@@ -471,63 +411,133 @@
return false;
}
- // Skip to the start of the pixel array.
- // We can do this here because there is no color table to read
- // in bit mask mode.
- if (!inIco && kBitMask_BmpInputFormat == inputFormat) {
- if (stream->skip(offset - bytesRead) != offset - bytesRead) {
- SkCodecPrintf("Error: unable to skip to image data.\n");
- return false;
- }
- }
- if (codecOut) {
- // BMPs-in-ICOs contain an alpha mask after the image which means we
- // cannot guarantee that an image is opaque, even if the bmp thinks
- // it is.
- bool isOpaque = kOpaque_SkAlphaType == alphaType;
- if (inIco) {
- alphaType = kUnpremul_SkAlphaType;
- }
- // Set the image info
- const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
- colorType, alphaType);
+ switch (inputFormat) {
+ case kStandard_BmpInputFormat: {
+ // BMPs-in-ICOs often contain an alpha mask after the image, which
+ // means we cannot guarantee that an image is opaque, even if the
+ // embedded bmp is opaque.
+ // We use |isOpaque| to indicate if the BMP itself is opaque, but
+ // still need to recommend kUnpremul when it is contained in an ICO.
+ SkColorType colorType = kN32_SkColorType;
+ SkAlphaType alphaType = inIco ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType;
+ bool isOpaque = true;
+ switch (bitsPerPixel) {
+ // Palette formats
+ case 1:
+ case 2:
+ case 4:
+ case 8:
+ // We cannot recommend a palette color type for ICOs because they
+ // may contain a transparency mask.
+ if (!inIco) {
+ colorType = kIndex_8_SkColorType;
+ }
+ break;
+ case 24:
+ case 32:
+ // 32-bit BMP-in-ICOs actually use the alpha channel in place of a
+ // transparency mask.
+ if (inIco) {
+ isOpaque = false;
+ }
+ break;
+ default:
+ SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
+ return false;
+ }
- // Return the codec
- switch (inputFormat) {
- case kStandard_BmpInputFormat:
+ if (codecOut) {
// We require streams to have a memory base for Bmp-in-Ico decodes.
SkASSERT(!inIco || nullptr != stream->getMemoryBase());
+
+ // Set the image info and create a codec.
+ const SkImageInfo imageInfo = SkImageInfo::Make(width, height, colorType,
+ alphaType);
*codecOut = new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
bytesPerColor, offset - bytesRead, rowOrder, isOpaque, inIco);
- return true;
- case kBitMask_BmpInputFormat:
- // Bmp-in-Ico must be standard mode
- if (inIco) {
- SkCodecPrintf("Error: Icos may not use bit mask format.\n");
+
+ }
+ return true;
+ }
+
+ case kBitMask_BmpInputFormat: {
+ // Bmp-in-Ico must be standard mode
+ if (inIco) {
+ SkCodecPrintf("Error: Icos may not use bit mask format.\n");
+ return false;
+ }
+
+ switch (bitsPerPixel) {
+ case 16:
+ case 24:
+ case 32:
+ break;
+ default:
+ SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
+ return false;
+ }
+
+ // Skip to the start of the pixel array.
+ // We can do this here because there is no color table to read
+ // in bit mask mode.
+ if (stream->skip(offset - bytesRead) != offset - bytesRead) {
+ SkCodecPrintf("Error: unable to skip to image data.\n");
+ return false;
+ }
+
+ if (codecOut) {
+ // Check that input bit masks are valid and create the masks object
+ SkAutoTDelete<SkMasks> masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
+ if (nullptr == masks) {
+ SkCodecPrintf("Error: invalid input masks.\n");
return false;
}
+ // Set the image info
+ SkAlphaType alphaType = masks->getAlphaMask() ? kUnpremul_SkAlphaType :
+ kOpaque_SkAlphaType;
+ const SkImageInfo imageInfo = SkImageInfo::Make(width, height, kN32_SkColorType,
+ alphaType);
*codecOut = new SkBmpMaskCodec(imageInfo, stream, bitsPerPixel, masks.release(),
rowOrder);
- return true;
- case kRLE_BmpInputFormat:
- // Bmp-in-Ico must be standard mode
- // When inIco is true, this line cannot be reached, since we
- // require that RLE Bmps have a valid number of totalBytes, and
- // Icos skip the header that contains totalBytes.
- SkASSERT(!inIco);
+ }
+ return true;
+ }
+
+ case kRLE_BmpInputFormat: {
+ // We should not reach this point without a valid value of bitsPerPixel.
+ SkASSERT(4 == bitsPerPixel || 8 == bitsPerPixel || 24 == bitsPerPixel);
+
+ // Check for a valid number of total bytes when in RLE mode
+ if (totalBytes <= offset) {
+ SkCodecPrintf("Error: RLE requires valid input size.\n");
+ return false;
+ }
+ const size_t RLEBytes = totalBytes - offset;
+
+ // Bmp-in-Ico must be standard mode
+ // When inIco is true, this line cannot be reached, since we
+ // require that RLE Bmps have a valid number of totalBytes, and
+ // Icos skip the header that contains totalBytes.
+ SkASSERT(!inIco);
+
+ if (codecOut) {
+ // RLE inputs may skip pixels, leaving them as transparent. This
+ // is uncommon, but we cannot be certain that an RLE bmp will be
+ // opaque.
+ const SkImageInfo imageInfo = SkImageInfo::Make(width, height, kN32_SkColorType,
+ kUnpremul_SkAlphaType);
*codecOut = new SkBmpRLECodec(imageInfo, stream, bitsPerPixel, numColors,
bytesPerColor, offset - bytesRead, rowOrder, RLEBytes);
- return true;
- default:
- SkASSERT(false);
- return false;
+ }
+ return true;
}
+ default:
+ SkASSERT(false);
+ return false;
}
-
- return true;
}
/*
diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 39d357b..80a989a 100644
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -228,7 +228,7 @@
fSwizzler->swizzle(dstRow, fSrcBuffer.get());
}
- if (fInIco) {
+ if (fInIco && fIsOpaque) {
const int startScanline = this->currScanline();
if (startScanline < 0) {
// We are not performing a scanline decode.