Changeset 136189 in webkit

Timestamp:

Nov 29, 2012, 5:15:15 PM (14 years ago)

Author:

commit-queue@webkit.org

Message:

Optimization in image decoding.
​https://bugs.webkit.org/show_bug.cgi?id=88424

Patch by Viatcheslav Ostapenko <​v.ostapenko@samsung.com> on 2012-11-29
Reviewed by Brent Fulgham.

Reduce branching and multiplications in JPEG image decoding loops and functions.
Code is moved to the template functions with scale and color space template parameters
because they were reason of branches inside loops. With templated funtions compiler
will generate separate instance of function for every set of parameters removing
unreachable code in every condition where constant value is used.

Rebase and update of original patch by Misha Tyutyunik <​michael.tyuytunik@nokia.com> .

Thanks to Noel Gordon for his help in cleaning up remaining issues.

Covered by existing tests.

• platform/image-decoders/jpeg/JPEGImageDecoder.cpp:

(WebCore):
(WebCore::setPixel):
(WebCore::JPEGImageDecoder::outputScanlines):

• platform/image-decoders/jpeg/JPEGImageDecoder.h:

(JPEGImageDecoder):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/image-decoders/jpeg/JPEGImageDecoder.cpp (modified) (2 diffs)
• platform/image-decoders/jpeg/JPEGImageDecoder.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-11-29  Viatcheslav Ostapenko  <v.ostapenko@samsung.com>
+
+        Optimization in image decoding.
+        https://bugs.webkit.org/show_bug.cgi?id=88424
+
+        Reviewed by Brent Fulgham.
+
+        Reduce branching and multiplications in JPEG image decoding loops and functions.
+        Code is moved to the template functions with scale and color space template parameters
+        because they were reason of branches inside loops. With templated funtions compiler
+        will generate separate instance of function for every set of parameters removing
+        unreachable code in every condition where constant value is used.
+
+        Rebase and update of original patch by Misha Tyutyunik <michael.tyuytunik@nokia.com> .
+
+        Thanks to Noel Gordon for his help in cleaning up remaining issues.
+
+        Covered by existing tests.
+
+        * platform/image-decoders/jpeg/JPEGImageDecoder.cpp:
+        (WebCore):
+        (WebCore::setPixel):
+        (WebCore::JPEGImageDecoder::outputScanlines):
+        * platform/image-decoders/jpeg/JPEGImageDecoder.h:
+        (JPEGImageDecoder):
+
 2012-11-29  Kentaro Hara  <haraken@chromium.org>
 

trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp

@@ -647 +647 @@
 }
 
