[PATCH v5 0/1] MR10036: winemac.drv: Unconditionally use CreateIconFromResourceEx for app icons.
Sidesteps an issue cause by some apps having an inaccurate size for the resource in the GRPICONDIR size ; CreateIconFromResourceEx handles that case, but the old CGImage path would fault if the GRPICONDIR size was too large. --- It seems that some apps have different sizes in the GRPICONDIR and the IMAGE_RESOURCE_DATA_ENTRY. winemenubuilder has had a workaround for this since 2014 - see c205e6800a63a5df9960d8484a2e67687d53bc50. The size difference is great enough in some situations (e.g. Brotato) that the attempt read the resource may fault, but that's only because we passed the PNG data over to the Unix side of winemac.drv directly, and used CG methods to deal with it, which will attempt to copy in the entire array of bytes. If we use CreateIconFromResourceEx instead, for all cases (it can handle pngs), we do not fault, since that method does not attempt to copy the entire resource size. The original mailing list discussion about the winemenubuilder patch is [here](https://marc.info/?l=wine-patches&m=140903555724711&w=2). Huge thanks to @bshanks for spotting that patch. This has apparently always been faulting for some apps, but only became an issue after !10032. The subsequent syscall fault unwinds out of the pthread_once that !10032 added, so future threads that wind up there will hang. -- v5: winemac.drv: Unconditionally use CreateIconFromResourceEx for app icons. https://gitlab.winehq.org/wine/wine/-/merge_requests/10036
From: Tim Clem <tclem@codeweavers.com> CreateIconFromResourceEx can handle PNGs, so we can consolidate code paths by special-casing those for handling by CoreGraphics. Also use SizeofResource for the byte size of the icon. Some apps have an inaccurate size for resources in the GRPICONDIR. --- dlls/winemac.drv/dllmain.c | 25 ++++++++----------------- dlls/winemac.drv/image.c | 25 +++---------------------- dlls/winemac.drv/unixlib.h | 2 -- 3 files changed, 11 insertions(+), 41 deletions(-) diff --git a/dlls/winemac.drv/dllmain.c b/dlls/winemac.drv/dllmain.c index 7ece4dc99a3..b4b10d49ed8 100644 --- a/dlls/winemac.drv/dllmain.c +++ b/dlls/winemac.drv/dllmain.c @@ -334,31 +334,22 @@ static NTSTATUS WINAPI macdrv_app_icon(void *arg, ULONG size) icon_bits = LockResource(icon_res_data); if (icon_bits) { - static const BYTE png_magic[] = { 0x89, 0x50, 0x4e, 0x47 }; + HICON icon; entry->width = width; entry->height = height; - entry->size = icon_dir->idEntries[i].dwBytesInRes; - if (!memcmp(icon_bits, png_magic, sizeof(png_magic))) + /* dwBytesInRes from the icon_dir entry is wrong in some apps; use + SizeofResource instead. */ + icon = CreateIconFromResourceEx(icon_bits, SizeofResource(NULL, res_info), + TRUE, 0x00030000, width, height, 0); + if (icon) { - entry->png = (UINT_PTR)icon_bits; - entry->icon = 0; + entry->icon = HandleToUlong(icon); count++; } else - { - HICON icon = CreateIconFromResourceEx(icon_bits, icon_dir->idEntries[i].dwBytesInRes, - TRUE, 0x00030000, width, height, 0); - if (icon) - { - entry->icon = HandleToUlong(icon); - entry->png = 0; - count++; - } - else - WARN("failed to create icon %d from resource with ID %hd\n", i, icon_dir->idEntries[i].nID); - } + WARN("failed to create icon %d from resource with ID %hd\n", i, icon_dir->idEntries[i].nID); } else WARN("failed to lock RT_ICON resource %d with ID %hd\n", i, icon_dir->idEntries[i].nID); diff --git a/dlls/winemac.drv/image.c b/dlls/winemac.drv/image.c index eb5939ce438..cf927ce7d1a 100644 --- a/dlls/winemac.drv/image.c +++ b/dlls/winemac.drv/image.c @@ -278,28 +278,9 @@ CFArrayRef create_app_icon_images(void) { struct app_icon_entry *icon = &entries[i]; CGImageRef cgimage = NULL; - - if (icon->png) - { - CFDataRef data = CFDataCreate(NULL, param_ptr(icon->png), icon->size); - if (data) - { - CGDataProviderRef provider = CGDataProviderCreateWithCFData(data); - CFRelease(data); - if (provider) - { - cgimage = CGImageCreateWithPNGDataProvider(provider, NULL, FALSE, - kCGRenderingIntentDefault); - CGDataProviderRelease(provider); - } - } - } - else - { - HICON handle = UlongToHandle(icon->icon); - cgimage = create_cgimage_from_icon(handle, icon->width, icon->height); - NtUserDestroyCursor(handle, 0); - } + HICON handle = UlongToHandle(icon->icon); + cgimage = create_cgimage_from_icon(handle, icon->width, icon->height); + NtUserDestroyCursor(handle, 0); if (cgimage) { diff --git a/dlls/winemac.drv/unixlib.h b/dlls/winemac.drv/unixlib.h index d39b3ea5b83..dc22da02d20 100644 --- a/dlls/winemac.drv/unixlib.h +++ b/dlls/winemac.drv/unixlib.h @@ -54,9 +54,7 @@ { UINT32 width; UINT32 height; - UINT32 size; UINT32 icon; - UINT64 png; }; /* macdrv_app_quit_request params */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10036
On Thu Feb 5 22:21:57 2026 +0000, Alexandre Julliard wrote:
Yes, it's the max accessible size, so using `SizeofResource()` would be more correct. Now doing that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10036#note_128930
This merge request was approved by Brendan Shanks. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10036
participants (3)
-
Brendan Shanks (@bshanks) -
Tim Clem -
Tim Clem (@tclem)