Add NEON swap opts and use opts in SkSwizzler

src/codec/SkSwizzler.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
 #include "SkOpts.h"
@@ skipping 285 matching lines @@
         int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         dst[x] = SkPackARGB32NoCheck(0xFF, src[2], src[1], src[0]);
         src += deltaSrc;
     }
 }
 
+static void fast_swizzle_bgrx_to_n32(
+        void* dst, const uint8_t* src, int width, int bpp, int deltaSrc, int offset,
+        const SkPMColor ctable[]) {
+
+    // This function must not be called if we are sampling.  If we are not
+    // sampling, deltaSrc should equal bpp.
+    SkASSERT(deltaSrc == bpp);
+
+    // The default swizzle supports BGR->N32 and BGRX->N32.  This only
+    // supports BGRX->N32.
+    SkASSERT(4 == bpp);
+
+    // These swizzles trust that the alpha value is already 0xFF.
+#ifdef SK_PMCOLOR_IS_RGBA
+    SkOpts::swaprb_xxxa((uint32_t*) dst, (const uint32_t*) (src + offset), width);
+#else
+    memcpy(dst, src + offset, width * bpp);

[msarett, 2016/01/14 17:08:25]

This could be a no-op if we decoded directly into dst.  I think this makes an
argument to try decoding directly into dst.

I could see us making dst == src on decodes where the SrcConfig and DstColorType
have the same number of bytes per pixel.

One potential complication is subset decodes (where offset is non-zero).

[scroggo, 2016/01/14 17:55:06]

+1

+#endif
+}
+
 static void swizzle_bgrx_to_565(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
         int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         dst[x] = SkPack888ToRGB16(src[2], src[1], src[0]);
         src += deltaSrc;
     }
@@ skipping 20 matching lines @@
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         uint8_t alpha = src[3];
         dst[x] = SkPremultiplyARGBInline(alpha, src[2], src[1], src[0]);
         src += deltaSrc;
     }
 }
 
+static void fast_swizzle_bgra_to_n32_premul(

[scroggo, 2016/01/14 17:55:06]

Should we have a fast_swizzle to unpremul, as well?

Also, we've talked about decoding to the non-native 32 bit version. Should we
add those along with these changes or hold off?

[msarett, 2016/01/14 18:23:19]

Sorry I missed this.

This is implemented in this CL also.  It uses fast_swizzle_bgrx_to_n32 since the
code is identical.  I originally had them separated, but then I realized that
the code was the same.

[scroggo, 2016/01/14 19:08:06]

Ah, then should these functions be named something other than "n32"? _to_32?

[msarett, 2016/01/14 19:45:55]

Changed to _to_32.

+        void* dst, const uint8_t* src, int width, int bpp, int deltaSrc, int offset,
+        const SkPMColor ctable[]) {
+
+    // This function must not be called if we are sampling.  If we are not
+    // sampling, deltaSrc should equal bpp.
+    SkASSERT(deltaSrc == bpp);
+
+#ifdef SK_PMCOLOR_IS_RGBA
+    SkOpts::premul_swaprb_xxxa((uint32_t*) dst, (const uint32_t*) (src + offset), width);
+#else
+    SkOpts::premul_xxxa((uint32_t*) dst, (const uint32_t*) (src + offset), width);
+#endif
+}
+
 // kRGBX
+
 static void swizzle_rgbx_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
         int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         dst[x] = SkPackARGB32(0xFF, src[0], src[1], src[2]);
         src += deltaSrc;
     }
 }
 
+static void fast_swizzle_rgbx_to_n32(
+        void* dst, const uint8_t* src, int width, int bpp, int deltaSrc, int offset,
+        const SkPMColor ctable[]) {
+
+    // This function must not be called if we are sampling.  If we are not
+    // sampling, deltaSrc should equal bpp.
+    SkASSERT(deltaSrc == bpp);
+
+    // The default swizzle supports RGB->N32 and RGBX->N32.  This only
+    // supports RGBX->N32.
+    SkASSERT(4 == bpp);
+
+    // These swizzles trust that the alpha value is already 0xFF.
+#ifdef SK_PMCOLOR_IS_RGBA
+    memcpy(dst, src + offset, width * bpp);
+#else
+    SkOpts::swaprb_xxxa((uint32_t*) dst, (const uint32_t*) (src + offset), width);
+#endif
+}
+
 static void swizzle_rgbx_to_565(
        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
        int bytesPerPixel, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         dst[x] = SkPack888ToRGB16(src[0], src[1], src[2]);
         src += deltaSrc;
     }
 }
 
 // kRGBA
+
 static void swizzle_rgba_to_n32_premul(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
         int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         unsigned alpha = src[3];
         dst[x] = SkPremultiplyARGBInline(alpha, src[0], src[1], src[2]);
         src += deltaSrc;
@@ skipping 205 matching lines @@
                 case kGray_8_SkColorType:
                     proc = &swizzle_gray_to_gray;
                     break;
                 case kRGB_565_SkColorType:
                     proc = &swizzle_gray_to_565;
                     break;
                 default:
                     break;
             }
             break;
-        case kBGR:
         case kBGRX:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
                     proc = &swizzle_bgrx_to_n32;
