Guess who :)
for those not in #blackbox:
rudimentary event compression: expose, pointer motion and configure
requests. see the attached diff...
removing the server grab stuff works rather well and makes things
snappier. see the attached diff.
However, I found a bug, and a pretty nasty one at that.
A change to BlackboxWindow::unmapNotifyEvent() does bad things:
1 - it no longer reparents client windows to root if the client itself
doesn't do the reparent. this is bad because if an application has a long
lived window that it just wants to hide, it unmaps it. BUT, if that app
doesn't have time to reparent the window before blackbox deletes the
associated BlackboxWindow, then the client window will be destroyed (since
it is a child of the blackbox window). Also, the current behavior is a
violation of the icccm because of this very reason.
the attached diff fixes this.
2 - reparentNotifyEvent() deletes the window, and when the function
returns to unmapNotifyEvent(), you use invalid data, and then double
delete. ( XFlush( display ) <- display is invalid after delete ).
your comment leads me to believe that you assume that deleting an object
in member functions means it won't return up the stack to a member method.
this is completely incorrect, the function stack is completely separate
from object management, so doing something like:
object::one() {
two();
}
object::two() {
three();
}
object::three() {
delete this;
}
when the calls get up to three, it deletes the object, and then returns to
object::two() {BUT the object, ie the 'this' pointer, is invalid and
shouldn't be used), and then returns to object::one() (same invalidity
applies).
the attached diff fixes this as well.
Since noone besides shaleh has CVS access, I'll have to wait for him to
commit this. In the meantime, others can try it out to see if there are
any side effects I couldn't see.
NOTE: the patch is against the version in CVS at sourceforge.
--
--
Bradley T. Hughes - bhughes at trolltech.com
Trolltech AS - Waldemar Thranes gt. 98 N-0175 Oslo, Norway
? Makefile
? blackbox
? compress-nograb-unmapfix.diff
Index: BaseDisplay.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/BaseDisplay.cc,v
retrieving revision 1.2
diff -u -b -r1.2 BaseDisplay.cc
--- BaseDisplay.cc 5 Feb 2002 22:02:18 -0000 1.2
+++ BaseDisplay.cc 11 Feb 2002 10:19:51 -0000
@@ -196,7 +196,6 @@
_startup = True;
_shutdown = False;
- server_grabs = 0;
last_bad_window = None;
::base_display = this;
@@ -493,20 +492,6 @@
}
return True;
-}
-
-
-void BaseDisplay::grab(void) {
- if (! server_grabs++)
- XGrabServer(display);
-}
-
-
-void BaseDisplay::ungrab(void) {
- if (! --server_grabs)
- XUngrabServer(display);
-
- if (server_grabs < 0) server_grabs = 0;
}
Index: BaseDisplay.hh
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/BaseDisplay.hh,v
retrieving revision 1.2
diff -u -b -r1.2 BaseDisplay.hh
--- BaseDisplay.hh 5 Feb 2002 22:02:18 -0000 1.2
+++ BaseDisplay.hh 11 Feb 2002 10:19:51 -0000
@@ -128,7 +128,7 @@
LinkedList<BTimer> *timerList;
char *display_name, *application_name;
- int number_of_screens, server_grabs, colors_per_channel;
+ int number_of_screens, colors_per_channel;
protected:
@@ -308,8 +308,6 @@
void ungrabButton(unsigned int button, unsigned int modifiers,
Window grab_window) const;
- void grab(void);
- void ungrab(void);
void eventLoop(void);
void addTimer(BTimer *);
void removeTimer(BTimer *);
Index: Image.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/Image.cc,v
retrieving revision 1.1.1.22
diff -u -b -r1.1.1.22 Image.cc
--- Image.cc 25 Jan 2002 10:13:57 -0000 1.1.1.22
+++ Image.cc 11 Feb 2002 10:19:51 -0000
@@ -1893,8 +1893,6 @@
colors[i].flags = DoRed|DoGreen|DoBlue;
}
- basedisplay->grab();
-
for (i = 0; i < ncolors; i++)
if (! XAllocColor(basedisplay->getXDisplay(), colormap, &colors[i])) {
fprintf(stderr, i18n->getMessage(ImageSet, ImageColorAllocFail,
@@ -1904,8 +1902,6 @@
} else
colors[i].flags = DoRed|DoGreen|DoBlue;
- basedisplay->ungrab();
-
XColor icolors[256];
int incolors = (((1 << screen_depth) > 256) ? 256 : (1 << screen_depth));
@@ -1989,7 +1985,6 @@
red_color_table[i] = green_color_table[i] = blue_color_table[i] =
i / bits;
- basedisplay->grab();
for (i = 0; i < ncolors; i++) {
colors[i].red = (i * 0xffff) / (colors_per_channel - 1);
colors[i].green = (i * 0xffff) / (colors_per_channel - 1);
@@ -2006,8 +2001,6 @@
colors[i].flags = DoRed|DoGreen|DoBlue;
}
- basedisplay->ungrab();
-
XColor icolors[256];
int incolors = (((1 << screen_depth) > 256) ? 256 :
(1 << screen_depth));
@@ -2298,8 +2291,6 @@
void BImageControl::installRootColormap(void) {
- basedisplay->grab();
-
Bool install = True;
int i = 0, ncmap = 0;
Colormap *cmaps =
@@ -2315,8 +2306,6 @@
XFree(cmaps);
}
-
- basedisplay->ungrab();
}
Index: Screen.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/Screen.cc,v
retrieving revision 1.2
diff -u -b -r1.2 Screen.cc
--- Screen.cc 5 Feb 2002 22:02:18 -0000 1.2
+++ Screen.cc 11 Feb 2002 10:19:52 -0000
@@ -2188,7 +2188,7 @@
void BScreen::shutdown(void) {
- blackbox->grab();
+ XGrabServer( getBaseDisplay()->getXDisplay() );
XSelectInput(getBaseDisplay()->getXDisplay(), getRootWindow(), NoEventMask);
XSync(getBaseDisplay()->getXDisplay(), False);
@@ -2206,7 +2206,7 @@
slit->shutdown();
#endif // SLIT
- blackbox->ungrab();
+ XUngrabServer( getBaseDisplay()->getXDisplay() );
}
Index: Slit.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/Slit.cc,v
retrieving revision 1.2
diff -u -b -r1.2 Slit.cc
--- Slit.cc 5 Feb 2002 22:02:18 -0000 1.2
+++ Slit.cc 11 Feb 2002 10:19:52 -0000
@@ -88,8 +88,6 @@
Slit::~Slit() {
- blackbox->grab();
-
if (timer->isTiming()) timer->stop();
delete timer;
@@ -101,14 +99,10 @@
blackbox->removeSlitSearch(frame.window);
XDestroyWindow(display, frame.window);
-
- blackbox->ungrab();
}
void Slit::addClient(Window w) {
- blackbox->grab();
-
if (blackbox->validateWindow(w)) {
SlitClient *client = new SlitClient;
client->client_window = w;
@@ -164,8 +158,6 @@
blackbox->saveSlitSearch(client->icon_window, this);
reconfigure();
}
-
- blackbox->ungrab();
}
@@ -193,8 +185,6 @@
void Slit::removeClient(Window w, Bool remap) {
- blackbox->grab();
-
Bool reconf = False;
LinkedListIterator<SlitClient> it(clientList);
@@ -208,8 +198,6 @@
}
if (reconf) reconfigure();
-
- blackbox->ungrab();
}
@@ -603,8 +591,6 @@
void Slit::configureRequestEvent(XConfigureRequestEvent *e) {
- blackbox->grab();
-
if (blackbox->validateWindow(e->window)) {
Bool reconf = False;
XWindowChanges xwc;
@@ -636,8 +622,6 @@
if (reconf) reconfigure();
}
-
- blackbox->ungrab();
}
Index: Toolbar.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/Toolbar.cc,v
retrieving revision 1.3
diff -u -b -r1.3 Toolbar.cc
--- Toolbar.cc 5 Feb 2002 23:56:38 -0000 1.3
+++ Toolbar.cc 11 Feb 2002 10:19:52 -0000
@@ -997,8 +997,6 @@
void Toolbar::keyPressEvent(XKeyEvent *ke) {
if (ke->window == frame.workspace_label && editing) {
- blackbox->grab();
-
if (! new_workspace_name) {
new_workspace_name = new char[128];
new_name_pos = 0;
@@ -1094,8 +1092,6 @@
screen->getWindowStyle()->l_text_focus_gc, x + tw, 0, 1,
frame.label_h - 1);
}
-
- blackbox->ungrab();
}
}
Index: Window.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/Window.cc,v
retrieving revision 1.3
diff -u -b -r1.3 Window.cc
--- Window.cc 11 Feb 2002 06:39:42 -0000 1.3
+++ Window.cc 11 Feb 2002 10:19:53 -0000
@@ -69,7 +69,6 @@
blackbox = b;
display = blackbox->getXDisplay();
- blackbox->grab();
if (! validateClient()) return;
// fetch client size and placement
@@ -83,7 +82,6 @@
"failed\n"));
#endif // DEBUG
- b->ungrab();
return;
}
@@ -99,7 +97,6 @@
RootWindowOfScreen(wattrib.screen));
#endif // DEBUG
- b->ungrab();
return;
}
}
@@ -171,7 +168,6 @@
screen->getSlit()->addClient(client.window);
delete this;
- b->ungrab();
return;
}
#endif // SLIT
@@ -301,8 +297,6 @@
}
setFocusFlag(False);
-
- blackbox->ungrab();
}
@@ -1344,7 +1338,6 @@
frame.y + frame.border_w, frame.width, frame.height);
}
- blackbox->grab();
if (! validateClient()) return False;
Bool ret = False;
@@ -1382,8 +1375,6 @@
ret = True;
}
- blackbox->ungrab();
-
return ret;
}
@@ -1705,7 +1696,6 @@
void BlackboxWindow::installColormap(Bool install) {
- blackbox->grab();
if (! validateClient()) return;
int i = 0, ncmap = 0;
@@ -1735,8 +1725,6 @@
XFree(cmaps);
}
-
- blackbox->ungrab();
}
@@ -1771,7 +1759,6 @@
&atom_return, &foo, &nitems, &ulfoo,
(unsigned char **) &state) != Success) ||
(! state)) {
- blackbox->ungrab();
return False;
}
@@ -2169,7 +2156,6 @@
client.window);
#endif // DEBUG
- blackbox->grab();
if (! validateClient()) return;
Bool get_state_ret = getState();
@@ -2199,8 +2185,6 @@
deiconify(False);
break;
}
-
- blackbox->ungrab();
}
}
@@ -2208,7 +2192,6 @@
void BlackboxWindow::mapNotifyEvent(XMapEvent *ne) {
if ((ne->window == client.window) && (! ne->override_redirect)
&& (flags.visible)) {
- blackbox->grab();
if (! validateClient()) return;
if (decorations.titlebar) positionButtons();
@@ -2224,8 +2207,6 @@
flags.visible = True;
flags.iconic = False;
-
- blackbox->ungrab();
}
}
@@ -2238,7 +2219,6 @@
client.window);
#endif // DEBUG
- blackbox->grab();
if (! validateClient()) return;
XChangeSaveSet(display, client.window, SetModeDelete);
@@ -2251,16 +2231,31 @@
XUnmapWindow(display, frame.window);
XUnmapWindow(display, client.window);
+ XSync( display, False );
+
XEvent ev;
if (! XCheckTypedWindowEvent(display, client.window, ReparentNotify,&ev)) {
+ // according to the ICCCM - if the client doesn't reparent to
+ // root, then we have to do it for them
+ restoreGravity();
+ XReparentWindow( display, client.window, screen->getRootWindow(),
+ client.x, client.y );
+
+ // this is incorrect. when calling 'delete this;', the function stack
+ // isn't invalidated, so we do indeed return to this function, and
+ // then operate on invalide data with the XFlush() and second
+ // 'delete this;'. THIS IS BAD!
+ //
+ // also, if the application doesn't happen to
+ // reparent it's window to root before we call reparentNotify, then
+ // we end up deleting the client windows - THIS IS BAD!
+ //
// calls 'delete this' so we never get past the next line
- reparentNotifyEvent(&(ev.xreparent));
+ // reparentNotifyEvent(&(ev.xreparent));
}
XFlush(display);
- blackbox->ungrab();
-
delete this;
}
}
@@ -2270,6 +2265,9 @@
if (de->window == client.window) {
XUnmapWindow(display, frame.window);
+ fprintf(stderr, "BlackboxWindow::destroyNotifyEvent: removing window "
+ "0x%lx\n", de->window );
+
delete this;
}
}
@@ -2292,7 +2290,6 @@
void BlackboxWindow::propertyNotifyEvent(Atom atom) {
- blackbox->grab();
if (! validateClient()) return;
switch(atom) {
@@ -2390,8 +2387,6 @@
break;
}
-
- blackbox->ungrab();
}
@@ -2409,7 +2404,6 @@
void BlackboxWindow::configureRequestEvent(XConfigureRequestEvent *cr) {
if (cr->window == client.window) {
- blackbox->grab();
if (! validateClient()) return;
int cx = frame.x, cy = frame.y;
@@ -2452,14 +2446,11 @@
break;
}
}
-
- blackbox->ungrab();
}
}
void BlackboxWindow::buttonPressEvent(XButtonEvent *be) {
- blackbox->grab();
if (! validateClient()) return;
if (frame.maximize_button == be->window) {
@@ -2546,13 +2537,10 @@
}
}
}
-
- blackbox->ungrab();
}
void BlackboxWindow::buttonReleaseEvent(XButtonEvent *re) {
- blackbox->grab();
if (! validateClient()) return;
if (re->window == frame.maximize_button) {
@@ -2583,9 +2571,9 @@
XDrawRectangle(display, screen->getRootWindow(), screen->getOpGC(),
frame.move_x, frame.move_y, frame.resize_w,
frame.resize_h);
+ XUngrabServer( display );
configure(frame.move_x, frame.move_y, frame.width, frame.height);
- blackbox->ungrab();
} else {
configure(frame.x, frame.y, frame.width, frame.height);
}
@@ -2595,6 +2583,7 @@
XDrawRectangle(display, screen->getRootWindow(), screen->getOpGC(),
frame.resize_x, frame.resize_y,
frame.resize_w, frame.resize_h);
+ XUngrabServer( display );
screen->hideGeometry();
@@ -2611,13 +2600,11 @@
frame.resize_w - (frame.border_w * 2),
frame.resize_h - (frame.border_w * 2));
- blackbox->ungrab();
XUngrabPointer(display, CurrentTime);
} else if (re->window == frame.window) {
if (re->button == 2 && re->state == Mod1Mask)
XUngrabPointer(display, CurrentTime);
}
- blackbox->ungrab();
}
@@ -2638,7 +2625,7 @@
blackbox->maskWindowEvents(client.window, this);
if (! screen->doOpaqueMove()) {
- blackbox->grab();
+ XGrabServer( display );
frame.move_x = frame.x;
frame.move_y = frame.y;
@@ -2714,6 +2701,7 @@
Bool left = (me->window == frame.left_grip);
if (! flags.resizing) {
+ XGrabServer( display );
XGrabPointer(display, me->window, False, ButtonMotionMask |
ButtonReleaseMask, GrabModeAsync, GrabModeAsync, None,
((left) ? blackbox->getLowerLeftAngleCursor() :
@@ -2722,8 +2710,6 @@
flags.resizing = True;
- blackbox->grab();
-
int gx, gy;
frame.grab_x = me->x - frame.border_w;
frame.grab_y = me->y - (frame.border_w * 2);
@@ -2779,7 +2765,6 @@
void BlackboxWindow::shapeEvent(XShapeEvent *) {
if (blackbox->hasShapeExtensions()) {
if (flags.shaped) {
- blackbox->grab();
if (! validateClient()) return;
XShapeCombineShape(display, frame.window, ShapeBounding,
frame.mwm_border_w, frame.y_border +
@@ -2802,7 +2787,6 @@
XShapeCombineRectangles(display, frame.window, ShapeBounding, 0, 0,
xrect, num, ShapeUnion, Unsorted);
- blackbox->ungrab();
}
}
}
@@ -2816,7 +2800,6 @@
if (XCheckTypedWindowEvent(display, client.window, DestroyNotify, &e) ||
XCheckTypedWindowEvent(display, client.window, UnmapNotify, &e)) {
XPutBackEvent(display, &e);
- blackbox->ungrab();
return False;
}
Index: Workspace.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/Workspace.cc,v
retrieving revision 1.2
diff -u -b -r1.2 Workspace.cc
--- Workspace.cc 5 Feb 2002 22:02:18 -0000 1.2
+++ Workspace.cc 11 Feb 2002 10:19:53 -0000
@@ -243,12 +243,8 @@
win = win->getTransientFor();
}
- screen->getBlackbox()->grab();
-
XLowerWindow(screen->getBaseDisplay()->getXDisplay(), *nstack);
XRestackWindows(screen->getBaseDisplay()->getXDisplay(), nstack, i);
-
- screen->getBlackbox()->ungrab();
delete [] nstack;
}
Index: blackbox.cc
===================================================================
RCS file: /cvsroot/blackboxwm/blackbox/src/blackbox.cc,v
retrieving revision 1.2
diff -u -b -r1.2 blackbox.cc
--- blackbox.cc 11 Feb 2002 06:39:42 -0000 1.2
+++ blackbox.cc 11 Feb 2002 10:19:54 -0000
@@ -110,6 +110,8 @@
# include <libgen.h>
#endif // HAVE_LIBGEN_H
+#include <algorithm>
+
#ifndef HAVE_BASENAME
static inline char *basename (char *s) {
char *save = s;
@@ -145,9 +147,8 @@
Blackbox::Blackbox(int m_argc, char **m_argv, char *dpy_name, char *rc)
- : BaseDisplay(m_argv[0], dpy_name) {
- grab();
-
+ : BaseDisplay(m_argv[0], dpy_name)
+{
if (! XSupportsLocale())
fprintf(stderr, "X server does not support locale\n");
@@ -213,8 +214,6 @@
timer = new BTimer(this, this);
timer->setTimeout(0);
timer->fireOnce(True);
-
- ungrab();
}
@@ -254,14 +253,6 @@
void Blackbox::process_event(XEvent *e) {
- if ((masked == e->xany.window) && masked_window &&
- (e->type == MotionNotify)) {
- last_time = e->xmotion.time;
- masked_window->motionNotifyEvent(&e->xmotion);
-
- return;
- }
-
switch (e->type) {
case ButtonPress: {
// strip the lock key modifiers
@@ -388,6 +379,17 @@
}
case ConfigureRequest: {
+ // compress configure requests...
+ XEvent realevent;
+ int i = 0;
+ while( XCheckTypedWindowEvent( getXDisplay(),
+ e->xconfigurerequest.window,
+ ConfigureRequest, &realevent ) )
+ i++;
+ if ( i > 0 )
+ e = &realevent;
+ // fprintf( stderr, "compressed %d configure requests\n", i );
+
BlackboxWindow *win = (BlackboxWindow *) 0;
#ifdef SLIT
@@ -403,8 +405,6 @@
#endif // SLIT
} else {
- grab();
-
if (validateWindow(e->xconfigurerequest.window)) {
XWindowChanges xwc;
@@ -419,8 +419,6 @@
XConfigureWindow(getXDisplay(), e->xconfigurerequest.window,
e->xconfigurerequest.value_mask, &xwc);
}
-
- ungrab();
}
break;
@@ -512,6 +510,17 @@
}
case MotionNotify: {
+ // motion notify compression...
+ XEvent realevent;
+ int i = 0;
+ while ( XCheckTypedWindowEvent( getXDisplay(), e->xmotion.window,
+ MotionNotify, &realevent ) )
+ i++;
+ if ( i > 0 )
+ // if we have compressed some motion events, use the last one
+ e = &realevent;
+ // fprintf( stderr, "compressed %d motion notifies\n", i );
+
// strip the lock key modifiers
e->xbutton.state &= ~(NumLockMask | ScrollLockMask | LockMask);
@@ -567,13 +576,9 @@
} else if ((win = searchWindow(e->xcrossing.window))) {
if (win->getScreen()->isSloppyFocus() &&
(! win->isFocused()) && (! no_focus)) {
- grab();
-
if (((! sa.leave) || sa.inferior) && win->isVisible() &&
win->setInputFocus())
win->installColormap(True);
-
- ungrab();
}
} else if ((menu = searchMenu(e->xcrossing.window))) {
menu->enterNotifyEvent(&e->xcrossing);
@@ -613,6 +618,35 @@
}
case Expose: {
+ // compress expose events
+ XEvent realevent;
+ int i = 0, ex1, ey1, ex2, ey2;
+ ex1 = e->xexpose.x;
+ ey1 = e->xexpose.y;
+ ex2 = ex1 + e->xexpose.width - 1;
+ ey2 = ey1 + e->xexpose.height - 1;
+ while ( XCheckTypedWindowEvent( getXDisplay(), e->xexpose.window,
+ Expose, &realevent ) ) {
+ i++;
+
+ // merge expose area
+ ex1 = std::min( realevent.xexpose.x, ex1 );
+ ey1 = std::min( realevent.xexpose.y, ey1 );
+ ex2 = std::max( realevent.xexpose.x +
+ realevent.xexpose.width - 1, ex2 );
+ ey2 = std::max( realevent.xexpose.y +
+ realevent.xexpose.height - 1, ey2 );
+ }
+ if ( i > 0 )
+ e = &realevent;
+ // fprintf( stderr, "compressed %d expose events\n", i );
+
+ // use the merged area
+ e->xexpose.x = ex1;
+ e->xexpose.y = ey1;
+ e->xexpose.width = ex2 - ex1 + 1;
+ e->xexpose.height = ey2 - ey1 + 1;
+
BlackboxWindow *win = (BlackboxWindow *) 0;
Basemenu *menu = (Basemenu *) 0;
Toolbar *tbar = (Toolbar *) 0;
@@ -1623,8 +1657,6 @@
void Blackbox::real_reconfigure(void) {
- grab();
-
XrmDatabase new_blackboxrc = (XrmDatabase) 0;
char style[MAXPATHLEN + 64];
@@ -1664,8 +1696,6 @@
for (BScreen *screen = it.current(); screen; it++, screen = it.current()) {
screen->reconfigure();
}
-
- ungrab();
}
