Refactor RGBA/BGRA xform logic in SkCodecs

BUG=skia:

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

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

diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 7cd1dfe6c1dbcc262c5883418eeccad09efe935f..78f6b20d3b3fb6bb80c51cd59d6854853805999d 100644 (file)
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -636,10 +636,10 @@ bool SkBmpCodec::onSkipScanlines(int count) {
 void SkBmpCodec::applyColorXform(const SkImageInfo& dstInfo, void* dst, void* src) const {
     SkColorSpaceXform* xform = this->colorXform();
     if (xform) {
-        SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType());
-        SkColorSpaceXform::ColorFormat srcFormat = SkColorSpaceXform::kBGRA_8888_ColorFormat;
-        SkAlphaType alphaType = select_xform_alpha(dstInfo.alphaType(),
-                                                   this->getInfo().alphaType());
+        const SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType());
+        const SkColorSpaceXform::ColorFormat srcFormat = select_xform_format(kXformSrcColorType);
+        const SkAlphaType alphaType = select_xform_alpha(dstInfo.alphaType(),
+                                                         this->getInfo().alphaType());
         SkAssertResult(xform->apply(dstFormat, dst, srcFormat, src, dstInfo.width(),
                                     alphaType));
     }

diff --git a/src/codec/SkBmpCodec.h b/src/codec/SkBmpCodec.h
index a51bbcecc30b40a330bedfc6107d12ddc947fddc..0e38c840590b09a4495bca18f0810dffef8b2a50 100644 (file)
--- a/src/codec/SkBmpCodec.h
+++ b/src/codec/SkBmpCodec.h
@@ -110,6 +110,12 @@ protected:
     uint32_t* xformBuffer() const { return fXformBuffer.get(); }
     void resetXformBuffer(int count) { fXformBuffer.reset(new uint32_t[count]); }
 
