From 766e67ce575aaa6f817c842d591948c7c02b84cc Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 2 Jan 2019 17:19:52 +1100 Subject: [PATCH] FEATURE: introduce lossy color optimization on resized pngs This feature ensures optimized images run via pngquant, this results extreme amounts of savings for resized images. Effectively the only impact is that the color palette on small resized images is reduced to 256. To ensure safety we only apply this optimisation to images smaller than 500k. This commit also makes a bunch of image specs less fragile. --- app/models/optimized_image.rb | 6 +++++- lib/file_helper.rb | 9 +++++++-- spec/fixtures/images/pngquant.png | Bin 0 -> 9558 bytes spec/lib/upload_creator_spec.rb | 21 +++++++++++++++++++++ spec/models/optimized_image_spec.rb | 23 ++++++++++++----------- 5 files changed, 45 insertions(+), 14 deletions(-) create mode 100644 spec/fixtures/images/pngquant.png diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 01f870baa6b..5e5e8e41919 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -318,9 +318,13 @@ class OptimizedImage < ActiveRecord::Base convert_with(instructions, to, opts) end + MAX_PNGQUANT_SIZE = 500_000 + def self.convert_with(instructions, to, opts = {}) Discourse::Utils.execute_command(*instructions) - FileHelper.optimize_image!(to) + + allow_pngquant = to.downcase.ends_with?(".png") && File.size(to) < MAX_PNGQUANT_SIZE + FileHelper.optimize_image!(to, allow_pngquant: allow_pngquant) true rescue => e if opts[:raise_on_error] diff --git a/lib/file_helper.rb b/lib/file_helper.rb index f146b8b139d..3f148fc278c 100644 --- a/lib/file_helper.rb +++ b/lib/file_helper.rb @@ -82,7 +82,12 @@ class FileHelper tmp end - def self.optimize_image!(filename) + def self.optimize_image!(filename, allow_pngquant: false) + pngquant_options = false + if allow_pngquant + pngquant_options = { allow_lossy: true } + end + ImageOptim.new( # GLOBAL timeout: 15, @@ -92,7 +97,7 @@ class FileHelper advpng: false, pngcrush: false, pngout: false, - pngquant: false, + pngquant: pngquant_options, # JPG jpegoptim: { strip: SiteSetting.strip_image_metadata ? "all" : "none" }, jpegtran: false, diff --git a/spec/fixtures/images/pngquant.png b/spec/fixtures/images/pngquant.png new file mode 100644 index 0000000000000000000000000000000000000000..2ffb274f20acd27e4e90a621370c709e50b09dbb GIT binary patch literal 9558 zcmbVyWmFtdvn>P}AcP^oZGZ$PxWnM??(XjHkij8naCdii3r_IhGPt`3dwlQydTYIP z*SqgepI+T{y7uX+-e*^>4p)#9|Ac~%0s{l{Nm4>Y2?hoh;r)CY>BIXGE-UF228NtS zQbbV2WBD`_Sw&ktZQt3fNPZzTD~*e#m}M+gu28wL!be1VGwh3d!1MmZ4)FFX{j{DtEz>G~VXR%k}1FYF+o(~X4cCuQ+rSD(T zywA07)@P#;1|OcX-8thu+S<-L&)vJv+wTu&KeBh7rJ1FRA5@NTmJ?*zvQM!89A{te zjUfm%Cm7xsyyXoYz~0{9^Uh~I)w81yi+7TEyh3YF@2Em@gN1(BoWKPI*}-4u5HztzEooVT zqMK;Z?sv1&=0=Miz_CL}h)Xu!Ugi^*-A~iRe%C9mO-(oAM+Hk~9m?k~SAY2*?wb=8 zo9?29Zo@6iVWrW)!&=-=qyKDG!$SS6xghYO2*L2TFt+&=P}rN_V2z3|a5wszi8CasC$SprcNwtJ=X-|0#5dWqP}(rvUfl&S!;W=Ghe1 zrC4qr)4Hf;do&L&pU1aUn|OJSA{i@_vu6m!A=6fFc{kx>=z6MDkYIOX9dSC}KWp*4 zWQv`3@gBH?a8H4mm_N&LxTItbFa3;IoSFn@b(b^KZo8XZw3HNog+No=;Va-y%7 zr(^maxktlj+R8^J!ZW|bxP6O9%-9;uK##}%_d;jW8_PGGR!;lGtekpQrzg}b=xsZC1qY-L615R&b+lsrJ#d+(!51Cylnz2*R;E|Mm>0M$(z5%}aB-?n&}&A? zjD8vEW%0a#82`K3P&{!l<*w&kuwfagrB*U{DT8Sx>vAcr`{m&L)oJfe)XJaZa^{!X za@D!WYi^r=Q5mtZfG4sZz2Gw(8IYv=^1@?zo;l(gKoe)s2`~1;5N;RP#-yE|8QR}j zwr$qvqBXWJNS8i6@+Ymp)&DYq;dRsAHduc{p3CAFQH%fMgZqFZusXX*Rgd#tzFX%- z-~U;-b1pBT{Teu2Zb42&^v`LFxD0T>7%r5T;ngD|d+!ee67buCExpcK6K#ln&NU5^ zBc*zq@)%n7NWKE9bYT8XaQ+Z1ru?ZR!y^LzC29WVVWX>Y3;5b!s^7`(a8^H*IwXq& z%&zdI$ zTHvp~0lxvmoEfh^P;#5(5e4_~i@2H+zilSDv?-?W8(`SWq|O%>yqwDV zbRy$1`S$H$#vkah8LaxY-JR)io*a@_ZKE4eD&|G(h5%VW@{;N-i95JG#e%D68|Fvf z7a&y}AyJeo2{1t3F$cw>ZcHr48Gve9U9W2zEp3`qb^i*^Z!af`bJs`nRU}?KLy~@T zRpcv-RnV0-WsTR?u`JIqoawwR&nI%NcwI6%eZJ(W)^99m&o5kM)L!Gz z{qtM;FX|U8c;q?BmJ*V;Yev&r!^s}OA3Sg?tVVy=>KmQSOVfow!Fj7L2$qvPbjSAgZH>F?bpDaFU=!^Z3 zA?+%~+?W6!<`@Hz6@$aR_B5Qdb6F*C|9nW3%X5HDedXbNQsn%6B>M}wcJj=;_tJdk zqi$xwO}f9RBaYWc1fX{{AVezNMlrj6#i_H%zs{>ii^rgpA`5ns+3^GAFrmQpFvN6z zo?FUTV6tZ!hr9v_dFLy)RvJ&DegiGg-30W~^>0A$EBW;X)Qa=|S)LeVk@9lQ|TQ^$t(sRu8rnOw>z4T4;np|h|Uin4^@6#nu^6M#tK>yTE zmC$c4#m2gDoBZhpQ!h1%Z)P;zOROBVsqNKrhtTT6tddQ`l0lGA^qi9uFa^Fgs9lcD zTr*R)qHwTc(z*-(*I z9!2$v5-N(8=Cl>i_U885E}#f^SO5MRS$F!I-q-s!w$jd^sA=QJGYmyw3(}}`j*gHs zVE+B3Pr}VK41m}aB^nqd8qgVX+pVpSPfo}A=Bqg8Qe9Cenc2|Cib`^dqzK|Y(|u7? z2W@1q2X=hDa39?+WCZ7>H5C=5wekz{Uf!7 zp#uZinO|rGek4fb{9c~7dT8P~chMdUoXFS)9+RECR92n!IT%2Klstrxdt)Co(&v_0 zRJ@0Z*vD7#YYb2kFUe-;;`vFM14<6wunk~%$w9GVvbf=DZiv|YHR~YLyrTA|=%%KL z@&3wWG&~gJltMUel(a2P|M-3!k;}Xl)>w!4+6&fg2-P-EP!nP2kj1+12dxrh0 zr5cm`i@u9#e#zknp%1{f1AAlUB_#{{p4H%9n>xu#)By?X%9E9(@-#u`msnL{u28{9 zuPH5l3v#`_vQFbpfcr@xHW0|}1ZQ*a%k5w_7?_YK!=bq>Zf+eSHo5yeW;?K-If{|F zBo06^I>Q`VqYEcy;J^0$%cXpZ@JkDNP(-NkjZsx9#7W1+U$1`UWOR1 z!IzMVTSVGmkujw2LJ(}?cP|`6ndc>i*0h&jhD9~F&$Q)s{5tN&h%;rM|Hjf3@jt1Y z?(xei?hTdyj?z&4 z^`1vu&*y14@|@7yvEQ+y(>cR0pgEr*@2$&uCx%0$^8sx+x~oOk@pp_kLjU!kkCq3M|79T^Hrq;FguVNp=WO}sv~1rpU$(kU zdlk51*#vY@V3sCGqtRDCi38KSW(kKy>VG{kdGe z?3Fvu@9`Yp?@seNLf)1CslssJHKhAd=>DCz!dkE62H+PE8$>(|{yr&`XAXCB(X@hH zBxc%)Y$*r)GjcKZR^fgG026ojL?f{BEg=v2$nz{Kkv^Xa z{i5c#rvXwnYVR7A_5nrI$OYE4zm3gVVM6IzhA~;0nDz9=42D-=Avarz@xQ$iZP2ml7bG zrA%ZAe>-{NGg7PzN6MM@h4^Kn7!E1&8>90LUi+nB-o1^ywetzjBbDC5NO+aWT@HWb zVL`e<*jrwLc?&}igx|Afi6VlJR3zwaP<0tf4zym#Mf{)#T*3QXoA}khb%$Dn;Jnau z1cJ zr*(z}Sgu~}M^55GYv;12ZWWr8(uH6nQNyF@HJ7`NQ_F_wKi}rc?{5Th&<_Aq?4~kUsM?m@Q%AmiHNjY1i_U7E(fm1ta0dpSydH!-6)#yvvULtS!Z9T0w zKO1o#1NiKocp`qTdAandmrVBzWOB~Wuh!3peI|4ZmGl(wg#|qmFff&Wf`yeU*6%#; zual1ej|W^;mGlH*l)z$8qG@-BO_rUdrMRFTmHal`m$VGOI}?@%UtH{*8^zi>xiLTV zv~kLSwotu}o5U{~eb-QmKDM>65jGE(?JXt@N&i=*4Y2pvN4zW&GR9QO6Uq<20m<}V ze@RLnW_S@L#x+;*<`pF!0~_5b6;XtpxlXK)4d;`u&HL(-#d3GdpD9LSUx-jLt(;Muc!UFu! zGq#lKegXV#3j>S{1Dxs5GPqr(hEeY3@|Yf5)55{4i57O4+{c#~Oke#a!^aVOT7})P z29+3?+C9R(ZKyaIWV0D>@$5F4!qP(WrF8ay8TrQPuUv0PrE(vDAN%2iAqG<6eoM{i z$ytm1Q%mF=*?tlm<*^c|kg!?`nTrzHg*|T4~tt>b8 zI1$*mQiNrDQ0y4C$8%(lW}B)!5Ii(+K`C6$>zG9WNN#l_E4QJP;mIjMsah3bi;|TI zj{jnNGPh$ErA+ZFh_6!tcc==|_++nIO-_ZAudwc&d%qHHLrwGKPxz8v7@w4)94aBv zs;Rr^E)QcOo9Xi~z>htUr*>`4ZIjn-bnxN6ebQkp`P{-m>9ly2GJe; z1P<8KCL7q5Cz!nE+*`)+r29{gfH~*Dr1cSrc4{V?If%pCcC;)(d0Rs*Q0K79g7zX&x z=pHbz!1rf*ykqEryMXzBGRaa0MLj+Viv|j&W6Txp52WDJ+n}Lf28)%U_VhcC2}3-L z4Gau|q#oL6zQ$VQxAgvL|5O<7yO~=q5*S!@$KGfn>f1s}7PizsB2I@*IGn0s!)ue= zL5%xgCiCgy!o~BVk3WURXx&iKVFDspOB>cbw6)ut?_a_f)3bi!=5x_3gGITHit3|3 zgZ8>feGMIGR1zh@|IwU5N08>Q@{kUwYQRetm=o368w}p*Em@z(*-{z9ump&-`WF|7K1y#^XqpX?p87-{S3a7TRFRIjq#yI!7cS@ZFpUtym7FPf{3ajVWC~$F@bXOQRSB#suep8 zJb9Uii2s2S1}7%zB*R2BC6Ahzu?jUpb?1BKw+6=wsx|N_mG+9;FgIakBMG}e79EH^!}|LORDPxW$Y*g6L#$NND24d0rPftxoIp}@Yt?&No`b6zz=?K1W2aR_iEJyK&txjO)$??FOI z|2^09h-+(ex!2%?7)?o8XP94ys|ECFG?$Vl>jN-Ei_4#BZ~(PI5$p4blitXj63HIj zKZl8r7tnV)I?NjQ1L3~{>}D+l{|*x2)HhD~PaegQ+U3dE85$T5AP8 zJ7rL=JK+4!lzc3>rBVlUD_F8^VAAH*<66O*NF2UV%hVL_VrW!CuikOKMgldoyOu7p z20YShW8&l0{&bhrkwfE38DZ%ab{}OQy7)>(=96782ellO@vuQNpX9LLymI!S4ErtB z3#e)%i8jGTXLz*botRTb$C48Qm|#9#a!2==CF25>i;k4-MS^FQc|_hX+w5vTp4b8a{;N)yEteLx(USbKgX`R;ztcTjek#z_rK7Hx zYKzzzk-t;HMQAXW>TuJqkV5y4cQSzL&C$8o*D&#SMcSFOdczU^hdPof{zv3tczIO= zFQo~22XG0!SDOE*zdZMn%e(3LSoj&b`FIPI&IXi?Q|^0PI=6}@ZAehvB8Wl_4|jpm z8G!A^l3TQmd2sP_rZcD3xiX(=1 z7DmmMUWiF#`rdzuu>oYi!}5mpDO=nsuDmUUKng zslQw-{+Yeot)XvV-HXyWhP`XO0m$AoS zD4N!sX@Kekg3DGCE_Bb#wk0#TKF8ZpF6L{?Ws=;Gfw>{{^Ov-vO%E+mFM5FNU;$sJ zBU*0*$L;80wL++kaeroR+iJ?`(6z6nNEqoO9_`t*f=6T@+BC5dvy~HYv)|bN-_J@j zu}rpogpmV9p?7l8v#Zu_^4~y!ZdzWU*U1ht$vBZ~8W;OfNMC&Pe|`>(3hv|nY(j-8 zy63FVVq!Qi^wkKh2N4L}I=y7nCUhul|pmlBIqFs6-juC@IE{I4%XMx>-KkOB` zu2U0nm>BzAoh>{D4^cgBt~3S!%1ZK-k&R8+lP)t6&yNy$R)+S)T8QWi{rG>JkCYhw zk{jSsEy!5X9%fGyHJ&dUSYPNx`t!+qgt zXJvUcvd3vv$J3xl1i5A~=b$dxO@n_64*v)M{GAz;K^AgDGATaz$nYJh&M-qTtZ7(E zl4_Hp#F5_qC@~==_>rr$m0hSVLW^ml?J<|^tpT~-^c>A!TZ)6f^X?D_GcdGx98T2; zlQM)m-VP@+HI=NFo@zTO=~(9#n?d$-UhY~};1f2hr*d^hdT!HnshxK1VwSx3UkQ2G zS1c?=3zMWsrN{B3zR!32HLDH=Cw~*o2W@~@TrHwgd^MhWvlOXaQx0zr|M!U} zHi1at|7Y#+e^Vf>v90BtEYw;_UYa~;@3Q2&xRyEbyH=8`^TlDh&+KjFw34S=!qNzT zBSNi`CF}APTUR5uPHNP8qPjieyQYc3ubkweOOzb*!VZL^ijyeZ`f%!7{03UklS>q( zG{tQAXR!!RW~oX10D$)9+m}pABn{Gxa^t{$3QjJVzWTxKFD^_(jyFBiu*x2m8~~7K zo!c49H;pLQ-7A(87BBs(Mx$;q_`G)Ik%}U!>8WZ4-Q?QY6x0`WocHwn5?x-wo#6;8-vrr&eSc{-_RR1M@ z7*JSl=agpImW55Hig}+Jnm&ewG9wocu5EFgQ*HJhRTy^TuFmR)cy?$zv&mD-CB^ki zN-3ziai%mbE&61xi6{B6AB$ziwJR&T)v;O{0H`lv^FtbQP=opf2k!gPyxx z4%!y|O!K*s@++m#AGa29oAzX!Q;tfLnkfJB5AkPTF2P7*f5%R7?Lo$PZux3Xc{S=T znmJ0fdC`@ZH@CjCz5`8AcA`EU7~Bs|Nw2#UP0XY%v167o_~y@8#4st8%-%mT9-*mB z7L%kmZj143ePM>(MBsS$Q|o0Bu4DUmO%qb;?PEN>ofkfBg6NnM3yl_FN)D){f-^0y z-zK%c<2!(|nxK)&N`I|o%W)Lt*`kGYY-Cl^%}lFCBVFzo^{ZhiqN5!Dg^??5`sl<$ zP)nu*i&U3Bm$fnku}>#Ew345TODoG)l_Q9!oT2wzir%Q5StWcjeg}UI ziF@aD-b!`vmK$nF4`#*6#73pVBPV)B?oQY2+Z4CRpUWVzzoy$cWtjZ!mQlSKLYS6k z@}-%x4kG(};!65yPt50gZG=+QEXm&2u{mZ%wVx3p6cIu%5b7*p15b-F*@XCsIbi)M zdi7RNXz!>=Dyy0*%aN*9M#h>6@n701l3AQ0LQI(W8Iasg2w02{(41ACoU)nS58>># zgTn3r<;R8HQ-uz!f|b{nWpd>wsZN}jTF<`F0G&_ZTThg-Y|-0yP=lgI8g}zGb@U1x zp+7GYimaG;cJehjLR(t{8IPft0$)%KpSLpr=gE8}b|s^W0%8AtjbHio_I?K8atLK& z2NJk&(rwF@xp2S4me}doA9=%o!P}Q#kQ*5i-B56_IUT$A*%&r^MyilSigoR`eo(e6 zF-Q+S^=G0y6VP^G`3)1mFY(mNd>V%dme=ZOMSTs*_VvK zUZ>P;OD1_2R@I%-k|2U$Pt%4_cu*4Jd)6vZJ4)TPY+R=$!vT%)gLR;nVZo-3lojK= zyHv2TS@c${D?}6=HG)ju#@Ke0*Z-h8$$S!8QIw4*;u+^B2g8c>slMtXo}?_ss-7cN zEZ$&URYA9$ZDHsp-kx9)4O`l!g1Wi~-^fx>R45xmL}+9gc7_|;fs}G8SL&>2lq<$Z zo=pG9QO7?RYmFRrmt^t0dra!Pxy@9O%aYvD%a)ZBWYb|SVaM@dx}%r_>vWOIis|N4)7jy3Y7sAKZu`#PK| zr&l$-!ddpWt^P>21UV`v`e}(_SJp+)Di}%jeZi(NGOU|*@+zfg=VFWVNpi^hE(wz7 zDJgh}`mU9pl^ush|Rx$VZANiZ+D(t%Qgc;6OCB?GC zy;CEQUA3C2se^aA=DJNbHXJDF;03>GzQgB>X)=PVo!h0)^{R(%AYq`^!Heuy6~v5P zpG03j1n|2@oodg=h8^|n%@KMcHRKN6f6#v=5kX?ENP=kq4pEmWGhc={K!X77dl zk$eN^tFc?T3$g1?p+DtZ#x4+$w%RCeTfVM z@$S1||6_UmyFZ7!x~4{e=Y$?q@_qb!Mce7bcP)ENY%{gS$=$8E^J65bnwH}KG9G|a z_3k@eFqJq(zSmE_Tq-&>LXl1jO_ydU+OB>cqM)T;0Ja>V&)qk@!+6RB#E&^0X|_n6 ztQJA$+@~yyZa}Of;@Vbha`fU;C2tQ+v>0dW2$yyuGHm<7a(Q?kAKHSZ?dcR3emF ztQPS6()T+$4(r<7yI(l<)jDxx+bEvYHs3fJ*ReHokDgE>eBzEI(M08PyEx|Vvw$E% z+BIi(<{a))7lsOp^MaYqbfrxY_czAxqi(^k@2eK}J%5Z&epdGWv3Ro@Z>c1z!B|d3 z<#y`$=Y5I7zV`f=P_8rg3Wd|vqSLr=`Rl}%2*&QbZ?>ZutnMiPdpg2q;7(n1H+Ty= zlv&v1lfEZH!AHj%o4R`Z$wkrDyVhFIl6S!ja>n?vb=J_rH5u&OG9QeJ;HzHbF7}fqq5#y(8%fH zxVUI|zqs$#qp;y!`VZib^q*jp@K@J*FwwlDhQc;bA^eZJERl(!$eGMX{mp9OAK>6b z%h&%gN|mOgpBNssQZM0vz69MNp`svfP_`}^f3VgDJGuG>0J1$9OTG$2gqkBGZ*hMh z`__7Bhf&E?G_ABtX4Y^~9jv`b{ruc%!EE&_?pT!cGY#ae`Z%adr^_FM!a2XiQ0~)# z(?FCV$=D(k)vg;aS22O0{#f=|^K%(jN4H=H)0AQtaZCC!9Qa`nB1wnl%xF8?2qDd* zt8)uUMuH`lIIW}0i-jyaAAE}?oo!-MuXivt%Bu-N>;?|~Wo>H6`U!o*WuAff36m$D%%Htx4lPjC 50 - expect(cropped_hex).to eq(fixture_hex) ensure File.delete(tmp_path) if File.exists?(tmp_path) end @@ -128,7 +131,7 @@ describe OptimizedImage do end describe '.downsize' do - it 'should work correctly (requires correct version of image optim)' do + it 'should downsize logo' do tmp_path = "/tmp/downsized.png" begin @@ -138,12 +141,10 @@ describe OptimizedImage do "100x100\>" ) - fixture_path = "#{Rails.root}/spec/fixtures/images/downsized.png" - fixture_hex = Digest::MD5.hexdigest(File.read(fixture_path)) + info = FastImage.new(tmp_path) + expect(info.size).to eq([100, 27]) + expect(File.size(tmp_path)).to be < 2300 - downsized_hex = Digest::MD5.hexdigest(File.read(tmp_path)) - - expect(downsized_hex).to eq(fixture_hex) ensure File.delete(tmp_path) if File.exists?(tmp_path) end