Merge lp:~aacid/unity-2d/more_24_bit_stuff into lp:unity-2d

[Albert Astals Cid]

[lib] Load more types of 24 bit icons correctly, by preventing unnecessary ABGR → ARGB conversion in that case.

To the unit tests added three rgb dots to the existing colormapped image and then check for them. Also added a 24 bit non-colormapped image to check no ABGR → ARGB occurs.

Load more types of 24 bit icons correctly, added three rgb dots to the existing colomapped image and check for them and also add a 24 bit image non colormapped that shows we need to add the early return in the code

[Gerry Boland (gerboland)]

Looks good, let's see if Jenkins accepts it this time around.
Thank you for this!

review: Approve

[Unity Merger (unity-merger)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity-2d/290/console reported an error when processing this lp:~aacid/unity-2d/more_24_bit_stuff branch.
Not merging it.

[Gerry Boland (gerboland)]

I'm going to poke this again, as I see no reference to job 290 in the public Jenkins info.
https://jenkins.qa.ubuntu.com/job/automerge-unity-2d/

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity-2d/291/console reported an error when processing this lp:~aacid/unity-2d/more_24_bit_stuff branch.
Not merging it.

[Gerry Boland (gerboland)]

I tracked down the failure to the fact that g_type_init() needs to be called in the test initialisation. We needed to call this before using any GObject functions.

When we're using Unity2dApplication, gtk_init is called which looks after this, so it's only the unit tests which need it.

[lp:~aacid/unity-2d/more_24_bit_stuff]

1099. By Albert Astals Cid on 2012-05-21

[test] Need to call g_type_init() before using any GObject functions. This should fix gimageutilstest failing on Jenkins.

=== modified file 'libunity-2d-private/src/gimageutils.cpp'
--- libunity-2d-private/src/gimageutils.cpp	2012-04-18 09:30:53 +0000
+++ libunity-2d-private/src/gimageutils.cpp	2012-05-21 15:31:20 +0000
@@ -71,45 +71,45 @@
 
 QImage imageForPixbuf(const GdkPixbuf* pixbuf, const QString &name)
 {
-    QImage image;
+    QImage result;
     if (gdk_pixbuf_get_n_channels(pixbuf) == 3 && gdk_pixbuf_get_bits_per_sample(pixbuf) == 8 && !gdk_pixbuf_get_has_alpha(pixbuf)) {
-        image = QImage(gdk_pixbuf_get_pixels(pixbuf),
-                       gdk_pixbuf_get_width(pixbuf),
-                       gdk_pixbuf_get_height(pixbuf),
-                       gdk_pixbuf_get_rowstride(pixbuf),
-                       QImage::QImage::Format_RGB888);
-        image = image.convertToFormat(QImage::Format_ARGB32);
+        const QImage image = QImage(gdk_pixbuf_get_pixels(pixbuf),
+                             gdk_pixbuf_get_width(pixbuf),
+                             gdk_pixbuf_get_height(pixbuf),
+                             gdk_pixbuf_get_rowstride(pixbuf),
+                             QImage::QImage::Format_RGB888);
+        result = image.convertToFormat(QImage::Format_ARGB32);
     } else {
         if (gdk_pixbuf_get_n_channels(pixbuf) != 4 || gdk_pixbuf_get_bits_per_sample(pixbuf) != 8 || !gdk_pixbuf_get_has_alpha(pixbuf)) {
             UQ_WARNING << "Pixbuf is not in the expected format. Trying to load it anyway, will most likely fail" << name;
         }
-        image = QImage(gdk_pixbuf_get_pixels(pixbuf),
-                   gdk_pixbuf_get_width(pixbuf),
-                   gdk_pixbuf_get_height(pixbuf),
-                   gdk_pixbuf_get_rowstride(pixbuf),
-                   QImage::Format_ARGB32);
-    }
+        const QImage image = QImage(gdk_pixbuf_get_pixels(pixbuf),
+                             gdk_pixbuf_get_width(pixbuf),
+                             gdk_pixbuf_get_height(pixbuf),
+                             gdk_pixbuf_get_rowstride(pixbuf),
+                             QImage::Format_ARGB32);
 
 #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
-    /* ABGR → ARGB */
-    QImage swappedImage = image.rgbSwapped();
+        /* ABGR → ARGB */
+        result = image.rgbSwapped();
 #else
-    /* ABGR → BGRA */
-    /* Reference: https://bugs.launchpad.net/unity-2d/+bug/758782 */
-    QImage swappedImage(image.size(), image.format());
-    for (int i = 0; i < swappedImage.height(); ++i) {
-        QRgb* p = (QRgb*) image.constScanLine(i);
-        QRgb* q = (QRgb*) swappedImage.scanLine(i);
-        QRgb* end = p + image.width();
-        while (p < end) {
-            *q = qRgba(qAlpha(*p), qRed(*p), qGreen(*p), qBlue(*p));
-            p++;
-            q++;
+        /* ABGR → BGRA */
+        /* Reference: https://bugs.launchpad.net/unity-2d/+bug/758782 */
+        result = QImage(image.size(), image.format());
+        for (int i = 0; i < swappedImage.height(); ++i) {
+            QRgb* p = (QRgb*) image.constScanLine(i);
+            QRgb* q = (QRgb*) swappedImage.scanLine(i);
+            QRgb* end = p + image.width();
+            while (p < end) {
+                *q = qRgba(qAlpha(*p), qRed(*p), qGreen(*p), qBlue(*p));
+                p++;
+                q++;
+            }
         }
-    }
 #endif
+    }
 
-    return swappedImage;
+    return result;
 }
 
 } // namespace

