Improve PNG decode performance: avoid branching in the row write loop

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 156 matching lines @@
                 r = (r * alpha) >> 24;
                 g = (g * alpha) >> 24;
                 b = (b * alpha) >> 24;
             }
 
             // 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 setRGBAPremultiply(PixelData* dest, unsigned r, unsigned g, unsigned b, unsigned a)

[Peter Kasting, 2013/07/10 18:24:47]

I think we ought to be able to define setRGBA() in terms of these next two
functions.

I'm aware that we're trying to avoid the GIF decoder regression that came from
doing this sort of thing previously.  I think the main problem with that patch
was that its version of setRGBAPremultiply() moved the "if (!a)" clause outside
the "if (a < 255)" clause, resulting in the common case doing two comparisons
instead of one.  Since you haven't done that here, I think this patch ought to
be more performant than that one.  Can we try it?

[Noel Gordon, 2013/07/11 08:23:40]

Seems perfectly reasonable ...
In https://chromiumcodereview.appspot.com/17448010#msg12 I mentioned the exact
same thing to b.keleman post-facto in review after his patch landed, viz., why
was the if (!a) test moved?  He prepared a change to put it back to where it
was, and we landed that:

https://src.chromium.org/viewvc/blink?revision=153264&view=revision

but no joy: it did not fix the perf issue as noted in
https://code.google.com/p/chromium/issues/detail?id=254462#c9 
so I believe we'd see the perf regression if we went this way again.

+        {
+            if (a < 255) {
+                if (!a) {
+                    *dest = 0;
+                    return;
+                }
+
+                unsigned alpha = a * div255;
+                r = (r * alpha) >> 24;
+                g = (g * alpha) >> 24;
+                b = (b * alpha) >> 24;
+            }
+
+            *dest = SkPackARGB32NoCheck(a, r, g, b);
+        }
+
         inline void setRGBARaw(PixelData* dest, unsigned r, unsigned g, unsigned b, unsigned a)
         {
             *dest = SkPackARGB32NoCheck(a, r, g, b);
         }
 
     private:
         int width() const
         {
             return m_bitmap->bitmap().width();
         }
@@ skipping 236 matching lines @@
 
         IntSize m_size;
         bool m_sizeAvailable;
         bool m_isAllDataReceived;
         bool m_failed;
     };
 
 } // namespace WebCore
 
 #endif