From 19aff5dd5cd83141f12c234c4255a35f63e564cd Mon Sep 17 00:00:00 2001 From: Matt Sarett Date: Mon, 3 Apr 2017 16:01:10 -0400 Subject: [PATCH] 565 codec color xform support: fix colortable / incomplete image behavior This fixes a bug that was exposed when I added color space support for 565 decodes. Before this CL, we would sometimes fill incomplete images with R and B swapped. This fixes that issue. Part of the fix is the decision to do 565 xforms when building the color table (then just swizzle to 565), rather than do them per pixel after swizzling. Bug: skia: Change-Id: I09e1ec75aba09a4e288015ea746465d0c3f7d59f Reviewed-on: https://skia-review.googlesource.com/11137 Reviewed-by: Mike Klein Commit-Queue: Matt Sarett --- src/codec/SkBmpStandardCodec.cpp | 4 ++-- src/codec/SkCodecPriv.h | 33 ++++++++++++++++++++++++++------- src/codec/SkGifCodec.cpp | 3 ++- src/codec/SkPngCodec.cpp | 5 +++-- src/core/SkColorSpaceXformPriv.h | 6 ------ 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp index 7e39d61..a0ad787 100644 --- a/src/codec/SkBmpStandardCodec.cpp +++ b/src/codec/SkBmpStandardCodec.cpp @@ -125,7 +125,7 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo, } if (this->colorXform() && !fXformOnDecode) { - SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstColorType); + SkColorSpaceXform::ColorFormat dstFormat = select_xform_format_ct(dstColorType); SkColorSpaceXform::ColorFormat srcFormat = SkColorSpaceXform::kBGRA_8888_ColorFormat; SkAlphaType xformAlphaType = select_xform_alpha(dstAlphaType, this->getInfo().alphaType()); @@ -357,7 +357,7 @@ uint64_t SkBmpStandardCodec::onGetFillValue(const SkImageInfo& dstInfo) const { const SkPMColor* colorPtr = get_color_ptr(fColorTable.get()); if (colorPtr) { return get_color_table_fill_value(dstInfo.colorType(), dstInfo.alphaType(), colorPtr, 0, - this->colorXform()); + this->colorXform(), false); } return INHERITED::onGetFillValue(dstInfo); } diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h index 56a1900..5c8dc87 100644 --- a/src/codec/SkCodecPriv.h +++ b/src/codec/SkCodecPriv.h @@ -120,7 +120,7 @@ static inline const SkPMColor* get_color_ptr(SkColorTable* colorTable) { * Given that the encoded image uses a color table, return the fill value */ static inline uint64_t get_color_table_fill_value(SkColorType dstColorType, SkAlphaType alphaType, - const SkPMColor* colorPtr, uint8_t fillIndex, SkColorSpaceXform* colorXform) { + const SkPMColor* colorPtr, uint8_t fillIndex, SkColorSpaceXform* colorXform, bool isRGBA) { SkASSERT(nullptr != colorPtr); switch (dstColorType) { case kRGBA_8888_SkColorType: @@ -134,8 +134,11 @@ static inline uint64_t get_color_table_fill_value(SkColorType dstColorType, SkAl SkASSERT(colorXform); uint64_t dstColor; uint32_t srcColor = colorPtr[fillIndex]; + SkColorSpaceXform::ColorFormat srcFormat = + isRGBA ? SkColorSpaceXform::kRGBA_8888_ColorFormat + : SkColorSpaceXform::kBGRA_8888_ColorFormat; SkAssertResult(colorXform->apply(select_xform_format(dstColorType), &dstColor, - SkColorSpaceXform::kRGBA_8888_ColorFormat, &srcColor, 1, alphaType)); + srcFormat, &srcColor, 1, alphaType)); return dstColor; } default: @@ -321,11 +324,8 @@ static inline SkAlphaType select_xform_alpha(SkAlphaType dstAlphaType, SkAlphaTy } static inline bool apply_xform_on_decode(SkColorType dstColorType, SkEncodedInfo::Color srcColor) { - // We will apply the color xform when reading the color table if a form of 8888 is requested. - return SkEncodedInfo::kPalette_Color != srcColor || - (kRGBA_8888_SkColorType != dstColorType && - kBGRA_8888_SkColorType != dstColorType && - kIndex_8_SkColorType != dstColorType); + // We will apply the color xform when reading the color table unless F16 is requested. + return SkEncodedInfo::kPalette_Color != srcColor || kRGBA_F16_SkColorType == dstColorType; } /* @@ -365,4 +365,23 @@ static inline bool conversion_possible(const SkImageInfo& dst, const SkImageInfo } } +static inline SkColorSpaceXform::ColorFormat select_xform_format_ct(SkColorType colorType) { + switch (colorType) { + case kRGBA_8888_SkColorType: + return SkColorSpaceXform::kRGBA_8888_ColorFormat; + case kBGRA_8888_SkColorType: + return SkColorSpaceXform::kBGRA_8888_ColorFormat; + case kRGB_565_SkColorType: + case kIndex_8_SkColorType: +#ifdef SK_PMCOLOR_IS_RGBA + return SkColorSpaceXform::kRGBA_8888_ColorFormat; +#else + return SkColorSpaceXform::kBGRA_8888_ColorFormat; +#endif + default: + SkASSERT(false); + return SkColorSpaceXform::kRGBA_8888_ColorFormat; + } +} + #endif // SkCodecPriv_DEFINED diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp index a70f7be..40339b5 100644 --- a/src/codec/SkGifCodec.cpp +++ b/src/codec/SkGifCodec.cpp @@ -166,7 +166,8 @@ void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, size_t frameIn fCurrColorTable.reset(new SkColorTable(&color, 1)); } else if (this->colorXform() && !fXformOnDecode) { SkPMColor dstColors[256]; - const SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType()); + const SkColorSpaceXform::ColorFormat dstFormat = + select_xform_format_ct(dstInfo.colorType()); const SkColorSpaceXform::ColorFormat srcFormat = select_xform_format(kXformSrcColorType); const SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(), this->getInfo().alphaType()); diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp index f1601de..8bab368 100644 --- a/src/codec/SkPngCodec.cpp +++ b/src/codec/SkPngCodec.cpp @@ -269,7 +269,8 @@ bool SkPngCodec::createColorTable(const SkImageInfo& dstInfo, int* ctableCount) if (this->colorXform() && !apply_xform_on_decode(dstInfo.colorType(), this->getEncodedInfo().color())) { - const SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType()); + const SkColorSpaceXform::ColorFormat dstFormat = + select_xform_format_ct(dstInfo.colorType()); const SkColorSpaceXform::ColorFormat srcFormat = select_xform_format(kXformSrcColorType); const SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(), this->getInfo().alphaType()); @@ -1277,7 +1278,7 @@ uint64_t SkPngCodec::onGetFillValue(const SkImageInfo& dstInfo) const { SkAlphaType alphaType = select_xform_alpha(dstInfo.alphaType(), this->getInfo().alphaType()); return get_color_table_fill_value(dstInfo.colorType(), alphaType, colorPtr, 0, - this->colorXform()); + this->colorXform(), true); } return INHERITED::onGetFillValue(dstInfo); } diff --git a/src/core/SkColorSpaceXformPriv.h b/src/core/SkColorSpaceXformPriv.h index f8d7b4e..c020a0f 100644 --- a/src/core/SkColorSpaceXformPriv.h +++ b/src/core/SkColorSpaceXformPriv.h @@ -90,12 +90,6 @@ static inline SkColorSpaceXform::ColorFormat select_xform_format(SkColorType col return SkColorSpaceXform::kBGRA_8888_ColorFormat; case kRGBA_F16_SkColorType: return SkColorSpaceXform::kRGBA_F16_ColorFormat; - case kIndex_8_SkColorType: -#ifdef SK_PMCOLOR_IS_RGBA - return SkColorSpaceXform::kRGBA_8888_ColorFormat; -#else - return SkColorSpaceXform::kBGRA_8888_ColorFormat; -#endif case kRGB_565_SkColorType: return SkColorSpaceXform::kBGR_565_ColorFormat; default: -- 2.7.4