When populating the imagelists cache with icons of different sizes, it can happen that only some icon sizes actually succeed the extraction, with others failing with a NULL handle. This is especially true with some broken "executable packers" where for example, a 32x32 icon loads okay, but a 16x16 fails. Therefore, for icons that failed to load, create them from another icon that succeeded by resizing it, favoring icons that are the closest and larger (to reduce pixelation artefacts) and with the closest aspect ratio as the source of this operation (to be as generic as possible).
For example, if the icon that needs to be created must be 16x16, an 18x18 icon would get picked over either a 32x32 (it's further from 16x16) or a 15x15 (icons larger than 16x16 are favored since they're larger than the result, so smaller icons are only picked if no other available icon is larger).
This also fixes a regression, because at some point, we stopped failing if one icon failed to extract (even if PrivateExtractIconsW returned success, icon handle could still be NULL), so when SIC_IconAppend called ImageList_AddIcon with a NULL icon handle, it would go out of sync and the icons would be totally screwed up until the application is restarted.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45696 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
v2: Add test.
dlls/shell32/iconcache.c | 72 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 12 deletions(-)
diff --git a/dlls/shell32/iconcache.c b/dlls/shell32/iconcache.c index 6107e0c..5ffb293 100644 --- a/dlls/shell32/iconcache.c +++ b/dlls/shell32/iconcache.c @@ -345,13 +345,6 @@ static INT SIC_IconAppend (const WCHAR *sourcefile, INT src_index, HICON *hicons return ret; }
-static BOOL get_imagelist_icon_size(int list, SIZE *size) -{ - if (list < 0 || list >= ARRAY_SIZE(shell_imagelists)) return FALSE; - - return ImageList_GetIconSize( shell_imagelists[list], &size->cx, &size->cy ); -} - /**************************************************************************** * SIC_LoadIcon [internal] * @@ -362,15 +355,68 @@ static INT SIC_LoadIcon (const WCHAR *sourcefile, INT index, DWORD flags) { HICON hicons[ARRAY_SIZE(shell_imagelists)] = { 0 }; HICON hshortcuts[ARRAY_SIZE(hicons)] = { 0 }; + SIZE size[ARRAY_SIZE(shell_imagelists)]; unsigned int i; - SIZE size; - int ret; + INT ret = -1;
+ /* Keep track of the sizes in case any icon fails to get extracted */ for (i = 0; i < ARRAY_SIZE(hicons); i++) { - get_imagelist_icon_size( i, &size ); - if (!PrivateExtractIconsW( sourcefile, index, size.cx, size.cy, &hicons[i], 0, 1, 0 )) - WARN("Failed to load icon %d from %s.\n", index, debugstr_w(sourcefile)); + ImageList_GetIconSize(shell_imagelists[i], &size[i].cx, &size[i].cy); + PrivateExtractIconsW(sourcefile, index, size[i].cx, size[i].cy, &hicons[i], 0, 1, 0); + } + + /* Fill any icon handles that failed to get extracted, by resizing + another icon handle that succeeded and creating the icon from it. + Use a dumb O(n^2) algorithm since ARRAY_SIZE(hicons) is small */ + for (i = 0; i < ARRAY_SIZE(hicons); i++) + { + unsigned int k, ix, iy; + BOOL failed = TRUE; + if (hicons[i]) continue; + + for (k = 0; k < ARRAY_SIZE(hicons); k++) + { + if (hicons[k]) + { + ix = iy = k; + failed = FALSE; + break; + } + } + if (failed) goto fail; + + for (k++; k < ARRAY_SIZE(hicons); k++) + { + if (!hicons[k]) continue; + + /* Find closest-sized icon, but favor larger icons to resize from */ + if (size[k].cx >= size[i].cx) + ix = (size[ix].cx < size[i].cx || size[ix].cx > size[k].cx) ? k : ix; + else + ix = (size[ix].cx < size[i].cx && size[ix].cx < size[k].cx) ? k : ix; + + if (size[k].cy >= size[i].cy) + iy = (size[iy].cy < size[i].cy || size[iy].cy > size[k].cy) ? k : iy; + else + iy = (size[iy].cy < size[i].cy && size[iy].cy < size[k].cy) ? k : iy; + } + + /* Use the closest icon in aspect ratio if ix and iy differ */ + if (ix != iy) + { + float i_ratio, ix_ratio, iy_ratio; + i_ratio = (float)size[i].cx / (float)size[i].cy; + ix_ratio = (float)size[ix].cx / (float)size[ix].cy; + iy_ratio = (float)size[iy].cx / (float)size[iy].cy; + if (fabsf(ix_ratio - i_ratio) > fabsf(iy_ratio - i_ratio)) + ix = iy; + } + + /* If this fails, we have to abort to prevent the image lists from + becoming out of sync and completely screwing the icons up */ + hicons[i] = CopyImage(hicons[ix], IMAGE_ICON, size[i].cx, size[i].cy, 0); + if (!hicons[i]) goto fail; }
if (flags & GIL_FORSHORTCUT) @@ -403,6 +449,8 @@ static INT SIC_LoadIcon (const WCHAR *sourcefile, INT index, DWORD flags) }
ret = SIC_IconAppend( sourcefile, index, hicons, flags ); + +fail: for (i = 0; i < ARRAY_SIZE(hicons); i++) DestroyIcon(hicons[i]); return ret;