Make image decoders faster by refactoring hot loops that fills the lines of ImageFrame.

Source/core/platform/image-decoders/ImageDecoder.h

 /*
  * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
  * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 160 matching lines @@
                 unsigned alphaMult = a * fixPointMult;
                 r = fixPointUnsignedMultiply(r, alphaMult);
                 g = fixPointUnsignedMultiply(g, alphaMult);
                 b = fixPointUnsignedMultiply(b, alphaMult);
             }
             // Call the "NoCheck" version since we may deliberately pass non-premultiplied
             // values, and we don't want an assert.
             *dest = SkPackARGB32NoCheck(a, r, g, b);
         }
 
+        inline void fillRowFromRGBSource(int rowIndex, const unsigned char* src)

[Peter Kasting, 2013/06/01 01:22:12]

I don't think we gain much from having these first 4 wrapper functions and then
making the more complex ones private.  The wrappers basically just add a
getAddr() call, and getAddr() is (and will remain) public already, so I'd say
nuke these four and publicize the new functions below, then call those directly.

+        {
+            fillRowFromRGBSource(getAddr(0, rowIndex), src, width());
+        }
+
+        inline void fillRowFromRGBASource(int rowIndex, const unsigned char* src)
+        {
+            bool dummy;
+            fillRowFromRGBASource<false>(getAddr(0, rowIndex), src, width(), dummy);
+        }
+
+        inline void fillRowFromRGBASourceWithNonTrivialAlphaDetection(int rowIndex, const unsigned char *src, bool& hasNonTrivialAlpha)
+        {
+            fillRowFromRGBASource<true>(getAddr(0, rowIndex), src, width(), hasNonTrivialAlpha);
+        }
+
+        inline void fillRowFromInvertedCMYK(int rowIndex, const unsigned char* src)
+        {
+            fillRowFromInvertedCMYK(getAddr(0, rowIndex), src, width());
+        }
+
     private:
+        // We have specialized fill functions for each case. This way we avoid branches in the loop which gives performace.
+        // Note: we could benefit further from turning on auto vectorization in the compiler.
+
+        ALWAYS_INLINE static void fillRowFromRGBSource(PixelData* dst, const unsigned char* src, int numPixels)
+        {
+            const unsigned char* pixel = src;
+            for (int i = 0; i < numPixels; ++i, pixel += 3)
+                dst[i] = SkPackARGB32NoCheck(255, pixel[0], pixel[1], pixel[2]);
+        }
+
+        template <bool shouldUpdate>
+        inline static void updateAlphaMask(unsigned char alpha, unsigned char& mask)

[Peter Kasting, 2013/06/01 01:22:12]

Nit: Honestly, I'm not convinced pulling this into its own function really adds
much.  I'd just inline this.

+        {
+            if (shouldUpdate)
+                mask &= alpha;
+        }
+
+        template <bool shouldUpdateAlphaMask>
+        ALWAYS_INLINE static void fillRowFromRGBASourceNoPremultiply(PixelData* dst, const unsigned char* src, int numPixels, unsigned char& alphaMask)
+        {
+            const unsigned char* pixel = src;
+            for (int i = 0; i < numPixels; ++i, pixel += 4) {
+                unsigned char alpha = pixel[3];
+                dst[i] = SkPackARGB32NoCheck(alpha, pixel[0], pixel[1], pixel[2]);
+                updateAlphaMask<shouldUpdateAlphaMask>(alpha, alphaMask);
+            }
+        }
+
+        template <bool shouldUpdateAlphaMask>
+        ALWAYS_INLINE static void fillRowFromRGBASourcePremultiply(PixelData* dst, const unsigned char* src, int numPixels, unsigned char& alphaMask)
+        {
+            const unsigned char* pixel = src;
+            for (int i = 0; i < numPixels; ++i, pixel += 4) {
+                unsigned char r = pixel[0];
+                unsigned char g = pixel[1];
+                unsigned char b = pixel[2];
+                unsigned char a = pixel[3];
+
+                updateAlphaMask<shouldUpdateAlphaMask>(a, alphaMask);
+
+                // Note: if once we start using auto vectorization we should reevaluate whether the fast paths are
+                // worthwile when alpha is zero or 255. Currently without vectorization they are.
+                if (!a) {
+                    dst[i] = 0;
+                    continue;
+                }
+                if (a < 255) {
+                    unsigned alphaMult = a * fixPointMult;
+                    r = fixPointUnsignedMultiply(r, alphaMult);
+                    g = fixPointUnsignedMultiply(g, alphaMult);
+                    b = fixPointUnsignedMultiply(b, alphaMult);
+                }
+                dst[i] = SkPackARGB32NoCheck(a, r, g, b);
+            }
+        }
+
+        template <bool shouldUpdateHasNonTrivialAlpha>
+        ALWAYS_INLINE void fillRowFromRGBASource(PixelData* dst, const unsigned char* src, int numPixels, bool& hasNonTrivialAlpha)

[Peter Kasting, 2013/06/01 01:22:12]

Nit: If you take a bool* as the last arg, we can pass NULL instead of declaring
a |dummy| above.

+        {
+            unsigned char alphaMask = 255;
+            if (m_premultiplyAlpha)
+                fillRowFromRGBASourcePremultiply<shouldUpdateHasNonTrivialAlpha>(dst, src, numPixels, alphaMask);
+            else
+                fillRowFromRGBASourceNoPremultiply<shouldUpdateHasNonTrivialAlpha>(dst, src, numPixels, alphaMask);
+
+            if (shouldUpdateHasNonTrivialAlpha)
+                hasNonTrivialAlpha = alphaMask != 255;
+        }
+
+        ALWAYS_INLINE static void fillRowFromInvertedCMYK(PixelData* dst, const unsigned char* src, int numPixels)
+        {
+            // Source is 'Inverted CMYK', output is RGB.
+            // See: http://www.easyrgb.com/math.php?MATH=M12#text12
+            // Or: http://www.ilkeratalay.com/colorspacesfaq.php#rgb
+            // From CMYK to CMY:
+            // X =   X    * (1 -   K   ) +   K  [for X = C, M, or Y]
+            // Thus, from Inverted CMYK to CMY is:
+            // X = (1-iX) * (1 - (1-iK)) + (1-iK) => 1 - iX*iK
+            // From CMY (0..1) to RGB (0..1):
+            // R = 1 - C => 1 - (1 - iC*iK) => iC*iK  [G and B similar]
+
+            const unsigned char* pixel = src;
+            for (int i = 0; i < numPixels; ++i, pixel += 4) {
+                unsigned char c = pixel[0];
+                unsigned char m = pixel[1];
+                unsigned char y = pixel[2];
+                unsigned char k = pixel[3];
+
+                unsigned kMult = k * fixPointMult;
+                c = fixPointUnsignedMultiply(c, kMult);
+                m = fixPointUnsignedMultiply(m, kMult);
+                y = fixPointUnsignedMultiply(y, kMult);
+
+                dst[i] = SkPackARGB32NoCheck(255, c, m, y);

[Peter Kasting, 2013/06/01 01:22:12]

It strikes me that this function is effectively identical to
fillRowFromRGBASourcePremultiply except for:

* That function adds an updateAlphaMask() call; callers of this could pass false
for that template arg
* That function adds fastpaths for 0/255; seems like those might be a win here
if they're a win there?  (Not sure)
* This function passes 255 to the first arg of SkPackARGB32NoCheck()

If the last bullet becomes the only difference, we could basically combine these
two into a single function which takes an is_cmyk template arg and passes
"is_cmyk ? 255 : a" as the first arg to SkPackARGB32NoCheck().

I am always a fan of condensing code when it can be done without compromising
performance.

+            }
+        }
+
         int width() const
         {
             return m_bitmap->bitmap().width();
         }
 
         int height() const
         {
             return m_bitmap->bitmap().height();
         }
 
@@ skipping 204 matching lines @@
 
         IntSize m_size;
         bool m_sizeAvailable;
         bool m_isAllDataReceived;
         bool m_failed;
     };
 
 } // namespace WebCore
 
 #endif