Re: RFC: XEmbed Systray Patches
On Tue, 2006-08-15 at 12:14 +0100, Mike Hearn wrote: > Great work! Now let's hope Alexandre accepts it. We're working on it. I talked about it briefly with Alexandre this morning on IRC. ATM he thinks I should get rid of the low-level checks for the systray windows. He suggested that I do something like have the window created off-screen so it isn't mapped. He also didn't like the idea of adding more dependencies on the WS_EX_TRAYWINDOW style, so right now I'm trying to figure out a way to get rid of it entirely, since now is as good a time as any to finally ditch that particular hack. ATM I'm considering using the systray adaptor class name as an identifier and calling the dock function in X11DRV_CreateWindow. I'll put that together, and if it works, I'll send another RFC to the list. James
Re: RFC: XEmbed Systray Patches
On 8/15/06, James Liggett <[EMAIL PROTECTED]> wrote: Just FYI--I figured out what was wrong with my X test app. I just needed to set some size hints on the window, and now the size is perfect when the window docks. Now I've got my test app, a GNOME program, and some wine apps all peacefully coexisting on my tray, and they all dock perfectly. Guess there wasn't a bug in GNOME after all... ;) Great work! Now let's hope Alexandre accepts it.
Re: RFC: XEmbed Systray Patches
James Liggett <[EMAIL PROTECTED]> writes:
> +{
> +if ((X11DRV_get_systray_window( display ) == None) ||
> +(!(ex_style & WS_EX_TRAYWINDOW)))
> +{
> +XMapWindow( display, data->whole_window );
> +}
You don't want to add more dependencies on the WS_EX_TRAYWINDOW style,
it will have to be removed sooner or later. Also there's no reason to
do the mapping check at such a low level, since it's a window we
control you can simply avoid calling ShowWindow on it.
--
Alexandre Julliard
[EMAIL PROTECTED]
RFC: XEmbed Systray Patches
Hi Everyone,
First, I'd like to apologize for sending those tarballs to wine-devel,
as I've just been advised on IRC that such a practice discourages
reviews. I've attached a text version of the patches in the hopes that
it will get some more exposure.
Again, sorry for the trouble. :)
James
>From nobody Mon Sep 17 00:00:00 2001
From: James Liggett <[EMAIL PROTECTED]>
Date: Fri Aug 11 13:13:19 2006 -0700
Subject: [PATCH] winex11.drv: Add XEmbed atom definitions to x11drv.h
---
dlls/winex11.drv/x11drv.h |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
a240428bd9a0e280d852a49611034cd5dc7dbd71
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
index e5027c4..a4eb516 100644
--- a/dlls/winex11.drv/x11drv.h
+++ b/dlls/winex11.drv/x11drv.h
@@ -562,6 +562,8 @@ enum x11drv_atoms
XATOM_DndSelection,
XATOM__MOTIF_WM_HINTS,
XATOM__KDE_NET_WM_SYSTEM_TRAY_WINDOW_FOR,
+XATOM__NET_SYSTEM_TRAY_OPCODE,
+XATOM__NET_SYSTEM_TRAY_S0,
XATOM__NET_WM_MOVERESIZE,
XATOM__NET_WM_NAME,
XATOM__NET_WM_PID,
@@ -570,6 +572,7 @@ enum x11drv_atoms
XATOM__NET_WM_STATE_FULLSCREEN,
XATOM__NET_WM_WINDOW_TYPE,
XATOM__NET_WM_WINDOW_TYPE_UTILITY,
+XATOM__XEMBED_INFO,
XATOM_XdndAware,
XATOM_XdndEnter,
XATOM_XdndPosition,
--
1.2.4
>From nobody Mon Sep 17 00:00:00 2001
From: James Liggett <[EMAIL PROTECTED]>
Date: Fri Aug 11 13:14:46 2006 -0700
Subject: [PATCH] winex11.drv: Added systray atom names to x11drv.h.
---
dlls/winex11.drv/x11drv_main.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
75a3460ffbde801bed93236996d4b952e8ba4a1a
diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c
index 41d45a6..0403b5d 100644
--- a/dlls/winex11.drv/x11drv_main.c
+++ b/dlls/winex11.drv/x11drv_main.c
@@ -125,6 +125,8 @@ static const char * const atom_names[NB_
"DndSelection",
"_MOTIF_WM_HINTS",
"_KDE_NET_WM_SYSTEM_TRAY_WINDOW_FOR",
+"_NET_SYSTEM_TRAY_OPCODE",
+"_NET_SYSTEM_TRAY_S0",
"_NET_WM_MOVERESIZE",
"_NET_WM_NAME",
"_NET_WM_PID",
@@ -133,6 +135,7 @@ static const char * const atom_names[NB_
"_NET_WM_STATE_FULLSCREEN",
"_NET_WM_WINDOW_TYPE",
"_NET_WM_WINDOW_TYPE_UTILITY",
+"_XEMBED_INFO",
"XdndAware",
"XdndEnter",
"XdndPosition",
--
1.2.4
>From nobody Mon Sep 17 00:00:00 2001
From: James Liggett <[EMAIL PROTECTED]>
Date: Fri Aug 11 13:17:20 2006 -0700
Subject: [PATCH] winex11.drv: Dynamically create systray atom names based on the screen
we're running on.
---
dlls/winex11.drv/x11drv_main.c | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
46ca62dd8ed65dbdd658490430ca14847d8274f9
diff --git a/dlls/winex11.drv/x11drv_main.c b/dlls/winex11.drv/x11drv_main.c
index 0403b5d..9d375e9 100644
--- a/dlls/winex11.drv/x11drv_main.c
+++ b/dlls/winex11.drv/x11drv_main.c
@@ -104,6 +104,7 @@ static char input_style[20];
((ch) == 'n' || (ch) == 'N' || (ch) == 'f' || (ch) == 'F' || (ch) == '0')
Atom X11DRV_Atoms[NB_XATOMS - FIRST_XATOM];
+Atom systray_atom;
static const char * const atom_names[NB_XATOMS - FIRST_XATOM] =
{
@@ -419,6 +420,16 @@ static BOOL process_attach(void)
}
XInternAtoms( display, (char **)atom_names, NB_XATOMS - FIRST_XATOM, False, X11DRV_Atoms );
+
+if (DefaultScreen( display ) == 0)
+systray_atom = x11drv_atom(_NET_SYSTEM_TRAY_S0);
+else
+{
+char systray_buffer[29]; /* strlen(_NET_SYSTEM_TRAY_S4294967295)+1 */
+sprintf( systray_buffer, "_NET_SYSTEM_TRAY_S%d", DefaultScreen( display ) );
+systray_atom = XInternAtom( display, systray_buffer, False );
+}
+
if (TRACE_ON(synchronous)) XSynchronize( display, True );
--
1.2.4
>From nobody Mon Sep 17 00:00:00 2001
From: James Liggett <[EMAIL PROTECTED]>
Date: Fri Aug 11 13:19:10 2006 -0700
Subject: [PATCH] winex11.drv: Add system tray docking message defines to window.c
---
dlls/winex11.drv/window.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
035068f80b65110c6ba249920e73f5446bec6b08
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index 2bc90a7..0b87c52 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -58,6 +58,11 @@ static const char icon_window_prop[] =
static const char managed_prop[] = "__wine_x11_managed";
static const char visual_id_prop[]= "__wine_x11_visual_id";
+/* for XDG systray icons */
+#define SYSTEM_TRAY_REQUEST_DOCK0
+#define SYSTEM_TRAY_BEGIN_MESSAGE 1
+#define SYSTEM_TRAY_CANCEL_MESSAGE 2
+
/***
* is_window_managed
*
--
1.2.4
>From nobody Mon Sep 17 00:00:00 2001
From: James Liggett <[EMAIL PROTECTED]>
Date: Sat Aug 12 22:48:43 2006 -0700
Subject: [PATCH] winex11.drv: Add function X11DRV_get_systray to get the XEmbed system
tray running on the current display, if there is one.
---
dlls/winex11.drv/window.c
Re: RFC: XEmbed Systray Patches
On Wed, 2006-08-09 at 13:30 -0700, James Liggett wrote: > > also a problem where a window would dock but only be allocated 1 > > pixel, I think that was a bug in GNOME that is now fixed. > I'm not quite sure about that one--I've got an X test app that docks, > but I can only get 1 pixel wide. :( On wine, this isn't a problem > though. Still trying to figure out what I need to do. Just FYI--I figured out what was wrong with my X test app. I just needed to set some size hints on the window, and now the size is perfect when the window docks. Now I've got my test app, a GNOME program, and some wine apps all peacefully coexisting on my tray, and they all dock perfectly. Guess there wasn't a bug in GNOME after all... ;)
Re: RFC: XEmbed Systray Patches
On Thu, 2006-08-10 at 00:16 +0100, Mike Hearn wrote: > You can always stick some needed field in the X11DRV private data (you > can see how to access that in the code itself). OK--I apologize for being so dumb. it's the server reply that needs the extended styles field, not X11DRV_win_data. Sorry to waste your time. I think I've finally got this one figured out. The results from two days ago turned out to be false positives because: 1. I wasn't checking for the tray style right (Didn't get the extended styles when I needed to; I was trying to check for WS_EX_TRAYWINDOW in the regular styles flag--argh! I've fixed that now.) 2. I noticed that the patches (without the right mapping fixes) work properly (for some strange reason) after I killed metacity about 2 or 3 times to get rid of those pesky systray adaptor windows that don't go away. Now that I've fixed the mapping and properly check extended styles for the tray icon flag, the icons dock perfectly. I ran my test app 100 times in a row, and it worked every time! :) I haven't had to kill metacity once (Honest!) I'll get the patches together and send an update. Thanks for all your help, James
Re: RFC: XEmbed Systray Patches
On Thu, 2006-08-10 at 00:16 +0100, Mike Hearn wrote: > > You can always stick some needed field in the X11DRV private data (you > can see how to access that in the code itself). Are you referring to x11drv_win_data? I looked at that, but it seems cleaner if I just use GetWindowLongW. I don't know if that's considered correct in this case (Sorry if it's a really dumb question, but I'm not sure on that.) > > Oh dear :( Maybe you should ping one of the GNOME guys? Yes, oh dear indeed. Maybe I will talk to the GNOME people about that. James
Re: RFC: XEmbed Systray Patches
On 8/9/06, James Liggett <[EMAIL PROTECTED]> wrote: way to handle it. What we need to do is get the extended style of the window, but this seems to involve a bunch of wineserver calls that I'm not familiar with. Can you help me out on that? Thanks! You can always stick some needed field in the X11DRV private data (you can see how to access that in the code itself). I'm not quite sure about that one--I've got an X test app that docks, but I can only get 1 pixel wide. :( On wine, this isn't a problem though. Still trying to figure out what I need to do. Oh dear :( Maybe you should ping one of the GNOME guys? thanks -mike
Re: RFC: XEmbed Systray Patches
On Wed, 2006-08-09 at 12:16 +0100, Mike Hearn wrote: > Ah awesome, good work! I know when I looked at this originally it > seemed you could set a flag that told the embedder whether it was > already mapped or not, maybe that doesn't work properly. It's not that it doesn't work properly. What happened is that the x11drv maps any visible window, so that and the mapping flag were conflicting with one another. In order for this to work, the systray window (our icon) can *never* be mapped by WINE at any point during its life, or the tray will have problems with it, the extent of which depends on how said tray implements XEmbed (which explains disparities between the behavior of GNOME, XFce, and IceWM with the patches.) IOW, the tray has to handle everything with mapping/unmapping, or you get races. On that subject, I've found that with some more tests, we're not quite out of the woods yet. I think the problem now is that WINE wants to map the window every time the tray becomes visible. I've thought about handling this in X11DRV_set_window_pos, but I'm not sure of the correct way to handle it. What we need to do is get the extended style of the window, but this seems to involve a bunch of wineserver calls that I'm not familiar with. Can you help me out on that? Thanks! > There was > also a problem where a window would dock but only be allocated 1 > pixel, I think that was a bug in GNOME that is now fixed. I'm not quite sure about that one--I've got an X test app that docks, but I can only get 1 pixel wide. :( On wine, this isn't a problem though. Still trying to figure out what I need to do. James
Re: RFC: XEmbed Systray Patches
On 8/9/06, James Liggett <[EMAIL PROTECTED]> wrote: Actually, I don't think it has to do with synchronization--I think it has to do with window mapping. Ah awesome, good work! I know when I looked at this originally it seemed you could set a flag that told the embedder whether it was already mapped or not, maybe that doesn't work properly. There was also a problem where a window would dock but only be allocated 1 pixel, I think that was a bug in GNOME that is now fixed. Hopefully we can nail this all finally. thanks -mike
Re: RFC: XEmbed Systray Patches
On Tue, 2006-08-08 at 13:48 +0100, Mike Hearn wrote: > > It's not that we need to slow it down - slowing something down is > never an acceptable solution to a race. We are missing some kind of > synchronisation somewhere, > Actually, I don't think it has to do with synchronization--I think it has to do with window mapping. Today I did some more research on the issue, and I came across the XEmbed spec, and there's a pretty interesting tidbit in there [1] (look under the XEMBED_MAPPED section.) The problem is that the systray window has to be unmapped so that when we dock, the embedder (the systray applet that holds the icon) has to map the client (the systray icon) itself--we can't do it in WINE or we risk having the race conditions that we do (where the window becomes visible as a child of the root window and not the tray, or sometimes both the root window and tray) So here's the solution that I'm currently using to solve it: don't call XMapWindow on systray icon windows. That combined with the mapped flag in the XEMBED_INFO array we set just before we dock the icon allows it to work properly. I've run my small test app over 50 times straight (probably more by now--I've lost count,) and it works. I'll do some more tests to make sure everything is good, and hone my solution a little before submitting. The problem with this is that we probably want mapping if we don't have a tray to send our icon to, so I'll be playing with getting that to work cleanly. James Notes: [1]: http://standards.freedesktop.org/xembed-spec/latest/ar01s04.html
Re: RFC: XEmbed Systray Patches
On 8/7/06, James Liggett <[EMAIL PROTECTED]> wrote: Turns out you're right, Mike. If I add a small (2 ms) sleep after the dock event is sent, things work perfectly. :) But, this really strikes me as a hack that doesn't stand a chance of getting into Wine. Is there a better way to slow down the execution of this code. It's not that we need to slow it down - slowing something down is never an acceptable solution to a race. We are missing some kind of synchronisation somewhere, or perhaps we are doing something funny to the window immediately after we try and dock it, which causes it to undock. You need to trace exactly what happens to that window after it's docked. thanks -mike
Re: RFC: XEmbed Systray Patches
On Mon, 2006-08-07 at 18:02 +0100, Mike Hearn wrote: > On Sun, 06 Aug 2006 20:10:19 -0700, James Liggett wrote: > > so I suspect that this has something to do with stack problem > > More likely it's a speed issue - logging slows the code down a lot which > could "fix" a race condition. X is sort of susceptible to this kind of > thing it seems. Turns out you're right, Mike. If I add a small (2 ms) sleep after the dock event is sent, things work perfectly. :) But, this really strikes me as a hack that doesn't stand a chance of getting into Wine. Is there a better way to slow down the execution of this code. Thanks. > > > >
Re: RFC: XEmbed Systray Patches
On Mon, 2006-08-07 at 18:02 +0100, Mike Hearn wrote: > On Sun, 06 Aug 2006 20:10:19 -0700, James Liggett wrote: > > so I suspect that this has something to do with stack problem > > More likely it's a speed issue - logging slows the code down a lot which > could "fix" a race condition. X is sort of susceptible to this kind of > thing it seems. Interesting thought...I knew that logging could slow down code, but I didn't think it could slow it down so much that it actually allowed the icon to dock. When I last tried working with these, I don't recall the trace working to solve this problem (I could be wrong though) Nonetheless, these patches seem to work better than the last ones, but I don't really understand why that is.
Re: RFC: XEmbed Systray Patches
On Sun, 06 Aug 2006 20:10:19 -0700, James Liggett wrote: > so I suspect that this has something to do with stack problem More likely it's a speed issue - logging slows the code down a lot which could "fix" a race condition. X is sort of susceptible to this kind of thing it seems.
RFC: XEmbed Systray Patches
Hello everyone, After a long hiatus, I'm trying again to get XEmbed systray support into Wine. I've been working on it for the past few days and I've got these patches. These are based on the system tray code from the Wine version used in the Linux port of Picasa [1] (which in turn was based on the work of Mike Hearn and Rob Shearman.) These patches are basically the same thing, with some fixes: 1. Use proper wine_tsx11 locking. 2. Clean up inappropriate C++ comments in that code. 3. Properly populated the XEvent structure to send docking messages. (According to the XEmbed systray spec [2], any unused fields in the message must be set to 0.) 4. Broken into five atomic patches against today's GIT. In my testing on Gnome 2.14, these patches dock much more reliabaly than the last ones, but they're still not quite perfect. In most cases, programs dock fine until about the seventh consecutive run (behavior observed with Steam and a small test program I wrote.) However, I noticed that it works perfectly if I enabled a +x11drv trace, so I suspect that this has something to do with stack problems, but I've been unable to find the source of the problem. I'd appreciate it if someone could give me some assistance on tracking down this issue. If you'd like to test these, please apply them in the order that they're numbered. Any comments/suggestions appreciated. Thnaks, James Liggett [1]: http://code.google.com/wine.html [2]: http://standards.freedesktop.org/systemtray-spec/systemtray-spec-0.2.html#messages xembed-systray-patches.tar.gz Description: application/compressed-tar
