From 8ee58e18fd31a7d33ccab78820d61bab9ec1ec69 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 9 Mar 2023 03:48:29 +0100 Subject: [PATCH] refactor: `createThumbnailFromPath` takes `size` not `maxSize` (#37362) refactor: createThumbnailFromPath takes size not maxSize --- docs/api/native-image.md | 6 ++- docs/breaking-changes.md | 35 ++++++++++++++++++ .../api/electron_api_native_image_win.cc | 19 +++++----- spec/api-native-image-spec.ts | 24 ++++++++++++ spec/fixtures/assets/capybara.png | Bin 0 -> 5385 bytes 5 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 spec/fixtures/assets/capybara.png diff --git a/docs/api/native-image.md b/docs/api/native-image.md index 0ad349bb081a..a71812a8e9fc 100644 --- a/docs/api/native-image.md +++ b/docs/api/native-image.md @@ -119,13 +119,15 @@ Returns `NativeImage` Creates an empty `NativeImage` instance. -### `nativeImage.createThumbnailFromPath(path, maxSize)` _macOS_ _Windows_ +### `nativeImage.createThumbnailFromPath(path, size)` _macOS_ _Windows_ * `path` string - path to a file that we intend to construct a thumbnail out of. -* `maxSize` [Size](structures/size.md) - the maximum width and height (positive numbers) the thumbnail returned can be. The Windows implementation will ignore `maxSize.height` and scale the height according to `maxSize.width`. +* `size` [Size](structures/size.md) - the desired width and height (positive numbers) of the thumbnail. Returns `Promise` - fulfilled with the file's thumbnail preview image, which is a [NativeImage](native-image.md). +Note: The Windows implementation will ignore `size.height` and scale the height according to `size.width`. + ### `nativeImage.createFromPath(path)` * `path` string diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 22dc3a62f9c6..2cd7f80c6da1 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -14,6 +14,41 @@ This document uses the following convention to categorize breaking changes: ## Planned Breaking API Changes (24.0) +### API Changed: `nativeImage.createThumbnailFromPath(path, size)` + +The `maxSize` parameter has been changed to `size` to reflect that the size passed in will be the size the thumbnail created. Previously, Windows would not scale the image up if it were smaller than `maxSize`, and +macOS would always set the size to `maxSize`. Behavior is now the same across platforms. + +Updated Behavior: + +```js +// a 128x128 image. +const imagePath = path.join('path', 'to', 'capybara.png') + +// Scaling up a smaller image. +const upSize = { width: 256, height: 256 } +nativeImage.createThumbnailFromPath(imagePath, upSize).then(result => { + console.log(result.getSize()) // { width: 256, height: 256 } +}) + +// Scaling down a larger image. +const downSize = { width: 64, height: 64 } +nativeImage.createThumbnailFromPath(imagePath, downSize).then(result => { + console.log(result.getSize()) // { width: 64, height: 64 } +}) +``` + +Previous Behavior (on Windows): + +```js +// a 128x128 image +const imagePath = path.join('path', 'to', 'capybara.png') +const size = { width: 256, height: 256 } +nativeImage.createThumbnailFromPath(imagePath, size).then(result => { + console.log(result.getSize()) // { width: 128, height: 128 } +}) +``` + ### Deprecated: `BrowserWindow.setTrafficLightPosition(position)` `BrowserWindow.setTrafficLightPosition(position)` has been deprecated, the diff --git a/shell/common/api/electron_api_native_image_win.cc b/shell/common/api/electron_api_native_image_win.cc index 1cc998d3de53..baa924501543 100644 --- a/shell/common/api/electron_api_native_image_win.cc +++ b/shell/common/api/electron_api_native_image_win.cc @@ -43,7 +43,7 @@ v8::Local NativeImage::CreateThumbnailFromPath( if (FAILED(hr)) { promise.RejectWithErrorMessage( - "failed to create IShellItem from the given path"); + "Failed to create IShellItem from the given path"); return handle; } @@ -53,21 +53,20 @@ v8::Local NativeImage::CreateThumbnailFromPath( IID_PPV_ARGS(&pThumbnailCache)); if (FAILED(hr)) { promise.RejectWithErrorMessage( - "failed to acquire local thumbnail cache reference"); + "Failed to acquire local thumbnail cache reference"); return handle; } // Populate the IShellBitmap Microsoft::WRL::ComPtr pThumbnail; - WTS_CACHEFLAGS flags; - WTS_THUMBNAILID thumbId; - hr = pThumbnailCache->GetThumbnail(pItem.Get(), size.width(), - WTS_FLAGS::WTS_NONE, &pThumbnail, &flags, - &thumbId); + hr = pThumbnailCache->GetThumbnail( + pItem.Get(), size.width(), + WTS_FLAGS::WTS_SCALETOREQUESTEDSIZE | WTS_FLAGS::WTS_SCALEUP, &pThumbnail, + NULL, NULL); if (FAILED(hr)) { promise.RejectWithErrorMessage( - "failed to get thumbnail from local thumbnail cache reference"); + "Failed to get thumbnail from local thumbnail cache reference"); return handle; } @@ -75,14 +74,14 @@ v8::Local NativeImage::CreateThumbnailFromPath( HBITMAP hBitmap = NULL; hr = pThumbnail->GetSharedBitmap(&hBitmap); if (FAILED(hr)) { - promise.RejectWithErrorMessage("failed to extract bitmap from thumbnail"); + promise.RejectWithErrorMessage("Failed to extract bitmap from thumbnail"); return handle; } // convert HBITMAP to gfx::Image BITMAP bitmap; if (!GetObject(hBitmap, sizeof(bitmap), &bitmap)) { - promise.RejectWithErrorMessage("could not convert HBITMAP to BITMAP"); + promise.RejectWithErrorMessage("Could not convert HBITMAP to BITMAP"); return handle; } diff --git a/spec/api-native-image-spec.ts b/spec/api-native-image-spec.ts index d86f31db00b0..51f26679bf56 100644 --- a/spec/api-native-image-spec.ts +++ b/spec/api-native-image-spec.ts @@ -447,6 +447,30 @@ describe('nativeImage module', () => { const result = await nativeImage.createThumbnailFromPath(goodPath, goodSize); expect(result.isEmpty()).to.equal(false); }); + + it('returns the correct size if larger than the initial image', async () => { + // capybara.png is a 128x128 image. + const imgPath = path.join(fixturesPath, 'assets', 'capybara.png'); + const size = { width: 256, height: 256 }; + const result = await nativeImage.createThumbnailFromPath(imgPath, size); + expect(result.getSize()).to.deep.equal(size); + }); + + it('returns the correct size if is the same as the initial image', async () => { + // capybara.png is a 128x128 image. + const imgPath = path.join(fixturesPath, 'assets', 'capybara.png'); + const size = { width: 128, height: 128 }; + const result = await nativeImage.createThumbnailFromPath(imgPath, size); + expect(result.getSize()).to.deep.equal(size); + }); + + it('returns the correct size if smaller than the initial image', async () => { + // capybara.png is a 128x128 image. + const imgPath = path.join(fixturesPath, 'assets', 'capybara.png'); + const maxSize = { width: 64, height: 64 }; + const result = await nativeImage.createThumbnailFromPath(imgPath, maxSize); + expect(result.getSize()).to.deep.equal(maxSize); + }); }); describe('addRepresentation()', () => { diff --git a/spec/fixtures/assets/capybara.png b/spec/fixtures/assets/capybara.png new file mode 100644 index 0000000000000000000000000000000000000000..a61e43f7362b13b53e2f08cbfaf05df56d74c97b GIT binary patch literal 5385 zcma)AWmuHm)_xd9Qegmvp=1OJkwy?07`nSrLK=n|VkqfuQIJk)X{37)MUW0D=@Jwq zq#5ww`=0MT-}!g;bM0rko)P0^q-N0ARV60Dzl|^Pgxg z=$~j{F7AKyKftHj?8s}9CrBLww1KLMhy}`#%ghpGZpG#0===u&hWr$IZvbd5z$7^L9d;d2u?qG5t;Qe>^f) zZWgXcXEYM!1pC8lW{z@4i!n0(N%YV9yG}IH`ahYR-2UCxb%Wf0BHX-OJly|qUmq3y z;}ubNMOs~F{=<*p75xkOU%r3m5#|1q{Qnj6cc*`OuUkbBiE{sQZ3rT2lVyAWpa3b# zNa}du>=+ZHb-pxToe1+M;;%^(+~dvjlDtjqbw^*8U>M;$H>t-%X}53waMLbj%H3!E z&D4T*McJK^2EEG48|Lf5ualu{1|JN+1_uS^_DbgmQkH=31xc7B_6=mZ9bX=YDsH$B zsU)O{viQ`#->zM}xaj<~8?akDjF1^q1m6dd<3t6@NHD_4UJ@w$ujWe!g6f5lH)o{2 zsMnI;vkIW!Xfw|@#>TYtQMUR4w$kwwrdJ`gtTcER;(=m&d`qpmr1Q0Q-QmQSl7QI! zr%(#h)gJTuJ%(9Zf-?_B4<@E$CXvBrO`nAd|F)gb9WtXUZ3rZDK}(yzqt)&UJ4&Oe zEfNlDXiD*FmKciC<;W3^%i+7~U;1nbE@O;d&2h%#J+GdnT5Baaj~_KoZ{0$KR*({< zP`Sv%?t#5_KVI;Aa(jx!!2J3ToWoeK?#*-q! z8ne*uumPUKR>3ZywTx;4B|83c%-v7x)O7W$jE~NL=vuG$?o~2D9^b0|rvDMyQuG`D z+qR8ua(6k>-6`x?NA^xfZ#F`xTa2hP0kAghxn~q5O&gn|{ zV!X>@{wJM>883&G`%{LnDXCihGFei&%F>t@2}nW1r$0eA$CWjG2Av3&zc0*hX*(`#$EeinZKiCPxH(K!^W3% zegrKp21zR5FUKfDUzHcEYwd&^5Bmqa6gRo*!4D##kWkt6;OuNnotZW>`0klcO+Us+ zLHvj&fiesHcnvBkwiavsYGuLjYOdTxlI$Q)nEB7}b+hSxD`HpsM$n4Lmew zI|UjrHOjBfJazGkxAdP!$vVw5%ZM7)?ZrKYFk~qj2eY2>XOw>cd(yGh_MDvrl#S-2 z>?dFMb@RvdCJ+xs>9feBi;d+dk9d^( zIQ+dqNY~ko(5zLFvb1e_{Xr!qb8D|+TBt=&B-o(YH0Y8dANfXmRlRGIIMJjFP-}cP zuLj)PzGO|`tjd|tw1}b9If@;fL4RrrrQMB#7T$g68h5rh6#Ovu>(0ZUXv>yuk_@>m zCjn;C+GLOddk(D}BE*?@wHcJvAFYc5k8@ z(A>N*q|}Rz`AS60XDghGKqKA)`lRe@{gJkR%Q`yGMnp2fd;5*Ba(dfgxvqTJI+xxi zcJ-*|l4>&~aRh_IMG^HAnj!tzSS(P-%^m@ZIFysfB{a*}XS-~EtE?m*4Pi*h6$Ekk zpW;6@LDTv;PZby@`MPCGuDX8xbT&>Uc4&7d?Aqe)YWh=fYnCHjP=uI2upzG^I?-j6 zY!8z#V!uV^SLa-eeRYwQ1I}`rGZ6=bgW6xQgg_?;nsY^~rC%c#9k*u_&+8nVz!n_K@vh)F36+&)Q7LoZguCS-+!)e_9>9<9v{W`IZ#2pQs~kQ&Nz@nY$yC2<-L-xgtW(7g z_H`>Q3QYDz%31cciqpu6K?1V6;yRPLGmhqmCMz!Qk*g*tG2jv7&hO(YdoPz&Q=;-%%f@K zbdy#n@AkcXig@7gQ2^eqImecQt`hNMI}FhO)v)|Zaf%h9!Am4++<~6N8W@({tSJ^S zoj2lur?)^o_tQ60$mXMgY>Dxh)|3Sw8;*K*@c?ht`n(avvJX^-c`wNIW~_`(m|lKP zt2v%uudR&JtDi3K*6nFDWG;8-bwwtC^`J)`Ldj*&x}PBlC_}{8{R(i=>_k1JYD;uD zBsK9?M?rg4&O6x+0-lyHN!bT(zrYhM$G@?kk8)OIV;%JfAKL3KHgJ|b;;W=$#GF~I z_^H+=pE=p{-n0~%$T1V;5licix+TF70vus~8u{KnNH4C|^8IFwpC^>LM;(EA^;*UFAOR(-+OfpD7RdBAz@Ajd3ll{^WQ|FJt&=L84}q zyr}`re)Ww!7${OurSzoo(*+&8eAew<_Gl1I2DS2sEpGW5-e2B5(-U>uUHVj<4}3aP}z5Z zokp)M3%o-w1CaqJdVr8bg?-}6SvPrqV?x#{=Zv`i+|LuaZxFWM$V^zSp=3&zpCwD& zZ+-eWC7e*O+HOBvLRxouf|9$F3RM&PkqnhK4R;3(u+;%7MX~qhisvlD89bLRJxsMr zb(o5`=jn2gFc`59T#&pa#73g~QPG3_zQg#>Gl9I;M9QSQAi`>aEL~6K6DwvfDv4%Q z%*aCm7!9L>gi$B`0=o;Ru{qfi(fob8O&3~NUB>i+FaK(^0Y`$MB%9EFJHlj*2HrA7 zCUL0Qwi8Ts_X8KRuxxDA$XBKA6_bjeBl5@T_i`(&_D;ibrMogdkFgMZV9xfRJEIn= zW@*(Ymq4QrC<^hphDi#{pz3{%DeD@JcGgwQfwnefERb4Uk*Niztr#khP`!g`6-(e| z$p|=H(zDBuqUuGz>r*mJcU!rMIxsr%R{b^ohg4<($;>qSC1^K75*Xdh`)I{rDjfpr zIulwQLB(h*RP6GDAzJD0Y{w%;z)+I4Z5T69@vBDEs>8qY#LW{`$pI- z?Xa<$lsh+u|H3o6c%g2?Ue?`x$H7udXuMuT+LtBi!P8QG`_nH!OMSj)o<9r*cle&h z=WFCJh>NG`DR@&8B4Xle))T4PYo_&)m;k~`f!w0Idn<0dtT#66h1ONDX~I2U?vzbA zdChZllr3w=I8|;?+OR*m7+O{?Vfzj}!Ln$HCk){cY~Nqd{>`{II}lI*j?h>&G`Y!A zuaTgtTjQn1r|HtXkA_Uy!!IYm)x6RUjD$X_Vz(Wt3~NQ%_#gSnSMkajknc-rPq28@ zea0NBSy!ika6y-fssO1n1!WbAj&vJc(pmHN?jnb3rdzWwO%M9jcL(DY0>EdzQC3+= zl`wSzY*1^hD#j9w$RAQ^!#mDsPB+$1{rHIqiwVNzfz1?KTj_rB=?$Tl0vRP%zI$ev zlraI8_!L&*=*72EpMm{3ZvxdiXu??bmP(rTVVfuuMm)e0G`(G1p-TyT}H}Id9!Y9pSNYVos#=Z z(f{uLz}o+!M@wyt0vOLN0RwAMf*`+l)6KsC%FNWQCf|-x=?istZ$&BjoB-;WJ=ydE z)<8S9WH8UoMRud}sZa5{2l7x0ocXPd&hQ>#>*Qn9C$422P5q9c&&n=5Hib{l-YhGs z14>m@^2MsChrD3S15Tvf`yD`&Y!b{f@+;&$Ltnkuty$laWM_m$!KSSlCy{oFtnzJW z4i=mX$~6D1`i$#{0wed_1jZ9}=tg3Xd7ss`l(&=Q-XVK4#?nkv#3mrUsrt~_AXHXj zjKcIwsqRQI2MrE%>cO%-j-#uoXzL(qWybSe_Lav#al6VC384$+9@kBc7qp zIfr10Z@m2$MP)%L0)f-Oh$A?#oI@ZGk_rVp=sKIQy*IwvT={_+MH48HuYO{EZpB?N zel!d_!UyI4?s>ou!GbZtnvGm`kYY^jqq8k?ZBLQ8Ucy&rYYs?qqE7__7~K!wPfo!rqQ zjc__b4e~*MAV849@JGyJ>y%)ko!!J&w24VdzVGC*VerX;w$Cs2_BQXmQi{ngi!apZ zg$PQdl8+o7pEbO1hJqM>%$%M)K2OKB$&jhH)tFcY(U%#u`?tT%viO4amjt+ZYp^te zh-lUze|!l3yvJ9^nz^(qj;!-6LC5r_{CwoXupO;Y4fh7mb7HG-GSOtb>k2npj3K$q zw;{hz1B8v{l8|qZD}EI%Mj@IkFTU}5zFf;lN=l-ihG7;2xd%mxozb&{h(u&dM;?>HBKZRmnCKff%WvA5loQ@6=xd`q~bXcnR~iE~33b8oHNw>KiU zFHtqO#aCMF!%d}5Xnhxl$6#TcL5=irQ|1q}i(dWjbEE$nGUW!`+xyKX%?(~T+=u)b OEGo*X$y7?22LB5(h0s_4 literal 0 HcmV?d00001