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 725102 - Patches to rework the evdev backend's keymap handling
Patches to rework the evdev backend's keymap handling
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2014-02-24 22:22 UTC by Rui Matos
Modified: 2014-02-27 10:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
evdev: Add a conditional define guard to expose API (1.96 KB, patch)
2014-02-24 22:22 UTC, Rui Matos
committed Details | Review
evdev: Implement keyboard repeat (6.64 KB, patch)
2014-02-24 22:22 UTC, Rui Matos
committed Details | Review
evdev: Keep latched and locked modifier state when switching keymaps (1.80 KB, patch)
2014-02-24 22:22 UTC, Rui Matos
committed Details | Review
evdev: Don't update xkb state with pressed keys on keymap change (3.93 KB, patch)
2014-02-24 22:22 UTC, Rui Matos
committed Details | Review
evdev: Make the keymap available (2.15 KB, patch)
2014-02-24 22:22 UTC, Rui Matos
committed Details | Review

Description Rui Matos 2014-02-24 22:22:34 UTC
These are all on top of the libinput port from bug 720566.

The first patch just marks the evdev public API as explicitly not
under the same API/ABI guarantees as the rest of Clutter as dicussed
on bug 720566.
Comment 1 Rui Matos 2014-02-24 22:22:37 UTC
Created attachment 270191 [details] [review]
evdev: Add a conditional define guard to expose API

The evdev backend has always been excluded from Clutter's API
stability guarantee though in an informal way. This commit makes it
explicit by forcing users to define CLUTTER_ENABLE_COMPOSITOR_API.
Comment 2 Rui Matos 2014-02-24 22:22:42 UTC
Created attachment 270192 [details] [review]
evdev: Implement keyboard repeat

The kernel keyboard repeat functionality isn't configurable and
libinput rightfully ignores it.

This implements keyboard repeat in userspace allowing for consumers to
set the initial delay and repeat intervals.
Comment 3 Rui Matos 2014-02-24 22:22:46 UTC
Created attachment 270193 [details] [review]
evdev: Keep latched and locked modifier state when switching keymaps
Comment 4 Rui Matos 2014-02-24 22:22:50 UTC
Created attachment 270194 [details] [review]
evdev: Don't update xkb state with pressed keys on keymap change

Doing so is unlikely to work reliably. Instead, switching the keymap
should be done at a time when no key is currently pressed down, but
let's leave that task to higher level code.

This allows us to remove key state tracking at yet another level in
the stack since higher level code likely already tracks this for other
purposes.
Comment 5 Rui Matos 2014-02-24 22:22:54 UTC
Created attachment 270195 [details] [review]
evdev: Make the keymap available

Make the keymap available so that consumers don't have to duplicate it
if they need it.
Comment 6 Emmanuele Bassi (:ebassi) 2014-02-24 22:48:57 UTC
Review of attachment 270191 [details] [review]:

looks good.
Comment 7 Emmanuele Bassi (:ebassi) 2014-02-24 23:13:00 UTC
Review of attachment 270192 [details] [review]:

looks okay.

::: clutter/evdev/clutter-device-manager-evdev.c
@@ +307,3 @@
+    {
+    case 1:
+

cute.
Comment 8 Emmanuele Bassi (:ebassi) 2014-02-24 23:13:35 UTC
Review of attachment 270193 [details] [review]:

looks okay.
Comment 9 Emmanuele Bassi (:ebassi) 2014-02-24 23:14:20 UTC
Review of attachment 270194 [details] [review]:

I'm all for dropping code.

looks good.
Comment 10 Emmanuele Bassi (:ebassi) 2014-02-24 23:16:03 UTC
Review of attachment 270195 [details] [review]:

looks good. needs a minor documentation fix.

::: clutter/evdev/clutter-device-manager-evdev.c
@@ +1517,3 @@
+ * clutter_evdev_get_keyboard_map: (skip)
+ * @evdev: the #ClutterDeviceManager created by the evdev backend
+ *

missing actual description.

::: clutter/evdev/clutter-evdev.h
@@ +86,3 @@
 						     struct xkb_keymap    *keymap);
 
+struct xkb_keymap * clutter_evdev_get_keyboard_map (ClutterDeviceManager *evdev);

noticed that all the symbols are missing the CLUTTER_AVAILABLE_IN_* annotation.

we'll need a commit that adds it.
Comment 11 Rui Matos 2014-02-27 10:55:46 UTC
Also pushed a couple of patches with the versioning annotations and
doc tags.

Attachment 270191 [details] pushed as 133f95f - evdev: Add a conditional define guard to expose API
Attachment 270192 [details] pushed as a6bd53e - evdev: Implement keyboard repeat
Attachment 270193 [details] pushed as 945ee57 - evdev: Keep latched and locked modifier state when switching keymaps
Attachment 270194 [details] pushed as 2a7d550 - evdev: Don't update xkb state with pressed keys on keymap change
Attachment 270195 [details] pushed as d67b38f - evdev: Make the keymap available