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 <mtklein@chromium.org>
Commit-Queue: Matt Sarett <msarett@google.com>

diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 7e39d610c4cde4c28150ed2ffc40166d9211b01b..a0ad7875ebf65b83b0dfbafbe7cee7941396bed6 100644 (file)
--- 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 56a1900220b308c4098b64e5e6604916986f12b5..5c8dc8747aa977ee62c3c7bf58339609c2c0d82a 100644 (file)
--- 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 a70f7be14eb55721b431711cd3592af6309befad..40339b52c603ab1e2e64235d9e018a7867ac4a4f 100644 (file)
--- 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 f1601de189bf1dc94030697de7c94b9bd83de013..8bab368cdf62f54e5f7dc97623f492cc79d72194 100644 (file)
--- 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 f8d7b4e8d9bd09ac51f28e4ea0b814bac7ae8f98..c020a0f8b55faec9c97fca4fa61e1e0b7fe58e5b 100644 (file)
--- 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: