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

Proposed by Albert Astals Cid on 2012-05-15
Status: Merged
Approved by: Gerry Boland on 2012-05-21
Approved revision: 1099
Merged at revision: 1101
Proposed branch: lp:~aacid/unity-2d/more_24_bit_stuff
Merge into: lp:unity-2d
Diff against target: 142 lines (+52/-28)
3 files modified
libunity-2d-private/src/gimageutils.cpp (+28/-28)
libunity-2d-private/tests/CMakeLists.txt (+2/-0)
libunity-2d-private/tests/gimageutilstest.cpp (+22/-0)
To merge this branch: bzr merge lp:~aacid/unity-2d/more_24_bit_stuff
Reviewer Review Type Date Requested Status
Gerry Boland 2012-05-15 Approve on 2012-05-17
Review via email: mp+105797@code.launchpad.net

Commit message

[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.

Description of the change

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

To post a comment you must log in.
Gerry Boland (gerboland) wrote :

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

review: Approve
Unity Merger (unity-merger) wrote :

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

Unity Merger (unity-merger) wrote :

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) wrote :

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) wrote :

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) wrote :

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 updated on 2012-05-21
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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libunity-2d-private/src/gimageutils.cpp'
2--- libunity-2d-private/src/gimageutils.cpp 2012-04-18 09:30:53 +0000
3+++ libunity-2d-private/src/gimageutils.cpp 2012-05-21 15:31:20 +0000
4@@ -71,45 +71,45 @@
5
6 QImage imageForPixbuf(const GdkPixbuf* pixbuf, const QString &name)
7 {
8- QImage image;
9+ QImage result;
10 if (gdk_pixbuf_get_n_channels(pixbuf) == 3 && gdk_pixbuf_get_bits_per_sample(pixbuf) == 8 && !gdk_pixbuf_get_has_alpha(pixbuf)) {
11- image = QImage(gdk_pixbuf_get_pixels(pixbuf),
12- gdk_pixbuf_get_width(pixbuf),
13- gdk_pixbuf_get_height(pixbuf),
14- gdk_pixbuf_get_rowstride(pixbuf),
15- QImage::QImage::Format_RGB888);
16- image = image.convertToFormat(QImage::Format_ARGB32);
17+ const QImage image = QImage(gdk_pixbuf_get_pixels(pixbuf),
18+ gdk_pixbuf_get_width(pixbuf),
19+ gdk_pixbuf_get_height(pixbuf),
20+ gdk_pixbuf_get_rowstride(pixbuf),
21+ QImage::QImage::Format_RGB888);
22+ result = image.convertToFormat(QImage::Format_ARGB32);
23 } else {
24 if (gdk_pixbuf_get_n_channels(pixbuf) != 4 || gdk_pixbuf_get_bits_per_sample(pixbuf) != 8 || !gdk_pixbuf_get_has_alpha(pixbuf)) {
25 UQ_WARNING << "Pixbuf is not in the expected format. Trying to load it anyway, will most likely fail" << name;
26 }
27- image = QImage(gdk_pixbuf_get_pixels(pixbuf),
28- gdk_pixbuf_get_width(pixbuf),
29- gdk_pixbuf_get_height(pixbuf),
30- gdk_pixbuf_get_rowstride(pixbuf),
31- QImage::Format_ARGB32);
32- }
33+ const QImage image = QImage(gdk_pixbuf_get_pixels(pixbuf),
34+ gdk_pixbuf_get_width(pixbuf),
35+ gdk_pixbuf_get_height(pixbuf),
36+ gdk_pixbuf_get_rowstride(pixbuf),
37+ QImage::Format_ARGB32);
38
39 #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
40- /* ABGR → ARGB */
41- QImage swappedImage = image.rgbSwapped();
42+ /* ABGR → ARGB */
43+ result = image.rgbSwapped();
44 #else
45- /* ABGR → BGRA */
46- /* Reference: https://bugs.launchpad.net/unity-2d/+bug/758782 */
47- QImage swappedImage(image.size(), image.format());
48- for (int i = 0; i < swappedImage.height(); ++i) {
49- QRgb* p = (QRgb*) image.constScanLine(i);
50- QRgb* q = (QRgb*) swappedImage.scanLine(i);
51- QRgb* end = p + image.width();
52- while (p < end) {
53- *q = qRgba(qAlpha(*p), qRed(*p), qGreen(*p), qBlue(*p));
54- p++;
55- q++;
56+ /* ABGR → BGRA */
57+ /* Reference: https://bugs.launchpad.net/unity-2d/+bug/758782 */
58+ result = QImage(image.size(), image.format());
59+ for (int i = 0; i < swappedImage.height(); ++i) {
60+ QRgb* p = (QRgb*) image.constScanLine(i);
61+ QRgb* q = (QRgb*) swappedImage.scanLine(i);
62+ QRgb* end = p + image.width();
63+ while (p < end) {
64+ *q = qRgba(qAlpha(*p), qRed(*p), qGreen(*p), qBlue(*p));
65+ p++;
66+ q++;
67+ }
68 }
69- }
70 #endif
71+ }
72
73- return swappedImage;
74+ return result;
75 }
76
77 } // namespace
78
79=== modified file 'libunity-2d-private/tests/CMakeLists.txt'
80--- libunity-2d-private/tests/CMakeLists.txt 2012-04-24 15:34:45 +0000
81+++ libunity-2d-private/tests/CMakeLists.txt 2012-05-21 15:31:20 +0000
82@@ -9,6 +9,7 @@
83 ${GLIB_INCLUDE_DIRS}
84 ${QT_QTTEST_INCLUDE_DIR}
85 ${X11_XTest_INCLUDE_PATH}
86+ ${GDK_INCLUDE_DIRS}
87 )
88
89 set(LIBUNITY_2D_TEST_DIR ${libunity-2d-private_BINARY_DIR}/tests)
90@@ -42,6 +43,7 @@
91 pointerbarriertest
92 hotkeytest
93 gkeysequenceparser
94+ gimageutilstest
95 )
96
97 target_link_libraries(pointerbarriertest ${X11_XTest_LIB})
98
99=== modified file 'libunity-2d-private/tests/gimageutilstest.cpp'
100--- libunity-2d-private/tests/gimageutilstest.cpp 2012-04-23 10:48:12 +0000
101+++ libunity-2d-private/tests/gimageutilstest.cpp 2012-05-21 15:31:20 +0000
102@@ -28,6 +28,12 @@
103 {
104 Q_OBJECT
105 private Q_SLOTS:
106+ void init()
107+ {
108+ //before using any GObject functions, need to initialize the type system
109+ g_type_init();
110+ }
111+
112 void test24bit()
113 {
114 GError *err = NULL;
115@@ -38,6 +44,22 @@
116 QCOMPARE(image.width(), 32);
117 QCOMPARE(image.height(), 32);
118 QVERIFY(!image.isNull());
119+ QCOMPARE(image.pixel(0, 0), qRgb(255, 0, 0));
120+ QCOMPARE(image.pixel(1, 0), qRgb(0, 255, 0));
121+ QCOMPARE(image.pixel(2, 0), qRgb(0, 0, 255));
122+ }
123+
124+ void test24bit_2()
125+ {
126+ GError *err = NULL;
127+ const QString path = unity2dDirectory() + "/libunity-2d-private/tests/verification/24bit_2.png";
128+ const GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(path.toLocal8Bit().constData(), &err);
129+ QVERIFY(!err);
130+ const QImage image = GImageUtils::imageForPixbuf(pixbuf, path);
131+ QCOMPARE(image.width(), 100);
132+ QCOMPARE(image.height(), 100);
133+ QVERIFY(!image.isNull());
134+ QCOMPARE(image.pixel(3, 3), qRgb(250, 244, 216));
135 }
136 };
137
138
139=== modified file 'libunity-2d-private/tests/verification/24bit.png'
140Binary 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
141=== added file 'libunity-2d-private/tests/verification/24bit_2.png'
142Binary 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

Subscribers

People subscribed via source and target branches