From 80de7801ce5a1c7e8962e25b36adfbb18f435b31 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 11 Feb 2021 11:23:05 +0100 Subject: [PATCH] Fix alpha handling of QImage::setPixel It was treated differently depending on format, made it consistently behave the same for all formats (following the behavior of the primary formats). Change-Id: Ie24e19957d076fdf3ebd333074e26ede187489eb Reviewed-by: Eirik Aavitsland (cherry picked from commit c32cd44d34910cfd42e32537578e4a573138a282) Reviewed-by: Qt Cherry-pick Bot --- src/gui/image/qimage.cpp | 11 +++- src/gui/painting/qpixellayout.cpp | 22 +++++-- tests/auto/gui/image/qimage/tst_qimage.cpp | 72 ++++++++++++++++++---- 3 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/gui/image/qimage.cpp b/src/gui/image/qimage.cpp index 33c12993d71..23d36c27577 100644 --- a/src/gui/image/qimage.cpp +++ b/src/gui/image/qimage.cpp @@ -2470,7 +2470,7 @@ void QImage::setPixel(int x, int y, uint index_or_rgb) ((uint *)s)[x] = index_or_rgb; return; case Format_RGB16: - ((quint16 *)s)[x] = qConvertRgb32To16(qUnpremultiply(index_or_rgb)); + ((quint16 *)s)[x] = qConvertRgb32To16(index_or_rgb); return; case Format_RGBX8888: ((uint *)s)[x] = ARGB2RGBA(0xff000000 | index_or_rgb); @@ -2491,6 +2491,10 @@ void QImage::setPixel(int x, int y, uint index_or_rgb) case Format_A2RGB30_Premultiplied: ((uint *)s)[x] = qConvertArgb32ToA2rgb30(index_or_rgb); return; + case Format_RGBA64: + case Format_RGBA64_Premultiplied: + ((QRgba64 *)s)[x] = QRgba64::fromArgb32(index_or_rgb); + return; case Format_Invalid: case NImageFormats: Q_ASSERT(false); @@ -2500,7 +2504,10 @@ void QImage::setPixel(int x, int y, uint index_or_rgb) } const QPixelLayout *layout = &qPixelLayouts[d->format]; - layout->storeFromARGB32PM(s, &index_or_rgb, x, 1, nullptr, nullptr); + if (!hasAlphaChannel()) + layout->storeFromRGB32(s, &index_or_rgb, x, 1, nullptr, nullptr); + else + layout->storeFromARGB32PM(s, &index_or_rgb, x, 1, nullptr, nullptr); } /*! diff --git a/src/gui/painting/qpixellayout.cpp b/src/gui/painting/qpixellayout.cpp index 8dd62b0c007..ca6bca344f3 100644 --- a/src/gui/painting/qpixellayout.cpp +++ b/src/gui/painting/qpixellayout.cpp @@ -1333,7 +1333,7 @@ static void QT_FASTCALL storeRGB64FromRGB32(uchar *dest, const uint *src, int in { QRgba64 *d = reinterpret_cast(dest) + index; for (int i = 0; i < count; ++i) - d[i] = QRgba64::fromArgb32(src[i]); + d[i] = QRgba64::fromArgb32(src[i] | 0xff000000); } static const uint *QT_FASTCALL fetchRGBA64ToARGB32PM(uint *buffer, const uchar *src, int index, int count, @@ -1345,12 +1345,24 @@ static const uint *QT_FASTCALL fetchRGBA64ToARGB32PM(uint *buffer, const uchar * return buffer; } +template static void QT_FASTCALL storeRGBA64FromARGB32PM(uchar *dest, const uint *src, int index, int count, const QList *, QDitherInfo *) { QRgba64 *d = reinterpret_cast(dest) + index; - for (int i = 0; i < count; ++i) + for (int i = 0; i < count; ++i) { d[i] = QRgba64::fromArgb32(src[i]).unpremultiplied(); + if (Mask) + d[i].setAlpha(65535); + } +} + +static void QT_FASTCALL storeRGBA64FromARGB32(uchar *dest, const uint *src, int index, int count, + const QList *, QDitherInfo *) +{ + QRgba64 *d = reinterpret_cast(dest) + index; + for (int i = 0; i < count; ++i) + d[i] = QRgba64::fromArgb32(src[i]); } // Note: @@ -1437,15 +1449,15 @@ QPixelLayout qPixelLayouts[QImage::NImageFormats] = { { false, false, QPixelLayout::BPP64, rbSwap_4x16, convertPassThrough, nullptr, fetchRGB64ToRGB32, fetchPassThrough64, - storeRGB64FromRGB32, storeRGB64FromRGB32 }, // Format_RGBX64 + storeRGBA64FromARGB32PM, storeRGB64FromRGB32 }, // Format_RGBX64 { true, false, QPixelLayout::BPP64, rbSwap_4x16, convertARGB32ToARGB32PM, nullptr, fetchRGBA64ToARGB32PM, fetchRGBA64ToRGBA64PM, - storeRGBA64FromARGB32PM, storeRGB64FromRGB32 }, // Format_RGBA64 + storeRGBA64FromARGB32PM, storeRGB64FromRGB32 }, // Format_RGBA64 { true, true, QPixelLayout::BPP64, rbSwap_4x16, convertPassThrough, nullptr, fetchRGB64ToRGB32, fetchPassThrough64, - storeRGB64FromRGB32, storeRGB64FromRGB32 }, // Format_RGBA64_Premultiplied + storeRGBA64FromARGB32, storeRGB64FromRGB32 }, // Format_RGBA64_Premultiplied { false, false, QPixelLayout::BPP16, nullptr, convertGrayscale16ToRGB32, convertGrayscale16ToRGBA64, fetchGrayscale16ToRGB32, fetchGrayscale16ToRGBA64, diff --git a/tests/auto/gui/image/qimage/tst_qimage.cpp b/tests/auto/gui/image/qimage/tst_qimage.cpp index 1554ac36025..888ab4c8f76 100644 --- a/tests/auto/gui/image/qimage/tst_qimage.cpp +++ b/tests/auto/gui/image/qimage/tst_qimage.cpp @@ -102,6 +102,10 @@ private slots: void setPixel_data(); void setPixel(); + void setPixelWithAlpha_data(); + void setPixelWithAlpha(); + void setPixelColorWithAlpha_data(); + void setPixelColorWithAlpha(); void defaultColorTable_data(); void defaultColorTable(); @@ -1456,6 +1460,62 @@ void tst_QImage::setPixel() } } +void tst_QImage::setPixelWithAlpha_data() +{ + QTest::addColumn("format"); + + for (int c = QImage::Format_RGB32; c < QImage::NImageFormats; ++c) { + if (c == QImage::Format_Grayscale8) + continue; + if (c == QImage::Format_Grayscale16) + continue; + if (c == QImage::Format_Alpha8) + continue; + QTest::newRow(qPrintable(formatToString(QImage::Format(c)))) << QImage::Format(c); + } +} + +void tst_QImage::setPixelWithAlpha() +{ + QFETCH(QImage::Format, format); + QImage image(1, 1, format); + QRgb referenceColor = qRgba(0, 170, 85, 170); + image.setPixel(0, 0, referenceColor); + + if (!image.hasAlphaChannel()) + referenceColor = 0xff000000 | referenceColor; + + QRgb color = image.pixel(0, 0); + QCOMPARE(qRed(color) & 0xf0, qRed(referenceColor) & 0xf0); + QCOMPARE(qGreen(color) & 0xf0, qGreen(referenceColor) & 0xf0); + QCOMPARE(qBlue(color) & 0xf0, qBlue(referenceColor) & 0xf0); + QCOMPARE(qAlpha(color) & 0xf0, qAlpha(referenceColor) & 0xf0); +} + +void tst_QImage::setPixelColorWithAlpha_data() +{ + setPixelWithAlpha_data(); +} + +void tst_QImage::setPixelColorWithAlpha() +{ + QFETCH(QImage::Format, format); + QImage image(1, 1, format); + image.setPixelColor(0, 0, QColor(170, 85, 255, 170)); + QRgb referenceColor = qRgba(170, 85, 255, 170); + + if (!image.hasAlphaChannel()) + referenceColor = 0xff000000 | referenceColor; + else if (image.pixelFormat().premultiplied() == QPixelFormat::Premultiplied) + referenceColor = qPremultiply(referenceColor); + + QRgb color = image.pixel(0, 0); + QCOMPARE(qRed(color) & 0xf0, qRed(referenceColor) & 0xf0); + QCOMPARE(qGreen(color) & 0xf0, qGreen(referenceColor) & 0xf0); + QCOMPARE(qBlue(color) & 0xf0, qBlue(referenceColor) & 0xf0); + QCOMPARE(qAlpha(color) & 0xf0, qAlpha(referenceColor) & 0xf0); +} + void tst_QImage::convertToFormatPreserveDotsPrMeter() { QImage img(100, 100, QImage::Format_ARGB32_Premultiplied); @@ -2351,17 +2411,7 @@ void tst_QImage::fillColor() void tst_QImage::fillColorWithAlpha_data() { - QTest::addColumn("format"); - - for (int c = QImage::Format_RGB32; c < QImage::NImageFormats; ++c) { - if (c == QImage::Format_Grayscale8) - continue; - if (c == QImage::Format_Grayscale16) - continue; - if (c == QImage::Format_Alpha8) - continue; - QTest::newRow(qPrintable(formatToString(QImage::Format(c)))) << QImage::Format(c); - } + setPixelWithAlpha_data(); } void tst_QImage::fillColorWithAlpha()