Fix color xforms for Index8 bmps

Thanks to Gold for catching this.

BUG=skia:4895

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4426

Change-Id: Icc816d933e9e2fd312858a5430fa21a47722563e
Reviewed-on: https://skia-review.googlesource.com/4426
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>

diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 85b965e..b801309 100644 (file)
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -276,9 +276,16 @@ SkCodec::Result SkBmpRLECodec::onPrepareToDecode(const SkImageInfo& dstInfo,
     fSampleX = 1;
     fLinesToSkip = 0;
 
+    SkColorType colorTableColorType = dstInfo.colorType();
+    if (this->colorXform()) {
+        // Just set a known colorType for the colorTable.  No need to actually transform
+        // the colors in the colorTable since we do not allow decoding RLE to kIndex8.
+        colorTableColorType = kBGRA_8888_SkColorType;
+    }
+
     // Create the color table if necessary and prepare the stream for decode
     // Note that if it is non-NULL, inputColorCount will be modified
-    if (!this->createColorTable(dstInfo.colorType(), inputColorCount)) {
+    if (!this->createColorTable(colorTableColorType, inputColorCount)) {
         SkCodecPrintf("Error: could not create color table.\n");
         return SkCodec::kInvalidInput;
     }

diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 2a703fd..9b346f3 100644 (file)
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -25,10 +25,11 @@ SkBmpStandardCodec::SkBmpStandardCodec(int width, int height, const SkEncodedInf
     , fBytesPerColor(bytesPerColor)
     , fOffset(offset)
     , fSwizzler(nullptr)
-    , fSrcBuffer(new uint8_t [this->srcRowBytes()])
+    , fSrcBuffer(new uint8_t[this->srcRowBytes()])
     , fIsOpaque(isOpaque)
     , fInIco(inIco)
     , fAndMaskRowBytes(fInIco ? SkAlign4(compute_row_bytes(this->getInfo().width(), 1)) : 0)
+    , fXformOnDecode(false)
 {}
 
 /*
@@ -90,9 +91,16 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
             return false;
         }
 
+        SkColorType packColorType = dstColorType;
+        SkAlphaType packAlphaType = dstAlphaType;
+        if (this->colorXform()) {
+            packColorType = kBGRA_8888_SkColorType;
+            packAlphaType = kUnpremul_SkAlphaType;
+        }
+
         // Choose the proper packing function
-        bool isPremul = (kPremul_SkAlphaType == dstAlphaType) && !fIsOpaque;
-        PackColorProc packARGB = choose_pack_color_proc(isPremul, dstColorType);
+        bool isPremul = (kPremul_SkAlphaType == packAlphaType) && !fIsOpaque;
+        PackColorProc packARGB = choose_pack_color_proc(isPremul, packColorType);
 
         // Fill in the color table
         uint32_t i = 0;
@@ -116,6 +124,15 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
             colorTable[i] = SkPackARGB32NoCheck(0xFF, 0, 0, 0);
         }
 
+        if (this->colorXform() && !fXformOnDecode) {
+            SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstColorType);
+            SkColorSpaceXform::ColorFormat srcFormat = SkColorSpaceXform::kBGRA_8888_ColorFormat;
+            SkAlphaType xformAlphaType = select_xform_alpha(dstAlphaType,
+                                                            this->getInfo().alphaType());
+            SkAssertResult(this->colorXform()->apply(dstFormat, colorTable, srcFormat, colorTable,
+                                                     maxColors, xformAlphaType));
+        }
+
         // Set the color table
         fColorTable.reset(new SkColorTable(colorTable, maxColors));
     }
@@ -182,8 +199,12 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op
 
 SkCodec::Result SkBmpStandardCodec::onPrepareToDecode(const SkImageInfo& dstInfo,
         const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) {
+    fXformOnDecode = false;
     if (this->colorXform()) {
-        this->resetXformBuffer(dstInfo.width());
+        fXformOnDecode = apply_xform_on_decode(dstInfo.colorType(), this->getEncodedInfo().color());
+        if (fXformOnDecode) {
+            this->resetXformBuffer(dstInfo.width());
+        }
     }
 
     // Create the color table if necessary and prepare the stream for decode
@@ -220,7 +241,8 @@ int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo, void* dst, size_t
 
         void* dstRow = SkTAddOffset<void>(dst, row * dstRowBytes);
 
-        if (this->colorXform()) {
+        if (fXformOnDecode) {
+            SkASSERT(this->colorXform());
             SkImageInfo xformInfo = dstInfo.makeWH(fSwizzler->swizzleWidth(), dstInfo.height());
             fSwizzler->swizzle(this->xformBuffer(), fSrcBuffer.get());
             this->applyColorXform(xformInfo, dstRow, this->xformBuffer());

diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h
index 64c4635..12c12e8 100644 (file)
--- a/src/codec/SkBmpStandardCodec.h
+++ b/src/codec/SkBmpStandardCodec.h
@@ -94,6 +94,7 @@ private:
     const bool                          fIsOpaque;
     const bool                          fInIco;
     const size_t                        fAndMaskRowBytes; // only used for fInIco decodes
+    bool                                fXformOnDecode;
 
     typedef SkBmpCodec INHERITED;
 };

diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h
index f210303..a13cdbb 100644 (file)
--- a/src/codec/SkCodecPriv.h
+++ b/src/codec/SkCodecPriv.h
@@ -11,6 +11,7 @@
 #include "SkColorPriv.h"
 #include "SkColorSpaceXform.h"
 #include "SkColorTable.h"
+#include "SkEncodedInfo.h"
 #include "SkImageInfo.h"
 #include "SkTypes.h"
 
@@ -152,6 +153,12 @@ 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
         default:
             SkASSERT(false);
             return SkColorSpaceXform::kRGBA_8888_ColorFormat;
@@ -362,6 +369,11 @@ static inline SkAlphaType select_xform_alpha(SkAlphaType dstAlphaType, SkAlphaTy
     return (kOpaque_SkAlphaType == srcAlphaType) ? kOpaque_SkAlphaType : dstAlphaType;
 }
 
+static inline bool apply_xform_on_decode(SkColorType dstColorType, SkEncodedInfo::Color srcColor) {
+    // We will apply the color xform when reading the color table, unless F16 is requested.
+    return SkEncodedInfo::kPalette_Color != srcColor || kRGBA_F16_SkColorType == dstColorType;
+}
+
 /*
  * Alpha Type Conversions
  * - kOpaque to kOpaque, kUnpremul, kPremul is valid

diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index da6e062..a7359ee 100644 (file)
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -231,7 +231,7 @@ bool SkPngCodec::createColorTable(const SkImageInfo& dstInfo, int* ctableCount)
     if (png_get_tRNS(fPng_ptr, fInfo_ptr, &alphas, &numColorsWithAlpha, nullptr)) {
         // If we are performing a color xform, it will handle the premultiply.  Otherwise,
         // we'll do it here.
-        bool premultiply =  !this->colorXform() && needs_premul(dstInfo, this->getInfo());
+        bool premultiply = !this->colorXform() && needs_premul(dstInfo, this->getInfo());
 
         // Choose which function to use to create the color table. If the final destination's
         // colortype is unpremultiplied, the color table will store unpremultiplied colors.
@@ -267,9 +267,7 @@ bool SkPngCodec::createColorTable(const SkImageInfo& dstInfo, int* ctableCount)
     // If we are not decoding to F16, we can color xform now and store the results
     // in the color table.
     if (this->colorXform() && kRGBA_F16_SkColorType != dstInfo.colorType()) {
-        SkColorSpaceXform::ColorFormat xformColorFormat = is_rgba(dstInfo.colorType()) ?
-                SkColorSpaceXform::kRGBA_8888_ColorFormat :
-                SkColorSpaceXform::kBGRA_8888_ColorFormat;
+        SkColorSpaceXform::ColorFormat xformColorFormat = select_xform_format(dstInfo.colorType());
         SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(),
                                                         this->getInfo().alphaType());
         SkAssertResult(this->colorXform()->apply(xformColorFormat, colorTable,
@@ -1111,11 +1109,6 @@ void SkPngCodec::initializeXformParams() {
     }
 }
 
-static inline bool apply_xform_on_decode(SkColorType dstColorType, SkEncodedInfo::Color srcColor) {
-    // We will apply the color xform when reading the color table, unless F16 is requested.
-    return SkEncodedInfo::kPalette_Color != srcColor || kRGBA_F16_SkColorType == dstColorType;
-}
-
 void SkPngCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& options) {
     SkImageInfo swizzlerInfo = dstInfo;
     Options swizzlerOptions = options;