Made QImage::fill(uint pixel) for RGB888 accept QRgb values.

Previously QImage::fill() for Format_RGB888 expected a BGR value
instead of the RGB order defined by QRgb, making it counter intuitive to
use related to the 32-bit formats.

Fixed the QPixelLayout data for RGB888 and changed the byte order of
quint24 based on what the optimized image conversion routines expect.

Change-Id: I72926debbc6f5b5cb10b8aa0b2a2a916a04db946
Reviewed-by: Kim M. Kalland <kim.kalland@nokia.com>

diff --git a/dist/changes-5.0.0 b/dist/changes-5.0.0
index c5db085..6cea7fe 100644 (file)
--- a/dist/changes-5.0.0
+++ b/dist/changes-5.0.0
@@ -444,6 +444,10 @@ QtGui
 * QOpenGLPaintDevice has been added to be able to use QPainter to render into
   the currently bound context.
 
+* Behavioral change in QImage::fill() on an image with format Format_RGB888:
+  For consistency with RGB32 and other 32-bit formats, function now expects
+  image data in RGB layout as opposed to BGR layout.
+
 QtWidgets
 ---------
 * QInputContext removed as well as related getters and setters on QWidget and QApplication.

diff --git a/src/gui/painting/qdrawhelper.cpp b/src/gui/painting/qdrawhelper.cpp
index 985ef68..f8c50b4 100644 (file)
--- a/src/gui/painting/qdrawhelper.cpp
+++ b/src/gui/painting/qdrawhelper.cpp
@@ -268,7 +268,7 @@ QPixelLayout qPixelLayouts[QImage::NImageFormats] = {
     { 6, 12, 6,  6, 6,  0, 6, 18,  true, QPixelLayout::BPP24, convertToARGB32PM, convertFromARGB32PM }, // Format_ARGB6666_Premultiplied
     { 5, 10, 5,  5, 5,  0, 0,  0, false, QPixelLayout::BPP16, convertToRGB32, convertFromARGB32PM }, // Format_RGB555
     { 5, 18, 5, 13, 5,  8, 8,  0,  true, QPixelLayout::BPP24, convertToARGB32PM, convertFromARGB32PM }, // Format_ARGB8555_Premultiplied
-    { 8,  0, 8,  8, 8, 16, 0,  0, false, QPixelLayout::BPP24, convertToRGB32, convertFromARGB32PM }, // Format_RGB888
+    { 8, 16, 8,  8, 8,  0, 0,  0, false, QPixelLayout::BPP24, convertToRGB32, convertFromARGB32PM }, // Format_RGB888
     { 4,  8, 4,  4, 4,  0, 0,  0, false, QPixelLayout::BPP16, convertToRGB32, convertFromARGB32PM }, // Format_RGB444
     { 4,  8, 4,  4, 4,  0, 4, 12,  true, QPixelLayout::BPP16, convertToARGB32PM, convertFromARGB32PM } // Format_ARGB4444_Premultiplied
 };

diff --git a/src/gui/painting/qdrawhelper_p.h b/src/gui/painting/qdrawhelper_p.h
index ec9efcd..5df2b1f 100644 (file)
--- a/src/gui/painting/qdrawhelper_p.h
+++ b/src/gui/painting/qdrawhelper_p.h
@@ -677,14 +677,14 @@ struct quint24 {
 
 inline quint24::quint24(uint value)
 {
-    data[0] = uchar(value);
+    data[0] = uchar(value >> 16);
     data[1] = uchar(value >> 8);
-    data[2] = uchar(value >> 16);
+    data[2] = uchar(value);
 }
 
 inline quint24::operator uint() const
 {
-    return data[0] | (data[1] << 8) | (data[2] << 16);
+    return data[2] | (data[1] << 8) | (data[0] << 16);
 }
 
 template <class T>

diff --git a/tests/auto/gui/image/qimage/tst_qimage.cpp b/tests/auto/gui/image/qimage/tst_qimage.cpp
index 0fad243..bee6ff9 100644 (file)
--- a/tests/auto/gui/image/qimage/tst_qimage.cpp
+++ b/tests/auto/gui/image/qimage/tst_qimage.cpp
@@ -137,6 +137,8 @@ private slots:
 
     void fillColorWithAlpha();
 
+    void fillRGB888();
+
     void rgbSwapped_data();
     void rgbSwapped();
 
@@ -1047,11 +1049,11 @@ void tst_QImage::setPixel_data()
     QTest::newRow("ARGB8555_Premultiplied blue") << int(QImage::Format_ARGB8555_Premultiplied)
                                    << 0xff0000ff << 0x001fffu;
     QTest::newRow("RGB888 red") << int(QImage::Format_RGB888)
-                                << 0xffff0000 << 0x0000ffu;
+                                << 0xffff0000 << 0xff0000u;
     QTest::newRow("RGB888 green") << int(QImage::Format_RGB888)
                                   << 0xff00ff00 << 0x00ff00u;
     QTest::newRow("RGB888 blue") << int(QImage::Format_RGB888)
-                                 << 0xff0000ff << 0xff0000u;
+                                 << 0xff0000ff << 0x0000ffu;
 }
 
 void tst_QImage::setPixel()
@@ -1872,6 +1874,21 @@ void tst_QImage::fillColorWithAlpha()
     QCOMPARE(argb32pm.pixel(0, 0), 0x7f7f0000u);
 }
 
+void tst_QImage::fillRGB888()
+{
+    QImage expected(1, 1, QImage::Format_RGB888);
+    QImage actual(1, 1, QImage::Format_RGB888);
+
+    for (int c = Qt::black; c < Qt::transparent; ++c) {
+        QColor color = QColor(Qt::GlobalColor(c));
+
+        expected.fill(color);
+        actual.fill(color.rgba());
+
+        QCOMPARE(actual.pixel(0, 0), expected.pixel(0, 0));
+    }
+}
+
 void tst_QImage::rgbSwapped_data()
 {
     QTest::addColumn<QImage::Format>("format");

diff --git a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
index bd075eb..3d0ec9f 100644 (file)
--- a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
+++ b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
@@ -3172,7 +3172,7 @@ void tst_QPainter::drawImage_task258776()
     QImage src(16, 16, QImage::Format_RGB888);
     QImage dest(33, 33, QImage::Format_RGB888);
     src.fill(0x00ff00);
-    dest.fill(0x0000ff);
+    dest.fill(0xff0000);
 
     QPainter painter(&dest);
     painter.drawImage(QRectF(0.499, 0.499, 32, 32), src, QRectF(0, 0, 16, 16));