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

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

Index: Source/core/platform/image-decoders/ImageDecoder.h
diff --git a/Source/core/platform/image-decoders/ImageDecoder.h b/Source/core/platform/image-decoders/ImageDecoder.h
index 724e27c918dc3e3c7a5dcd00f2ef98bfa3cd2aeb..27b08a61eee9d8eac74be358be5db3e19994b723 100644
--- a/Source/core/platform/image-decoders/ImageDecoder.h
+++ b/Source/core/platform/image-decoders/ImageDecoder.h
@@ -178,7 +178,125 @@ namespace WebCore {
             *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();