[PATCH libinput 3/3] touchpad: add support for per-finger hovering information
Benjamin Tissoires
benjamin.tissoires at gmail.com
Wed May 6 14:51:08 PDT 2015
On Wed, May 6, 2015 at 5:33 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> 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.
To keep the OpenGL analogy, how about:
litest_begin_event_frame()
litest_end_event_frame()
>
>>
>> >
>> >> + !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.
>
My brain thank you :)
Cheers,
Benjamin
> 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