GNOME Bugzilla – Bug 725561
DeviceManagerXi2: Cache the client pointer
Last modified: 2014-03-03 14:06:36 UTC
Jasper found that we call clutter_device_manager_get_core_device during st-widget creation which seems to invole a round trip since we moved to Xi2 ...
Created attachment 270772 [details] [review] DeviceManagerXi2: Cache the client pointer Currently clutter_device_manager_xi2_get_core_device always does a round trip to query the client. So avoid that by caching the client pointer and only update it when the xi devices change.
Review of attachment 270772 [details] [review]: doesn't this patch imply that we do a roundtrip every time a new device is created or removed? this means that when we construct the DeviceManager singleton we also roundtrip a bunch of times. there's also the question as to why St is querying the core device at construction time.
(In reply to comment #2) > Review of attachment 270772 [details] [review]: > > doesn't this patch imply that we do a roundtrip every time a new device is > created or removed? Yes. But how often does this happen? (The only common case I think of is disabling / enabling a touchpad). > this means that when we construct the DeviceManager > singleton we also roundtrip a bunch of times. Why "a bunch of times" ? Shouldn't this happen only once or what am I missing? > there's also the question as to why St is querying the core device at > construction time. This is for widgets that track hover state: https://git.gnome.org/browse/gnome-shell/tree/src/st/st-widget.c#n1721 We also call it in shell_global_sync_pointer: https://git.gnome.org/browse/gnome-shell/tree/src/shell-global.c#n1267
Oh seems like Jasper already changed the code in gnome-shell https://git.gnome.org/browse/gnome-shell/commit/?id=b7eb1f3e8b8806eab6bd238cb73745dea3ab524d
(In reply to comment #3) > (In reply to comment #2) > > Review of attachment 270772 [details] [review] [details]: > > > > doesn't this patch imply that we do a roundtrip every time a new device is > > created or removed? > > Yes. But how often does this happen? (The only common case I think of is > disabling / enabling a touchpad). true. > > this means that when we construct the DeviceManager > > singleton we also roundtrip a bunch of times. > > Why "a bunch of times" ? Shouldn't this happen only once or what am I missing? you're right, I thought you where calling update_client_pointer() from add_device()/remove_device(), instead of directly from the event translation. > > there's also the question as to why St is querying the core device at > > construction time. > > This is for widgets that track hover state: > https://git.gnome.org/browse/gnome-shell/tree/src/st/st-widget.c#n1721 > We also call it in shell_global_sync_pointer: > https://git.gnome.org/browse/gnome-shell/tree/src/shell-global.c#n1267 both cases should probably be removed, and the Shell should probably use the core pointer, instead of asking X11, since it's pretty much guaranteed to exist. anyway, the patch looks good.
Review of attachment 270772 [details] [review]: ACK
Attachment 270772 [details] pushed as 4a3ad9c - DeviceManagerXi2: Cache the client pointer