[PATCH v6 0/4] Partial IShellItemImageFactory implementation (#52673)
Jinoh Kang (4): include: Add definition for IShellItemImageFactory. shell32/tests: Add tests for IShellItemImageFactory. shell32: Add stub for IShellItemImageFactory. shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail). dlls/shell32/Makefile.in | 2 +- dlls/shell32/shellitem.c | 241 +++++++++++++++++++++++++++++++++ dlls/shell32/tests/shlfolder.c | 51 +++++++ include/shobjidl.idl | 30 ++++ 4 files changed, 323 insertions(+), 1 deletion(-) -- 2.34.1
Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v1 -> v2: no changes v2 -> v3: use lowercase letters for uuid(<hex>) v3 -> v4: no changes v4 -> v5: s/typedef DWORD SIIGBF;/typedef int SIIGBF;/ (thanks nsivov) v5 -> v6: no changes include/shobjidl.idl | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/include/shobjidl.idl b/include/shobjidl.idl index c1a36ab5e93..a23f3cfdd36 100644 --- a/include/shobjidl.idl +++ b/include/shobjidl.idl @@ -471,6 +471,36 @@ interface IShellItem2 : IShellItem [out] BOOL *pf); } +/***************************************************************************** + * IShellItemImageFactory interface + */ +[ + object, + uuid(bcc18b79-ba16-442f-80c4-8a59c30c463b), + pointer_default(unique) +] +interface IShellItemImageFactory : IUnknown +{ + [v1_enum] enum _SIIGBF { + SIIGBF_RESIZETOFIT = 0x00000000, + SIIGBF_BIGGERSIZEOK = 0x00000001, + SIIGBF_MEMORYONLY = 0x00000002, + SIIGBF_ICONONLY = 0x00000004, + SIIGBF_THUMBNAILONLY = 0x00000008, + SIIGBF_INCACHEONLY = 0x00000010, + SIIGBF_CROPTOSQUARE = 0x00000020, + SIIGBF_WIDETHUMBNAILS = 0x00000040, + SIIGBF_ICONBACKGROUND = 0x00000080, + SIIGBF_SCALEUP = 0x00000100, + }; + typedef int SIIGBF; + + HRESULT GetImage( + [in] SIZE size, + [in] SIIGBF flags, + [out] HBITMAP *phbm); +} + typedef [v1_enum] enum tagNWMF { NWMF_UNLOADING = 0x00000001, NWMF_USERINITED = 0x00000002, -- 2.34.1
Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v1 -> v2: mark test broken on Windows 7 v2 -> v3: remove erroneous todo_wine v3 -> v4: no changes v4 -> v5: test if IShellItemImageFactory::GetImage loads windowscodecs.dll v5 -> v6: no changes dlls/shell32/tests/shlfolder.c | 54 ++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index f99045c33d5..ba2e16c138a 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -4433,6 +4433,59 @@ static void test_contextmenu(IContextMenu *menu, BOOL background) DestroyMenu(hmenu); } +static void test_IShellItemImageFactory(void) +{ + HRESULT ret; + IShellItem *shellitem; + LPITEMIDLIST pidl_desktop = NULL; + + if (!pSHCreateShellItem) + { + win_skip("SHCreateShellItem isn't available\n"); + return; + } + + ret = SHGetSpecialFolderLocation(NULL, CSIDL_DESKTOP, &pidl_desktop); + ok(ret == S_OK, "Got 0x%08lx\n", ret); + if (SUCCEEDED(ret)) + { + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem); + ok(SUCCEEDED(ret), "SHCreateShellItem returned %lx\n", ret); + if (SUCCEEDED(ret)) + { + IShellItemImageFactory *siif; + ret = IShellItem_QueryInterface(shellitem, &IID_IShellItemImageFactory, (void **)&siif); + todo_wine + ok(ret == S_OK, "QueryInterface returned %lx\n", ret); + if (SUCCEEDED(ret)) + { + HBITMAP hbm = NULL; + SIZE size = {32, 32}; + + ok(!GetModuleHandleA("windowscodecs.dll"), "WIC should not have already been loaded\n"); + + ret = IShellItemImageFactory_GetImage(siif, size, SIIGBF_BIGGERSIZEOK, &hbm); + todo_wine + ok(ret == S_OK || broken(ret == E_PENDING /* win7 */), "GetImage returned %lx\n", ret); + ok(FAILED(ret) == !hbm, "result = %lx but bitmap = %p\n", ret, hbm); + + todo_wine + ok(!!GetModuleHandleA("windowscodecs.dll"), "WIC should have been loaded\n"); + + if (SUCCEEDED(ret) && hbm) + { + DWORD objtype = GetObjectType(hbm); + ok(objtype == OBJ_BITMAP, "Expected type OBJ_BITMAP, got %lu\n", objtype); + DeleteObject(hbm); + } + IShellItemImageFactory_Release(siif); + } + IShellItem_Release(shellitem); + } + ILFree(pidl_desktop); + } +} + static void test_GetUIObject(void) { IShellFolder *psf_desktop; @@ -5385,6 +5438,7 @@ START_TEST(shlfolder) test_SHCreateShellItemArray(); test_ShellItemArrayEnumItems(); test_desktop_IPersist(); + test_IShellItemImageFactory(); test_GetUIObject(); test_CreateViewObject_contextmenu(); test_SHSimpleIDListFromPath(); -- 2.34.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=113347 Your paranoid android. === w1064v1809 (32 bit report) === shell32: shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064 (32 bit report) === shell32: shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064v1809 (64 bit report) === shell32: shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064 (64 bit report) === shell32: shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064_2qxl (64 bit report) === shell32: shlfolder.c:4964: Test failed: MKDIR: expected notification type 8, got: 40000 shlfolder.c:4971: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3) === w10pro64_ar (64 bit report) === shell32: shlfolder.c:4964: Test failed: RMDIR: expected notification type 10, got: 40000 shlfolder.c:4971: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10) === w10pro64_he (64 bit report) === shell32: shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w10pro64_ja (64 bit report) === shell32: shlfolder.c:4964: Test failed: CREATE: expected notification type 2, got: 40000 shlfolder.c:4971: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 2) shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w10pro64_zh_CN (64 bit report) === shell32: shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3)
On 4/25/22 18:21, Jinoh Kang wrote:
+ if (!pSHCreateShellItem) + { + win_skip("SHCreateShellItem isn't available\n"); + return; + } You probably don't need to check that for new tests. I don't see skips from it on win7+ test results.
+ ret = SHGetSpecialFolderLocation(NULL, CSIDL_DESKTOP, &pidl_desktop); + ok(ret == S_OK, "Got 0x%08lx\n", ret); + if (SUCCEEDED(ret)) + { + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem); + ok(SUCCEEDED(ret), "SHCreateShellItem returned %lx\n", ret); + if (SUCCEEDED(ret)) + { + IShellItemImageFactory *siif; + ret = IShellItem_QueryInterface(shellitem, &IID_IShellItemImageFactory, (void **)&siif); + todo_wine + ok(ret == S_OK, "QueryInterface returned %lx\n", ret); + if (SUCCEEDED(ret)) + { + HBITMAP hbm = NULL; + SIZE size = {32, 32}; + + ok(!GetModuleHandleA("windowscodecs.dll"), "WIC should not have already been loaded\n"); + + ret = IShellItemImageFactory_GetImage(siif, size, SIIGBF_BIGGERSIZEOK, &hbm); + todo_wine + ok(ret == S_OK || broken(ret == E_PENDING /* win7 */), "GetImage returned %lx\n", ret); + ok(FAILED(ret) == !hbm, "result = %lx but bitmap = %p\n", ret, hbm); What's going on with win7? Have you tried different arguments or running on some temp folder, instead of the desktop one? I would be great to have it pass. Maybe size/flags problem? + + todo_wine + ok(!!GetModuleHandleA("windowscodecs.dll"), "WIC should have been loaded\n"); I don't think windowscodecs check needs to be here. I understand you used it to verify that windowscodecs is loaded, but now that you know it, you don't have to put as a requirement. Also this has a potential to break, because of the test order. + + if (SUCCEEDED(ret) && hbm) + { + DWORD objtype = GetObjectType(hbm); + ok(objtype == OBJ_BITMAP, "Expected type OBJ_BITMAP, got %lu\n", objtype); + DeleteObject(hbm); + }
I don't think this is necessary. Method explicitly claims to return HBITMAP, and we can assume it is such handle if it's not 0.
+ IShellItemImageFactory_Release(siif); + } + IShellItem_Release(shellitem); + } + ILFree(pidl_desktop); + } I suggest removing some of that error handling, leaving only what's needed because of todo.
On 4/26/22 21:34, Nikolay Sivov wrote:
On 4/25/22 18:21, Jinoh Kang wrote:
+ if (!pSHCreateShellItem) + { + win_skip("SHCreateShellItem isn't available\n"); + return; + } You probably don't need to check that for new tests. I don't see skips from it on win7+ test results.
ACK.
+ ret = SHGetSpecialFolderLocation(NULL, CSIDL_DESKTOP, &pidl_desktop); + ok(ret == S_OK, "Got 0x%08lx\n", ret); + if (SUCCEEDED(ret)) + { + ret = pSHCreateShellItem(NULL, NULL, pidl_desktop, &shellitem); + ok(SUCCEEDED(ret), "SHCreateShellItem returned %lx\n", ret); + if (SUCCEEDED(ret)) + { + IShellItemImageFactory *siif; + ret = IShellItem_QueryInterface(shellitem, &IID_IShellItemImageFactory, (void **)&siif); + todo_wine + ok(ret == S_OK, "QueryInterface returned %lx\n", ret); + if (SUCCEEDED(ret)) + { + HBITMAP hbm = NULL; + SIZE size = {32, 32}; + + ok(!GetModuleHandleA("windowscodecs.dll"), "WIC should not have already been loaded\n"); + + ret = IShellItemImageFactory_GetImage(siif, size, SIIGBF_BIGGERSIZEOK, &hbm); + todo_wine + ok(ret == S_OK || broken(ret == E_PENDING /* win7 */), "GetImage returned %lx\n", ret); + ok(FAILED(ret) == !hbm, "result = %lx but bitmap = %p\n", ret, hbm); What's going on with win7? Have you tried different arguments or running on some temp folder, instead of the desktop one? I would be great to have it pass. Maybe size/flags problem?
There is nothing wrong with the parameters. In win7, E_PENDING appears intermittently but not always. IShellItemImageFactory::GetImage isn't supposed to return E_PENDING anyway, so it's probably a win7 bug. The documentation for IExtractIcon::GetIconLocation does mention that E_PENDING is returned when the GIL_ASYNC flag is specified and it is expected that loading the icon will take a long time (presumably not cached) [1]; howver, there isn't an equivalent flag for IShellItemImageFactory::GetImage. I'll try, but I'm highly skeptical that any other IDL would change the situation. I'll have to run the same test several times to assure that it doesn't happen in win7; even then, the bug might pop up some time later to screw up future test sessions. After all, E_PENDING does not indicate the caller's fault of any kind. If it indeed returned that, we could safely assume that the implementation itself was broken without worrying about anything else.
+ + todo_wine + ok(!!GetModuleHandleA("windowscodecs.dll"), "WIC should have been loaded\n"); I don't think windowscodecs check needs to be here. I understand you used it to verify that windowscodecs is loaded, but now that you know it, you don't have to put as a requirement. Also this has a potential to break, because of the test order.
ACK.
+ + if (SUCCEEDED(ret) && hbm) + { + DWORD objtype = GetObjectType(hbm); + ok(objtype == OBJ_BITMAP, "Expected type OBJ_BITMAP, got %lu\n", objtype); + DeleteObject(hbm); + }
I don't think this is necessary. Method explicitly claims to return HBITMAP, and we can assume it is such handle if it's not 0.
Ok, I'll replace them with DIB pixel format check.
+ IShellItemImageFactory_Release(siif); + } + IShellItem_Release(shellitem); + } + ILFree(pidl_desktop); + } I suggest removing some of that error handling, leaving only what's needed because of todo.
ACK. [1]: https://docs.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_cor... -- Sincerely, Jinoh Kang
Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v1 -> v2: no changes v2 -> v3: change iface lpVtbl initialization order to match struct order v3 -> v4: no changes v4 -> v5: %lu -> %d (SIIGBF) v5 -> v6: no changes dlls/shell32/shellitem.c | 50 ++++++++++++++++++++++++++++++++++ dlls/shell32/tests/shlfolder.c | 1 - 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c index 0a3a76cbd6a..0381fa67f0d 100644 --- a/dlls/shell32/shellitem.c +++ b/dlls/shell32/shellitem.c @@ -39,6 +39,7 @@ typedef struct _ShellItem { LONG ref; LPITEMIDLIST pidl; IPersistIDList IPersistIDList_iface; + IShellItemImageFactory IShellItemImageFactory_iface; } ShellItem; typedef struct _CustomDestinationList { @@ -56,6 +57,11 @@ static inline ShellItem *impl_from_IPersistIDList( IPersistIDList *iface ) return CONTAINING_RECORD(iface, ShellItem, IPersistIDList_iface); } +static inline ShellItem *impl_from_IShellItemImageFactory( IShellItemImageFactory *iface ) +{ + return CONTAINING_RECORD(iface, ShellItem, IShellItemImageFactory_iface); +} + static inline CustomDestinationList *impl_from_ICustomDestinationList( ICustomDestinationList *iface ) { return CONTAINING_RECORD(iface, CustomDestinationList, ICustomDestinationList_iface); @@ -79,6 +85,10 @@ static HRESULT WINAPI ShellItem_QueryInterface(IShellItem2 *iface, REFIID riid, { *ppv = &This->IPersistIDList_iface; } + else if (IsEqualIID(&IID_IShellItemImageFactory, riid)) + { + *ppv = &This->IShellItemImageFactory_iface; + } else { FIXME("not implemented for %s\n", shdebugstr_guid(riid)); *ppv = NULL; @@ -536,6 +546,45 @@ static const IPersistIDListVtbl ShellItem_IPersistIDList_Vtbl = { ShellItem_IPersistIDList_GetIDList }; +static HRESULT WINAPI ShellItem_IShellItemImageFactory_QueryInterface(IShellItemImageFactory *iface, + REFIID riid, void **ppv) +{ + ShellItem *This = impl_from_IShellItemImageFactory(iface); + return IShellItem2_QueryInterface(&This->IShellItem2_iface, riid, ppv); +} + +static ULONG WINAPI ShellItem_IShellItemImageFactory_AddRef(IShellItemImageFactory *iface) +{ + ShellItem *This = impl_from_IShellItemImageFactory(iface); + return IShellItem2_AddRef(&This->IShellItem2_iface); +} + +static ULONG WINAPI ShellItem_IShellItemImageFactory_Release(IShellItemImageFactory *iface) +{ + ShellItem *This = impl_from_IShellItemImageFactory(iface); + return IShellItem2_Release(&This->IShellItem2_iface); +} + +static HRESULT WINAPI ShellItem_IShellItemImageFactory_GetImage(IShellItemImageFactory *iface, + SIZE size, SIIGBF flags, HBITMAP *phbm) +{ + ShellItem *This = impl_from_IShellItemImageFactory(iface); + static int once; + + if (!once++) + FIXME("%p ({%lu, %lu} %d %p): stub\n", This, size.cx, size.cy, flags, phbm); + + *phbm = NULL; + return E_NOTIMPL; +} + +static const IShellItemImageFactoryVtbl ShellItem_IShellItemImageFactory_Vtbl = { + ShellItem_IShellItemImageFactory_QueryInterface, + ShellItem_IShellItemImageFactory_AddRef, + ShellItem_IShellItemImageFactory_Release, + ShellItem_IShellItemImageFactory_GetImage, +}; + HRESULT WINAPI IShellItem_Constructor(IUnknown *pUnkOuter, REFIID riid, void **ppv) { @@ -553,6 +602,7 @@ HRESULT WINAPI IShellItem_Constructor(IUnknown *pUnkOuter, REFIID riid, void **p This->ref = 1; This->pidl = NULL; This->IPersistIDList_iface.lpVtbl = &ShellItem_IPersistIDList_Vtbl; + This->IShellItemImageFactory_iface.lpVtbl = &ShellItem_IShellItemImageFactory_Vtbl; ret = IShellItem2_QueryInterface(&This->IShellItem2_iface, riid, ppv); IShellItem2_Release(&This->IShellItem2_iface); diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index ba2e16c138a..78f199eaafc 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -4455,7 +4455,6 @@ static void test_IShellItemImageFactory(void) { IShellItemImageFactory *siif; ret = IShellItem_QueryInterface(shellitem, &IID_IShellItemImageFactory, (void **)&siif); - todo_wine ok(ret == S_OK, "QueryInterface returned %lx\n", ret); if (SUCCEEDED(ret)) { -- 2.34.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=113348 Your paranoid android. === w1064v1507 (32 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 8) === w1064v1809 (32 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064 (32 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064v1809 (64 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064 (64 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064_2qxl (64 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w10pro64_ar (64 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w10pro64_he (64 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10) shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3) === w10pro64_ja (64 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w10pro64_zh_CN (64 bit report) === shell32: shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3)
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52673 Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v1 -> v2: no changes v3 -> v4: - Remove patch "shell32: Factor out ShellItem_get_uiobject." - Use IShellItem2_BindToHandler instead of factoring out ShellItem_get_uiobject - Use CreateCompatibleDC(NULL) instead of GetDC(NULL) - Remove new dependency on win32u, use GetIconInfo instead - Remove new dependency on windowcodecs, use gdiplus instead v4 -> v5: revert to using windowscodecs v5 -> v6: always set [out] parameter of IShellItemImageFactory::GetImage dlls/shell32/Makefile.in | 2 +- dlls/shell32/shellitem.c | 195 ++++++++++++++++++++++++++++++++- dlls/shell32/tests/shlfolder.c | 2 - 3 files changed, 194 insertions(+), 5 deletions(-) diff --git a/dlls/shell32/Makefile.in b/dlls/shell32/Makefile.in index eeb6cd63d60..0cfaac8e9cc 100644 --- a/dlls/shell32/Makefile.in +++ b/dlls/shell32/Makefile.in @@ -2,7 +2,7 @@ EXTRADEFS = -D_SHELL32_ MODULE = shell32.dll IMPORTLIB = shell32 IMPORTS = uuid shlwapi user32 gdi32 advapi32 -DELAYIMPORTS = ole32 oleaut32 shdocvw version comctl32 gdiplus +DELAYIMPORTS = ole32 oleaut32 shdocvw version comctl32 gdiplus windowscodecs C_SRCS = \ appbar.c \ diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c index 0381fa67f0d..7ce8669fdfb 100644 --- a/dlls/shell32/shellitem.c +++ b/dlls/shell32/shellitem.c @@ -31,9 +31,12 @@ #include "pidl.h" #include "shell32_main.h" #include "debughlp.h" +#include "wincodec.h" WINE_DEFAULT_DEBUG_CHANNEL(shell); +HRESULT WINAPI WICCreateImagingFactory_Proxy(UINT, IWICImagingFactory**); + typedef struct _ShellItem { IShellItem2 IShellItem2_iface; LONG ref; @@ -565,17 +568,205 @@ static ULONG WINAPI ShellItem_IShellItemImageFactory_Release(IShellItemImageFact return IShellItem2_Release(&This->IShellItem2_iface); } +static HRESULT ShellItem_get_icons(ShellItem *This, SIZE size, HICON *big_icon, HICON *small_icon) +{ + HRESULT hr; + IBindCtx *pbc; + IExtractIconW *ei; + WCHAR icon_file[MAX_PATH]; + INT source_index; + UINT gil_in_flags = 0, gil_out_flags; + INT iconsize; + + iconsize = min(size.cx, size.cy); + if (iconsize <= 0 || iconsize > 0x7fff) + iconsize = 0x7fff; + + hr = CreateBindCtx(0, &pbc); + if (FAILED(hr)) goto done; + + hr = IShellItem2_BindToHandler(&This->IShellItem2_iface, pbc, &BHID_SFUIObject, + &IID_IExtractIconW, (void **)&ei); + IBindCtx_Release(pbc); + if (FAILED(hr)) goto done; + + hr = IExtractIconW_GetIconLocation(ei, gil_in_flags, icon_file, MAX_PATH, &source_index, &gil_out_flags); + if (FAILED(hr)) goto free_ei; + + if (!(gil_out_flags & GIL_NOTFILENAME)) + { + UINT ei_res; + + if (source_index == -1) + source_index = 0; /* special case for some control panel applications */ + + FIXME("%s %d\n", debugstr_w(icon_file), source_index); + ei_res = ExtractIconExW(icon_file, source_index, big_icon, small_icon, 1); + if (!ei_res || ei_res == (UINT)-1) + { + WARN("Failed to extract icon.\n"); + hr = E_FAIL; + } + } + else + { + hr = IExtractIconW_Extract(ei, icon_file, source_index, big_icon, small_icon, MAKELONG(iconsize, iconsize)); + } + +free_ei: + IExtractIconW_Release(ei); +done: + return hr; +} + +static HICON choose_best_icon(HICON *icons, UINT count, SIZE size_limit, SIZE *out_size) +{ + HICON best_icon = NULL; + SIZE best_size = {0, 0}; + UINT i; + + for (i = 0; i < count; i++) + { + ICONINFO iinfo; + BITMAP bm; + SIZE size; + BOOL is_color, ret; + + if (!icons[i] || !GetIconInfo(icons[i], &iinfo)) continue; + + is_color = iinfo.hbmColor != NULL; + ret = GetObjectW(is_color ? iinfo.hbmColor : iinfo.hbmMask, sizeof(bm), &bm); + DeleteObject(iinfo.hbmColor); + DeleteObject(iinfo.hbmMask); + if (!ret) continue; + + size.cx = bm.bmWidth; + size.cy = is_color ? abs(bm.bmHeight) : abs(bm.bmHeight) / 2; + + if (!best_icon || (best_size.cx < size.cx && size.cx <= size_limit.cx && + best_size.cy < size.cy && size.cy <= size_limit.cy)) + { + best_icon = icons[i]; + best_size = size; + } + } + + *out_size = best_size; + return best_icon; +} + +static HRESULT ShellItem_get_icon_bitmap(ShellItem *This, IWICImagingFactory *imgfactory, + SIZE size, SIIGBF flags, IWICBitmap **bitmap) +{ + HRESULT hr; + HICON icons[2] = { NULL, NULL }, best_icon; + SIZE best_icon_size; + UINT i; + + *bitmap = NULL; + + hr = ShellItem_get_icons(This, size, &icons[0], &icons[1]); + if (FAILED(hr)) return hr; + + best_icon = choose_best_icon(icons, ARRAY_SIZE(icons), size, &best_icon_size); + for (i = 0; i < ARRAY_SIZE(icons); i++) + if (icons[i] && icons[i] != best_icon) DeleteObject(icons[i]); + + if (!best_icon) return E_FAIL; + + hr = IWICImagingFactory_CreateBitmapFromHICON(imgfactory, best_icon, bitmap); + DeleteObject(best_icon); + return hr; +} + +static HRESULT convert_wicbitmapsource_to_gdi(IWICImagingFactory *imgfactory, + IWICBitmapSource *bitmapsource, HBITMAP *gdibitmap) +{ + BITMAPINFOHEADER bmi; + HRESULT hr; + UINT width, height; + IWICBitmapSource *newsrc; + HDC dc; + HBITMAP bm; + void *bits; + + *gdibitmap = NULL; + + hr = WICConvertBitmapSource(&GUID_WICPixelFormat32bppBGRA, bitmapsource, &newsrc); + if (FAILED(hr)) goto done; + + hr = IWICBitmapSource_GetSize(newsrc, &width, &height); + if (FAILED(hr)) goto free_newsrc; + + dc = CreateCompatibleDC(NULL); + if (!dc) + { + hr = E_FAIL; + goto free_newsrc; + } + + memset(&bmi, 0, sizeof(bmi)); + bmi.biSize = sizeof(bmi); + bmi.biWidth = width; + bmi.biHeight = -height; + bmi.biPlanes = 1; + bmi.biBitCount = 32; + bmi.biCompression = BI_RGB; + + bm = CreateDIBSection(dc, (const BITMAPINFO *)&bmi, DIB_RGB_COLORS, &bits, NULL, 0); + DeleteDC(dc); + if (!bm) + { + WARN("Cannot create bitmap.\n"); + hr = E_FAIL; + goto free_newsrc; + } + + hr = IWICBitmapSource_CopyPixels(newsrc, NULL, width * 4, width * height * 4, bits); + if (FAILED(hr)) + { + DeleteObject(bm); + goto free_newsrc; + } + + hr = S_OK; + *gdibitmap = bm; + +free_newsrc: + IWICBitmapSource_Release(newsrc); +done: + return hr; +} + static HRESULT WINAPI ShellItem_IShellItemImageFactory_GetImage(IShellItemImageFactory *iface, SIZE size, SIIGBF flags, HBITMAP *phbm) { ShellItem *This = impl_from_IShellItemImageFactory(iface); + HRESULT hr; + IWICImagingFactory *imgfactory; + IWICBitmap *bitmap = NULL; static int once; if (!once++) - FIXME("%p ({%lu, %lu} %d %p): stub\n", This, size.cx, size.cy, flags, phbm); + FIXME("%p ({%lu, %lu} %d %p): partial stub\n", This, size.cx, size.cy, flags, phbm); *phbm = NULL; - return E_NOTIMPL; + + if (flags != SIIGBF_BIGGERSIZEOK) return E_NOTIMPL; + + hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &imgfactory); + if (SUCCEEDED(hr)) + { + hr = ShellItem_get_icon_bitmap(This, imgfactory, size, flags, &bitmap); + if (SUCCEEDED(hr)) + { + hr = convert_wicbitmapsource_to_gdi(imgfactory, (IWICBitmapSource *)bitmap, phbm); + IWICBitmap_Release(bitmap); + } + IWICImagingFactory_Release(imgfactory); + } + + return hr; } static const IShellItemImageFactoryVtbl ShellItem_IShellItemImageFactory_Vtbl = { diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c index 78f199eaafc..507b3b1eac5 100644 --- a/dlls/shell32/tests/shlfolder.c +++ b/dlls/shell32/tests/shlfolder.c @@ -4464,11 +4464,9 @@ static void test_IShellItemImageFactory(void) ok(!GetModuleHandleA("windowscodecs.dll"), "WIC should not have already been loaded\n"); ret = IShellItemImageFactory_GetImage(siif, size, SIIGBF_BIGGERSIZEOK, &hbm); - todo_wine ok(ret == S_OK || broken(ret == E_PENDING /* win7 */), "GetImage returned %lx\n", ret); ok(FAILED(ret) == !hbm, "result = %lx but bitmap = %p\n", ret, hbm); - todo_wine ok(!!GetModuleHandleA("windowscodecs.dll"), "WIC should have been loaded\n"); if (SUCCEEDED(ret) && hbm) -- 2.34.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=113349 Your paranoid android. === w1064v1809 (32 bit report) === shell32: shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w1064 (32 bit report) === shell32: shlfolder.c:4961: Test failed: RMDIR: expected notification type 10, got: 40000 shlfolder.c:4968: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10) === w1064v1809 (64 bit report) === shell32: shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) shlfolder.c:4961: Test failed: RMDIR: expected notification type 10, got: 40000 shlfolder.c:4968: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10) === w1064 (64 bit report) === shell32: shlfolder.c:4961: Test failed: RMDIR: expected notification type 10, got: 40000 shlfolder.c:4968: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10) === w1064_2qxl (64 bit report) === shell32: shlfolder.c:4961: Test failed: MKDIR: expected notification type 8, got: 40000 shlfolder.c:4968: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3) === w10pro64_ar (64 bit report) === shell32: shlfolder.c:4961: Test failed: RMDIR: expected notification type 10, got: 40000 shlfolder.c:4968: Test failed: GetDisplayNameOf failed: 0x80070057 shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10) shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3) === w10pro64_he (64 bit report) === shell32: shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 2) shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w10pro64_ja (64 bit report) === shell32: shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === w10pro64_zh_CN (64 bit report) === shell32: shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000) === debian11 (32 bit report) === shell32: shelllink.c:771: Test failed: dirty (0x00000000) shelllink.c:771: Test failed: got 0x00000001 shelllink.c:771: Test failed: Didn't expect NULL Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x6a2e91bd).
participants (3)
-
Jinoh Kang -
Marvin -
Nikolay Sivov