Okan Demirmen <[email protected]> writes:
> On Thu 2013.05.02 at 00:22 -0400, [email protected] wrote:
> Hi,
>
> First off, thanks for the detailed report!
>
>> While using emacs org mode and entering a timestamp using [C-c !] the
>> emacs window would drop to the bottom of the X stacking order. This
>> command can be traced to the raise-frame function which sends a raise
>> window request to cwm.
>>
>> It happened consistently for me, but when I asked another user on
>> daemonforums to try it he found it only happened on his macppc and not
>> his i386 machine. Also, I had it go away briefly when I fumbled in
>> getting debugging information (unstripped but no -g or maybe a mix of
>> object files with -g and without, not sure now). I mention this in
>> case you don't see the problem yourself. There's an uninitialized auto
>> involved, which I believe to be the cause of the pseudo-intermittency.
>>
>> With debugging info I could step through
>> xevents.c::xev_handle_configurerequest
>> and see the following:
>>
>> 1. The call to XConfigureWindow reuses the value mask from the
>> incoming XConfigureReqestEvent.
>> 2. The value of that mask was 64. i.e. CWStackMode (defined as 1 << 6 in X.h)
>> 3. CWStackMode is not one of the bits looked for in the preceding
>> check masks and set corresponding wc field statements.
>> 4. Not involved in my scenario I don't think, but CWSibling is also missing.
>> 5. The XWindowChanges variable wc used for the XConfigureWindow call
>> happens to have the value 1 for its stack_mode field for me. 1 is
>> defined as Below in X.h. Above is 0.
>>
>> When I add checks for CWStackMode and CWSibling with appropriate sets
>> of the corresponding wc fields (excuse the pun) the problem seems
>> fixed. If it's on top it stays on top. If it's lower it raises.
>
> Unfortunately, I'm not able to replicate, nor I suppose understand, the
> current behavior wrt to the behavior with the patch applied - basically,
> there's no change in my tests. I could be missing your point...
Sorry I was unclear. Annotations follow (excuse me if this is gets too
verbose or if I state obvious things)...
static void
xev_handle_configurerequest(XEvent *ee)
{
XConfigureRequestEvent *e = &ee->xconfigurerequest;
struct client_ctx *cc;
struct screen_ctx *sc;
-> wc is not initialized here (of course, this isn't C++), particularly the
-> .sibling and .stack_mode fields. As matters later, these fields
-> correspond, I believe (I have an Xlib book on order -- the X man pages
-> leave me guessing slightly on this point), to the .above and .detail
-> fields in XConfigureRequestEvent. See XConfigureRequestEvent(3) and
-> XConfigureWindow(3). This latter page has problems you can see below,
-> btw. I intend to report this (to OpenBSD? to Xorg?) later along with
-> another man page problem but don't have my notes handy for that.
XWindowChanges wc;
if ((cc = client_find(e->window)) != NULL) {
sc = cc->sc;
if (e->value_mask & CWWidth)
cc->geom.w = e->width;
if (e->value_mask & CWHeight)
cc->geom.h = e->height;
if (e->value_mask & CWX)
cc->geom.x = e->x;
if (e->value_mask & CWY)
cc->geom.y = e->y;
-> This if block is useless since wc.border_width is re-assigned below.
if (e->value_mask & CWBorderWidth)
wc.border_width = e->border_width;
if (cc->geom.x == 0 && cc->geom.w >= sc->view.w)
cc->geom.x -= cc->bwidth;
if (cc->geom.y == 0 && cc->geom.h >= sc->view.h)
cc->geom.y -= cc->bwidth;
-> Next is where wc's members get their values, but two of them
-> (namely .sibling and .stack_mode) are not set so will have fairly
unpredictable
-> values. No doubt when you run you happen to get a zero for
-> wc.stacking_mode, which corresponds to Above. I get 1, corresponding to
-> Below. If you try my steps again, but instead put my test program window
-> on the bottom and click on a corner hanging out from under, my guess is
-> that you'll see nothing happen. But if you look at my code you can see
-> that clicking on the window should make it request a raise in the
-> stacking order.
->
-> From XConfigureWindow(3):
->
-> /* Configure window value mask bits */
->
-> #define .el CWX (1<<0) [[what's this .el doing
here?]]
-> #define .el CWY (1<<1)
-> #define .el CWWidth (1<<2)
-> #define .el CWHeight (1<<3)
-> #define .el CWBorderWidth (1<<4)
-> #define .el CWSibling (1<<5)
-> #define .el CWStackMode (1<<6)
-> /* Values */
->
-> typedef struct {
-> int x, y;
-> int width, height;
-> int border_width;
-> Window sibling;
-> int stack_mode;
-> } XWindowChanges;
->
-> From X.h:
-> #define Above 0
-> #define Below 1
-> #define TopIf 2
-> #define BottomIf 3
-> #define Opposite 4
->
wc.x = cc->geom.x;
wc.y = cc->geom.y;
wc.width = cc->geom.w;
wc.height = cc->geom.h;
wc.border_width = cc->bwidth;
-> Next we're re-using the e->value_mask that came in with the client's
-> configure request for our outgoing configure request. But e->value_mask
-> was set to tell us what we can look at on ee->xconfigurerequest.
-> XConfigureWindow needs a mask to say which fields we set on wc. The set
-> of fields we assigned on wc is not the same as the set of fields that
-> were assigned on e by the client's request (wc lacks sibling and stack_mode,
-> or rather has not very predictable values for those since we didn't
-> assign to them). When I looked in the debugger I could see that
-> e->value_mask was 64, a.k.a. 1<<6, a.k.a. CWStackMode, and that wc.stack_mode
-> happened to be 1 somehow. The result was to send a stacking order
-> request with this uninitialized value, 1 or Below, for wc.stack_mode
-> and sending emacs or my test program to the bottom. You may have 0
-> (Above) sending your window to the top. If you start with it at the top,
-> you see no action.
->
XConfigureWindow(X_Dpy, cc->win, e->value_mask, &wc);
xu_configure(cc);
} else {
/* let it do what it wants, it'll be ours when we map it. */
wc.x = e->x;
wc.y = e->y;
wc.width = e->width;
wc.height = e->height;
wc.border_width = e->border_width;
wc.stack_mode = Above;
e->value_mask &= ~CWStackMode;
XConfigureWindow(X_Dpy, e->window, e->value_mask, &wc);
...
I hope the problem is clearer now. If not, let me know and I'll try
again. I often struggle to make myself understood it seems.
So if it is clear, then about my patch. I thought it might be too bold
to say so initially, but I don't think re-using e->value_mask in
XConfigureWindow is wise here. If the changes structure ever gets new
members then we're right back to the same problem. I'd think it better
to dedicate a separate mask variable for XConfigureWindow's use that
gets its bits set as the corresponding changes struct fields are
set. e.g. something like this (untested)...
...
unsigned long wc_values_mask;
...
wc_values_mask = 0
...
if (e->value_mask & CWSibling) {
wc.sibling = e->above;
wc_values_mask |= CWSibling;
}
if (e->value_mask & CWStackMode) {
wc.stack_mode = e->detail;
wc_values_mask |= CWStackMode;
}
wc.x = cc->geom.x;
wc.y = cc->geom.y;
wc.width = cc->geom.w;
wc.height = cc->geom.h;
wc.border_width = cc->bwidth;
wc_values_mask |= CWX | CWY | CWWidth | CWHeight | CWBorderWidth;
XConfigureWindow(X_Dpy, e->window, wc_values_mask, &wc);
But I'm not an Xlib programmer, so I have no idea if it's a realistic
expectation for XConfigureRequestEvent and XWindowChanges to ever
change.
--
Mike Small
[email protected]
