From c8037dc5edda42cacd839df4e1c7d8cfd0953309 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Tue, 5 Dec 2017 13:55:24 -0500 Subject: Reland "Hide SkEncodedInfo" This partially reverts commit 1793e7bb46c1f9d430c1a699a1c3d3168159b659. Hide SkEncodedInfo Bug: skia:7353 Bug: skia:6839 This contains information that is not necessary for clients to know. The Color enum tells the number of components in the input, but this is only interesting internally (to the SkSwizzler). Similarly, the Alpha enum differs from SkAlphaType in that it has kBinary instead of kPremul. This is useful information only internally for determining whether the SkColorSpaceXform needs to premultiply. The bitsPerComponent is potentially useful for a client; Android (in SkAndroidCodec) uses it to determine the SkColorType. Rather than exposing bitsPerComponent, make SkAndroidCodec a friend so it can access the SkEncodedInfo. A future change will change SkCodec to recommend F16 for bitsPerComponent > 8, but that will be more involved; it was the reason for the revert of this CL. Switch conversionSupported to use an SkColorType, which is enough info. Replace the SkEncodedInfo::Alpha field on SkCodec::FrameInfo with an SkAlphaType. SkCodec still needs an SkEncodedInfo, so move its header (which is already not SK_API) to include/private. TBR=mtklein@chromium.org,reed@google.com Change-Id: I928b1f55317602cb37d29da63b53026c8d139cee Reviewed-on: https://skia-review.googlesource.com/80860 Reviewed-by: Leon Scroggins Commit-Queue: Leon Scroggins --- dm/DM.cpp | 2 +- gn/tests.gni | 1 + include/codec/SkAndroidCodec.h | 2 - include/codec/SkCodec.h | 11 +- include/codec/SkEncodedInfo.h | 201 ----------------------------------- include/private/SkEncodedInfo.h | 172 ++++++++++++++++++++++++++++++ src/codec/SkAndroidCodec.cpp | 4 - src/codec/SkCodec.cpp | 16 +-- src/codec/SkFrameHolder.h | 10 +- src/codec/SkGifCodec.cpp | 4 +- src/codec/SkHeifCodec.h | 2 +- src/codec/SkIcoCodec.h | 2 +- src/codec/SkJpegCodec.h | 2 +- src/codec/SkWbmpCodec.cpp | 2 +- src/codec/SkWbmpCodec.h | 2 +- src/codec/SkWebpCodec.cpp | 7 +- src/codec/SkWebpCodec.h | 12 +-- tests/CodecAnimTest.cpp | 36 +++---- tests/CodecRecommendedTypeTest.cpp | 40 +++++++ third_party/gif/SkGifImageReader.cpp | 7 +- third_party/gif/SkGifImageReader.h | 2 +- 21 files changed, 271 insertions(+), 266 deletions(-) delete mode 100644 include/codec/SkEncodedInfo.h create mode 100644 include/private/SkEncodedInfo.h create mode 100644 tests/CodecRecommendedTypeTest.cpp diff --git a/dm/DM.cpp b/dm/DM.cpp index aa9cc10543..6fa81e7137 100644 --- a/dm/DM.cpp +++ b/dm/DM.cpp @@ -731,7 +731,7 @@ static void push_codec_srcs(Path path) { }; for (const char* brdExt : brdExts) { if (0 == strcmp(brdExt, ext)) { - bool gray = codec->getEncodedInfo().color() == SkEncodedInfo::kGray_Color; + bool gray = codec->getInfo().colorType() == kGray_8_SkColorType; push_brd_srcs(path, gray); break; } diff --git a/gn/tests.gni b/gn/tests.gni index eb20aaba0d..03e0a44fea 100644 --- a/gn/tests.gni +++ b/gn/tests.gni @@ -35,6 +35,7 @@ tests_sources = [ "$_tests/CodecAnimTest.cpp", "$_tests/CodecExactReadTest.cpp", "$_tests/CodecPartialTest.cpp", + "$_tests/CodecRecommendedTypeTest.cpp", "$_tests/CodecTest.cpp", "$_tests/ColorFilterTest.cpp", "$_tests/ColorMatrixTest.cpp", diff --git a/include/codec/SkAndroidCodec.h b/include/codec/SkAndroidCodec.h index 8dfa8ba5fe..0d6cc185d9 100644 --- a/include/codec/SkAndroidCodec.h +++ b/include/codec/SkAndroidCodec.h @@ -43,8 +43,6 @@ public: virtual ~SkAndroidCodec(); - const SkEncodedInfo& getEncodedInfo() const; - const SkImageInfo& getInfo() const { return fInfo; } /** diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index d3035a7a71..43f0176081 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -9,11 +9,11 @@ #define SkCodec_DEFINED #include "../private/SkTemplates.h" +#include "../private/SkEncodedInfo.h" #include "SkCodecAnimation.h" #include "SkColor.h" #include "SkColorSpaceXform.h" #include "SkEncodedImageFormat.h" -#include "SkEncodedInfo.h" #include "SkEncodedOrigin.h" #include "SkImageInfo.h" #include "SkPixmap.h" @@ -169,8 +169,6 @@ public: */ const SkImageInfo& getInfo() const { return fSrcInfo; } - const SkEncodedInfo& getEncodedInfo() const { return fEncodedInfo; } - /** * Returns the image orientation stored in the EXIF data. * If there is no EXIF data, or if we cannot read the EXIF data, returns kTopLeft. @@ -612,7 +610,7 @@ public: * This is conservative; it will still return non-opaque if e.g. a * color index-based frame has a color with alpha but does not use it. */ - SkEncodedInfo::Alpha fAlpha; + SkAlphaType fAlphaType; /** * How this frame should be modified before decoding the next one. @@ -662,6 +660,8 @@ public: } protected: + const SkEncodedInfo& getEncodedInfo() const { return fEncodedInfo; } + using XformFormat = SkColorSpaceXform::ColorFormat; SkCodec(int width, @@ -847,7 +847,7 @@ private: * * Will be called for the appropriate frame, prior to initializing the colorXform. */ - virtual bool conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor, + virtual bool conversionSupported(const SkImageInfo& dst, SkColorType srcColor, bool srcIsOpaque, const SkColorSpace* srcCS) const; /** * Return whether these dimensions are supported as a scale. @@ -924,5 +924,6 @@ private: friend class DM::CodecSrc; // for fillIncompleteImage friend class SkSampledCodec; friend class SkIcoCodec; + friend class SkAndroidCodec; // for fEncodedInfo }; #endif // SkCodec_DEFINED diff --git a/include/codec/SkEncodedInfo.h b/include/codec/SkEncodedInfo.h deleted file mode 100644 index dce88cadc1..0000000000 --- a/include/codec/SkEncodedInfo.h +++ /dev/null @@ -1,201 +0,0 @@ -/* - * Copyright 2016 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SkEncodedInfo_DEFINED -#define SkEncodedInfo_DEFINED - -#include "SkImageInfo.h" - -class SkColorSpace; - -struct SkEncodedInfo { -public: - - enum Alpha { - kOpaque_Alpha, - kUnpremul_Alpha, - - // Each pixel is either fully opaque or fully transparent. - // There is no difference between requesting kPremul or kUnpremul. - kBinary_Alpha, - }; - - /* - * We strive to make the number of components per pixel obvious through - * our naming conventions. - * Ex: kRGB has 3 components. kRGBA has 4 components. - * - * This sometimes results in redundant Alpha and Color information. - * Ex: kRGB images must also be kOpaque. - */ - enum Color { - // PNG, WBMP - kGray_Color, - - // PNG - kGrayAlpha_Color, - - // PNG, GIF, BMP - kPalette_Color, - - // PNG, RAW - kRGB_Color, - kRGBA_Color, - - // BMP - kBGR_Color, - kBGRX_Color, - kBGRA_Color, - - // JPEG, WEBP - kYUV_Color, - - // WEBP - kYUVA_Color, - - // JPEG - // Photoshop actually writes inverted CMYK data into JPEGs, where zero - // represents 100% ink coverage. For this reason, we treat CMYK JPEGs - // as having inverted CMYK. libjpeg-turbo warns that this may break - // other applications, but the CMYK JPEGs we see on the web expect to - // be treated as inverted CMYK. - kInvertedCMYK_Color, - kYCCK_Color, - }; - - static SkEncodedInfo Make(Color color, Alpha alpha, int bitsPerComponent) { - SkASSERT(1 == bitsPerComponent || - 2 == bitsPerComponent || - 4 == bitsPerComponent || - 8 == bitsPerComponent || - 16 == bitsPerComponent); - - switch (color) { - case kGray_Color: - SkASSERT(kOpaque_Alpha == alpha); - break; - case kGrayAlpha_Color: - SkASSERT(kOpaque_Alpha != alpha); - break; - case kPalette_Color: - SkASSERT(16 != bitsPerComponent); - break; - case kRGB_Color: - case kBGR_Color: - case kBGRX_Color: - SkASSERT(kOpaque_Alpha == alpha); - SkASSERT(bitsPerComponent >= 8); - break; - case kYUV_Color: - case kInvertedCMYK_Color: - case kYCCK_Color: - SkASSERT(kOpaque_Alpha == alpha); - SkASSERT(8 == bitsPerComponent); - break; - case kRGBA_Color: - SkASSERT(kOpaque_Alpha != alpha); - SkASSERT(bitsPerComponent >= 8); - break; - case kBGRA_Color: - case kYUVA_Color: - SkASSERT(kOpaque_Alpha != alpha); - SkASSERT(8 == bitsPerComponent); - break; - default: - SkASSERT(false); - break; - } - - return SkEncodedInfo(color, alpha, bitsPerComponent); - } - - /* - * Returns an SkImageInfo with Skia color and alpha types that are the - * closest possible match to the encoded info. - */ - SkImageInfo makeImageInfo(int width, int height, sk_sp colorSpace) const { - switch (fColor) { - case kGray_Color: - SkASSERT(kOpaque_Alpha == fAlpha); - return SkImageInfo::Make(width, height, kGray_8_SkColorType, - kOpaque_SkAlphaType, colorSpace); - case kGrayAlpha_Color: - SkASSERT(kOpaque_Alpha != fAlpha); - return SkImageInfo::Make(width, height, kN32_SkColorType, - kUnpremul_SkAlphaType, colorSpace); - case kPalette_Color: { - SkAlphaType alphaType = (kOpaque_Alpha == fAlpha) ? kOpaque_SkAlphaType : - kUnpremul_SkAlphaType; - return SkImageInfo::Make(width, height, kN32_SkColorType, - alphaType, colorSpace); - } - case kRGB_Color: - case kBGR_Color: - case kBGRX_Color: - case kYUV_Color: - case kInvertedCMYK_Color: - case kYCCK_Color: - SkASSERT(kOpaque_Alpha == fAlpha); - return SkImageInfo::Make(width, height, kN32_SkColorType, - kOpaque_SkAlphaType, colorSpace); - case kRGBA_Color: - case kBGRA_Color: - case kYUVA_Color: - SkASSERT(kOpaque_Alpha != fAlpha); - return SkImageInfo::Make(width, height, kN32_SkColorType, - kUnpremul_SkAlphaType, std::move(colorSpace)); - default: - SkASSERT(false); - return SkImageInfo::MakeUnknown(); - } - } - - Color color() const { return fColor; } - Alpha alpha() const { return fAlpha; } - bool opaque() const { return fAlpha == kOpaque_Alpha; } - - uint8_t bitsPerComponent() const { return fBitsPerComponent; } - - uint8_t bitsPerPixel() const { - switch (fColor) { - case kGray_Color: - return fBitsPerComponent; - case kGrayAlpha_Color: - return 2 * fBitsPerComponent; - case kPalette_Color: - return fBitsPerComponent; - case kRGB_Color: - case kBGR_Color: - case kYUV_Color: - return 3 * fBitsPerComponent; - case kRGBA_Color: - case kBGRA_Color: - case kBGRX_Color: - case kYUVA_Color: - case kInvertedCMYK_Color: - case kYCCK_Color: - return 4 * fBitsPerComponent; - default: - SkASSERT(false); - return 0; - } - } - -private: - - SkEncodedInfo(Color color, Alpha alpha, uint8_t bitsPerComponent) - : fColor(color) - , fAlpha(alpha) - , fBitsPerComponent(bitsPerComponent) - {} - - Color fColor; - Alpha fAlpha; - uint8_t fBitsPerComponent; -}; - -#endif diff --git a/include/private/SkEncodedInfo.h b/include/private/SkEncodedInfo.h new file mode 100644 index 0000000000..217b859ed8 --- /dev/null +++ b/include/private/SkEncodedInfo.h @@ -0,0 +1,172 @@ +/* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkEncodedInfo_DEFINED +#define SkEncodedInfo_DEFINED + +#include "SkImageInfo.h" + +class SkColorSpace; + +struct SkEncodedInfo { +public: + + enum Alpha { + kOpaque_Alpha, + kUnpremul_Alpha, + + // Each pixel is either fully opaque or fully transparent. + // There is no difference between requesting kPremul or kUnpremul. + kBinary_Alpha, + }; + + /* + * We strive to make the number of components per pixel obvious through + * our naming conventions. + * Ex: kRGB has 3 components. kRGBA has 4 components. + * + * This sometimes results in redundant Alpha and Color information. + * Ex: kRGB images must also be kOpaque. + */ + enum Color { + // PNG, WBMP + kGray_Color, + + // PNG + kGrayAlpha_Color, + + // PNG, GIF, BMP + kPalette_Color, + + // PNG, RAW + kRGB_Color, + kRGBA_Color, + + // BMP + kBGR_Color, + kBGRX_Color, + kBGRA_Color, + + // JPEG, WEBP + kYUV_Color, + + // WEBP + kYUVA_Color, + + // JPEG + // Photoshop actually writes inverted CMYK data into JPEGs, where zero + // represents 100% ink coverage. For this reason, we treat CMYK JPEGs + // as having inverted CMYK. libjpeg-turbo warns that this may break + // other applications, but the CMYK JPEGs we see on the web expect to + // be treated as inverted CMYK. + kInvertedCMYK_Color, + kYCCK_Color, + }; + + static SkEncodedInfo Make(Color color, Alpha alpha, int bitsPerComponent) { + SkASSERT(1 == bitsPerComponent || + 2 == bitsPerComponent || + 4 == bitsPerComponent || + 8 == bitsPerComponent || + 16 == bitsPerComponent); + + switch (color) { + case kGray_Color: + SkASSERT(kOpaque_Alpha == alpha); + break; + case kGrayAlpha_Color: + SkASSERT(kOpaque_Alpha != alpha); + break; + case kPalette_Color: + SkASSERT(16 != bitsPerComponent); + break; + case kRGB_Color: + case kBGR_Color: + case kBGRX_Color: + SkASSERT(kOpaque_Alpha == alpha); + SkASSERT(bitsPerComponent >= 8); + break; + case kYUV_Color: + case kInvertedCMYK_Color: + case kYCCK_Color: + SkASSERT(kOpaque_Alpha == alpha); + SkASSERT(8 == bitsPerComponent); + break; + case kRGBA_Color: + SkASSERT(kOpaque_Alpha != alpha); + SkASSERT(bitsPerComponent >= 8); + break; + case kBGRA_Color: + case kYUVA_Color: + SkASSERT(kOpaque_Alpha != alpha); + SkASSERT(8 == bitsPerComponent); + break; + default: + SkASSERT(false); + break; + } + + return SkEncodedInfo(color, alpha, bitsPerComponent); + } + + /* + * Returns an SkImageInfo with Skia color and alpha types that are the + * closest possible match to the encoded info. + */ + SkImageInfo makeImageInfo(int width, int height, sk_sp colorSpace) const { + auto ct = kGray_Color == fColor ? kGray_8_SkColorType : + kN32_SkColorType ; + auto alpha = kOpaque_Alpha == fAlpha ? kOpaque_SkAlphaType + : kUnpremul_SkAlphaType; + return SkImageInfo::Make(width, height, ct, alpha, std::move(colorSpace)); + } + + Color color() const { return fColor; } + Alpha alpha() const { return fAlpha; } + bool opaque() const { return fAlpha == kOpaque_Alpha; } + + uint8_t bitsPerComponent() const { return fBitsPerComponent; } + + uint8_t bitsPerPixel() const { + switch (fColor) { + case kGray_Color: + return fBitsPerComponent; + case kGrayAlpha_Color: + return 2 * fBitsPerComponent; + case kPalette_Color: + return fBitsPerComponent; + case kRGB_Color: + case kBGR_Color: + case kYUV_Color: + return 3 * fBitsPerComponent; + case kRGBA_Color: + case kBGRA_Color: + case kBGRX_Color: + case kYUVA_Color: + case kInvertedCMYK_Color: + case kYCCK_Color: + return 4 * fBitsPerComponent; + default: + SkASSERT(false); + return 0; + } + } + +private: + + SkEncodedInfo(Color color, Alpha alpha, uint8_t bitsPerComponent) + : fColor(color) + , fAlpha(alpha) + , fBitsPerComponent(bitsPerComponent) + {} + + Color fColor; + Alpha fAlpha; + uint8_t fBitsPerComponent; +}; + +#endif diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp index cd23680932..a694b4b916 100644 --- a/src/codec/SkAndroidCodec.cpp +++ b/src/codec/SkAndroidCodec.cpp @@ -64,10 +64,6 @@ SkAndroidCodec::SkAndroidCodec(SkCodec* codec) SkAndroidCodec::~SkAndroidCodec() {} -const SkEncodedInfo& SkAndroidCodec::getEncodedInfo() const { - return fCodec->getEncodedInfo(); -} - std::unique_ptr SkAndroidCodec::MakeFromStream(std::unique_ptr stream, SkPngChunkReader* chunkReader) { auto codec = SkCodec::MakeFromStream(std::move(stream), nullptr, chunkReader); if (nullptr == codec) { diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp index a0f1fb2706..7de083db3c 100644 --- a/src/codec/SkCodec.cpp +++ b/src/codec/SkCodec.cpp @@ -158,7 +158,7 @@ SkCodec::SkCodec(const SkEncodedInfo& info, const SkImageInfo& imageInfo, SkCodec::~SkCodec() {} -bool SkCodec::conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor, +bool SkCodec::conversionSupported(const SkImageInfo& dst, SkColorType srcColor, bool srcIsOpaque, const SkColorSpace* srcCS) const { if (!valid_alpha(dst.alphaType(), srcIsOpaque)) { return false; @@ -173,7 +173,7 @@ bool SkCodec::conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color s case kRGB_565_SkColorType: return srcIsOpaque; case kGray_8_SkColorType: - return SkEncodedInfo::kGray_Color == srcColor && srcIsOpaque && + return kGray_8_SkColorType == srcColor && srcIsOpaque && !needs_color_xform(dst, srcCS, false); case kAlpha_8_SkColorType: // conceptually we can convert anything into alpha_8, but we haven't actually coded @@ -224,7 +224,7 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, const Options& options) { const int index = options.fFrameIndex; if (0 == index) { - if (!this->conversionSupported(info, fEncodedInfo.color(), fEncodedInfo.opaque(), + if (!this->conversionSupported(info, fSrcInfo.colorType(), fEncodedInfo.opaque(), fSrcInfo.colorSpace()) || !this->initializeColorXform(info, fEncodedInfo.alpha(), options.fPremulBehavior)) { @@ -253,7 +253,7 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, const auto* frame = frameHolder->getFrame(index); SkASSERT(frame); - if (!this->conversionSupported(info, fEncodedInfo.color(), !frame->hasAlpha(), + if (!this->conversionSupported(info, fSrcInfo.colorType(), !frame->hasAlpha(), fSrcInfo.colorSpace())) { return kInvalidConversion; } @@ -297,9 +297,7 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, } } - FrameInfo frameInfo; - SkAssertResult(this->getFrameInfo(index, &frameInfo)); - return this->initializeColorXform(info, frameInfo.fAlpha, options.fPremulBehavior) + return this->initializeColorXform(info, frame->reportedAlpha(), options.fPremulBehavior) ? kSuccess : kInvalidConversion; } @@ -630,6 +628,10 @@ bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Al if (!this->usesColorXform()) { return true; } + // FIXME: In SkWebpCodec, if a frame is blending with a prior frame, we don't need + // a colorXform to do a color correct premul, since the blend step will handle + // premultiplication. But there is no way to know whether we need to blend from + // inside this call. bool needsColorCorrectPremul = needs_premul(dstInfo.alphaType(), encodedAlpha) && SkTransferFunctionBehavior::kRespect == premulBehavior; if (needs_color_xform(dstInfo, fSrcInfo.colorSpace(), needsColorCorrectPremul)) { diff --git a/src/codec/SkFrameHolder.h b/src/codec/SkFrameHolder.h index 29539ff908..ab308b39db 100644 --- a/src/codec/SkFrameHolder.h +++ b/src/codec/SkFrameHolder.h @@ -40,14 +40,14 @@ public: int frameId() const { return fId; } /** - * Whether this frame reports alpha. + * How this frame reports its alpha. * * This only considers the rectangle of this frame, and * considers it to have alpha even if it is opaque once * blended with the frame behind it. */ - bool reportsAlpha() const { - return this->onReportsAlpha(); + SkEncodedInfo::Alpha reportedAlpha() const { + return this->onReportedAlpha(); } /** @@ -130,7 +130,7 @@ public: } protected: - virtual bool onReportsAlpha() const = 0; + virtual SkEncodedInfo::Alpha onReportedAlpha() const = 0; private: static constexpr int kUninitialized = -2; @@ -166,7 +166,7 @@ public: /** * Compute the opacity and required frame, based on - * whether the frame reportsAlpha and how it blends + * the frame's reportedAlpha and how it blends * with prior frames. */ void setAlphaAndRequiredFrame(SkFrame*); diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp index 17c8617500..dff8136d75 100644 --- a/src/codec/SkGifCodec.cpp +++ b/src/codec/SkGifCodec.cpp @@ -144,8 +144,8 @@ bool SkGifCodec::onGetFrameInfo(int i, SkCodec::FrameInfo* frameInfo) const { frameInfo->fDuration = frameContext->getDuration(); frameInfo->fRequiredFrame = frameContext->getRequiredFrame(); frameInfo->fFullyReceived = frameContext->isComplete(); - frameInfo->fAlpha = frameContext->hasAlpha() ? SkEncodedInfo::kBinary_Alpha - : SkEncodedInfo::kOpaque_Alpha; + frameInfo->fAlphaType = frameContext->hasAlpha() ? kUnpremul_SkAlphaType + : kOpaque_SkAlphaType; frameInfo->fDisposalMethod = frameContext->getDisposalMethod(); } return true; diff --git a/src/codec/SkHeifCodec.h b/src/codec/SkHeifCodec.h index 6eb8e9c3b8..20bec6539e 100644 --- a/src/codec/SkHeifCodec.h +++ b/src/codec/SkHeifCodec.h @@ -47,7 +47,7 @@ protected: return SkEncodedImageFormat::kHEIF; } - bool conversionSupported(const SkImageInfo&, SkEncodedInfo::Color, bool, + bool conversionSupported(const SkImageInfo&, SkColorType, bool, const SkColorSpace*) const override { // This class checks for conversion after creating colorXform. return true; diff --git a/src/codec/SkIcoCodec.h b/src/codec/SkIcoCodec.h index 06ddeba9b4..c43fcf8cc0 100644 --- a/src/codec/SkIcoCodec.h +++ b/src/codec/SkIcoCodec.h @@ -48,7 +48,7 @@ protected: SkScanlineOrder onGetScanlineOrder() const override; - bool conversionSupported(const SkImageInfo&, SkEncodedInfo::Color, bool, + bool conversionSupported(const SkImageInfo&, SkColorType, bool, const SkColorSpace*) const override { // This will be checked by the embedded codec. return true; diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h index 41814d2ead..7fab209b6e 100644 --- a/src/codec/SkJpegCodec.h +++ b/src/codec/SkJpegCodec.h @@ -58,7 +58,7 @@ protected: bool onDimensionsSupported(const SkISize&) override; - bool conversionSupported(const SkImageInfo&, SkEncodedInfo::Color, bool, + bool conversionSupported(const SkImageInfo&, SkColorType, bool, const SkColorSpace*) const override { // This class checks for conversion after creating colorXform. return true; diff --git a/src/codec/SkWbmpCodec.cpp b/src/codec/SkWbmpCodec.cpp index 63380e109c..d8b10287f6 100644 --- a/src/codec/SkWbmpCodec.cpp +++ b/src/codec/SkWbmpCodec.cpp @@ -108,7 +108,7 @@ SkEncodedImageFormat SkWbmpCodec::onGetEncodedFormat() const { return SkEncodedImageFormat::kWBMP; } -bool SkWbmpCodec::conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor, +bool SkWbmpCodec::conversionSupported(const SkImageInfo& dst, SkColorType /*srcColor*/, bool srcIsOpaque, const SkColorSpace* srcCS) const { return valid_color_type(dst) && valid_alpha(dst.alphaType(), srcIsOpaque); } diff --git a/src/codec/SkWbmpCodec.h b/src/codec/SkWbmpCodec.h index 57efe21e3b..192189d1ec 100644 --- a/src/codec/SkWbmpCodec.h +++ b/src/codec/SkWbmpCodec.h @@ -28,7 +28,7 @@ protected: Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, int*) override; bool onRewind() override; - bool conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor, + bool conversionSupported(const SkImageInfo& dst, SkColorType srcColor, bool srcIsOpaque, const SkColorSpace* srcCS) const override; // No need to Xform; all pixels are either black or white. bool usesColorXform() const override { return false; } diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp index d5614d6213..2ff4681337 100644 --- a/src/codec/SkWebpCodec.cpp +++ b/src/codec/SkWebpCodec.cpp @@ -192,7 +192,8 @@ static WEBP_CSP_MODE webp_decode_mode(SkColorType dstCT, bool premultiply) { SkWebpCodec::Frame* SkWebpCodec::FrameHolder::appendNewFrame(bool hasAlpha) { const int i = this->size(); - fFrames.emplace_back(i, hasAlpha); + fFrames.emplace_back(i, hasAlpha ? SkEncodedInfo::kUnpremul_Alpha + : SkEncodedInfo::kOpaque_Alpha); return &fFrames[i]; } @@ -300,8 +301,8 @@ bool SkWebpCodec::onGetFrameInfo(int i, FrameInfo* frameInfo) const { // libwebp only reports fully received frames for an // animated image. frameInfo->fFullyReceived = true; - frameInfo->fAlpha = frame->hasAlpha() ? SkEncodedInfo::kUnpremul_Alpha - : SkEncodedInfo::kOpaque_Alpha; + frameInfo->fAlphaType = frame->hasAlpha() ? kUnpremul_SkAlphaType + : kOpaque_SkAlphaType; frameInfo->fDisposalMethod = frame->getDisposalMethod(); } diff --git a/src/codec/SkWebpCodec.h b/src/codec/SkWebpCodec.h index 134dafa3d5..010771bcff 100644 --- a/src/codec/SkWebpCodec.h +++ b/src/codec/SkWebpCodec.h @@ -58,22 +58,22 @@ private: class Frame : public SkFrame { public: - Frame(int i, bool alpha) + Frame(int i, SkEncodedInfo::Alpha alpha) : INHERITED(i) - , fReportsAlpha(alpha) + , fReportedAlpha(alpha) {} Frame(Frame&& other) : INHERITED(other.frameId()) - , fReportsAlpha(other.fReportsAlpha) + , fReportedAlpha(other.fReportedAlpha) {} protected: - bool onReportsAlpha() const override { - return fReportsAlpha; + SkEncodedInfo::Alpha onReportedAlpha() const override { + return fReportedAlpha; } private: - const bool fReportsAlpha; + const SkEncodedInfo::Alpha fReportedAlpha; typedef SkFrame INHERITED; }; diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp index e354ea6cbf..b274ee538a 100644 --- a/tests/CodecAnimTest.cpp +++ b/tests/CodecAnimTest.cpp @@ -67,9 +67,8 @@ static bool restore_previous(const SkCodec::FrameInfo& info) { } DEF_TEST(Codec_frames, r) { - #define kOpaque SkEncodedInfo::kOpaque_Alpha - #define kUnpremul SkEncodedInfo::kUnpremul_Alpha - #define kBinary SkEncodedInfo::kBinary_Alpha + #define kOpaque kOpaque_SkAlphaType + #define kUnpremul kUnpremul_SkAlphaType #define kKeep SkCodecAnimation::DisposalMethod::kKeep #define kRestoreBG SkCodecAnimation::DisposalMethod::kRestoreBGColor #define kRestorePrev SkCodecAnimation::DisposalMethod::kRestorePrevious @@ -79,8 +78,8 @@ DEF_TEST(Codec_frames, r) { // One less than fFramecount, since the first frame is always // independent. std::vector fRequiredFrames; - // Same, since the first frame should match getEncodedInfo - std::vector fAlphas; + // Same, since the first frame should match getInfo + std::vector fAlphas; // The size of this one should match fFrameCount for animated, empty // otherwise. std::vector fDurations; @@ -89,15 +88,15 @@ DEF_TEST(Codec_frames, r) { } gRecs[] = { { "required.gif", 7, { 0, 1, 2, 3, 4, 5 }, - { kOpaque, kBinary, kBinary, kBinary, kBinary, kBinary }, + { kOpaque, kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul }, { 100, 100, 100, 100, 100, 100, 100 }, 0, { kKeep, kRestoreBG, kKeep, kKeep, kKeep, kRestoreBG, kKeep } }, { "alphabetAnim.gif", 13, { SkCodec::kNone, 0, 0, 0, 0, 5, 6, SkCodec::kNone, SkCodec::kNone, 9, 10, 11 }, - { kBinary, kBinary, kBinary, kBinary, kBinary, kBinary, - kBinary, kBinary, kBinary, kBinary, kBinary, kBinary }, + { kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, + kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul }, { 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100 }, 0, { kKeep, kRestorePrev, kRestorePrev, kRestorePrev, kRestorePrev, @@ -116,8 +115,8 @@ DEF_TEST(Codec_frames, r) { { "randPixelsAnim.gif", 13, // required frames { 0, 1, 2, 3, 4, 3, 6, 7, 7, 7, 9, 9 }, - { kBinary, kBinary, kBinary, kBinary, kBinary, kBinary, - kBinary, kBinary, kBinary, kBinary, kBinary, kBinary }, + { kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, + kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul }, // durations { 0, 1000, 170, 40, 220, 7770, 90, 90, 90, 90, 90, 90, 90 }, // repetition count @@ -161,7 +160,6 @@ DEF_TEST(Codec_frames, r) { }; #undef kOpaque #undef kUnpremul - #undef kBinary #undef kKeep #undef kRestorePrev #undef kRestoreBG @@ -270,22 +268,20 @@ DEF_TEST(Codec_frames, r) { rec.fName, i, rec.fDurations[i], frameInfo.fDuration); } - auto to_string = [](SkEncodedInfo::Alpha alpha) { + auto to_string = [](SkAlphaType alpha) { switch (alpha) { - case SkEncodedInfo::kUnpremul_Alpha: + case kUnpremul_SkAlphaType: return "unpremul"; - case SkEncodedInfo::kOpaque_Alpha: + case kOpaque_SkAlphaType: return "opaque"; - case SkEncodedInfo::kBinary_Alpha: - return "binary"; default: SkASSERT(false); return "unknown"; } }; - auto expectedAlpha = 0 == i ? codec->getEncodedInfo().alpha() : rec.fAlphas[i-1]; - auto alpha = frameInfo.fAlpha; + auto expectedAlpha = 0 == i ? codec->getInfo().alphaType() : rec.fAlphas[i-1]; + auto alpha = frameInfo.fAlphaType; if (expectedAlpha != alpha) { ERRORF(r, "%s's frame %i has wrong alpha type! expected: %s\tactual: %s", rec.fName, i, to_string(expectedAlpha), to_string(alpha)); @@ -319,9 +315,7 @@ DEF_TEST(Codec_frames, r) { auto decode = [&](SkBitmap* bm, int index, int cachedIndex) { auto decodeInfo = info; if (index > 0) { - auto alphaType = frameInfos[index].fAlpha == SkEncodedInfo::kOpaque_Alpha - ? kOpaque_SkAlphaType : kPremul_SkAlphaType; - decodeInfo = info.makeAlphaType(alphaType); + decodeInfo = info.makeAlphaType(frameInfos[index].fAlphaType); } bm->allocPixels(decodeInfo); if (cachedIndex != SkCodec::kNone) { diff --git a/tests/CodecRecommendedTypeTest.cpp b/tests/CodecRecommendedTypeTest.cpp new file mode 100644 index 0000000000..180bf2f8da --- /dev/null +++ b/tests/CodecRecommendedTypeTest.cpp @@ -0,0 +1,40 @@ +/* + * Copyright 2017 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkAndroidCodec.h" +#include "SkBitmap.h" +#include "SkCodec.h" +#include "SkColorSpace.h" +#include "SkEncodedImageFormat.h" +#include "SkImageEncoder.h" +#include "SkImageInfo.h" +#include "SkStream.h" + +#include "Test.h" + +DEF_TEST(Codec_recommendedF16, r) { + // Encode an F16 bitmap. SkEncodeImage will encode this to a true-color PNG + // with a bit depth of 16. SkAndroidCodec should always recommend F16 for + // such a PNG. + SkBitmap bm; + bm.allocPixels(SkImageInfo::Make(10, 10, kRGBA_F16_SkColorType, + kPremul_SkAlphaType, SkColorSpace::MakeSRGBLinear())); + // What is drawn is not important. + bm.eraseColor(SK_ColorBLUE); + + SkDynamicMemoryWStream wstream; + REPORTER_ASSERT(r, SkEncodeImage(&wstream, bm, SkEncodedImageFormat::kPNG, 100)); + auto data = wstream.detachAsData(); + auto androidCodec = SkAndroidCodec::MakeFromData(std::move(data)); + if (!androidCodec) { + ERRORF(r, "Failed to create SkAndroidCodec"); + return; + } + + REPORTER_ASSERT(r, androidCodec->computeOutputColorType(kN32_SkColorType) + == kRGBA_F16_SkColorType); +} diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp index a54fb59d3b..a04f8e11d0 100644 --- a/third_party/gif/SkGifImageReader.cpp +++ b/third_party/gif/SkGifImageReader.cpp @@ -879,15 +879,16 @@ static bool restore_bg(const SkFrame& frame) { return frame.getDisposalMethod() == SkCodecAnimation::DisposalMethod::kRestoreBGColor; } -bool SkGIFFrameContext::onReportsAlpha() const { +SkEncodedInfo::Alpha SkGIFFrameContext::onReportedAlpha() const { // Note: We could correct these after decoding - i.e. some frames may turn out to be // independent and opaque if they do not use the transparent pixel, but that would require // checking whether each pixel used the transparent index. - return is_palette_index_valid(this->transparentPixel()); + return is_palette_index_valid(this->transparentPixel()) ? SkEncodedInfo::kBinary_Alpha + : SkEncodedInfo::kOpaque_Alpha; } void SkFrameHolder::setAlphaAndRequiredFrame(SkFrame* frame) { - const bool reportsAlpha = frame->reportsAlpha(); + const bool reportsAlpha = frame->reportedAlpha() != SkEncodedInfo::kOpaque_Alpha; const auto screenRect = SkIRect::MakeWH(fScreenWidth, fScreenHeight); const auto frameRect = frame_rect_on_screen(frame->frameRect(), screenRect); diff --git a/third_party/gif/SkGifImageReader.h b/third_party/gif/SkGifImageReader.h index b5eca10dfb..5d8597fccf 100644 --- a/third_party/gif/SkGifImageReader.h +++ b/third_party/gif/SkGifImageReader.h @@ -247,7 +247,7 @@ public: SkGIFColorMap& localColorMap() { return m_localColorMap; } protected: - bool onReportsAlpha() const override; + SkEncodedInfo::Alpha onReportedAlpha() const override; private: int m_transparentPixel; // Index of transparent pixel. Value is kNotFound if there is no transparent pixel. -- cgit v1.2.3