After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693354 - XI2 usage for fake GDK events slows down window movement
XI2 usage for fake GDK events slows down window movement
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2013-02-07 19:13 UTC by drago01
Modified: 2013-02-07 20:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ui: Cache gdevice in maybe_redirect_mouse_event (1.49 KB, patch)
2013-02-07 20:14 UTC, drago01
rejected Details | Review
ui: Don't use gdk_device_manager_get_client_pointer (1.08 KB, patch)
2013-02-07 20:23 UTC, drago01
committed Details | Review

Description drago01 2013-02-07 19:13:21 UTC
I have noticed that in 3.7.x window movement while a fast drawing app like glxgears is open is very slow. The window lags behind the pointer and if I move the pointer fast enough no movement takes place until I release the button. So something seems to be blocking the compositor at this time.

I can reproduce this with raw mutter so I bisected it and the result was:

c1ac9d1dff0d851f2b35983a7789b22e17a05904 is the first bad commit
commit c1ac9d1dff0d851f2b35983a7789b22e17a05904
Author: Jasper St. Pierre <jstpierre@mecheye.net>
Date:   Tue Nov 20 20:57:13 2012 -0500

    ui: Use XI2 to fake GDK events
    
    This removes our final dependency on Core Events, meaning
    we can remove support code for them soon.
    
    This commit is a bit ugly as it requires ui having a dependency on
    core, but this is already a hack, so this is thus a hack inside a
    hack, and two hacks make a right or however that goes.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=688779

:040000 040000 5cc25872fa192fb2fd4bd383152b4b818dec2faf 7c8a36d51bc1bc6f23887e87fb28616ee937bcbe M	src


And indeed just one commit before this one works, and starting with this it stops working. 

A simple hack like this one:

---------
diff --git a/src/ui/ui.c b/src/ui/ui.c
index 1fe5cd4..c01a922 100644
--- a/src/ui/ui.c
+++ b/src/ui/ui.c
@@ -108,6 +108,8 @@ is_input_event (XEvent *event)
 static gboolean
 maybe_redirect_mouse_event (XEvent *xevent)
 {
+  return FALSE;
+#if 0
   GdkDisplay *gdisplay;
   GdkDeviceManager *gmanager;
   GdkDevice *gdevice;
@@ -232,6 +234,7 @@ maybe_redirect_mouse_event (XEvent *xevent)
   gdk_event_free (gevent);
 
   return TRUE;
+#endif
 }
 
 typedef struct _EventFunc EventFunc;
-------

Restores performance while it does not have any obvious side effects (events in the window menu still work fine).

So why is this code needed at all what it is supposed to do? How does it "get confused by our direct use of X grabs in the core code" ? How does this confusion show up?

There seems to be nothing obvious either why this does block the compositor.
Comment 1 drago01 2013-02-07 19:15:41 UTC
FWIW I am running F18 with xorg-x11-server-Xorg-1.13.2-1.fc18.x86_64
Comment 2 drago01 2013-02-07 19:19:57 UTC
OK this seems needed to avoid bug 599181
Comment 3 drago01 2013-02-07 20:14:47 UTC
Created attachment 235443 [details] [review]
ui: Cache gdevice in maybe_redirect_mouse_event

gdk_device_manager_get_client_pointer which in calls
XIGetClientPointer seems to be very slow in a XI2 world.

Use a static variable instead of querying it every time to avoid
taking the hit.
Comment 4 drago01 2013-02-07 20:15:14 UTC
Review of attachment 235443 [details] [review]:

Breaks menu.
Comment 5 drago01 2013-02-07 20:23:29 UTC
Created attachment 235444 [details] [review]
ui: Don't use gdk_device_manager_get_client_pointer

gdk_device_manager_get_client_pointer which in calls
XIGetClientPointer seems to be very slow in a XI2 world.

So use
	gdk_x11_device_manager_lookup (gmanager, META_VIRTUAL_CORE_POINTER_ID)
instead.


Seems like the menu breakage has nothing to do with my patch. 
According to Jasper they have always been broken since the XI2 move.

This patch uses gdk_x11_device_manager_lookup (gmanager, META_VIRTUAL_CORE_POINTER_ID)
instead of the static variable to avoid the expensive call.
Comment 6 Jasper St. Pierre (not reading bugmail) 2013-02-07 20:27:46 UTC
Review of attachment 235444 [details] [review]:

OK.
Comment 7 drago01 2013-02-07 20:54:52 UTC
Attachment 235444 [details] pushed as b33b4a8 - ui: Don't use gdk_device_manager_get_client_pointer