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
This commit is contained in:
msarett 2016-02-12 15:00:10 -08:00 committed by Commit bot
parent 3478f753ff
commit deabdb5b97
10 changed files with 68 additions and 41 deletions

View File

@ -57,12 +57,25 @@ SkCodec::Result SkBmpMaskCodec::onGetPixels(const SkImageInfo& dstInfo,
return kSuccess; return kSuccess;
} }
SkCodec::Result SkBmpMaskCodec::prepareToDecode(const SkImageInfo& dstInfo, bool SkBmpMaskCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& options) {
const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) { // Create the swizzler
// Initialize the mask swizzler
fMaskSwizzler.reset(SkMaskSwizzler::CreateMaskSwizzler(dstInfo, this->getInfo(), fMasks, fMaskSwizzler.reset(SkMaskSwizzler::CreateMaskSwizzler(dstInfo, this->getInfo(), fMasks,
this->bitsPerPixel(), options)); 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; return SkCodec::kSuccess;
} }

View File

@ -44,6 +44,7 @@ protected:
private: private:
bool initializeSwizzler(const SkImageInfo& dstInfo, const Options& options);
SkSampler* getSampler(bool createIfNecessary) override { SkSampler* getSampler(bool createIfNecessary) override {
SkASSERT(fMaskSwizzler); SkASSERT(fMaskSwizzler);
return fMaskSwizzler; return fMaskSwizzler;

View File

@ -152,9 +152,9 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
return true; return true;
} }
void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) { bool SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
// Get swizzler configuration // Get swizzler configuration
SkSwizzler::SrcConfig config = SkSwizzler::kUnknown; SkSwizzler::SrcConfig config;
switch (this->bitsPerPixel()) { switch (this->bitsPerPixel()) {
case 1: case 1:
config = SkSwizzler::kIndex1; config = SkSwizzler::kIndex1;
@ -180,6 +180,7 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op
break; break;
default: default:
SkASSERT(false); SkASSERT(false);
return false;
} }
// Get a pointer to the color table if it exists // Get a pointer to the color table if it exists
@ -187,7 +188,11 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op
// Create swizzler // Create swizzler
fSwizzler.reset(SkSwizzler::CreateSwizzler(config, colorPtr, dstInfo, opts)); 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, 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 the color table to the client if necessary
copy_color_table(dstInfo, this->fColorTable, inputColorPtr, inputColorCount); copy_color_table(dstInfo, this->fColorTable, inputColorPtr, inputColorCount);
// Initialize a swizzler // Initialize a swizzler if necessary
this->initializeSwizzler(dstInfo, options); if (!this->initializeSwizzler(dstInfo, options)) {
SkCodecPrintf("Error: cannot initialize swizzler.\n");
return SkCodec::kInvalidConversion;
}
return SkCodec::kSuccess; return SkCodec::kSuccess;
} }

View File

@ -72,7 +72,7 @@ private:
*/ */
bool createColorTable(SkAlphaType alphaType, int* colorCount); 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, int decodeRows(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,
const Options& opts) override; const Options& opts) override;

View File

@ -162,6 +162,15 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
ctable = nullptr; ctable = nullptr;
} }
{
SkAlphaType canonical;
if (!SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &canonical)
|| canonical != info.alphaType())
{
return kInvalidConversion;
}
}
if (!this->rewindIfNeeded()) { if (!this->rewindIfNeeded()) {
return kCouldNotRewind; return kCouldNotRewind;
} }

View File

@ -136,12 +136,7 @@ inline bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src)
case kN32_SkColorType: case kN32_SkColorType:
return true; return true;
case kRGB_565_SkColorType: case kRGB_565_SkColorType:
return kOpaque_SkAlphaType == dst.alphaType(); return src.alphaType() == kOpaque_SkAlphaType;
case kGray_8_SkColorType:
if (kOpaque_SkAlphaType != dst.alphaType()) {
return false;
}
// Fall through
default: default:
return dst.colorType() == src.colorType(); return dst.colorType() == src.colorType();
} }

View File

