From b9ba2217566531b51f3c5c5158bfcc386539d810 Mon Sep 17 00:00:00 2001
From: Eirik Aavitsland <eirik.aavitsland@qt.io>
Date: Fri, 25 May 2018 16:15:14 +0200
Subject: [PATCH] webp handler: support alpha-less reading and writing

Webp files can be with or without alpha channel. The handler would
ignore this and read all as Format_ARGB32 images, and write all as
having alpha, in both cases losing that important bit of information.

As a driveby, simplify the endianness handling in write(). By always
converting the source image to an endianness-independent QImage
format, no special handling is required.

Task-number: QTBUG-48628
Change-Id: I624ed72b18a8b59a542979efcb4e8ff81214e0d7
Reviewed-by: Liang Qi <liang.qi@qt.io>
---
 .../imageformats/webp/qwebphandler.cpp        |  27 +++++++++---------
 tests/auto/webp/images/kollada_noalpha.webp   | Bin 0 -> 3570 bytes
 tests/auto/webp/tst_qwebp.cpp                 |  23 +++++++++++----
 tests/auto/webp/webp.qrc                      |   1 +
 4 files changed, 32 insertions(+), 19 deletions(-)
 create mode 100644 tests/auto/webp/images/kollada_noalpha.webp

diff --git a/src/plugins/imageformats/webp/qwebphandler.cpp b/src/plugins/imageformats/webp/qwebphandler.cpp
index 3a7bf434..12c4001f 100644
--- a/src/plugins/imageformats/webp/qwebphandler.cpp
+++ b/src/plugins/imageformats/webp/qwebphandler.cpp
@@ -174,7 +174,8 @@ bool QWebpHandler::read(QImage *image)
     if (status != VP8_STATUS_OK)
         return false;
 
-    QImage frame(m_iter.width, m_iter.height, QImage::Format_ARGB32);
+    QImage::Format format = m_features.has_alpha ? QImage::Format_ARGB32 : QImage::Format_RGB32;
+    QImage frame(m_iter.width, m_iter.height, format);
     uint8_t *output = frame.bits();
     size_t output_size = frame.sizeInBytes();
 #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
@@ -219,13 +220,10 @@ bool QWebpHandler::write(const QImage &image)
     }
 
     QImage srcImage = image;
-#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
-    if (srcImage.format() != QImage::Format_ARGB32)
-        srcImage = srcImage.convertToFormat(QImage::Format_ARGB32);
-#else /* Q_BIG_ENDIAN */
-    if (srcImage.format() != QImage::Format_RGBA8888)
-        srcImage = srcImage.convertToFormat(QImage::Format_RGBA8888);
-#endif
+    bool alpha = srcImage.hasAlphaChannel();
+    QImage::Format newFormat = alpha ? QImage::Format_RGBA8888 : QImage::Format_RGB888;
+    if (srcImage.format() != newFormat)
+        srcImage = srcImage.convertToFormat(newFormat);
 
     WebPPicture picture;
     WebPConfig config;
@@ -238,13 +236,14 @@ bool QWebpHandler::write(const QImage &image)
     picture.width = srcImage.width();
     picture.height = srcImage.height();
     picture.use_argb = 1;
