From 7a9900d6d34e437bb24beb5524a1f6488ae138c9 Mon Sep 17 00:00:00 2001 From: msarett Date: Thu, 8 Sep 2016 10:14:04 -0700 Subject: Checking for valid colorType, alphaType, colorSpace in SkCodec * Refactor to share code between SkPngCodec and SkWebpCodec * Didn't end up sharing with SkJpegCodec but did refactor that code a bit * Disallow conversions to F16 with non-linear color spaces * Fail to decode if we fail to create a SkColorSpaceXform (should be an assert soon). We used to fallback on a legacy decode if we failed to create the transform. * A bunch of name changes BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2319293003 Review-Url: https://codereview.chromium.org/2319293003 --- src/codec/SkBmpCodec.cpp | 2 +- src/codec/SkBmpMaskCodec.cpp | 2 +- src/codec/SkBmpRLECodec.cpp | 2 +- src/codec/SkBmpStandardCodec.cpp | 2 +- src/codec/SkCodecPriv.h | 42 +++++++++++++++++++++++++++++++++++++-- src/codec/SkGifCodec.cpp | 5 ++--- src/codec/SkJpegCodec.cpp | 43 +++++++++++++++++----------------------- src/codec/SkJpegCodec.h | 4 ++-- src/codec/SkPngCodec.cpp | 39 +++++------------------------------- src/codec/SkRawCodec.cpp | 2 +- src/codec/SkWebpCodec.cpp | 27 ++++--------------------- 11 files changed, 76 insertions(+), 94 deletions(-) (limited to 'src/codec') diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp index 1c999eba3c..2f796ad667 100644 --- a/src/codec/SkBmpCodec.cpp +++ b/src/codec/SkBmpCodec.cpp @@ -603,7 +603,7 @@ int32_t SkBmpCodec::getDstRow(int32_t y, int32_t height) const { SkCodec::Result SkBmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo, const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) { - if (!conversion_possible(dstInfo, this->getInfo())) { + if (!conversion_possible_ignore_color_space(dstInfo, this->getInfo())) { SkCodecPrintf("Error: cannot convert input type to output type.\n"); return kInvalidConversion; } diff --git a/src/codec/SkBmpMaskCodec.cpp b/src/codec/SkBmpMaskCodec.cpp index 7d2d398bb4..5b28252f7c 100644 --- a/src/codec/SkBmpMaskCodec.cpp +++ b/src/codec/SkBmpMaskCodec.cpp @@ -39,7 +39,7 @@ SkCodec::Result SkBmpMaskCodec::onGetPixels(const SkImageInfo& dstInfo, return kInvalidScale; } - if (!conversion_possible(dstInfo, this->getInfo())) { + if (!conversion_possible_ignore_color_space(dstInfo, this->getInfo())) { SkCodecPrintf("Error: cannot convert input type to output type.\n"); return kInvalidConversion; } diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp index 02b42f6d90..252d63e352 100644 --- a/src/codec/SkBmpRLECodec.cpp +++ b/src/codec/SkBmpRLECodec.cpp @@ -44,7 +44,7 @@ SkCodec::Result SkBmpRLECodec::onGetPixels(const SkImageInfo& dstInfo, // Subsets are not supported. return kUnimplemented; } - if (!conversion_possible(dstInfo, this->getInfo())) { + if (!conversion_possible_ignore_color_space(dstInfo, this->getInfo())) { SkCodecPrintf("Error: cannot convert input type to output type.\n"); return kInvalidConversion; } diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp index 651ff83b8a..089e3105c2 100644 --- a/src/codec/SkBmpStandardCodec.cpp +++ b/src/codec/SkBmpStandardCodec.cpp @@ -48,7 +48,7 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo, SkCodecPrintf("Error: scaling not supported.\n"); return kInvalidScale; } - if (!conversion_possible(dstInfo, this->getInfo())) { + if (!conversion_possible_ignore_color_space(dstInfo, this->getInfo())) { SkCodecPrintf("Error: cannot convert input type to output type.\n"); return kInvalidConversion; } diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h index 9a8a43e835..4285786304 100644 --- a/src/codec/SkCodecPriv.h +++ b/src/codec/SkCodecPriv.h @@ -107,14 +107,18 @@ static inline bool valid_alpha(SkAlphaType dstAlpha, SkAlphaType srcAlpha) { } /* + * Original version of conversion_possible that does not account for color spaces. + * Used by codecs that have not been updated to support color spaces. + * * Most of our codecs support the same conversions: * - opaque to any alpha type * - 565 only if opaque * - premul to unpremul and vice versa - * - always support N32 + * - always support RGBA, BGRA * - otherwise match the src color type */ -static inline bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) { +static inline bool conversion_possible_ignore_color_space(const SkImageInfo& dst, + const SkImageInfo& src) { // Ensure the alpha type is valid if (!valid_alpha(dst.alphaType(), src.alphaType())) { return false; @@ -335,4 +339,38 @@ static inline SkAlphaType select_alpha_xform(SkAlphaType dstAlphaType, SkAlphaTy return (kOpaque_SkAlphaType == srcAlphaType) ? kOpaque_SkAlphaType : dstAlphaType; } +/* + * Alpha Type Conversions + * - kOpaque to kOpaque, kUnpremul, kPremul is valid + * - kUnpremul to kUnpremul, kPremul is valid + * + * Color Type Conversions + * - Always support kRGBA_8888, kBGRA_8888 + * - Support kRGBA_F16 when there is a linear dst color space + * - Support kIndex8 if it matches the src + * - Support k565, kGray8 if kOpaque and color correction is not required + */ +static inline bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) { + // Ensure the alpha type is valid. + if (!valid_alpha(dst.alphaType(), src.alphaType())) { + return false; + } + + // Check for supported color types. + switch (dst.colorType()) { + case kRGBA_8888_SkColorType: + case kBGRA_8888_SkColorType: + return true; + case kRGBA_F16_SkColorType: + return dst.colorSpace() && dst.colorSpace()->gammaIsLinear(); + case kIndex_8_SkColorType: + return kIndex_8_SkColorType == src.colorType(); + case kRGB_565_SkColorType: + case kGray_8_SkColorType: + return kOpaque_SkAlphaType == src.alphaType() && !needs_color_xform(dst, src); + default: + return false; + } +} + #endif // SkCodecPriv_DEFINED diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp index dcc25b8dfd..1e6e300a92 100644 --- a/src/codec/SkGifCodec.cpp +++ b/src/codec/SkGifCodec.cpp @@ -450,9 +450,8 @@ void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, SkPMColor* inp SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColor* inputColorPtr, int* inputColorCount, const Options& opts) { // Check for valid input parameters - if (!conversion_possible(dstInfo, this->getInfo())) { - return gif_error("Cannot convert input type to output type.\n", - kInvalidConversion); + if (!conversion_possible_ignore_color_space(dstInfo, this->getInfo())) { + return gif_error("Cannot convert input type to output type.\n", kInvalidConversion); } // Initialize color table and copy to the client if necessary diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp index fcf89d0c1d..d2f7cdb0fa 100644 --- a/src/codec/SkJpegCodec.cpp +++ b/src/codec/SkJpegCodec.cpp @@ -357,7 +357,7 @@ bool SkJpegCodec::onRewind() { * image has been implemented * Sets the output color space */ -bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColorXform) { +bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo) { if (kUnknown_SkAlphaType == dstInfo.alphaType()) { return false; } @@ -384,7 +384,7 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo case kBGRA_8888_SkColorType: if (isCMYK) { fDecoderMgr->dinfo()->out_color_space = JCS_CMYK; - } else if (needsColorXform) { + } else if (fColorXform) { // Our color transformation code requires RGBA order inputs, but it'll swizzle // to BGRA for us. fDecoderMgr->dinfo()->out_color_space = JCS_EXT_RGBA; @@ -393,7 +393,7 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo } return true; case kRGB_565_SkColorType: - if (needsColorXform) { + if (fColorXform) { return false; } @@ -405,14 +405,17 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo } return true; case kGray_8_SkColorType: - if (needsColorXform || JCS_GRAYSCALE != encodedColorType) { + if (fColorXform || JCS_GRAYSCALE != encodedColorType) { return false; } fDecoderMgr->dinfo()->out_color_space = JCS_GRAYSCALE; return true; case kRGBA_F16_SkColorType: - SkASSERT(needsColorXform); + SkASSERT(fColorXform); + if (!dstInfo.colorSpace()->gammaIsLinear()) { + return false; + } if (isCMYK) { fDecoderMgr->dinfo()->out_color_space = JCS_CMYK; @@ -545,16 +548,13 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo, return fDecoderMgr->returnFailure("setjmp", kInvalidInput); } + this->initializeColorXform(dstInfo); + // Check if we can decode to the requested destination and set the output color space - bool needsColorXform = needs_color_xform(dstInfo, this->getInfo()); - if (!this->setOutputColorSpace(dstInfo, needsColorXform)) { + if (!this->setOutputColorSpace(dstInfo)) { return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion); } - if (!this->initializeColorXform(dstInfo, needsColorXform)) { - return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters); - } - if (!jpeg_start_decompress(dinfo)) { return fDecoderMgr->returnFailure("startDecompress", kInvalidInput); } @@ -630,16 +630,12 @@ void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& SkASSERT(fSwizzler); } -bool SkJpegCodec::initializeColorXform(const SkImageInfo& dstInfo, bool needsColorXform) { - if (needsColorXform) { +void SkJpegCodec::initializeColorXform(const SkImageInfo& dstInfo) { + if (needs_color_xform(dstInfo, this->getInfo())) { fColorXform = SkColorSpaceXform::New(sk_ref_sp(this->getInfo().colorSpace()), sk_ref_sp(dstInfo.colorSpace())); - if (!fColorXform && kRGBA_F16_SkColorType == dstInfo.colorType()) { - return false; - } + SkASSERT(fColorXform); } - - return true; } SkSampler* SkJpegCodec::getSampler(bool createIfNecessary) { @@ -661,14 +657,11 @@ SkCodec::Result SkJpegCodec::onStartScanlineDecode(const SkImageInfo& dstInfo, return kInvalidInput; } - // Check if we can decode to the requested destination and set the output color space - bool needsColorXform = needs_color_xform(dstInfo, this->getInfo()); - if (!this->setOutputColorSpace(dstInfo, needsColorXform)) { - return kInvalidConversion; - } + this->initializeColorXform(dstInfo); - if (!this->initializeColorXform(dstInfo, needsColorXform)) { - return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters); + // Check if we can decode to the requested destination and set the output color space + if (!this->setOutputColorSpace(dstInfo)) { + return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion); } if (!jpeg_start_decompress(fDecoderMgr->dinfo())) { diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h index 192e4beb0c..30425eea3f 100644 --- a/src/codec/SkJpegCodec.h +++ b/src/codec/SkJpegCodec.h @@ -104,10 +104,10 @@ private: * * Sets the output color space. */ - bool setOutputColorSpace(const SkImageInfo& dst, bool needsColorXform); + bool setOutputColorSpace(const SkImageInfo& dst); void initializeSwizzler(const SkImageInfo& dstInfo, const Options& options); - bool initializeColorXform(const SkImageInfo& dstInfo, bool needsColorXform); + void initializeColorXform(const SkImageInfo& dstInfo); void allocateStorage(const SkImageInfo& dstInfo); int readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count); diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp index 34e6f91131..36ec21f0b8 100644 --- a/src/codec/SkPngCodec.cpp +++ b/src/codec/SkPngCodec.cpp @@ -357,25 +357,6 @@ static int bytes_per_pixel(int bitsPerPixel) { return bitsPerPixel / 8; } -static bool png_conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) { - // Ensure the alpha type is valid - if (!valid_alpha(dst.alphaType(), src.alphaType())) { - return false; - } - - // Check for supported color types - switch (dst.colorType()) { - case kRGBA_8888_SkColorType: - case kBGRA_8888_SkColorType: - case kRGBA_F16_SkColorType: - return true; - case kRGB_565_SkColorType: - return kOpaque_SkAlphaType == src.alphaType(); - default: - return dst.colorType() == src.colorType(); - } -} - void SkPngCodec::allocateStorage(const SkImageInfo& dstInfo) { switch (fXformMode) { case kSwizzleOnly_XformMode: @@ -422,7 +403,7 @@ public: Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options, SkPMColor ctable[], int* ctableCount) override { - if (!png_conversion_possible(dstInfo, this->getInfo()) || + if (!conversion_possible(dstInfo, this->getInfo()) || !this->initializeXforms(dstInfo, options, ctable, ctableCount)) { return kInvalidConversion; @@ -489,7 +470,7 @@ public: Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options, SkPMColor ctable[], int* ctableCount) override { - if (!png_conversion_possible(dstInfo, this->getInfo()) || + if (!conversion_possible(dstInfo, this->getInfo()) || !this->initializeXforms(dstInfo, options, ctable, ctableCount)) { return kInvalidConversion; @@ -807,20 +788,10 @@ bool SkPngCodec::initializeXforms(const SkImageInfo& dstInfo, const Options& opt fSwizzler.reset(nullptr); fColorXform = nullptr; - bool needsColorXform = needs_color_xform(dstInfo, this->getInfo()); - if (needsColorXform) { - if (kGray_8_SkColorType == dstInfo.colorType() || - kRGB_565_SkColorType == dstInfo.colorType()) - { - return false; - } - + if (needs_color_xform(dstInfo, this->getInfo())) { fColorXform = SkColorSpaceXform::New(sk_ref_sp(this->getInfo().colorSpace()), sk_ref_sp(dstInfo.colorSpace())); - - if (!fColorXform && kRGBA_F16_SkColorType == dstInfo.colorType()) { - return false; - } + SkASSERT(fColorXform); } // If the image is RGBA and we have a color xform, we can skip the swizzler. @@ -907,7 +878,7 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, const Options& options, SkPMColor ctable[], int* ctableCount, int* rowsDecoded) { - if (!png_conversion_possible(dstInfo, this->getInfo()) || + if (!conversion_possible(dstInfo, this->getInfo()) || !this->initializeXforms(dstInfo, options, ctable, ctableCount)) { return kInvalidConversion; diff --git a/src/codec/SkRawCodec.cpp b/src/codec/SkRawCodec.cpp index 37d2d358ae..2a6a48fdbb 100644 --- a/src/codec/SkRawCodec.cpp +++ b/src/codec/SkRawCodec.cpp @@ -683,7 +683,7 @@ SkCodec::Result SkRawCodec::onGetPixels(const SkImageInfo& requestedInfo, void* size_t dstRowBytes, const Options& options, SkPMColor ctable[], int* ctableCount, int* rowsDecoded) { - if (!conversion_possible(requestedInfo, this->getInfo())) { + if (!conversion_possible_ignore_color_space(requestedInfo, this->getInfo())) { SkCodecPrintf("Error: cannot convert input type to output type.\n"); return kInvalidConversion; } diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp index 0c3aa402bd..e8b27b2178 100644 --- a/src/codec/SkWebpCodec.cpp +++ b/src/codec/SkWebpCodec.cpp @@ -144,25 +144,6 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) { streamDeleter.release(), demux.release(), std::move(data)); } -static bool webp_conversion_possible(const SkImageInfo& dst, const SkImageInfo& src, - SkColorSpaceXform* colorXform) { - if (!valid_alpha(dst.alphaType(), src.alphaType())) { - return false; - } - - switch (dst.colorType()) { - case kRGBA_F16_SkColorType: - return nullptr != colorXform; - case kBGRA_8888_SkColorType: - case kRGBA_8888_SkColorType: - return true; - case kRGB_565_SkColorType: - return nullptr == colorXform && src.alphaType() == kOpaque_SkAlphaType; - default: - return false; - } -} - SkISize SkWebpCodec::onGetScaledDimensions(float desiredScale) const { SkISize dim = this->getInfo().dimensions(); // SkCodec treats zero dimensional images as errors, so the minimum size @@ -212,15 +193,15 @@ bool SkWebpCodec::onGetValidSubset(SkIRect* desiredSubset) const { SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, const Options& options, SkPMColor*, int*, int* rowsDecodedPtr) { + if (!conversion_possible(dstInfo, this->getInfo())) { + return kInvalidConversion; + } std::unique_ptr colorXform = nullptr; if (needs_color_xform(dstInfo, this->getInfo())) { colorXform = SkColorSpaceXform::New(sk_ref_sp(this->getInfo().colorSpace()), sk_ref_sp(dstInfo.colorSpace())); - } - - if (!webp_conversion_possible(dstInfo, this->getInfo(), colorXform.get())) { - return kInvalidConversion; + SkASSERT(colorXform); } WebPDecoderConfig config; -- cgit v1.2.3