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>

5 files changed, 33 insertions, 18 deletions

diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 7e39d610c4..a0ad7875eb 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 56a1900220..5c8dc8747a 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 a70f7be14e..40339b52c6 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 f1601de189..8bab368cdf 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 f8d7b4e8d9..c020a0f8b5 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: