From 1793e7bb46c1f9d430c1a699a1c3d3168159b659 Mon Sep 17 00:00:00 2001 From: Leon Scroggins Date: Tue, 5 Dec 2017 15:38:14 +0000 Subject: Revert "Hide SkEncodedInfo" This reverts commit c6f7a4ffa9522159efc42f7c948bba5e66bb8844. Reason for revert: Causing differences in Gold, stemming from the fact that this changes the recommended SkImageInfo for 16 bits-per-component PNG from N32 to F16. - an F16 bitmap already png-encodes to a 16 bits-per-component PNG, but it does not encode a linear colorspace (possibly a bug?). when we decode this PNG using getInfo(), it fails because it has an F16 color type and non-linear colorspace. (In the encode-srgb-png gm, this results in blank results for F16.) We could correct this on the encoder side, but it seems possible that a 16 bits-per-component PNG could be encoded with a different color space. In that case, we'd want SkCodec to recommend F16/SRGBLinear, but I think we'd want the SkCodec to store the encoded SkColorSpace so that we can Xform between the two. Currently SkCodec only stores one color space, so that will require a refactor. - When decoding 16-bits-per-component PNGs, we are now decoding them to F16. This shows differences in Gold. The srgb/gpu results now look more like F16. I think this is fine. Original change's description: > 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, use it to make the same decision that Android > would have made - 16 bits per component means to set the info to F16. Add > a test that computeOutputColorType behaves as expected. > > 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. > > Change-Id: Ie2cf11339bf999ebfd4390c0f448f7edd6feabda > Reviewed-on: https://skia-review.googlesource.com/79260 > Reviewed-by: Mike Reed > Reviewed-by: Mike Klein > Commit-Queue: Leon Scroggins TBR=mtklein@chromium.org,scroggo@google.com,reed@google.com Change-Id: I0c5dd1461e1b70d1e55349a8e7ee6b029c3f556e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:7353, skia:6839 Reviewed-on: https://skia-review.googlesource.com/80660 Reviewed-by: Leon Scroggins Commit-Queue: Leon Scroggins --- include/codec/SkAndroidCodec.h | 2 + include/codec/SkCodec.h | 10 +- include/codec/SkEncodedInfo.h | 201 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 5 deletions(-) create mode 100644 include/codec/SkEncodedInfo.h (limited to 'include/codec') diff --git a/include/codec/SkAndroidCodec.h b/include/codec/SkAndroidCodec.h index 0d6cc185d9..8dfa8ba5fe 100644 --- a/include/codec/SkAndroidCodec.h +++ b/include/codec/SkAndroidCodec.h @@ -43,6 +43,8 @@ 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 6a6a597f1f..d3035a7a71 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,6 +169,8 @@ 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. @@ -610,7 +612,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. */ - SkAlphaType fAlphaType; + SkEncodedInfo::Alpha fAlpha; /** * How this frame should be modified before decoding the next one. @@ -660,8 +662,6 @@ 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, SkColorType srcColor, + virtual bool conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor, bool srcIsOpaque, const SkColorSpace* srcCS) const; /** * Return whether these dimensions are supported as a scale. diff --git a/include/codec/SkEncodedInfo.h b/include/codec/SkEncodedInfo.h new file mode 100644 index 0000000000..dce88cadc1 --- /dev/null +++ b/include/codec/SkEncodedInfo.h @@ -0,0 +1,201 @@ +/* + * 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 -- cgit v1.2.3