From e3b6f6d16577c74433de1ca9e15402cdf285abca Mon Sep 17 00:00:00 2001
From: Allan Sandfeld Jensen <allan.jensen@qt.io>
Date: Fri, 18 Nov 2016 16:33:12 +0100
Subject: [PATCH] Fix blending of RGB32 on RGB32 with partial opacity

The alpha channel of an RGB32 image was not properly ignored when doing
blending with partial opacity.

Now the alpha value is properly ignored, which is both more correct
and faster. This also makes SSE2 and AVX2 implementations match NEON
which was already doing the right thing (though had dead code for
doing it wrong).

Change-Id: I4613b8d70ed8c2e36ced10baaa7a4a55bd36a940
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
---
 src/gui/painting/qblendfunctions.cpp          | 30 +++++++++++--------
 src/gui/painting/qdrawhelper_avx2.cpp         |  8 ++---
 src/gui/painting/qdrawhelper_neon.cpp         |  2 --
 src/gui/painting/qdrawhelper_sse2.cpp         | 11 +++----
 .../gui/painting/qpainter/tst_qpainter.cpp    | 20 +++++++++++++
 5 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/src/gui/painting/qblendfunctions.cpp b/src/gui/painting/qblendfunctions.cpp
index 0a5d458532d..a4a091a29f9 100644
--- a/src/gui/painting/qblendfunctions.cpp
+++ b/src/gui/painting/qblendfunctions.cpp
@@ -385,19 +385,25 @@ void qt_blend_rgb32_on_rgb32(uchar *destPixels, int dbpl,
             destPixels, dbpl, srcPixels, sbpl, w, h, const_alpha);
     fflush(stdout);
 #endif
-
-    if (const_alpha != 256) {
-        qt_blend_argb32_on_argb32(destPixels, dbpl, srcPixels, sbpl, w, h, const_alpha);
-        return;
-    }
-
     const uint *src = (const uint *) srcPixels;
     uint *dst = (uint *) destPixels;
-    int len = w * 4;
-    for (int y=0; y<h; ++y) {
-        memcpy(dst, src, len);
-        dst = (quint32 *)(((uchar *) dst) + dbpl);
-        src = (const quint32 *)(((const uchar *) src) + sbpl);
+    if (const_alpha == 256) {
+        const int len = w * 4;
+        for (int y = 0; y < h; ++y) {
+            memcpy(dst, src, len);
+            dst = (quint32 *)(((uchar *) dst) + dbpl);
+            src = (const quint32 *)(((const uchar *) src) + sbpl);
+        }
+        return;
+    } else if (const_alpha != 0) {
+        const_alpha = (const_alpha * 255) >> 8;
+        int ialpha = 255 - const_alpha;
+        for (int y=0; y<h; ++y) {
+            for (int x=0; x<w; ++x)
+                dst[x] = INTERPOLATE_PIXEL_255(dst[x], ialpha, src[x], const_alpha);
+            dst = (quint32 *)(((uchar *) dst) + dbpl);
+            src = (const quint32 *)(((const uchar *) src) + sbpl);
+        }
     }
 }
 
@@ -414,7 +420,7 @@ struct Blend_RGB32_on_RGB32_ConstAlpha {
     }
 
     inline void write(quint32 *dst, quint32 src) {
-        *dst = BYTE_MUL(src, m_alpha) + BYTE_MUL(*dst, m_ialpha);
+        *dst = INTERPOLATE_PIXEL_255(src, m_alpha, *dst, m_ialpha);
     }
 
     inline void flush(void *) {}