+                    fastProc = &fast_swizzle_bgrx_to_n32;
                     break;
                 case kRGB_565_SkColorType:
                     proc = &swizzle_bgrx_to_565;
                     break;
                 default:
                     break;
             }
             break;
         case kBGRA:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
-                    switch (dstInfo.alphaType()) {
-                        case kUnpremul_SkAlphaType:
+                    if (dstInfo.alphaType() == kUnpremul_SkAlphaType) {
+                        if (SkCodec::kYes_ZeroInitialized == zeroInit) {
+                            proc = &SkipLeading8888ZerosThen<swizzle_bgra_to_n32_unpremul>;
+                            fastProc = &SkipLeading8888ZerosThen<fast_swizzle_bgrx_to_n32>;
+                        } else {
                             proc = &swizzle_bgra_to_n32_unpremul;
-                            break;
-                        case kPremul_SkAlphaType:
+                            fastProc = &fast_swizzle_bgrx_to_n32;
+                        }
+                    } else {
+                        if (SkCodec::kYes_ZeroInitialized == zeroInit) {
+                            proc = &SkipLeading8888ZerosThen<swizzle_bgra_to_n32_premul>;
+                            fastProc = &SkipLeading8888ZerosThen<fast_swizzle_bgra_to_n32_premul>;
+                        } else {
                             proc = &swizzle_bgra_to_n32_premul;
-                            break;
-                        default:
-                            break;
+                            fastProc = &fast_swizzle_bgra_to_n32_premul;
+                        }
                     }
                     break;
                 default:
                     break;
             }
             break;
         case kRGBX:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
                     proc = &swizzle_rgbx_to_n32;
+                    fastProc = &fast_swizzle_rgbx_to_n32;
                     break;
                 case kRGB_565_SkColorType:
                     proc = &swizzle_rgbx_to_565;
                 default:
                     break;
             }
             break;
         case kRGBA:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
                     if (dstInfo.alphaType() == kUnpremul_SkAlphaType) {
                         if (SkCodec::kYes_ZeroInitialized == zeroInit) {
                             proc = &SkipLeading8888ZerosThen<swizzle_rgba_to_n32_unpremul>;
+                            fastProc = &SkipLeading8888ZerosThen<fast_swizzle_rgbx_to_n32>;
                         } else {
                             proc = &swizzle_rgba_to_n32_unpremul;
+                            fastProc = &fast_swizzle_rgbx_to_n32;
                         }
                     } else {
                         if (SkCodec::kYes_ZeroInitialized == zeroInit) {
                             proc = &SkipLeading8888ZerosThen<swizzle_rgba_to_n32_premul>;
                             fastProc = &SkipLeading8888ZerosThen<fast_swizzle_rgba_to_n32_premul>;
                         } else {
                             proc = &swizzle_rgba_to_n32_premul;
                             fastProc = &fast_swizzle_rgba_to_n32_premul;
                         }
                     }
                     break;
                 default:
                     break;
             }
             break;
         case kRGB:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
                     proc = &swizzle_rgbx_to_n32;
                     break;
+                case kRGB_565_SkColorType:

[msarett, 2016/01/14 17:08:25]

We have never needed this because we "fill" RGB PNGs to RGB(FF) in libpng.

We will stop doing this in libpng because it is slow and an extra pass over the
output.

[scroggo, 2016/01/14 17:55:06]

Is that a separate (future) CL?

[msarett, 2016/01/14 18:09:09]

Yes.  Maybe this doesn't belong in this CL?

[scroggo, 2016/01/14 18:14:51]

If it's only needed once we turn off filling, then I don't think it belongs.
(Unless it makes the rest of this code simpler?)

[msarett, 2016/01/14 18:27:51]

Done.

+                    proc = &swizzle_rgbx_to_565;
+                    break;
                 default:
                     break;
             }
+            break;
+        case kBGR:

[msarett, 2016/01/14 17:08:25]

This will diverge from BGRX because of the opt functions.

[scroggo, 2016/01/14 18:14:51]

Will there be opts for BGR? Or is that not conducive to using SIMD? Would it be
faster to make two passes: one to fill (in libpng) and then use SIMD to swizzle?
I'm guessing not?

[msarett, 2016/01/14 18:23:19]

Yes, I plan on opts for BGR and RGB (coming soon).

What you've suggested is an option, but I think we can do better than that.

+            switch (dstInfo.colorType()) {
+                case kN32_SkColorType:
+                    proc = &swizzle_bgrx_to_n32;
+                    break;
+                case kRGB_565_SkColorType:
+                    proc = &swizzle_bgrx_to_565;
+                    break;
+                default:
+                    break;
+            }
             break;
         case kRGB_565:
             switch (dstInfo.colorType()) {
                 case kRGB_565_SkColorType:
                     proc = &sample565;
                     break;
                 default:
                     break;
             }
             break;
@@ skipping 69 matching lines @@
 
     return fAllocatedWidth;
 }
 
 void SkSwizzler::swizzle(void* dst, const uint8_t* SK_RESTRICT src) {
     SkASSERT(nullptr != dst && nullptr != src);
     RowProc proc = fFastProc ? fFastProc : fProc;
     proc(SkTAddOffset<void>(dst, fDstOffsetBytes), src, fSwizzleWidth, fSrcBPP,
             fSampleX * fSrcBPP, fSrcOffsetUnits, fColorTable);
 }