Adjust the alpha type for pixelRefs.
Move SkBitmap's validate_alphaType to SkImageInfo, with the new
name SkColorTypeValidateAlphaType. Use it in SkPixelRef's constructors,
as well as in SkDecodingImageGenerator. This fixes a bug where an
SkPixelRef's SkAlphaType could get out of sync with its SkBitmap,
when both were assigned the same SkAlphaType.
R=reed@google.com, halcanary@google.com
Author: scroggo@google.com
Review URL: https://codereview.chromium.org/346593003
diff --git a/include/core/SkImageInfo.h b/include/core/SkImageInfo.h
index 17e1228..3d82dc8 100644
--- a/include/core/SkImageInfo.h
+++ b/include/core/SkImageInfo.h
@@ -124,6 +124,15 @@
///////////////////////////////////////////////////////////////////////////////
/**
+ * Return true if alphaType is supported by colorType. If there is a canonical
+ * alphaType for this colorType, return it in canonical.
+ */
+bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType,
+ SkAlphaType* canonical = NULL);
+
+///////////////////////////////////////////////////////////////////////////////
+
+/**
* Describe an image's dimensions and pixel type.
*/
struct SkImageInfo {
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 38df07c..82e0b4c 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -152,42 +152,11 @@
///////////////////////////////////////////////////////////////////////////////
-static bool validate_alphaType(SkColorType colorType, SkAlphaType alphaType,
- SkAlphaType* canonical = NULL) {
- switch (colorType) {
- case kUnknown_SkColorType:
- alphaType = kIgnore_SkAlphaType;
- break;
- case kAlpha_8_SkColorType:
- if (kUnpremul_SkAlphaType == alphaType) {
- alphaType = kPremul_SkAlphaType;
- }
- // fall-through
- case kIndex_8_SkColorType:
- case kARGB_4444_SkColorType:
- case kRGBA_8888_SkColorType:
- case kBGRA_8888_SkColorType:
- if (kIgnore_SkAlphaType == alphaType) {
- return false;
- }
- break;
- case kRGB_565_SkColorType:
- alphaType = kOpaque_SkAlphaType;
- break;
- default:
- return false;
- }
- if (canonical) {
- *canonical = alphaType;
- }
- return true;
-}
-
bool SkBitmap::setInfo(const SkImageInfo& origInfo, size_t rowBytes) {
SkImageInfo info = origInfo;
- if (!validate_alphaType(info.fColorType, info.fAlphaType,
- &info.fAlphaType)) {
+ if (!SkColorTypeValidateAlphaType(info.fColorType, info.fAlphaType,
+ &info.fAlphaType)) {
return reset_return_false(this);
}
@@ -228,7 +197,7 @@
#endif
bool SkBitmap::setAlphaType(SkAlphaType alphaType) {
- if (!validate_alphaType(fInfo.fColorType, alphaType, &alphaType)) {
+ if (!SkColorTypeValidateAlphaType(fInfo.fColorType, alphaType, &alphaType)) {
return false;
}
if (fInfo.fAlphaType != alphaType) {
@@ -1332,7 +1301,7 @@
if (!buffer.validate((info.width() >= 0) && (info.height() >= 0) &&
SkColorTypeIsValid(info.fColorType) &&
SkAlphaTypeIsValid(info.fAlphaType) &&
- validate_alphaType(info.fColorType, info.fAlphaType) &&
+ SkColorTypeValidateAlphaType(info.fColorType, info.fAlphaType) &&
info.validRowBytes(rowBytes))) {
return;
}
diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp
index 27c4573..e61cd7d 100644
--- a/src/core/SkImageInfo.cpp
+++ b/src/core/SkImageInfo.cpp
@@ -38,3 +38,34 @@
uint32_t packed = (fAlphaType << 8) | fColorType;
buffer.write32(packed);
}
+
+bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType,
+ SkAlphaType* canonical) {
+ switch (colorType) {
+ case kUnknown_SkColorType:
+ alphaType = kIgnore_SkAlphaType;
+ break;
+ case kAlpha_8_SkColorType:
+ if (kUnpremul_SkAlphaType == alphaType) {
+ alphaType = kPremul_SkAlphaType;
+ }
+ // fall-through
+ case kIndex_8_SkColorType:
+ case kARGB_4444_SkColorType:
+ case kRGBA_8888_SkColorType:
+ case kBGRA_8888_SkColorType:
+ if (kIgnore_SkAlphaType == alphaType) {
+ return false;
+ }
+ break;
+ case kRGB_565_SkColorType:
+ alphaType = kOpaque_SkAlphaType;
+ break;
+ default:
+ return false;
+ }
+ if (canonical) {
+ *canonical = alphaType;
+ }
+ return true;
+}
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index bfa4ae2..829c3f1 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -84,6 +84,9 @@
#define SKPIXELREF_PRELOCKED_LOCKCOUNT 123456789
SkPixelRef::SkPixelRef(const SkImageInfo& info) : fInfo(info) {
+ SkAssertResult(SkColorTypeValidateAlphaType(fInfo.colorType(), fInfo.alphaType(),
+ const_cast<SkAlphaType*>(&fInfo.fAlphaType)));
+
this->setMutex(NULL);
fRec.zero();
fLockCount = 0;
@@ -94,6 +97,9 @@
SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) {
+ SkAssertResult(SkColorTypeValidateAlphaType(fInfo.colorType(), fInfo.alphaType(),
+ const_cast<SkAlphaType*>(&fInfo.fAlphaType)));
+
this->setMutex(mutex);
fRec.zero();
fLockCount = 0;
@@ -112,6 +118,10 @@
: INHERITED(buffer)
, fInfo(read_info(buffer))
{
+ SkDEBUGCODE(SkAlphaType alphaType;)
+ SkASSERT(SkColorTypeValidateAlphaType(fInfo.colorType(), fInfo.alphaType(), &alphaType));
+ SkASSERT(fInfo.fAlphaType == alphaType);
+
this->setMutex(mutex);
fRec.zero();
fLockCount = 0;
diff --git a/src/images/SkDecodingImageGenerator.cpp b/src/images/SkDecodingImageGenerator.cpp
index 88cdef9..dfa093b 100644
--- a/src/images/SkDecodingImageGenerator.cpp
+++ b/src/images/SkDecodingImageGenerator.cpp
@@ -246,6 +246,11 @@
if (opts.fRequireUnpremul && info.fAlphaType != kOpaque_SkAlphaType) {
info.fAlphaType = kUnpremul_SkAlphaType;
}
+
+ if (!SkColorTypeValidateAlphaType(info.fColorType, info.fAlphaType, &info.fAlphaType)) {
+ return NULL;
+ }
+
return SkNEW_ARGS(DecodingImageGenerator,
(data, autoStream.detach(), info,
opts.fSampleSize, opts.fDitherImage));