From deabdb5b9712de9e3c6bbb9aa68ec2f20df80a8e Mon Sep 17 00:00:00 2001 From: msarett Date: Fri, 12 Feb 2016 15:00:10 -0800 Subject: [PATCH] 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 --- src/codec/SkBmpMaskCodec.cpp | 21 +++++++++++++++++---- src/codec/SkBmpMaskCodec.h | 1 + src/codec/SkBmpStandardCodec.cpp | 18 +++++++++++++----- src/codec/SkBmpStandardCodec.h | 2 +- src/codec/SkCodec.cpp | 9 +++++++++ src/codec/SkCodecPriv.h | 7 +------ src/codec/SkGifCodec.cpp | 11 +++++++---- src/codec/SkGifCodec.h | 2 +- src/codec/SkJpegCodec.cpp | 1 - src/codec/SkWbmpCodec.cpp | 37 ++++++++++++++++++------------------- 10 files changed, 68 insertions(+), 41 deletions(-) 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 @@ SkCodec::Result SkBmpMaskCodec::onGetPixels(const SkImageInfo& dstInfo, 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 @@ protected: 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 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo, 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 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op break; default: SkASSERT(false); + return false; } // Get a pointer to the color table if it exists @@ -187,7 +188,11 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op // 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 @@ SkCodec::Result SkBmpStandardCodec::prepareToDecode(const SkImageInfo& dstInfo, // 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 @@ private: */ 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..0da36bb 100644 --- a/src/codec/SkCodec.cpp +++ b/src/codec/SkCodec.cpp @@ -162,6 +162,15 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t 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 @@ inline bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) 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 @@ SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColo // 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 @@ private: * @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 @@ void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& } 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 void setup_color_table(SkColorType colorType, } } -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 @@ bool SkWbmpCodec::onRewind() { 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 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info, 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 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info, // Initialize the swizzler SkAutoTDelete 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 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo, 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 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo, // 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); -- 2.7.4