[PATCH libinput 3/3] touchpad: add support for per-finger hovering information

Peter Hutterer peter.hutterer at who-t.net
Wed May 6 14:33:56 PDT 2015


On Wed, May 06, 2015 at 10:45:20AM -0400, Benjamin Tissoires wrote:
> On Wed, May 6, 2015 at 1:08 AM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > On Thu, Apr 30, 2015 at 04:30:25PM -0400, Benjamin Tissoires wrote:
> >> From: Benjamin Tissoires <benjamin.tissoires at redhat.com>
> >>
> >> When the device supports true hovering, it reports this
> >> information through ABS_MT_DISTANCE.
> >> When this axis is available, we should rely on it to
> >> (un)hover the touches as BTN_TOUCH is most of the time
> >> unreliable (generated by the mouse emulation in the kernel).
> >>
> >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>
> >> ---
> >>  src/evdev-mt-touchpad.c   |   40 +++++++++++-
> >>  src/evdev-mt-touchpad.h   |    2 +
> >>  test/Makefile.am          |    1 +
> >>  test/litest-atmel-hover.c |  149 ++++++++++++++++++++++++++++++++++++++++
> >>  test/litest.c             |  153 +++++++++++++++++++++++++++++++++++++-----
> >>  test/litest.h             |   26 +++++++-
> >>  test/touchpad.c           |  166 +++++++++++++++++++++++++++++++++++++++++++++
> >>  7 files changed, 519 insertions(+), 18 deletions(-)
> >>  create mode 100644 test/litest-atmel-hover.c
> >>
> >> @@ -833,9 +839,17 @@ send_btntool(struct litest_device *d)

[...]

> >>       litest_event(d, EV_KEY, BTN_TOOL_QUINTTAP, d->ntouches_down == 5);
> >>  }
> >>
> >> -void
> >> -litest_touch_down(struct litest_device *d, unsigned int slot,
> >> -               double x, double y)
> >> +static inline int
> >> +litest_validate_event(struct input_event *ev, int send_ev_syn)
> >> +{
> >> +     return !((int16_t)ev->type == EV_SYN &&
> >> +             (int16_t)ev->code == SYN_REPORT &&
> >
> > are those casts needed?
> 
> Maybe not...
> 
> >
> > also, this function is a bit awkwardly named: it doesn't validate as such,
> > it just tells us when to filter syn events.
> >
> > and that brings us to the next bit: I think you can use
> > litest_push_event_frame()/litest_pop_event_frame() instead of passing a
> > parameter around. more on that in the tests below.
> >
> 
> ... but that is moot given that it will be removed by the use of push/pop :)
> 
> Speaking of which. I think the name is awful :) I had to refer to the
> code to understand what that means. Maybe I am too biased by my OpenGL
> background where push/pop meant "push/restore the current state of the
> graphic pipeline". Here it is just: "prevent the syn event to be
> sent"/"OK, now send it".

this is in internal API so we can change it if you come up with a better
name. can't remember what I modelled it after, tbh, so I don't have a track
record of why it's called that way, sorry.

> 
> >
> >> +             !send_ev_syn);
> >> +}
> >
> >
> >> +

[...]

