Revert of Fix colorType/alphaType checks in SkCodec (patchset #5 id:80001 of https://codereview.chromium.org/1695473002/ )
Reason for revert:
Really bad images in Gold.
Original issue's description:
> Fix colorType/alphaType checks in SkCodec
>
> Make getPixels() and startScanlineDecode() behave
> consistently.
>
> Require that kGray8 decodes are opaque.
>
> Assert that creating the swizzler succeeds.
>
> BUG=skia:4203
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1695473002
>
> Committed: https://skia.googlesource.com/skia/+/c7578b6cdd03b61f076ffc7956efd952d6c301c0
TBR=scroggo@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:4203
Review URL: https://codereview.chromium.org/1694023002
diff --git a/src/codec/SkBmpMaskCodec.cpp b/src/codec/SkBmpMaskCodec.cpp
index 21d231b..b603de9 100644
--- a/src/codec/SkBmpMaskCodec.cpp
+++ b/src/codec/SkBmpMaskCodec.cpp
@@ -57,12 +57,25 @@
return kSuccess;
}
-SkCodec::Result SkBmpMaskCodec::prepareToDecode(const SkImageInfo& dstInfo,
- const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) {
- // Initialize the mask swizzler
+bool SkBmpMaskCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& options) {
+ // Create the swizzler
fMaskSwizzler.reset(SkMaskSwizzler::CreateMaskSwizzler(dstInfo, this->getInfo(), fMasks,
this->bitsPerPixel(), options));
- SkASSERT(fMaskSwizzler);
+
+ if (nullptr == fMaskSwizzler.get()) {
+ return false;
+ }
+
+ return true;
+}
+
+SkCodec::Result SkBmpMaskCodec::prepareToDecode(const SkImageInfo& dstInfo,
+ const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) {
+ // Initialize a the mask swizzler
+ if (!this->initializeSwizzler(dstInfo, options)) {
+ SkCodecPrintf("Error: cannot initialize swizzler.\n");
+ return SkCodec::kInvalidConversion;
+ }
return SkCodec::kSuccess;
}
diff --git a/src/codec/SkBmpMaskCodec.h b/src/codec/SkBmpMaskCodec.h
index 93b3bca..73a8b37 100644
--- a/src/codec/SkBmpMaskCodec.h
+++ b/src/codec/SkBmpMaskCodec.h
@@ -44,6 +44,7 @@
private:
+ bool initializeSwizzler(const SkImageInfo& dstInfo, const Options& options);
SkSampler* getSampler(bool createIfNecessary) override {
SkASSERT(fMaskSwizzler);
return fMaskSwizzler;
diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 39d357b..47b1070 100644
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -152,9 +152,9 @@
return true;
}
-void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
+bool SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
// Get swizzler configuration
- SkSwizzler::SrcConfig config = SkSwizzler::kUnknown;
+ SkSwizzler::SrcConfig config;
switch (this->bitsPerPixel()) {
case 1:
config = SkSwizzler::kIndex1;
@@ -180,6 +180,7 @@
break;
default:
SkASSERT(false);
+ return false;
}
// Get a pointer to the color table if it exists
@@ -187,7 +188,11 @@
// Create swizzler
fSwizzler.reset(SkSwizzler::CreateSwizzler(config, colorPtr, dstInfo, opts));
- SkASSERT(fSwizzler);
+
+ if (nullptr == fSwizzler.get()) {
+ return false;
+ }
+ return true;
}
SkCodec::Result SkBmpStandardCodec::prepareToDecode(const SkImageInfo& dstInfo,
@@ -202,8 +207,11 @@
// Copy the color table to the client if necessary
copy_color_table(dstInfo, this->fColorTable, inputColorPtr, inputColorCount);
- // Initialize a swizzler
- this->initializeSwizzler(dstInfo, options);
+ // Initialize a swizzler if necessary
+ if (!this->initializeSwizzler(dstInfo, options)) {
+ SkCodecPrintf("Error: cannot initialize swizzler.\n");
+ return SkCodec::kInvalidConversion;
+ }
return SkCodec::kSuccess;
}
diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h
index 557f653..d3e2ed3 100644
--- a/src/codec/SkBmpStandardCodec.h
+++ b/src/codec/SkBmpStandardCodec.h
@@ -72,7 +72,7 @@
*/
bool createColorTable(SkAlphaType alphaType, int* colorCount);
- void initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts);
+ bool initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts);
int decodeRows(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,
const Options& opts) override;
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index bb7b265..0da36bb1 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -162,6 +162,15 @@
ctable = nullptr;
}
+ {
+ SkAlphaType canonical;
+ if (!SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &canonical)
+ || canonical != info.alphaType())
+ {
+ return kInvalidConversion;
+ }
+ }
+
if (!this->rewindIfNeeded()) {
return kCouldNotRewind;
}
diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h
index 5d66118..235c4e2 100644
--- a/src/codec/SkCodecPriv.h
+++ b/src/codec/SkCodecPriv.h
@@ -136,12 +136,7 @@
case kN32_SkColorType:
return true;
case kRGB_565_SkColorType:
- return kOpaque_SkAlphaType == dst.alphaType();
- case kGray_8_SkColorType:
- if (kOpaque_SkAlphaType != dst.alphaType()) {
- return false;
- }
- // Fall through
+ return src.alphaType() == kOpaque_SkAlphaType;
default:
return dst.colorType() == src.colorType();
}
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 785c17e..a938f5f 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -436,16 +436,19 @@
// Initialize color table and copy to the client if necessary
this->initializeColorTable(dstInfo, inputColorPtr, inputColorCount);
- this->initializeSwizzler(dstInfo, opts);
- return kSuccess;
+ return this->initializeSwizzler(dstInfo, opts);
}
-void SkGifCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
+SkCodec::Result SkGifCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
const SkIRect* frameRect = fFrameIsSubset ? &fFrameRect : nullptr;
fSwizzler.reset(SkSwizzler::CreateSwizzler(SkSwizzler::kIndex, colorPtr, dstInfo, opts,
frameRect));
- SkASSERT(fSwizzler);
+
+ if (nullptr != fSwizzler.get()) {
+ return kSuccess;
+ }
+ return kUnimplemented;
}
bool SkGifCodec::readRow() {
diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h
index 010a344..91861b2 100644
--- a/src/codec/SkGifCodec.h
+++ b/src/codec/SkGifCodec.h
@@ -128,7 +128,7 @@
* @param options Informs the swizzler if destination memory is zero initialized.
* Contains subset information.
*/
- void initializeSwizzler(const SkImageInfo& dstInfo,
+ Result initializeSwizzler(const SkImageInfo& dstInfo,
const Options& options);
SkSampler* getSampler(bool createIfNecessary) override {
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index e1440af..f920c98 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -372,7 +372,6 @@
}
fSwizzler.reset(SkSwizzler::CreateSwizzler(srcConfig, nullptr, dstInfo, options));
- SkASSERT(fSwizzler);
fStorage.reset(get_row_bytes(fDecoderMgr->dinfo()));
fSrcRow = fStorage.get();
}
diff --git a/src/codec/SkWbmpCodec.cpp b/src/codec/SkWbmpCodec.cpp
index 706d5dd..af1ab46 100644
--- a/src/codec/SkWbmpCodec.cpp
+++ b/src/codec/SkWbmpCodec.cpp
@@ -30,19 +30,6 @@
}
}
-static inline bool valid_color_type(SkColorType colorType, SkAlphaType alphaType) {
- switch (colorType) {
- case kN32_SkColorType:
- case kIndex_8_SkColorType:
- return true;
- case kGray_8_SkColorType:
- case kRGB_565_SkColorType:
- return kOpaque_SkAlphaType == alphaType;
- default:
- return false;
- }
-}
-
static bool read_byte(SkStream* stream, uint8_t* data)
{
return stream->read(data, 1) == 1;
@@ -97,6 +84,16 @@
SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info, const SkPMColor* ctable,
const Options& opts) {
+ // Create the swizzler based on the desired color type
+ switch (info.colorType()) {
+ case kIndex_8_SkColorType:
+ case kN32_SkColorType:
+ case kRGB_565_SkColorType:
+ case kGray_8_SkColorType:
+ break;
+ default:
+ return nullptr;
+ }
return SkSwizzler::CreateSwizzler(SkSwizzler::kBit, ctable, info, opts);
}
@@ -127,8 +124,7 @@
return kUnimplemented;
}
- if (!valid_color_type(info.colorType(), info.alphaType()) ||
- !valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
+ if (!valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
return kInvalidConversion;
}
@@ -137,7 +133,9 @@
// Initialize the swizzler
SkAutoTDelete<SkSwizzler> swizzler(this->initializeSwizzler(info, ctable, options));
- SkASSERT(swizzler);
+ if (nullptr == swizzler.get()) {
+ return kInvalidConversion;
+ }
// Perform the decode
SkISize size = info.dimensions();
@@ -195,8 +193,7 @@
return kUnimplemented;
}
- if (!valid_color_type(dstInfo.colorType(), dstInfo.alphaType()) ||
- !valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
+ if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
return kInvalidConversion;
}
@@ -210,7 +207,9 @@
// Initialize the swizzler
fSwizzler.reset(this->initializeSwizzler(dstInfo, get_color_ptr(fColorTable.get()), options));
- SkASSERT(fSwizzler);
+ if (nullptr == fSwizzler.get()) {
+ return kInvalidConversion;
+ }
fSrcBuffer.reset(fSrcRowBytes);