Revert of Add color space xform support to SkJpegCodec (includes F16!) (patchset #9 id:260001 of https://codereview.chromium.org/2174493002/ )

Reason for revert:
Breaking MSAN

Original issue's description:
> Add color space xform support to SkJpegCodec (includes F16!)
>
> Also changes SkColorXform to support:
> RGBA->RGBA
> RGBA->BGRA
>
> Instead of:
> RGBA->SkPMColor
>
> TBR=reed@google.com
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2174493002
> CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
>
> Committed: https://skia.googlesource.com/skia/+/73d55332e2846dd05e9efdaa2f017bcc3872884b

TBR=mtklein@google.com,reed@google.com,herb@google.com,brianosman@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review-Url: https://codereview.chromium.org/2195523002

8 files changed, 143 insertions, 332 deletions

diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 6160aa8e47..a81c759ff0 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -196,7 +196,7 @@ bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut,
 
     // libjpeg errors will be caught and reported here
     if (setjmp(decoderMgr->getJmpBuf())) {
-        return decoderMgr->returnFalse("ReadHeader");
+        return decoderMgr->returnFalse("setjmp");
     }
 
     // Initialize the decompress info and the source manager
@@ -212,7 +212,7 @@ bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut,
 
     // Read the jpeg header
     if (JPEG_HEADER_OK != jpeg_read_header(decoderMgr->dinfo(), true)) {
-        return decoderMgr->returnFalse("ReadHeader");
+        return decoderMgr->returnFalse("read_header");
     }
 
     if (codecOut) {
@@ -268,8 +268,7 @@ SkJpegCodec::SkJpegCodec(int width, int height, const SkEncodedInfo& info, SkStr
     : INHERITED(width, height, info, stream, std::move(colorSpace), origin)
     , fDecoderMgr(decoderMgr)
     , fReadyState(decoderMgr->dinfo()->global_state)
-    , fSwizzleSrcRow(nullptr)
-    , fColorXformSrcRow(nullptr)
+    , fSrcRow(nullptr)
     , fSwizzlerSubset(SkIRect::MakeEmpty())
     , fICCData(std::move(iccData))
 {}
@@ -342,16 +341,14 @@ SkISize SkJpegCodec::onGetScaledDimensions(float desiredScale) const {
 bool SkJpegCodec::onRewind() {
     JpegDecoderMgr* decoderMgr = nullptr;
     if (!ReadHeader(this->stream(), nullptr, &decoderMgr)) {
-        return fDecoderMgr->returnFalse("onRewind");
+        return fDecoderMgr->returnFalse("could not rewind");
     }
     SkASSERT(nullptr != decoderMgr);
     fDecoderMgr.reset(decoderMgr);
 
     fSwizzler.reset(nullptr);
-    fSwizzleSrcRow = nullptr;
-    fColorXformSrcRow = nullptr;
+    fSrcRow = nullptr;
     fStorage.reset();
-    fColorXform.reset(nullptr);
 
     return true;
 }
@@ -361,23 +358,22 @@ bool SkJpegCodec::onRewind() {
  * image has been implemented
  * Sets the output color space
  */
-bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColorXform) {
-    if (kUnknown_SkAlphaType == dstInfo.alphaType()) {
+bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dst) {
+    if (kUnknown_SkAlphaType == dst.alphaType()) {
         return false;
     }
 
-    if (kOpaque_SkAlphaType != dstInfo.alphaType()) {
+    if (kOpaque_SkAlphaType != dst.alphaType()) {
         SkCodecPrintf("Warning: an opaque image should be decoded as opaque "
                       "- it is being decoded as non-opaque, which will draw slower\n");
     }
 
-    // Check if we will decode to CMYK.  libjpeg-turbo does not convert CMYK to RGBA, so
-    // we must do it ourselves.
-    J_COLOR_SPACE encodedColorType = fDecoderMgr->dinfo()->jpeg_color_space;
-    bool isCMYK = (JCS_CMYK == encodedColorType || JCS_YCCK == encodedColorType);
+    // Check if we will decode to CMYK because a conversion to RGBA is not supported
+    J_COLOR_SPACE colorSpace = fDecoderMgr->dinfo()->jpeg_color_space;
+    bool isCMYK = JCS_CMYK == colorSpace || JCS_YCCK == colorSpace;
 
     // Check for valid color types and set the output color space
-    switch (dstInfo.colorType()) {
+    switch (dst.colorType()) {
         case kRGBA_8888_SkColorType:
             if (isCMYK) {
                 fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
@@ -388,19 +384,11 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo
         case kBGRA_8888_SkColorType:
             if (isCMYK) {
                 fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
-            } else if (needsColorXform) {
-                // Our color transformation code requires RGBA order inputs, but it'll swizzle
-                // to BGRA for us.
-                fDecoderMgr->dinfo()->out_color_space = JCS_EXT_RGBA;
             } else {
                 fDecoderMgr->dinfo()->out_color_space = JCS_EXT_BGRA;
             }
             return true;
         case kRGB_565_SkColorType:
-            if (needsColorXform) {
-                return false;
-            }
-
             if (isCMYK) {
                 fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
             } else {
@@ -413,19 +401,12 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo
             }
             return true;
         case kGray_8_SkColorType:
-            if (needsColorXform || JCS_GRAYSCALE != encodedColorType) {
-                return false;
-            }
-
-            fDecoderMgr->dinfo()->out_color_space = JCS_GRAYSCALE;
-            return true;
-        case kRGBA_F16_SkColorType:
-            SkASSERT(needsColorXform);
-
             if (isCMYK) {
-                fDecoderMgr->dinfo()->out_color_space = JCS_CMYK;
+                return false;
             } else {
-                fDecoderMgr->dinfo()->out_color_space = JCS_EXT_RGBA;
+                // We will enable decodes to gray even if the image is color because this is
+                // much faster than decoding to color and then converting
+                fDecoderMgr->dinfo()->out_color_space = JCS_GRAYSCALE;
             }
             return true;
         default:
@@ -439,7 +420,7 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dstInfo, bool needsColo
  */
 bool SkJpegCodec::onDimensionsSupported(const SkISize& size) {
     if (setjmp(fDecoderMgr->getJmpBuf())) {
-        return fDecoderMgr->returnFalse("onDimensionsSupported");
+        return fDecoderMgr->returnFalse("onDimensionsSupported/setjmp");
     }
 
     const unsigned int dstWidth = size.width();
@@ -474,83 +455,6 @@ bool SkJpegCodec::onDimensionsSupported(const SkISize& size) {
     return true;
 }
 
-static bool needs_color_xform(const SkImageInfo& dstInfo, const SkImageInfo& srcInfo) {
-    // FIXME (msarett):
-    // Do a better check for color space equality.
-    return (kRGBA_F16_SkColorType == dstInfo.colorType()) ||
-           (dstInfo.colorSpace() && (dstInfo.colorSpace() != srcInfo.colorSpace()));
-}
-
-int SkJpegCodec::readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count) {
-    // Set the jump location for libjpeg-turbo errors
-    if (setjmp(fDecoderMgr->getJmpBuf())) {
-        return 0;
-    }
-
-    // When fSwizzleSrcRow is non-null, it means that we need to swizzle.  In this case,
-    // we will always decode into fSwizzlerSrcRow before swizzling into the next buffer.
-    // We can never swizzle "in place" because the swizzler may perform sampling and/or
-    // subsetting.
-    // When fColorXformSrcRow is non-null, it means that we need to color xform and that
-    // we cannot color xform "in place" (many times we can, but not when the dst is F16).
-    // In this case, we will color xform from fColorXformSrc into the dst.
-    JSAMPLE* decodeDst = (JSAMPLE*) dst;
-    uint32_t* swizzleDst = (uint32_t*) dst;
-    size_t decodeDstRowBytes = rowBytes;
-    size_t swizzleDstRowBytes = rowBytes;
-    if (fSwizzleSrcRow && fColorXformSrcRow) {
-        decodeDst = (JSAMPLE*) fSwizzleSrcRow;
-        swizzleDst = fColorXformSrcRow;
-        decodeDstRowBytes = 0;
-        swizzleDstRowBytes = 0;
-    } else if (fColorXformSrcRow) {
-        decodeDst = (JSAMPLE*) fColorXformSrcRow;
-        swizzleDst = fColorXformSrcRow;
-        decodeDstRowBytes = 0;
-        swizzleDstRowBytes = 0;
-    } else if (fSwizzleSrcRow) {
-        decodeDst = (JSAMPLE*) fSwizzleSrcRow;
-        decodeDstRowBytes = 0;
-    }
-
-    for (int y = 0; y < count; y++) {
-        uint32_t lines = jpeg_read_scanlines(fDecoderMgr->dinfo(), &decodeDst, 1);
-        sk_msan_mark_initialized(decodeDst, decodeDst + rowBytes, "skbug.com/4550");
-        if (0 == lines) {
-            return y;
-        }
-
-        if (fSwizzler) {
-            fSwizzler->swizzle(swizzleDst, decodeDst);
-        }
-
-        if (fColorXform) {
-            int width = dstInfo.width();
-            switch (dstInfo.colorType()) {
-                case kRGBA_8888_SkColorType:
-                    fColorXform->applyToRGBA((uint32_t*) dst, swizzleDst, width);
-                    break;
-                case kBGRA_8888_SkColorType:
-                    fColorXform->applyToBGRA((uint32_t*) dst, swizzleDst, width);
-                    break;
-                case kRGBA_F16_SkColorType:
-                    fColorXform->applyToF16((uint64_t*) dst, swizzleDst, width);
-                    break;
-                default:
-                    SkASSERT(false);
-                    break;
-            }
-
-            dst = SkTAddOffset<void>(dst, rowBytes);
-        }
-
-        decodeDst = SkTAddOffset<JSAMPLE>(decodeDst, decodeDstRowBytes);
-        swizzleDst = SkTAddOffset<uint32_t>(swizzleDst, swizzleDstRowBytes);
-    }
-
-    return count;
-}
-
 /*
  * Performs the jpeg decode
  */
@@ -572,15 +476,11 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
     }
 
     // Check if we can decode to the requested destination and set the output color space
-    bool needsColorXform = needs_color_xform(dstInfo, this->getInfo());
-    if (!this->setOutputColorSpace(dstInfo, needsColorXform)) {
-        return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion);
-    }
-
-    if (!this->initializeColorXform(dstInfo, needsColorXform)) {
-        return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters);
+    if (!this->setOutputColorSpace(dstInfo)) {
+        return fDecoderMgr->returnFailure("conversion_possible", kInvalidConversion);
     }
 
+    // Now, given valid output dimensions, we can start the decompress
     if (!jpeg_start_decompress(dinfo)) {
         return fDecoderMgr->returnFailure("startDecompress", kInvalidInput);
     }
@@ -594,37 +494,39 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
         this->initializeSwizzler(dstInfo, options);
     }
 
-    this->allocateStorage(dstInfo);
+    // Perform the decode a single row at a time
+    uint32_t dstHeight = dstInfo.height();
 
-    int rows = this->readRows(dstInfo, dst, dstRowBytes, dstInfo.height());
-    if (rows < dstInfo.height()) {
-        *rowsDecoded = rows;
-        return fDecoderMgr->returnFailure("Incomplete image data", kIncompleteInput);
+    JSAMPLE* dstRow;
+    if (fSwizzler) {
+        // write data to storage row, then sample using swizzler
+        dstRow = fSrcRow;
+    } else {
+        // write data directly to dst
+        dstRow = (JSAMPLE*) dst;
     }
 
-    return kSuccess;
-}
+    for (uint32_t y = 0; y < dstHeight; y++) {
+        // Read rows of the image
+        uint32_t lines = jpeg_read_scanlines(dinfo, &dstRow, 1);
+        sk_msan_mark_initialized(dstRow, dstRow + dstRowBytes, "skbug.com/4550");
 
-void SkJpegCodec::allocateStorage(const SkImageInfo& dstInfo) {
-    size_t swizzleBytes = 0;
-    if (fSwizzler) {
-        swizzleBytes = get_row_bytes(fDecoderMgr->dinfo());
-        SkASSERT(!fColorXform || SkIsAlign4(swizzleBytes));
-    }
+        // If we cannot read enough rows, assume the input is incomplete
+        if (lines != 1) {
+            *rowsDecoded = y;
+            return fDecoderMgr->returnFailure("Incomplete image data", kIncompleteInput);
+        }
 
-    size_t xformBytes = 0;
-    if (kRGBA_F16_SkColorType == dstInfo.colorType()) {
-        SkASSERT(fColorXform);
-        xformBytes = dstInfo.width() * sizeof(SkColorSpaceXform::RGBA32);
+        if (fSwizzler) {
+            // use swizzler to sample row
+            fSwizzler->swizzle(dst, dstRow);
+            dst = SkTAddOffset<JSAMPLE>(dst, dstRowBytes);
+        } else {
+            dstRow = SkTAddOffset<JSAMPLE>(dstRow, dstRowBytes);
+        }
     }
 
-    size_t totalBytes = swizzleBytes + xformBytes;
-    if (totalBytes > 0) {
-        fStorage.reset(totalBytes);
-        fSwizzleSrcRow = (swizzleBytes > 0) ? fStorage.get() : nullptr;
-        fColorXformSrcRow = (xformBytes > 0) ?
-                SkTAddOffset<uint32_t>(fStorage.get(), swizzleBytes) : nullptr;
-    }
+    return kSuccess;
 }
 
 void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& options) {
@@ -641,9 +543,9 @@ void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options&
             break;
         case JCS_CMYK:
             preSwizzled = false;
-            swizzlerInfo = SkEncodedInfo::Make(SkEncodedInfo::kInvertedCMYK_Color,
-                                               swizzlerInfo.alpha(),
-                                               swizzlerInfo.bitsPerComponent());
+            swizzlerInfo = SkEncodedInfo::Make(
+                    SkEncodedInfo::kInvertedCMYK_Color, swizzlerInfo.alpha(),
+                    swizzlerInfo.bitsPerComponent());
             break;
         default:
             break;
@@ -661,28 +563,17 @@ void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options&
     fSwizzler.reset(SkSwizzler::CreateSwizzler(swizzlerInfo, nullptr, dstInfo, swizzlerOptions,
                                                nullptr, preSwizzled));
     SkASSERT(fSwizzler);
-}
-
-bool SkJpegCodec::initializeColorXform(const SkImageInfo& dstInfo, bool needsColorXform) {
-    if (needsColorXform) {
-        fColorXform = SkColorSpaceXform::New(sk_ref_sp(this->getInfo().colorSpace()),
-                                             sk_ref_sp(dstInfo.colorSpace()));
-        if (!fColorXform && kRGBA_F16_SkColorType == dstInfo.colorType()) {
-            return false;
-        }
-    }
-
-    return true;
+    fStorage.reset(get_row_bytes(fDecoderMgr->dinfo()));
+    fSrcRow = fStorage.get();
 }
 
 SkSampler* SkJpegCodec::getSampler(bool createIfNecessary) {
     if (!createIfNecessary || fSwizzler) {
-        SkASSERT(!fSwizzler || (fSwizzleSrcRow && fStorage.get() == fSwizzleSrcRow));
+        SkASSERT(!fSwizzler || (fSrcRow && fStorage.get() == fSrcRow));
         return fSwizzler;
     }
 
     this->initializeSwizzler(this->dstInfo(), this->options());
-    this->allocateStorage(this->dstInfo());
     return fSwizzler;
 }
 
@@ -695,15 +586,11 @@ SkCodec::Result SkJpegCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
     }
 
     // Check if we can decode to the requested destination and set the output color space
-    bool needsColorXform = needs_color_xform(dstInfo, this->getInfo());
-    if (!this->setOutputColorSpace(dstInfo, needsColorXform)) {
+    if (!this->setOutputColorSpace(dstInfo)) {
         return kInvalidConversion;
     }
 
-    if (!this->initializeColorXform(dstInfo, needsColorXform)) {
-        return fDecoderMgr->returnFailure("initializeColorXform", kInvalidParameters);
-    }
-
+    // Now, given valid output dimensions, we can start the decompress
     if (!jpeg_start_decompress(fDecoderMgr->dinfo())) {
         SkCodecPrintf("start decompress failed\n");
         return kInvalidInput;
@@ -764,37 +651,62 @@ SkCodec::Result SkJpegCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
     }
 #endif
 
-    this->allocateStorage(dstInfo);
-
     return kSuccess;
 }
 
 int SkJpegCodec::onGetScanlines(void* dst, int count, size_t dstRowBytes) {
-    int rows = this->readRows(this->dstInfo(), dst, dstRowBytes, count);
-    if (rows < count) {
-        // This allows us to skip calling jpeg_finish_decompress().
-        fDecoderMgr->dinfo()->output_scanline = this->dstInfo().height();
+    // Set the jump location for libjpeg errors
+    if (setjmp(fDecoderMgr->getJmpBuf())) {
+        return fDecoderMgr->returnFailure("setjmp", kInvalidInput);
+    }
+    // Read rows one at a time
+    JSAMPLE* dstRow;
+    size_t srcRowBytes = get_row_bytes(fDecoderMgr->dinfo());
+    if (fSwizzler) {
+        // write data to storage row, then sample using swizzler
+        dstRow = fSrcRow;
+    } else {
+        // write data directly to dst
+        SkASSERT(count == 1 || dstRowBytes >= srcRowBytes);
+        dstRow = (JSAMPLE*) dst;
     }
 
-    return rows;
+    for (int y = 0; y < count; y++) {
+        // Read row of the image
+        uint32_t rowsDecoded = jpeg_read_scanlines(fDecoderMgr->dinfo(), &dstRow, 1);
+        sk_msan_mark_initialized(dstRow, dstRow + srcRowBytes, "skbug.com/4550");
+        if (rowsDecoded != 1) {
+            fDecoderMgr->dinfo()->output_scanline = this->dstInfo().height();
+            return y;
+        }
+
+        if (fSwizzler) {
+            // use swizzler to sample row
+            fSwizzler->swizzle(dst, dstRow);
+            dst = SkTAddOffset<JSAMPLE>(dst, dstRowBytes);
+        } else {
+            dstRow = SkTAddOffset<JSAMPLE>(dstRow, dstRowBytes);
+        }
+    }
+    return count;
 }
 
 bool SkJpegCodec::onSkipScanlines(int count) {
     // Set the jump location for libjpeg errors
     if (setjmp(fDecoderMgr->getJmpBuf())) {
-        return fDecoderMgr->returnFalse("onSkipScanlines");
+        return fDecoderMgr->returnFalse("setjmp");
     }
 
 #ifdef TURBO_HAS_SKIP
     return (uint32_t) count == jpeg_skip_scanlines(fDecoderMgr->dinfo(), count);
 #else
-    if (!fSwizzleSrcRow) {
+    if (!fSrcRow) {
         fStorage.reset(get_row_bytes(fDecoderMgr->dinfo()));
-        fSwizzleSrcRow = fStorage.get();
+        fSrcRow = fStorage.get();
     }
 
     for (int y = 0; y < count; y++) {
-        if (1 != jpeg_read_scanlines(fDecoderMgr->dinfo(), &fSwizzleSrcRow, 1)) {
+        if (1 != jpeg_read_scanlines(fDecoderMgr->dinfo(), &fSrcRow, 1)) {
             return false;
         }
     }
diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h
index 7eb5100a8f..7aa275ce4e 100644
--- a/src/codec/SkJpegCodec.h
+++ b/src/codec/SkJpegCodec.h
@@ -10,7 +10,6 @@
 
 #include "SkCodec.h"
 #include "SkColorSpace.h"
-#include "SkColorSpaceXform.h"
 #include "SkImageInfo.h"
 #include "SkSwizzler.h"
 #include "SkStream.h"
@@ -100,46 +99,34 @@ private:
 
     /*
      * Checks if the conversion between the input image and the requested output
-     * image has been implemented.
-     *
-     * Sets the output color space.
+     * image has been implemented
+     * Sets the output color space
      */
-    bool setOutputColorSpace(const SkImageInfo& dst, bool needsColorXform);
+    bool setOutputColorSpace(const SkImageInfo& dst);
 
+    // scanline decoding
     void initializeSwizzler(const SkImageInfo& dstInfo, const Options& options);
-    bool initializeColorXform(const SkImageInfo& dstInfo, bool needsColorXform);
-    void allocateStorage(const SkImageInfo& dstInfo);
-    int readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count);
-
-    /*
-     * Scanline decoding.
-     */
     SkSampler* getSampler(bool createIfNecessary) override;
     Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options,
             SkPMColor ctable[], int* ctableCount) override;
     int onGetScanlines(void* dst, int count, size_t rowBytes) override;
     bool onSkipScanlines(int count) override;
 
-    SkAutoTDelete<JpegDecoderMgr>      fDecoderMgr;
-
+    SkAutoTDelete<JpegDecoderMgr> fDecoderMgr;
     // We will save the state of the decompress struct after reading the header.
     // This allows us to safely call onGetScaledDimensions() at any time.
-    const int                          fReadyState;
-
-
-    SkAutoTMalloc<uint8_t>             fStorage;
-    uint8_t*                           fSwizzleSrcRow;
-    uint32_t*                          fColorXformSrcRow;
+    const int                     fReadyState;
 
+    // scanline decoding
+    SkAutoTMalloc<uint8_t>     fStorage;    // Only used if sampling is needed
+    uint8_t*                   fSrcRow;     // Only used if sampling is needed
     // libjpeg-turbo provides some subsetting.  In the case that libjpeg-turbo
     // cannot take the exact the subset that we need, we will use the swizzler
     // to further subset the output from libjpeg-turbo.
-    SkIRect                            fSwizzlerSubset;
-
-    SkAutoTDelete<SkSwizzler>          fSwizzler;
-    std::unique_ptr<SkColorSpaceXform> fColorXform;
+    SkIRect                    fSwizzlerSubset;
+    SkAutoTDelete<SkSwizzler>  fSwizzler;
     
-    sk_sp<SkData>                      fICCData;
+    sk_sp<SkData>              fICCData;
 
     typedef SkCodec INHERITED;
 };
diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp
index 4fbfa6cd9e..924f24f6fc 100644
--- a/src/core/SkColorSpaceXform.cpp
+++ b/src/core/SkColorSpaceXform.cpp
@@ -629,7 +629,7 @@ SkColorSpaceXform_Base<Dst>::SkColorSpaceXform_Base(const sk_sp<SkColorSpace>& s
 
 template <>
 void SkColorSpaceXform_Base<SkColorSpace::kSRGB_GammaNamed>
-::applyToRGBA(RGBA32* dst, const RGBA32* src, int len) const
+::applyTo8888(SkPMColor* dst, const RGBA32* src, int len) const
 {
     if (fColorLUT) {
         handle_color_lut(dst, src, len, fColorLUT.get());
@@ -641,7 +641,7 @@ void SkColorSpaceXform_Base<SkColorSpace::kSRGB_GammaNamed>
 
 template <>
 void SkColorSpaceXform_Base<SkColorSpace::k2Dot2Curve_GammaNamed>
-::applyToRGBA(RGBA32* dst, const RGBA32* src, int len) const
+::applyTo8888(SkPMColor* dst, const RGBA32* src, int len) const
 {
     if (fColorLUT) {
         handle_color_lut(dst, src, len, fColorLUT.get());
@@ -653,7 +653,7 @@ void SkColorSpaceXform_Base<SkColorSpace::k2Dot2Curve_GammaNamed>
 
 template <>
 void SkColorSpaceXform_Base<SkColorSpace::kNonStandard_GammaNamed>
-::applyToRGBA(RGBA32* dst, const RGBA32* src, int len) const
+::applyTo8888(SkPMColor* dst, const RGBA32* src, int len) const
 {
     if (fColorLUT) {
         handle_color_lut(dst, src, len, fColorLUT.get());
@@ -663,43 +663,6 @@ void SkColorSpaceXform_Base<SkColorSpace::kNonStandard_GammaNamed>
     SkOpts::color_xform_RGB1_to_table(dst, src, len, fSrcGammaTables, fSrcToDst, fDstGammaTables);
 }
 
-template <>
-void SkColorSpaceXform_Base<SkColorSpace::kSRGB_GammaNamed>
-::applyToBGRA(BGRA32* dst, const RGBA32* src, int len) const
-{
-    if (fColorLUT) {
-        handle_color_lut(dst, src, len, fColorLUT.get());
-        src = dst;
-    }
-
-    SkOpts::color_xform_RGB1_to_srgb_swaprb(dst, src, len, fSrcGammaTables, fSrcToDst);
-}
-
-template <>
-void SkColorSpaceXform_Base<SkColorSpace::k2Dot2Curve_GammaNamed>
-::applyToBGRA(BGRA32* dst, const RGBA32* src, int len) const
-{
-    if (fColorLUT) {
-        handle_color_lut(dst, src, len, fColorLUT.get());
-        src = dst;
-    }
-
-    SkOpts::color_xform_RGB1_to_2dot2_swaprb(dst, src, len, fSrcGammaTables, fSrcToDst);
-}
-
-template <>
-void SkColorSpaceXform_Base<SkColorSpace::kNonStandard_GammaNamed>
-::applyToBGRA(BGRA32* dst, const RGBA32* src, int len) const
-{
-    if (fColorLUT) {
-        handle_color_lut(dst, src, len, fColorLUT.get());
-        src = dst;
-    }
-
-    SkOpts::color_xform_RGB1_to_table_swaprb(dst, src, len, fSrcGammaTables, fSrcToDst,
-                                             fDstGammaTables);
-}
-
 template <SkColorSpace::GammaNamed T>
 void SkColorSpaceXform_Base<T>
 ::applyToF16(RGBAF16* dst, const RGBA32* src, int len) const
diff --git a/src/core/SkColorSpaceXform.h b/src/core/SkColorSpaceXform.h
index 2696b8582a..0a5b35c489 100644
--- a/src/core/SkColorSpaceXform.h
+++ b/src/core/SkColorSpaceXform.h
@@ -15,7 +15,6 @@ class SkColorSpaceXform : SkNoncopyable {
 public:
 
     typedef uint32_t RGBA32;
-    typedef uint32_t BGRA32;
     typedef uint64_t RGBAF16;
 
     /**
@@ -33,8 +32,7 @@ public:
      *  The src is stored as RGBA (8888) and is treated as opaque.
      *  TODO (msarett): Support non-opaque srcs.
      */
-    virtual void applyToRGBA(RGBA32* dst, const RGBA32* src, int len) const = 0;
-    virtual void applyToBGRA(BGRA32* dst, const RGBA32* src, int len) const = 0;
+    virtual void applyTo8888(SkPMColor* dst, const RGBA32* src, int len) const = 0;
     virtual void applyToF16(RGBAF16* dst, const RGBA32* src, int len) const = 0;
 
     virtual ~SkColorSpaceXform() {}
@@ -44,8 +42,7 @@ template <SkColorSpace::GammaNamed Dst>
 class SkColorSpaceXform_Base : public SkColorSpaceXform {
 public:
 
-    void applyToRGBA(RGBA32* dst, const RGBA32* src, int len) const override;
-    void applyToBGRA(BGRA32* dst, const RGBA32* src, int len) const override;
+    void applyTo8888(SkPMColor* dst, const RGBA32* src, int len) const override;
     void applyToF16(RGBAF16* dst, const RGBA32* src, int len) const override;
 
     static constexpr int      kDstGammaTableSize = 1024;
diff --git a/src/core/SkOpts.cpp b/src/core/SkOpts.cpp
index 9ba7bc7178..c577bf9603 100644
--- a/src/core/SkOpts.cpp
+++ b/src/core/SkOpts.cpp
@@ -76,9 +76,6 @@ namespace SkOpts {
     DEFINE_DEFAULT(color_xform_RGB1_to_srgb);
     DEFINE_DEFAULT(color_xform_RGB1_to_table);
     DEFINE_DEFAULT(color_xform_RGB1_to_linear);
-    DEFINE_DEFAULT(color_xform_RGB1_to_2dot2_swaprb);
-    DEFINE_DEFAULT(color_xform_RGB1_to_srgb_swaprb);
-    DEFINE_DEFAULT(color_xform_RGB1_to_table_swaprb);
 #undef DEFINE_DEFAULT
 
     // Each Init_foo() is defined in src/opts/SkOpts_foo.cpp.
diff --git a/src/core/SkOpts.h b/src/core/SkOpts.h
index 87489ee94e..fc424e1473 100644
--- a/src/core/SkOpts.h
+++ b/src/core/SkOpts.h
@@ -80,16 +80,7 @@ namespace SkOpts {
     extern void (*color_xform_RGB1_to_linear)(uint64_t* dst, const uint32_t* src, int len,
                                               const float* const srcTables[3],
                                               const float srcToDstMatrix[16]);
-    extern void (*color_xform_RGB1_to_2dot2_swaprb) (uint32_t* dst, const uint32_t* src, int len,
-                                                     const float* const srcTables[3],
-                                                     const float srcToDstMatrix[16]);
-    extern void (*color_xform_RGB1_to_srgb_swaprb)(uint32_t* dst, const uint32_t* src, int len,
-                                                   const float* const srcTables[3],
-                                                   const float srcToDstMatrix[16]);
-    extern void (*color_xform_RGB1_to_table_swaprb)(uint32_t* dst, const uint32_t* src, int len,
-                                                    const float* const srcTables[3],
-                                                    const float srcToDstMatrix[16],
-                                                    const uint8_t* const dstTables[3]);
+
 }
 
 #endif//SkOpts_DEFINED
diff --git a/src/opts/SkColorXform_opts.h b/src/opts/SkColorXform_opts.h
index b3da55c1fd..b5b7f81bbf 100644
--- a/src/opts/SkColorXform_opts.h
+++ b/src/opts/SkColorXform_opts.h
@@ -41,19 +41,10 @@ enum DstGamma {
     kLinear_DstGamma,
 };
 
-template <DstGamma kDstGamma, bool kSwapRB>
+template <DstGamma kDstGamma>
 static void color_xform_RGB1(void* dst, const uint32_t* src, int len,
                              const float* const srcTables[3], const float matrix[16],
                              const uint8_t* const dstTables[3]) {
-    int kRShift = 0;
-    int kGShift = 8;
-    int kBShift = 16;
-    int kAShift = 24;
-    if (kSwapRB) {
-        kBShift = 0;
-        kRShift = 16;
-    }
-
     Sk4f rXgXbX = Sk4f::Load(matrix +  0),
          rYgYbY = Sk4f::Load(matrix +  4),
          rZgZbZ = Sk4f::Load(matrix +  8),
@@ -86,8 +77,7 @@ static void color_xform_RGB1(void* dst, const uint32_t* src, int len,
             dstBlues  = rXgXbX[2]*reds + rYgYbY[2]*greens + rZgZbZ[2]*blues + rTgTbT[2];
         };
 
-        auto store_4 = [&dstReds, &dstGreens, &dstBlues, &dst, &dstTables, kRShift, kGShift,
-                        kBShift, kAShift] {
+        auto store_4 = [&dstReds, &dstGreens, &dstBlues, &dst, &dstTables] {
             if (kSRGB_DstGamma == kDstGamma || k2Dot2_DstGamma == kDstGamma) {
                 Sk4f (*linear_to_curve)(const Sk4f&) = (kSRGB_DstGamma == kDstGamma) ?
                         sk_linear_to_srgb_needs_trunc : linear_to_2dot2;
@@ -102,10 +92,10 @@ static void color_xform_RGB1(void* dst, const uint32_t* src, int len,
                 dstGreens = sk_clamp_0_255(dstGreens);
                 dstBlues  = sk_clamp_0_255(dstBlues);
 
-                auto rgba = (float_to_int(dstReds)   << kRShift)
-                          | (float_to_int(dstGreens) << kGShift)
-                          | (float_to_int(dstBlues)  << kBShift)
-                          | (Sk4i{0xFF}              << kAShift);
+                auto rgba = (float_to_int(dstReds)   << SK_R32_SHIFT)
+                          | (float_to_int(dstGreens) << SK_G32_SHIFT)
+                          | (float_to_int(dstBlues)  << SK_B32_SHIFT)
+                          | (Sk4i{0xFF}              << SK_A32_SHIFT);
                 rgba.store((uint32_t*) dst);
 
                 dst = SkTAddOffset<void>(dst, 4 * sizeof(uint32_t));
@@ -119,22 +109,22 @@ static void color_xform_RGB1(void* dst, const uint32_t* src, int len,
                 Sk4i indicesBlues  = Sk4f_round(scaledBlues);
 
                 uint32_t* dst32 = (uint32_t*) dst;
-                dst32[0] = dstTables[0][indicesReds  [0]] << kRShift
-                         | dstTables[1][indicesGreens[0]] << kGShift
-                         | dstTables[2][indicesBlues [0]] << kBShift
-                         | 0xFF                           << kAShift;
-                dst32[1] = dstTables[0][indicesReds  [1]] << kRShift
-                         | dstTables[1][indicesGreens[1]] << kGShift
-                         | dstTables[2][indicesBlues [1]] << kBShift
-                         | 0xFF                           << kAShift;
-                dst32[2] = dstTables[0][indicesReds  [2]] << kRShift
-                         | dstTables[1][indicesGreens[2]] << kGShift
-                         | dstTables[2][indicesBlues [2]] << kBShift
-                         | 0xFF                           << kAShift;
-                dst32[3] = dstTables[0][indicesReds  [3]] << kRShift
-                         | dstTables[1][indicesGreens[3]] << kGShift
-                         | dstTables[2][indicesBlues [3]] << kBShift
-                         | 0xFF                           << kAShift;
+                dst32[0] = dstTables[0][indicesReds  [0]] << SK_R32_SHIFT
+                         | dstTables[1][indicesGreens[0]] << SK_G32_SHIFT
+                         | dstTables[2][indicesBlues [0]] << SK_B32_SHIFT
+                         | 0xFF                           << SK_A32_SHIFT;
+                dst32[1] = dstTables[0][indicesReds  [1]] << SK_R32_SHIFT
+                         | dstTables[1][indicesGreens[1]] << SK_G32_SHIFT
+                         | dstTables[2][indicesBlues [1]] << SK_B32_SHIFT
+                         | 0xFF                           << SK_A32_SHIFT;
+                dst32[2] = dstTables[0][indicesReds  [2]] << SK_R32_SHIFT
+                         | dstTables[1][indicesGreens[2]] << SK_G32_SHIFT
+                         | dstTables[2][indicesBlues [2]] << SK_B32_SHIFT
+                         | 0xFF                           << SK_A32_SHIFT;
+                dst32[3] = dstTables[0][indicesReds  [3]] << SK_R32_SHIFT
+                         | dstTables[1][indicesGreens[3]] << SK_G32_SHIFT
+                         | dstTables[2][indicesBlues [3]] << SK_B32_SHIFT
+                         | 0xFF                           << SK_A32_SHIFT;
 
                 dst = SkTAddOffset<void>(dst, 4 * sizeof(uint32_t));
             } else {
@@ -177,21 +167,17 @@ static void color_xform_RGB1(void* dst, const uint32_t* src, int len,
             uint32_t rgba;
             SkNx_cast<uint8_t>(float_to_int(dstPixel)).store(&rgba);
             rgba |= 0xFF000000;
-            if (kSwapRB) {
-                *((uint32_t*) dst) = SkSwizzle_RB(rgba);
-            } else {
-                *((uint32_t*) dst) = rgba;
-            }
+            *((uint32_t*) dst) = SkSwizzle_RGBA_to_PMColor(rgba);
             dst = SkTAddOffset<void>(dst, sizeof(uint32_t));
         } else if (kTable_DstGamma == kDstGamma) {
             Sk4f scaledPixel = Sk4f::Min(Sk4f::Max(1023.0f * dstPixel, 0.0f), 1023.0f);
 
             Sk4i indices = Sk4f_round(scaledPixel);
 
-            *((uint32_t*) dst) = dstTables[0][indices[0]] << kRShift
-                               | dstTables[1][indices[1]] << kGShift
-                               | dstTables[2][indices[2]] << kBShift
-                               | 0xFF                     << kAShift;
+            *((uint32_t*) dst) = dstTables[0][indices[0]] << SK_R32_SHIFT
+                               | dstTables[1][indices[1]] << SK_G32_SHIFT
+                               | dstTables[2][indices[2]] << SK_B32_SHIFT
+                               | 0xFF                     << SK_A32_SHIFT;
 
             dst = SkTAddOffset<void>(dst, sizeof(uint32_t));
         } else {
@@ -209,42 +195,23 @@ static void color_xform_RGB1(void* dst, const uint32_t* src, int len,
 
 static void color_xform_RGB1_to_2dot2(uint32_t* dst, const uint32_t* src, int len,
                                       const float* const srcTables[3], const float matrix[16]) {
-    color_xform_RGB1<k2Dot2_DstGamma, false>(dst, src, len, srcTables, matrix, nullptr);
+    color_xform_RGB1<k2Dot2_DstGamma>(dst, src, len, srcTables, matrix, nullptr);
 }
 
 static void color_xform_RGB1_to_srgb(uint32_t* dst, const uint32_t* src, int len,
                                      const float* const srcTables[3], const float matrix[16]) {
-    color_xform_RGB1<kSRGB_DstGamma, false>(dst, src, len, srcTables, matrix, nullptr);
+    color_xform_RGB1<kSRGB_DstGamma>(dst, src, len, srcTables, matrix, nullptr);
 }
 
 static void color_xform_RGB1_to_table(uint32_t* dst, const uint32_t* src, int len,
                                       const float* const srcTables[3], const float matrix[16],
                                       const uint8_t* const dstTables[3]) {
-    color_xform_RGB1<kTable_DstGamma, false>(dst, src, len, srcTables, matrix, dstTables);
+    color_xform_RGB1<kTable_DstGamma>(dst, src, len, srcTables, matrix, dstTables);
 }
 
 static void color_xform_RGB1_to_linear(uint64_t* dst, const uint32_t* src, int len,
                                        const float* const srcTables[3], const float matrix[16]) {
-    color_xform_RGB1<kLinear_DstGamma, false>(dst, src, len, srcTables, matrix, nullptr);
-}
-
-static void color_xform_RGB1_to_2dot2_swaprb(uint32_t* dst, const uint32_t* src, int len,
-                                             const float* const srcTables[3],
-                                             const float matrix[16]) {
-    color_xform_RGB1<k2Dot2_DstGamma, true>(dst, src, len, srcTables, matrix, nullptr);
-}
-
-static void color_xform_RGB1_to_srgb_swaprb(uint32_t* dst, const uint32_t* src, int len,
-                                            const float* const srcTables[3],
-                                            const float matrix[16]) {
-    color_xform_RGB1<kSRGB_DstGamma, true>(dst, src, len, srcTables, matrix, nullptr);
-}
-
-static void color_xform_RGB1_to_table_swaprb(uint32_t* dst, const uint32_t* src, int len,
-                                             const float* const srcTables[3],
-                                             const float matrix[16],
-                                             const uint8_t* const dstTables[3]) {
-    color_xform_RGB1<kTable_DstGamma, true>(dst, src, len, srcTables, matrix, dstTables);
+    color_xform_RGB1<kLinear_DstGamma>(dst, src, len, srcTables, matrix, nullptr);
 }
 
 }  // namespace SK_OPTS_NS
diff --git a/src/opts/SkOpts_sse41.cpp b/src/opts/SkOpts_sse41.cpp
index e70cedeb31..e1e024d948 100644
--- a/src/opts/SkOpts_sse41.cpp
+++ b/src/opts/SkOpts_sse41.cpp
@@ -21,12 +21,9 @@ namespace SkOpts {
         srcover_srgb_srgb    = sse41::srcover_srgb_srgb;
         blit_row_s32a_opaque = sse41::blit_row_s32a_opaque;
 
-        color_xform_RGB1_to_2dot2        = sse41::color_xform_RGB1_to_2dot2;
-        color_xform_RGB1_to_srgb         = sse41::color_xform_RGB1_to_srgb;
-        color_xform_RGB1_to_table        = sse41::color_xform_RGB1_to_table;
-        color_xform_RGB1_to_linear       = sse41::color_xform_RGB1_to_linear;
-        color_xform_RGB1_to_2dot2_swaprb = sse41::color_xform_RGB1_to_2dot2_swaprb;
-        color_xform_RGB1_to_srgb_swaprb  = sse41::color_xform_RGB1_to_srgb_swaprb;
-        color_xform_RGB1_to_table_swaprb = sse41::color_xform_RGB1_to_table_swaprb;
+        color_xform_RGB1_to_2dot2  = sse41::color_xform_RGB1_to_2dot2;
+        color_xform_RGB1_to_srgb   = sse41::color_xform_RGB1_to_srgb;
+        color_xform_RGB1_to_table  = sse41::color_xform_RGB1_to_table;
+        color_xform_RGB1_to_linear = sse41::color_xform_RGB1_to_linear;
     }
 }