Commit c6b475b2 authored by Havoc Pennington's avatar Havoc Pennington Committed by Havoc Pennington
Browse files

Optimizations for managing new windows (do not all take effect if

2002-10-21  Havoc Pennington  <hp@redhat.com>

	Optimizations for managing new windows (do not all take effect if
	METACITY_DEBUG=1). Bug #96404

	* src/keybindings.c (meta_change_keygrab): use error trap nesting
	and conditionalize on meta_is_verbose() to avoid a ton of XSync

	* src/display.c (meta_change_button_grab): ditto

	Throughout: move to new error trap setup to save on XSync calls,
	new setup is:

	* src/errors.c (meta_error_trap_push_with_return): new function,
	an error trap that needs to care about return value and thus
	sync even if an outer trap still exists
	(meta_error_trap_pop_with_return): new function
	(meta_error_trap_pop): add "last_request_was_roundtrip"
	argument allowing us to avoid XSync() if we just did
	a GetProperty or whatever.

	* src/util.c (meta_warning): flush the warning file descriptor

	* src/Makefile.am (INCLUDES): define G_LOG_DOMAIN
parent 37a1e6f5
2002-10-21 Havoc Pennington <hp@redhat.com>
Optimizations for managing new windows (do not all take effect if
METACITY_DEBUG=1). Bug #96404
* src/keybindings.c (meta_change_keygrab): use error trap nesting
and conditionalize on meta_is_verbose() to avoid a ton of XSync
* src/display.c (meta_change_button_grab): ditto
Throughout: move to new error trap setup to save on XSync calls,
new setup is:
* src/errors.c (meta_error_trap_push_with_return): new function,
an error trap that needs to care about return value and thus
sync even if an outer trap still exists
(meta_error_trap_pop_with_return): new function
(meta_error_trap_pop): add "last_request_was_roundtrip"
argument allowing us to avoid XSync() if we just did
a GetProperty or whatever.
* src/util.c (meta_warning): flush the warning file descriptor
* src/Makefile.am (INCLUDES): define G_LOG_DOMAIN
2002-10-20 Havoc Pennington <hp@pobox.com>
* src/ui.c (meta_image_window_new): put multihead stuff in
......
......@@ -3,3 +3,4 @@ Focus windows on map: see http://bugzilla.gnome.org/show_bug.cgi?id=82921
Keep panel always on top: http://bugzilla.gnome.org/show_bug.cgi?id=81551
Edge flipping: http://bugzilla.gnome.org/show_bug.cgi?id=82917
Opaque resize: http://bugzilla.gnome.org/show_bug.cgi?id=92618
Super+click to resize: http://bugzilla.gnome.org/show_bug.cgi?id=79315
SUBDIRS=wm-tester tools themes
INCLUDES=@METACITY_CFLAGS@ -DMETACITY_LIBEXECDIR=\"$(libexecdir)\" -DHOST_ALIAS=\"@HOST_ALIAS@\" -DMETACITY_LOCALEDIR=\"$(prefix)/@DATADIRNAME@/locale\" -DMETACITY_PKGDATADIR=\"$(pkgdatadir)\" -DMETACITY_DATADIR=\"$(datadir)\"
INCLUDES=@METACITY_CFLAGS@ -DMETACITY_LIBEXECDIR=\"$(libexecdir)\" -DHOST_ALIAS=\"@HOST_ALIAS@\" -DMETACITY_LOCALEDIR=\"$(prefix)/@DATADIRNAME@/locale\" -DMETACITY_PKGDATADIR=\"$(pkgdatadir)\" -DMETACITY_DATADIR=\"$(datadir)\" -DG_LOG_DOMAIN=\"metacity\"
EGGFILES= \
eggaccelerators.c \
......
......@@ -371,7 +371,7 @@ meta_window_delete (MetaWindow *window,
window->desc);
XKillClient (window->display->xdisplay, window->xwindow);
}
meta_error_trap_pop (window->display);
meta_error_trap_pop (window->display, FALSE);
meta_display_ping_window (window->display,
window,
......@@ -438,7 +438,7 @@ meta_window_kill (MetaWindow *window)
window->desc);
meta_error_trap_push (window->display);
XKillClient (window->display->xdisplay, window->xwindow);
meta_error_trap_pop (window->display);
meta_error_trap_pop (window->display, FALSE);
}
void
......
......@@ -84,7 +84,7 @@ static void update_window_grab_modifiers (MetaDisplay *display);
static void prefs_changed_callback (MetaPreference pref,
void *data);
static int
static void
set_utf8_string_hint (MetaDisplay *display,
Window xwindow,
Atom atom,
......@@ -95,7 +95,7 @@ set_utf8_string_hint (MetaDisplay *display,
xwindow, atom,
display->atom_utf8_string,
8, PropModeReplace, (guchar*) val, strlen (val));
return meta_error_trap_pop (display);
meta_error_trap_pop (display, FALSE);
}
static void
......@@ -258,6 +258,7 @@ meta_display_open (const char *name)
*/
display->name = g_strdup (XDisplayName (name));
display->xdisplay = xdisplay;
display->error_trap_synced_at_last_pop = TRUE;
display->error_traps = 0;
display->error_trap_handler = NULL;
display->server_grab_count = 0;
......@@ -497,7 +498,7 @@ meta_display_open (const char *name)
/* Force a new FocusIn (does this work?) */
XSetInputFocus (display->xdisplay, focus, ret_to, CurrentTime);
meta_error_trap_pop (display);
meta_error_trap_pop (display, FALSE);
}
meta_display_ungrab (display);
......@@ -667,10 +668,18 @@ meta_display_screen_for_xwindow (MetaDisplay *display,
Window xwindow)
{
XWindowAttributes attr;
int result;
meta_error_trap_push (display);
XGetWindowAttributes (display->xdisplay, xwindow, &attr);
if (meta_error_trap_pop (display) != Success)
attr.screen = NULL;
result = XGetWindowAttributes (display->xdisplay, xwindow, &attr);
meta_error_trap_pop (display, TRUE);
/* Note, XGetWindowAttributes is on all kinds of crack
* and returns 1 on success 0 on failure, rather than Success
* on success.
*/
if (result == 0 || attr.screen == NULL)
return NULL;
return meta_display_screen_for_x_screen (display, attr.screen);
......@@ -930,7 +939,7 @@ window_raise_with_delay_callback (void *data)
window->xwindow,
&root, &child,
&root_x, &root_y, &x, &y, &mask);
meta_error_trap_pop (window->display);
meta_error_trap_pop (window->display, TRUE);
if ((window->frame && POINT_IN_RECT (root_x, root_y, window->frame->rect)) ||
(window->frame == NULL && POINT_IN_RECT (root_x, root_y, window->rect)))
......@@ -1340,7 +1349,7 @@ event_callback (XEvent *event,
window->frame->xwindow);
meta_error_trap_push (display);
meta_window_destroy_frame (window->frame->window);
meta_error_trap_pop (display);
meta_error_trap_pop (display, FALSE);
}
else
{
......@@ -1452,7 +1461,7 @@ event_callback (XEvent *event,
meta_error_trap_push (display);
XConfigureWindow (display->xdisplay, event->xconfigurerequest.window,
xwcm, &xwc);
meta_error_trap_pop (display);
meta_error_trap_pop (display, FALSE);
}
else
{
......@@ -2043,7 +2052,7 @@ meta_spew_event (MetaDisplay *display,
meta_error_trap_push (display);
str = XGetAtomName (display->xdisplay,
event->xproperty.atom);
meta_error_trap_pop (display);
meta_error_trap_pop (display, TRUE);
if (event->xproperty.state == PropertyNewValue)
state = "PropertyNewValue";
......@@ -2077,7 +2086,7 @@ meta_spew_event (MetaDisplay *display,
meta_error_trap_push (display);
str = XGetAtomName (display->xdisplay,
event->xclient.message_type);
meta_error_trap_pop (display);
meta_error_trap_pop (display, TRUE);
extra = g_strdup_printf ("type: %s format: %d\n",
str ? str : "(unknown atom)",
event->xclient.format);
......@@ -2266,10 +2275,9 @@ meta_display_set_grab_op_cursor (MetaDisplay *display,
#define GRAB_MASK (PointerMotionMask | PointerMotionHintMask | \
ButtonPressMask | ButtonReleaseMask)
meta_error_trap_push (display);
if (change_pointer)
{
meta_error_trap_push_with_return (display);
XChangeActivePointerGrab (display->xdisplay,
GRAB_MASK,
cursor,
......@@ -2277,11 +2285,20 @@ meta_display_set_grab_op_cursor (MetaDisplay *display,
meta_topic (META_DEBUG_WINDOW_OPS,
"Changed pointer with XChangeActivePointerGrab()\n");
if (meta_error_trap_pop_with_return (display, FALSE) != Success)
{
meta_topic (META_DEBUG_WINDOW_OPS,
"Error trapped from XChangeActivePointerGrab()\n");
if (display->grab_have_pointer)
display->grab_have_pointer = FALSE;
}
}
else
{
g_assert (screen != NULL);
meta_error_trap_push (display);
if (XGrabPointer (display->xdisplay,
grab_xwindow,
False,
......@@ -2295,15 +2312,9 @@ meta_display_set_grab_op_cursor (MetaDisplay *display,
meta_topic (META_DEBUG_WINDOW_OPS,
"XGrabPointer() returned GrabSuccess\n");
}
}
if (meta_error_trap_pop (display) != Success)
{
meta_topic (META_DEBUG_WINDOW_OPS,
"Error trapped from XGrabPointer()\n");
if (display->grab_have_pointer)
display->grab_have_pointer = FALSE;
meta_error_trap_pop (display, TRUE);
}
#undef GRAB_MASK
if (cursor != None)
......@@ -2501,12 +2512,12 @@ meta_change_button_grab (MetaDisplay *display,
int modmask)
{
int ignored_mask;
meta_error_trap_push (display);
ignored_mask = 0;
while (ignored_mask < (int) display->ignored_modifier_mask)
{
int result;
if (ignored_mask & ~(display->ignored_modifier_mask))
{
/* Not a combination of ignored modifiers
......@@ -2515,8 +2526,10 @@ meta_change_button_grab (MetaDisplay *display,
++ignored_mask;
continue;
}
meta_error_trap_push (display);
if (meta_is_debugging ())
meta_error_trap_push_with_return (display);
if (grab)
XGrabButton (display->xdisplay, button, modmask | ignored_mask,
xwindow, False,
......@@ -2528,15 +2541,23 @@ meta_change_button_grab (MetaDisplay *display,
else
XUngrabButton (display->xdisplay, button, modmask | ignored_mask,
xwindow);
result = meta_error_trap_pop (display);
if (result != Success)
meta_verbose ("Failed to grab button %d with mask 0x%x for window 0x%lx error code %d\n",
button, modmask | ignored_mask, xwindow, result);
if (meta_is_debugging ())
{
int result;
result = meta_error_trap_pop_with_return (display, FALSE);
if (result != Success)
meta_verbose ("Failed to %s button %d with mask 0x%x for window 0x%lx error code %d\n",
grab ? "grab" : "ungrab",
button, modmask | ignored_mask, xwindow, result);
}
++ignored_mask;
}
meta_error_trap_pop (display, FALSE);
}
void
......@@ -2681,7 +2702,7 @@ meta_display_update_active_window_hint (MetaDisplay *display)
display->atom_net_active_window,
XA_WINDOW,
32, PropModeReplace, (guchar*) data, 2);
meta_error_trap_pop (display);
meta_error_trap_pop (display, FALSE);
tmp = tmp->next;
}
......@@ -3191,7 +3212,7 @@ convert_property (MetaDisplay *display,
conversion_targets[2] = display->atom_timestamp;
conversion_targets[3] = display->atom_version;
meta_error_trap_push (display);
meta_error_trap_push_with_return (display);
if (target == display->atom_targets)
XChangeProperty (display->xdisplay, w, property,
XA_ATOM, 32, PropModeReplace,
......@@ -3206,16 +3227,17 @@ convert_property (MetaDisplay *display,
(unsigned char *)icccm_version, 2);
else
{
meta_error_trap_pop (display);
meta_error_trap_pop_with_return (display, FALSE);
return FALSE;
}
if (meta_error_trap_pop (display) != Success)
if (meta_error_trap_pop_with_return (display, FALSE) != Success)
return FALSE;
/* Be sure the PropertyNotify has arrived so we
* can send SelectionNotify
*/
/* FIXME the error trap pop synced anyway, right? */
XSync (display->xdisplay, False);
return TRUE;
......@@ -3240,7 +3262,7 @@ process_selection_request (MetaDisplay *display,
meta_error_trap_push (display);
str = XGetAtomName (display->xdisplay,
event->xselectionrequest.selection);
meta_error_trap_pop (display);
meta_error_trap_pop (display, TRUE);
meta_verbose ("Selection request with selection %s window 0x%lx not a WM_Sn selection we recognize\n",
str ? str : "(bad atom)", event->xselectionrequest.owner);
......@@ -3267,13 +3289,18 @@ process_selection_request (MetaDisplay *display,
unsigned long num, rest;
unsigned char *data;
meta_error_trap_push (display);
XGetWindowProperty (display->xdisplay,
event->xselectionrequest.requestor,
event->xselectionrequest.property, 0, 256, False,
display->atom_atom_pair,
&type, &format, &num, &rest, &data);
if (meta_error_trap_pop (display) == Success)
meta_error_trap_push_with_return (display);
if (XGetWindowProperty (display->xdisplay,
event->xselectionrequest.requestor,
event->xselectionrequest.property, 0, 256, False,
display->atom_atom_pair,
&type, &format, &num, &rest, &data) != Success)
{
meta_error_trap_pop_with_return (display, TRUE);
return;
}
if (meta_error_trap_pop_with_return (display, TRUE) == Success)
{
/* FIXME: to be 100% correct, should deal with rest > 0,
* but since we have 4 possible targets, we will hardly ever
......@@ -3296,7 +3323,7 @@ process_selection_request (MetaDisplay *display,
event->xselectionrequest.property,
display->atom_atom_pair,
32, PropModeReplace, data, num);
meta_error_trap_pop (display);
meta_error_trap_pop (display, FALSE);
meta_XFree (data);
}
}
......@@ -3350,7 +3377,7 @@ process_selection_clear (MetaDisplay *display,
meta_error_trap_push (display);
str = XGetAtomName (display->xdisplay,
event->xselectionclear.selection);
meta_error_trap_pop (display);
meta_error_trap_pop (display, TRUE);
meta_verbose ("Selection clear with selection %s window 0x%lx not a WM_Sn selection we recognize\n",
str ? str : "(bad atom)", event->xselectionclear.window);
......
......@@ -167,6 +167,7 @@ struct _MetaDisplay
guint static_gravity_works : 1;
/*< private-ish >*/
guint error_trap_synced_at_last_pop : 1;
MetaEventQueue *events;
GSList *screens;
GHashTable *window_ids;
......
......@@ -26,6 +26,8 @@
#include <stdlib.h>
#include <gdk/gdk.h>
static int sync_count = 0;
static int x_error_handler (Display *display,
XErrorEvent *error);
static int x_io_error_handler (Display *display);
......@@ -37,13 +39,20 @@ meta_errors_init (void)
XSetIOErrorHandler (x_io_error_handler);
}
void
meta_error_trap_push (MetaDisplay *display)
static void
meta_error_trap_push_internal (MetaDisplay *display,
gboolean need_sync)
{
/* GDK resets the error handler on each push */
int (* old_error_handler) (Display *,
XErrorEvent *);
if (need_sync)
{
XSync (display->xdisplay, False);
++sync_count;
}
gdk_error_trap_push ();
/* old_error_handler will just be equal to x_error_handler
......@@ -60,17 +69,23 @@ meta_error_trap_push (MetaDisplay *display)
}
display->error_traps += 1;
meta_topic (META_DEBUG_ERRORS, "%d traps remain\n", display->error_traps);
}
int
meta_error_trap_pop (MetaDisplay *display)
static int
meta_error_trap_pop_internal (MetaDisplay *display,
gboolean need_sync)
{
int result;
g_assert (display->error_traps > 0);
/* just use GDK trap, but we do the sync since GDK doesn't */
XSync (display->xdisplay, False);
if (need_sync)
{
XSync (display->xdisplay, False);
++sync_count;
}
result = gdk_error_trap_pop ();
......@@ -92,9 +107,75 @@ meta_error_trap_pop (MetaDisplay *display)
display->error_trap_handler = NULL;
}
meta_topic (META_DEBUG_ERRORS, "%d traps\n", display->error_traps);
return result;
}
void
meta_error_trap_push (MetaDisplay *display)
{
meta_error_trap_push_internal (display, FALSE);
}
void
meta_error_trap_pop (MetaDisplay *display,
gboolean last_request_was_roundtrip)
{
gboolean need_sync;
/* we only have to sync when popping the outermost trap */
need_sync = (display->error_traps == 1 && !last_request_was_roundtrip);
if (need_sync)
meta_topic (META_DEBUG_SYNC, "%d: Syncing on error_trap_pop, traps = %d, roundtrip = %d\n",
sync_count, display->error_traps, last_request_was_roundtrip);
display->error_trap_synced_at_last_pop = need_sync || last_request_was_roundtrip;
meta_error_trap_pop_internal (display, need_sync);
}
void
meta_error_trap_push_with_return (MetaDisplay *display)
{
gboolean need_sync;
/* We don't sync on push_with_return if there are no traps
* currently, because we assume that any errors were either covered
* by a previous pop, or were fatal.
*
* More generally, we don't sync if we were synchronized last time
* we popped. This is known to be the case if there are no traps,
* but we also keep a flag so we know whether it's the case otherwise.
*/
if (!display->error_trap_synced_at_last_pop)
need_sync = TRUE;
else
need_sync = FALSE;
if (need_sync)
meta_topic (META_DEBUG_SYNC, "%d: Syncing on error_trap_push_with_return, traps = %d\n",
sync_count, display->error_traps);
meta_error_trap_push_internal (display, FALSE);
}
int
meta_error_trap_pop_with_return (MetaDisplay *display,
gboolean last_request_was_roundtrip)
{
if (!last_request_was_roundtrip)
meta_topic (META_DEBUG_SYNC, "%d: Syncing on error_trap_pop_with_return, traps = %d, roundtrip = %d\n",
sync_count, display->error_traps, last_request_was_roundtrip);
display->error_trap_synced_at_last_pop = TRUE;
return meta_error_trap_pop_internal (display,
!last_request_was_roundtrip);
}
static int
x_error_handler (Display *xdisplay,
XErrorEvent *error)
......
......@@ -25,9 +25,15 @@
#include "util.h"
#include "display.h"
void meta_errors_init (void);
void meta_error_trap_push (MetaDisplay *display);
void meta_errors_init (void);
void meta_error_trap_push (MetaDisplay *display);
void meta_error_trap_pop (MetaDisplay *display,
gboolean last_request_was_roundtrip);
void meta_error_trap_push_with_return (MetaDisplay *display);
/* returns X error code, or 0 for no error */
int meta_error_trap_pop (MetaDisplay *display);
int meta_error_trap_pop_with_return (MetaDisplay *display,
gboolean last_request_was_roundtrip);
#endif
......@@ -124,7 +124,7 @@ meta_window_ensure_frame (MetaWindow *window)
window->rect.x,
window->rect.y);
/* FIXME handle this error */
meta_error_trap_pop (window->display);
meta_error_trap_pop (window->display, FALSE);
/* stick frame to the window */
window->frame = frame;
......@@ -180,7 +180,7 @@ meta_window_destroy_frame (MetaWindow *window)
/* FIXME where to put it back depends on the gravity */
window->frame->rect.x,
window->frame->rect.y);
meta_error_trap_pop (window->display);
meta_error_trap_pop (window->display, FALSE);
meta_display_unregister_x_window (window->display,
frame->xwindow);
......
......@@ -231,7 +231,7 @@ read_rgb_icon (MetaDisplay *display,
gulong *best_mini;
int mini_w, mini_h;
meta_error_trap_push (display);
meta_error_trap_push_with_return (display);
type = None;
data = NULL;
result = XGetWindowProperty (display->xdisplay,
......@@ -241,7 +241,7 @@ read_rgb_icon (MetaDisplay *display,
False, XA_CARDINAL, &type, &format, &nitems,
&bytes_after, ((guchar **)&data));
err = meta_error_trap_pop (display);
err = meta_error_trap_pop_with_return (display, TRUE);
if (err != Success ||
result != Success)
......@@ -406,7 +406,7 @@ try_pixmap_and_mask (MetaDisplay *display,
w, h);
}
meta_error_trap_pop (display);
meta_error_trap_pop (display, FALSE);
if (mask)
{
......@@ -470,7 +470,7 @@ get_kwm_win_icon (MetaDisplay *display,
*pixmap = None;
*mask = None;
meta_error_trap_push (display);
meta_error_trap_push_with_return (display);
icons = NULL;
result = XGetWindowProperty (display->xdisplay, xwindow,
display->atom_kwm_win_icon,
......@@ -480,7 +480,7 @@ get_kwm_win_icon (MetaDisplay *display,
&type, &format, &nitems,
&bytes_after, (guchar **)&icons);
err = meta_error_trap_pop (display);
err = meta_error_trap_pop_with_return (display, TRUE);
if (err != Success ||
result != Success)
return;
......
......@@ -799,7 +799,6 @@ meta_change_keygrab (MetaDisplay *display,
int keycode,
int modmask)
{
int result;
int ignored_mask;
/* Grab keycode/modmask, together with
......@@ -812,6 +811,9 @@ meta_change_keygrab (MetaDisplay *display,
grab ? "Grabbing" : "Ungrabbing",
keysym_name (keysym),
modmask, xwindow);
/* efficiency, avoid so many XSync() */
meta_error_trap_push (display);
ignored_mask = 0;
while (ignored_mask < (int) display->ignored_modifier_mask)
......@@ -824,8 +826,9 @@ meta_change_keygrab (MetaDisplay *display,
++ignored_mask;
continue;
}
meta_error_trap_push (display);
if (meta_is_debugging ())
meta_error_trap_push_with_return (display);
if (grab)
XGrabKey (display->xdisplay, keycode,
modmask | ignored_mask,
......@@ -836,21 +839,28 @@ meta_change_keygrab (MetaDisplay *display,
XUngrabKey (display->xdisplay, keycode,
modmask | ignored_mask,
xwindow);
result = meta_error_trap_pop (display);
if (grab && result != Success)
{
if (result == BadAccess)
meta_warning (_("Some other program is already using the key %s with modifiers %x as a binding\n"), keysym_name (keysym), modmask | ignored_mask);
else
meta_topic (META_DEBUG_KEYBINDINGS,
"Failed to grab key %s with modifiers %x\n",