[PATCH v5 0/4] Partial IShellItemImageFactory implementation (#52673)

[Jinoh Kang]

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       | 239 +++++++++++++++++++++++++++++++++
 dlls/shell32/tests/shlfolder.c |  51 +++++++
 include/shobjidl.idl           |  30 +++++
 4 files changed, 321 insertions(+), 1 deletion(-)

[PATCH v5 1/4] include: Add definition for IShellItemImageFactory.

[Jinoh Kang]

Signed-off-by: Jinoh Kang 
---

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)

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,

[PATCH v5 1/4] include: Add definition for IShellItemImageFactory.

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov

[PATCH v5 2/4] shell32/tests: Add tests for IShellItemImageFactory.

[Jinoh Kang]

Signed-off-by: Jinoh Kang 
---

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

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();

[PATCH v5 2/4] shell32/tests: Add tests for IShellItemImageFactory.

[Marvin]

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=113244

Your paranoid android.

=== w1064v1809 (32 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)

=== w1064 (32 bit report) ===

shell32:
shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== w1064_tsign (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: 3)

=== w1064 (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)

=== w1064_2qxl (64 bit report) ===

shell32:
shlfolder.c:4981: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== 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: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: 40000)

[PATCH v5 3/4] shell32: Add stub for IShellItemImageFactory.

[Jinoh Kang]

Signed-off-by: Jinoh Kang 
---

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)

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))
             {

[PATCH v5 3/4] shell32: Add stub for IShellItemImageFactory.

[Marvin]

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=113245

Your paranoid android.

=== w1064v1809 (32 bit report) ===

shell32:
shlfolder.c:4963: Test failed: RMDIR: expected notification type 10, got: 40000
shlfolder.c:4970: Test failed: GetDisplayNameOf failed: 0x80070057
shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10)

=== w1064 (32 bit report) ===

shell32:
shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== w1064_tsign (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:4963: Test failed: MKDIR: expected notification type 8, got: 40000
shlfolder.c:4970: Test failed: GetDisplayNameOf failed: 0x80070057
shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3)

=== w1064 (64 bit report) ===

shell32:
shlfolder.c:4963: Test failed: RMDIR: expected notification type 10, got: 40000
shlfolder.c:4970: Test failed: GetDisplayNameOf failed: 0x80070057
shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10)

=== w1064_2qxl (64 bit report) ===

shell32:
shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== w1064_tsign (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: 40000)
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:4963: Test failed: RMDIR: expected notification type 10, got: 40000
shlfolder.c:4970: Test failed: GetDisplayNameOf failed: 0x80070057
shlfolder.c:4980: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 10)

[PATCH v5 4/4] shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail).

[Jinoh Kang]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52673
Signed-off-by: Jinoh Kang 
---

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

dlls/shell32/Makefile.in       |   2 +-
 dlls/shell32/shellitem.c       | 195 ++++++++++++++++++++++++++++++++-
 dlls/shell32/tests/shlfolder.c |   2 -
 3 files changed, 193 insertions(+), 6 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..a1b3bc57c66 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,203 @@ 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)

[PATCH v5 4/4] shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail).

[Jin-oh Kang]

On Mon, Apr 25, 2022, 1:44 AM Jinoh Kang  wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52673
> Signed-off-by: Jinoh Kang
>
> 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
>
> dlls/shell32/Makefile.in       |   2 +-
>  dlls/shell32/shellitem.c       | 195 ++++++++++++++++++++++++++++++++-
>  dlls/shell32/tests/shlfolder.c |   2 -
>  3 files changed, 193 insertions(+), 6 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..a1b3bc57c66 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,203 @@ 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;

This line shouldn't be deleted.

-    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

[PATCH v5 4/4] shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail).

[Marvin]

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=113246

Your paranoid android.

=== w1064v1809 (32 bit report) ===

shell32:
shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== w1064 (32 bit report) ===

shell32:
shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== w1064_tsign (32 bit report) ===

shell32:
shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== w1064v1809 (64 bit report) ===

shell32:
shlfolder.c:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 3)

=== 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:4978: Test failed: Didn't expect a WM_USER_NOTIFY message (event: 40000)

=== 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: 3)

=== w10pro64_he (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)

=== w10pro64_ja (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_zh_CN (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)

[PATCH v5 4/4] shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail).

[Esme Povirk (she/they)]

gdiplus uses windowscodecs for image file loading and saving, so it
probably doesn't matter much which one you use.

[PATCH v5 4/4] shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail).

[Jin-oh Kang]

On Mon, Apr 25, 2022, 10:35 AM Esme Povirk (she/they) 
wrote:
> gdiplus uses windowscodecs for image file loading and saving, so it
> probably doesn't matter much which one you use.

After talk with Nikolay, I eventually choosed to stick with WIC mainly
because windowscodecs does not have startup/shutdown functions and thus
more light on side effects than gdiplus. The test in this patch series also
shows that Windows (probably) uses WIC for image conversion, not gdiplus.

Further testing revealed that invoking WIC for image conversion does not
actually load (delay-imported) gdiplus on both Windows and Wine.
>