[Qt] Make ImageBufferQt use premultiplied converting functions in Color.h instead...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 19:14:53 +0000 (19:14 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Oct 2012 19:14:53 +0000 (19:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=98582

Patch by Huang Dongsung <luxtella@company100.net> on 2012-10-08
Reviewed by Eric Seidel.

Source/WebCore:

Only Qt port failed canvas/philip/tests/2d.imageData.put.unchanged.html because
ImageBufferQt converts from or to a premultiplied color using its own code. When
we convert twice from an unmultiplied color to a premultiplied color and then to
an unmultiplied color using this code, the calculated result are pretty
different against the input color. It is why Gtk passes this test although Qt
fails. Gtk port uses them in Color.h.

In addition, Qt should use the functions in Color.h to reduce duplicated codes.

Tests: canvas/philip/tests/2d.imageData.put.unchanged.html: Fixed a test case

* platform/graphics/qt/ImageBufferQt.cpp:
(WebCore::getImageData):
(WebCore::convertBGRAToRGBA):
(WebCore::ImageBuffer::putByteArray):

LayoutTests:

Unskipping canvas/philip/tests/2d.imageData.put.unchanged.html that is passing.

* platform/qt/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@130672 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/qt/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp

index 2997bab..6dd86bf 100644 (file)
@@ -1,3 +1,14 @@
+2012-10-08  Huang Dongsung  <luxtella@company100.net>
+
+        [Qt] Make ImageBufferQt use premultiplied converting functions in Color.h instead of its own mechanism.
+        https://bugs.webkit.org/show_bug.cgi?id=98582
+
+        Reviewed by Eric Seidel.
+
+        Unskipping canvas/philip/tests/2d.imageData.put.unchanged.html that is passing.
+
+        * platform/qt/TestExpectations:
+
 2012-10-08  Shadi Khalek  <shadi@chromium.org>
 
         Fix video-test.js relativeURL() bug
index 259788a..59c5e01 100644 (file)
@@ -2106,7 +2106,6 @@ canvas/philip/tests/2d.gradient.radial.cone.top.html
 canvas/philip/tests/2d.gradient.radial.touch1.html
 canvas/philip/tests/2d.gradient.radial.touch2.html
 canvas/philip/tests/2d.gradient.radial.touch3.html
-canvas/philip/tests/2d.imageData.put.unchanged.html
 canvas/philip/tests/2d.line.cap.open.html
 canvas/philip/tests/2d.line.join.open.html
 canvas/philip/tests/2d.missingargs.html
index fe0835e..6c028ef 100644 (file)
@@ -1,3 +1,26 @@
+2012-10-08  Huang Dongsung  <luxtella@company100.net>
+
+        [Qt] Make ImageBufferQt use premultiplied converting functions in Color.h instead of its own mechanism.
+        https://bugs.webkit.org/show_bug.cgi?id=98582
+
+        Reviewed by Eric Seidel.
+
+        Only Qt port failed canvas/philip/tests/2d.imageData.put.unchanged.html because
+        ImageBufferQt converts from or to a premultiplied color using its own code. When
+        we convert twice from an unmultiplied color to a premultiplied color and then to
+        an unmultiplied color using this code, the calculated result are pretty
+        different against the input color. It is why Gtk passes this test although Qt
+        fails. Gtk port uses them in Color.h.
+
+        In addition, Qt should use the functions in Color.h to reduce duplicated codes.
+
+        Tests: canvas/philip/tests/2d.imageData.put.unchanged.html: Fixed a test case
+
+        * platform/graphics/qt/ImageBufferQt.cpp:
+        (WebCore::getImageData):
+        (WebCore::convertBGRAToRGBA):
+        (WebCore::ImageBuffer::putByteArray):
+
 2012-10-08  Tab Atkins  <jackalmage@gmail.com>
 
         Remove "orphaned units" quirk
index b9b9ebf..ac27dc2 100644 (file)
@@ -188,6 +188,16 @@ void ImageBuffer::platformTransformColorSpace(const Vector<int>& lookUpTable)
         m_data.m_painter->begin(&m_data.m_pixmap);
 }
 
+static inline quint32 convertABGRToARGB(quint32 pixel)
+{
+    return ((pixel << 16) & 0xff0000) | ((pixel >> 16) & 0xff) | (pixel & 0xff00ff00);
+}
+
+static inline quint32 convertARGBToABGR(quint32 pixel)
+{
+    return convertABGRToARGB(pixel);
+}
+
 template <Multiply multiplied>
 PassRefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBufferData& imageData, const IntSize& size)
 {
@@ -232,40 +242,18 @@ PassRefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBuffe
     const uchar* bits = image.constBits();
 
     quint32* destRows = reinterpret_cast_ptr<quint32*>(&data[desty * rect.width() * 4 + destx * 4]);
-
-    if (multiplied == Unmultiplied) {
-        for (int y = 0; y < numRows; ++y) {
-            const quint32* scanLine = reinterpret_cast_ptr<const quint32*>(bits + (y + originy) * bytesPerLine);
-            for (int x = 0; x < numColumns; x++) {
-                QRgb pixel = scanLine[x + originx];
-                int alpha = qAlpha(pixel);
-                // Un-premultiply and convert RGB to BGR.
-                if (alpha == 255)
-                    destRows[x] = (0xFF000000
-                                | (qBlue(pixel) << 16)
-                                | (qGreen(pixel) << 8)
-                                | (qRed(pixel)));
-                else if (alpha > 0)
-                    destRows[x] = ((alpha << 24)
-                                | (((255 * qBlue(pixel)) / alpha)) << 16)
-                                | (((255 * qGreen(pixel)) / alpha) << 8)
-                                | ((255 * qRed(pixel)) / alpha);
-                else
-                    destRows[x] = 0;
-            }
-            destRows += rect.width();
-        }
-    } else {
-        for (int y = 0; y < numRows; ++y) {
-            const quint32* scanLine = reinterpret_cast_ptr<const quint32*>(bits + (y + originy) * bytesPerLine);
-            for (int x = 0; x < numColumns; x++) {
-                QRgb pixel = scanLine[x + originx];
-                // Convert RGB to BGR.
-                destRows[x] = ((pixel << 16) & 0xff0000) | ((pixel >> 16) & 0xff) | (pixel & 0xff00ff00);
-
-            }
-            destRows += rect.width();
+    for (int y = 0; y < numRows; ++y) {
+        const quint32* scanLine = reinterpret_cast_ptr<const quint32*>(bits + (y + originy) * bytesPerLine);
+        for (int x = 0; x < numColumns; x++) {
+            QRgb pixel = scanLine[x + originx];
+            Color pixelColor;
+            if (multiplied == Unmultiplied)
+                pixelColor = colorFromPremultipliedARGB(Color(qRed(pixel), qGreen(pixel), qBlue(pixel), qAlpha(pixel)).rgb());
+            else
+                pixelColor = Color(qRed(pixel), qGreen(pixel), qBlue(pixel), qAlpha(pixel));
+            destRows[x] = convertARGBToABGR(pixelColor.rgb());
         }
+        destRows += rect.width();
     }
 
     return result.release();
@@ -281,22 +269,6 @@ PassRefPtr<Uint8ClampedArray> ImageBuffer::getPremultipliedImageData(const IntRe
     return getImageData<Premultiplied>(rect, m_data, m_size);
 }
 
-static inline unsigned int premultiplyABGRtoARGB(unsigned int x)
-{
-    unsigned int a = x >> 24;
-    if (a == 255)
-        return (x << 16) | ((x >> 16) & 0xff) | (x & 0xff00ff00);
-    unsigned int t = (x & 0xff00ff) * a;
-    t = (t + ((t >> 8) & 0xff00ff) + 0x800080) >> 8;
-    t = ((t << 16) | (t >> 16)) & 0xff00ff;
-
-    x = ((x >> 8) & 0xff) * a;
-    x = (x + ((x >> 8) & 0xff) + 0x80);
-    x &= 0xff00;
-    x |= t | (a << 24);
-    return x;
-}
-
 void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, CoordinateSystem)
 {
     ASSERT(sourceRect.width() > 0);
@@ -334,26 +306,16 @@ void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, c
 
     const quint32* srcScanLine = reinterpret_cast_ptr<const quint32*>(source->data() + originy * srcBytesPerRow + originx * 4);
 
-    if (multiplied == Unmultiplied) {
-        for (int y = 0; y < numRows; ++y) {
-            quint32* destScanLine = reinterpret_cast_ptr<quint32*>(bits + y * bytesPerLine);
-            for (int x = 0; x < numColumns; x++) {
-                // Premultiply and convert BGR to RGB.
-                quint32 pixel = srcScanLine[x];
-                destScanLine[x] = premultiplyABGRtoARGB(pixel);
-            }
-            srcScanLine += sourceSize.width();
-        }
-    } else {
-        for (int y = 0; y < numRows; ++y) {
-            quint32* destScanLine = reinterpret_cast_ptr<quint32*>(bits + y * bytesPerLine);
-            for (int x = 0; x < numColumns; x++) {
-                // Convert BGR to RGB.
-                quint32 pixel = srcScanLine[x];
-                destScanLine[x] = ((pixel << 16) & 0xff0000) | ((pixel >> 16) & 0xff) | (pixel & 0xff00ff00);
-            }
-            srcScanLine += sourceSize.width();
+    for (int y = 0; y < numRows; ++y) {
+        quint32* destScanLine = reinterpret_cast_ptr<quint32*>(bits + y * bytesPerLine);
+        for (int x = 0; x < numColumns; x++) {
+            quint32 pixel = convertABGRToARGB(srcScanLine[x]);
+            if (multiplied == Unmultiplied)
+                destScanLine[x] = premultipliedARGBFromColor(Color(pixel));
+            else
+                destScanLine[x] = pixel;
         }
+        srcScanLine += sourceSize.width();
     }
 
     bool isPainting = m_data.m_painter->isActive();