Hi Mike, I could have replied earlier - your 4 line diff exploded into a huge diff to fix event handling (recall I said I was sure some other stuff was not right :)).
If I don't have a working re-write soon, I'll address just the bits you've pointed out separately and move on from there. Thank you for your detailed reports so far! Thanks, Okan On Thu, May 23, 2013 at 9:36 PM, Mike Small <[email protected]> wrote: > > Maybe a problem with my previous report was that the steps only gave a > reproducible case on macppc (and even that was surprising since the > uninitialized variable, wc.stack_mode, had to happen to pick up the > value 1 -- the value for Below -- though it consistently happened for me > and another user on daemonforums on our macppc machines). Here are > steps I can use to show the bug on an amd64 machine. This variation > should be more likely to be reproducible in general since any value > other than 1 for wc.stack_mode will yield incorrect behaviour. > > I) with emacs (this time 21.3 from the 5.3 cd, but that shouldn't matter): > 1. Place the emacs window above another window > 2. In the *Scratch* buffer evaluate the following expression: > (lower-frame) ; eval an expression by pressing ctrl-c, ctrl-e > ; after it or ctrl-meta-x in it > > The frame incorrectly stays on top. > > II) without emacs: > 1. compile the following program > ======================================================================== > #include <X11/Xlib.h> > #include <X11/Xutil.h> > > #include <err.h> > #include <stdlib.h> > > const char msg_lower[] = "Click to lower."; > const char msg_quit[] = "Q to quit."; > > int > main() > { > Display *dpy; > Screen *s; > Window win; > XEvent ev; > GC gc; > XGCValues values; > KeySym k; > unsigned long black; > > dpy = XOpenDisplay(NULL); > if (!dpy) > errx(1, "XOpenDpy: Couldn't open X dpy."); > s = DefaultScreenOfDisplay(dpy); > black = BlackPixelOfScreen(s); > > win = XCreateSimpleWindow(dpy, RootWindowOfScreen(s), > WidthOfScreen(s)/2 - 100, > HeightOfScreen(s)/2 - 100, > 180, 150, 2, > black, WhitePixelOfScreen(s)); > XSelectInput(dpy, win, ButtonPressMask|KeyReleaseMask|ExposureMask); > XMapWindow(dpy, win); > > values.font = XLoadFont(dpy, "fixed"); > gc = XCreateGC(dpy, win, GCFont, &values); > > while (1) { > XNextEvent(dpy, &ev); > switch (ev.type) { > case ButtonPress: > XLowerWindow(dpy, win); > break; > case KeyRelease: > k = XkbKeycodeToKeysym(dpy, ev.xkey.keycode, 0, 0); > if (k == XK_q || k == XK_Q) { > XCloseDisplay(dpy); > return 0; > } > break; > case Expose: > XDrawString(dpy, win, gc, 20, 20, msg_lower, > sizeof(msg_lower) - 1); > XDrawString(dpy, win, gc, 20, 75, msg_quit, > sizeof(msg_quit) - 1); > break; > } > } > return 1; > } > > ======================================================================== > 2. place its window above another window > 3. click on its window > > The window again incorrectly stays on top. > > After applying the following patch, I see the right behaviour on my > amd64 machine. The windows drop to the bottom of the stacking order in > each case. > > Index: xevents.c > =================================================================== > RCS file: /cvs/xenocara/app/cwm/xevents.c,v > retrieving revision 1.71 > diff -u -u -p -r1.71 xevents.c > --- xevents.c 5 Apr 2013 17:07:25 -0000 1.71 > +++ xevents.c 24 Apr 2013 02:49:00 -0000 > @@ -151,6 +151,10 @@ xev_handle_configurerequest(XEvent *ee) > cc->geom.y = e->y; > if (e->value_mask & CWBorderWidth) > wc.border_width = e->border_width; > + if (e->value_mask & CWSibling) > + wc.sibling = e->above; > + if (e->value_mask & CWStackMode) > + wc.stack_mode = e->detail; > > if (cc->geom.x == 0 && cc->geom.w >= sc->view.w) > cc->geom.x -= cc->bwidth; > > > dmesg for this other machine (acpi disabled to get it booting -- I'll > provide a proper report for that issue sometime): > > OpenBSD 5.3 (GENERIC) #53: Tue Mar 12 18:15:44 MDT 2013 > [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC > real mem = 1600815104 (1526MB) > avail mem = 1535799296 (1464MB) > mainbus0 at root > bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xfcd90 (44 entries) > bios0: vendor American Megatrends Inc. version "306" date 09/14/2007 > bios0: ASUSTeK Computer Inc. Z35FM > acpi at bios0 not configured > mpbios0 at bios0: Intel MP Specification 1.4 > cpu0 at mainbus0: apid 0 (boot processor) > cpu0: Intel(R) Core(TM)2 CPU T7200 @ 2.00GHz, 1995.32 MHz > cpu0: > FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,NXE,LONG,LAHF,PERF > cpu0: 4MB 64b/line 16-way L2 cache > cpu0: apic clock running at 166MHz > cpu at mainbus0: not configured > mpbios0: bus 0 is type PCI > mpbios0: bus 1 is type PCI > mpbios0: bus 2 is type PCI > mpbios0: bus 3 is type PCI > mpbios0: bus 4 is type PCI > mpbios0: bus 5 is type PCI > mpbios0: bus 6 is type ISA > ioapic0 at mainbus0: apid 2 pa 0xfec00000, version 20, 24 pins > cpu0: unknown Enhanced SpeedStep CPU, msr 0x06130c2806000c28 > cpu0: using only highest and lowest power states > cpu0: Enhanced SpeedStep 1995 MHz: speeds: 2000, 1000 MHz > pci0 at mainbus0 bus 0 > pchb0 at pci0 dev 0 function 0 "Intel 82945GM Host" rev 0x03 > vga1 at pci0 dev 2 function 0 "Intel 82945GM Video" rev 0x03 > wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation) > wsdisplay0: screen 1-5 added (80x25, vt100 emulation) > intagp0 at vga1 > agp0 at intagp0: aperture at 0xd0000000, size 0x10000000 > inteldrm0 at vga1: apic 2 int 16 > drm0 at inteldrm0 > "Intel 82945GM Video" rev 0x03 at pci0 dev 2 function 1 not configured > azalia0 at pci0 dev 27 function 0 "Intel 82801GB HD Audio" rev 0x02: msi > azalia0: codecs: Realtek ALC660, Motorola/0x3055, using Realtek ALC660 > audio0 at azalia0 > ppb0 at pci0 dev 28 function 0 "Intel 82801GB PCIE" rev 0x02: msi > pci1 at ppb0 bus 1 > ppb1 at pci0 dev 28 function 1 "Intel 82801GB PCIE" rev 0x02 > pci2 at ppb1 bus 4 > wpi0 at pci2 dev 0 function 0 "Intel PRO/Wireless 3945ABG" rev 0x02: msi, > MoW1, address 00:13:02:1a:b1:b4 > uhci0 at pci0 dev 29 function 0 "Intel 82801GB USB" rev 0x02: apic 2 int 23 > uhci1 at pci0 dev 29 function 1 "Intel 82801GB USB" rev 0x02: apic 2 int 19 > uhci2 at pci0 dev 29 function 2 "Intel 82801GB USB" rev 0x02: apic 2 int 18 > uhci3 at pci0 dev 29 function 3 "Intel 82801GB USB" rev 0x02: apic 2 int 16 > ehci0 at pci0 dev 29 function 7 "Intel 82801GB USB" rev 0x02: apic 2 int 23 > usb0 at ehci0: USB revision 2.0 > uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1 > ppb2 at pci0 dev 30 function 0 "Intel 82801BAM Hub-to-PCI" rev 0xe2 > pci3 at ppb2 bus 5 > "Ricoh 5C832 Firewire" rev 0x00 at pci3 dev 3 function 0 not configured > sdhc0 at pci3 dev 3 function 1 "Ricoh 5C822 SD/MMC" rev 0x19: apic 2 int 20 > sdmmc0 at sdhc0 > "Ricoh 5C843 MMC" rev 0x01 at pci3 dev 3 function 2 not configured > "Ricoh 5C592 Memory Stick" rev 0x0a at pci3 dev 3 function 3 not configured > "Ricoh 5C852 xD" rev 0x05 at pci3 dev 3 function 4 not configured > rl0 at pci3 dev 4 function 0 "Realtek 8139" rev 0x10: apic 2 int 20, address > 00:18:f3:cf:55:06 > rlphy0 at rl0 phy 0: RTL internal PHY > pcib0 at pci0 dev 31 function 0 "Intel 82801GBM LPC" rev 0x02 > pciide0 at pci0 dev 31 function 1 "Intel 82801GB IDE" rev 0x02: DMA, channel > 0 configured to compatibility, channel 1 configured to compatibility > wd0 at pciide0 channel 0 drive 0: <ST980825A> > wd0: 16-sector PIO, LBA48, 76319MB, 156301488 sectors > atapiscsi0 at pciide0 channel 0 drive 1 > scsibus0 at atapiscsi0: 2 targets > cd0 at scsibus0 targ 0 lun 0: <TSSTcorp, CD/DVDW TS-L632D, AS05> ATAPI > 5/cdrom removable > wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 5 > cd0(pciide0:0:1): using PIO mode 4, Ultra-DMA mode 2 > pciide0: channel 1 ignored (disabled) > usb1 at uhci0: USB revision 1.0 > uhub1 at usb1 "Intel UHCI root hub" rev 1.00/1.00 addr 1 > usb2 at uhci1: USB revision 1.0 > uhub2 at usb2 "Intel UHCI root hub" rev 1.00/1.00 addr 1 > usb3 at uhci2: USB revision 1.0 > uhub3 at usb3 "Intel UHCI root hub" rev 1.00/1.00 addr 1 > usb4 at uhci3: USB revision 1.0 > uhub4 at usb4 "Intel UHCI root hub" rev 1.00/1.00 addr 1 > isa0 at pcib0 > isadma0 at isa0 > pckbc0 at isa0 port 0x60/5 > pckbd0 at pckbc0 (kbd slot) > pckbc0: using irq 1 for kbd slot > wskbd0 at pckbd0: console keyboard, using wsdisplay0 > pms0 at pckbc0 (aux slot) > pckbc0: using irq 12 for aux slot > wsmouse0 at pms0 mux 0 > pms0: Synaptics touchpad, firmware 6.2 > pcppi0 at isa0 port 0x61 > spkr0 at pcppi0 > mtrr: Pentium Pro MTRR support > uhub5 at uhub2 port 1 "Chicony Generic USB Hub" rev 1.10/1.00 addr 2 > uhidev0 at uhub5 port 1 configuration 1 interface 0 "Chicony PFU-65 USB > Keyboard" rev 1.10/1.00 addr 3 > uhidev0: iclass 3/1 > ukbd0 at uhidev0: 8 variable keys, 6 key codes > wskbd1 at ukbd0 mux 1 > wskbd1: connecting to wsdisplay0 > uhidev1 at uhub5 port 3 configuration 1 interface 0 "Microsoft Microsoft > 3-Button Mouse with IntelliEye(TM)" rev 1.10/3.00 addr 4 > uhidev1: iclass 3/1 > ums0 at uhidev1: 3 buttons, Z dir > wsmouse1 at ums0 mux 0 > vscsi0 at root > scsibus1 at vscsi0: 256 targets > softraid0 at root > scsibus2 at softraid0: 256 targets > root on wd0a (72eb03b5e79429f1.a) swap on wd0b dump on wd0b > > > > > Mike Small <[email protected]> writes: > >> 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.