@ -436,16 +436,19 @@ SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColo
// Initialize color table and copy to the client if necessary // Initialize color table and copy to the client if necessary
this->initializeColorTable(dstInfo, inputColorPtr, inputColorCount); this->initializeColorTable(dstInfo, inputColorPtr, inputColorCount);
this->initializeSwizzler(dstInfo, opts); return this->initializeSwizzler(dstInfo, opts);
return kSuccess;
} }
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 SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
const SkIRect* frameRect = fFrameIsSubset ? &fFrameRect : nullptr; const SkIRect* frameRect = fFrameIsSubset ? &fFrameRect : nullptr;
fSwizzler.reset(SkSwizzler::CreateSwizzler(SkSwizzler::kIndex, colorPtr, dstInfo, opts, fSwizzler.reset(SkSwizzler::CreateSwizzler(SkSwizzler::kIndex, colorPtr, dstInfo, opts,
frameRect)); frameRect));
SkASSERT(fSwizzler);
if (nullptr != fSwizzler.get()) {
return kSuccess;
}
return kUnimplemented;
} }
bool SkGifCodec::readRow() { bool SkGifCodec::readRow() {

View File

@ -128,7 +128,7 @@ private:
* @param options Informs the swizzler if destination memory is zero initialized. * @param options Informs the swizzler if destination memory is zero initialized.
* Contains subset information. * Contains subset information.
*/ */
void initializeSwizzler(const SkImageInfo& dstInfo, Result initializeSwizzler(const SkImageInfo& dstInfo,
const Options& options); const Options& options);
SkSampler* getSampler(bool createIfNecessary) override { SkSampler* getSampler(bool createIfNecessary) override {

View File

@ -372,7 +372,6 @@ void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options&
} }
fSwizzler.reset(SkSwizzler::CreateSwizzler(srcConfig, nullptr, dstInfo, options)); fSwizzler.reset(SkSwizzler::CreateSwizzler(srcConfig, nullptr, dstInfo, options));
SkASSERT(fSwizzler);
fStorage.reset(get_row_bytes(fDecoderMgr->dinfo())); fStorage.reset(get_row_bytes(fDecoderMgr->dinfo()));
fSrcRow = fStorage.get(); fSrcRow = fStorage.get();
} }

View File

@ -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) static bool read_byte(SkStream* stream, uint8_t* data)
{ {
return stream->read(data, 1) == 1; return stream->read(data, 1) == 1;
@ -97,6 +84,16 @@ bool SkWbmpCodec::onRewind() {
SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info, const SkPMColor* ctable, SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info, const SkPMColor* ctable,
const Options& opts) { 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); return SkSwizzler::CreateSwizzler(SkSwizzler::kBit, ctable, info, opts);
} }
@ -127,8 +124,7 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
return kUnimplemented; return kUnimplemented;
} }
if (!valid_color_type(info.colorType(), info.alphaType()) || if (!valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
!valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
return kInvalidConversion; return kInvalidConversion;
} }
@ -137,7 +133,9 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
// Initialize the swizzler // Initialize the swizzler
SkAutoTDelete<SkSwizzler> swizzler(this->initializeSwizzler(info, ctable, options)); SkAutoTDelete<SkSwizzler> swizzler(this->initializeSwizzler(info, ctable, options));
SkASSERT(swizzler); if (nullptr == swizzler.get()) {
return kInvalidConversion;
}
// Perform the decode // Perform the decode
SkISize size = info.dimensions(); SkISize size = info.dimensions();
@ -195,8 +193,7 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
return kUnimplemented; return kUnimplemented;
} }
if (!valid_color_type(dstInfo.colorType(), dstInfo.alphaType()) || if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
return kInvalidConversion; return kInvalidConversion;
} }
@ -210,7 +207,9 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
// Initialize the swizzler // Initialize the swizzler
fSwizzler.reset(this->initializeSwizzler(dstInfo, get_color_ptr(fColorTable.get()), options)); fSwizzler.reset(this->initializeSwizzler(dstInfo, get_color_ptr(fColorTable.get()), options));
SkASSERT(fSwizzler); if (nullptr == fSwizzler.get()) {
return kInvalidConversion;
}
fSrcBuffer.reset(fSrcRowBytes); fSrcBuffer.reset(fSrcRowBytes);