=== modified file 'libunity-2d-private/tests/CMakeLists.txt'
--- libunity-2d-private/tests/CMakeLists.txt	2012-04-24 15:34:45 +0000
+++ libunity-2d-private/tests/CMakeLists.txt	2012-05-21 15:31:20 +0000
@@ -9,6 +9,7 @@
     ${GLIB_INCLUDE_DIRS}
     ${QT_QTTEST_INCLUDE_DIR}
     ${X11_XTest_INCLUDE_PATH}
+    ${GDK_INCLUDE_DIRS}
     )
 
 set(LIBUNITY_2D_TEST_DIR ${libunity-2d-private_BINARY_DIR}/tests)
@@ -42,6 +43,7 @@
     pointerbarriertest
     hotkeytest
     gkeysequenceparser
+    gimageutilstest
     )
 
 target_link_libraries(pointerbarriertest ${X11_XTest_LIB})

=== modified file 'libunity-2d-private/tests/gimageutilstest.cpp'
--- libunity-2d-private/tests/gimageutilstest.cpp	2012-04-23 10:48:12 +0000
+++ libunity-2d-private/tests/gimageutilstest.cpp	2012-05-21 15:31:20 +0000
@@ -28,6 +28,12 @@
 {
     Q_OBJECT
 private Q_SLOTS:
+    void init()
+    {
+        //before using any GObject functions, need to initialize the type system
+        g_type_init();
+    }
+
     void test24bit()
     {
         GError *err = NULL;
@@ -38,6 +44,22 @@
         QCOMPARE(image.width(), 32);
         QCOMPARE(image.height(), 32);
         QVERIFY(!image.isNull());
+        QCOMPARE(image.pixel(0, 0), qRgb(255, 0, 0));
+        QCOMPARE(image.pixel(1, 0), qRgb(0, 255, 0));
+        QCOMPARE(image.pixel(2, 0), qRgb(0, 0, 255));
+    }
+
+    void test24bit_2()
+    {
+        GError *err = NULL;
+        const QString path = unity2dDirectory() + "/libunity-2d-private/tests/verification/24bit_2.png";
+        const GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(path.toLocal8Bit().constData(), &err);
+        QVERIFY(!err);
+        const QImage image = GImageUtils::imageForPixbuf(pixbuf, path);
+        QCOMPARE(image.width(), 100);
+        QCOMPARE(image.height(), 100);
+        QVERIFY(!image.isNull());
+        QCOMPARE(image.pixel(3, 3), qRgb(250, 244, 216));
     }
 };
 

=== modified file 'libunity-2d-private/tests/verification/24bit.png'
Binary files libunity-2d-private/tests/verification/24bit.png	2012-04-23 10:48:12 +0000 and libunity-2d-private/tests/verification/24bit.png	2012-05-21 15:31:20 +0000 differ
=== added file 'libunity-2d-private/tests/verification/24bit_2.png'
Binary files libunity-2d-private/tests/verification/24bit_2.png	1970-01-01 00:00:00 +0000 and libunity-2d-private/tests/verification/24bit_2.png	2012-05-21 15:31:20 +0000 differ