GNOME Bugzilla – Bug 728968
evdev: Implement touch support
Last modified: 2014-05-21 12:29:36 UTC
Here's a few patches to implement touch support in the evdev input backend through the LIBINPUT_EVENT_TOUCH_* events. Asides from LIBINPUT_EVENT_TOUCH_CANCEL (which happens on all current touches), the translation of events is pretty straightforward, LIBINPUT_EVENT_TOUCH_FRAME is left unhandled as there's no Clutter equivalence.
Created attachment 275158 [details] [review] evdev: Set core device on translated events And ensure the core pointer shares the same stage than the slave device when those events are set. This fixes problems on the evdev backend where the last touch unsets the stage on the device, but nothing sets it back afterwards.
Created attachment 275159 [details] [review] evdev: Manage LIBINPUT_EVENT_TOUCH_* events Those are translated into CLUTTER_TOUCH_* ClutterEvents. As the "NULL" ClutterEventSequence is special cased, the slot=0 value is avoided. Frame events are ignored, as there is no Clutter equivalence, and Cancel events are sent to all current individual touches.
Created attachment 275160 [details] [review] evdev: Add clutter_evdev_event_sequence_get_slot() This function helps know the libinput slot used by a sequence.
Review of attachment 275158 [details] [review]: looks good.
Review of attachment 275159 [details] [review]: looks generally okay to me, with two minor adjustments. ::: clutter/evdev/clutter-device-manager-evdev.c @@ +70,3 @@ + guint32 id; + gfloat x; + gfloat y; we do a ClutterPoint structure that can be used for this. @@ +1052,3 @@ + ClutterTouchState *touch; + + ClutterInputDeviceEvdev *device_evdev = you probably want g_slice_new0() here.
Review of attachment 275160 [details] [review]: looks good, but since it's new API it'll need to land after the clutter-1.20 branch has been created. ::: clutter/evdev/clutter-input-device-evdev.c @@ +238,3 @@ +clutter_evdev_event_sequence_get_slot (const ClutterEventSequence *sequence) +{ + * Returns: the libinput touch slot. probably want to check for NULL. I'm not sure GPOINTER_TO_INT() is NULL safe, and better safe than sorry.
Attachment 275158 [details] pushed as 76d48f7 - evdev: Set core device on translated events Attachment 275159 [details] pushed as 50b3d7c - evdev: Manage LIBINPUT_EVENT_TOUCH_* events Attachment 275160 [details] pushed as 9510d6a - evdev: Add clutter_evdev_event_sequence_get_slot()
(In reply to comment #5) > Review of attachment 275159 [details] [review]: > > looks generally okay to me, with two minor adjustments. > > ::: clutter/evdev/clutter-device-manager-evdev.c > @@ +70,3 @@ > + guint32 id; > + gfloat x; > + gfloat y; > > we do a ClutterPoint structure that can be used for this. Oh, missed that one, updated that to use a ClutterPoint before pushing > > @@ +1052,3 @@ > + ClutterTouchState *touch; > + > + ClutterInputDeviceEvdev *device_evdev = > > you probably want g_slice_new0() here. Well... :), g_free is more friendly as a GDestroyNotify, and it's unlikely that there's going to be many of those. I updated the code to use GSlice anyway. (In reply to comment #6) > Review of attachment 275160 [details] [review]: > > looks good, but since it's new API it'll need to land after the clutter-1.20 > branch has been created. Yup, updated everything to "1.20" before pushing > > ::: clutter/evdev/clutter-input-device-evdev.c > @@ +238,3 @@ > +clutter_evdev_event_sequence_get_slot (const ClutterEventSequence *sequence) > +{ > + * Returns: the libinput touch slot. > > probably want to check for NULL. I'm not sure GPOINTER_TO_INT() is NULL safe, > and better safe than sorry. AFAIR it is (just a cast after all), I changed it anyway, a NULL sequence is completely unexpected if used on touch events from the evdev input backend, and return -1 spots more clearly as an error situation, might have been a g_return_val_if_fail() on second thought...