The X-to-Win32 icon code assumes 32-bit alignment, but creates icons from device-dependent bitmaps, which are 16-bit aligned. I am including a patch that fixes this issue, but also includes numerous other icon-related updates: 1) Modified the code to update icons directly to windows rather than modifying the window's class, which does not update custom window icons. 2) There is no point in having one class for every window, aside from trying to set custom icons via the class, so I converted to using a single class for all client windows. 3) Modified the bitmap data alignment to 16 bits in winmultiwindowicons.c (3 places). 4) Changed windialogs.c to set icons via window properties rather than class properties, and use LoadImage() for small icons, because LoadIcon() can only opn large icons. SInce this code is redundant across the dialogs, I put it in the < winCenterDialog function, along with a few other redundant instructions, and renamed in winInitDialog(). 5) As a more experimental addition, I have included code to load RGBA NET_WM icons. This needs to be tested on pre-XP systems (i.e. Win2K) to see how the OS handles Icon creation from RGBA data.
Created attachment 3311 [details] [review] Patch to implement the changes described above.
Works OK for me, except that the .XWinrc system memu customization fails, for me, see:- http://cygwin.com/ml/cygwin-xfree/2005-09/msg00152.html (minor fix #if 0 to 1 at 604a588 in winmultiwindowwindow.c) Also I couldn't compile Xming.exe with the winwindow.h addition (OK if missed out) Colin Harrison
Created attachment 3330 [details] [review] Updated patch; fixes SetupSysMenu. This fixes the SetupSysMenu problem mentioned by Colin. I had it under "#if 0" because I had not fully tested that change. Note: I also have not yet tested NET_WM_ICON icons under Win2K.
My winwindow.h problem (when cross-compiling for Xming.exe using mingw):- ... In file included from winmultiwindowwm.c:68: winwindow.h:142: error: syntax error before "winInitMultiWindowClass" winwindow.h:142: warning: type defaults to `int' in declaration of `winInitMultiWindowClass' winwindow.h:142: warning: data definition has no type or storage class ... can be fixed by copying in these lines (from winclipboard.h) to winwindow.h /* Fixups to prevent collisions between Windows and X headers */ #define ATOM DWORD
A compromise fix for winwindow.h and Xming:- --- ./programs/Xserver/hw/xwin/save_winwindow.h 2005-09-20 17:54:58.000000000 +0100 +++ ./programs/Xserver/hw/xwin/winwindow.h 2005-09-20 17:40:51.000000000 +0100 @@ -137,6 +137,11 @@ /* * winmultiwindowicons.c */ +/* Fixup to prevent a syntax error in Xming */ +#ifdef __CYGWIN__ +ATOM +winInitMultiWindowClass(BOOL Reset); +#endif void winUpdateIcon (Window id); And I was told to always declare all functions in the headers and follow 'coding standards':) So far I've only seen kde NetIcons of weird? sizes, and no others e.g. Window 3440608: found 34 x 34 NetIcon Window 3440608: found 18 x 18 NetIcon Window 3440608: no 32 x 32 NetIcon
I'm working on an update to this patch that: 1) Destroys icons from deleted windows, in place of code that destroys per-window Class icons. 2) The NET_WM_ICON code for ARGB icons uses a 'blank' mask bitmap, copied from a MSDN example. The mask should actually be filled out correctly, at least to make sure Win2K icons will work.
There seem to be a lot of fixes in this patch. Would it make sense to break it up into smaller patches, each on addressing a single issue? It would make it easier to test and commit the fixes that are complete vs one that are still being tested and enhanced.
That makes sense to me as well Stuart. Joe - what's the status on these ??
I'll take this on as Joe isn't responding?
Sorry about the phenomenal bug spam, guys. Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Created attachment 19690 [details] [review] Minimal patch to correct icon data alignment Created a new patch containing just the fix for the defect described (extacted from the original patch)
Thanks Jon, looks good. Colin?
Jon's patch is OK, but the rest of Joe's stuff is good as well? However Joe never completely finished off, but as it is, it has worked well for me at all colour quality depths. Alpha icons still needs tidying and good simple test cases. The latter has stopped me from coding this further. _NET_WM_ICON icons can come in 'odd' sizes (as in other than Windows common sizes of 16, 32, 48 pixels). These can go wrong at 16bit colour quality etc. if the DIB route is not taken (via Joe's code).
Created attachment 20378 [details] My current 16x16 and 32x32 net icon testcases This is how I test the conversions at the moment.
Joe got a z-position wrong in winInitDialog()... .... + /* Center dialog window in the screen. Not done for multi-monitor systems, where + * it is likely to end up split across the screens. In that case, it appears + * near the Tray icon. + */ + if (GetSystemMetrics(SM_CMONITORS)>1) { + /* Still need to refresh the frame change. */ + SetWindowPos (hwndDlg, HWND_TOPMOST, 0,0,0,0, + SWP_NOMOVE | SWP_NOSIZE | SWP_FRAMECHANGED); + } else { .... The dialog should not be just HWND_TOP IMH0.
bugzilla-daemon@freedesktop.org wrote: > http://bugs.freedesktop.org/show_bug.cgi?id=4491 > > > > > > --- Comment #13 from Colin Harrison <colin.harrison@virgin.net> 2008-11-14 02:06:27 PST --- > Jon's patch is OK, but the rest of Joe's stuff is good as well? However Joe > never completely finished off, but as it is, it has worked well for me at all > colour quality depths. Alpha icons still needs tidying and good simple test > cases. The latter has stopped me from coding this further. _NET_WM_ICON icons > can come in 'odd' sizes (as in other than Windows common sizes of 16, 32, 48 > pixels). These can go wrong at 16bit colour quality etc. if the DIB route is > not taken (via Joe's code). > I did have some mistakes in the code. I never got around to really debugging it, because I did not get feedback as to whether is was working or not. There were some inconsistencies in MS documentation, and I wasn't sure about using newer Alpha features. I can look into this again, now that the code is actually getting used. Joe Krahn
Hi Joe, Cygwin/X now use your code, but without using the CreateDIBSection http://cygwin-ports.svn.sourceforge.net/viewvc/cygwin-ports/ports/trunk/xorg/xorg-server/cygwin-net_wm_icon-support.patch?revision=4968 This patch does not work well at color depths other than 32 (for _NET_WM_ICON) I still use your NetWMToWinIcon() code in Xming, but not at odd sizes (i.e. not 16, 32, 48 pixels) when not 32bpp http://www.straightrunning.com/XmingCode/patch4/winmultiwindowicons.c.patch Thanks Colin
Created attachment 20540 [details] Xming's latest convertors Add an alternative _NET_WM_ICON to HICON converter for use when not using a desktop colour depth of 32. Unlike the original, that used a DIB section and let Windows handle alpha, this one creates a DDB image and mask and just uses alpha as a threshold (127) to set icon opacity or transparency.
Created attachment 20546 [details] [review] Xming's latest convertors Honest..I tested this one, this time :)
Merged the data-alignment fix part of the patch (which fixes the headline issue) as commit bf65523ab0b39774f07a7ae478ff3f5653fad469 Leaving this bug open to track the other good stuff in here (although per comment #7 please feel free to raise separate bugs for those parts) Joe: I notice as part of your patch you change things from using one class per window to using a single class. Is this just cleanup or does this fix some problem as well? The other (very minor) aesthetic issue I notice with the icon converter is that if the X icon is noticably non-square, the converted image is aligned to the top-left of the windows icon (which must be square, I think), rather than being centered.
As a PS to my last patch.. Atom caching caching does not survive server regeneration so static Atom _XA_NET_WM_ICON; static int generation; ... if (generation != serverGeneration) { generation = serverGeneration; _XA_NET_WM_ICON = MakeAtom("_NET_WM_ICON", 12, FALSE); } ... in winXIconToHICON() is better.
Merged some parts extracted from this patch: commit 996357e905c1082479bb238110b93bc170b8cb84 Cygwin/X: Update icons directly, rather than modifying the window's class commit a72865868f03b675f86990476fcee601822894b3 Cygwin/X: Consolidate dialog initialization in winInitDialog() I think that leaves: (1) change to a single class for all client windows, and (2) convert icons from _NET_WM_ICON hints
Changes to use a single native window class applied as 17e67c407d130c325d3899c18d68b8eef6a88bea
Created attachment 28480 [details] [review] Updated NET_WM_ICON patch Okay, I've taken the current icon converter code from Xming, added a slight tweak so we don't try to generate icons with alpha on Windows 2000, as that is still allegedly supported by Cygwin 1.7. Hopefully this is ok to commmit and we can close this bug at last :-) (This patch is also at http://sourceware.org/bugzilla/show_bug.cgi?id=10455)
With a few minor changes I sent Jon (for his patch above) this should now be OK. Just under 4 years to do this must a record! I'm sure that more fun could be had with the icon conversion code.... perhaps an alpha blend, there maybe something in pixman to help with this. But let's put this one to bed now. Thanks, Colin
_NET_WM_ICON conversion code pushed as commit a400dbb38f93030d51afe806b4b20d5ef501c855. Thanks!
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.