From ea4f0a240400d8023f0f748479c941af2e9dae92 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 3 Jul 2023 11:42:25 -0700 Subject: [PATCH] Preserve Transparency in Memory Drawing (#3627) Fix #3624. Use the same logic as elsewhere in the same module to invoke `imagesavealpha` when appropriate. (I confess that I do not understand the use case where you would not use imagesavealpha.) The fix was easy; writing a test was not. Google to the rescue. --- phpstan.neon.dist | 2 +- .../Worksheet/MemoryDrawing.php | 3 + .../Functions/LookupRef/SortByTest.php | 2 +- .../Functions/LookupRef/SortTest.php | 2 +- .../Writer/Xlsx/MemoryDrawingTest.php | 91 ++++++++++++++++++ tests/data/Writer/XLSX/issue.3624b.png | Bin 0 -> 7949 bytes 6 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Xlsx/MemoryDrawingTest.php create mode 100644 tests/data/Writer/XLSX/issue.3624b.png diff --git a/phpstan.neon.dist b/phpstan.neon.dist index ef2ae14fa..52de75803 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -20,7 +20,7 @@ parameters: processTimeout: 300.0 checkMissingIterableValueType: false ignoreErrors: - - '~^Parameter \#1 \$im(age)? of function (imagedestroy|imageistruecolor|imagealphablending|imagesavealpha|imagecolortransparent|imagecolorsforindex|imagesavealpha|imagesx|imagesy|imagepng) expects (GdImage|resource), GdImage\|resource given\.$~' + - '~^Parameter \#1 \$im(age)? of function (imagedestroy|imageistruecolor|imagealphablending|imagesavealpha|imagecolortransparent|imagecolorsforindex|imagesavealpha|imagesx|imagesy|imagepng|imagecolorat) expects (GdImage|resource), GdImage\|resource given\.$~' - '~^Parameter \#2 \$src_im(age)? of function imagecopy expects (GdImage|resource), GdImage\|resource given\.$~' # Accept a bit anything for assert methods - '~^Parameter \#2 .* of static method PHPUnit\\Framework\\Assert\:\:assert\w+\(\) expects .*, .* given\.$~' diff --git a/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php b/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php index 3e2baff6d..cf616eb75 100644 --- a/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php +++ b/src/PhpSpreadsheet/Worksheet/MemoryDrawing.php @@ -162,6 +162,9 @@ class MemoryDrawing extends BaseDrawing } $mimeType = self::identifyMimeType($imageString); + if (imageistruecolor($gdImage) || imagecolortransparent($gdImage) >= 0) { + imagesavealpha($gdImage, true); + } $renderingFunction = self::identifyRenderingFunction($mimeType); $drawing = new self(); diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortByTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortByTest.php index 616ba9367..a2eb006d5 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortByTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortByTest.php @@ -20,7 +20,7 @@ class SortByTest extends TestCase * @dataProvider providerSortWithScalarArgumentErrorReturns * * @param mixed $sortIndex - * @param mixed$sortOrder + * @param mixed $sortOrder */ public function testSortByWithArgumentErrorReturns($sortIndex, $sortOrder = 1): void { diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortTest.php index 1cd2a138c..3145cfd3e 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortTest.php @@ -20,7 +20,7 @@ class SortTest extends TestCase * @dataProvider providerSortWithScalarArgumentErrorReturns * * @param mixed $sortIndex - * @param mixed$sortOrder + * @param mixed $sortOrder */ public function testSortWithScalarArgumentErrorReturns($sortIndex, $sortOrder = 1): void { diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/MemoryDrawingTest.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/MemoryDrawingTest.php new file mode 100644 index 000000000..bbc7fe230 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/MemoryDrawingTest.php @@ -0,0 +1,91 @@ +outfile !== '') { + unlink($this->outfile); + $this->outfile = ''; + } + } + + /** + * Test save and load XLSX file with transparent png. + */ + public function testIssue3624(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $contents = file_get_contents('tests/data/Writer/XLSX/issue.3624b.png'); + $stamp = MemoryDrawing::fromString("$contents"); + $stamp->setName('Stamp'); + $stamp->setHeight(120); + $stamp->setCoordinates('A2'); + $stamp->setWorksheet($sheet); + $this->outfile = File::temporaryFilename(); + $writer = new XlsxWriter($spreadsheet); + $writer->save($this->outfile); + $spreadsheet->disconnectWorksheets(); + + $reader = new XlsxReader(); + $reloadedSpreadsheet = $reader->load($this->outfile); + $rsheet = $reloadedSpreadsheet->getActiveSheet(); + $drawings = $rsheet->getDrawingCollection(); + self::assertCount(1, $drawings); + foreach ($drawings as $drawing) { + if ($drawing instanceof Drawing) { + $path = $drawing->getPath(); + $contents = file_get_contents($path); + $gdImage = imagecreatefromstring("$contents"); + if ($gdImage === false) { + self::fail('unexpected failure in imagecreatefromstring'); + } else { + self::assertTrue(self::checkTransparent($gdImage)); + } + } else { + self::fail('Unexpected drawing not in Drawing class'); + } + } + $reloadedSpreadsheet->disconnectWorksheets(); + } + + /** + * Determine if image uses transparency. + * + * @see https://stackoverflow.com/questions/5495275/how-to-check-if-a-png-image-has-transparency-using-gd + * + * @param GdImage|resource $im + */ + private static function checkTransparent($im): bool + { + $width = imagesx($im); // Get the width of the image + $height = imagesy($im); // Get the height of the image + + // We run the image pixel by pixel and as soon as we find a transparent pixel we stop and return true. + for ($i = 0; $i < $width; ++$i) { + for ($j = 0; $j < $height; ++$j) { + $rgba = imagecolorat($im, $i, $j); + if (($rgba & 0x7F000000) >> 24) { + return true; + } + } + } + + // If we dont find any pixel the function will return false. + return false; + } +} diff --git a/tests/data/Writer/XLSX/issue.3624b.png b/tests/data/Writer/XLSX/issue.3624b.png new file mode 100644 index 0000000000000000000000000000000000000000..48665d02c5f03b7a5cb0ef5968f5595e5a84b965 GIT binary patch literal 7949 zcmV+oAM)UdP)@d%fPj|DLT~uPw**TDB4hMRJA-{m$(9KKs+9x~C_h z;JN3_VSt(L>iX*DTeqx7J?habOi>)iJ^WrUMNyPC0R3koSV=e*PFyYdX8s>d-|V!g z!GygERDXb+{N#Ms@X9>FCjbr*Z;}&`Te?|~GR<|~%!+2#hgTkRTsE^~yegRchSUh) z96rMv7BGnV4UTZr6E~}KP*3zXO*zc*pqY)C*(hEEnCBfcTQ{>UGuy}C8iJe#a1Ou@ z;f80-tOS4$%xn`k+rg^Jw4!>HIM;O`$IWcY%;xc$#QfKp>9LvJHnY2C_RP%o0CFlP zB>^1ae?z$8Wiz{EX6GO>>fkP$**!B`0nmqfGwQ(}CpRR5dlqy2mYKa{W>+xZgP7YT zFj*|3x{6mKB>_xvF$QoyHnaE4>>{{&6|3nxeC{r8bo_G6*(*a4z^!xYRWtj*%sv6g z=OHnRP=k|@lJ&WN3Z^=4WCw)+js)N{0QdvE$icf9z%YKN!LS%xOdF#8#DVBp36p7*EETPYjW{rtegv2d1u@}D6X%{ z-y=wX7=Cil@cR8p{u~@#5OCjtXYvt@*kOqj#u7~0fVc85GyB@iegd=YbwWg!A@}Rz z_Gegequ}6@=wx#42z)ma(Ne?rcmBkjOoO!q>-ZfQRG-3n9&usdNW^j-+#3Lx;O=CN zVp#(ts|p9Z?sxry0n$8Qz-r6|#PuZ%@DO62K<_qk%3o8;W3FakNPPzGT?E7AoJ6ES zwFI&J4}kk5$*yS?U;{Wi2$TH?Yr23z&0*jn>-vgBTNtk<=zk`Vi^92ea6bp?+Yrm2K`f64<~;`IK7gfs!-3nf{gMS>1F!_;p7X!*5Z5K=BNq@> zdmG?VA6N1HL%qRkWLYx&ste*kV-EWizb{yGt@Wx7J20o-fg+`O}vIj4_>bL-;y zKGb5{^OKc>J;;U?#<1G}t~CQt`4Bk-M`i#p4KM2O&DSKPNr^QA&V9@2To0~^6niAr z@=XLawnU}1ML~ec_9?YZhXCikR3R_{F@^UgoIHpJuL>t0;rH#3)2{b^1;n-L+;;)& zrzzmZ5X+CixnICyzS~J~tpyHNIs!O%rG97t4>Rem9YHT$6oBb2Ex+O+&7z3q>u`lW zfPN1E%R1xYh{Z2AoXG&k%7@PR_#>D7Zk#FkGve_{I!`yo%AT zyaV@Ba4#cNI=``S?n8k4e-O`i0q#Mk2iOh=n^=JLXxA;_BW9l=+A!OKX0R$t}6y-CV(4oaup8x8bX@PI2Gk#HIPn_FH3am zP{j6rC=lYu@`8zVR8z2!KXqc+&nqKTb#Q-xSpFJfd85-}S*a-wPH~~C;6B3x6r{}d zvXu7l%M65E6@%+g3duSysyaxK6v-_juKY<#_%Rl6$+sKOpd_>nqy4bRwBmDDuA4a9W?7I7@a zVmW|0FG+2FxroWSCY2+HQoWy8+w}rS!M%)x{ipDGr=$>qL~Hp0K>b3380)^wxLedo z4mOEf@4)@ZK^M&_qM+|>0KA1&Ur9+ZMlP5Vxr7I6!y*@Xi{(|5F~|1&-#q1| z0`d|#_j5$6rc=PBSiS*}I~v5uLY)Sfbc_dC!tXgTyk;fZ6$xj|Af(BnGJPvvTBpU=yjk*Oe zLz?S&&&td};aDH-8i5%901{vq|J%ZZ_d$=duBlJ1IUxVb%X3BIhM8o+Dp)|rxX-@W z0DG<~bO;6&3)BCKg{VbxDlLxSHvSBq`vVNBwHFb4Zy8{9S5ovM3-|5!^N1=B#w1D}4^`#>xip~Ac z|L#A_Ycf$Xg8ME&7aJDhsBo=<8_9N^-=K3@B=^F>wNnJxz$M$rq6}jq&w?u_oyd#C z#YzHDKii1B*bY%nqyqh23v@S7VrlUmgdXdh`)_c6TctLgqK*Qb!SE`BGgh$TFG0tR zI4)I60woTJUoWD)nD(OE|85DABI*7tB&_T0Utvr>h2i#6RF9q}2b_ z!mBEU13tK6j*IiMpmG@{McOl&h+x7#FlX`yu;~9~%qpTq3#NE3LPF|O7B|9%e&Dm)J?DMDBC63#labmeCZVh^FMSRw6 z7mXqZN6t{AjvavCJ%<3xY)T$Ci3<46Wsy#l5X<<^3-NzRT?5#q78!Fsi(L=w7%(5$ zRhLGQh}{k7@mq-bZb)fwMLw(hmQ^&<(7T4u56e!CW<+&Pc5_4$GK~Q8_KO6$YXCbV zi{(1g2pYT(18XE;%=n3j+;CCzH}mbEhAY3Wdf`G9fbZu+c#>WM@ulYO6k@TKbsFKaBTRuBUT>&`2z1ogx5q_^vv9Xe44dOP!=O#o)`-+e^ zq!U3|CUnoaDH^IQ0NisB&wp{dB`xqhll!pHuq5$lj!Vmt7@IznMSJSL@o8{y5jtuP z;Lcz&m1sOYLZIL-ChHOan04QwuB(ImrW46ET&E6%_eCR#k}bl}dlSDG17j-7wwpZ| zc3Zdx8`Fe#ShBT68H;vbHsRGyGrHAnfYp_lg;6yga7QZ8qrV`3dJBKQg2{W6xQ{Ch-&s)!0zMGtEZRulebG2u0-&LpQW9!!^pJN~XG7<$a< zK9TX<#`xI$WE}q|M=rQF3^cOFu}IgXc^O+{oJypt+W>2?tqb6`qBBlv;EE-1?h+=C z86KUqNUA7&*$+W3yUL7bc+dVAZy~q)ri{mAJU)5wHPVM$vRF#eN`VF;K{gp{L0qvX z0GS2ur;FR)-E~iQXylv@l<`0P>jyv zaEisLfUc{F@p2zOb!8tyylH(DLyhMIAGRc`Qa?S9Nh(D?=5H`d}wkUBV*aUfF3(?pVO@tx6}K zM-so)ipK*OV0R>V5J87d2Nq4g1XL(aw*cz>D z-n*83LjWgevChB zCWt@J0aTOk4Sh0E@N!V z(vmQ8(!>IP4Q7n)Wo~P(Qs>yZ#Exj>lQ1#hK(h09glbT!5!)F5x zMQK&fL;p`nh}2xDlxm2~R+Inn^XwI^-Hgg!%KAOL1*x!@k&25PF0u#FvI(x(6OPt1 z9O(OAfS>5CXD)_)NfHc5t(I32u{=228Q;S_2E`xN=c#{=m6mZpbXW8Aebf z^Ar1%!*LEfvB*X1 z69gtk)h6`+Q#=Pn__PBucH@s6;H(m@ik0jmc6q6>-oRWR$vFAa*1>*Znuz9OVUj0! zZHP?~`?}BIes;?`(gb5d0XxoZdD zlteXn%ftV!?#_lZG9MRiGzVyswM(EGhaQsa`>a-849|J|zAx9#$-P45OP+?LN|ga5 z71dScU_F3iEFlT+BS>-JVRR?|#RTh_tB)Ja`f+bRDSncba#&EC0`P!{v_@2h1XvS` zmd(q&)$F-7^uI`Iv{haCvFBuW#()^CvAbp#BIgf_%G(wjDWVuCO8LrY0v9AbT;#-J zQA$mR6UuRFlIlB<(w2}YO&SLm$pN@{2k%D$R7{H0dfrtD>q*uBQO{ zymPz0a3q@wo+~|~yU~Yok16-5kKI+aDoS)q2yOGpG zGf2^FHAfPKbhUSb;tl|-6gd(9Ut;kYD|paW*%0fM`O_8 z=cJH6%lFw<|C}2~=3dMx*Y+G5dVcR(wo5d)j#T6qq&De-_-|PheA2^_GX}*XRN@2f zyNPY)9*mMrK+j*q0xf0a!dMKRb1tPAH*+RJ;Mw!y1%#emx65=Q7OhdWbxDH6w4z7B zxl;*M4b2caqhG|PABiH6|>cmfaFuI}Xu?)S=Gs3x_{L!^m-OGtccQ7Op)&i1&%BEUl=B=B~YVz}1K3#%zBS52-3HQr6b= zIT6c~;M_Sm%gHY!4Fv3ozpb0ZEda8D%j-3xlXD`$^d9=N5(+71B#q5)39zdW(I*U6 zZFkfO>ya_k0XLCKHK}uI)Sf;Pu}@opI(ItIp2-B<`byrw3O|5NFzM_JwYX`I%mhN3 zdl__QPK;K+S6RoU37cjuX_YXH<3Gc59)?7IK`I(*&Yx|TG&I;RvFDHldE~6Uv>*aI z1563$P9_*s6v^zqw}MdS9;6hjEhoj^P#_CZzs>70lD40+AT{_iDBgponv-6w>R--n zS%O7DeO?gf*o%@@NQ2`vuzXaiEaA$M zkOXo*5Ig5M)5LX7yode-3%cr_y&Wq$cAy3qOF_bt?K=6&!G5!}38z+F>^hX8$zh^K z9k>$-Zd~NRU4m7)DqA`8azBo1GEnxaZrFMJO+%=AfpO;~)ElaoCY5EQTq`1)^LQ)~ zz*PI7AVK<0=wRKw7fScV&PDK~mhu=x2+0H`*=HdGU{_PL|c*tu4B~D>TwGqS`94P7CT9E$ug=Wf2|dY#L)0ge&jf~>SDG0 zOi_H?^HL2mj{k-BF%ewPxeBg{apxyBb}@jE;WaH>TXXj|$vuxqQGt?57{=clRm8;d z(mo=a(nEx4$={#J*T)rv#XBLME#PnJXJR)ox`&t2gi*DN62K^?=(Jsc`#`D|_Y)#y zlkWbgA%Ii0L-+NfNE&&fRUpcLKt zc~r4ZaJHnSq1L+5KG91JLcE#I@PCK_d1I?xj@AcE3m=9880QjvToQA+PXyex zj4@=51Q`~Hjyt526=dZ$CMTF`EhR8YSt>Apg(}$;i~3k3j{3_|wL#ev^LqFOnl58%!TxW4tU2ZqPqGXAyQ9DVj<5YjD_+08L6k@ zbA$>d%DDEgsI|T)`e)gd~TmarutH>7 z*q%f8reh$cQNeB2qNT=2l%8x>6j#OXx;JU=-H+n#OYY!GAIcoXYe5R*L-AW4j(HDd ztv>;Xr($e)1FjYjZB_ZHpmU-rQC0+mql7~?SfHq)+;1V}!sM=uJCYFS*q)@UN852Q zh6z4)Wvlw~;8NuFVR4k;r|m*)Y1)j|@&)L!ad5CdB-sR9o?rZpoKo^sq=XmCs{4H? zFZ9y?6#?wFR7X_Yp^_Y9UfjDlu;61{<1sjQMfL`_M6Dc15-M+rotRTg(q~!xwaQ}I zu1Ys_ex3n=8C-uLA+1PbY84#&Pc-@b1suWSR{eqqQ6pnesVrG8f3DM_s}t8}3K^3% zIe-5So+XugyK$mSmii@c03=Zbt9VTU4p#%26|M9L1uw|C&j9Wh0QeUE7HZAVNz!vC zs?OSeGeOse+(2z}_#o}{?rt6x)XDw91ZGF740ZxWN|caFGYKa?1-NX7&po=`2A5Qk zhT!Is6mLm~prlwqETuG0UG?axQX#Z@+Bn&yB0-2%(UU$l8=8=s^lPSAzKQLeOdNFG z>J(CtYoepMXK6SEU{Vw?N}r={AM_RE;4C){EP_ZLq+g7Wq=1JK-3j*T;5oX#O0`1j zpxT+Jlm*QLzalw7^)jsfP|f%6m&9T?1A}rBoJ>z8@!p*VylD}gG8vkA;kK2my9_jy zcOQP^h8R2z-HQ6Sy2uUm>8mc{l{g$M^7Ci|wIacoYCD#6v(yP4tZ~{o1PDHKr-5a; z2CE^s?Q{#1QII0}Pzdjji?n#gVakzxMcKH$0PdZYIC&Zs@GNhMPfefw)DFgO3g85> z%pC%AXm0X3j(b~ZQctmY7#p`tBe?NU8j$TsaHHaa7LCUGJ7s8GuFrJ=9rZpze~b#NQCh2{=`&5!MlWR`Qcl}`OcJ?6kNn`Kp zIJ5pE0XJ#eMr>hMiGO2F(}ZjyUzH8ce63=FHlg3N6jv)o2)st!s&Y@0zcY-=Rtt+* zuLa%^;P`blvaU9;xAVm$M}~21X~N%DV7((y^C0ANj2EV((N{ng*oIRc-q>K zh%xN@Ksw-c+)8W+uoGuF;oJr2q`v{=dDlY0WFimX-!5S?XtW~wh z+zuBv!@(R7IhWvmfEAv!acvi?^gCpOo?s%bK>S{nB!8&7Q`2->yEP)>QN^qbxMW&qYq(7OQl zL#*^rtuiB1OX&A_18s`d@!4C*vb+OV;-XZP^f_*}1kUFB0pZHXxl&0PB@^z|MO2W0 zO;VxXp+wwnVDUc1g4mYtUUE%JO+daP*H_zwoczawx1o=-0<1>W6&O?30gCS)8%t#B z9#;4NqEux?CSXmrm)wVix*#X9lqAy>+EU}+>nkKld^9a&`Fg-TX=lFgk>O%D+HEYf zve*!LajSGpWR^=VfO0IhN=K~gtN?3*dl6X`cF)nt;m>V=b`#m4?*R6;iD*QMW75>uRN>RHg;~9?AfPI5rY}%jfzYQt3xII`&WuE6tln(o@=l%p-M= zrD~FP!=8`ewW5!+0-Trf8XiI82OLR&{w|JryWu#O(VLo73$lvjArcGIQuID6oz>4^ zLi8Bfqzce9@RKM?;PQ&>nz#*6zk!N*AXOT)sMlq&)Wl7j3q-(jas@%@AskDuLimM_zUoA^w{fjl3WJ6hLQUEOm9k|CI!$jZshTG5dh z5ILEwsYZBbdV|B(cLHz=DpBPMj%;rS7f~`9V z_GAoquyF4Q*Y0~`NS=q)pS~P67VBDT)r!*3n(m)=KxjE`-3-8NW4Ml0&HX|#2DAj7 z`xV@whY-EZB)PRg2@k^90=KZ@b67T~v3>6u;buKJx**)GB0cBCdo2DkTQ#oZ86M&F z#6`{0GF;^3EhioFVtDyDJv;urD`Pko$@rYnl~o^S1(-YLpF;O&tKKpu;#-L2A4TW3 zY6-^9hdmTP3SwOobn0c62lYu!x9|U$cidwlg~}2r*_3{qyP+0E8J>aSS_2z>viN-0 z`T?k;_pp!-L|pH*QjM9lCX2q`6X>9vGyy#Vpg+OsUy;yT_=3slxwQLJ5)kmnfu57U zlN_kCDxO-o=eI!nZ*Bc$E2}`Q_4C(%m?;HACH;69O zU8(6BLKIJr)uYs5P-zv?ay6kAJe8U;vS8>1LjZXI!6S8_Xj!wj^dc6Du>e_df462hH`C1;e5VTwpeafkU&;cl ziTm{*5!FKw$%D@58k8QQB9LQz%Pbk*&whadGI4tkxT=d zEx4a7{%?ABL_>gIQ8D<^P>Mc|WqS)dCqN^s)5xXv#~8dK`Lslu~}IHDN=NXRkCl(+WZGiX1Nii)fm^3V0`!N~Ej=Nc zdjP+rlmfJ`R`E4t&ruJ%9`&e4J?c@9deoyAL1y;