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

[Nikolay Sivov]

I think this needs a lot of cleanup.

On 4/26/22 21:17, Jinoh Kang wrote:
>
>
> +
>   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)

You don't really need two icons, for SIIGBF_BIGGERSIZEOK.
> +{
> +    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;

You probably can get rid of gil_in_flags and pbc.

> +
> +    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;
> +        }

Instead of this you can probably do SHGetFileInfo(SHGFI_SYSICONINDEX), 
and then pick appropriate imagelist.
> +    }
> +    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;
> +}

This looks too complicated. System imaglists come in few sizes. Desired 
size is know on GetImage(), I think it make sense to look for exact 
match first, and load that. For non-file based case Extract() has size 
argument on its own, should that do something to pick "the best" size?

> +
> +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);
I don't think you need a device context for this.

> +    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;
>   }