-#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
-    if (!WebPPictureImportBGRA(&picture, srcImage.bits(), srcImage.bytesPerLine())) {
-#else /* Q_BIG_ENDIAN */
-    if (!WebPPictureImportRGBA(&picture, srcImage.bits(), srcImage.bytesPerLine())) {
-#endif
-        qWarning() << "failed to import image data to webp picture.";
+    bool failed = false;
+    if (alpha)
+        failed = !WebPPictureImportRGBA(&picture, srcImage.bits(), srcImage.bytesPerLine());
+    else
+        failed = !WebPPictureImportRGB(&picture, srcImage.bits(), srcImage.bytesPerLine());
 
+    if (failed) {
+        qWarning() << "failed to import image data to webp picture.";
         WebPPictureFree(&picture);
         return false;
     }
diff --git a/tests/auto/webp/images/kollada_noalpha.webp b/tests/auto/webp/images/kollada_noalpha.webp
new file mode 100644
index 0000000000000000000000000000000000000000..b5abe5d4f5af2327e8dc0ee6d8f37aeef66262f4
GIT binary patch
literal 3570
zcmV<O4Gr>ANk&HM4FCXFMM6+kP&gpo4FCYpQvjU-DzpKh06vjKo=K&oBOxc$N@%bW
z31n{FUkn~!$NsC_*_Zs6_ut0vc?%EsugM?UIOgmR)z4smEB$ZtC$zuwf2}_Dyudlm
z`GEbi{j~Q}|1tYp?t}L0*aP}6`p^IUgWiH4&wt-~Z+`H6BY$fDzx%80kLm&c*VF_5
zr&#}>`N6=$g1h<KukjpkP!{sIc&DfBjNeXM0zAbDT0GB2kJ18NrDqiRG<ALtbJz8n
zzoeC?L3d;1>Z<5KwQU1ksNPpAL!8vapTX=Sbc84uQ(<(-!%q9>iokIqhs$+W=5f>&
zp@JnG$vEeLFotiFOqETm%H-qT6^Fk0^(LI$r_AY76P3-4*7X=)x|CY=)zraYU6HoL
zF@BT$o&Ux}XdM}6TJDM4#N23(v#OliB^Tw^FW}EjqiELX5LD*1N3Z>6VEb`T&iTeE
zPuwmYGMI^GWYYq<!anx<0r}vVNqUP#A$r)I@d#qQT?ro)=W;T+&_@@Z4C67*_Dv>2
zGWRWoci=vqK6fy5wD=LJ_(}sEsL@0yY+=fhMUyTpSO3rNj7ZBm#Bz_bqm&Rk>iVxv
zWZ8zoMsp1$mz%=54c_my1Z>c@3DM~yYL%+tqsP?N<k^X$IXuz1CPG73^U!%95#T-b
zEjgi0J@Xs9qR_;l6uQ&!)sT^<l9<fAysV<=c&$~C_j47kzWHp#p~s;UkRO$Xq`wW<
z#-@dtcGY#$*=tL{dea42)utKOvl}ugZ2*&R$28P!3eLRP6hbScPb%vqKDW3;7hH3h
zW*Ye>KB`r9C6<5@ZdDNaWNN;nXjW=ZQQr_0b_+zJ*j~fZbi^=8WR;<Q+^Qk;$klyD
z(2^j%c<Q1bOpRC6Z3@o3#Siwb>8h6@wNVeIIRF6u`qzL-)1!E^OYzyUH*Q|<NfVg0
zqcoYY=xjI4{W7Iu0r3H_^Z^B&fPO3uB(Q!BiZ-S@incvZQS=~UJ1!JZDGtqV2PLlJ
zOnTj`zW51siY93cE@W{&*7Zb8fzF~I8nj`rsWlNB?NV(U4|*yDooO^cNTWs#@Owuu
zB%nUy;fuXrWSbl4cBsyTDwlYy$ZrL*#?DZ>v}JV~mP)WEK}|P=JZZ}zs#VCc)FnDb
zAmOvGd5%DTJEKYmpO_DYRgpDlS+sis_zi0cCKHorC0Iv=;zOymP#6Cy*Fd=y5yuWF
z=4CTr+9tbWp?Le8ZZmGIUUIu?D07v6Pr^=NJQES@nv+^q{V+7(h%ls^23o@XcWMVc
zB~?Lifz!0>sDkliV}jA26hpiIP;6XlBkT5d!i`H*5K1Ofyp2?>H~p%0a7<6SqZ(IT
z$Pw=6(B`x0cu@ov?-iU6wTLPL2_Jn<z;8Wq&G{?Hip6+th?DZ$&Bz@?GZ1hHYC;%C
z?5Hn6l#1SaB;`2kfP4yQpZHI<uC_FbfI77WTnzNQuGd|&@rZ-G>Tv8-#+tHVIw#TI
z1VNnfR#WvPZXv_W&hP0K6wm*r-SM_f!<A)sA~d8PR(p7TryAF4or+nD>ZKXXo4Eca
z1S=Ap6Q}D-^xPE%cCD7Ljx2beL*xapOMx#G{C8hBG^;5)GgQ8n>M#1-e)N<>L+526
z;o>|L6vafoDvNaaFJ+0<ap3eGD$|U__4A~dAgStUc|hOsafBBD)i7g-Yn2`KXCJF_
zlV<FJ%}LRCL49a(Mi*G?JIZ%`?NeC5Mu?@*Cb)jns9M?(YTD*A#y;d{lJTg2uvUDk
zTTwDud`Ot5o}ak2#y7B|7ia;14#G4ZHz0zwwh?nTKG(oRaPmh7a4;O=ciNC?FT&$#
zK^=ZE!jRdP?|n*q<2aV>!RVu;+-ldzI2@r}QbV?}wGU~*V*P>|XCWV0yF0b8MSz7y
z0<M4|fw2-ZDp4@9Y!Df3r>%0gDMx=?Xa9-kt|a{PTL{o<jN6Ww+(&$J1v^N2Xn@Xk
zl^pEL++^UP#|cPCR{ue#tJvD>@Ps$gZIgSz1a_u0(z_Bnkq_e2hc!ooWmtvEvUzZ&
zj9R**e2Kz!AgsTWneyOEQ5~t;ej(Mgt)j=i8qb%Ei@k%5r7ig_9h-a0alg5a>SJdt
z9jsmT5&o><Bk6n^LA(}&5lIvL)zZ0_G>Osg-KhQ1%nt-{=`H{5J9BGm7;2JLF<{GY
zSP6@UKVT&@x<~W+t}KfojYYZXo1XahO@)*Q0<1&vkBdiZVdJshrb|SrPTzEqu@X)v
zdWb`SVEHIBU}@fm0WneA85b&-3t~|+@+bk@wu9T>EI(oBe1d3Y9J+4csm~UDLpT>w
zqxH>S*BvWmylIUjTVQHuSEar;ogFb?)kg4P@RWq`_-&actOQJ|Ntx)6o$puC7aZ@5
z<81yJe1k;Af1<}Ls{SRd+E(Mc6@YdWybTu<bHnz`;(k4~Tz1F~JP~m8>;zQT+jimt
zELum^y;g|mZQ78LFgC8FzIhQ}yONr8K24q~VW4%a7HZ9it%hRcF+!g^l$Zb{c|U{d
z6U0p*Z7O%qM&9gga#e^6`t)B*_Rud_RjmBCM8#La`u+1|RueW?SCY;l<1sI4GS7%5
zVzqvg-MB}33zq8lK!3Vu>w)YuTIxlc%T(yrw3crLN+CyD@7`NG##Q=sqaBz^tJ-O(
z9XA1W4*YJ=QMiTUO46i5dQAd~FrArBrvkP4iNpJ1opTrIW5v{X8$S1s|0pterh98T
z@RFjrPUZ-Uz=1Y1fDAL5?Wsqh!*a#xOW9&%2bXDi*XeRyGJw;k9AmuPrcEi47(!ZK
zZH|)3xfe^~C+*%O6=nb{VOFcoxd_cyiYaMsYL_vSTL$e}a^RZc_&@B*bVC6@4<H+1
zsJ%u>8pREZHe;AueX@m;2}ERvd~bS|xJAJww-mMRoE~TukM*<ZNp>DZq{s$SNkg~Y
z*!8u!gY%f%Y1L@dy-GH7PSLdCY}e$Cd=dL*xZZGW%BA0RLlfFa(OXTkvuhW(alD9M
zESJO9qmbD@w`<AdNYooD#1bPur-QL#{PWL1p!TPHre3GFA|p}m7#HT;TT$UbRwS+S
zvT#qa<HTj_K%<14gf43xN`G$}b8PF0Xz?e6g1S3en|RdzyhjyX<|=VQg{9s{cJp6e
z2YSp3Z8vs*J~GLBvW#dma$3FvRMk7lb|(K{v>C2>y26L(2Zc%$DYJnU5eGwH!6Q-c
z<Gj?Y2<VT!u*?W)D#SsAKCT_tJKb~1>nVFLI<CKJGn8u$mfZ=PdsnE!`7OCf#DhwX
zi~RxyVxjlE2;{@u$2c0Ee>)aq)(b6-9MzdH82SN{Jt<|2Calv+)u9I@bz-d3K?y<m
zLxYa>!ZnrPHUJj_CY6rTOn&JXJ6Rw&ge9wJ=HUVwvV;W$?Apq5oGG9sO37XVDidD#
zApP$R9oMm`auSc=94Akwyr~kdmT<9009(CPkbYn9BD5*KPcR;KIvy@~<m@m%!I@V?
z@m|WxVun{AsR#4FDn~}g<!8u=C3{LxR?IaDQH(=ZF<6Ez!c_~J^=b51aVNeKfjD2K
zBF?9u=XBr>g2!SAHq-eA&U(khqv1#w4%^$@SGzzzkl9MkX8g3gd1%&>TBB8DF|?4W
zqOoyEGG^Lyq+CL%;!-5Alu3FKooQj-R{33&t^h}&M?SL;w9~Oq&-tlmXXi?#TLm~{
z^jU|0trrl(jwY*gV|pvNxA*j3X8a-L+8x`c_-WETH6rnM<)z+SSMxXF4<`!O@-;zB
z$P(?-{J)_*k~As2a4(QJ_VH*qfB0M>lfUN(EQ-E=-r#WowCV2b8w%Gia@E5c=j_90
zy;GI%hrRj%Ab3uJM9*htL!eu9ia4HDMy;2n3mY6^9}p#42gR!r`G^9{Re@0L9U^VK
znKHA(pB(z3Cl_+O-zDnK8O>4lOkL>p_K+}@mYm+($B3MwgxMdh8ERx6TNS@1i|<J6
z36}I)6;wZpNil~t8O8XsDd;=0-GBemFi(R^T*sHD=oc~cPm^zAaW$jyva_v)3;EQk
zpkDhH76&BYIM;7DKc35W@AGPo?c$Nx>@ngaWXwG2)Cuhg2goF(Tyxu9%@{j#Y`Z)<
zoZiq-y=EXvi+&xV8=JO0rxet(om$SO?mRbej{@52id$Fw$j$^6J|I>A*Y3;)m3|GS
z06W!`T|Y#!R}sELe0i*cXY*?yaC0+vlZKQb!a?gK7|nUpvLfxuwW>VMml=F!|3HaE
z4R=3lj%oLK3T%Mq3(2`DKm1G$J1&EhTkbcu3S5>%H^3un`f{rVAp19^6X7WTE&ru^
z6IL%5lnck`%9j*rCQ^&Q(HoBJJKh#2B8cyD7Ia!VByiuj=o95_Tc*q<uiyip!Jcdq
zPq8D5FuzHtjo&t-+-%K`|1VK^e)d&T-`Hh+J88p1;{b|2e6`AzTCJehRMIdRDKIF2
z0001n5gx7C>q>&W-F!R!sUHZyf_%1@NBQ6C#bVTCTqZB@<kW~M)00{_5~c(a-IScG
zooOl<B$w&AvyttB+`9M-+`~Cmx)Un7O4+s9+zG3|xfBa#<Sd)PMZq);c|^DovE#>6
zrO9B9#e3fXEvMx=TyaHVv%9m}cQ5*=P4GrY0001U&vMD|{)yp9H6XHXg;pT?S1abO
sDvoRs00h_o000#<8S}L$)njrp<Y`7v94Uwxdj~$P1UdsFzyJUM0PZ#NIsgCw

literal 0
HcmV?d00001

diff --git a/tests/auto/webp/tst_qwebp.cpp b/tests/auto/webp/tst_qwebp.cpp
index ad4a376b..17289a5f 100644
--- a/tests/auto/webp/tst_qwebp.cpp
+++ b/tests/auto/webp/tst_qwebp.cpp
@@ -53,15 +53,18 @@ void tst_qwebp::readImage_data()
 {
     QTest::addColumn<QString>("fileName");
     QTest::addColumn<QSize>("size");
+    QTest::addColumn<bool>("alpha");
 
-    QTest::newRow("kollada") << QString("kollada") << QSize(436, 160);
-    QTest::newRow("kollada_lossless") << QString("kollada_lossless") << QSize(436, 160);
+    QTest::newRow("kollada") << QString("kollada") << QSize(436, 160) << true;
+    QTest::newRow("kollada_lossless") << QString("kollada_lossless") << QSize(436, 160) << true;
+    QTest::newRow("kollada_noalpha") << QString("kollada_noalpha") << QSize(436, 160) << false;
 }
 
 void tst_qwebp::readImage()
 {
     QFETCH(QString, fileName);
     QFETCH(QSize, size);
+    QFETCH(bool, alpha);
 
     const QString path = QStringLiteral(":/images/") + fileName + QStringLiteral(".webp");
     QImageReader reader(path);
@@ -69,6 +72,7 @@ void tst_qwebp::readImage()
     QImage image = reader.read();
     QVERIFY2(!image.isNull(), qPrintable(reader.errorString()));
     QCOMPARE(image.size(), size);
+    QCOMPARE(image.hasAlphaChannel(), alpha);
 }
 
 void tst_qwebp::readAnimation_data()
@@ -136,10 +140,13 @@ void tst_qwebp::writeImage_data()
     QTest::addColumn<QString>("postfix");
     QTest::addColumn<int>("quality");
     QTest::addColumn<QSize>("size");
+    QTest::addColumn<bool>("alpha");
     QTest::addColumn<bool>("needcheck");
 
-    QTest::newRow("kollada-75") << QString("kollada") << QString(".png") << 75 << QSize(436, 160) << false;
-    QTest::newRow("kollada-100") << QString("kollada") << QString(".png") << 100 << QSize(436, 160) << true;
+    QTest::newRow("kollada-75") << QString("kollada") << QString(".png") << 75 << QSize(436, 160) << true << false;
+    QTest::newRow("kollada-100") << QString("kollada") << QString(".png") << 100 << QSize(436, 160) << true << true;
+    QTest::newRow("kollada_noalpha-75") << QString("kollada_noalpha") << QString(".webp") << 75 << QSize(436, 160) << false << false;
+    QTest::newRow("kollada_noalpha-100") << QString("kollada_noalpha") << QString(".webp") << 100 << QSize(436, 160) << false << true;
 }
 
 void tst_qwebp::writeImage()
@@ -148,6 +155,7 @@ void tst_qwebp::writeImage()
     QFETCH(QString, postfix);
     QFETCH(int, quality);
     QFETCH(QSize, size);
+    QFETCH(bool, alpha);
     QFETCH(bool, needcheck);
 
     const QString path = QString("%1-%2.webp").arg(fileName).arg(quality);
@@ -162,8 +170,13 @@ void tst_qwebp::writeImage()
     writer.setQuality(quality);
     QVERIFY2(writer.write(image), qPrintable(writer.errorString()));
 
+    QImage reread(path);
+    QVERIFY(!reread.isNull());
+    QVERIFY(reread.size() == size);
+    QVERIFY(reread.hasAlphaChannel() == alpha);
+
     if (needcheck)
-        QVERIFY(image == QImage(path));
+        QVERIFY(image == reread);
 }
 
 QTEST_MAIN(tst_qwebp)
diff --git a/tests/auto/webp/webp.qrc b/tests/auto/webp/webp.qrc
index 6519e585..7d0c6261 100644
--- a/tests/auto/webp/webp.qrc
+++ b/tests/auto/webp/webp.qrc
@@ -4,5 +4,6 @@
         <file>images/kollada.webp</file>
         <file>images/kollada_lossless.webp</file>
         <file>images/kollada_animation.webp</file>
+        <file>images/kollada_noalpha.webp</file>
     </qresource>
 </RCC>
-- 
GitLab