+    /*
+     * BMPs are typically encoded as BGRA/BGR so this is a more efficient choice
+     * than RGBA.
+     */
+    static const SkColorType kXformSrcColorType = kBGRA_8888_SkColorType;
+
 private:
 
     /*

diff --git a/src/codec/SkBmpMaskCodec.cpp b/src/codec/SkBmpMaskCodec.cpp
index 3ee1bb77fc7aaa38172fdf0e770abc223578168f..9612a23b538f6b9026f57c85c322f08c56e61cfb 100644 (file)
--- a/src/codec/SkBmpMaskCodec.cpp
+++ b/src/codec/SkBmpMaskCodec.cpp
@@ -60,7 +60,7 @@ SkCodec::Result SkBmpMaskCodec::onPrepareToDecode(const SkImageInfo& dstInfo,
 
     SkImageInfo swizzlerInfo = dstInfo;
     if (this->colorXform()) {
-        swizzlerInfo = swizzlerInfo.makeColorType(kBGRA_8888_SkColorType);
+        swizzlerInfo = swizzlerInfo.makeColorType(kXformSrcColorType);
         if (kPremul_SkAlphaType == dstInfo.alphaType()) {
             swizzlerInfo = swizzlerInfo.makeAlphaType(kUnpremul_SkAlphaType);
         }

diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 0bc6f60542fe6d3a769c7adabcfdc1f2f171dedf..c2574ddef860de9baf7e781800cdd59c701faff9 100644 (file)
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -341,7 +341,7 @@ int SkBmpRLECodec::decodeRows(const SkImageInfo& info, void* dst, size_t dstRowB
     SkImageInfo decodeInfo = dstInfo;
     if (decodeDst) {
         if (this->colorXform()) {
-            decodeInfo = decodeInfo.makeColorType(kBGRA_8888_SkColorType);
+            decodeInfo = decodeInfo.makeColorType(kXformSrcColorType);
             if (kRGBA_F16_SkColorType == dstInfo.colorType()) {
                 int count = height * dstInfo.width();
                 this->resetXformBuffer(count);

diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 004c2571d5a82a198cc0b7452b0bea6d9f33150b..eeb3a1bbe5fe8d542667c0325bd3edb5d9b3fcaf 100644 (file)
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -183,7 +183,7 @@ void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Op
     SkImageInfo swizzlerInfo = dstInfo;
     SkCodec::Options swizzlerOptions = opts;
     if (this->colorXform()) {
-        swizzlerInfo = swizzlerInfo.makeColorType(kBGRA_8888_SkColorType);
+        swizzlerInfo = swizzlerInfo.makeColorType(kXformSrcColorType);
         if (kPremul_SkAlphaType == dstInfo.alphaType()) {
             swizzlerInfo = swizzlerInfo.makeAlphaType(kUnpremul_SkAlphaType);
         }

diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index e773ed71e95dd206c15c562a645f8ef647cad397..7fc3c9805a4d5e11ac5b0548762e4dae38ee26c7 100644 (file)
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -144,10 +144,12 @@ int SkGifCodec::onGetRepetitionCount() {
     return fReader->loopCount();
 }
 
+static const SkColorType kXformSrcColorType = kRGBA_8888_SkColorType;
+
 void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, size_t frameIndex) {
     SkColorType colorTableColorType = dstInfo.colorType();
     if (this->colorXform()) {
-        colorTableColorType = kRGBA_8888_SkColorType;
+        colorTableColorType = kXformSrcColorType;
     }
 
     sk_sp<SkColorTable> currColorTable = fReader->getColorTable(colorTableColorType, frameIndex);
@@ -158,10 +160,10 @@ void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, size_t frameIn
         fCurrColorTable.reset(new SkColorTable(&color, 1));
     } else if (this->colorXform() && !fXformOnDecode) {
         SkPMColor dstColors[256];
-        SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType());
-        SkColorSpaceXform::ColorFormat srcFormat = SkColorSpaceXform::kRGBA_8888_ColorFormat;
-        SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(),
-                                                        this->getInfo().alphaType());
+        const SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType());
+        const SkColorSpaceXform::ColorFormat srcFormat = select_xform_format(kXformSrcColorType);
+        const SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(),
+                                                              this->getInfo().alphaType());
         SkAssertResult(this->colorXform()->apply(dstFormat, dstColors, srcFormat,
                                                  currColorTable->readColors(),
                                                  currColorTable->count(), xformAlphaType));
@@ -261,7 +263,7 @@ void SkGifCodec::initializeSwizzler(const SkImageInfo& dstInfo, size_t frameInde
 
     SkImageInfo swizzlerInfo = dstInfo;
     if (this->colorXform()) {
-        swizzlerInfo = swizzlerInfo.makeColorType(kRGBA_8888_SkColorType);
+        swizzlerInfo = swizzlerInfo.makeColorType(kXformSrcColorType);
         if (kPremul_SkAlphaType == dstInfo.alphaType()) {
             swizzlerInfo = swizzlerInfo.makeAlphaType(kUnpremul_SkAlphaType);
         }
@@ -470,7 +472,7 @@ void SkGifCodec::applyXformRow(const SkImageInfo& dstInfo, void* dst, const uint
         fSwizzler->swizzle(fXformBuffer.get(), src);
 
         const SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType());
-        const SkColorSpaceXform::ColorFormat srcFormat = SkColorSpaceXform::kRGBA_8888_ColorFormat;
+        const SkColorSpaceXform::ColorFormat srcFormat = select_xform_format(kXformSrcColorType);
         const SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(),
                                                               this->getInfo().alphaType());
         const int xformWidth = get_scaled_dimension(dstInfo.width(), fSwizzler->sampleX());

diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 614cde9c1e16671d33750ea1d44d8fcd18ce9c0e..69b80273df1fa971a504643acb63bc51edef79b6 100644 (file)
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -385,8 +385,8 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo) {
             if (isCMYK) {
                 fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
             } else if (this->colorXform()) {
-                // Our color transformation code requires RGBA order inputs, but it'll swizzle
-                // to BGRA for us.
+                // Always using RGBA as the input format for color xforms makes the
+                // implementation a little simpler.
                 fDecoderMgr->dinfo()->out_color_space = JCS_EXT_RGBA;
             } else {
                 fDecoderMgr->dinfo()->out_color_space = JCS_EXT_BGRA;

diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 3cf972dc760659be8bd5ce50dc9bb79dd1f44d6e..758ffa51305509d8c8750ff536bf511b9fcb9ef4 100644 (file)
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -212,6 +212,8 @@ void SkPngCodec::processData() {
     }
 }
 
+static const SkColorType kXformSrcColorType = kRGBA_8888_SkColorType;
+
 // Note: SkColorTable claims to store SkPMColors, which is not necessarily the case here.
 bool SkPngCodec::createColorTable(const SkImageInfo& dstInfo, int* ctableCount) {
 
@@ -224,7 +226,7 @@ bool SkPngCodec::createColorTable(const SkImageInfo& dstInfo, int* ctableCount)
     // Contents depend on tableColorType and our choice of if/when to premultiply:
     // { kPremul, kUnpremul, kOpaque } x { RGBA, BGRA }
     SkPMColor colorTable[256];
-    SkColorType tableColorType = this->colorXform() ? kRGBA_8888_SkColorType : dstInfo.colorType();
+    SkColorType tableColorType = this->colorXform() ? kXformSrcColorType : dstInfo.colorType();
 
     png_bytep alphas;
     int numColorsWithAlpha = 0;
@@ -267,11 +269,12 @@ 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 = select_xform_format(dstInfo.colorType());
-        SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(),
-                                                        this->getInfo().alphaType());
-        SkAssertResult(this->colorXform()->apply(xformColorFormat, colorTable,
-                SkColorSpaceXform::kRGBA_8888_ColorFormat, colorTable, numColors, xformAlphaType));
+        const SkColorSpaceXform::ColorFormat dstFormat = select_xform_format(dstInfo.colorType());
+        const SkColorSpaceXform::ColorFormat srcFormat = select_xform_format(kXformSrcColorType);
+        const SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(),
+                                                              this->getInfo().alphaType());
+        SkAssertResult(this->colorXform()->apply(dstFormat, colorTable, srcFormat, colorTable,
+                       numColors, xformAlphaType));
     }
 
     // Pad the color table with the last color in the table (or black) in the case that
@@ -430,7 +433,7 @@ void SkPngCodec::allocateStorage(const SkImageInfo& dstInfo) {
 }
 
 void SkPngCodec::applyXformRow(void* dst, const void* src) {
-    const SkColorSpaceXform::ColorFormat srcColorFormat = SkColorSpaceXform::kRGBA_8888_ColorFormat;
+    const SkColorSpaceXform::ColorFormat srcColorFormat = select_xform_format(kXformSrcColorType);
     switch (fXformMode) {
         case kSwizzleOnly_XformMode:
             fSwizzler->swizzle(dst, (const uint8_t*) src);
@@ -1068,8 +1071,6 @@ bool SkPngCodec::initializeXforms(const SkImageInfo& dstInfo, const Options& opt
     }
 
     // If the image is RGBA and we have a color xform, we can skip the swizzler.
-    // FIXME (msarett):
-    // Support more input types to this->colorXform() (ex: RGB, Gray) and skip the swizzler more often.
     if (this->colorXform() && SkEncodedInfo::kRGBA_Color == this->getEncodedInfo().color() &&
         !options.fSubset)
     {
@@ -1116,7 +1117,7 @@ void SkPngCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& o
     if (this->colorXform() &&
         apply_xform_on_decode(dstInfo.colorType(), this->getEncodedInfo().color()))
     {
-        swizzlerInfo = swizzlerInfo.makeColorType(kRGBA_8888_SkColorType);
+        swizzlerInfo = swizzlerInfo.makeColorType(kXformSrcColorType);
         if (kPremul_SkAlphaType == dstInfo.alphaType()) {
             swizzlerInfo = swizzlerInfo.makeAlphaType(kUnpremul_SkAlphaType);
         }

diff --git a/src/codec/SkRawCodec.cpp b/src/codec/SkRawCodec.cpp
index 151363239cb4feb77c97afd76f13c59234881b80..93995c42fb291db5b40070b0f63c8417ddbdfd20 100644 (file)
--- a/src/codec/SkRawCodec.cpp
+++ b/src/codec/SkRawCodec.cpp
@@ -688,10 +688,11 @@ SkCodec::Result SkRawCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
         return kInvalidConversion;
     }
 
+    static const SkColorType kXformSrcColorType = kRGBA_8888_SkColorType;
     SkImageInfo swizzlerInfo = dstInfo;
     std::unique_ptr<uint32_t[]> xformBuffer = nullptr;
     if (this->colorXform()) {
-        swizzlerInfo = swizzlerInfo.makeColorType(kRGBA_8888_SkColorType);
+        swizzlerInfo = swizzlerInfo.makeColorType(kXformSrcColorType);
         xformBuffer.reset(new uint32_t[dstInfo.width()]);
     }
 
@@ -742,7 +743,7 @@ SkCodec::Result SkRawCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
             swizzler->swizzle(xformBuffer.get(), &srcRow[0]);
 
             const SkColorSpaceXform::ColorFormat srcFormat =
-                    SkColorSpaceXform::kRGBA_8888_ColorFormat;
+                    select_xform_format(kXformSrcColorType);
             const SkColorSpaceXform::ColorFormat dstFormat =
                     select_xform_format(dstInfo.colorType());
             this->colorXform()->apply(dstFormat, dstRow, srcFormat, xformBuffer.get(),