> >> +
> >> +START_TEST(touchpad_hover_down_hover_down)
> >> +{
> >> +     struct litest_device *dev = litest_current_device();
> >> +     struct libinput *li = dev->libinput;
> >> +     int i;
> >> +
> >> +     litest_drain_events(li);
> >> +
> >> +     litest_hover_start(dev, 0, 50, 50, 1);
> >> +
> >> +     for (i = 0; i < 3; i++) {
> >> +
> >> +             /* hover the finger */
> >> +             litest_hover_move_to(dev, 0, 50, 50, 70, 70, 10, 10, 1);
> >> +
> >> +             litest_assert_empty_queue(li);
> >> +
> >> +             /* touch the finger */
> >> +             litest_touch_move_to(dev, 0, 70, 70, 50, 50, 10, 10);
> >
> > pls add a touch_down() call before each touch_move_to, it makes it a lot
> > more obvious that the finger is down now (goes for all tests).
> 
> Unfortunatelly, that is not possible (at least without making my brain
> smoke due to overheat).
> touch_down/hover_down are sent when a new slot has to be allocated. So
> if we send a touch_down after a hover_start, we will receive a new
> tracking ID and this is bad.
> 
> I'll try to think if I can do something better...

oh, right. in that case don't worry about it, just leave it as is, the
comments are enough. if it gets more complex we can change it later.

Cheers,
   Peter

> 
> >
> >> +
> >> +             libinput_dispatch(li);
> >> +
> >> +             litest_assert_only_typed_events(li,
> >> +                                             LIBINPUT_EVENT_POINTER_MOTION);
> >> +     }
> >> +
> >> +     litest_hover_end(dev, 0, 1);
> >> +
> >> +     /* start a new touch to be sure */
> >> +     litest_touch_down(dev, 0, 50, 50);
> >> +     litest_touch_move_to(dev, 0, 50, 50, 70, 70, 10, 10);
> >> +     litest_touch_up(dev, 0);
> >> +
> >> +     litest_assert_only_typed_events(li,
> >> +                                     LIBINPUT_EVENT_POINTER_MOTION);
> >> +}
> >> +END_TEST
> >> +
> >> +START_TEST(touchpad_hover_down_up)
> >> +{
> >> +     struct litest_device *dev = litest_current_device();
> >> +     struct libinput *li = dev->libinput;
> >> +
> >> +     litest_drain_events(li);
> >> +
> >> +     /* hover two fingers, and a touch */
> >
> > ok, here's where I think using the push/pop pairs will do the same job but
> > be slightly nicer in terms of API.
> >
> > litest_push_event_frame();
> >
> >> +     litest_hover_start(dev, 0, 50, 50, 0);
> >> +     litest_hover_start(dev, 1, 50, 50, 0);
> >> +     litest_touch_down(dev, 2, 50, 50);
> >
> > litest_pop_event_frame();
> >
> >> +
> >> +     litest_assert_empty_queue(li);
> >> +
> >> +     /* hover first finger, end second and third in same frame */
> >
> > litest_push_event_frame();
> >
> >> +     litest_hover_move(dev, 0, 70, 70, 0);
> >> +     litest_hover_end(dev, 1, 0);
> >> +     litest_touch_up(dev, 2);
> >
> > litest_push_event_frame();
> >> +
> >> +     litest_assert_empty_queue(li);
> >> +
> >> +     /* now move the finger */
> >> +     litest_touch_move_to(dev, 0, 50, 50, 70, 70, 10, 10);
> >> +
> >> +     litest_touch_up(dev, 0);
> >> +
> >> +     litest_assert_only_typed_events(li,
> >> +                                     LIBINPUT_EVENT_POINTER_MOTION);
> >> +}
> >> +END_TEST
> >> +
> >> +START_TEST(touchpad_hover_2fg_noevent)
> >> +{
> >> +     struct litest_device *dev = litest_current_device();
> >> +     struct libinput *li = dev->libinput;
> >> +
> >> +     litest_drain_events(li);
> >> +
> >> +     /* hover two fingers */
> >
> > litest_push_event_frame();
> >
> >> +     litest_hover_start(dev, 0, 25, 25, 0);
> >> +     litest_hover_start(dev, 1, 50, 50, 1);
> >> +
> > litest_pop_event_frame();
> >
> >> +     litest_hover_move_two_touches(dev, 25, 25, 50, 50, 50, 50, 10, 0);
> >> +
> >
> > litest_push_event_frame();
> >
> >> +     litest_hover_end(dev, 0, 0);
> >> +     litest_hover_end(dev, 1, 1);
> >
> > litest_pop_event_frame();
> >> +
> >> +     litest_assert_empty_queue(li);
> >> +}
> >> +END_TEST
> >> +
> >> +START_TEST(touchpad_hover_2fg_1fg_down)
> >> +{
> >> +     struct litest_device *dev = litest_current_device();
> >> +     struct libinput *li = dev->libinput;
> >> +     int i;
> >> +
> >> +     litest_drain_events(li);
> >> +
> >> +     /* hover two fingers */
> >
> > litest_push_event_frame();
> >
> >> +     litest_hover_start(dev, 0, 25, 25, 0);
> >> +     litest_touch_down(dev, 1, 50, 50);
> >
> > litest_pop_event_frame();
> >> +
> >> +     for (i = 0; i < 10; i++) {
> > litest_push_event_frame();
> >> +             litest_hover_move(dev, 0, 25 + 5 * i, 25 + 5 * i, 0);
> >> +             litest_touch_move(dev, 1, 50 + 5 * i, 50 - 5 * i);
> >
> > litest_pop_event_frame();
> >
> >> +     }
> >> +
> > litest_push_event_frame();
> >> +     litest_hover_end(dev, 0, 0);
> >> +     litest_touch_up(dev, 1);
> >
> > litest_pop_event_frame();
> >
> > right, so after adding these I think this should work. you can drop the
> > extra parameter and just use the push/pop instead.
> 
> OK, thanks. Will do.
> 
> Cheers,
> Benjamin
> 
> >
> > Cheers,
> >    Peter
> >
> >> +
> >> +     litest_assert_only_typed_events(li,
> >> +                                     LIBINPUT_EVENT_POINTER_MOTION);
> >> +}
> >> +END_TEST
> >> +
> >>  static void
> >>  assert_btnevent_from_device(struct litest_device *device,
> >>                           unsigned int button,
> >> @@ -4241,6 +4400,13 @@ int main(int argc, char **argv) {
> >>       litest_add_for_device("touchpad:semi-mt-hover", touchpad_semi_mt_hover_2fg_noevent, LITEST_SYNAPTICS_HOVER_SEMI_MT);
> >>       litest_add_for_device("touchpad:semi-mt-hover", touchpad_semi_mt_hover_2fg_1fg_down, LITEST_SYNAPTICS_HOVER_SEMI_MT);
> >>
> >> +     litest_add("touchpad:hover", touchpad_hover_noevent, LITEST_TOUCHPAD|LITEST_HOVER, LITEST_ANY);
> >> +     litest_add("touchpad:hover", touchpad_hover_down, LITEST_TOUCHPAD|LITEST_HOVER, LITEST_ANY);
> >> +     litest_add("touchpad:hover", touchpad_hover_down_up, LITEST_TOUCHPAD|LITEST_HOVER, LITEST_ANY);
> >> +     litest_add("touchpad:hover", touchpad_hover_down_hover_down, LITEST_TOUCHPAD|LITEST_HOVER, LITEST_ANY);
> >> +     litest_add("touchpad:hover", touchpad_hover_2fg_noevent, LITEST_TOUCHPAD|LITEST_HOVER, LITEST_ANY);
> >> +     litest_add("touchpad:hover", touchpad_hover_2fg_1fg_down, LITEST_TOUCHPAD|LITEST_HOVER, LITEST_ANY);
> >> +
> >>       litest_add_for_device("touchpad:trackpoint", touchpad_trackpoint_buttons, LITEST_SYNAPTICS_TRACKPOINT_BUTTONS);
> >>       litest_add_for_device("touchpad:trackpoint", touchpad_trackpoint_mb_scroll, LITEST_SYNAPTICS_TRACKPOINT_BUTTONS);
> >>       litest_add_for_device("touchpad:trackpoint", touchpad_trackpoint_mb_click, LITEST_SYNAPTICS_TRACKPOINT_BUTTONS);
> >> --
> >> 1.7.1
> >>
> >
> >> _______________________________________________
> >> wayland-devel mailing list
> >> wayland-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >


More information about the wayland-devel mailing list