From fd2cb81503ce3e19d46ce69c9366c3a6306e991d Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Wed, 17 Jul 2019 19:14:01 -0700 Subject: [PATCH] Respect EXIF information while decompressing images. (#9905) Adds a unit-test asserting this behavior. --- ci/licenses_golden/licenses_flutter | 1 + lib/ui/BUILD.gn | 5 +- lib/ui/fixtures/Horizontal.jpg | Bin 0 -> 11343 bytes lib/ui/painting/image_decoder.cc | 56 ++------------------- lib/ui/painting/image_decoder_unittests.cc | 47 +++++++++++++++++ 5 files changed, 56 insertions(+), 53 deletions(-) create mode 100644 lib/ui/fixtures/Horizontal.jpg diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index d480325c6a9d1..375dff55b6e06 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -332,6 +332,7 @@ FILE: ../../../flutter/lib/ui/dart_ui.cc FILE: ../../../flutter/lib/ui/dart_ui.h FILE: ../../../flutter/lib/ui/dart_wrapper.h FILE: ../../../flutter/lib/ui/fixtures/DashInNooglerHat.jpg +FILE: ../../../flutter/lib/ui/fixtures/Horizontal.jpg FILE: ../../../flutter/lib/ui/fixtures/ui_test.dart FILE: ../../../flutter/lib/ui/geometry.dart FILE: ../../../flutter/lib/ui/hash_codes.dart diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index a1f15b09676e3..b6033292acd7b 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -151,7 +151,10 @@ source_set("ui") { if (current_toolchain == host_toolchain) { test_fixtures("ui_unittests_fixtures") { dart_main = "fixtures/ui_test.dart" - fixtures = [ "fixtures/DashInNooglerHat.jpg" ] + fixtures = [ + "fixtures/DashInNooglerHat.jpg", + "fixtures/Horizontal.jpg", + ] } executable("ui_unittests") { diff --git a/lib/ui/fixtures/Horizontal.jpg b/lib/ui/fixtures/Horizontal.jpg new file mode 100644 index 0000000000000000000000000000000000000000..e080f249fb1ae6d16b2770e8b3beb8e7726fb105 GIT binary patch literal 11343 zcmeHsXIzs@)At34AQt2pl%j(4fOJArj8X-p_n;!Z_bvycc(6oz2SFeZAT*^n73D|~ z=>!x6RJtg5q)G{S_KK+IzTf-9dp}>Fv;3HBuIx3t^Pkz-*%`_k%6sUbmb#`oL_-5X z=fNLD`2e-61vxuFkd6){3_%bh#6a^KL&x|M@ANc=(1>b<^peK(f;JwmPPlYPiqb{r+nBG=j7+-r6?}$;VWis=V@av zX6xxL9%St$E-5A<4xL5^d0E@K+WT?a*gHCVDDix0XyoB`wo~FUL+ME9c&XSsIiJ7k zV{dd-*Vy)|tF3|^5Bd!E=^(`*cQ1E)KWpwFcQ+4T#ULdfDsx59haZdca8rl)xhnCP z>*#Z36Lqr_yyBzVBvK6Va@hN|j2?}BeiJUfdF3=9+tlos>!aTJ$SP*4z;kP?@Y z5(OhfeSOy{bN2M$hB;c>c>4P(@qn4XFUj5OC)@uhJ3P=! zQN_m|hTGWF(@lxz-{Jp0M90(4*&$ff+Rt8z2Udrugq*0PobeBJ@Q4GE#CK$ZyFbA> z!~IYD2Z4VO_y>W15cmgye-QY;hrqA!%H9LSRDmFPf++KlyuOpCpQo>rrxytGBp`WZ zO`TnETnHMq&rj_hBpr(wiiXN}eZ0OWdw7cl&>EUT;Lc#CQE*ARVPDq~HeaOI? zfyWymbt5*Lkyp9wIjdpsUwqOwK4E*94j*A-=inC*6haEipk(Fb6%>V7PoLzkV`~w1mg0J5EBmCCwJ9lw$@d=4Z$tkI6_?%oqUVZ_wu)LzO zs=B7OuDLMjj^wTW|biYtqYiz{0wgJ$7n0XMHZ zUetDHr{z%FQ@9QMY!twR<^(UVtd#5}DXB~El7CNr_g^>4D7Y%@rwl{;=xBg3(Xl{i z=%W)pWuYvh3Oa-9cD>7)Pn$ZeNuF3Ae9)OD?ADL3IQ$M3Ixr`%vvuf?^$jS@S%s#ix;vtN(H25tKVQ>hO_S!k!0h)75LX6sT8l@i*I zH?N_biYM!0uPzEN`<%R&m^*MM;b3xg+Ebnqk4nmRSEI4D}=|17@c#%<5ZY1TA1Ak12sA(A$$xy zzbHSovj@xzF+9jT-A{TuK05~7^0~PaBsM7t;h2j;~nivU&ctY|?+`ol7Xup#@703j-tutU1ei2L`y4 za9s$%^&5}`2*AqAq$QNQ`?1yB>sjP-TjRjP9;3Mf0Fifxa!re^ zsB&)qNtRgMVzT7jO1hp{3@biR{qE>Ru>Pf(_7etQh2wcGiJipzzyWF|tPKD*+$~==An-|RY8+%OL)xQeRGU}@P=#{9}U+O(Q z&t-%OsQ2@di0xWErbN#lP`o#_=+IEpm1<*;Cwdd61^7x?tzobO^d-O!Ab#Hl)V+|4 z$Hl;Um$ngK(OyPu*jddb&mva@=|(6$|5<>zq)Hj(UNr8sAD{WvN~pj>%Fbd;;_WsdU= zY=jM)q|wK!_XN>>Y}PZbmWgTtJ_7+eI#>{H{-37Q6o(T&J9Lk12;4^MkqyEIQz!$_ zM&B&qyU$~N_){8V*qn@1HBL%ifD!|332LjHdh63@OqI4_-?3Pu&$5|I9oH%wuT)Z? z2evCk#+Ajma^cXh{_WuC!RSXAY`5gbKt1NZ0SLJPwZ&zYnb;!+R=MdF1ex7}U46BddjaUsv&j-U} z#r)7tksHEKid%vXo3aTiDMbl48{mOPz!`+Pqi+!f<(pnEI$=)BJbrqcRTQV&K5^fu z+UQ2zlV=o&!*R8wam{LxYr+8YvG*~LrjND{v|LDZ=eSIMa3N6%ol?Bev7Voe-86i+ zS+dUjwXk{Ye#a^H#W~ zwpKGQ?F3o))8`)H0tR-veqxDvu{)k^m~Ep5m6(69CW4c=>zYDoSVh>=Z2HjiX~Ux} z9H_0L!;Af&BLr_R%w*3nY>8wQa@-fHfvSHfyxGI{Pgs8F0WF0pX}kva3vjTs(4}JL7rpF1*byqpKsMQ?CBQAD;_rb zhag64yWhJojz6wS;qL1Dlh0?r)4R8+F;|@*IAiq%E7_@>ye27_ z+utK;R~&SNLqH&`vr?l8yhIkRONf@;Z;;ChwCT3TBz40-5w(7O+%{%@lO}Zm_^^EhuFohsK?5Rn2s)%a$x`iTM=QJ~+W|0YXZ7UwrbWfbxY*m;aEAFFZ zY*Ro~ITUCCr=KJx8zraUziSiM-B>+(@E~{B!m|UC7{6BkNbS03$6UD9hGXmkdc>;D=sJ@`4&87W1*+oP zeorja8(?00sxiJ1d|Xk70-vD8!TUF=>j9*`*zljEvM>stI0`hz8@ zDzidOC?`|Vr0pmx`Y!m5xEf&bJ25&bR8Q-Z$SLDU%-fDuVGT4U^&G$70jAe#4dxdP zp1k0r9(;TTWkYmcWEV9HG-#bNb4hCHeFI~3AzMVy#D`b)*q-hw%s@s#Q{VHtU?Jac z#^;SLe|q%ll)S14AC5i6F|B4b2tAXwtz7g*xBToOv{VF(nG3_N;$oGZ=ixa1Q;7&J zeR^$8{a58oT1a7wbFd-FS^wRTh*U!ov4gSbnRWPXNSRaSi8XQ?`CzY6AmKktai^sC zU0eFsO*}3d-H?uUbJT@_u}DDw)5$RY?Qebf28;KKBk>Y?xjy zC6#KD^Ihk#!rf0AkL;OJWxO~UKUNBk5X-AI)99vRHEVa4k)rsyV4{njx=HTx;U)px zfJv=j{I`L2r0~40U2!pD7#NOzqSi{j`Hh;c&r3HR2+zw_Tp1XMy7Fg$)wLD|(mt$k ziY#89JTvJUpvfEIpbmu$s1#ssuHD_*e=fN~d!D^vg=-Z^@?hxUzE9H&Z@YZtKVn!$ zo+Fziy7RTt2;~!ch2X&8cj$yqZgo$UZ323ft39{i0|hdq?m5%hCH@99h(`cIhG44! zgiIRbHj=+1)Kr>wN?Kt-RFq%TjSn^)XAS`sZ~tTEQ+0w_#Q`L-NqN2GH%!VUAwlXv z-IMg5%?Pj3H?L_Q4O()DRM5>Q_bzr|SunEoF^PZpLb=Pl1n2iGH_fj`rm3I)c8ft7 zDKk;hKHw6$p;csyOdGPOUq**xit6eENx)*^vaz7D)z0i*FWFNm+;|QXgU)Z3Y z{KV#2X&R?A$NeZmUfmi9-e-(XAe>jmgOK)q1k(SFm*j>QYm37bH zfn-(7*435bLsPwJd49?jD+;Xck1Gc5w0`I`a<}-p$jvY^UDUk4d8;8kJMri4yyN>n*NsNtRq8`>bX_1!Wa=_f0fw%!K_ne zP$#lwu0<_zypYjezEuTtBOxM;v3PxX*$2a0`Czm#^-(QD7CKU;EX_A6sV*Niq90Y~ z@yyget@8D5hxwNqX?|CI<2PHN5Yu$;TS5_jcWx- zbu=2x8wtQru)=Zmrme}MLm(RZ3Drfi-ytj!yQa5Tdad5ctwR`Ck!(_p^aX$L5sSBN zR{i=`f&AVubYoDdW3d}~>-2;?6iDE3e||KPE1Cjf%6y;R<>WQFQf?GO{I+EJM=Z_t zb~H!R_N&_M#cjteB(}U1{Wvof?8f^~Tgp7~iU-fJGw|Hh$EfyL7VDnlRZ{A5@iq24 zkq6^6C}p3WS76otRh2{fW&;=DGU`4NOSD>R%r1Udp!dO6^kI#SLbuZWEm!sOq+W+= z4P7p-45_{OHy)=xf40@EZT9AlPD{JP6T6lL&-OU)q=~;!g=`Z0^0^09tricWzsN13 zSmrKWnwWDlP!m1($Us}|1m~k4b|fs11wm5Ix^W_@`t(Z=EC`f-!eq|c_|u}E!xr_R zdKTEeNLKp?ZGD~7Ue~Qolej!<*0u^S#*HS$3%`l1apNpyp0Nz8TPNJ>JoN0m#{_GD zl9m)Yi1Ew_uIV>zbwwEDP4|?hs3SI^kiQrX~P(jS9`oVpYE*(E*5>a z(jxjJCDLZ%Wt2HONsXk3o9}uCZ=fy*nff!nh&cMye91+jxwi7wFt)WkROrE|;Kk#N zvo7azSh6B~=(Y97YE%r?+&a{Pi*w&h2!{lqwJZaee?RKSm)SG;IF_%dtVPWztT{Vf zv-Y6$%15(~G=~G}@tSTWtDF2KK0lvet#SFInckXXrkjXtjBU*>sBnZ46w)B_mdxs$?T5(C2STmP-`q z6L4vc&%gU@G0$D1Q(3P9SBdU+!8T)LU;PBmsz^_fOA#FYjYZdNw~&8cs8M>m#Pz}0 zLu@KeCngS(XPa~8t*@{4o?hUsb?L0NvUPe#?0Np%^MiM#Z+y~|iLH&Ld#EfWWUE@K z;U~*0kRy(4bVkl%frmpBbh>$DzgquQ z-c$0;qp9zPrLq24jMY4QV|`L1LGE2Q@`<)laMRbbqMnH2REP&fL3og!a=VaIO%XRTT{|qPn}c9Zd0unQJ3xC;!G)CZ zxR(7k+fmbtx}xH2Xu6ndxALCWN2#)?`aaMs=HPGYVe396IX_H(OefKr2V#8$F z!Z9(s6E0PwWwp0m$RFB@HeE871DOvVULS2p zKkBV|IahO(eN}Gn0r#2p^QRrE)fwALgFjd8IdDlk)STTS=$5Jkqrqv( z^th_1XoOAOmvWcOkrXHtyE4~FcrtfU|>v>?3!j>Vl~atIp$ z?AeR`-?L_^TpLy3`WsLZC0GZ7EjW^gBUNg!CD${Eg0VEuFZ>OM5iyDzH_+BTTWVBS z5jBxn|K}6ad)g-MeX{RQ*|HXC>yO{BbE)hrw`KPsXyl1V?LR59JGCOJsfy_0@LE3~ zDS9x=CT}+$@!_(&S)gxk_Cm=Xr)C)rf13d09^3_^!jk#cp^}c9UydL7q$AS6>Md9w z_Kxg&W#fR3zT#u{4{X~e%7g8e(^3uAzORnwZkj&obBCVq3th5AG0wJ4$G_nrmDtPk zWm6!Rhl8SBF|^6me6d@C8-~tiIt`!8vCl%whU0NAUpeBRq}Jdhb(>jZ2`%F|e*}Fe zAC0n$b3#aewEw)BCZndJaeG8v+#O@gz zE$cBkCUzw4{itT!#l?_#lU?KYZ9L}r{EQ8I&C~YdlW@B)>(!nK%g_NdVeX6N(GCzn z0TY1pzJJSs1b^%70SOdBmo~j9!dTW&L-F9)*dswY!9~`p>QmYQi!HQat~@X9Jhb47 zn8F?%nAg1$6MS}YW|=j-Ydvo=rzboAY(;xF_sEe4A@QawoKAn7aS<}iPxpRKtZZAg z^gzGxB_mNFj;Ic_LF>B8uE;d_3iOY>bg7$%Bkx;j1b4)P$+X6cOCzOtaBCtR4kzRm zOs8^3yb?U8)p9q{!*5n>kN2YF0=lW9D^e^d#`dF5ym?CIsF1;zm*7(3siZM=i6v6? zh}vvr5NX;`^s{`)A8L`x+|Q0drH-4XOQdTiF0b_K9!&AXbL#rGR$cfYSH$6({X|<+ z?TV8q$U5(<&-4mKl1Q63KkAmQly!CgN@BA<+{rx>KT}NvVIu33i!cefmok4{$}v;R z|M5q!@QE&(R@8lm!UL~`F29j^IHR%_ZD2gnzmV%a9hp-nh|1katXELx6!z}#|C$GE z`0Z7>Npwqv%#i7>G$sDVEneElWQQ01^5B`4h zWIpd$;ekH0la_mf#%nYnk1Ko7m_`eX|FDemm`c^_h~q0oT7gdqC!BH#1wt64P*yg` zt5S_7Va_G=`EtJ5)i*Pazq0N>^ROq%{=|*aBMfvZGI;jP<7*+!2TI7EbI0lt(ei?) zNhDLZ0XbKMm2P!_*w9Ob(;NNR3Q_Fv)|~eJ*^;RKCAqBasj4w+;DtcQn?UFCuP|$ z!={m!Kf-kmHjsO05FPDQP=0_?bhq{HC_T7?>`&nXr4xrC=cA*Kt=t}aO6Q++4Z^&o zxxulmT-Hx!+WPXr^7JOh*e;gXwt{#?hfS8yw0GOV`IADMsK%`&y*6W#sZJP+iN5<; z=vcTSPDB>N1g^nwQAS7zgV91F!2ZC8g~PQ0p!+`d&@=BhnEX*x8dwq${j1)vv$pm^ z4FO(1uqXkWh|MJ*)-&A=3JPy68dlI7&g&ecC-ouvgtMnA28*AcNMeoo%iJNm0x#D4 zW+vBiU7gR&dO=9FF~UZY`}lQ^kF!J{QN6YqxjF1||Dud1$Y*@apLET^XLq-`TviKF zLDm?_1}>$qd@^;ym)nB!QLYWhbngR&AF$xKq}X%Xn%Z!|C(9u$w3)hU*gH2QwK)Di z)r4)_|QbH;(ui0iSS;w*xC+*HI zyr-35Ja7ZaMH<*de_M$r4=0z^45>c7U;+BWp(%5y6sJZ|n)M zm#Z-cp&TX&C885PI{_%|L^v=ixBzzpUZNz}tZ)HNh#gNo(tvYs1U5NUCitt|@MB@Q zU02^`0g(zE&I1XP3;~bqB)E=6NC?9T5rwGL0AN}W=I<4RquJ-oFv;iw1V$QJotvr5{vLjV0@jHJD~d_S&(z=v=(pGW!aj9};{dpC@N=a{J>} zk!*wlhFuN)Dt$hzjOr^yw|H?uwWk3G!91I*JD~+~lzyb;u-+`I&9kxj4F-vey1spu#i5%!$ s&i9Ad)?D>@HvjsD84K%fdS@p0C+dngq0$)Oe?TD|2P^Ks`lk&25C0ILJpcdz literal 0 HcmV?d00001 diff --git a/lib/ui/painting/image_decoder.cc b/lib/ui/painting/image_decoder.cc index 200349eb4b636..4811fb86032c8 100644 --- a/lib/ui/painting/image_decoder.cc +++ b/lib/ui/painting/image_decoder.cc @@ -133,62 +133,14 @@ static sk_sp ImageFromCompressedData( TRACE_EVENT0("flutter", __FUNCTION__); flow.Step(__FUNCTION__); - auto codec = SkCodec::MakeFromData(data); + auto decoded_image = SkImage::MakeFromEncoded(data); - if (!codec) { - return nullptr; - } - - const auto encoded_info = codec->getInfo(); - - if (encoded_info.dimensions().isEmpty()) { - return nullptr; - } - - const double desired_width = - target_width.value_or(encoded_info.dimensions().width()); - const double desired_height = - target_height.value_or(encoded_info.dimensions().height()); - - const auto scale_x = desired_width / encoded_info.dimensions().width(); - const auto scale_y = desired_height / encoded_info.dimensions().height(); - - const double scale = std::min({scale_x, scale_y, 1.0}); - - if (scale <= 0.0) { - return nullptr; - } - - // We ask the codec for one of the natively supported dimensions. This may not - // be exactly what we need, but it will also be smaller than 1:1. We will - // still have to perform a resize, but from a smaller intermediate buffer. In - // case no resize needs to be performed, ResizeRasterImage will just return - // the original image. - const auto scaled_dimensions = codec->getScaledDimensions(scale); - - if (scaled_dimensions.isEmpty()) { - return nullptr; - } - - const auto decoded_info = encoded_info.makeWH(scaled_dimensions.width(), - scaled_dimensions.height()); - - SkBitmap decoded_bitmap; - if (!decoded_bitmap.tryAllocPixels(decoded_info)) { - FML_LOG(ERROR) << "Could not perform allocation for image decoding."; - return nullptr; - } - - const auto decompression_result = codec->getPixels(decoded_bitmap.pixmap()); - if (decompression_result != SkCodec::Result::kSuccess) { - FML_LOG(ERROR) << "Could not perform image decompression. Error: " - << SkCodec::ResultToString(decompression_result); + if (!decoded_image) { return nullptr; } - decoded_bitmap.setImmutable(); - - auto decoded_image = SkImage::MakeFromBitmap(decoded_bitmap); + // Make sure to resolve all lazy images. + decoded_image = decoded_image->makeRasterImage(); if (!decoded_image) { return nullptr; diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index a81426b94f904..b107905576b2c 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -195,6 +195,53 @@ TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) { latch.Wait(); } +TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) { + auto loop = fml::ConcurrentMessageLoop::Create(); + TaskRunners runners(GetCurrentTestName(), // label + GetThreadTaskRunner(), // platform + CreateNewThread("gpu"), // gpu + CreateNewThread("ui"), // ui + CreateNewThread("io") // io + ); + + fml::AutoResetWaitableEvent latch; + + std::unique_ptr io_manager; + std::unique_ptr image_decoder; + + SkISize decoded_size = SkISize::MakeEmpty(); + auto decode_image = [&]() { + image_decoder = std::make_unique( + runners, loop->GetTaskRunner(), io_manager->GetWeakIOManager()); + + ImageDecoder::ImageDescriptor image_descriptor; + image_descriptor.data = OpenFixtureAsSkData("Horizontal.jpg"); + + ASSERT_TRUE(image_descriptor.data); + ASSERT_GE(image_descriptor.data->size(), 0u); + + ImageDecoder::ImageResult callback = [&](SkiaGPUObject image) { + ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread()); + ASSERT_TRUE(image.get()); + decoded_size = image.get()->dimensions(); + latch.Signal(); + }; + image_decoder->Decode(std::move(image_descriptor), callback); + }; + + auto setup_io_manager_and_decode = [&]() { + io_manager = std::make_unique(runners.GetIOTaskRunner()); + runners.GetUITaskRunner()->PostTask(decode_image); + }; + + runners.GetIOTaskRunner()->PostTask(setup_io_manager_and_decode); + + latch.Wait(); + + ASSERT_EQ(decoded_size.width(), 600); + ASSERT_EQ(decoded_size.height(), 200); +} + TEST_F(ImageDecoderFixtureTest, CanDecodeWithoutAGPUContext) { auto loop = fml::ConcurrentMessageLoop::Create(); TaskRunners runners(GetCurrentTestName(), // label