+template <int colorSpace>
+void setPixel(ImageFrame& buffer, ImageFrame::PixelData* currentAddress, JSAMPARRAY samples, int column)
+{
+    JSAMPLE* jsample = *samples + column * (static_cast<J_COLOR_SPACE>(colorSpace) == JCS_RGB ? 3 : 4);
+
+    switch (static_cast<J_COLOR_SPACE>(colorSpace)) {
+#if defined(TURBO_JPEG_RGB_SWIZZLE)
+    case JCS_EXT_BGRA:
+        buffer.setRGBA(currentAddress, jsample[2], jsample[1], jsample[1], 0xFF);
+        break;
+    case JCS_EXT_RGBA: // Fallback to JSC_RGB case here.
+#endif
+    case JCS_RGB:
+        buffer.setRGBA(currentAddress, jsample[0], jsample[1], jsample[2], 0xFF);
+        break;
+    case JCS_CMYK:
+        // 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]
+        unsigned k = jsample[3];
+        buffer.setRGBA(currentAddress, jsample[0] * k / 255, jsample[1] * k / 255, jsample[2] * k / 255, 0xFF);
+        break;
+    }
+}
+
+template <int colorSpace, bool isScaled>
+bool JPEGImageDecoder::outputScanlines(ImageFrame& buffer)
+{
+    JSAMPARRAY samples = m_reader->samples();
+    jpeg_decompress_struct* info = m_reader->info();
+
+    int width = isScaled ? m_scaledColumns.size() : info->output_width;
+
+    while (info->output_scanline < info->output_height) {
+        // jpeg_read_scanlines will increase the scanline counter, so we
+        // save the scanline before calling it.
+        int sourceY = info->output_scanline;
+        /* Request one scanline.  Returns 0 or 1 scanlines. */
+        if (jpeg_read_scanlines(info, samples, 1) != 1)
+            return false;
+
+        int destY = scaledY(sourceY);
+        if (destY < 0)
+            continue;
+
+#if USE(QCMSLIB)
+        if (m_reader->colorTransform() && colorSpace == JCS_RGB)
+            qcms_transform_data(m_reader->colorTransform(), *samples, *samples, info->output_width);
+#endif
+
+        ImageFrame::PixelData* currentAddress = buffer.getAddr(0, destY);
+
+        for (int x = 0; x < width; ++x) {
+            setPixel<colorSpace>(buffer, currentAddress, samples, isScaled ? m_scaledColumns[x] : x);
+            ++currentAddress;
+        }
+    }
+    return true;
+}
+
+template <int colorSpace>
+bool JPEGImageDecoder::outputScanlines(ImageFrame& buffer)
+{
+    return m_scaled ? outputScanlines<colorSpace, true>(buffer) : outputScanlines<colorSpace, false>(buffer);
+}
+
 bool JPEGImageDecoder::outputScanlines()
 {
@@ -685 +757 @@
 #endif
 
-    JSAMPARRAY samples = m_reader->samples();
-
-    while (info->output_scanline < info->output_height) {
-        // jpeg_read_scanlines will increase the scanline counter, so we
-        // save the scanline before calling it.
-        int sourceY = info->output_scanline;
-        /* Request one scanline.  Returns 0 or 1 scanlines. */
-        if (jpeg_read_scanlines(info, samples, 1) != 1)
-            return false;
-
-        int destY = scaledY(sourceY);
-        if (destY < 0)
-            continue;
-#if USE(QCMSLIB)
-        if (m_reader->colorTransform() && info->out_color_space == JCS_RGB)
-            qcms_transform_data(m_reader->colorTransform(), *samples, *samples, info->output_width);
-#endif
-        int width = m_scaled ? m_scaledColumns.size() : info->output_width;
-        for (int x = 0; x < width; ++x) {
-            JSAMPLE* jsample = *samples + (m_scaled ? m_scaledColumns[x] : x) * ((info->out_color_space == JCS_RGB) ? 3 : 4);
-            if (info->out_color_space == JCS_RGB)
-                buffer.setRGBA(x, destY, jsample[0], jsample[1], jsample[2], 0xFF);
+    switch (info->out_color_space) {
+    // The code inside outputScanlines<int, bool> will be executed
+    // for each pixel, so we want to avoid any extra comparisons there.
+    // That is why we use template and template specializations here so
+    // the proper code will be generated at compile time.
+    case JCS_RGB:
+        return outputScanlines<JCS_RGB>(buffer);
 #if defined(TURBO_JPEG_RGB_SWIZZLE)
-            else if (info->out_color_space == JCS_EXT_RGBA)
-                buffer.setRGBA(x, destY, jsample[0], jsample[1], jsample[2], 0xFF);
-            else if (info->out_color_space == JCS_EXT_BGRA)
-                buffer.setRGBA(x, destY, jsample[2], jsample[1], jsample[0], 0xFF);
-#endif
-            else if (info->out_color_space == JCS_CMYK) {
-                // 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]
-                unsigned k = jsample[3];
-                buffer.setRGBA(x, destY, jsample[0] * k / 255, jsample[1] * k / 255, jsample[2] * k / 255, 0xFF);
-            } else {
-                ASSERT_NOT_REACHED();
-                return setFailed();
-            }
-        }
-    }
-
-    return true;
+    case JCS_EXT_RGBA:
+        return outputScanlines<JCS_EXT_RGBA>(buffer);
+    case JCS_EXT_BGRA:
+        return outputScanlines<JCS_EXT_BGRA>(buffer);
+#endif
+    case JCS_CMYK:
+        return outputScanlines<JCS_CMYK>(buffer);
+    default:
+        ASSERT_NOT_REACHED();
+    }
+
+    return setFailed();
 }
 

trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.h

@@ -63 +63 @@
         void decode(bool onlySize);
 
+        template <int colorSpace>
+        bool outputScanlines(ImageFrame& buffer);
+
+        template <int colorSpace, bool isScaled>
+        bool outputScanlines(ImageFrame& buffer);
+
         OwnPtr<JPEGImageReader> m_reader;
     };