diff --git a/src/gui/painting/qdrawhelper_avx2.cpp b/src/gui/painting/qdrawhelper_avx2.cpp
index acc9bc7ba1a..9c1335298eb 100644
--- a/src/gui/painting/qdrawhelper_avx2.cpp
+++ b/src/gui/painting/qdrawhelper_avx2.cpp
@@ -267,11 +267,9 @@ void qt_blend_rgb32_on_rgb32_avx2(uchar *destPixels, int dbpl,
         // 2) interpolate pixels with AVX2
         for (; x < (w - 7); x += 8) {
             const __m256i srcVector = _mm256_lddqu_si256((const __m256i *)&src[x]);
-            if (!_mm256_testz_si256(srcVector, srcVector)) {
-                __m256i dstVector = _mm256_load_si256((__m256i *)&dst[x]);
-                INTERPOLATE_PIXEL_255_AVX2(srcVector, dstVector, constAlphaVector, oneMinusConstAlpha, colorMask, half);
-                _mm256_store_si256((__m256i *)&dst[x], dstVector);
-            }
+            __m256i dstVector = _mm256_load_si256((__m256i *)&dst[x]);
+            INTERPOLATE_PIXEL_255_AVX2(srcVector, dstVector, constAlphaVector, oneMinusConstAlpha, colorMask, half);
+            _mm256_store_si256((__m256i *)&dst[x], dstVector);
         }
 
         // 3) Epilogue
diff --git a/src/gui/painting/qdrawhelper_neon.cpp b/src/gui/painting/qdrawhelper_neon.cpp
index d51b43961c5..a833520b006 100644
--- a/src/gui/painting/qdrawhelper_neon.cpp
+++ b/src/gui/painting/qdrawhelper_neon.cpp
@@ -523,8 +523,6 @@ void qt_blend_rgb32_on_rgb32_neon(uchar *destPixels, int dbpl,
                     vst1q_u32((uint32_t *)&dst[x], vcombine_u32(result32_low, result32_high));
                 }
                 for (; x<w; ++x) {
-                    uint s = src[x];
-                    s = BYTE_MUL(s, const_alpha);
                     dst[x] = INTERPOLATE_PIXEL_255(src[x], const_alpha, dst[x], one_minus_const_alpha);
                 }
                 dst = (quint32 *)(((uchar *) dst) + dbpl);
diff --git a/src/gui/painting/qdrawhelper_sse2.cpp b/src/gui/painting/qdrawhelper_sse2.cpp
index 5ff08e81536..edce70d2d0b 100644
--- a/src/gui/painting/qdrawhelper_sse2.cpp
+++ b/src/gui/painting/qdrawhelper_sse2.cpp
@@ -101,7 +101,6 @@ void qt_blend_rgb32_on_rgb32_sse2(uchar *destPixels, int dbpl,
     quint32 *dst = (quint32 *) destPixels;
     if (const_alpha != 256) {
         if (const_alpha != 0) {
-            const __m128i nullVector = _mm_set1_epi32(0);
             const __m128i half = _mm_set1_epi16(0x80);
             const __m128i colorMask = _mm_set1_epi32(0x00ff00ff);
 
@@ -119,12 +118,10 @@ void qt_blend_rgb32_on_rgb32_sse2(uchar *destPixels, int dbpl,
 
                 for (; x < w-3; x += 4) {
                     __m128i srcVector = _mm_loadu_si128((const __m128i *)&src[x]);
-                    if (_mm_movemask_epi8(_mm_cmpeq_epi32(srcVector, nullVector)) != 0xffff) {
-                        const __m128i dstVector = _mm_load_si128((__m128i *)&dst[x]);
-                        __m128i result;
-                        INTERPOLATE_PIXEL_255_SSE2(result, srcVector, dstVector, constAlphaVector, oneMinusConstAlpha, colorMask, half);
-                        _mm_store_si128((__m128i *)&dst[x], result);
-                    }
+                    const __m128i dstVector = _mm_load_si128((__m128i *)&dst[x]);
+                    __m128i result;
+                    INTERPOLATE_PIXEL_255_SSE2(result, srcVector, dstVector, constAlphaVector, oneMinusConstAlpha, colorMask, half);
+                    _mm_store_si128((__m128i *)&dst[x], result);
                 }
                 SIMD_EPILOGUE(x, w, 3)
                     dst[x] = INTERPOLATE_PIXEL_255(src[x], const_alpha, dst[x], one_minus_const_alpha);
diff --git a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
index 254ab1f8ad3..c729b2f94c8 100644
--- a/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
+++ b/tests/auto/gui/painting/qpainter/tst_qpainter.cpp
@@ -300,6 +300,8 @@ private slots:
 
     void QTBUG56252();
 
+    void blendNullRGB32();
+
 private:
     void fillData();
     void setPenColor(QPainter& p);
@@ -5139,6 +5141,24 @@ void tst_QPainter::QTBUG56252()
     // If no crash or illegal memory read, all is fine
 }
 
+void tst_QPainter::blendNullRGB32()
+{
+    quint32 data[16] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+
+    QImage nullImage((const uchar*)data, 16, 1,  QImage::Format_RGB32);
+    QImage image(16, 1, QImage::Format_RGB32);
+    image.fill(Qt::white);
+
+    QPainter paint(&image);
+    paint.setCompositionMode(QPainter::CompositionMode_Source);
+    paint.setOpacity(0.5);
+    paint.drawImage(0, 0, nullImage);
+    paint.end();
+
+    for (int i=0; i < image.width(); ++i)
+        QVERIFY(image.pixel(i,0) != 0xffffffff);
+}
+
 QTEST_MAIN(tst_QPainter)
 
 #include "tst_qpainter.moc"