Currently shell32 only transfers the plain icon for `Shell_NotifyIcon` calls, ignoring balloon icons. This patch allows transferring both images to explorer.exe tray.
-- v9: shell32: add support for balloon icon copying shell32: refactor notify_icon to allow copying multiple icons
From: Sergei Chernyadyev serg.cherniadjev@gmail.com
--- dlls/shell32/systray.c | 41 +++++++++++++++++++++---------------- programs/explorer/systray.c | 31 ++++++++++++++++++---------- 2 files changed, 43 insertions(+), 29 deletions(-)
diff --git a/dlls/shell32/systray.c b/dlls/shell32/systray.c index 78717cfcdca..ab1a772c1c2 100644 --- a/dlls/shell32/systray.c +++ b/dlls/shell32/systray.c @@ -34,6 +34,16 @@
WINE_DEFAULT_DEBUG_CHANNEL(systray);
+struct notify_data_icon +{ + /* data for the icon bitmap */ + UINT width; + UINT height; + UINT planes; + UINT bpp; + char buffer[]; +}; + struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; @@ -51,13 +61,10 @@ struct notify_data /* platform-independent format for NOTIFYICONDATA */ WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; - /* data for the icon bitmap */ - UINT width; - UINT height; - UINT planes; - UINT bpp; + struct notify_data_icon icons[]; };
+ /************************************************************************* * Shell_NotifyIcon [SHELL32.296] * Shell_NotifyIconA [SHELL32.297] @@ -180,7 +187,7 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid) cbMaskBits = (bmMask.bmPlanes * bmMask.bmWidth * bmMask.bmHeight * bmMask.bmBitsPixel + 15) / 16 * 2; if (iconinfo.hbmColor) cbColourBits = (bmColour.bmPlanes * bmColour.bmWidth * bmColour.bmHeight * bmColour.bmBitsPixel + 15) / 16 * 2; - cds.cbData = sizeof(*data) + cbMaskBits + cbColourBits; + cds.cbData = sizeof(*data) + sizeof(struct notify_data_icon) + cbMaskBits + cbColourBits; buffer = malloc(cds.cbData); if (!buffer) { @@ -192,23 +199,21 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid)
data = (struct notify_data *)buffer; memset( data, 0, sizeof(*data) ); - buffer += sizeof(*data); - GetBitmapBits(iconinfo.hbmMask, cbMaskBits, buffer); + GetBitmapBits(iconinfo.hbmMask, cbMaskBits, &data->icons[0].buffer); if (!iconinfo.hbmColor) { - data->width = bmMask.bmWidth; - data->height = bmMask.bmHeight / 2; - data->planes = 1; - data->bpp = 1; + data->icons[0].width = bmMask.bmWidth; + data->icons[0].height = bmMask.bmHeight / 2; + data->icons[0].planes = 1; + data->icons[0].bpp = 1; } else { - data->width = bmColour.bmWidth; - data->height = bmColour.bmHeight; - data->planes = bmColour.bmPlanes; - data->bpp = bmColour.bmBitsPixel; - buffer += cbMaskBits; - GetBitmapBits(iconinfo.hbmColor, cbColourBits, buffer); + data->icons[0].width = bmColour.bmWidth; + data->icons[0].height = bmColour.bmHeight; + data->icons[0].planes = bmColour.bmPlanes; + data->icons[0].bpp = bmColour.bmBitsPixel; + GetBitmapBits(iconinfo.hbmColor, cbColourBits, &data->icons[0].buffer[cbMaskBits]); DeleteObject(iconinfo.hbmColor); } DeleteObject(iconinfo.hbmMask); diff --git a/programs/explorer/systray.c b/programs/explorer/systray.c index b47c2239abd..27428b2c026 100644 --- a/programs/explorer/systray.c +++ b/programs/explorer/systray.c @@ -36,6 +36,16 @@ WINE_DEFAULT_DEBUG_CHANNEL(systray); #define TRAY_MINIMIZE_ALL 419 #define TRAY_MINIMIZE_ALL_UNDO 416
+struct notify_data_icon +{ + /* data for the icon bitmap */ + UINT width; + UINT height; + UINT planes; + UINT bpp; + char buffer[]; +}; + struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; @@ -54,10 +64,7 @@ struct notify_data /* platform-independent format for NOTIFYICONDATA */ DWORD dwInfoFlags; GUID guidItem; /* data for the icon bitmap */ - UINT width; - UINT height; - UINT planes; - UINT bpp; + struct notify_data_icon icons[]; };
#define ICON_DISPLAY_HIDDEN -1 @@ -836,6 +843,7 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds) { struct icon *icon = NULL; const struct notify_data *data; + const struct notify_data_icon *data_icon; NOTIFYICONDATAW nid; int ret = FALSE;
@@ -860,22 +868,23 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds)
/* FIXME: if statement only needed because we don't support interprocess * icon handles */ - if ((nid.uFlags & NIF_ICON) && cds->cbData > sizeof(*data)) + if ((nid.uFlags & NIF_ICON) && cds->cbData > (sizeof(*data) + sizeof(struct notify_data_icon))) { LONG cbMaskBits; LONG cbColourBits; - const char *buffer = (const char *)(data + 1); + data_icon = &data->icons[0];
- cbMaskBits = (data->width * data->height + 15) / 16 * 2; - cbColourBits = (data->planes * data->width * data->height * data->bpp + 15) / 16 * 2; + cbMaskBits = (data_icon->width * data_icon->height + 15) / 16 * 2; + cbColourBits = (data_icon->planes * data_icon->width * data_icon->height * data_icon->bpp + 15) / 16 * 2;
- if (cds->cbData < sizeof(*data) + cbMaskBits + cbColourBits) + if (cds->cbData < sizeof(*data) + sizeof(*data_icon) + cbMaskBits + cbColourBits) { ERR( "buffer underflow\n" ); return FALSE; } - nid.hIcon = CreateIcon(NULL, data->width, data->height, data->planes, data->bpp, - buffer, buffer + cbMaskBits); + nid.hIcon = CreateIcon(NULL, data_icon->width, data_icon->height, data_icon->planes, data_icon->bpp, + &data_icon->buffer[0], &data_icon->buffer[cbMaskBits]); + data_icon = (const struct notify_data_icon*)&data_icon->buffer[cbMaskBits + cbColourBits]; }
/* try forwarding to the display driver first */
From: Sergei Chernyadyev serg.cherniadjev@gmail.com
--- dlls/shell32/systray.c | 100 +++++++++++++++++++++++------------- programs/explorer/systray.c | 34 ++++++++++-- 2 files changed, 96 insertions(+), 38 deletions(-)
diff --git a/dlls/shell32/systray.c b/dlls/shell32/systray.c index ab1a772c1c2..709a0471633 100644 --- a/dlls/shell32/systray.c +++ b/dlls/shell32/systray.c @@ -132,6 +132,12 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid) COPYDATASTRUCT cds; struct notify_data data_buffer; struct notify_data *data = &data_buffer; + ICONINFO iconinfo[2] = { { 0 }, { 0 } }; + BITMAP bmMask[2]; + BITMAP bmColour[2]; + LONG cbMaskBits[2] = { 0, 0}; + LONG cbColourBits[2] = { 0, 0 }; + LONG iconCount = 0; BOOL ret;
TRACE("dwMessage = %ld, nid->cbSize=%ld\n", dwMessage, nid->cbSize); @@ -166,60 +172,84 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid) * icon handles */ if (nid->uFlags & NIF_ICON) { - ICONINFO iconinfo; - BITMAP bmMask; - BITMAP bmColour; - LONG cbMaskBits; - LONG cbColourBits = 0; - char *buffer; - - if (!GetIconInfo(nid->hIcon, &iconinfo)) + if (!GetIconInfo(nid->hIcon, &iconinfo[iconCount])) goto noicon;
- if (!GetObjectW(iconinfo.hbmMask, sizeof(bmMask), &bmMask) || - (iconinfo.hbmColor && !GetObjectW(iconinfo.hbmColor, sizeof(bmColour), &bmColour))) + if (!GetObjectW(iconinfo[iconCount].hbmMask, sizeof(bmMask), &bmMask[iconCount]) || + (iconinfo[iconCount].hbmColor && !GetObjectW(iconinfo[iconCount].hbmColor, sizeof(bmColour), &bmColour[iconCount]))) { - DeleteObject(iconinfo.hbmMask); - if (iconinfo.hbmColor) DeleteObject(iconinfo.hbmColor); + DeleteObject(iconinfo[iconCount].hbmMask); + if (iconinfo[iconCount].hbmColor) DeleteObject(iconinfo[iconCount].hbmColor); goto noicon; } + cbMaskBits[iconCount] = (bmMask[iconCount].bmPlanes * bmMask[iconCount].bmWidth * bmMask[iconCount].bmHeight * bmMask[iconCount].bmBitsPixel + 15) / 16 * 2; + if (iconinfo[iconCount].hbmColor) + cbColourBits[iconCount] = (bmColour[iconCount].bmPlanes * bmColour[iconCount].bmWidth * bmColour[iconCount].bmHeight * bmColour[iconCount].bmBitsPixel + 15) / 16 * 2; + iconCount++; + } +noicon: + if ((nid->uFlags & NIF_INFO) && (nid->dwInfoFlags & NIIF_USER)) + { + if (!GetIconInfo(nid->hBalloonIcon, &iconinfo[iconCount])) + goto noballoon;
- cbMaskBits = (bmMask.bmPlanes * bmMask.bmWidth * bmMask.bmHeight * bmMask.bmBitsPixel + 15) / 16 * 2; - if (iconinfo.hbmColor) - cbColourBits = (bmColour.bmPlanes * bmColour.bmWidth * bmColour.bmHeight * bmColour.bmBitsPixel + 15) / 16 * 2; - cds.cbData = sizeof(*data) + sizeof(struct notify_data_icon) + cbMaskBits + cbColourBits; + if (!GetObjectW(iconinfo[iconCount].hbmMask, sizeof(bmMask), &bmMask[iconCount]) || + (iconinfo[iconCount].hbmColor && !GetObjectW(iconinfo[iconCount].hbmColor, sizeof(bmColour), &bmColour[iconCount]))) + { + DeleteObject(iconinfo[iconCount].hbmMask); + if (iconinfo[iconCount].hbmColor) DeleteObject(iconinfo[iconCount].hbmColor); + goto noballoon; + } + cbMaskBits[iconCount] = (bmMask[iconCount].bmPlanes * bmMask[iconCount].bmWidth * bmMask[iconCount].bmHeight * bmMask[iconCount].bmBitsPixel + 15) / 16 * 2; + if (iconinfo[iconCount].hbmColor) + cbColourBits[iconCount] = (bmColour[iconCount].bmPlanes * bmColour[iconCount].bmWidth * bmColour[iconCount].bmHeight * bmColour[iconCount].bmBitsPixel + 15) / 16 * 2; + iconCount++; + } +noballoon: + if (iconCount > 0) + { + char *buffer; + struct notify_data_icon *data_icon; + cds.cbData = sizeof(*data) + sizeof(struct notify_data_icon) * iconCount + cbMaskBits[0] + cbMaskBits[1] + cbColourBits[0] + cbColourBits[1]; buffer = malloc(cds.cbData); if (!buffer) { - DeleteObject(iconinfo.hbmMask); - if (iconinfo.hbmColor) DeleteObject(iconinfo.hbmColor); + for (unsigned int i = 0; i < iconCount; i++) + { + if (iconinfo[i].hbmMask) DeleteObject(iconinfo[i].hbmMask); + if (iconinfo[i].hbmColor) DeleteObject(iconinfo[i].hbmColor); + } SetLastError(E_OUTOFMEMORY); return FALSE; }
data = (struct notify_data *)buffer; + data_icon = &data->icons[0]; memset( data, 0, sizeof(*data) ); - GetBitmapBits(iconinfo.hbmMask, cbMaskBits, &data->icons[0].buffer); - if (!iconinfo.hbmColor) + for (unsigned int i = 0; i < iconCount; i++) { - data->icons[0].width = bmMask.bmWidth; - data->icons[0].height = bmMask.bmHeight / 2; - data->icons[0].planes = 1; - data->icons[0].bpp = 1; + GetBitmapBits(iconinfo[i].hbmMask, cbMaskBits[i], &data_icon->buffer); + if (!iconinfo[i].hbmColor) + { + data_icon->width = bmMask[i].bmWidth; + data_icon->height = bmMask[i].bmHeight / 2; + data_icon->planes = 1; + data_icon->bpp = 1; + } + else + { + data_icon->width = bmColour[i].bmWidth; + data_icon->height = bmColour[i].bmHeight; + data_icon->planes = bmColour[i].bmPlanes; + data_icon->bpp = bmColour[i].bmBitsPixel; + GetBitmapBits(iconinfo[i].hbmColor, cbColourBits[i], &data_icon->buffer[cbMaskBits[i]]); + DeleteObject(iconinfo[i].hbmColor); + } + DeleteObject(iconinfo[i].hbmMask); + data_icon = (struct notify_data_icon*)&data_icon->buffer[cbMaskBits[i] + cbColourBits[i]]; } - else - { - data->icons[0].width = bmColour.bmWidth; - data->icons[0].height = bmColour.bmHeight; - data->icons[0].planes = bmColour.bmPlanes; - data->icons[0].bpp = bmColour.bmBitsPixel; - GetBitmapBits(iconinfo.hbmColor, cbColourBits, &data->icons[0].buffer[cbMaskBits]); - DeleteObject(iconinfo.hbmColor); - } - DeleteObject(iconinfo.hbmMask); }
-noicon: data->hWnd = HandleToLong( nid->hWnd ); data->uID = nid->uID; data->uFlags = nid->uFlags; diff --git a/programs/explorer/systray.c b/programs/explorer/systray.c index 27428b2c026..b552682e008 100644 --- a/programs/explorer/systray.c +++ b/programs/explorer/systray.c @@ -730,7 +730,17 @@ static BOOL modify_icon( struct icon *icon, NOTIFYICONDATAW *nid ) lstrcpynW( icon->info_title, nid->szInfoTitle, ARRAY_SIZE( icon->info_title )); icon->info_flags = nid->dwInfoFlags; icon->info_timeout = max(min(nid->uTimeout, BALLOON_SHOW_MAX_TIMEOUT), BALLOON_SHOW_MIN_TIMEOUT); - icon->info_icon = nid->hBalloonIcon; + + if (icon->info_icon) DestroyIcon(icon->info_icon); + if ((icon->info_flags & NIIF_ICONMASK) == NIIF_USER) + { + icon->info_icon = CopyIcon(nid->hBalloonIcon); + } + else + { + icon->info_icon = NULL; + } + update_balloon( icon ); } if (icon->state & NIS_HIDDEN) hide_icon( icon ); @@ -849,6 +859,7 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds)
if (cds->cbData < sizeof(*data)) return FALSE; data = cds->lpData; + data_icon = &data->icons[0];
nid.cbSize = sizeof(nid); nid.hWnd = LongToHandle( data->hWnd ); @@ -868,11 +879,10 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds)
/* FIXME: if statement only needed because we don't support interprocess * icon handles */ - if ((nid.uFlags & NIF_ICON) && cds->cbData > (sizeof(*data) + sizeof(struct notify_data_icon))) + if ((nid.uFlags & NIF_ICON) && cds->cbData > ((char*)data_icon - (char*)data) + sizeof(struct notify_data_icon)) { LONG cbMaskBits; LONG cbColourBits; - data_icon = &data->icons[0];
cbMaskBits = (data_icon->width * data_icon->height + 15) / 16 * 2; cbColourBits = (data_icon->planes * data_icon->width * data_icon->height * data_icon->bpp + 15) / 16 * 2; @@ -887,6 +897,23 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds) data_icon = (const struct notify_data_icon*)&data_icon->buffer[cbMaskBits + cbColourBits]; }
+ if ((nid.uFlags & NIF_INFO) && (nid.dwInfoFlags & NIIF_USER) && cds->cbData > ((char*)data_icon - (char*)data) + sizeof(struct notify_data_icon)) + { + /* Balloon icon */ + LONG cbMaskBits; + LONG cbColourBits; + + cbMaskBits = (data_icon->width * data_icon->height + 15) / 16 * 2; + cbColourBits = (data_icon->planes * data_icon->width * data_icon->height * data_icon->bpp + 15) / 16 * 2; + + if (cds->cbData < sizeof(*data) + sizeof(*data_icon) + cbMaskBits + cbColourBits) + { + WINE_ERR("buffer underflow\n"); + return FALSE; + } + nid.hBalloonIcon = CreateIcon(NULL, data_icon->width, data_icon->height, data_icon->planes, data_icon->bpp, + &data_icon->buffer[0], &data_icon->buffer[cbMaskBits]); + } /* try forwarding to the display driver first */ if (cds->dwData == NIM_ADD || !(icon = get_icon( nid.hWnd, nid.uID ))) { @@ -921,6 +948,7 @@ static BOOL handle_incoming(HWND hwndSource, COPYDATASTRUCT *cds)
done: if (nid.hIcon) DestroyIcon( nid.hIcon ); + if (nid.hBalloonIcon) DestroyIcon( nid.hBalloonIcon ); sync_taskbar_buttons(); return ret; }
Maybe it's time to support inter-process icon handles?
Rémi Bernon (@rbernon) commented about dlls/shell32/systray.c:
WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem;
- /* data for the icon bitmap */
- UINT width;
- UINT height;
- UINT planes;
- UINT bpp;
- struct notify_data_icon icons[];
Then if we prefer to do it like this for now, I think we only need two icon data here as NOTIFYICONDATA has only two HICON, no need for a variable sized array?
On Fri Sep 13 09:35:28 2024 +0000, Rémi Bernon wrote:
Then if we prefer to do it like this for now, I think we only need two icon data here as NOTIFYICONDATA has only two HICON, no need for a variable sized array?
I did this, because image data follows right after the struct, so I thought it's a bad idea to add two similar fields for two different images and stack two image bitmaps one after another straight away.
Also here the struct becomes smaller if there are no images present.
On Fri Sep 13 19:30:15 2024 +0000, Rémi Bernon wrote:
Maybe it's time to support inter-process icon handles?
I don't know, do icons really persist in shared memory on windows? Seems like an overkill for 16x16 images.
On Fri Sep 13 19:35:00 2024 +0000, Sergei Chernyadyev wrote:
I don't know, do icons really persist in shared memory on windows? Seems like an overkill for 16x16 images.
I think it does, but it's indeed probably a lot of work to make that work in Wine yet.
On Fri Sep 13 19:29:22 2024 +0000, Sergei Chernyadyev wrote:
I did this, because image data follows right after the struct, so I thought it's a bad idea to add two similar fields for two different images and stack two image bitmaps one after another straight away. Also here the struct becomes smaller if there are no images present.
I think it would be better to wrap only the metadata in the struct, while keeping the data itself appended. Otherwise it looks like an array of `notify_data_icon` although each struct is also variable sized.
On Fri Sep 13 19:35:00 2024 +0000, Rémi Bernon wrote:
I think it does, but it's indeed probably a lot of work to make that work in Wine yet.
Judging by functions looks like it isn't, it's attached to a module (HINSTANCE/HMODULE), not sure though why WINAPI chose HICON instead of plain HBITMAP object (maybe due to the mask layer being required). So I doubt that they would share the memory region (especially that small) to explorer.exe, so the icon could be displayed in systray.
On Fri Sep 13 19:39:57 2024 +0000, Rémi Bernon wrote:
I think it would be better to wrap only the metadata in the struct, while keeping the data itself appended. Otherwise it looks like an array of `notify_data_icon` although each struct is also variable sized.
Hmmm is it better to do it like this?
``` notify_data -> notify_data_icon -> image bitmap data -> second notify_data_icon -> another image bitmap data ```
where `notify_data_icon`:
```C struct notify_data_icon { /* data for the icon bitmap */ UINT width; UINT height; UINT planes; UINT bpp; }; ```
and `notify_data` will look like that (without image related fields):
```C struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; UINT uID; UINT uFlags; UINT uCallbackMessage; WCHAR szTip[128]; DWORD dwState; DWORD dwStateMask; WCHAR szInfo[256]; union { UINT uTimeout; UINT uVersion; } u; WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; ```
On Mon Sep 16 16:40:14 2024 +0000, Sergei Chernyadyev wrote:
Hmmm is it better to do it like this?
notify_data -> notify_data_icon -> image bitmap data -> second notify_data_icon -> another image bitmap datawhere `notify_data_icon`:
struct notify_data_icon { /* data for the icon bitmap */ UINT width; UINT height; UINT planes; UINT bpp; };and `notify_data` will look like that (without image related fields):
struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; UINT uID; UINT uFlags; UINT uCallbackMessage; WCHAR szTip[128]; DWORD dwState; DWORD dwStateMask; WCHAR szInfo[256]; union { UINT uTimeout; UINT uVersion; } u; WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem;
I was thinking more like:
``` struct notify_data_icon { UINT width; UINT height; UINT planes; UINT bpp; };
struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; UINT uID; UINT uFlags; UINT uCallbackMessage; struct notify_data_icon icon; /* icon bitmap info */ WCHAR szTip[128]; DWORD dwState; DWORD dwStateMask; WCHAR szInfo[256]; union { UINT uTimeout; UINT uVersion; } u; WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; struct notify_data_icon balloon_icon; /* balloon icon bitmap info */ BYTE icon_data[]; /* data for the icon bitmap followed by balloon icon bitmap */ }; ```
On Mon Sep 16 17:29:24 2024 +0000, Rémi Bernon wrote:
I was thinking more like:
struct notify_data_icon { UINT width; UINT height; UINT planes; UINT bpp; }; struct notify_data /* platform-independent format for NOTIFYICONDATA */ { LONG hWnd; UINT uID; UINT uFlags; UINT uCallbackMessage; struct notify_data_icon icon; /* icon bitmap info */ WCHAR szTip[128]; DWORD dwState; DWORD dwStateMask; WCHAR szInfo[256]; union { UINT uTimeout; UINT uVersion; } u; WCHAR szInfoTitle[64]; DWORD dwInfoFlags; GUID guidItem; struct notify_data_icon balloon_icon; /* balloon icon bitmap info */ BYTE icon_data[]; /* data for the icon bitmap followed by balloon icon bitmap */ };
seems to be feasible, yet there will be additional 8 fields to be sent through sendmessage even in case if there are no icons to be sent over to explorer.exe
-
Rémi Bernon (＠rbernon) -
Sergei Chernyadyev
-
Sergei Chernyadyev (＠Cherser-s)