Try to lookup icon information from shell32.dll. If an icon is not found, default to the file icon.
Instead of defaulting to -1, which was failing for Affinity Photo's installer.
-- v5: shell32: Try to lookup icon, with fallback on failure
From: James McDonnell topgamer7@gmail.com
Try to lookup icon information from shell32.dll. If an icon is not found, default to the help icon.
This resolves the issue where in affinity photos installer it would crash because hIcon was returned as 0 --- dlls/shell32/iconcache.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/dlls/shell32/iconcache.c b/dlls/shell32/iconcache.c index 8f9519d2ca8..c72671eb528 100644 --- a/dlls/shell32/iconcache.c +++ b/dlls/shell32/iconcache.c @@ -976,6 +976,16 @@ INT WINAPI SHGetIconOverlayIndexW(LPCWSTR pszIconPath, INT iIconIndex) return -1; }
+/**************************************************************************** + * StockIconIdToResourceId [SHELL32.@] + * + * Map the SHSTOCKICONID value to shresdef.h IDI_ values + */ +INT StockIconIdToResourceId(SHSTOCKICONID id) +{ + return id + 1; +} + /**************************************************************************** * SHGetStockIconInfo [SHELL32.@] * @@ -1000,16 +1010,24 @@ HRESULT WINAPI SHGetStockIconInfo(SHSTOCKICONID id, UINT flags, SHSTOCKICONINFO
GetSystemDirectoryW(sii->szPath, MAX_PATH);
- /* no icons defined: use default */ - sii->iIcon = -IDI_SHELL_FILE; + sii->iIcon = id; lstrcatW(sii->szPath, L"\shell32.dll");
- if (flags) - FIXME("flags 0x%x not implemented\n", flags); + if (flags & ~SHGSI_ICON) + FIXME("unhandled flags 0x%x\n", flags);
sii->hIcon = NULL; - if (flags & SHGSI_ICON) - sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(sii->iIcon)); + if (flags & SHGSI_ICON) { + sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(StockIconIdToResourceId(id))); + + // many icons do not exist yet in wine, map to the question mark + if (!sii->hIcon) { + FIXME("Icon %p missing, defaulting to help icon.\n", sii->hIcon); + + sii->iIcon = IDI_SHELL_HELP - 1; + sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(IDI_SHELL_HELP)); + } + } sii->iSysImageIndex = -1;
TRACE("%3d: returning %s (%d)\n", id, debugstr_w(sii->szPath), sii->iIcon);
Zhiyi Zhang (@zhiyi) commented about dlls/shell32/iconcache.c:
- sii->iIcon = -IDI_SHELL_FILE;
- sii->iIcon = id; lstrcatW(sii->szPath, L"\shell32.dll");
- if (flags)
FIXME("flags 0x%x not implemented\n", flags);
if (flags & ~SHGSI_ICON)
FIXME("unhandled flags 0x%x\n", flags);sii->hIcon = NULL;
- if (flags & SHGSI_ICON)
sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(sii->iIcon));
- if (flags & SHGSI_ICON) {
sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(StockIconIdToResourceId(id)));// many icons do not exist yet in wine, map to the question mark
Use /**/ for comments.
Zhiyi Zhang (@zhiyi) commented about dlls/shell32/iconcache.c:
- if (flags)
FIXME("flags 0x%x not implemented\n", flags);
if (flags & ~SHGSI_ICON)
FIXME("unhandled flags 0x%x\n", flags);sii->hIcon = NULL;
- if (flags & SHGSI_ICON)
sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(sii->iIcon));
- if (flags & SHGSI_ICON) {
sii->hIcon = LoadIconW(GetModuleHandleW(sii->szPath), MAKEINTRESOURCEW(StockIconIdToResourceId(id)));// many icons do not exist yet in wine, map to the question markif (!sii->hIcon) {FIXME("Icon %p missing, defaulting to help icon.\n", sii->hIcon);sii->iIcon = IDI_SHELL_HELP - 1;
Please add some tests.
Zhiyi Zhang (@zhiyi) commented about dlls/shell32/iconcache.c:
return -1; }
+/****************************************************************************
- StockIconIdToResourceId [SHELL32.@]
- Map the SHSTOCKICONID value to shresdef.h IDI_ values
- */
+INT StockIconIdToResourceId(SHSTOCKICONID id) +{
- return id + 1;
I don't think this worth adding a function.
On Sun Jul 20 03:30:00 2025 +0000, Zhiyi Zhang wrote:
Please add some tests. And I don't think falling back to IDI_SHELL_HELP is correct.
Well no, its not. But it at least returns an icon. There are only 43 icons defined in dlls/shell32/resources. But this function is supposed to return something like 180.
This at least returns an icon, as opposed to failing because the icon does not exist.
On Sun Jul 20 03:44:12 2025 +0000, James McDonnell wrote:
Well no, its not. But it at least returns an icon. There are only 43 icons defined in dlls/shell32/resources. But this function is supposed to return something like 180. This at least returns an icon, as opposed to failing because the icon does not exist.
It seems like there is a need to fallback to something. That or create the missing icons. Given the wine projects conservative nature regarding IP, I doubt generating them with AI would be accepted.
If using a default of an existing icon is not a good solution, what would you suggest?
On Sun Jul 20 03:47:32 2025 +0000, James McDonnell wrote:
It seems like there is a need to fallback to something. That or create the missing icons. Given the wine projects conservative nature regarding IP, I doubt generating them with AI would be accepted. If using a default of an existing icon is not a good solution, what would you suggest?
I would add the missing icons when necessary. There are some icon sets out there with licenses compatible with Wine's license.
On Sun Jul 20 04:22:06 2025 +0000, Zhiyi Zhang wrote:
I would add the missing icons when necessary. There are some icon sets out there with licenses compatible with Wine's license.
For example, 4213781ae0f8b45c7ef01e8f4c28a47d8c348e3b
-
James McDonnell -
James McDonnell (＠ElementalWarrior)
-
Zhiyi Zhang (＠zhiyi)