From c1e5f600abaee393e66fb04e2038bc97d82851ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20R=C3=B8dal?= Date: Tue, 24 Jul 2012 15:23:49 +0200 Subject: 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 --- dist/changes-5.0.0 | 4 ++++ src/gui/painting/qdrawhelper.cpp | 2 +- src/gui/painting/qdrawhelper_p.h | 6 +++--- tests/auto/gui/image/qimage/tst_qimage.cpp | 21 +++++++++++++++++++-- tests/auto/gui/painting/qpainter/tst_qpainter.cpp | 2 +- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/dist/changes-5.0.0 b/dist/changes-5.0.0 index c5db0852b1..6cea7fed9a 100644 --- 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 985ef68401..f8c50b475c 100644 --- 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 ec9efcd49a..5df2b1f6fa 100644 --- 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 diff --git a/tests/auto/gui/image/qimage/tst_qimage.cpp b/tests/auto/gui/image/qimage/tst_qimage.cpp index 0fad243cb2..bee6ff9f8c 100644 --- 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("format"); diff --git a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp index bd075eb2fe..3d0ec9f425 100644 --- 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)); -- cgit v1.2.3