Re: [PATCH libinput 0/9] Improvements for clickpad touchpads
Hi Peter, So as you requested I've been looking at your wip/clickpad-improvements branch. I've been mostly reading the code / patches and here is a long list of comments : 1) I like the state-machine concept, and having a diagram. I think this is a good way to deal with this, otherwise we will end up adding layers of kludges until we've something which no one understands and which works most of the time 2) You only handle 2 buttons areas left and right, this won't work for the Thinkpad *40's series. I've been thinking about a way to make your state-machine work with more buttons without exploding the diagram / amount of states. I think that we need to define BUTTON_EVENT_IN_BUTTON0 .. BUTTON_EVENT_IN_BUTTON7, store one of these in t-button.current_button and then have states like BUTTON_STATE_NEW and events / transition conditions like finger in other button area. Then in the switch cases in the statemachine where we now check for the other button, check for all buttons, and put an if there to turn the current button into a break, ie in tp_button_left_handle_event have: switch(event) { case BUTTON_EVENT_IN_BUTTON0: case BUTTON_EVENT_IN_BUTTON1: case BUTTON_EVENT_IN_BUTTON2: case BUTTON_EVENT_IN_BUTTON3: case BUTTON_EVENT_IN_BUTTON4: case BUTTON_EVENT_IN_BUTTON5: case BUTTON_EVENT_IN_BUTTON6: case BUTTON_EVENT_IN_BUTTON7: if (event == t-button.current_button) break; t-button.state = BUTTON_STATE_TO_OTHER_BUTTON; tp_button_set_leave_timer(tp, t); break; This will significantly simplify the state-machine while allowing more buttons, ie BUTTON_STATE_LEFT_TO_RIGHT and BUTTON_STATE_RIGHT_TO_LEFT would become a single BUTTON_STATE_TO_OTHER_BUTTON state, and likewise BUTTON_STATE_LEFT_NEW and BUTTON_STATE_RIGHT_NEW would both simply become BUTTON_STATE_NEW, etc. I can rework the state machine + code to work like this if you agree that this is a good idea. 3) You don't seem to do anything with INPUT_PROP_BUTTONPAD, I'm not sure that is a good idea 4) In pin finger you assume that the finger which is the closest to the bottom edge is the one doing the clicking, this will not work on the Thinkpad *40 series. IE there may be a palm on the touchpad which will be lower then the finger clicking on the trackpoint button areas. I know we don't have palm detection yet, but in general this just feels wrong. Maybe just pin all fingers on a click and rely on unpinning to do its job to allow click + drag ? 5) In tp_post_softbutton_buttons you don't seem to deal with a physical click coming before, or at the same time, as the finger down this means that a user ie using the trackpoint and then doing a quick right click, will get a left click (t-button.state == BUTTON_STATE_RIGHT_NEW) or no click at all (tp-nfingers_down == 0). Note that neither will get corrected later since current == old will be true. To fix this I suggest that we check if all touch button states are stable, and if not set a click pending flag and stop there. Then when ever a touch button state becomes stable check the click pending flag, and if a click is pending process it then. Note that I say all touch button states must be stable because one of the things I expect libinput to fix is moving the pointer with my right input finger and then clicking the right button area with my right thumb. The thumb will then be the last finger down and we want it to be stable too so that we properly report a right click. Note that this scenario also advocates in favor of pinning all fingers, so that any movement of the index finger detected by the tp at this time also does not get reported as this likely is an unwanted side-effect of the click too. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2 libinput 1/5] test: add litest helper functions for creating uinput devices
Hi, On 04/01/2014 05:47 AM, Peter Hutterer wrote: Both functions accept a series of event types/codes tuples, terminated by -1. For the even type INPUT_PROP_MAX (an invalid type otherwise) the code is used as a property to enable. The _abs function als takes an array of absinfo, with absinfo.value determining the axis to change. If none are given, abs axes are initialized with default settings. Both functions abort on failure, so the caller does not need to check the return value. Example code for creating a rel device: struct libevdev_uinput *uinput; struct input_id id = { ... }; uinput = litest_create_uinput_device(foo, id, EV_REL, REL_X, EV_REL, REL_Y, EV_KEY, BTN_LEFT, INPUT_PROP_MAX, INPUT_PROP_BUTTONPAD, -1); libevdev_uinput_write_event(uinput, EV_REL, REL_X, -1); libevdev_uinput_write_event(uinput, EV_SYN, SYN_REPORT, 0); ... libevdev_uinput_destroy(uinput); Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans --- Changes to v1: - make absinfo a -1-terminated list as well test/litest-bcm5974.c | 45 +++- test/litest-generic-highres-touch.c | 34 +++--- test/litest-synaptics-st.c | 37 +++ test/litest-synaptics.c | 45 +++- test/litest-trackpoint.c| 32 +++--- test/litest-wacom-touch.c | 33 ++ test/litest.c | 88 +++ test/litest.h | 8 ++ test/path.c | 207 9 files changed, 244 insertions(+), 285 deletions(-) diff --git a/test/litest-bcm5974.c b/test/litest-bcm5974.c index 5a8ce8a..6b7a22b 100644 --- a/test/litest-bcm5974.c +++ b/test/litest-bcm5974.c @@ -95,7 +95,6 @@ static struct litest_device_interface interface = { void litest_create_bcm5974(struct litest_device *d) { - struct libevdev *dev; struct input_absinfo abs[] = { { ABS_X, 1472, 5472, 75 }, { ABS_Y, 1408, 4448, 129 }, @@ -105,36 +104,26 @@ litest_create_bcm5974(struct litest_device *d) { ABS_MT_POSITION_X, 1472, 5472, 75 }, { ABS_MT_POSITION_Y, 1408, 4448, 129 }, { ABS_MT_TRACKING_ID, 0, 65535, 0 }, - { ABS_MT_PRESSURE, 0, 255, 0 } + { ABS_MT_PRESSURE, 0, 255, 0 }, + { .value = -1 }, + }; + struct input_id id = { + .bustype = 0x3, + .vendor = 0x5ac, + .product = 0x249, }; - struct input_absinfo *a; - int rc; d-interface = interface; - - dev = libevdev_new(); - ck_assert(dev != NULL); - - libevdev_set_name(dev, bcm5974); - libevdev_set_id_bustype(dev, 0x3); - libevdev_set_id_vendor(dev, 0x5ac); - libevdev_set_id_product(dev, 0x249); - libevdev_enable_event_code(dev, EV_KEY, BTN_LEFT, NULL); - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_FINGER, NULL); - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_QUINTTAP, NULL); - libevdev_enable_event_code(dev, EV_KEY, BTN_TOUCH, NULL); - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_DOUBLETAP, NULL); - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_TRIPLETAP, NULL); - libevdev_enable_event_code(dev, EV_KEY, BTN_TOOL_QUADTAP, NULL); - - ARRAY_FOR_EACH(abs, a) - libevdev_enable_event_code(dev, EV_ABS, a-value, a); - - rc = libevdev_uinput_create_from_device(dev, - LIBEVDEV_UINPUT_OPEN_MANAGED, - d-uinput); - ck_assert_int_eq(rc, 0); - libevdev_free(dev); + d-uinput = litest_create_uinput_abs_device(bcm5974, id, + abs, + EV_KEY, BTN_LEFT, + EV_KEY, BTN_TOOL_FINGER, + EV_KEY, BTN_TOOL_QUINTTAP, + EV_KEY, BTN_TOUCH, + EV_KEY, BTN_TOOL_DOUBLETAP, + EV_KEY, BTN_TOOL_TRIPLETAP, + EV_KEY, BTN_TOOL_QUADTAP, + -1, -1); } struct litest_test_device litest_bcm5974_device = { diff --git a/test/litest-generic-highres-touch.c b/test/litest-generic-highres-touch.c index 68615c3..bb226d6 100644 --- a/test/litest-generic-highres-touch.c +++ b/test/litest-generic-highres-touch.c @@ -94,9 +94,6 @@ static struct litest_device_interface interface
Re: [PATCH libinput 2/5] test: if no teardown func is set, use the default
Hi, On 04/01/2014 05:47 AM, Peter Hutterer wrote: Reduces the amount of boilerplate code. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/litest-bcm5974.c | 2 +- test/litest-generic-highres-touch.c | 2 +- test/litest-keyboard.c | 2 +- test/litest-mouse.c | 2 +- test/litest-synaptics-st.c | 2 +- test/litest-trackpoint.c| 2 +- test/litest-wacom-touch.c | 2 +- test/litest.c | 3 ++- 8 files changed, 9 insertions(+), 8 deletions(-) diff --git a/test/litest-bcm5974.c b/test/litest-bcm5974.c index 6b7a22b..ff822f9 100644 --- a/test/litest-bcm5974.c +++ b/test/litest-bcm5974.c @@ -131,6 +131,6 @@ struct litest_test_device litest_bcm5974_device = { .features = LITEST_TOUCHPAD | LITEST_CLICKPAD | LITEST_BUTTON, .shortname = bcm5974, .setup = litest_bcm5974_setup, - .teardown = litest_generic_device_teardown, + .teardown = NULL, .create = litest_create_bcm5974, }; Hmm, why not simply delete all the .teardown inits completely ? C guarantees that if a part of a struct is initialized, the rest will be set to 0. Only if no init at all is done then for non globals you may get garbage. Other then that, this patch looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans diff --git a/test/litest-generic-highres-touch.c b/test/litest-generic-highres-touch.c index bb226d6..4d21b0d 100644 --- a/test/litest-generic-highres-touch.c +++ b/test/litest-generic-highres-touch.c @@ -123,6 +123,6 @@ struct litest_test_device litest_generic_highres_touch_device = { .features = LITEST_TOUCH, .shortname = generic-highres-touch, .setup = litest_generic_highres_touch_setup, - .teardown = litest_generic_device_teardown, + .teardown = NULL, .create = litest_create_generic_highres_touch, }; diff --git a/test/litest-keyboard.c b/test/litest-keyboard.c index dd91158..ab05014 100644 --- a/test/litest-keyboard.c +++ b/test/litest-keyboard.c @@ -109,6 +109,6 @@ struct litest_test_device litest_keyboard_device = { .features = LITEST_KEYS, .shortname = default keyboard, .setup = litest_keyboard_setup, - .teardown = litest_generic_device_teardown, + .teardown = NULL, .create = litest_create_keyboard, }; diff --git a/test/litest-mouse.c b/test/litest-mouse.c index 2fde095..2f70767 100644 --- a/test/litest-mouse.c +++ b/test/litest-mouse.c @@ -70,6 +70,6 @@ struct litest_test_device litest_mouse_device = { .features = LITEST_POINTER | LITEST_BUTTON | LITEST_WHEEL, .shortname = mouse, .setup = litest_mouse_setup, - .teardown = litest_generic_device_teardown, + .teardown = NULL, .create = litest_create_mouse, }; diff --git a/test/litest-synaptics-st.c b/test/litest-synaptics-st.c index d13d9a2..de56c22 100644 --- a/test/litest-synaptics-st.c +++ b/test/litest-synaptics-st.c @@ -126,6 +126,6 @@ struct litest_test_device litest_synaptics_touchpad_device = { .features = LITEST_TOUCHPAD | LITEST_BUTTON | LITEST_SINGLE_TOUCH, .shortname = synaptics ST, .setup = litest_synaptics_touchpad_setup, - .teardown = litest_generic_device_teardown, + .teardown = NULL, .create = litest_create_synaptics_touchpad, }; diff --git a/test/litest-trackpoint.c b/test/litest-trackpoint.c index e0b79c5..1c0fb0a 100644 --- a/test/litest-trackpoint.c +++ b/test/litest-trackpoint.c @@ -61,6 +61,6 @@ struct litest_test_device litest_trackpoint_device = { .features = LITEST_POINTER | LITEST_BUTTON, .shortname = trackpoint, .setup = litest_trackpoint_setup, - .teardown = litest_generic_device_teardown, + .teardown = NULL, .create = litest_create_trackpoint, }; diff --git a/test/litest-wacom-touch.c b/test/litest-wacom-touch.c index 01a5a5d..e9119a9 100644 --- a/test/litest-wacom-touch.c +++ b/test/litest-wacom-touch.c @@ -122,6 +122,6 @@ struct litest_test_device litest_wacom_touch_device = { .features = LITEST_TOUCH, .shortname = wacom-touch, .setup = litest_wacom_touch_setup, - .teardown = litest_generic_device_teardown, + .teardown = NULL, .create = litest_create_wacom_touch, }; diff --git a/test/litest.c b/test/litest.c index f7fe24e..6767952 100644 --- a/test/litest.c +++ b/test/litest.c @@ -119,7 +119,8 @@ litest_add_tcase_for_device(struct suite *suite, t-name = strdup(test_name); t-tc = tcase_create(test_name); list_insert(suite-tests, t-node); - tcase_add_checked_fixture(t-tc, dev-setup, dev-teardown); + tcase_add_checked_fixture(t-tc, dev-setup, + dev-teardown ? dev-teardown : litest_generic_device_teardown); tcase_add_test(t-tc, func); suite_add_tcase(suite-suite, t-tc
Re: [PATCH libinput 3/5] test: allow for description-based test devices
, .shortname = synaptics, .setup = litest_synaptics_clickpad_setup, - .teardown = litest_generic_device_teardown, - .create = litest_create_synaptics_clickpad, + .teardown = NULL, + .create = NULL, + .interface = interface, + + .name = SynPS/2 Synaptics TouchPad, + .id = input_id, + .events = events, + .absinfo = absinfo, }; diff --git a/test/litest.c b/test/litest.c index 6767952..23ba76b 100644 --- a/test/litest.c +++ b/test/litest.c @@ -61,6 +61,12 @@ struct suite { static struct litest_device *current_device; +struct libevdev_uinput * +litest_create_uinput_device_from_description(const char *name, + struct input_id *id, + const struct input_absinfo *abs, + const int *events); + Hmm, shouldn't this be either static, or in a header file ? Other then that, this patch looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans struct litest_device *litest_current_device(void) { return current_device; } @@ -342,7 +348,15 @@ litest_create_device(enum litest_device_type which) dev = devices; while (*dev) { if ((*dev)-type == which) { - (*dev)-create(d); + if ((*dev)-create) + (*dev)-create(d); + else { + d-uinput = litest_create_uinput_device_from_description((*dev)-name, + (*dev)-id, + (*dev)-absinfo, + (*dev)-events); + d-interface = (*dev)-interface; + } break; } dev++; @@ -368,10 +382,12 @@ litest_create_device(enum litest_device_type which) ck_assert(d-libinput_device != NULL); libinput_device_ref(d-libinput_device); - d-interface-min[ABS_X] = libevdev_get_abs_minimum(d-evdev, ABS_X); - d-interface-max[ABS_X] = libevdev_get_abs_maximum(d-evdev, ABS_X); - d-interface-min[ABS_Y] = libevdev_get_abs_minimum(d-evdev, ABS_Y); - d-interface-max[ABS_Y] = libevdev_get_abs_maximum(d-evdev, ABS_Y); + if (d-interface) { + d-interface-min[ABS_X] = libevdev_get_abs_minimum(d-evdev, ABS_X); + d-interface-max[ABS_X] = libevdev_get_abs_maximum(d-evdev, ABS_X); + d-interface-min[ABS_Y] = libevdev_get_abs_minimum(d-evdev, ABS_Y); + d-interface-max[ABS_Y] = libevdev_get_abs_maximum(d-evdev, ABS_Y); + } return d; } @@ -410,10 +426,54 @@ litest_event(struct litest_device *d, unsigned int type, libevdev_uinput_write_event(d-uinput, type, code, value); } +static int +auto_assign_value(struct litest_device *d, + const struct input_event *ev, + int slot, int x, int y) +{ + static int tracking_id; + int value = ev-value; + + if (value != LITEST_AUTO_ASSIGN || ev-type != EV_ABS) + return value; + + switch (ev-code) { + case ABS_X: + case ABS_MT_POSITION_X: + value = litest_scale(d, ABS_X, x); + break; + case ABS_Y: + case ABS_MT_POSITION_Y: + value = litest_scale(d, ABS_Y, y); + break; + case ABS_MT_TRACKING_ID: + value = ++tracking_id; + break; + case ABS_MT_SLOT: + value = slot; + break; + } + + return value; +} + + void litest_touch_down(struct litest_device *d, unsigned int slot, int x, int y) { - d-interface-touch_down(d, slot, x, y); + struct input_event *ev; + + if (d-interface-touch_down) { + d-interface-touch_down(d, slot, x, y); + return; + } + + ev = d-interface-touch_down_events; + while (ev (int16_t)ev-type != -1 (int16_t)ev-code != -1) { + int value = auto_assign_value(d, ev, slot, x, y); + litest_event(d, ev-type, ev-code, value); + ev++; + } } void @@ -421,23 +481,43 @@ litest_touch_up(struct litest_device *d, unsigned int slot) { struct input_event *ev; struct input_event up[] = { - { .type = EV_ABS, .code = ABS_MT_SLOT, .value = slot }, + { .type = EV_ABS, .code = ABS_MT_SLOT, .value = LITEST_AUTO_ASSIGN }, { .type = EV_ABS, .code = ABS_MT_TRACKING_ID, .value = -1 }, { .type = EV_SYN, .code = SYN_REPORT, .value = 0 }, + { .type = -1, .code = -1 } }; if (d-interface-touch_up) { d-interface-touch_up(d
Re: [PATCH libinput 5/5] test: switch the remaining devices to a description-based device
Hi, On 04/01/2014 05:47 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/litest-bcm5974.c | 147 ++- test/litest-generic-highres-touch.c | 128 test/litest-keyboard.c | 231 +--- test/litest-mouse.c | 48 test/litest-synaptics-st.c | 131 test/litest-trackpoint.c| 39 +++--- test/litest-wacom-touch.c | 127 7 files changed, 419 insertions(+), 432 deletions(-) Looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans p.s. I'm wondering if we should remove support for direct device instantiation, now all devices use the descriptor based approach ? diff --git a/test/litest-bcm5974.c b/test/litest-bcm5974.c index ff822f9..ab944a7 100644 --- a/test/litest-bcm5974.c +++ b/test/litest-bcm5974.c @@ -34,97 +34,66 @@ static void litest_bcm5974_setup(void) litest_set_current_device(d); } -static void -litest_bcm5974_touch_down(struct litest_device *d, - unsigned int slot, - int x, int y) -{ - static int tracking_id; - struct input_event *ev; - struct input_event down[] = { - { .type = EV_KEY, .code = BTN_TOOL_FINGER, .value = 1 }, - { .type = EV_KEY, .code = BTN_TOUCH, .value = 1 }, - { .type = EV_ABS, .code = ABS_X, .value = x }, - { .type = EV_ABS, .code = ABS_Y, .value = y }, - { .type = EV_ABS, .code = ABS_PRESSURE, .value = 30 }, - { .type = EV_ABS, .code = ABS_MT_SLOT, .value = slot }, - { .type = EV_ABS, .code = ABS_MT_TRACKING_ID, .value = ++tracking_id }, - { .type = EV_ABS, .code = ABS_MT_POSITION_X, .value = x }, - { .type = EV_ABS, .code = ABS_MT_POSITION_Y, .value = y }, - { .type = EV_SYN, .code = SYN_REPORT, .value = 0 }, - }; +struct input_event down[] = { + { .type = EV_KEY, .code = BTN_TOOL_FINGER, .value = 1 }, + { .type = EV_KEY, .code = BTN_TOUCH, .value = 1 }, + { .type = EV_ABS, .code = ABS_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_PRESSURE, .value = 30 }, + { .type = EV_ABS, .code = ABS_MT_SLOT, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_TRACKING_ID, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_SYN, .code = SYN_REPORT, .value = 0 }, + { .type = -1, .code = -1 }, +}; - down[2].value = litest_scale(d, ABS_X, x); - down[3].value = litest_scale(d, ABS_Y, y); - down[7].value = litest_scale(d, ABS_X, x); - down[8].value = litest_scale(d, ABS_Y, y); - - ARRAY_FOR_EACH(down, ev) - litest_event(d, ev-type, ev-code, ev-value); -} - -void -litest_bcm5974_move(struct litest_device *d, unsigned int slot, int x, int y) -{ - struct input_event *ev; - struct input_event move[] = { - { .type = EV_ABS, .code = ABS_MT_SLOT, .value = slot }, - { .type = EV_ABS, .code = ABS_X, .value = x }, - { .type = EV_ABS, .code = ABS_Y, .value = y }, - { .type = EV_ABS, .code = ABS_MT_POSITION_X, .value = x }, - { .type = EV_ABS, .code = ABS_MT_POSITION_Y, .value = y }, - { .type = EV_KEY, .code = BTN_TOOL_FINGER, .value = 1 }, - { .type = EV_KEY, .code = BTN_TOUCH, .value = 1 }, - { .type = EV_SYN, .code = SYN_REPORT, .value = 0 }, - }; - - move[1].value = litest_scale(d, ABS_X, x); - move[2].value = litest_scale(d, ABS_Y, y); - move[3].value = litest_scale(d, ABS_X, x); - move[4].value = litest_scale(d, ABS_Y, y); - - ARRAY_FOR_EACH(move, ev) - litest_event(d, ev-type, ev-code, ev-value); -} +static struct input_event move[] = { + { .type = EV_ABS, .code = ABS_MT_SLOT, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_KEY, .code = BTN_TOOL_FINGER, .value = 1 }, + { .type = EV_KEY, .code = BTN_TOUCH, .value = 1 }, + { .type = EV_SYN, .code = SYN_REPORT, .value = 0 }, + { .type = -1, .code = -1 }, +}; static struct litest_device_interface interface = { - .touch_down = litest_bcm5974_touch_down, - .touch_move = litest_bcm5974_move, + .touch_down_events
Self introduction Hans de Goede
Hi All, I guess most of you already know me, but since I'm about to become a whole lot more active on this list, I thought it would be a good idea to start with a self-intro: My name is Hans de Goede, and I'm active as a FOSS contributor / developer since 1997. Recently I've mainly been working on hwmon kernel drivers, usb webcam kernel drivers and userspace support libs, usb emulation and redirection in qemu, libusb, usbdevfs and getting Xorg to run without root rights. I've been working for Red Hat since 2008, first on anaconda, the installer, and for the last 3 years on Spice (focussing on usb redirection). Recently I've moved to Red Hat's graphics team, where I will be mostly working on the input side. My first Wayland related project will be working on improving touchpad and esp. clickpad support in libinput. I've been working Peter Hutterer's clickpad-improvements patch-set and I will soon send a new version of this set. Although I'm a seasoned FOSS developer I'm new to the world of Wayland, so I might ask some newbie questions every now and then and make some beginner mistakes. When I do please feel free to correct me :) Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 02/20] touchpad: after a click, lock the finger to its current position
From: Peter Hutterer peter.hutte...@who-t.net On clickpads, clicking the pad usually causes some motion events. To avoid erroneous movements, lock the finger into position on the click and don't allow for motion events until we move past a given threshold (currently 2% of the touchpad diagonal). HdG: Instead of pinning the lowest touch assuming that that is the one holding the physical button, simply pin all touches, and unpin as soon as a touch is moved more then the threshold. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 79 - src/evdev-mt-touchpad.h | 12 +++- 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 8021db2..fbe0a7b 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -32,6 +32,7 @@ #define DEFAULT_MIN_ACCEL_FACTOR 0.16 #define DEFAULT_MAX_ACCEL_FACTOR 1.0 #define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0 +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ static inline int tp_hysteresis(int in, int center, int margin) @@ -182,6 +183,7 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t) t-dirty = true; t-is_pointer = false; t-state = TOUCH_END; + t-pinned.is_pinned = false; assert(tp-nfingers_down = 1); tp-nfingers_down--; tp-queued |= TOUCHPAD_EVENT_MOTION; @@ -346,50 +348,43 @@ tp_process_key(struct tp_dispatch *tp, } static void -tp_unpin_finger(struct tp_dispatch *tp) +tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t) { - struct tp_touch *t; - tp_for_each_touch(tp, t) { - if (t-is_pinned) { - t-is_pinned = false; + unsigned int xdist, ydist; + struct tp_touch *tmp; + + if (!t-pinned.is_pinned) + return; + + xdist = abs(t-x - t-pinned.center_x); + ydist = abs(t-y - t-pinned.center_y); - if (t-state != TOUCH_END - tp-nfingers_down == 1) - t-is_pointer = true; + if (xdist * xdist + ydist * ydist + tp-buttons.motion_dist * tp-buttons.motion_dist) + return; + + t-pinned.is_pinned = false; + + tp_for_each_touch(tp, tmp) { + if (tmp-is_pointer) break; - } } + + if (t-state != TOUCH_END !tmp-is_pointer) + t-is_pointer = true; } static void -tp_pin_finger(struct tp_dispatch *tp) +tp_pin_fingers(struct tp_dispatch *tp) { - struct tp_touch *t, - *pinned = NULL; + struct tp_touch *t; tp_for_each_touch(tp, t) { - if (t-is_pinned) { - pinned = t; - break; - } - } - - assert(!pinned); - - pinned = tp_current_touch(tp); - - if (tp-nfingers_down != 1) { - tp_for_each_touch(tp, t) { - if (t == pinned) - continue; - - if (t-y pinned-y) - pinned = t; - } + t-is_pointer = false; + t-pinned.is_pinned = true; + t-pinned.center_x = t-x; + t-pinned.center_y = t-y; } - - pinned-is_pinned = true; - pinned-is_pointer = false; } static void @@ -409,16 +404,19 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time) tp_motion_hysteresis(tp, t); tp_motion_history_push(t); + + tp_unpin_finger(tp, t); } - /* We have a physical button down event on a clickpad. For drag and - drop, this means we try to identify which finger pressed the - physical button and pin it, i.e. remove pointer-moving - capabilities from it. + /* +* We have a physical button down event on a clickpad. To avoid +* spurious pointer moves by the clicking finger we pin all fingers. +* We unpin fingers when they move more then a certain threshold to +* to allow drag and drop. */ if ((tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) !tp-buttons.has_buttons) - tp_pin_finger(tp); + tp_pin_fingers(tp); } static void @@ -441,9 +439,6 @@ tp_post_process_state(struct tp_dispatch *tp, uint32_t time) tp-buttons.old_state = tp-buttons.state; - if (tp-queued TOUCHPAD_EVENT_BUTTON_RELEASE) - tp_unpin_finger(tp); - tp-queued = TOUCHPAD_EVENT_NONE; } @@ -796,6 +791,8 @@ tp_init(struct tp_dispatch *tp, tp-hysteresis.margin_y = diagonal / DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR; + tp-buttons.motion_dist = diagonal
[PATCH libinput 10/20] touchpad: Add tp_button_touch_active function
We don't want touches in the button area to cause the pointer to move, add a tp_button_touch_active function which the main code in evdev-mt-touchpad can call to see if a touch should be consider a candidate for being the pointer, should be taken into account for 2 finger scrolling, etc. The idea behind the main code polling for this is that in the future with ie edge scrolling we will have another independent state machine, which may also want to block a touch from being the pointer, so it is best for the main code to test all independent state machines, rather then having the state-machines poke the is_pointer variabel directly. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 5 + src/evdev-mt-touchpad.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index f953cd1..e789a87 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -602,3 +602,8 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time) return rc; } +int +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t) +{ + return t-button.state == BUTTON_STATE_AREA; +} diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 8d8dd84..04da6a6 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -229,4 +229,7 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time); int tp_button_handle_state(struct tp_dispatch *tp, uint32_t time); +int +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t); + #endif -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 20/20] Change internal timestamps to uint64_t to properly deal with wrapping
We store timestamps in ms since system boot (CLOCK_MONOTONIC). This will wrap after circa 50 days. I've considered making our code wrapping safe, but that won't work. We also use our internal timestamps to program timer-fds for timeouts. And we store ms in a single integer but the kernel uses 2 integers, one for seconds and one for usec/nanosec. So at 32 bits our ms containing integer will wrap in 50 days, while the kernels seconds storing integer lasts a lot longer. So when we wrap our ms timestamps, we will be programming the timer-fds with a seconds value in the past. So change all our internal timestamps to uint64_t to avoid the wrapping when programming the timer-fds. Note that we move from 64-bit timestamps to 32-bit timestamps when calling the foo_notify_bar functions from libinput-private.h. Having 64 bit timestamps has no use past this point, since the wayland input protocol uses 32 bit timestamps (and clients will have to deal with wrapping). Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 22 ++--- src/evdev-mt-touchpad-tap.c | 42 - src/evdev-mt-touchpad.c | 24 +++ src/evdev-mt-touchpad.h | 14 +++--- src/evdev-touchpad.c| 26 - src/evdev.c | 12 ++-- src/evdev.h | 2 +- src/filter.c| 16 src/filter.h| 6 +++--- 9 files changed, 82 insertions(+), 82 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index c40b05c..7b1d27b 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -97,7 +97,7 @@ is_inside_left_area(struct tp_dispatch *tp, struct tp_touch *t) } static void -tp_button_set_timer(struct tp_dispatch *tp, uint32_t timeout) +tp_button_set_timer(struct tp_dispatch *tp, uint64_t timeout) { struct itimerspec its; its.it_interval.tv_sec = 0; @@ -287,7 +287,7 @@ static void tp_button_handle_event(struct tp_dispatch *tp, struct tp_touch *t, enum button_event event, - uint32_t time) + uint64_t time) { enum button_state current = t-button.state; @@ -317,7 +317,7 @@ tp_button_handle_event(struct tp_dispatch *tp, } int -tp_button_handle_state(struct tp_dispatch *tp, uint32_t time) +tp_button_handle_state(struct tp_dispatch *tp, uint64_t time) { struct tp_touch *t; @@ -347,7 +347,7 @@ tp_button_handle_state(struct tp_dispatch *tp, uint32_t time) } static void -tp_button_handle_timeout(struct tp_dispatch *tp, uint32_t now) +tp_button_handle_timeout(struct tp_dispatch *tp, uint64_t now) { struct tp_touch *t; @@ -362,7 +362,7 @@ tp_button_handle_timeout(struct tp_dispatch *tp, uint32_t now) int tp_process_button(struct tp_dispatch *tp, const struct input_event *e, - uint32_t time) + uint64_t time) { uint32_t mask = 1 (e-code - BTN_LEFT); @@ -390,7 +390,7 @@ tp_button_timeout_handler(void *data) uint64_t expires; int len; struct timespec ts; - uint32_t now; + uint64_t now; len = read(tp-buttons.timer_fd, expires, sizeof expires); if (len != sizeof expires) @@ -400,7 +400,7 @@ tp_button_timeout_handler(void *data) log_error(timerfd read error: %s\n, strerror(errno)); clock_gettime(CLOCK_MONOTONIC, ts); - now = ts.tv_sec * 1000 + ts.tv_nsec / 100; + now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; tp_button_handle_timeout(tp, now); } @@ -469,7 +469,7 @@ tp_destroy_buttons(struct tp_dispatch *tp) } static int -tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) +tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; enum libinput_pointer_button_state state; @@ -505,7 +505,7 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) } static int -tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) +tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; @@ -537,7 +537,7 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) } static int -tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) +tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; enum libinput_pointer_button_state state; @@ -607,7 +607,7 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) } int -tp_post_button_events(struct tp_dispatch *tp, uint32_t time) +tp_post_button_events(struct tp_dispatch *tp, uint64_t time) { if (tp
[PATCH libinput 08/20] touchpad: Only enable clickfingers on Apple touchpads
From: Peter Hutterer peter.hutte...@who-t.net Apple touchpads don't have visible markings for the software button areas that almost all other vendors use. OS X provides clickfinger behaviour instead, where a click with two fingers on the touchpad generate a right button click. Use that same behaviour in libinput. For all other touchpads, use the software button areas introduced in a follow-up commit. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-buttons.c | 7 ++- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 12 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index c3c97b0..08783a3 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -60,6 +60,11 @@ tp_init_buttons(struct tp_dispatch *tp, tp-buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD; + if (libevdev_get_id_vendor(device-evdev) == 0x5ac) /* Apple */ + tp-buttons.use_clickfinger = true; + else + tp-buttons.use_clickfinger = false; + return 0; } @@ -142,7 +147,7 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time) if (tp-buttons.has_buttons) rc = tp_post_physical_buttons(tp, time); - else + else if (tp-buttons.use_clickfinger) rc = tp_post_clickfinger_buttons(tp, time); return rc; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 85cf7e5..87d291a 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -129,6 +129,7 @@ struct tp_dispatch { struct { bool has_buttons; /* true for physical LMR buttons */ + bool use_clickfinger; /* number of fingers decides button number */ uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ diff --git a/test/touchpad.c b/test/touchpad.c index f4d7839..bbae6cd 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -217,7 +217,7 @@ END_TEST START_TEST(touchpad_1fg_clickfinger) { - struct litest_device *dev = litest_current_device(); + struct litest_device *dev = litest_create_device(LITEST_BCM5974); struct libinput *li = dev-libinput; struct libinput_event *event; struct libinput_event_pointer *ptrev; @@ -237,12 +237,14 @@ START_TEST(touchpad_1fg_clickfinger) LIBINPUT_POINTER_BUTTON_STATE_PRESSED); assert_button_event(li, BTN_LEFT, LIBINPUT_POINTER_BUTTON_STATE_RELEASED); + + litest_delete_device(dev); } END_TEST START_TEST(touchpad_2fg_clickfinger) { - struct litest_device *dev = litest_current_device(); + struct litest_device *dev = litest_create_device(LITEST_BCM5974); struct libinput *li = dev-libinput; struct libinput_event *event; struct libinput_event_pointer *ptrev; @@ -264,6 +266,8 @@ START_TEST(touchpad_2fg_clickfinger) LIBINPUT_POINTER_BUTTON_STATE_PRESSED); assert_button_event(li, BTN_RIGHT, LIBINPUT_POINTER_BUTTON_STATE_RELEASED); + + litest_delete_device(dev); } END_TEST @@ -362,8 +366,8 @@ int main(int argc, char **argv) { litest_add(touchpad:tap, touchpad_1fg_tap_n_drag, LITEST_TOUCHPAD, LITEST_ANY); litest_add(touchpad:tap, touchpad_2fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); - litest_add(touchpad:clickfinger, touchpad_1fg_clickfinger, LITEST_TOUCHPAD, LITEST_ANY); - litest_add(touchpad:clickfinger, touchpad_2fg_clickfinger, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); + litest_add_no_device(touchpad:clickfinger, touchpad_1fg_clickfinger); + litest_add_no_device(touchpad:clickfinger, touchpad_2fg_clickfinger); litest_add(touchpad:click, touchpad_btn_left, LITEST_TOUCHPAD, LITEST_CLICKPAD); litest_add(touchpad:click, clickpad_btn_left, LITEST_CLICKPAD, LITEST_ANY); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 14/20] touchpad: Ignore non left clicks on clickpads
We should never get any non left button events on clickpads, but if we do these might confuse our state, so complain about it and ignore these. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index ec36280..22dbb45 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -370,6 +370,13 @@ tp_process_button(struct tp_dispatch *tp, uint32_t time) { uint32_t mask = 1 (e-code - BTN_LEFT); + + /* Ignore other buttons on clickpads */ + if (tp-buttons.is_clickpad e-code != BTN_LEFT) { + log_bug(received non BTN_LEFT button event on a clickpad (kernel bug?)\n); + return 0; + } + if (e-value) { tp-buttons.state |= mask; tp-queued |= TOUCHPAD_EVENT_BUTTON_PRESS; -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 16/20] touchpad: softbuttons: Deal with a click arriving before any touches
It is possible for a click to get reported before any related touch events get reported, here is the relevant part of an evemu-record session on a T440s: E: 3.985585 # SYN_REPORT (0) -- E: 3.997419 0003 0039 -001 # EV_ABS / ABS_MT_TRACKING_ID -1 E: 3.997419 0001 014a # EV_KEY / BTN_TOUCH0 E: 3.997419 0003 0018 # EV_ABS / ABS_PRESSURE 0 E: 3.997419 0001 0145 # EV_KEY / BTN_TOOL_FINGER 0 E: 3.997419 # SYN_REPORT (0) -- E: 5.117881 0001 0110 0001 # EV_KEY / BTN_LEFT 1 E: 5.117881 # SYN_REPORT (0) -- E: 5.133422 0003 0039 0187 # EV_ABS / ABS_MT_TRACKING_ID 187 E: 5.133422 0003 0035 3098 # EV_ABS / ABS_MT_POSITION_X3098 E: 5.133422 0003 0036 3282 # EV_ABS / ABS_MT_POSITION_Y3282 E: 5.133422 0003 003a 0046 # EV_ABS / ABS_MT_PRESSURE 46 E: 5.133422 0001 014a 0001 # EV_KEY / BTN_TOUCH1 E: 5.133422 0003 3102 # EV_ABS / ABS_X3102 E: 5.133422 0003 0001 3282 # EV_ABS / ABS_Y3282 E: 5.133422 0003 0018 0046 # EV_ABS / ABS_PRESSURE 46 E: 5.133422 0001 0145 0001 # EV_KEY / BTN_TOOL_FINGER 1 E: 5.133422 # SYN_REPORT (0) -- Notice the BTN_LEFT event all by itself! To deal with this if a physical click registers before we get any touches, wait for the first touch to resolve the click. Also see the new activity diagram for the tp_post_softbutton_buttons method which has been added to doc/touchpad-softbutton-state-machine.svg Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- doc/touchpad-softbutton-state-machine.svg | 423 ++ src/evdev-mt-touchpad-buttons.c | 57 ++-- src/evdev-mt-touchpad.h | 1 + 3 files changed, 358 insertions(+), 123 deletions(-) diff --git a/doc/touchpad-softbutton-state-machine.svg b/doc/touchpad-softbutton-state-machine.svg index 1838e35..1142343 100644 --- a/doc/touchpad-softbutton-state-machine.svg +++ b/doc/touchpad-softbutton-state-machine.svg @@ -1,173 +1,390 @@ -svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=516px height=759px version=1.1 +svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=1560px height=1624px version=1.1 defs/ g transform=translate(0.5,0.5) -path d=M 190 352 L 216 352 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ -path d=M 221 352 L 214 355 L 216 352 L 214 348 Z fill=#00 stroke=#00 stroke-miterlimit=10 pointer-events=none/ -ellipse cx=113 cy=61 rx=49.5 ry=30 fill=#ff stroke=#00 stroke-width=2 pointer-events=none/ +path d=M 232 441 L 257 441 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ +path d=M 263 441 L 256 445 L 257 441 L 256 438 Z fill=#00 stroke=#00 stroke-miterlimit=10 pointer-events=none/ +ellipse cx=154 cy=151 rx=49.5 ry=30 fill=#ff stroke=#00 stroke-width=2 pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px -text x=113 y=51 +text x=154 y=141 NONE/text -text x=113 y=65 +text x=154 y=155 on-entry:/text -text x=113 y=79 +text x=154 y=169 curr = none/text /g -rect x=40 y=301 width=150 height=101 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +rect x=82 y=391 width=150 height=101 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px -text x=115 y=335 +text x=157 y=424 BOTTOM_NEW/text -text x=115 y=349 +text x=157 y=438 on-entry:/text -text x=115 y=363 +text x=157 y=452 curr = button/text -text x=115 y=377 +text x=157 y=466 start inner timeout/text /g -rect x=351 y=303 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +rect x=392 y=392 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px -text x=416 y=343 +text x=457 y=432 AREA/text -text x=416 y=357 +text x=457 y=446 on-entry:/text -text x=416 y=371 +text x=457 y=460 curr =area/text /g -path d=M 243 327 C 245 324 249 322 254 322 L 287 322 C 292 322 296 324 298 327 L 318 350 C 319 351 319 353 318 354 L 298 377 C 296 380 292 382 287 382 L 254 382 C 249 382 245 380 243 377 L 223 354 C 222 353 222 351 223 350 L 243 327 Z fill=#ffd966 stroke=#00 stroke-width=2 stroke-miterlimit=10 pointer-events=none/ +path d=M 284 416 C 287 413 291 411 295 411 L 329 411 C 333 411 337 413 340 416 L 360 439 C 361 441 361 442 360 443 L 340 466 C 337 469 333 471 329 471 L 295 471 C 291 471 287 469 284 466 L 264 443 C 264 442 264 441 264 439 L 284 416 Z fill=#ffd966 stroke
[PATCH libinput 15/20] touchpad: post_button_events: Remove TOUCHPAD_EVENT_BUTTON_PRESS/RELEASE test
We already check for old != current everywhere, so this is not needed; And in tp_post_softbutton_buttons we want to do delay button down reporting if we don't have touch info yet in which case this check actually gets in the way. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 4 1 file changed, 4 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 22dbb45..efbc898 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -597,10 +597,6 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) int tp_post_button_events(struct tp_dispatch *tp, uint32_t time) { - if ((tp-queued - (TOUCHPAD_EVENT_BUTTON_PRESS|TOUCHPAD_EVENT_BUTTON_RELEASE)) == 0) - return 0; - if (tp-buttons.is_clickpad) { if (tp-buttons.use_clickfinger) return tp_post_clickfinger_buttons(tp, time); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 18/20] touchpad: Remove clickpad clicked test from 2 finger scrolling handling
This is no longer needed now that we take the button area and pinned fingers into account. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 5 - 1 file changed, 5 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 7333ec9..e7ef357 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -498,11 +498,6 @@ tp_post_scroll_events(struct tp_dispatch *tp, uint32_t time) struct tp_touch *t; int nfingers_down = 0; - /* don't scroll if a clickpad is held down */ - if (tp-buttons.is_buttonpad - (tp-buttons.state || tp-buttons.old_state)) - return 0; - /* Only count active touches for 2 finger scrolling */ tp_for_each_touch(tp, t) { if (tp_touch_active(tp, t)) -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 03/20] touchpad: reset the tap timer_fd to -1 on destroy
From: Peter Hutterer peter.hutte...@who-t.net No real effect, just for safety Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-tap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index 5fa712f..bcc5700 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -615,6 +615,8 @@ tp_destroy_tap(struct tp_dispatch *tp) libinput_remove_source(tp-device-base.seat-libinput, tp-tap.source); tp-tap.source = NULL; } - if (tp-tap.timer_fd -1) + if (tp-tap.timer_fd -1) { close(tp-tap.timer_fd); + tp-tap.timer_fd = -1; + } } -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 04/20] touchpad: don't allow tapping while any button is down
From: Peter Hutterer peter.hutte...@who-t.net Immediately set the state to DEAD, waiting for the tap release to go back to idle. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-tap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index bcc5700..863e004 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -508,7 +508,7 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint32_t time) struct tp_touch *t; int filter_motion = 0; - if (tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) + if (tp-buttons.state != 0) tp_tap_handle_event(tp, TAP_EVENT_BUTTON, time); tp_for_each_touch(tp, t) { -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 11/20] touchpad: Rework is_pointer handling
Move scanning for a suitable touch to be the pointer to tp_process_state and take tp_button_touch_active into account. Note this adds a tp_touch_active helper since we want to do the same checks in other places too (ie to see if a finger should count for 2 finger scrolling). Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 47 +++ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 9df4a78..b671211 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -152,8 +152,6 @@ tp_get_touch(struct tp_dispatch *tp, unsigned int slot) static inline void tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t) { - struct tp_touch *tmp; - if (t-state != TOUCH_UPDATE) { tp_motion_history_reset(t); t-dirty = true; @@ -161,15 +159,6 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t) tp-nfingers_down++; assert(tp-nfingers_down = 1); tp-queued |= TOUCHPAD_EVENT_MOTION; - - tp_for_each_touch(tp, tmp) { - if (tmp-is_pointer) - break; - } - - if (!tmp-is_pointer) { - t-is_pointer = true; - } } } @@ -341,7 +330,6 @@ static void tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t) { unsigned int xdist, ydist; - struct tp_touch *tmp; if (!t-pinned.is_pinned) return; @@ -349,19 +337,9 @@ tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t) xdist = abs(t-x - t-pinned.center_x); ydist = abs(t-y - t-pinned.center_y); - if (xdist * xdist + ydist * ydist + if (xdist * xdist + ydist * ydist = tp-buttons.motion_dist * tp-buttons.motion_dist) - return; - - t-pinned.is_pinned = false; - - tp_for_each_touch(tp, tmp) { - if (tmp-is_pointer) - break; - } - - if (t-state != TOUCH_END !tmp-is_pointer) - t-is_pointer = true; + t-pinned.is_pinned = false; } static void @@ -377,6 +355,13 @@ tp_pin_fingers(struct tp_dispatch *tp) } } +static int +tp_touch_active(struct tp_dispatch *tp, struct tp_touch *t) +{ + return (t-state == TOUCH_BEGIN || t-state == TOUCH_UPDATE) + !t-pinned.is_pinned tp_button_touch_active(tp, t); +} + static void tp_process_state(struct tp_dispatch *tp, uint32_t time) { @@ -409,6 +394,20 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time) if ((tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) !tp-buttons.has_buttons) tp_pin_fingers(tp); + + /* If we don't have a touch as pointer find a suitable one */ + tp_for_each_touch(tp, t) { + if (t-is_pointer) + break; + } + if (!t-is_pointer) { + tp_for_each_touch(tp, t) { + if (tp_touch_active(tp, t)) { + t-is_pointer = true; + break; + } + } + } } static void -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 07/20] touchpad: save the active clickfinger button
From: Peter Hutterer peter.hutte...@who-t.net To avoid having a button left press and a button right release if the number of fingers changes. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-buttons.c | 22 +- src/evdev-mt-touchpad.h | 1 + 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 8946fc7..c3c97b0 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -75,23 +75,27 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) if (current == old) return 0; - switch (tp-nfingers_down) { + if (current) { + switch (tp-nfingers_down) { case 1: button = BTN_LEFT; break; case 2: button = BTN_RIGHT; break; case 3: button = BTN_MIDDLE; break; default: return 0; - } - - if (current) + } + tp-buttons.active = button; state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; - else + } else { + button = tp-buttons.active; + tp-buttons.active = 0; state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + } - pointer_notify_button(tp-device-base, - time, - button, - state); + if (button) + pointer_notify_button(tp-device-base, + time, + button, + state); return 1; } diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index d84c9e8..85cf7e5 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -132,6 +132,7 @@ struct tp_dispatch { uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ + unsigned int active;/* currently active button, for release event */ } buttons; /* physical buttons */ struct { -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 13/20] touchpad: Use INPUT_PROP_BUTTONPAD instead of checking for buttons
And warn if INPUT_PROP_BUTTONPAD mismatches right/middle buttons presence. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 34 ++ src/evdev-mt-touchpad.c | 4 ++-- src/evdev-mt-touchpad.h | 7 +++ 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index e789a87..ec36280 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -410,9 +410,17 @@ tp_init_buttons(struct tp_dispatch *tp, int width, height; double diagonal; + tp-buttons.is_clickpad = libevdev_has_property(device-evdev, +INPUT_PROP_BUTTONPAD); + if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) || - libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) - tp-buttons.has_buttons = true; + libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) { + if (tp-buttons.is_clickpad) + log_bug(clickpad advertising right button (kernel bug?)\n); + } else { + if (!tp-buttons.is_clickpad) + log_bug(non clickpad without right button (kernel bug)?\n); + } width = abs(device-abs.max_x - device-abs.min_x); height = abs(device-abs.max_y - device-abs.min_y); @@ -423,10 +431,7 @@ tp_init_buttons(struct tp_dispatch *tp, if (libevdev_get_id_vendor(device-evdev) == 0x5ac) /* Apple */ tp-buttons.use_clickfinger = true; - tp-buttons.use_softbuttons = !tp-buttons.use_clickfinger - !tp-buttons.has_buttons; - - if (tp-buttons.use_softbuttons) { + if (tp-buttons.is_clickpad !tp-buttons.use_clickfinger) { tp-buttons.area.top_edge = height * .8 + device-abs.min_y; tp-buttons.area.rightbutton_left_edge = width/2 + device-abs.min_x; tp-buttons.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); @@ -585,21 +590,18 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) int tp_post_button_events(struct tp_dispatch *tp, uint32_t time) { - int rc; - if ((tp-queued (TOUCHPAD_EVENT_BUTTON_PRESS|TOUCHPAD_EVENT_BUTTON_RELEASE)) == 0) return 0; - if (tp-buttons.has_buttons) - rc = tp_post_physical_buttons(tp, time); - else if (tp-buttons.use_clickfinger) - rc = tp_post_clickfinger_buttons(tp, time); - else if (tp-buttons.use_softbuttons) - rc = tp_post_softbutton_buttons(tp, time); - + if (tp-buttons.is_clickpad) { + if (tp-buttons.use_clickfinger) + return tp_post_clickfinger_buttons(tp, time); + else + return tp_post_softbutton_buttons(tp, time); + } - return rc; + return tp_post_physical_buttons(tp, time); } int diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index b671211..910bd2a 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -392,7 +392,7 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time) * to allow drag and drop. */ if ((tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) - !tp-buttons.has_buttons) + tp-buttons.is_clickpad) tp_pin_fingers(tp); /* If we don't have a touch as pointer find a suitable one */ @@ -496,7 +496,7 @@ static int tp_post_scroll_events(struct tp_dispatch *tp, uint32_t time) { /* don't scroll if a clickpad is held down */ - if (!tp-buttons.has_buttons + if (tp-buttons.is_buttonpad (tp-buttons.state || tp-buttons.old_state)) return 0; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 04da6a6..5cb9ae2 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -154,19 +154,18 @@ struct tp_dispatch { } accel; struct { - bool has_buttons; /* true for physical LMR buttons */ + bool is_clickpad; /* true for clickpads */ bool use_clickfinger; /* number of fingers decides button number */ - bool use_softbuttons; /* use software-button area */ uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ unsigned int active;/* currently active button, for release event */ - /* Only used if has_buttons is false. The software button area is always + /* Only used for clickpads. The software button area is always * a horizontal strip across
[PATCH libinput 05/20] touchpad: move button-related code into a separate file
From: Peter Hutterer peter.hutte...@who-t.net This is about to become more complicated with the support for software button areas. Move it to a separate file to have it logically grouped together. No functional changes. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- src/Makefile.am | 1 + src/evdev-mt-touchpad-buttons.c | 146 src/evdev-mt-touchpad.c | 102 ++-- src/evdev-mt-touchpad.h | 11 +++ 4 files changed, 162 insertions(+), 98 deletions(-) create mode 100644 src/evdev-mt-touchpad-buttons.c diff --git a/src/Makefile.am b/src/Makefile.am index 579ed25..ffa6a29 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -14,6 +14,7 @@ libinput_la_SOURCES = \ evdev-mt-touchpad.c \ evdev-mt-touchpad.h \ evdev-mt-touchpad-tap.c \ + evdev-mt-touchpad-buttons.c \ evdev-touchpad.c\ filter.c\ filter.h\ diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c new file mode 100644 index 000..8946fc7 --- /dev/null +++ b/src/evdev-mt-touchpad-buttons.c @@ -0,0 +1,146 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided as is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include math.h + +#include evdev-mt-touchpad.h + +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ + +int +tp_process_button(struct tp_dispatch *tp, + const struct input_event *e, + uint32_t time) +{ + uint32_t mask = 1 (e-code - BTN_LEFT); + if (e-value) { + tp-buttons.state |= mask; + tp-queued |= TOUCHPAD_EVENT_BUTTON_PRESS; + } else { + tp-buttons.state = ~mask; + tp-queued |= TOUCHPAD_EVENT_BUTTON_RELEASE; + } + + return 0; +} + +int +tp_init_buttons(struct tp_dispatch *tp, + struct evdev_device *device) +{ + int width, height; + double diagonal; + + if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) || + libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) + tp-buttons.has_buttons = true; + + width = abs(device-abs.max_x - device-abs.min_x); + height = abs(device-abs.max_y - device-abs.min_y); + diagonal = sqrt(width*width + height*height); + + tp-buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD; + + return 0; +} + +static int +tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) +{ + uint32_t current, old, button; + enum libinput_pointer_button_state state; + + current = tp-buttons.state; + old = tp-buttons.old_state; + + if (current == old) + return 0; + + switch (tp-nfingers_down) { + case 1: button = BTN_LEFT; break; + case 2: button = BTN_RIGHT; break; + case 3: button = BTN_MIDDLE; break; + default: + return 0; + } + + if (current) + state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + else + state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + + pointer_notify_button(tp-device-base, + time, + button, + state); + return 1; +} + +static int +tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) +{ + uint32_t current, old, button; + + current = tp-buttons.state; + old = tp-buttons.old_state; + button = BTN_LEFT; + + while (current || old) { + enum libinput_pointer_button_state state; + + if ((current 0x1
[PATCH libinput 06/20] doc: add state machine SVG to EXTRA_DIST
From: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- doc/Makefile.am | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 31b673b..75fa98a 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -1,3 +1,5 @@ +EXTRA_DIST = touchpad-tap-state-machine.svg + if HAVE_DOXYGEN noinst_DATA = html/index.html @@ -12,7 +14,7 @@ clean-local: $(AM_V_at)rm -rf html doc_src= $(shell find html -type f -printf html/%P\n 2/dev/null) -EXTRA_DIST = $(builddir)/html/index.html $(doc_src) +EXTRA_DIST += $(builddir)/html/index.html $(doc_src) endif -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 17/20] touchpad: Ignore fingers in button area for 2 finger scroll
Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 910bd2a..7333ec9 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -442,7 +442,7 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint32_t time) double tmpx, tmpy; tp_for_each_touch(tp, t) { - if (t-dirty) { + if (tp_touch_active(tp, t) t-dirty) { nchanged++; tp_get_delta(t, tmpx, tmpy); @@ -495,12 +495,21 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint32_t time) static int tp_post_scroll_events(struct tp_dispatch *tp, uint32_t time) { + struct tp_touch *t; + int nfingers_down = 0; + /* don't scroll if a clickpad is held down */ if (tp-buttons.is_buttonpad (tp-buttons.state || tp-buttons.old_state)) return 0; - if (tp-nfingers_down != 2) { + /* Only count active touches for 2 finger scrolling */ + tp_for_each_touch(tp, t) { + if (tp_touch_active(tp, t)) + nfingers_down++; + } + + if (nfingers_down != 2) { /* terminate scrolling with a zero scroll event to notify * caller that it really ended now */ if (tp-scroll.state != SCROLL_STATE_NONE) { -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 19/20] touchpad: handle_timeouts: Remove unused return value
Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 3b8b07b..c40b05c 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -346,22 +346,17 @@ tp_button_handle_state(struct tp_dispatch *tp, uint32_t time) return 0; } -static int +static void tp_button_handle_timeout(struct tp_dispatch *tp, uint32_t now) { struct tp_touch *t; - uint32_t min_timeout = INT_MAX; tp_for_each_touch(tp, t) { if (t-button.timeout != 0 t-button.timeout = now) { tp_button_clear_timer(tp, t); tp_button_handle_event(tp, t, BUTTON_EVENT_TIMEOUT, now); } - if (t-button.timeout != 0) - min_timeout = min(t-button.timeout, min_timeout); } - - return min_timeout == INT_MAX ? 0 : min_timeout; } int -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 00/20] clickpad-improvements v2
Hi All, Here is my improved version of Peter's clickpad-improvements patch-set. Changes in v2: * after a click, lock the finger to its current position Pin all fingers until they move more then the threshold in * touchpad: add clickpad-style software buttons Simplify the state machine used in * touchpad: Add tp_button_touch_active function New patches / improvements on to of Peter's original series Please review. Regards, Hans p.s. Peter is on vacation till April 22nd, this respin of his series is posted with his permission. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 12/20] Add a log_bug macro
For logging when things happen which should not happen. We may want to do something more fancy in the future but for now this suffices. Modelled after log_bug in libevdev. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 21627b0..f778f6e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -80,6 +80,7 @@ struct libinput_source; #define log_debug(...) log_msg(LIBINPUT_LOG_PRIORITY_DEBUG, __VA_ARGS__) #define log_info(...) log_msg(LIBINPUT_LOG_PRIORITY_INFO, __VA_ARGS__) #define log_error(...) log_msg(LIBINPUT_LOG_PRIORITY_ERROR, __VA_ARGS__) +#define log_bug(...) log_msg(LIBINPUT_LOG_PRIORITY_ERROR, BUG: __VA_ARGS__) void log_msg(enum libinput_log_priority priority, const char *format, ...); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 01/20] touchpad: set ntouches for single-touch pads depending on key bits
From: Peter Hutterer peter.hutte...@who-t.net A single-touch touchpad that provides BTN_TOOL_TRIPLETAP has 3 touches, etc. There aren't a lot of these out there, but some touchpads don't have slots but do provide two- or three-finger detection. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index bbbd8f3..8021db2 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -715,9 +715,29 @@ tp_init_slots(struct tp_dispatch *tp, tp-slot = absinfo-value; tp-has_mt = true; } else { - tp-ntouches = 5; /* FIXME: based on DOUBLETAP, etc. */ + struct map { + unsigned int code; + int ntouches; + } max_touches[] = { + { BTN_TOOL_QUINTTAP, 5 }, + { BTN_TOOL_QUADTAP, 4 }, + { BTN_TOOL_TRIPLETAP, 3 }, + { BTN_TOOL_DOUBLETAP, 2 }, + }; + struct map *m; + tp-slot = 0; tp-has_mt = false; + tp-ntouches = 1; + + ARRAY_FOR_EACH(max_touches, m) { + if (libevdev_has_event_code(device-evdev, + EV_KEY, + m-code)) { + tp-ntouches = m-ntouches; + break; + } + } } tp-touches = calloc(tp-ntouches, sizeof(struct tp_touch)); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 09/20] touchpad: add clickpad-style software buttons
From: Peter Hutterer peter.hutte...@who-t.net This is a slightly fancier implementation than the simplest model and ported over from libtouchpad. It implements a state machine for the software buttons with left and right buttons currently implemented. Buttons are oriented left-to-right, in a horizontal bar. No random button placement allowed. In general, the procedure is: - if a finger sets down in the left button area, a click is a left click - if a finger sets down in the right button area, a click is a right click - if a finger leaves the button area, a click is a left click - if a finger starts outside the button area, a click is a left click Two timeouts are used to handle buttons more smoothly: - if a finger sets down in a button area but immediately moves over to a different area, that area takes effect on a click. - if a finger leaves a button area and immediately clicks or moves back into the area, the button still takes effect on a click. - if a finger changes between areas and stays there for a timeout, that area takes effect on a click. HdG: -Simplified the state machine a bit -Renamed the button area states to BOTTOM_foo to make it easier to later add support for a top button area such as can be found one the Thinkpad [2-5]40 series. -Init area.top_edge to INT_MAX in the non soft button case to make the entire state machine a nop in that case Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com --- doc/Makefile.am | 2 +- doc/touchpad-softbutton-state-machine.svg | 173 src/evdev-mt-touchpad-buttons.c | 453 +- src/evdev-mt-touchpad.c | 15 + src/evdev-mt-touchpad.h | 48 src/libinput.h| 40 +++ 6 files changed, 728 insertions(+), 3 deletions(-) create mode 100644 doc/touchpad-softbutton-state-machine.svg diff --git a/doc/Makefile.am b/doc/Makefile.am index 75fa98a..a33638d 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -1,4 +1,4 @@ -EXTRA_DIST = touchpad-tap-state-machine.svg +EXTRA_DIST = touchpad-tap-state-machine.svg touchpad-softbutton-state-machine.svg if HAVE_DOXYGEN diff --git a/doc/touchpad-softbutton-state-machine.svg b/doc/touchpad-softbutton-state-machine.svg new file mode 100644 index 000..1838e35 --- /dev/null +++ b/doc/touchpad-softbutton-state-machine.svg @@ -0,0 +1,173 @@ +svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=516px height=759px version=1.1 +defs/ +g transform=translate(0.5,0.5) +path d=M 190 352 L 216 352 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ +path d=M 221 352 L 214 355 L 216 352 L 214 348 Z fill=#00 stroke=#00 stroke-miterlimit=10 pointer-events=none/ +ellipse cx=113 cy=61 rx=49.5 ry=30 fill=#ff stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=113 y=51 +NONE/text +text x=113 y=65 +on-entry:/text +text x=113 y=79 +curr = none/text +/g +rect x=40 y=301 width=150 height=101 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=115 y=335 +BOTTOM_NEW/text +text x=115 y=349 +on-entry:/text +text x=115 y=363 +curr = button/text +text x=115 y=377 +start inner timeout/text +/g +rect x=351 y=303 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=416 y=343 +AREA/text +text x=416 y=357 +on-entry:/text +text x=416 y=371 +curr =area/text +/g +path d=M 243 327 C 245 324 249 322 254 322 L 287 322 C 292 322 296 324 298 327 L 318 350 C 319 351 319 353 318 354 L 298 377 C 296 380 292 382 287 382 L 254 382 C 249 382 245 380 243 377 L 223 354 C 222 353 222 351 223 350 L 243 327 Z fill=#ffd966 stroke=#00 stroke-width=2 stroke-miterlimit=10 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=271 y=349 +finger in/text +text x=271 y=363 +area/text +/g +rect x=50 y=623 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=115 y=677 +BOTTOM/text +/g +path d=M 243 6 C 245 3 249 1 254 1 L 287 1 C 292 1 296 3 298 6 L 318 29 C 319 31 319 32 318 33 L 298 56 C 296 59 292 61 287 61 L 254 61 C 249 61 245 59 243 56 L 223 33 C 222 32 222 31 223 29 L 243 6 Z fill=#ffd966 stroke=#00 stroke-width=2 stroke-miterlimit=10 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=271 y=28 +finger/text +text x=271 y=42 +up/text +/g +path d=M 22 482 C 25 479 29 477 33 477 L 67 477 C 71 477 75 479 78 482 L 98 505 C 99 506 99 508 98 509 L 78 532 C 75 535 71 537
Re: [PATCH libinput 08/20] touchpad: Only enable clickfingers on Apple touchpads
Hi, On 04/15/2014 03:44 PM, Daniel Stone wrote: Hi, On 15 April 2014 13:28, Hans de Goede hdego...@redhat.com wrote: Apple touchpads don't have visible markings for the software button areas that almost all other vendors use. OS X provides clickfinger behaviour instead, where a click with two fingers on the touchpad generate a right button click. Use that same behaviour in libinput. For all other touchpads, use the software button areas introduced in a follow-up commit. Some of the early Samsung Chromebooks had an Apple-style setup: it would be good to expose this as a tuneable so it could be set through configuration. Agreed, but that will have to wait till we have some sort of config framework in place. Creating a config framework is on my / Peter's TODO (who ever gets around to it first). Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 13/20] touchpad: Use INPUT_PROP_BUTTONPAD instead of checking for buttons
Hi, On 04/24/2014 07:34 AM, Peter Hutterer wrote: On Tue, Apr 15, 2014 at 02:28:10PM +0200, Hans de Goede wrote: And warn if INPUT_PROP_BUTTONPAD mismatches right/middle buttons presence. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net I've gone through all patches again and they're now all really Reviewed-by: Peter Hutterer peter.hutte...@who-t.net, except for the bits where things break obviously :) I'll start getting some tests ready for all this so we can verify the behaviour, but meanwhile you can grab the rebased series from my github, wip/clickpad-improvements. Thanks for the review. I'll be busy with the xorg rebase for rawhide for the rest of this week. I'll look into fixing the issues you've found next week. Feel free to beat me to it :) Esp. since I no longer have a laptop with clickpad, making testing all this hard. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] Name-space the scroll event types
Hi, On 04/28/2014 07:38 AM, Peter Hutterer wrote: To provide a generic naming system of type_direction. That will become more important once we add new axes as part of the ongoing work to support graphics tablets. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans --- src/libinput.h | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libinput.h b/src/libinput.h index d771e21..df9a382 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -147,8 +147,14 @@ enum libinput_pointer_button_state { * Axes on a device that are not x or y coordinates. */ enum libinput_pointer_axis { - LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL = 0, - LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL = 1 + LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL = 0, + LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL = 1, + + + /** @deprecated Use @ref LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL instead */ + LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL = LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL, + /** @deprecated Use @ref LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL instead */ + LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL = LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL }; /** ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 10/20] touchpad: Add tp_button_touch_active function
Hi, On 04/24/2014 07:21 AM, Peter Hutterer wrote: On Tue, Apr 15, 2014 at 02:28:07PM +0200, Hans de Goede wrote: We don't want touches in the button area to cause the pointer to move, add a tp_button_touch_active function which the main code in evdev-mt-touchpad can call to see if a touch should be consider a candidate for being the pointer, should be taken into account for 2 finger scrolling, etc. The idea behind the main code polling for this is that in the future with ie edge scrolling we will have another independent state machine, which may also want to block a touch from being the pointer, so it is best for the main code to test all independent state machines, rather then having the state-machines poke the is_pointer variabel directly. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 5 + src/evdev-mt-touchpad.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index f953cd1..e789a87 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -602,3 +602,8 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time) return rc; } +int +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t) +{ +return t-button.state == BUTTON_STATE_AREA; +} diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 8d8dd84..04da6a6 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -229,4 +229,7 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time); int tp_button_handle_state(struct tp_dispatch *tp, uint32_t time); +int +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t); + #endif -- 1.9.0 There's a bit missing here, but it's not exposed until the next patch: for single-touch touchpads, button.state is never set because all touches are marked as fake. It remains on state NONE and thus never contributes to tp_button_touch_active. From what I can tell, the if (fake) continue; bit in tp_button_handle_state is superfluous anyway, the top area is INT_MAX, so we can likely just drop this. After a bit of a pause I'm back to working on this. I agree that this bit can and should just be dropped, I've done so in my local tree / for the next version of this patch-set. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 11/17] touchpad: Ignore non left clicks on clickpads
We should never get any non left button events on clickpads, but if we do these might confuse our state, so complain about it and ignore these. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 76e6843..f1d65be 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -367,6 +367,13 @@ tp_process_button(struct tp_dispatch *tp, uint32_t time) { uint32_t mask = 1 (e-code - BTN_LEFT); + + /* Ignore other buttons on clickpads */ + if (tp-buttons.is_clickpad e-code != BTN_LEFT) { + log_bug(received non BTN_LEFT button event on a clickpad (kernel bug?)\n); + return 0; + } + if (e-value) { tp-buttons.state |= mask; tp-queued |= TOUCHPAD_EVENT_BUTTON_PRESS; -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 16/17] touchpad: handle_timeouts: Remove unused return value
Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index c35d14a..75e1ff7 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -343,22 +343,17 @@ tp_button_handle_state(struct tp_dispatch *tp, uint32_t time) return 0; } -static int +static void tp_button_handle_timeout(struct tp_dispatch *tp, uint32_t now) { struct tp_touch *t; - uint32_t min_timeout = INT_MAX; tp_for_each_touch(tp, t) { if (t-button.timeout != 0 t-button.timeout = now) { tp_button_clear_timer(tp, t); tp_button_handle_event(tp, t, BUTTON_EVENT_TIMEOUT, now); } - if (t-button.timeout != 0) - min_timeout = min(t-button.timeout, min_timeout); } - - return min_timeout == INT_MAX ? 0 : min_timeout; } int -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 07/17] touchpad: Only enable clickfingers on Apple touchpads
From: Peter Hutterer peter.hutte...@who-t.net Apple touchpads don't have visible markings for the software button areas that almost all other vendors use. OS X provides clickfinger behaviour instead, where a click with two fingers on the touchpad generate a right button click. Use that same behaviour in libinput. For all other touchpads, use the software button areas introduced in a follow-up commit. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-buttons.c | 9 +++-- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 12 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 8265e38..ef514f7 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -60,6 +60,11 @@ tp_init_buttons(struct tp_dispatch *tp, tp-buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD; + if (libevdev_get_id_vendor(device-evdev) == 0x5ac) /* Apple */ + tp-buttons.use_clickfinger = true; + else + tp-buttons.use_clickfinger = false; + return 0; } @@ -134,7 +139,7 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) int tp_post_button_events(struct tp_dispatch *tp, uint32_t time) { - int rc; + int rc = 0; if ((tp-queued (TOUCHPAD_EVENT_BUTTON_PRESS|TOUCHPAD_EVENT_BUTTON_RELEASE)) == 0) @@ -142,7 +147,7 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time) if (tp-buttons.has_buttons) rc = tp_post_physical_buttons(tp, time); - else + else if (tp-buttons.use_clickfinger) rc = tp_post_clickfinger_buttons(tp, time); return rc; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 85cf7e5..87d291a 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -129,6 +129,7 @@ struct tp_dispatch { struct { bool has_buttons; /* true for physical LMR buttons */ + bool use_clickfinger; /* number of fingers decides button number */ uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ diff --git a/test/touchpad.c b/test/touchpad.c index f4d7839..bbae6cd 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -217,7 +217,7 @@ END_TEST START_TEST(touchpad_1fg_clickfinger) { - struct litest_device *dev = litest_current_device(); + struct litest_device *dev = litest_create_device(LITEST_BCM5974); struct libinput *li = dev-libinput; struct libinput_event *event; struct libinput_event_pointer *ptrev; @@ -237,12 +237,14 @@ START_TEST(touchpad_1fg_clickfinger) LIBINPUT_POINTER_BUTTON_STATE_PRESSED); assert_button_event(li, BTN_LEFT, LIBINPUT_POINTER_BUTTON_STATE_RELEASED); + + litest_delete_device(dev); } END_TEST START_TEST(touchpad_2fg_clickfinger) { - struct litest_device *dev = litest_current_device(); + struct litest_device *dev = litest_create_device(LITEST_BCM5974); struct libinput *li = dev-libinput; struct libinput_event *event; struct libinput_event_pointer *ptrev; @@ -264,6 +266,8 @@ START_TEST(touchpad_2fg_clickfinger) LIBINPUT_POINTER_BUTTON_STATE_PRESSED); assert_button_event(li, BTN_RIGHT, LIBINPUT_POINTER_BUTTON_STATE_RELEASED); + + litest_delete_device(dev); } END_TEST @@ -362,8 +366,8 @@ int main(int argc, char **argv) { litest_add(touchpad:tap, touchpad_1fg_tap_n_drag, LITEST_TOUCHPAD, LITEST_ANY); litest_add(touchpad:tap, touchpad_2fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); - litest_add(touchpad:clickfinger, touchpad_1fg_clickfinger, LITEST_TOUCHPAD, LITEST_ANY); - litest_add(touchpad:clickfinger, touchpad_2fg_clickfinger, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); + litest_add_no_device(touchpad:clickfinger, touchpad_1fg_clickfinger); + litest_add_no_device(touchpad:clickfinger, touchpad_2fg_clickfinger); litest_add(touchpad:click, touchpad_btn_left, LITEST_TOUCHPAD, LITEST_CLICKPAD); litest_add(touchpad:click, clickpad_btn_left, LITEST_CLICKPAD, LITEST_ANY); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 10/17] touchpad: Use INPUT_PROP_BUTTONPAD instead of checking for buttons
And warn if INPUT_PROP_BUTTONPAD mismatches right/middle buttons presence. Also fix the bcm5974 to properly advertise INPUT_PROP_BUTTONPAD. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 34 ++ src/evdev-mt-touchpad.c | 4 ++-- src/evdev-mt-touchpad.h | 7 +++ test/litest-bcm5974.c | 1 + 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index e47a55e..76e6843 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -407,9 +407,17 @@ tp_init_buttons(struct tp_dispatch *tp, int width, height; double diagonal; + tp-buttons.is_clickpad = libevdev_has_property(device-evdev, + INPUT_PROP_BUTTONPAD); + if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) || - libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) - tp-buttons.has_buttons = true; + libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) { + if (tp-buttons.is_clickpad) + log_bug(clickpad advertising right button (kernel bug?)\n); + } else { + if (!tp-buttons.is_clickpad) + log_bug(non clickpad without right button (kernel bug)?\n); + } width = abs(device-abs.max_x - device-abs.min_x); height = abs(device-abs.max_y - device-abs.min_y); @@ -420,10 +428,7 @@ tp_init_buttons(struct tp_dispatch *tp, if (libevdev_get_id_vendor(device-evdev) == 0x5ac) /* Apple */ tp-buttons.use_clickfinger = true; - tp-buttons.use_softbuttons = !tp-buttons.use_clickfinger - !tp-buttons.has_buttons; - - if (tp-buttons.use_softbuttons) { + if (tp-buttons.is_clickpad !tp-buttons.use_clickfinger) { tp-buttons.area.top_edge = height * .8 + device-abs.min_y; tp-buttons.area.rightbutton_left_edge = width/2 + device-abs.min_x; tp-buttons.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); @@ -583,21 +588,18 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) int tp_post_button_events(struct tp_dispatch *tp, uint32_t time) { - int rc = 0; - if ((tp-queued (TOUCHPAD_EVENT_BUTTON_PRESS|TOUCHPAD_EVENT_BUTTON_RELEASE)) == 0) return 0; - if (tp-buttons.has_buttons) - rc = tp_post_physical_buttons(tp, time); - else if (tp-buttons.use_clickfinger) - rc = tp_post_clickfinger_buttons(tp, time); - else if (tp-buttons.use_softbuttons) - rc = tp_post_softbutton_buttons(tp, time); - + if (tp-buttons.is_clickpad) { + if (tp-buttons.use_clickfinger) + return tp_post_clickfinger_buttons(tp, time); + else + return tp_post_softbutton_buttons(tp, time); + } - return rc; + return tp_post_physical_buttons(tp, time); } int diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 924ba16..f5e0300 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -409,7 +409,7 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time) * to allow drag and drop. */ if ((tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) - !tp-buttons.has_buttons) + tp-buttons.is_clickpad) tp_pin_fingers(tp); } @@ -499,7 +499,7 @@ static int tp_post_scroll_events(struct tp_dispatch *tp, uint32_t time) { /* don't scroll if a clickpad is held down */ - if (!tp-buttons.has_buttons + if (tp-buttons.is_clickpad (tp-buttons.state || tp-buttons.old_state)) return 0; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 5509e93..78a74df 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -154,19 +154,18 @@ struct tp_dispatch { } accel; struct { - bool has_buttons; /* true for physical LMR buttons */ + bool is_clickpad; /* true for clickpads */ bool use_clickfinger; /* number of fingers decides button number */ - bool use_softbuttons; /* use software-button area */ uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ unsigned int active;/* currently active button, for release event */ - /* Only used if has_buttons is false. The software button area is always + /* Only used for clickpads. The software button area is always
[PATCH libinput v3 00/17] clickpad-improvements v3
Hans de Goede Tue, 15 Apr 2014 05:29:33 -0700 Hi All, Here is the 3th version of the clickpad-improvements patch-set Peter and I have been working on. Changes in v2: -after a click, lock the finger to its current position Pin all fingers until they move more then the threshold in -touchpad: add clickpad-style software buttons Simplify the state machine used in -touchpad: Add tp_button_touch_active function New patches / improvements on to of Peter's original series Changes in v3: -Based on Peter's rebased version of my v2 of this patch series -Dropped: touchpad: don't allow tapping while any button is down -Misc. coding style fixes -Fix a bunch of compiler warnings -Changed DONT EDIT THIS FILE comment in evdev-mt-touchpad-buttons.c to BEFORE YOU EDIT THIS FILE -Drop if (fake) continue test in tp_button_handle_state(), it is not necessary and causes issues for single touch touchpads -touchpad: after a click, lock the finger to its current position Reset pinned.is_pinned on touch begin, so that fingers coming down after a click don't start pinned -Merged touchpad: Add tp_button_touch_active function and touchpad: Rework is_pointer handling into a single patch -Fixed touchpad: Rework is_pointer handling issue, changed the logic to only set a touch as pointer when it transitions to BUTTON_STATE_AREA or is unpinned -Fixed a bunch of tests failing because the bcm5974 device was not properly advertising INPUT_PROP_BUTTONPAD Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 04/17] touchpad: move button-related code into a separate file
From: Peter Hutterer peter.hutte...@who-t.net This is about to become more complicated with the support for software button areas. Move it to a separate file to have it logically grouped together. No functional changes. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Hans de Goede hdego...@redhat.com --- src/Makefile.am | 1 + src/evdev-mt-touchpad-buttons.c | 145 src/evdev-mt-touchpad.c | 102 ++-- src/evdev-mt-touchpad.h | 11 +++ 4 files changed, 161 insertions(+), 98 deletions(-) create mode 100644 src/evdev-mt-touchpad-buttons.c diff --git a/src/Makefile.am b/src/Makefile.am index 579ed25..ffa6a29 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -14,6 +14,7 @@ libinput_la_SOURCES = \ evdev-mt-touchpad.c \ evdev-mt-touchpad.h \ evdev-mt-touchpad-tap.c \ + evdev-mt-touchpad-buttons.c \ evdev-touchpad.c\ filter.c\ filter.h\ diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c new file mode 100644 index 000..1d54c6f --- /dev/null +++ b/src/evdev-mt-touchpad-buttons.c @@ -0,0 +1,145 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided as is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include math.h + +#include evdev-mt-touchpad.h + +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ + +int +tp_process_button(struct tp_dispatch *tp, + const struct input_event *e, + uint32_t time) +{ + uint32_t mask = 1 (e-code - BTN_LEFT); + if (e-value) { + tp-buttons.state |= mask; + tp-queued |= TOUCHPAD_EVENT_BUTTON_PRESS; + } else { + tp-buttons.state = ~mask; + tp-queued |= TOUCHPAD_EVENT_BUTTON_RELEASE; + } + + return 0; +} + +int +tp_init_buttons(struct tp_dispatch *tp, + struct evdev_device *device) +{ + int width, height; + double diagonal; + + if (libevdev_has_event_code(device-evdev, EV_KEY, BTN_MIDDLE) || + libevdev_has_event_code(device-evdev, EV_KEY, BTN_RIGHT)) + tp-buttons.has_buttons = true; + + width = abs(device-abs.max_x - device-abs.min_x); + height = abs(device-abs.max_y - device-abs.min_y); + diagonal = sqrt(width*width + height*height); + + tp-buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD; + + return 0; +} + +static int +tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) +{ + uint32_t current, old, button; + enum libinput_pointer_button_state state; + + current = tp-buttons.state; + old = tp-buttons.old_state; + + if (current == old) + return 0; + + switch (tp-nfingers_down) { + case 1: button = BTN_LEFT; break; + case 2: button = BTN_RIGHT; break; + case 3: button = BTN_MIDDLE; break; + default: + return 0; + } + + if (current) + state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; + else + state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + + pointer_notify_button(tp-device-base, + time, + button, + state); + return 1; +} + +static int +tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) +{ + uint32_t current, old, button; + + current = tp-buttons.state; + old = tp-buttons.old_state; + button = BTN_LEFT; + + while (current || old) { + enum
[PATCH libinput v3 03/17] touchpad: reset the tap timer_fd to -1 on destroy
From: Peter Hutterer peter.hutte...@who-t.net No real effect, just for safety Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-tap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index 5fa712f..bcc5700 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -615,6 +615,8 @@ tp_destroy_tap(struct tp_dispatch *tp) libinput_remove_source(tp-device-base.seat-libinput, tp-tap.source); tp-tap.source = NULL; } - if (tp-tap.timer_fd -1) + if (tp-tap.timer_fd -1) { close(tp-tap.timer_fd); + tp-tap.timer_fd = -1; + } } -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 02/17] touchpad: after a click, lock the finger to its current position
From: Peter Hutterer peter.hutte...@who-t.net On clickpads, clicking the pad usually causes some motion events. To avoid erroneous movements, lock the finger into position on the click and don't allow for motion events until we move past a given threshold (currently 2% of the touchpad diagonal). HdG: Instead of pinning the lowest touch assuming that that is the one holding the physical button, simply pin all touches, and unpin as soon as a touch is moved more then the threshold. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 80 - src/evdev-mt-touchpad.h | 12 +++- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index b4a89e3..381bb90 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -32,6 +32,7 @@ #define DEFAULT_MIN_ACCEL_FACTOR 0.16 #define DEFAULT_MAX_ACCEL_FACTOR 1.0 #define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0 +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ static inline int tp_hysteresis(int in, int center, int margin) @@ -158,6 +159,7 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t) tp_motion_history_reset(t); t-dirty = true; t-state = TOUCH_BEGIN; + t-pinned.is_pinned = false; tp-nfingers_down++; assert(tp-nfingers_down = 1); tp-queued |= TOUCHPAD_EVENT_MOTION; @@ -182,6 +184,7 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t) t-dirty = true; t-is_pointer = false; t-state = TOUCH_END; + t-pinned.is_pinned = false; assert(tp-nfingers_down = 1); tp-nfingers_down--; tp-queued |= TOUCHPAD_EVENT_MOTION; @@ -346,50 +349,43 @@ tp_process_key(struct tp_dispatch *tp, } static void -tp_unpin_finger(struct tp_dispatch *tp) +tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t) { - struct tp_touch *t; - tp_for_each_touch(tp, t) { - if (t-is_pinned) { - t-is_pinned = false; + unsigned int xdist, ydist; + struct tp_touch *tmp = NULL; + + if (!t-pinned.is_pinned) + return; + + xdist = abs(t-x - t-pinned.center_x); + ydist = abs(t-y - t-pinned.center_y); - if (t-state != TOUCH_END - tp-nfingers_down == 1) - t-is_pointer = true; + if (xdist * xdist + ydist * ydist + tp-buttons.motion_dist * tp-buttons.motion_dist) + return; + + t-pinned.is_pinned = false; + + tp_for_each_touch(tp, tmp) { + if (tmp-is_pointer) break; - } } + + if (t-state != TOUCH_END !tmp-is_pointer) + t-is_pointer = true; } static void -tp_pin_finger(struct tp_dispatch *tp) +tp_pin_fingers(struct tp_dispatch *tp) { - struct tp_touch *t, - *pinned = NULL; + struct tp_touch *t; tp_for_each_touch(tp, t) { - if (t-is_pinned) { - pinned = t; - break; - } - } - - assert(!pinned); - - pinned = tp_current_touch(tp); - - if (tp-nfingers_down != 1) { - tp_for_each_touch(tp, t) { - if (t == pinned) - continue; - - if (t-y pinned-y) - pinned = t; - } + t-is_pointer = false; + t-pinned.is_pinned = true; + t-pinned.center_x = t-x; + t-pinned.center_y = t-y; } - - pinned-is_pinned = true; - pinned-is_pointer = false; } static void @@ -409,16 +405,19 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time) tp_motion_hysteresis(tp, t); tp_motion_history_push(t); + + tp_unpin_finger(tp, t); } - /* We have a physical button down event on a clickpad. For drag and - drop, this means we try to identify which finger pressed the - physical button and pin it, i.e. remove pointer-moving - capabilities from it. + /* +* We have a physical button down event on a clickpad. To avoid +* spurious pointer moves by the clicking finger we pin all fingers. +* We unpin fingers when they move more then a certain threshold to +* to allow drag and drop. */ if ((tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) !tp-buttons.has_buttons) - tp_pin_finger(tp
[PATCH libinput v3 17/17] Change internal timestamps to uint64_t to properly deal with wrapping
We store timestamps in ms since system boot (CLOCK_MONOTONIC). This will wrap after circa 50 days. I've considered making our code wrapping safe, but that won't work. We also use our internal timestamps to program timer-fds for timeouts. And we store ms in a single integer but the kernel uses 2 integers, one for seconds and one for usec/nanosec. So at 32 bits our ms containing integer will wrap in 50 days, while the kernels seconds storing integer lasts a lot longer. So when we wrap our ms timestamps, we will be programming the timer-fds with a seconds value in the past. So change all our internal timestamps to uint64_t to avoid the wrapping when programming the timer-fds. Note that we move from 64-bit timestamps to 32-bit timestamps when calling the foo_notify_bar functions from libinput-private.h. Having 64 bit timestamps has no use past this point, since the wayland input protocol uses 32 bit timestamps (and clients will have to deal with wrapping). Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 22 ++--- src/evdev-mt-touchpad-tap.c | 42 - src/evdev-mt-touchpad.c | 24 +++ src/evdev-mt-touchpad.h | 14 +++--- src/evdev-touchpad.c| 26 - src/evdev.c | 12 ++-- src/evdev.h | 2 +- src/filter.c| 16 src/filter.h| 6 +++--- 9 files changed, 82 insertions(+), 82 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 75e1ff7..9f2d7f9 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -95,7 +95,7 @@ is_inside_left_area(struct tp_dispatch *tp, struct tp_touch *t) } static void -tp_button_set_timer(struct tp_dispatch *tp, uint32_t timeout) +tp_button_set_timer(struct tp_dispatch *tp, uint64_t timeout) { struct itimerspec its; its.it_interval.tv_sec = 0; @@ -286,7 +286,7 @@ static void tp_button_handle_event(struct tp_dispatch *tp, struct tp_touch *t, enum button_event event, - uint32_t time) + uint64_t time) { enum button_state current = t-button.state; @@ -316,7 +316,7 @@ tp_button_handle_event(struct tp_dispatch *tp, } int -tp_button_handle_state(struct tp_dispatch *tp, uint32_t time) +tp_button_handle_state(struct tp_dispatch *tp, uint64_t time) { struct tp_touch *t; @@ -344,7 +344,7 @@ tp_button_handle_state(struct tp_dispatch *tp, uint32_t time) } static void -tp_button_handle_timeout(struct tp_dispatch *tp, uint32_t now) +tp_button_handle_timeout(struct tp_dispatch *tp, uint64_t now) { struct tp_touch *t; @@ -359,7 +359,7 @@ tp_button_handle_timeout(struct tp_dispatch *tp, uint32_t now) int tp_process_button(struct tp_dispatch *tp, const struct input_event *e, - uint32_t time) + uint64_t time) { uint32_t mask = 1 (e-code - BTN_LEFT); @@ -387,7 +387,7 @@ tp_button_timeout_handler(void *data) uint64_t expires; int len; struct timespec ts; - uint32_t now; + uint64_t now; len = read(tp-buttons.timer_fd, expires, sizeof expires); if (len != sizeof expires) @@ -397,7 +397,7 @@ tp_button_timeout_handler(void *data) log_error(timerfd read error: %s\n, strerror(errno)); clock_gettime(CLOCK_MONOTONIC, ts); - now = ts.tv_sec * 1000 + ts.tv_nsec / 100; + now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; tp_button_handle_timeout(tp, now); } @@ -467,7 +467,7 @@ tp_destroy_buttons(struct tp_dispatch *tp) } static int -tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) +tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; enum libinput_pointer_button_state state; @@ -503,7 +503,7 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) } static int -tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) +tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; @@ -535,7 +535,7 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint32_t time) } static int -tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) +tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) { uint32_t current, old, button; enum libinput_pointer_button_state state; @@ -605,7 +605,7 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) } int -tp_post_button_events(struct tp_dispatch *tp, uint32_t time) +tp_post_button_events(struct tp_dispatch *tp, uint64_t time) { if (tp
[PATCH libinput v3 06/17] touchpad: save the active clickfinger button
From: Peter Hutterer peter.hutte...@who-t.net To avoid having a button left press and a button right release if the number of fingers changes. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-buttons.c | 22 +- src/evdev-mt-touchpad.h | 1 + 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 1d54c6f..8265e38 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -75,23 +75,27 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint32_t time) if (current == old) return 0; - switch (tp-nfingers_down) { + if (current) { + switch (tp-nfingers_down) { case 1: button = BTN_LEFT; break; case 2: button = BTN_RIGHT; break; case 3: button = BTN_MIDDLE; break; default: return 0; - } - - if (current) + } + tp-buttons.active = button; state = LIBINPUT_POINTER_BUTTON_STATE_PRESSED; - else + } else { + button = tp-buttons.active; + tp-buttons.active = 0; state = LIBINPUT_POINTER_BUTTON_STATE_RELEASED; + } - pointer_notify_button(tp-device-base, - time, - button, - state); + if (button) + pointer_notify_button(tp-device-base, + time, + button, + state); return 1; } diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index d84c9e8..85cf7e5 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -132,6 +132,7 @@ struct tp_dispatch { uint32_t state; uint32_t old_state; uint32_t motion_dist; /* for pinned touches */ + unsigned int active;/* currently active button, for release event */ } buttons; /* physical buttons */ struct { -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 01/17] touchpad: set ntouches for single-touch pads depending on key bits
From: Peter Hutterer peter.hutte...@who-t.net A single-touch touchpad that provides BTN_TOOL_TRIPLETAP has 3 touches, etc. There aren't a lot of these out there, but some touchpads don't have slots but do provide two- or three-finger detection. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 109441d..b4a89e3 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -715,9 +715,29 @@ tp_init_slots(struct tp_dispatch *tp, tp-slot = absinfo-value; tp-has_mt = true; } else { - tp-ntouches = 5; /* FIXME: based on DOUBLETAP, etc. */ + struct map { + unsigned int code; + int ntouches; + } max_touches[] = { + { BTN_TOOL_QUINTTAP, 5 }, + { BTN_TOOL_QUADTAP, 4 }, + { BTN_TOOL_TRIPLETAP, 3 }, + { BTN_TOOL_DOUBLETAP, 2 }, + }; + struct map *m; + tp-slot = 0; tp-has_mt = false; + tp-ntouches = 1; + + ARRAY_FOR_EACH(max_touches, m) { + if (libevdev_has_event_code(device-evdev, + EV_KEY, + m-code)) { + tp-ntouches = m-ntouches; + break; + } + } } tp-touches = calloc(tp-ntouches, sizeof(struct tp_touch)); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 14/17] touchpad: Ignore fingers in button area for 2 finger scroll
Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index f5e0300..fc4a4f7 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -445,7 +445,7 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint32_t time) double tmpx, tmpy; tp_for_each_touch(tp, t) { - if (t-dirty) { + if (tp_touch_active(tp, t) t-dirty) { nchanged++; tp_get_delta(t, tmpx, tmpy); @@ -498,12 +498,21 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint32_t time) static int tp_post_scroll_events(struct tp_dispatch *tp, uint32_t time) { + struct tp_touch *t; + int nfingers_down = 0; + /* don't scroll if a clickpad is held down */ if (tp-buttons.is_clickpad (tp-buttons.state || tp-buttons.old_state)) return 0; - if (tp-nfingers_down != 2) { + /* Only count active touches for 2 finger scrolling */ + tp_for_each_touch(tp, t) { + if (tp_touch_active(tp, t)) + nfingers_down++; + } + + if (nfingers_down != 2) { /* terminate scrolling with a zero scroll event to notify * caller that it really ended now */ if (tp-scroll.state != SCROLL_STATE_NONE) { -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 05/17] doc: add state machine SVG to EXTRA_DIST
From: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Hans de Goede hdego...@redhat.com --- doc/Makefile.am | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 31b673b..75fa98a 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -1,3 +1,5 @@ +EXTRA_DIST = touchpad-tap-state-machine.svg + if HAVE_DOXYGEN noinst_DATA = html/index.html @@ -12,7 +14,7 @@ clean-local: $(AM_V_at)rm -rf html doc_src= $(shell find html -type f -printf html/%P\n 2/dev/null) -EXTRA_DIST = $(builddir)/html/index.html $(doc_src) +EXTRA_DIST += $(builddir)/html/index.html $(doc_src) endif -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 12/17] touchpad: post_button_events: Remove TOUCHPAD_EVENT_BUTTON_PRESS/RELEASE test
We already check for old != current everywhere, so this is not needed; And in tp_post_softbutton_buttons we want to do delay button down reporting if we don't have touch info yet in which case this check actually gets in the way. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 4 1 file changed, 4 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index f1d65be..3206ccb 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -595,10 +595,6 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint32_t time) int tp_post_button_events(struct tp_dispatch *tp, uint32_t time) { - if ((tp-queued - (TOUCHPAD_EVENT_BUTTON_PRESS|TOUCHPAD_EVENT_BUTTON_RELEASE)) == 0) - return 0; - if (tp-buttons.is_clickpad) { if (tp-buttons.use_clickfinger) return tp_post_clickfinger_buttons(tp, time); -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 13/17] touchpad: softbuttons: Deal with a click arriving before any touches
It is possible for a click to get reported before any related touch events get reported, here is the relevant part of an evemu-record session on a T440s: E: 3.985585 # SYN_REPORT (0) -- E: 3.997419 0003 0039 -001 # EV_ABS / ABS_MT_TRACKING_ID -1 E: 3.997419 0001 014a # EV_KEY / BTN_TOUCH0 E: 3.997419 0003 0018 # EV_ABS / ABS_PRESSURE 0 E: 3.997419 0001 0145 # EV_KEY / BTN_TOOL_FINGER 0 E: 3.997419 # SYN_REPORT (0) -- E: 5.117881 0001 0110 0001 # EV_KEY / BTN_LEFT 1 E: 5.117881 # SYN_REPORT (0) -- E: 5.133422 0003 0039 0187 # EV_ABS / ABS_MT_TRACKING_ID 187 E: 5.133422 0003 0035 3098 # EV_ABS / ABS_MT_POSITION_X3098 E: 5.133422 0003 0036 3282 # EV_ABS / ABS_MT_POSITION_Y3282 E: 5.133422 0003 003a 0046 # EV_ABS / ABS_MT_PRESSURE 46 E: 5.133422 0001 014a 0001 # EV_KEY / BTN_TOUCH1 E: 5.133422 0003 3102 # EV_ABS / ABS_X3102 E: 5.133422 0003 0001 3282 # EV_ABS / ABS_Y3282 E: 5.133422 0003 0018 0046 # EV_ABS / ABS_PRESSURE 46 E: 5.133422 0001 0145 0001 # EV_KEY / BTN_TOOL_FINGER 1 E: 5.133422 # SYN_REPORT (0) -- Notice the BTN_LEFT event all by itself! To deal with this if a physical click registers before we get any touches, wait for the first touch to resolve the click. Also see the new activity diagram for the tp_post_softbutton_buttons method which has been added to doc/touchpad-softbutton-state-machine.svg Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- doc/touchpad-softbutton-state-machine.svg | 423 ++ src/evdev-mt-touchpad-buttons.c | 57 ++-- src/evdev-mt-touchpad.h | 1 + 3 files changed, 358 insertions(+), 123 deletions(-) diff --git a/doc/touchpad-softbutton-state-machine.svg b/doc/touchpad-softbutton-state-machine.svg index 1838e35..1142343 100644 --- a/doc/touchpad-softbutton-state-machine.svg +++ b/doc/touchpad-softbutton-state-machine.svg @@ -1,173 +1,390 @@ -svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=516px height=759px version=1.1 +svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=1560px height=1624px version=1.1 defs/ g transform=translate(0.5,0.5) -path d=M 190 352 L 216 352 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ -path d=M 221 352 L 214 355 L 216 352 L 214 348 Z fill=#00 stroke=#00 stroke-miterlimit=10 pointer-events=none/ -ellipse cx=113 cy=61 rx=49.5 ry=30 fill=#ff stroke=#00 stroke-width=2 pointer-events=none/ +path d=M 232 441 L 257 441 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ +path d=M 263 441 L 256 445 L 257 441 L 256 438 Z fill=#00 stroke=#00 stroke-miterlimit=10 pointer-events=none/ +ellipse cx=154 cy=151 rx=49.5 ry=30 fill=#ff stroke=#00 stroke-width=2 pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px -text x=113 y=51 +text x=154 y=141 NONE/text -text x=113 y=65 +text x=154 y=155 on-entry:/text -text x=113 y=79 +text x=154 y=169 curr = none/text /g -rect x=40 y=301 width=150 height=101 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +rect x=82 y=391 width=150 height=101 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px -text x=115 y=335 +text x=157 y=424 BOTTOM_NEW/text -text x=115 y=349 +text x=157 y=438 on-entry:/text -text x=115 y=363 +text x=157 y=452 curr = button/text -text x=115 y=377 +text x=157 y=466 start inner timeout/text /g -rect x=351 y=303 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +rect x=392 y=392 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px -text x=416 y=343 +text x=457 y=432 AREA/text -text x=416 y=357 +text x=457 y=446 on-entry:/text -text x=416 y=371 +text x=457 y=460 curr =area/text /g -path d=M 243 327 C 245 324 249 322 254 322 L 287 322 C 292 322 296 324 298 327 L 318 350 C 319 351 319 353 318 354 L 298 377 C 296 380 292 382 287 382 L 254 382 C 249 382 245 380 243 377 L 223 354 C 222 353 222 351 223 350 L 243 327 Z fill=#ffd966 stroke=#00 stroke-width=2 stroke-miterlimit=10 pointer-events=none/ +path d=M 284 416 C 287 413 291 411 295 411 L 329 411 C 333 411 337 413 340 416 L 360 439 C 361 441 361 442 360 443 L 340 466 C 337 469 333 471 329 471 L 295 471 C 291 471 287 469 284 466 L 264 443 C 264 442 264 441 264 439 L 284 416 Z fill=#ffd966
[PATCH libinput v3 08/17] touchpad: Add clickpad-style software buttons
From: Peter Hutterer peter.hutte...@who-t.net This is a slightly fancier implementation than the simplest model and ported over from libtouchpad. It implements a state machine for the software buttons with left and right buttons currently implemented. Buttons are oriented left-to-right, in a horizontal bar. No random button placement allowed. In general, the procedure is: - if a finger sets down in the left button area, a click is a left click - if a finger sets down in the right button area, a click is a right click - if a finger leaves the button area, a click is a left click - if a finger starts outside the button area, a click is a left click Two timeouts are used to handle buttons more smoothly: - if a finger sets down in a button area but immediately moves over to a different area, that area takes effect on a click. - if a finger leaves a button area and immediately clicks or moves back into the area, the button still takes effect on a click. - if a finger changes between areas and stays there for a timeout, that area takes effect on a click. HdG: -Simplified the state machine a bit -Renamed the button area states to BOTTOM_foo to make it easier to later add support for a top button area such as can be found one the Thinkpad [2-5]40 series. -Init area.top_edge to INT_MAX in the non soft button case to make the entire state machine a nop in that case Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- doc/Makefile.am | 2 +- doc/touchpad-softbutton-state-machine.svg | 173 src/evdev-mt-touchpad-buttons.c | 450 +- src/evdev-mt-touchpad.c | 15 + src/evdev-mt-touchpad.h | 48 src/libinput.h| 40 +++ 6 files changed, 725 insertions(+), 3 deletions(-) create mode 100644 doc/touchpad-softbutton-state-machine.svg diff --git a/doc/Makefile.am b/doc/Makefile.am index 75fa98a..a33638d 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -1,4 +1,4 @@ -EXTRA_DIST = touchpad-tap-state-machine.svg +EXTRA_DIST = touchpad-tap-state-machine.svg touchpad-softbutton-state-machine.svg if HAVE_DOXYGEN diff --git a/doc/touchpad-softbutton-state-machine.svg b/doc/touchpad-softbutton-state-machine.svg new file mode 100644 index 000..1838e35 --- /dev/null +++ b/doc/touchpad-softbutton-state-machine.svg @@ -0,0 +1,173 @@ +svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=516px height=759px version=1.1 +defs/ +g transform=translate(0.5,0.5) +path d=M 190 352 L 216 352 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ +path d=M 221 352 L 214 355 L 216 352 L 214 348 Z fill=#00 stroke=#00 stroke-miterlimit=10 pointer-events=none/ +ellipse cx=113 cy=61 rx=49.5 ry=30 fill=#ff stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=113 y=51 +NONE/text +text x=113 y=65 +on-entry:/text +text x=113 y=79 +curr = none/text +/g +rect x=40 y=301 width=150 height=101 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=115 y=335 +BOTTOM_NEW/text +text x=115 y=349 +on-entry:/text +text x=115 y=363 +curr = button/text +text x=115 y=377 +start inner timeout/text +/g +rect x=351 y=303 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=416 y=343 +AREA/text +text x=416 y=357 +on-entry:/text +text x=416 y=371 +curr =area/text +/g +path d=M 243 327 C 245 324 249 322 254 322 L 287 322 C 292 322 296 324 298 327 L 318 350 C 319 351 319 353 318 354 L 298 377 C 296 380 292 382 287 382 L 254 382 C 249 382 245 380 243 377 L 223 354 C 222 353 222 351 223 350 L 243 327 Z fill=#ffd966 stroke=#00 stroke-width=2 stroke-miterlimit=10 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=271 y=349 +finger in/text +text x=271 y=363 +area/text +/g +rect x=50 y=623 width=130 height=100 rx=6 ry=6 fill=#ccffcc stroke=#00 stroke-width=2 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=115 y=677 +BOTTOM/text +/g +path d=M 243 6 C 245 3 249 1 254 1 L 287 1 C 292 1 296 3 298 6 L 318 29 C 319 31 319 32 318 33 L 298 56 C 296 59 292 61 287 61 L 254 61 C 249 61 245 59 243 56 L 223 33 C 222 32 222 31 223 29 L 243 6 Z fill=#ffd966 stroke=#00 stroke-width=2 stroke-miterlimit=10 pointer-events=none/ +g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px +text x=271 y=28 +finger/text +text x=271 y=42
[PATCH libinput v3 15/17] touchpad: Remove clickpad clicked test from 2 finger scrolling handling
This is no longer needed now that we take the button area and pinned fingers into account. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 5 - 1 file changed, 5 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index fc4a4f7..05c622b 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -501,11 +501,6 @@ tp_post_scroll_events(struct tp_dispatch *tp, uint32_t time) struct tp_touch *t; int nfingers_down = 0; - /* don't scroll if a clickpad is held down */ - if (tp-buttons.is_clickpad - (tp-buttons.state || tp-buttons.old_state)) - return 0; - /* Only count active touches for 2 finger scrolling */ tp_for_each_touch(tp, t) { if (tp_touch_active(tp, t)) -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput v3 02/17] touchpad: after a click, lock the finger to its current position
Hi, On 05/22/2014 08:05 AM, Peter Hutterer wrote: On Tue, May 20, 2014 at 04:34:50PM +0200, Hans de Goede wrote: From: Peter Hutterer peter.hutte...@who-t.net On clickpads, clicking the pad usually causes some motion events. To avoid erroneous movements, lock the finger into position on the click and don't allow for motion events until we move past a given threshold (currently 2% of the touchpad diagonal). HdG: Instead of pinning the lowest touch assuming that that is the one holding the physical button, simply pin all touches, and unpin as soon as a touch is moved more then the threshold. Can you merge the two commit messages together into one? if we don't have the original patch in the tree we don't need the specific history of this patch. if you are concerned about preserving authorship feel free to change to yourself as author or add a Co-authored-by: tag, either you or me, I don't mind either way. That applies to all other patches too please. Done. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 80 - src/evdev-mt-touchpad.h | 12 +++- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index b4a89e3..381bb90 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -32,6 +32,7 @@ #define DEFAULT_MIN_ACCEL_FACTOR 0.16 #define DEFAULT_MAX_ACCEL_FACTOR 1.0 #define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0 +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ Didn't spot this but the comment should probably be changed to /* 2% of size */ to avoid confusion about it being 2% or 0.02%. Fixed. other than that - series looks good, rev-by where missing if you want. tested on my x220 and on the T440s and it works as expected. Send me a pull request for the lot and I'll merge it, no need to re-send all these patches. I'm going through your review comments now, I'll send a pull-req when I'm done. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput v3 09/17] touchpad: Rework is_pointer handling
On 05/22/2014 05:35 AM, Peter Hutterer wrote: On Tue, May 20, 2014 at 04:34:57PM +0200, Hans de Goede wrote: We don't want touches in the button area to cause the pointer to move. So instead of making a touch the pointer when it moves to TOUCH_BEGIN, wait with making it the pointer until its buttons state moves to BUTTON_STATE_AREA. Note that a touch in the main area of the touchpad will move to BUTTON_STATE_AREA immediately. I stared at that code for a while before I realised why this wouldn't regress older touchpads - please add here that If software-buttons are not enabled, any finger is in the BUTTON_STATE_AREA. Updated the commit msg with this. two more style-only fixes below. While at it also refactor the is_pointer setting in general, removing code duplicition wrt checking that another touch is not already the pointer on unpinning a finger, and add safeguards that unpinning does not make a finger which is not in button state BUTTON_STATE_AREA the pointer, nor that the button code makes a pinned finger the pointer. All these sanity checks are combined into a new tp_button_active function, since they should be taken into account for 2 finger scrolling, etc. too. Signed-off-by: Hans de Goede hdego...@redhat.com Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 7 ++ src/evdev-mt-touchpad.c | 49 + src/evdev-mt-touchpad.h | 6 + test/touchpad.c | 8 +++ 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 61055ac..e47a55e 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -142,6 +142,7 @@ tp_button_set_state(struct tp_dispatch *tp, struct tp_touch *t, break; case BUTTON_STATE_AREA: t-button.curr = BUTTON_EVENT_IN_AREA; +tp_set_pointer(tp, t); break; case BUTTON_STATE_BOTTOM: break; @@ -598,3 +599,9 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t time) return rc; } + +int +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t) +{ +return t-button.state == BUTTON_STATE_AREA; +} diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 7f73f6e..924ba16 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -152,8 +152,6 @@ tp_get_touch(struct tp_dispatch *tp, unsigned int slot) static inline void tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t) { -struct tp_touch *tmp = NULL; - if (t-state != TOUCH_UPDATE) { tp_motion_history_reset(t); t-dirty = true; @@ -162,15 +160,6 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t) tp-nfingers_down++; assert(tp-nfingers_down = 1); tp-queued |= TOUCHPAD_EVENT_MOTION; - -tp_for_each_touch(tp, tmp) { -if (tmp-is_pointer) -break; -} - -if (!tmp-is_pointer) { -t-is_pointer = true; -} } } @@ -342,7 +331,6 @@ static void tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t) { unsigned int xdist, ydist; -struct tp_touch *tmp = NULL; if (!t-pinned.is_pinned) return; @@ -350,19 +338,11 @@ tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t) xdist = abs(t-x - t-pinned.center_x); ydist = abs(t-y - t-pinned.center_y); -if (xdist * xdist + ydist * ydist -tp-buttons.motion_dist * tp-buttons.motion_dist) -return; - -t-pinned.is_pinned = false; - -tp_for_each_touch(tp, tmp) { -if (tmp-is_pointer) -break; +if (xdist * xdist + ydist * ydist = +tp-buttons.motion_dist * tp-buttons.motion_dist) { don't indent the line here please. No idea what exactly you want me to do here, note the indentation is unchanged from the original check a few lines higher up. Not fixed. +t-pinned.is_pinned = false; +tp_set_pointer(tp, t); } - -if (t-state != TOUCH_END !tmp-is_pointer) -t-is_pointer = true; } static void @@ -378,6 +358,27 @@ tp_pin_fingers(struct tp_dispatch *tp) } } +static int +tp_touch_active(struct tp_dispatch *tp, struct tp_touch *t) +{ +return (t-state == TOUCH_BEGIN || t-state == TOUCH_UPDATE) +!t-pinned.is_pinned tp_button_touch_active(tp, t); +} + +void tp_set_pointer(struct tp_dispatch *tp, struct tp_touch *t) please add a line-break after void Done. Regards, Hans Cheers, Peter +{ +struct tp_touch *tmp = NULL; + +/* Only set the touch as pointer if we don't have one yet
Re: [PATCH libinput v3 11/17] touchpad: Ignore non left clicks on clickpads
Hi, On 05/22/2014 03:45 AM, Peter Hutterer wrote: On Tue, May 20, 2014 at 04:34:59PM +0200, Hans de Goede wrote: We should never get any non left button events on clickpads, but if we do these might confuse our state, so complain about it and ignore these. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 76e6843..f1d65be 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -367,6 +367,13 @@ tp_process_button(struct tp_dispatch *tp, uint32_t time) { uint32_t mask = 1 (e-code - BTN_LEFT); + +/* Ignore other buttons on clickpads */ +if (tp-buttons.is_clickpad e-code != BTN_LEFT) { +log_bug(received non BTN_LEFT button event on a clickpad (kernel bug?)\n); I'd be useful to print the actual button code here. log_bug(received %s button event on a clickpad (kernel bug?)\n, libevdev_event_code_get_name(tp-device-evdev, EV_KEY, e-code)); Done. Regards, Hans Cheers, Peter +return 0; +} + if (e-value) { tp-buttons.state |= mask; tp-queued |= TOUCHPAD_EVENT_BUTTON_PRESS; -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PULL] libinput clickpad improvements
Hi Peter, Please pull from my personal libinput git repo for the libinput clickpad improvements we've been working on: The following changes since commit 40bae41159d9694c4089aedc58e2abd261f8106f: configure.ac: libinput 0.2 (2014-05-22 08:10:42 +0200) are available in the git repository at: git://people.freedesktop.org/~jwrdegoede/libinput clickpad-improvements-v2 for you to fetch changes up to 89165da6d6b90d466edf3283b710c0931bdcc8dd: Change internal timestamps to uint64_t to properly deal with wrapping (2014-05-22 14:51:41 +0200) Hans de Goede (9): touchpad: Rework is_pointer handling touchpad: Use INPUT_PROP_BUTTONPAD instead of checking for buttons touchpad: Ignore non left clicks on clickpads touchpad: post_button_events: Remove TOUCHPAD_EVENT_BUTTON_PRESS/RELEASE test touchpad: softbuttons: Deal with a click arriving before any touches touchpad: Ignore fingers in button area for 2 finger scroll touchpad: Remove clickpad clicked test from 2 finger scrolling handling touchpad: handle_timeouts: Remove unused return value Change internal timestamps to uint64_t to properly deal with wrapping Peter Hutterer (7): touchpad: set ntouches for single-touch pads depending on key bits touchpad: after a click, lock the finger to its current position touchpad: reset the tap timer_fd to -1 on destroy touchpad: move button-related code into a separate file doc: add state machine SVG to EXTRA_DIST touchpad: save the active clickfinger button touchpad: Add clickpad-style software buttons doc/Makefile.am | 4 +- doc/touchpad-softbutton-state-machine.svg | 390 +++ src/Makefile.am | 1 + src/evdev-mt-touchpad-buttons.c | 625 ++ src/evdev-mt-touchpad-tap.c | 46 +-- src/evdev-mt-touchpad.c | 271 ++--- src/evdev-mt-touchpad.h | 87 - src/evdev-touchpad.c | 26 +- src/evdev.c | 12 +- src/evdev.h | 2 +- src/filter.c | 16 +- src/filter.h | 6 +- src/libinput.h| 40 ++ test/litest-bcm5974.c | 1 + test/touchpad.c | 20 +- 15 files changed, 1317 insertions(+), 230 deletions(-) create mode 100644 doc/touchpad-softbutton-state-machine.svg create mode 100644 src/evdev-mt-touchpad-buttons.c Regards, Hans p.s. The Change internal timestamps to uint64_t to properly deal with wrapping patch causes Jonas' acceleration patches to no longer apply, I've a rebased version of them available in my master branch. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 2/4] test: restore log priority after each test
Hi, On 05/23/2014 06:37 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/log.c | 12 1 file changed, 12 insertions(+) diff --git a/test/log.c b/test/log.c index 0c5508f..b8cc767 100644 --- a/test/log.c +++ b/test/log.c @@ -74,6 +74,7 @@ END_TEST START_TEST(log_handler_invoked) { struct libinput *li; + enum libinput_log_priority pri = libinput_log_get_priority(); libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG); libinput_log_set_handler(simple_log_handler, NULL); @@ -86,12 +87,14 @@ START_TEST(log_handler_invoked) log_handler_called = 0; libinput_destroy(li); + libinput_log_set_priority(pri); } END_TEST START_TEST(log_userdata_NULL) { struct libinput *li; + enum libinput_log_priority pri = libinput_log_get_priority(); libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG); libinput_log_set_handler(simple_log_handler, NULL); @@ -104,12 +107,16 @@ START_TEST(log_userdata_NULL) log_handler_called = 0; libinput_destroy(li); + + libinput_log_set_priority(pri); + libinput_log_set_priority(pri); Restoring the log priority twice here seems excessive :) With this fixed, the entire series is: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans } END_TEST START_TEST(log_userdata) { struct libinput *li; + enum libinput_log_priority pri = libinput_log_get_priority(); libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG); libinput_log_set_handler(simple_log_handler, li); @@ -122,12 +129,14 @@ START_TEST(log_userdata) log_handler_called = 0; libinput_destroy(li); + libinput_log_set_priority(pri); } END_TEST START_TEST(log_handler_NULL) { struct libinput *li; + enum libinput_log_priority pri = libinput_log_get_priority(); libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG); libinput_log_set_handler(NULL, NULL); @@ -141,12 +150,14 @@ START_TEST(log_handler_NULL) libinput_log_set_handler(simple_log_handler, NULL); libinput_destroy(li); + libinput_log_set_priority(pri); } END_TEST START_TEST(log_priority) { struct libinput *li; + enum libinput_log_priority pri = libinput_log_get_priority(); libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_ERROR); libinput_log_set_handler(simple_log_handler, NULL); @@ -164,6 +175,7 @@ START_TEST(log_priority) log_handler_called = 0; libinput_destroy(li); + libinput_log_set_priority(pri); } END_TEST ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Add basic mouse pointer acceleration libinput patch breaks make check
Hi All, I've picked Jonas' 2 mouse accel patches into my local tree, and noticed $subject. This should be fixed before these patches get merged. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Add basic mouse pointer acceleration libinput patch breaks make check
Hi, On 05/23/2014 02:57 PM, Jonas Ådahl wrote: On Fri, May 23, 2014 at 07:22:38AM -0500, Jason Ekstrand wrote: It might be better to reply to the patch. This e-mail is liable to be forgotten if the patches sit for very long. --Jason Ekstrand I did see it though and made a mental note about it, so I will look into why it makes some test fail. The tests in question seem to check for not only a certain type of event being reported, but also a certain value being reported, likely the changed accel makes the reported value different. Now the question is if we want to just fix the tests to work with the new accel, or maybe make the tests less prone to breaking ? Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 4/6] touchpad: Stop scrolling on a button click / tap
On a button click / tap the scrolling event handler no longer gets called, ensure that any in progress scrolling is stopped. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index ed668ee..f28cd13 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -549,11 +549,15 @@ tp_post_events(struct tp_dispatch *tp, uint64_t time) struct tp_touch *t = tp_current_touch(tp); double dx, dy; - if (tp_post_button_events(tp, time) != 0) + if (tp_post_button_events(tp, time) != 0) { + tp_stop_scroll_events(tp, time); return; + } - if (tp_tap_handle_state(tp, time) != 0) + if (tp_tap_handle_state(tp, time) != 0) { + tp_stop_scroll_events(tp, time); return; + } if (tp_post_scroll_events(tp, time) != 0) return; -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 3/6] touchpad: Refactor tp_post_scroll_events()
Put the actual scroll event posting in the straight path. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 2455c36..ed668ee 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -536,11 +536,11 @@ tp_post_scroll_events(struct tp_dispatch *tp, uint64_t time) if (nfingers_down != 2) { tp_stop_scroll_events(tp, time); - } else { - tp_post_twofinger_scroll(tp, time); - return 1; + return 0; } - return 0; + + tp_post_twofinger_scroll(tp, time); + return 1; } static void -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 6/6] touchpad: Increase 2 finger scrolling threshold
With the one px threshold we can easily end up accidentally locking scrolling to the wrong direction. As an added benefit a larger threshold also reduces the chance of a second finger temporarily coming down causing scrolling to start and the first finger to no longer be seen as the pointer. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index e7943f6..4a1bf29 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -464,11 +464,11 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) tp_filter_motion(tp, dx, dy, time); if (tp-scroll.state == SCROLL_STATE_NONE) { - /* Require at least one px scrolling to start */ - if (dy = -1.0 || dy = 1.0) { + /* Require at least three px scrolling to start */ + if (dy = -3.0 || dy = 3.0) { tp-scroll.state = SCROLL_STATE_SCROLLING; tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL); - } else if (dx = -1.0 || dx = 1.0) { + } else if (dx = -3.0 || dx = 3.0) { tp-scroll.state = SCROLL_STATE_SCROLLING; tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); } -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 2/6] touchpad: Fix sending of scroll stop events
Setting tp-scroll.direction = 0 before checking tp-scroll.direction to see if we need to send stop scroll events for vert / horz scrolling does not really work well. Also we need to check direction with an axis mask, not the axis number. While at it also add a tp_stop_scroll_events() helper function for this. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 39 +++ 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 040939b..2455c36 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -500,6 +500,28 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) } } +static void +tp_stop_scroll_events(struct tp_dispatch *tp, uint64_t time) +{ + if (tp-scroll.state == SCROLL_STATE_NONE) + return; + + /* terminate scrolling with a zero scroll event */ + if (tp-scroll.direction (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL)) + pointer_notify_axis(tp-device-base, + time, + LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL, + 0); + if (tp-scroll.direction (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL)) + pointer_notify_axis(tp-device-base, + time, + LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL, + 0); + + tp-scroll.state = SCROLL_STATE_NONE; + tp-scroll.direction = 0; +} + static int tp_post_scroll_events(struct tp_dispatch *tp, uint64_t time) { @@ -513,22 +535,7 @@ tp_post_scroll_events(struct tp_dispatch *tp, uint64_t time) } if (nfingers_down != 2) { - /* terminate scrolling with a zero scroll event to notify -* caller that it really ended now */ - if (tp-scroll.state != SCROLL_STATE_NONE) { - tp-scroll.state = SCROLL_STATE_NONE; - tp-scroll.direction = 0; - if (tp-scroll.direction LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL) - pointer_notify_axis(tp-device-base, - time, - LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL, - 0); - if (tp-scroll.direction LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL) - pointer_notify_axis(tp-device-base, - time, - LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL, - 0); - } + tp_stop_scroll_events(tp, time); } else { tp_post_twofinger_scroll(tp, time); return 1; -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 5/6] touchpad: Don't allow diagonal scrolling
We pin scrolling to the initial direction, so a 2 finger scroll starting in the vertical direction, will from then on only generate vertical scroll events, and the same for horizontal. But if the first 2 finger motion is diagonal, then we go into a diagonal scrolling mode where we post both horizontal and vertical scrolling events. This is inconsistent, if we want to allow both at the same time (which we don;t), we should always allow both, not only when the first motion is diagonal. This commit fixes things so that if the initial motion is diagonal, we go into vertical scrolling mode as that is what the user most likely wants. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index f28cd13..e7943f6 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -465,14 +465,12 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) if (tp-scroll.state == SCROLL_STATE_NONE) { /* Require at least one px scrolling to start */ - if (dx = -1.0 || dx = 1.0) { - tp-scroll.state = SCROLL_STATE_SCROLLING; - tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); - } - if (dy = -1.0 || dy = 1.0) { tp-scroll.state = SCROLL_STATE_SCROLLING; tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL); + } else if (dx = -1.0 || dx = 1.0) { + tp-scroll.state = SCROLL_STATE_SCROLLING; + tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); } if (tp-scroll.state == SCROLL_STATE_NONE) -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/6] touchpad: Clear touches being the pointer when doing 2 finger scrolling
When doing 2 finger scrolling we don't want any spurious movement events after scrolling. touchpad_2fg_no_motion tests for this, but it lifts touch 0 (which is the pointer as it came down first) first, so it only catches the case where touch 1 suddenly gets promoted to being the pointer. However if touch 1 is lifted first, then touch 0 is still the pointer and will cause spurious movement events. Swap the 2 litest_touch_up calls to catch this (and make the test fail), and add code to clear the is_pointer flag on all touched when doing 2 finger scrolling to fix it again. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 4 test/touchpad.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 89cebd5..040939b 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -479,6 +479,10 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) return; } + /* Stop spurious MOTION events at the end of scrolling */ + tp_for_each_touch(tp, t) + t-is_pointer = false; + if (dy != 0.0 (tp-scroll.direction (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL))) { pointer_notify_axis(tp-device-base, diff --git a/test/touchpad.c b/test/touchpad.c index 959978e..dc2e25b 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -74,8 +74,8 @@ START_TEST(touchpad_2fg_no_motion) litest_touch_down(dev, 1, 70, 20); litest_touch_move_to(dev, 0, 20, 20, 80, 80, 5); litest_touch_move_to(dev, 1, 70, 20, 80, 50, 5); - litest_touch_up(dev, 0); litest_touch_up(dev, 1); + litest_touch_up(dev, 0); libinput_dispatch(li); -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 5/6] touchpad: Don't allow diagonal scrolling
Hi, On 05/23/2014 05:00 PM, Jasper St. Pierre wrote: Any reason you switched the order of these two if statements around? Yes to prefer vertical scrolling when both dx and dy meet the threshold on the initial 2 finger motion, as is stated in the commit message. Regards, Hans On Fri, May 23, 2014 at 10:06 AM, Hans de Goede hdego...@redhat.com wrote: We pin scrolling to the initial direction, so a 2 finger scroll starting in the vertical direction, will from then on only generate vertical scroll events, and the same for horizontal. But if the first 2 finger motion is diagonal, then we go into a diagonal scrolling mode where we post both horizontal and vertical scrolling events. This is inconsistent, if we want to allow both at the same time (which we don;t), we should always allow both, not only when the first motion is diagonal. This commit fixes things so that if the initial motion is diagonal, we go into vertical scrolling mode as that is what the user most likely wants. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index f28cd13..e7943f6 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -465,14 +465,12 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) if (tp-scroll.state == SCROLL_STATE_NONE) { /* Require at least one px scrolling to start */ - if (dx = -1.0 || dx = 1.0) { - tp-scroll.state = SCROLL_STATE_SCROLLING; - tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); - } - if (dy = -1.0 || dy = 1.0) { tp-scroll.state = SCROLL_STATE_SCROLLING; tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL); + } else if (dx = -1.0 || dx = 1.0) { + tp-scroll.state = SCROLL_STATE_SCROLLING; + tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); } if (tp-scroll.state == SCROLL_STATE_NONE) -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Add basic mouse pointer acceleration libinput patch breaks make check
Hi, On 05/23/2014 05:27 PM, Jonas Ådahl wrote: On Fri, May 23, 2014 at 04:03:56PM +0200, Hans de Goede wrote: Hi, On 05/23/2014 02:57 PM, Jonas Ådahl wrote: On Fri, May 23, 2014 at 07:22:38AM -0500, Jason Ekstrand wrote: It might be better to reply to the patch. This e-mail is liable to be forgotten if the patches sit for very long. --Jason Ekstrand I did see it though and made a mental note about it, so I will look into why it makes some test fail. The tests in question seem to check for not only a certain type of event being reported, but also a certain value being reported, likely the changed accel makes the reported value different. Now the question is if we want to just fix the tests to work with the new accel, or maybe make the tests less prone to breaking ? If the tests doesn't care about the actual acceleration, I'd say tests should not assume input delta (x, y) means a known output delta (x', y'). Agreed, still the tests need to be fixed before we can apply your Add basic mouse pointer acceleration libinput patch. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 5/6] touchpad: Don't allow diagonal scrolling
Hi, On 05/25/2014 10:34 AM, Jonas Ådahl wrote: On Fri, May 23, 2014 at 04:06:26PM +0200, Hans de Goede wrote: We pin scrolling to the initial direction, so a 2 finger scroll starting in the vertical direction, will from then on only generate vertical scroll events, and the same for horizontal. But if the first 2 finger motion is diagonal, then we go into a diagonal scrolling mode where we post both horizontal and vertical scrolling events. This is inconsistent, if we want to allow both at the same time (which we don;t), we should always allow both, not only when the first motion is diagonal. Is this really the case? Personally I expect to be able to scroll both horizontally and vertically at the same time, for example for scrolling around in a large photograph, and this is already how it works with the current synaptics driver in X. Since Peter did the original lock scrolling to one direction bits (I think), I'll let answering this up to Peter. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 2/6] touchpad: Fix sending of scroll stop events
Hi, On 05/25/2014 02:13 PM, Jonas Ådahl wrote: On Fri, May 23, 2014 at 04:06:23PM +0200, Hans de Goede wrote: Setting tp-scroll.direction = 0 before checking tp-scroll.direction to see if we need to send stop scroll events for vert / horz scrolling does not really work well. Also we need to check direction with an axis mask, not the axis number. While at it also add a tp_stop_scroll_events() helper function for this. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad.c | 39 +++ 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 040939b..2455c36 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -500,6 +500,28 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) } } +static void +tp_stop_scroll_events(struct tp_dispatch *tp, uint64_t time) +{ +if (tp-scroll.state == SCROLL_STATE_NONE) +return; + +/* terminate scrolling with a zero scroll event */ +if (tp-scroll.direction (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL)) +pointer_notify_axis(tp-device-base, +time, +LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL, +0); +if (tp-scroll.direction (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL)) +pointer_notify_axis(tp-device-base, +time, +LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL, +0); I can see that you didn't introduce sending 0-valued axis events in this patch, but why do we do so? weston will ignore it anyway, and if it wouldn't or other compositor wouldn't, it might confuse clients assuming that a scroll event will always have a direction. Another one where I'm going to let the answer up to Peter I'm afraid. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/2] evdev-mt-touchpad-buttons: Rename some variables and functions
Rename some clickpad softbutton area variables to have bottom in their name, this is a preperation patch for adding top softbutton area support. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-buttons.c | 26 +- src/evdev-mt-touchpad.h | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 65fa21b..b4d3920 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -75,23 +75,23 @@ button_event_to_str(enum button_event event) { } static inline bool -is_inside_button_area(struct tp_dispatch *tp, struct tp_touch *t) +is_inside_bottom_button_area(struct tp_dispatch *tp, struct tp_touch *t) { - return t-y = tp-buttons.area.top_edge; + return t-y = tp-buttons.bottom_area.top_edge; } static inline bool -is_inside_right_area(struct tp_dispatch *tp, struct tp_touch *t) +is_inside_bottom_right_area(struct tp_dispatch *tp, struct tp_touch *t) { - return is_inside_button_area(tp, t) - t-x tp-buttons.area.rightbutton_left_edge; + return is_inside_bottom_button_area(tp, t) + t-x tp-buttons.bottom_area.rightbutton_left_edge; } static inline bool -is_inside_left_area(struct tp_dispatch *tp, struct tp_touch *t) +is_inside_bottom_left_area(struct tp_dispatch *tp, struct tp_touch *t) { - return is_inside_button_area(tp, t) - !is_inside_right_area(tp, t); + return is_inside_bottom_button_area(tp, t) + !is_inside_bottom_right_area(tp, t); } static void @@ -327,9 +327,9 @@ tp_button_handle_state(struct tp_dispatch *tp, uint64_t time) if (t-state == TOUCH_END) { tp_button_handle_event(tp, t, BUTTON_EVENT_UP, time); } else if (t-dirty) { - if (is_inside_right_area(tp, t)) + if (is_inside_bottom_right_area(tp, t)) tp_button_handle_event(tp, t, BUTTON_EVENT_IN_BOTTOM_R, time); - else if (is_inside_left_area(tp, t)) + else if (is_inside_bottom_left_area(tp, t)) tp_button_handle_event(tp, t, BUTTON_EVENT_IN_BOTTOM_L, time); else tp_button_handle_event(tp, t, BUTTON_EVENT_IN_AREA, time); @@ -432,8 +432,8 @@ tp_init_buttons(struct tp_dispatch *tp, tp-buttons.use_clickfinger = true; if (tp-buttons.is_clickpad !tp-buttons.use_clickfinger) { - tp-buttons.area.top_edge = height * .8 + device-abs.min_y; - tp-buttons.area.rightbutton_left_edge = width/2 + device-abs.min_x; + tp-buttons.bottom_area.top_edge = height * .8 + device-abs.min_y; + tp-buttons.bottom_area.rightbutton_left_edge = width/2 + device-abs.min_x; tp-buttons.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); if (tp-buttons.timer_fd == -1) @@ -447,7 +447,7 @@ tp_init_buttons(struct tp_dispatch *tp, if (tp-buttons.source == NULL) return -1; } else { - tp-buttons.area.top_edge = INT_MAX; + tp-buttons.bottom_area.top_edge = INT_MAX; } return 0; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 41e3ca4..62abe59 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -170,7 +170,7 @@ struct tp_dispatch { struct { int32_t top_edge; int32_t rightbutton_left_edge; - } area; + } bottom_area; unsigned int timeout; /* current timeout in ms */ -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/7] filter: Add motion filter destruction helper
Hi, The entire series looks good, and isL Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans On 05/26/2014 11:27 PM, Jonas Ådahl wrote: Signed-off-by: Jonas Ådahl jad...@gmail.com --- src/evdev-mt-touchpad.c | 3 +-- src/filter.c| 9 + src/filter.h| 3 +++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 89cebd5..751132c 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -606,8 +606,7 @@ tp_destroy(struct evdev_dispatch *dispatch) tp_destroy_tap(tp); tp_destroy_buttons(tp); - if (tp-filter) - tp-filter-interface-destroy(tp-filter); + motion_filter_destroy(tp-filter); free(tp-touches); free(tp); } diff --git a/src/filter.c b/src/filter.c index 2c23da1..22c3ed8 100644 --- a/src/filter.c +++ b/src/filter.c @@ -331,3 +331,12 @@ create_pointer_accelator_filter(accel_profile_func_t profile) return filter-base; } + +void +motion_filter_destroy(struct motion_filter *filter) +{ + if (!filter) + return; + + filter-interface-destroy(filter); +} diff --git a/src/filter.h b/src/filter.h index 0ef3d03..ada4f93 100644 --- a/src/filter.h +++ b/src/filter.h @@ -59,4 +59,7 @@ typedef double (*accel_profile_func_t)(struct motion_filter *filter, struct motion_filter * create_pointer_accelator_filter(accel_profile_func_t filter); +void +motion_filter_destroy(struct motion_filter *filter); + #endif /* FILTER_H */ ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
Hi, On 06/04/2014 10:21 AM, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 11:17:58AM +1000, Peter Hutterer wrote: On Tue, Jun 03, 2014 at 10:41:16PM +0200, Jonas Ådahl wrote: On Tue, Jun 03, 2014 at 03:34:55PM +1000, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, +int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, +int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; Shouldn't this be returned also if !enable? It would be a bit confusing if disabling is allowed, but enabling is not. This was a conscious decision that applies to the other functions as well: if you're doing something that's not technically invalid, we're not reporting an error. This means that an error only needs to be handled if something really is off. I'd argue that disabling tapping on a device that doesn't support tapping is a technically invalid operation that is simply handled in the cases where it makes no sense. I'm with Peter here, that its easier for our API consumers if they caan always safely call disable. Also, in what circumstances will config.tap be set, but finger count returning 0? What type of devices would have tapping configuration enabled with no tapping possible? I expect none, this approach was just the safest and most copy-paste-proof way of checking. + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. Should it be libinput_device_config_tap_enable() here instead? ah, thanks. one always escapes the refacturing... + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +int +libinput_device_config_tap_get_finger_count(struct libinput_device *device); I wonder where doing things like this, and libinput_device_config_scroll_get_methods() will lead us. It'd mean we expose some device information via the config API, and some via direct device getters, which might not be the best thing for consistency. Just to be clear - you're talking about ditching the _config_ part of the interface? If so, yeah, doable but I
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} Didn't we agree in an off-list discussion to add a get_default instead, so to that ie a cmdline app for querying config info can not only display the current value but also the device default value for a config option ? This get_default would replace the reset, apps can simple implement that functionality themselves through the get_default. Regards, Hans diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +int +libinput_device_config_tap_get_finger_count(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable tap-to-click on this device, with a default mapping of 1, 2, 3 + * finger tap mapping to left, right, middle click, respectively. + * Tapping is limited by the number of simultaneous touches + * supported by the device, see + * libinput_device_config_tap_get_finger_count(). + * + * @param device The device to configure + * @param enable Non-zero to enable, zero to disable + * + * @return A config status code. Disabling tapping on a device that does not + * support tapping always succeeds. + * + * @see libinput_device_config_tap_get_finger_count + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable); + +/** + * @ingroup config + * + * Check if tap-to-click is enabled on this device. If the device does not + * support tapping, this function always returns 0. + * + * @param device The device to configure + * + * @return 1 if enabled, 0 otherwise. + * + * @see
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Now that we have run-time changes of the tap.enabled state move the check to the IDLE state only. Otherwise the tap machine may hang if tapping is disabled while a gesture is in progress. Two basic tests are added to check for the tap default setting - which is now tap disabled by default, because a bike shed's correct color is green. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-tap.c | 56 +++-- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 32 ++ 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index eee334f..24ea8c3 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -438,13 +438,13 @@ static void tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) { enum tp_tap_state current; - if (!tp-tap.enabled) - return; current = tp-tap.state; switch(tp-tap.state) { case TAP_STATE_IDLE: + if (!tp-tap.enabled) + break; tp_tap_idle_handle_event(tp, event, time); break; case TAP_STATE_TOUCH: @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data) unsigned int tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) { - if (!tp-tap.enabled) - return 0; - if (tp-tap.timeout tp-tap.timeout = time) { tp_tap_clear_timer(tp); tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) return tp-tap.timeout; } +static int +tp_tap_config_count(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return min(tp-ntouches, 3); /* we only do up to 3 finger tap */ +} + +static enum libinput_config_status +tp_tap_config_enable(struct libinput_device *device, int enabled) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; Please user container_of here for proper type checking rather then just a blatant cast of $random_type_foo to $random_type_bar. Same for all the other occurrences of the same pattern below. + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + if (tp_tap_config_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; This is a repeat of the test done by the higher level wrapper, as such this seems unnecessary by me. Regards, Hans + + tp-tap.enabled = enabled; + + return LIBINPUT_CONFIG_STATUS_SUCCESS; +} + +static int +tp_tap_config_is_enabled(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return tp-tap.enabled; +} + +static void +tp_tap_config_reset(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + tp-tap.enabled = false; +} + int tp_init_tap(struct tp_dispatch *tp) { + tp-tap.config.count = tp_tap_config_count; + tp-tap.config.enable = tp_tap_config_enable; + tp-tap.config.is_enabled = tp_tap_config_is_enabled; + tp-tap.config.reset = tp_tap_config_reset; + tp-device-base.config.tap = tp-tap.config; + tp-tap.state = TAP_STATE_IDLE; tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); @@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp) return -1; } - tp-tap.enabled = 1; /* FIXME */ - return 0; } diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index d514ed6..0b31e9a 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -203,6 +203,7 @@ struct tp_dispatch { struct { bool enabled; + struct libinput_device_config_tap config; int timer_fd; struct libinput_source *source; unsigned int timeout; diff --git a/test/touchpad.c b/test/touchpad.c index 35781c3..060b529 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap) struct libinput *li = dev-libinput; struct libinput_event *event; + libinput_device_config_tap_enable(dev-libinput_device, 1); + litest_drain_events(li); litest_touch_down(dev, 0, 50, 50); @@ -141,6 +143,8 @@ START_TEST(touchpad_1fg_tap_n_drag) struct libinput *li = dev-libinput; struct libinput_event *event; +
Re: [PATCH libinput 02/10] Add an interface to enable/disable tapping
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Provide an interface to enable/disable tapping, with a default mapping of 1/2/3 fingers mapping to L/R/M button events, respectively. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net I see that in all the other config interfaces you use get / set, yet here you use enable / is_enabled. I think it would be more consistent to use get / set with a bool return / argument here too. This would also mix better with the get_default I've suggested. Regards, Hans --- src/libinput-private.h | 13 + src/libinput.c | 33 +++ src/libinput.h | 73 +- 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index 61cdc79..020167e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -69,12 +69,25 @@ struct libinput_seat { uint32_t button_count[KEY_CNT]; }; +struct libinput_device_config_tap { + int (*count)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + +struct libinput_device_config { + struct libinput_device_config_tap *tap; +}; + struct libinput_device { struct libinput_seat *seat; struct list link; void *user_data; int terminated; int refcount; + struct libinput_device_config config; }; typedef void (*libinput_source_dispatch_t)(void *data); diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..6a713bb 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1182,3 +1182,36 @@ libinput_event_touch_get_base_event(struct libinput_event_touch *event) { return event-base; } + +LIBINPUT_EXPORT int +libinput_device_config_tap_get_finger_count(struct libinput_device *device) +{ + return device-config.tap ? device-config.tap-count(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable) +{ + if (enable + libinput_device_config_tap_get_finger_count(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.tap-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_tap_is_enabled(struct libinput_device *device) +{ + if (libinput_device_config_tap_get_finger_count(device) == 0) + return 0; + + return device-config.tap-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_tap_reset(struct libinput_device *device) +{ + if (device-config.tap) + device-config.tap-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index c9ec71a..0c84547 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1362,8 +1362,79 @@ enum libinput_config_status { const char * libinput_config_status_to_str(enum libinput_config_status status); +/** + * @ingroup config + * + * Check if the device supports tap-to-click. See + * libinput_device_config_tap_set() for more information. + * + * @param device The device to configure + * @return The number of fingers that can generate a tap event, or 0 if the + * device does not support tapping. + * + * @see libinput_device_config_tap_enable + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +int +libinput_device_config_tap_get_finger_count(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable tap-to-click on this device, with a default mapping of 1, 2, 3 + * finger tap mapping to left, right, middle click, respectively. + * Tapping is limited by the number of simultaneous touches + * supported by the device, see + * libinput_device_config_tap_get_finger_count(). + * + * @param device The device to configure + * @param enable Non-zero to enable, zero to disable + * + * @return A config status code. Disabling tapping on a device that does not + * support tapping always succeeds. + * + * @see libinput_device_config_tap_get_finger_count + * @see libinput_device_config_tap_is_enabled + * @see libinput_device_config_tap_reset + */ +enum libinput_config_status +libinput_device_config_tap_enable(struct libinput_device *device, + int enable); + +/** + * @ingroup config + * + * Check if tap-to-click is enabled on this device. If the device does not + * support tapping, this function always returns 0. + * + * @param device The device to configure + * + * @return 1 if enabled, 0 otherwise. + * + * @see libinput_device_config_tap_get_finger_count + * @see libinput_device_config_tap_enable + * @see
Re: [PATCH libinput 04/10] Add a config interface for scroll methods
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: --- src/libinput-private.h | 9 ++ src/libinput.c | 35 src/libinput.h | 74 ++ 3 files changed, 118 insertions(+) Looks good to me. Regards, Hans diff --git a/src/libinput-private.h b/src/libinput-private.h index 020167e..d3570a4 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -77,8 +77,17 @@ struct libinput_device_config_tap { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_scroll { + int (*methods)(struct libinput_device *device); + enum libinput_config_status (*set)(struct libinput_device *device, +enum libinput_scroll_method method); + enum libinput_scroll_method (*get)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; + struct libinput_device_config_scroll *scroll; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 6a713bb..b2388e6 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1215,3 +1215,38 @@ libinput_device_config_tap_reset(struct libinput_device *device) if (device-config.tap) device-config.tap-reset(device); } + +LIBINPUT_EXPORT unsigned int +libinput_device_config_scroll_get_methods(struct libinput_device *device) +{ + return device-config.scroll ? + device-config.scroll-methods(device) : + LIBINPUT_SCROLL_METHOD_NONE; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_scroll_set_method(struct libinput_device *device, + enum libinput_scroll_method method) +{ + if ((method libinput_device_config_scroll_get_methods(device)) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.scroll-set(device, method); +} + +LIBINPUT_EXPORT enum libinput_scroll_method +libinput_device_config_scroll_get_method(struct libinput_device *device) +{ + if (libinput_device_config_scroll_get_methods(device) == + LIBINPUT_SCROLL_METHOD_NONE) + return LIBINPUT_SCROLL_METHOD_NONE; + + return device-config.scroll-get(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_scroll_reset(struct libinput_device *device) +{ + if (device-config.scroll) + device-config.scroll-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 0c84547..571f7ba 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1434,6 +1434,80 @@ libinput_device_config_tap_is_enabled(struct libinput_device *device); void libinput_device_config_tap_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Devices without a physical scroll wheel (such as touchpads) may emulate + * scroll events in software through one or more methods. + */ +enum libinput_scroll_method { + /** + * No scroll method available or selected. + */ + LIBINPUT_SCROLL_METHOD_NONE = 0, + /** + * Scrolling is triggered by moving a finger at the edge of the + * touchpad. + */ + LIBINPUT_SCROLL_METHOD_EDGE = (1 0), + /** + * Scrolling is triggered by moving two fingers simultaneously. + */ + LIBINPUT_SCROLL_METHOD_TWOFINGER = (1 1), +}; + +/** + * @ingroup config + * + * Check the available scroll methods on this device. + * + * @param device The device to configure + * + * @return A bitmask of the available scroll methods + */ +unsigned int +libinput_device_config_scroll_get_methods(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the scroll method on this device. Only one method at a time may be + * chosen for each device. + * + * @param device The device to configure + * @param method The scroll method to chose + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_scroll_set_method(struct libinput_device *device, + enum libinput_scroll_method method); + +/** + * @ingroup config + * + * Get the currently selected scroll method on this device. If a device does + * not support configurable scroll methods, the return value is always + * LIBINPUT_SCROLL_METHOD_NONE. + * + * @param device The device to configure + * + * @return The currently selected scroll method + */ +enum libinput_scroll_method +libinput_device_config_scroll_get_method(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset to the default scroll method for this device, if any. If the device + * does not support configurable scroll methods this function does nothing. + * + * @param device The device to configure + */ +void
Re: [PATCH libinput 05/10] touchpad: hook up scroll config
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 43 +++ src/evdev-mt-touchpad.h | 1 + 2 files changed, 44 insertions(+) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 26d5f7d..c1c994a 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -737,8 +737,51 @@ tp_init_accel(struct tp_dispatch *touchpad, double diagonal) } static int +tp_config_scroll_methods(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; As with previous patches, please user container_of here for proper type checking. Same for all the other occurrences of the same pattern below. Otherwise this looks good. Regards, Hans + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return LIBINPUT_SCROLL_METHOD_TWOFINGER; +} + +static enum libinput_config_status +tp_config_scroll_set(struct libinput_device *device, + enum libinput_scroll_method method) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + if (method != LIBINPUT_SCROLL_METHOD_TWOFINGER) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return LIBINPUT_CONFIG_STATUS_SUCCESS; +} + +static enum libinput_scroll_method +tp_config_scroll_get(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return LIBINPUT_SCROLL_METHOD_TWOFINGER; +} + +static void +tp_config_scroll_reset(struct libinput_device *device) +{ + /* two-finger scrolling is hardcoded, nothing to do */ +} + +static int tp_init_scroll(struct tp_dispatch *tp) { + tp-scroll.config.methods = tp_config_scroll_methods; + tp-scroll.config.set = tp_config_scroll_set; + tp-scroll.config.get = tp_config_scroll_get; + tp-scroll.config.reset = tp_config_scroll_reset; + tp-device-base.config.scroll = tp-scroll.config; + tp-scroll.direction = 0; tp-scroll.state = SCROLL_STATE_NONE; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 0b31e9a..d89d74c 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -195,6 +195,7 @@ struct tp_dispatch { } buttons; /* physical buttons */ struct { + struct libinput_device_config_scroll config; enum scroll_state state; enum libinput_pointer_axis direction; } scroll; ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 06/10] Add a config interface to change device rotation
Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 9 +++ src/libinput.c | 33 src/libinput.h | 69 ++ 3 files changed, 111 insertions(+) Looks good. Regards, Hans diff --git a/src/libinput-private.h b/src/libinput-private.h index d3570a4..0d2a1b1 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -85,9 +85,18 @@ struct libinput_device_config_scroll { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_rotation { + int (*increment)(struct libinput_device *device); + enum libinput_config_status (*set)(struct libinput_device *device, +int degrees_cw); + int (*get)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; + struct libinput_device_config_rotation *rotation; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index b2388e6..2572f5b 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1250,3 +1250,36 @@ libinput_device_config_scroll_reset(struct libinput_device *device) if (device-config.scroll) device-config.scroll-reset(device); } + +LIBINPUT_EXPORT int +libinput_device_config_rotation_get_increment(struct libinput_device *device) +{ + return device-config.rotation ? + device-config.rotation-increment(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_rotation_set(struct libinput_device *device, + int degrees_cw) +{ + if (libinput_device_config_rotation_get_increment(device) == 0) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.rotation-set(device, degrees_cw); +} + +LIBINPUT_EXPORT int +libinput_device_config_rotation_get(struct libinput_device *device) +{ + if (libinput_device_config_rotation_get_increment(device) == 0) + return 0; + + return device-config.rotation-get(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_rotation_reset(struct libinput_device *device) +{ + if (libinput_device_config_rotation_get_increment(device) != 0) + device-config.rotation-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 571f7ba..328d050 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1508,6 +1508,75 @@ libinput_device_config_scroll_get_method(struct libinput_device *device); void libinput_device_config_scroll_reset(struct libinput_device *device); + +/** + * @ingroup config + * + * Query the rotation increment for this device, if any. The return value is + * the increment in degrees. For example, a device that returns 90 may only + * be rotated in 90-degree increments. + * + * @param device The device to configure + * + * @return The rotation increment in degrees, or 0 if the device cannot be + * rotated + */ +int +libinput_device_config_rotation_get_increment(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the rotation for this device, in degrees clockwise. This rotation + * applies to the physical orientation of the device, i.e. if a tablet is + * moved from landscape to portrait format, clockwise, this represents a + * 90-degree rotation. In the diagram below, if a is the device origin 0/0, + * after the rotation the coordinate b sends 0/0 coordinates and a sends + * xmax/0. + * + * @code + * +-++-+ + * |a||b a| + * | | - | | + * |b|| | + * +-+| | + * | | + * +-+ + * @endcode + * + * @param device The device to configure + * @param degrees_cw The number of degrees to rotate clockwise + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_rotation_set(struct libinput_device *device, + int degrees_cw); + +/** + * @ingroup config + * + * Get the current rotation for this device, in degrees clockwise. If the + * device does not support rotation, this function always returns 0. + * + * @param device The device to configure + * + * @return The rotation in degrees clockwise + */ +int +libinput_device_config_rotation_get(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset the rotation to the device's default setting. If thd evice does not + * support rotation, this function does nothing. + * + * @param device The device to configure + */ +void
Re: [PATCH libinput 07/10] Add a basic pointer acceleration API
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: Only exposes two knobs - speed and precision which have historically been the only two knobs exposed anyway on most UIs. We could go for something fancier but really, I think this will be enough. The only open question is whether speed will be enough for high-dpi devices. I would like to see an implementation of this before adding this API. Regards, Hans Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 12 src/libinput.c | 53 + src/libinput.h | 80 ++ 3 files changed, 145 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 0d2a1b1..85113bd 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -93,10 +93,22 @@ struct libinput_device_config_rotation { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_accel { + int (*available)(struct libinput_device *device); + enum libinput_config_status (*set_speed)(struct libinput_device *device, + unsigned int speed); + enum libinput_config_status (*set_precision)(struct libinput_device *device, + unsigned int precision); + int (*get_speed)(struct libinput_device *device); + int (*get_precision)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; struct libinput_device_config_rotation *rotation; + struct libinput_device_config_accel *accel; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 2572f5b..5a068f1 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1283,3 +1283,56 @@ libinput_device_config_rotation_reset(struct libinput_device *device) if (libinput_device_config_rotation_get_increment(device) != 0) device-config.rotation-reset(device); } + + +LIBINPUT_EXPORT int +libinput_device_config_accel_is_available(struct libinput_device *device) +{ + return device-config.accel ? + device-config.accel-available(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_accel_set_speed(struct libinput_device *device, +unsigned int speed) +{ + if (!libinput_device_config_accel_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.accel-set_speed(device, speed); +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_accel_set_precision(struct libinput_device *device, +unsigned int precision) +{ + if (!libinput_device_config_accel_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.accel-set_precision(device, precision); +} + +LIBINPUT_EXPORT unsigned int +libinput_device_config_accel_get_speed(struct libinput_device *device) +{ + if (!libinput_device_config_accel_is_available(device)) + return 0; + + return device-config.accel-get_speed(device); +} + +LIBINPUT_EXPORT unsigned int +libinput_device_config_accel_get_precision(struct libinput_device *device) +{ + if (!libinput_device_config_accel_is_available(device)) + return 0; + + return device-config.accel-get_precision(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_accel_reset(struct libinput_device *device) +{ + if (device-config.accel) + device-config.accel-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 328d050..1b6207c 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1577,6 +1577,86 @@ libinput_device_config_rotation_get(struct libinput_device *device); void libinput_device_config_rotation_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Check if a device uses libinput-internal pointer-acceleration. + * + * @param device The device to configure + * + * @return 0 if the device is not accelerated, nonzero if it is accelerated + */ +int +libinput_device_config_accel_is_available(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the speed of this pointer device, where 0% is the minimum pointer + * acceleration to be applied (none or slowed down, depending on the device) + * and 100% is the maximum amount of acceleration to be applied. + * + * @param device The device to configure + * @param speed The abstract speed identifier, ranged 0% to 100%. + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_accel_set_speed(struct
Re: [PATCH libinput 09/10] Add config API for pointer modes
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/libinput-private.h | 9 ++ src/libinput.c | 37 src/libinput.h | 77 ++ 3 files changed, 123 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 9a3e629..0de 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -112,12 +112,21 @@ struct libinput_device_config_disable_while_typing { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_pointer_mode { + int (*modes)(struct libinput_device *device); + enum libinput_config_status (*set)(struct libinput_device *device, +enum libinput_device_pointer_mode); + enum libinput_device_pointer_mode (*get)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; struct libinput_device_config_rotation *rotation; struct libinput_device_config_accel *accel; struct libinput_device_config_disable_while_typing *dwt; + struct libinput_device_config_pointer_mode *mode; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 33a8e90..5324407 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1369,3 +1369,40 @@ libinput_device_config_disable_while_typing_reset(struct libinput_device *device if (device-config.dwt) device-config.dwt-reset(device); } + +LIBINPUT_EXPORT unsigned int +libinput_device_config_pointer_mode_get_modes(struct libinput_device *device) +{ + return device-config.mode ? + device-config.mode-modes(device) : + LIBINPUT_POINTER_MODE_NATIVE_ONLY; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_pointer_mode_set_mode(struct libinput_device *device, + enum libinput_device_pointer_mode mode) +{ + if (libinput_device_config_pointer_mode_get_modes(device) == + LIBINPUT_POINTER_MODE_NATIVE_ONLY + mode != LIBINPUT_POINTER_MODE_NATIVE_ONLY) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.mode-set(device, mode); +} + +LIBINPUT_EXPORT enum libinput_device_pointer_mode +libinput_device_config_pointer_mode_get_mode(struct libinput_device *device) +{ + if (libinput_device_config_pointer_mode_get_modes(device) == + LIBINPUT_POINTER_MODE_NATIVE_ONLY) + return LIBINPUT_POINTER_MODE_NATIVE_ONLY; + + return device-config.mode-get(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_pointer_mode_reset(struct libinput_device *device) +{ + if (device-config.mode) + device-config.mode-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 62d0c0f..1a51b82 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1713,6 +1713,83 @@ libinput_device_config_disable_while_typing_is_enabled(struct libinput_device *d void libinput_device_config_disable_while_typing_reset(struct libinput_device *device); +/** + * @ingroup config + */ +enum libinput_device_pointer_mode { + /** + * The device only works in native mode and does not support mode + * switching. Native mode may be absolute or relative, depending on + * the device. + */ + LIBINPUT_POINTER_MODE_NATIVE_ONLY = 0, + /** + * The device behaves like an absolute input device, e.g. like a + * touchscreen. + */ + LIBINPUT_POINTER_MODE_ABSOLUTE = (1 0), + /** + * The device behaves like a relative input device, e.g. like a + * touchpad. + */ + LIBINPUT_POINTER_MODE_RELATIVE = (1 1), +}; + +/** + * @ingroup config + * + * Get the supported device modes for this device. Absolute pointer devices + * such as graphics tablet may be used in absolute mode or relative mode. + * + * @note A device that supports relative and absolute mode may be + * pointer-accelerated in relative mode. How will these interact, what will the available method of the pointer accel return when in absolute mode ? What will the set / get methods return ? Regards, Hans + * + * @param device The device to configure + * + * @return A bitmask of the available pointer modes, or + * POINTER_MODE_NATIVE_ONLY if the device does not allow mode switching + */ +unsigned int +libinput_device_config_pointer_mode_get_modes(struct libinput_device *device); + +/** + * @ingroup config + * + * Set the pointer mode for this device. + * + * @param device The device to configure + * @param mode The pointer mode to switch the device to + * + * @return A config
Re: [PATCH libinput 10/10] Add config api for middle button emulation
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: --- src/libinput-private.h | 8 src/libinput.c | 33 ++ src/libinput.h | 54 ++ 3 files changed, 95 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 0de..28f071a 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -120,6 +120,13 @@ struct libinput_device_config_pointer_mode { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_middlebutton_emulation { + int (*available)(struct libinput_device *device); + int (*enable)(struct libinput_device *device, int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + Again set / get / get_default ? Other then that this looks good. Regards, Hans struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; @@ -127,6 +134,7 @@ struct libinput_device_config { struct libinput_device_config_accel *accel; struct libinput_device_config_disable_while_typing *dwt; struct libinput_device_config_pointer_mode *mode; + struct libinput_device_config_middlebutton_emulation *mbemu; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 5324407..bd06960 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1406,3 +1406,36 @@ libinput_device_config_pointer_mode_reset(struct libinput_device *device) if (device-config.mode) device-config.mode-reset(device); } + +LIBINPUT_EXPORT int +libinput_device_config_middlebutton_emulation_is_available(struct libinput_device *device) +{ + return device-config.mbemu ? + device-config.mbemu-available(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_middlebutton_emulation_enable(struct libinput_device *device, + int enable) +{ + if (!libinput_device_config_middlebutton_emulation_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.mbemu-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_middlebutton_emulation_is_enabled(struct libinput_device *device) +{ + if (!libinput_device_config_middlebutton_emulation_is_available(device)) + return 0; + + return device-config.mbemu-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_middlebutton_emulation_reset(struct libinput_device *device) +{ + if (device-config.mbemu) + device-config.mbemu-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 1a51b82..b78283f 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1789,6 +1789,60 @@ libinput_device_config_pointer_mode_get_mode(struct libinput_device *device); void libinput_device_config_pointer_mode_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Devices without a physical middle button may provide middle-button + * emulation by pressing the left and the right button simultaneously. + * + * @param device The device to configure + * + * @return 1 if available, 0 if not available + */ +int +libinput_device_config_middlebutton_emulation_is_available(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable or disable middle button emulation on this device. Note that + * enabling middle button emulation causes a delay in the delivery of button + * events. + * + * @param device The device to configure + * @param enable 1 to enable, 0 to disable + * + * @return A config status code + */ +enum libinput_config_status +libinput_device_config_middlebutton_emulation_enable(struct libinput_device *device, + int enable); + +/** + * @ingroup config + * + * Check if middle button emulation is enabled on this device. If the device + * does not support middle button emulation, this function returns 0. + * + * @param device The device to configure + * + * @return 0 if disabled, 1 if enabled + */ +int +libinput_device_config_middlebutton_emulation_is_enabled(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset to the default emulation status. If the device does not support + * middle button emulation, this function does nothing. + * + * @param device The device to configure + * + */ +void +libinput_device_config_middlebutton_emulation_reset(struct libinput_device *device); + #ifdef __cplusplus } ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] touchpad: Drop the scroll direction lock, increase threshold instead
Hi, On 06/04/2014 06:09 AM, Peter Hutterer wrote: The direction lock was intended to avoid erroneous horizontal scroll events when scrolling vertically (and vice versa). Some testing on my touchpad here shows that it is too easy to accidentally lock the direction when no lock is intended (e.g. moving around an image). And quite hard to figure out what a pure vertical gesture is. I get movements from 90 degrees to 70 degrees for something my brain would consider vertical scrolling. Depending on the hand position, the fingers actually perform a slight curve, not a straight line. Hence - drop the direction lock, but increase the threshold a little. It doesn't totally avoid horizontal scroll events but keeps them minimal. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Ok, I give up. I thought I could find a simple way of locking the scroll directions but I ran out of time and the simplest approach didn't work that greatly. This patch replaces 5 and 6 of this series (the ones I never pushed) Thanks, looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans src/evdev-mt-touchpad.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 26d5f7d..466cf5e 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -463,21 +463,18 @@ tp_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time) tp_filter_motion(tp, dx, dy, time); - if (tp-scroll.state == SCROLL_STATE_NONE) { - /* Require at least one px scrolling to start */ - if (dx = -1.0 || dx = 1.0) { - tp-scroll.state = SCROLL_STATE_SCROLLING; - tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); - } - - if (dy = -1.0 || dy = 1.0) { - tp-scroll.state = SCROLL_STATE_SCROLLING; - tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL); - } - - if (tp-scroll.state == SCROLL_STATE_NONE) - return; + /* Require at least three px scrolling to start */ + if (dy = -3.0 || dy = 3.0) { + tp-scroll.state = SCROLL_STATE_SCROLLING; + tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_VERTICAL_SCROLL); } + if (dx = -3.0 || dx = 3.0) { + tp-scroll.state = SCROLL_STATE_SCROLLING; + tp-scroll.direction |= (1 LIBINPUT_POINTER_AXIS_HORIZONTAL_SCROLL); + } + + if (tp-scroll.state == SCROLL_STATE_NONE) + return; /* Stop spurious MOTION events at the end of scrolling */ tp_for_each_touch(tp, t) ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 08/10] Add a config API for disable-while-typing
Hi, On 06/03/2014 07:35 AM, Peter Hutterer wrote: --- src/libinput-private.h | 9 src/libinput.c | 33 + src/libinput.h | 56 ++ 3 files changed, 98 insertions(+) diff --git a/src/libinput-private.h b/src/libinput-private.h index 85113bd..9a3e629 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -104,11 +104,20 @@ struct libinput_device_config_accel { void (*reset)(struct libinput_device *device); }; +struct libinput_device_config_disable_while_typing { + int (*available)(struct libinput_device *device); + enum libinput_config_status (*enable)(struct libinput_device *device, + int enable); + int (*is_enabled)(struct libinput_device *device); + void (*reset)(struct libinput_device *device); +}; + set / get / get_default ? Regards, Hans struct libinput_device_config { struct libinput_device_config_tap *tap; struct libinput_device_config_scroll *scroll; struct libinput_device_config_rotation *rotation; struct libinput_device_config_accel *accel; + struct libinput_device_config_disable_while_typing *dwt; }; struct libinput_device { diff --git a/src/libinput.c b/src/libinput.c index 5a068f1..33a8e90 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -1336,3 +1336,36 @@ libinput_device_config_accel_reset(struct libinput_device *device) if (device-config.accel) device-config.accel-reset(device); } + +LIBINPUT_EXPORT int +libinput_device_config_disable_while_typing_is_available(struct libinput_device *device) +{ + return device-config.dwt ? + device-config.dwt-available(device) : 0; +} + +LIBINPUT_EXPORT enum libinput_config_status +libinput_device_config_disable_while_typing_enable(struct libinput_device *device, +int enable) +{ + if (!libinput_device_config_disable_while_typing_is_available(device)) + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED; + + return device-config.dwt-enable(device, enable); +} + +LIBINPUT_EXPORT int +libinput_device_config_disable_while_typing_is_enabled(struct libinput_device *device) +{ + if (!libinput_device_config_disable_while_typing_is_available(device)) + return 0; + + return device-config.dwt-is_enabled(device); +} + +LIBINPUT_EXPORT void +libinput_device_config_disable_while_typing_reset(struct libinput_device *device) +{ + if (device-config.dwt) + device-config.dwt-reset(device); +} diff --git a/src/libinput.h b/src/libinput.h index 1b6207c..62d0c0f 100644 --- a/src/libinput.h +++ b/src/libinput.h @@ -1657,6 +1657,62 @@ libinput_device_config_accel_get_precision(struct libinput_device *device); void libinput_device_config_accel_reset(struct libinput_device *device); +/** + * @ingroup config + * + * Check if this device supports a disable-while-typing feature. This + * feature is usually available on built-in touchpads where hand placement + * may cause erroneous events on the touchpad while typing. + * + * This feature is available on the device that is being disabled (i.e. the + * touchpad), not on the device causing the device to be disabled. Which + * devices trigger this feature is implementation-dependent. + * + * @param device The device to configure + * @return non-zero if disable while typing is available for this device + */ +int +libinput_device_config_disable_while_typing_is_available(struct libinput_device *device); + +/** + * @ingroup config + * + * Enable or disable the disable-while-typing feature. When enabled, the + * device will not send events while and shortly after another device + * generates events. + * + * @param device The device to configure + * @param enable 0 to disable, 1 to enable + * + * @return 0 on success or a negative errno on failure + * @retval A config status code + */ +enum libinput_config_status +libinput_device_config_disable_while_typing_enable(struct libinput_device *device, +int enable); + +/** + * @ingroup config + * + * Check if the feature is currently enabled. + * + * @param device The device to configure + * + * @return 0 if the feature is disabled, non-zero if the feature is enabled + */ +int +libinput_device_config_disable_while_typing_is_enabled(struct libinput_device *device); + +/** + * @ingroup config + * + * Reset to the default settings. + * + * @param device The device to configure + */ +void +libinput_device_config_disable_while_typing_reset(struct libinput_device *device); + #ifdef __cplusplus } #endif ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org
[PATCH libinput 3/3] evdev-mt-touchpad-tap: Switch over to new timer subsystem
Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-tap.c | 76 ++--- src/evdev-mt-touchpad.c | 1 - src/evdev-mt-touchpad.h | 7 + 3 files changed, 10 insertions(+), 74 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index 2b0d21c..588570c 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -30,9 +30,7 @@ #include stdbool.h #include stdio.h #include string.h -#include time.h #include unistd.h -#include sys/timerfd.h #include evdev-mt-touchpad.h @@ -120,22 +118,13 @@ tp_tap_notify(struct tp_dispatch *tp, static void tp_tap_set_timer(struct tp_dispatch *tp, uint64_t time) { - uint64_t timeout = time + DEFAULT_TAP_TIMEOUT_PERIOD; - struct itimerspec its; - - its.it_interval.tv_sec = 0; - its.it_interval.tv_nsec = 0; - its.it_value.tv_sec = timeout / 1000; - its.it_value.tv_nsec = (timeout % 1000) * 1000 * 1000; - timerfd_settime(tp-tap.timer_fd, TFD_TIMER_ABSTIME, its, NULL); - - tp-tap.timeout = timeout; + libinput_timer_set(tp-tap.timer, time + DEFAULT_TAP_TIMEOUT_PERIOD); } static void tp_tap_clear_timer(struct tp_dispatch *tp) { - tp-tap.timeout = 0; + libinput_timer_cancel(tp-tap.timer); } static void @@ -548,60 +537,21 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time) } static void -tp_tap_timeout_handler(void *data) +tp_tap_handle_timeout(uint64_t time, void *data) { - struct tp_dispatch *touchpad = data; - uint64_t expires; - int len; - struct timespec ts; - uint64_t now; - - len = read(touchpad-tap.timer_fd, expires, sizeof expires); - if (len != sizeof expires) - /* This will only happen if the application made the fd -* non-blocking, but this function should only be called -* upon the timeout, so lets continue anyway. */ - log_error(timerfd read error: %s\n, strerror(errno)); - - clock_gettime(CLOCK_MONOTONIC, ts); - now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; - - tp_tap_handle_timeout(touchpad, now); -} - -unsigned int -tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) -{ - if (!tp-tap.enabled) - return 0; - - if (tp-tap.timeout tp-tap.timeout = time) { - tp_tap_clear_timer(tp); - tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); - } + struct tp_dispatch *tp = data; - return tp-tap.timeout; + tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); } int tp_init_tap(struct tp_dispatch *tp) { tp-tap.state = TAP_STATE_IDLE; - tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); - if (tp-tap.timer_fd == -1) - return -1; - - tp-tap.source = - libinput_add_fd(tp-device-base.seat-libinput, - tp-tap.timer_fd, - tp_tap_timeout_handler, - tp); - - if (tp-tap.source == NULL) { - close(tp-tap.timer_fd); - return -1; - } + libinput_timer_init(tp-tap.timer, + tp-device-base.seat-libinput, + tp_tap_handle_timeout, tp); tp-tap.enabled = 1; /* FIXME */ @@ -611,13 +561,5 @@ tp_init_tap(struct tp_dispatch *tp) void tp_destroy_tap(struct tp_dispatch *tp) { - if (tp-tap.source) { - libinput_remove_source(tp-device-base.seat-libinput, - tp-tap.source); - tp-tap.source = NULL; - } - if (tp-tap.timer_fd -1) { - close(tp-tap.timer_fd); - tp-tap.timer_fd = -1; - } + libinput_timer_cancel(tp-tap.timer); } diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index ba02122..d749a29 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -754,7 +754,6 @@ tp_init(struct tp_dispatch *tp, tp-base.interface = tp_interface; tp-device = device; - tp-tap.timer_fd = -1; if (tp_init_slots(tp, device) != 0) return -1; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 1749a55..0b1457d 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -200,9 +200,7 @@ struct tp_dispatch { struct { bool enabled; - int timer_fd; - struct libinput_source *source; - unsigned int timeout; + struct libinput_timer timer; enum tp_tap_state state; } tap; }; @@ -219,9 +217,6 @@ tp_set_pointer(struct tp_dispatch *tp, struct tp_touch *t); int tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time); -unsigned int -tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time); - int tp_init_tap(struct
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Hi, On 06/04/2014 10:57 PM, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote: Hi, On 06/03/2014 07:34 AM, Peter Hutterer wrote: Now that we have run-time changes of the tap.enabled state move the check to the IDLE state only. Otherwise the tap machine may hang if tapping is disabled while a gesture is in progress. Two basic tests are added to check for the tap default setting - which is now tap disabled by default, because a bike shed's correct color is green. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-tap.c | 56 +++-- src/evdev-mt-touchpad.h | 1 + test/touchpad.c | 32 ++ 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index eee334f..24ea8c3 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -438,13 +438,13 @@ static void tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) { enum tp_tap_state current; - if (!tp-tap.enabled) - return; current = tp-tap.state; switch(tp-tap.state) { case TAP_STATE_IDLE: + if (!tp-tap.enabled) + break; tp_tap_idle_handle_event(tp, event, time); break; case TAP_STATE_TOUCH: @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data) unsigned int tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) { - if (!tp-tap.enabled) - return 0; - if (tp-tap.timeout tp-tap.timeout = time) { tp_tap_clear_timer(tp); tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) return tp-tap.timeout; } +static int +tp_tap_config_count(struct libinput_device *device) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; + struct tp_dispatch *tp = container_of(dispatch, tp, base); + + return min(tp-ntouches, 3); /* we only do up to 3 finger tap */ +} + +static enum libinput_config_status +tp_tap_config_enable(struct libinput_device *device, int enabled) +{ + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)-dispatch; Please user container_of here for proper type checking rather then just a blatant cast of $random_type_foo to $random_type_bar. Same for all the other occurrences of the same pattern below. Would just like to point out that there are several places this cast is already in place, and starting to use container_of would introduce inconsistencies. I think this kind of casting isn't necessarily bad for single (or primary) inheritance types of objects. I really believe that we should make use of compiler type checking were possible. Lets go with Peters suggestion to do a separate patch to move all these kind of casts over to container_of in one big patch. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/3] Add a timer subsystem
Hi, On 06/05/2014 12:18 AM, Jonas Ådahl wrote: On Wed, Jun 04, 2014 at 05:26:34PM +0200, Hans de Goede wrote: Currently we are using DIY timers in the touchpad softbutton and tap handling code, and at least the softbutton code gets its wrong. It uses one timer-fd per touchpad to set a timeout per touch, which means that if a timeout is set for 100ms from now for touch 1, and then a timeout gets set 50 ms later for 200 ms from now, then the timeout for touch 1 will come 150 ms too late. This commits adds a proper timer subsystem so that we've one place to deal with timer handling, and so that we can only get it wrong (well hopefully we get it right) in one place. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/Makefile.am| 2 + src/libinput-private.h | 9 ++- src/libinput.c | 11 +++- src/timer.c| 149 + src/timer.h| 56 +++ 5 files changed, 224 insertions(+), 3 deletions(-) create mode 100644 src/timer.c create mode 100644 src/timer.h diff --git a/src/Makefile.am b/src/Makefile.am index efb089a..8b56348 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -22,6 +22,8 @@ libinput_la_SOURCES = \ path.c \ udev-seat.c \ udev-seat.h \ +timer.c \ +timer.h \ ../include/linux/input.h libinput_la_LIBADD = $(MTDEV_LIBS) \ diff --git a/src/libinput-private.h b/src/libinput-private.h index 0e92e68..ed8190d 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -28,6 +28,8 @@ #include libinput.h #include libinput-util.h +struct libinput_source; + struct libinput_interface_backend { int (*resume)(struct libinput *libinput); void (*suspend)(struct libinput *libinput); @@ -40,6 +42,12 @@ struct libinput { struct list seat_list; +struct { +struct list list; +struct libinput_source *source; +int fd; +} timer; + struct libinput_event **events; size_t events_count; size_t events_len; @@ -79,7 +87,6 @@ struct libinput_device { typedef void (*libinput_source_dispatch_t)(void *data); -struct libinput_source; #define log_debug(...) log_msg(LIBINPUT_LOG_PRIORITY_DEBUG, __VA_ARGS__) #define log_info(...) log_msg(LIBINPUT_LOG_PRIORITY_INFO, __VA_ARGS__) diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..e74d29f 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -33,6 +33,7 @@ #include libinput.h #include libinput-private.h #include evdev.h +#include timer.h struct libinput_source { libinput_source_dispatch_t dispatch; @@ -485,6 +486,12 @@ libinput_init(struct libinput *libinput, list_init(libinput-source_destroy_list); list_init(libinput-seat_list); +if (libinput_timer_subsys_init(libinput) != 0) { +free(libinput-events); +close(libinput-epoll_fd); +return -1; +} + return 0; } @@ -521,8 +528,6 @@ libinput_destroy(struct libinput *libinput) while ((event = libinput_get_event(libinput))) libinput_event_destroy(event); -libinput_drop_destroyed_sources(libinput); - free(libinput-events); list_for_each_safe(seat, next_seat, libinput-seat_list, link) { @@ -534,6 +539,8 @@ libinput_destroy(struct libinput *libinput) libinput_seat_destroy(seat); } +libinput_timer_subsys_exit(libinput); +libinput_drop_destroyed_sources(libinput); close(libinput-epoll_fd); free(libinput); } diff --git a/src/timer.c b/src/timer.c new file mode 100644 index 000..349a6c9 --- /dev/null +++ b/src/timer.c @@ -0,0 +1,149 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided as is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
Re: [PATCH libinput 1/3] Add a timer subsystem
Hi, On 06/05/2014 05:40 AM, Peter Hutterer wrote: On Wed, Jun 04, 2014 at 05:26:34PM +0200, Hans de Goede wrote: Currently we are using DIY timers in the touchpad softbutton and tap handling code, and at least the softbutton code gets its wrong. It uses one timer-fd per touchpad to set a timeout per touch, which means that if a timeout is set for 100ms from now for touch 1, and then a timeout gets set 50 ms later for 200 ms from now, then the timeout for touch 1 will come 150 ms too late. This commits adds a proper timer subsystem so that we've one place to deal with timer handling, and so that we can only get it wrong (well hopefully we get it right) in one place. Signed-off-by: Hans de Goede hdego...@redhat.com --- [...] +static void +libinput_timer_handler(void *data) +{ +struct libinput *libinput = data; +struct libinput_timer *timer, *tmp; +struct timespec ts; +uint64_t now; +int r; + +r = clock_gettime(CLOCK_MONOTONIC, ts); +if (r) { +log_error(clock_gettime error: %s\n, strerror(errno)); +return; +} + +now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; + +list_for_each_safe(timer, tmp, libinput-timer.list, list) { +if (timer-expire = now) { +/* Clear the timer before calling timer_func, + as timer_func may re-arm it */ +libinput_timer_cancel(timer); +timer-timer_func(now, timer-timer_func_data); would it make sense to pass the timer itself into the timer func, for the use-cases where the func handles multiple timers and only needs to e.g. print and re-arm that timer? Maybe, if this were a public API I would agree with adding it now, just to be sure. But since it is not, I vote for leaving it out until we actually need it. this is an internal API so we can add it when needed later. +} +} +} + +int +libinput_timer_subsys_init(struct libinput *libinput) +{ +libinput-timer.fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); +if (libinput-timer.fd 0) +return -1; + +list_init(libinput-timer.list); + +libinput-timer.source = libinput_add_fd(libinput, + libinput-timer.fd, + libinput_timer_handler, + libinput); +if (!libinput-timer.source) { +close(libinput-timer.fd); +return -1; +} + +return 0; +} + +void +libinput_timer_subsys_exit(struct libinput *libinput) +{ +/* All timer uses should have destroyed their timers now */ typo: users Fixed. +struct libinput_timer { + struct libinput *libinput; + struct list list; + uint64_t expire; do me a favour and add a comment here and in libinput_timer_set that this is in abstime, not reltime. Done. + void (*timer_func)(uint64_t now, void *timer_func_data); + void *timer_func_data; +}; series: Reviewed-by: Peter Hutterer peter.hutte...@who-t.net with Jonas' comments and the above addressed. two other things that you may or may not want to add because they help debugging but require some more effort: - put a log_bug_libinput() into libinput_timer_set if the timer is current time - put a log_bug_libinput() into libinput_timer_set if the timer is more than say 5s into the future. this is arbitrary but a good safety guard that can save on some debugging time. An interesting idea, the kernel developer in me objects, as doing these checks means making a gettimeofday call, and those calls are far from cheap. But we should not be setting timers that often, so I'll put adding these checks on my todo list and I'll do an addon patch with them at some later date :) Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 2/3] evdev-mt-touchpad-buttons: Switch over to new timer subsystem
Besides being a nice cleanup, this gives us proper per touch timeouts. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 95 + src/evdev-mt-touchpad.c | 3 +- src/evdev-mt-touchpad.h | 9 ++-- 3 files changed, 24 insertions(+), 83 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index b2e87a3..831c01d 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -22,12 +22,10 @@ #include errno.h #include limits.h -#include time.h #include math.h #include string.h #include unistd.h #include linux/input.h -#include sys/timerfd.h #include evdev-mt-touchpad.h @@ -130,34 +128,17 @@ is_inside_top_middle_area(struct tp_dispatch *tp, struct tp_touch *t) } static void -tp_button_set_timer(struct tp_dispatch *tp, uint64_t timeout) -{ - struct itimerspec its; - its.it_interval.tv_sec = 0; - its.it_interval.tv_nsec = 0; - its.it_value.tv_sec = timeout / 1000; - its.it_value.tv_nsec = (timeout % 1000) * 1000 * 1000; - timerfd_settime(tp-buttons.timer_fd, TFD_TIMER_ABSTIME, its, NULL); -} - -static void tp_button_set_enter_timer(struct tp_dispatch *tp, struct tp_touch *t) { - t-button.timeout = t-millis + DEFAULT_BUTTON_ENTER_TIMEOUT; - tp_button_set_timer(tp, t-button.timeout); + libinput_timer_set(t-button.timer, + t-millis + DEFAULT_BUTTON_ENTER_TIMEOUT); } static void tp_button_set_leave_timer(struct tp_dispatch *tp, struct tp_touch *t) { - t-button.timeout = t-millis + DEFAULT_BUTTON_LEAVE_TIMEOUT; - tp_button_set_timer(tp, t-button.timeout); -} - -static void -tp_button_clear_timer(struct tp_dispatch *tp, struct tp_touch *t) -{ - t-button.timeout = 0; + libinput_timer_set(t-button.timer, + t-millis + DEFAULT_BUTTON_LEAVE_TIMEOUT); } /* @@ -168,7 +149,7 @@ static void tp_button_set_state(struct tp_dispatch *tp, struct tp_touch *t, enum button_state new_state, enum button_event event) { - tp_button_clear_timer(tp, t); + libinput_timer_cancel(t-button.timer); t-button.state = new_state; switch (t-button.state) { @@ -545,16 +526,11 @@ tp_button_handle_state(struct tp_dispatch *tp, uint64_t time) } static void -tp_button_handle_timeout(struct tp_dispatch *tp, uint64_t now) +tp_button_handle_timeout(uint64_t now, void *data) { - struct tp_touch *t; + struct tp_touch *t = data; - tp_for_each_touch(tp, t) { - if (t-button.timeout != 0 t-button.timeout = now) { - tp_button_clear_timer(tp, t); - tp_button_handle_event(tp, t, BUTTON_EVENT_TIMEOUT, now); - } - } + tp_button_handle_event(t-tp, t, BUTTON_EVENT_TIMEOUT, now); } int @@ -582,32 +558,11 @@ tp_process_button(struct tp_dispatch *tp, return 0; } -static void -tp_button_timeout_handler(void *data) -{ - struct tp_dispatch *tp = data; - uint64_t expires; - int len; - struct timespec ts; - uint64_t now; - - len = read(tp-buttons.timer_fd, expires, sizeof expires); - if (len != sizeof expires) - /* This will only happen if the application made the fd -* non-blocking, but this function should only be called -* upon the timeout, so lets continue anyway. */ - log_error(timerfd read error: %s\n, strerror(errno)); - - clock_gettime(CLOCK_MONOTONIC, ts); - now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; - - tp_button_handle_timeout(tp, now); -} - int tp_init_buttons(struct tp_dispatch *tp, struct evdev_device *device) { + struct tp_touch *t; int width, height; double diagonal; @@ -645,38 +600,28 @@ tp_init_buttons(struct tp_dispatch *tp, } else { tp-buttons.top_area.bottom_edge = INT_MIN; } - - tp-buttons.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); - if (tp-buttons.timer_fd == -1) - return -1; - - tp-buttons.source = - libinput_add_fd(tp-device-base.seat-libinput, - tp-buttons.timer_fd, - tp_button_timeout_handler, - tp); - if (tp-buttons.source == NULL) - return -1; } else { tp-buttons.bottom_area.top_edge = INT_MAX; tp-buttons.top_area.bottom_edge = INT_MIN; } + tp_for_each_touch(tp, t) { + t-button.state = BUTTON_STATE_NONE; + libinput_timer_init(t-button.timer
[PATCH libinput 0/3] Add a timer subsystem series v2
Hi, Here is v2 of my timer subsystem series with all the review comments fixed. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/3] Add a timer subsystem
Currently we are using DIY timers in the touchpad softbutton and tap handling code, and at least the softbutton code gets its wrong. It uses one timer-fd per touchpad to set a timeout per touch, which means that if a timeout is set for 100ms from now for touch 1, and then 50 ms later touch 2 sets a timeout for 200 ms from now, then the timeout for touch 1 will come 150 ms too late. This commits adds a proper timer subsystem so that we've one place to deal with timer handling, and so that we can only get it wrong (well hopefully we get it right) in one place. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/Makefile.am| 2 + src/libinput-private.h | 9 +++- src/libinput.c | 11 +++- src/timer.c| 144 + src/timer.h| 56 +++ 5 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 src/timer.c create mode 100644 src/timer.h diff --git a/src/Makefile.am b/src/Makefile.am index efb089a..8b56348 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -22,6 +22,8 @@ libinput_la_SOURCES = \ path.c \ udev-seat.c \ udev-seat.h \ + timer.c \ + timer.h \ ../include/linux/input.h libinput_la_LIBADD = $(MTDEV_LIBS) \ diff --git a/src/libinput-private.h b/src/libinput-private.h index 0e92e68..ed8190d 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -28,6 +28,8 @@ #include libinput.h #include libinput-util.h +struct libinput_source; + struct libinput_interface_backend { int (*resume)(struct libinput *libinput); void (*suspend)(struct libinput *libinput); @@ -40,6 +42,12 @@ struct libinput { struct list seat_list; + struct { + struct list list; + struct libinput_source *source; + int fd; + } timer; + struct libinput_event **events; size_t events_count; size_t events_len; @@ -79,7 +87,6 @@ struct libinput_device { typedef void (*libinput_source_dispatch_t)(void *data); -struct libinput_source; #define log_debug(...) log_msg(LIBINPUT_LOG_PRIORITY_DEBUG, __VA_ARGS__) #define log_info(...) log_msg(LIBINPUT_LOG_PRIORITY_INFO, __VA_ARGS__) diff --git a/src/libinput.c b/src/libinput.c index 6b7e8b8..5c66159 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -33,6 +33,7 @@ #include libinput.h #include libinput-private.h #include evdev.h +#include timer.h struct libinput_source { libinput_source_dispatch_t dispatch; @@ -485,6 +486,12 @@ libinput_init(struct libinput *libinput, list_init(libinput-source_destroy_list); list_init(libinput-seat_list); + if (libinput_timer_subsys_init(libinput) != 0) { + free(libinput-events); + close(libinput-epoll_fd); + return -1; + } + return 0; } @@ -521,8 +528,6 @@ libinput_destroy(struct libinput *libinput) while ((event = libinput_get_event(libinput))) libinput_event_destroy(event); - libinput_drop_destroyed_sources(libinput); - free(libinput-events); list_for_each_safe(seat, next_seat, libinput-seat_list, link) { @@ -534,6 +539,8 @@ libinput_destroy(struct libinput *libinput) libinput_seat_destroy(seat); } + libinput_timer_subsys_destroy(libinput); + libinput_drop_destroyed_sources(libinput); close(libinput-epoll_fd); free(libinput); } diff --git a/src/timer.c b/src/timer.c new file mode 100644 index 000..65fdd17 --- /dev/null +++ b/src/timer.c @@ -0,0 +1,144 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided as is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION
[PATCH libinput 3/3] evdev-mt-touchpad-tap: Switch over to new timer subsystem
Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-tap.c | 76 ++--- src/evdev-mt-touchpad.c | 1 - src/evdev-mt-touchpad.h | 7 + 3 files changed, 10 insertions(+), 74 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index 2b0d21c..588570c 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -30,9 +30,7 @@ #include stdbool.h #include stdio.h #include string.h -#include time.h #include unistd.h -#include sys/timerfd.h #include evdev-mt-touchpad.h @@ -120,22 +118,13 @@ tp_tap_notify(struct tp_dispatch *tp, static void tp_tap_set_timer(struct tp_dispatch *tp, uint64_t time) { - uint64_t timeout = time + DEFAULT_TAP_TIMEOUT_PERIOD; - struct itimerspec its; - - its.it_interval.tv_sec = 0; - its.it_interval.tv_nsec = 0; - its.it_value.tv_sec = timeout / 1000; - its.it_value.tv_nsec = (timeout % 1000) * 1000 * 1000; - timerfd_settime(tp-tap.timer_fd, TFD_TIMER_ABSTIME, its, NULL); - - tp-tap.timeout = timeout; + libinput_timer_set(tp-tap.timer, time + DEFAULT_TAP_TIMEOUT_PERIOD); } static void tp_tap_clear_timer(struct tp_dispatch *tp) { - tp-tap.timeout = 0; + libinput_timer_cancel(tp-tap.timer); } static void @@ -548,60 +537,21 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time) } static void -tp_tap_timeout_handler(void *data) +tp_tap_handle_timeout(uint64_t time, void *data) { - struct tp_dispatch *touchpad = data; - uint64_t expires; - int len; - struct timespec ts; - uint64_t now; - - len = read(touchpad-tap.timer_fd, expires, sizeof expires); - if (len != sizeof expires) - /* This will only happen if the application made the fd -* non-blocking, but this function should only be called -* upon the timeout, so lets continue anyway. */ - log_error(timerfd read error: %s\n, strerror(errno)); - - clock_gettime(CLOCK_MONOTONIC, ts); - now = ts.tv_sec * 1000ULL + ts.tv_nsec / 100; - - tp_tap_handle_timeout(touchpad, now); -} - -unsigned int -tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time) -{ - if (!tp-tap.enabled) - return 0; - - if (tp-tap.timeout tp-tap.timeout = time) { - tp_tap_clear_timer(tp); - tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); - } + struct tp_dispatch *tp = data; - return tp-tap.timeout; + tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); } int tp_init_tap(struct tp_dispatch *tp) { tp-tap.state = TAP_STATE_IDLE; - tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); - if (tp-tap.timer_fd == -1) - return -1; - - tp-tap.source = - libinput_add_fd(tp-device-base.seat-libinput, - tp-tap.timer_fd, - tp_tap_timeout_handler, - tp); - - if (tp-tap.source == NULL) { - close(tp-tap.timer_fd); - return -1; - } + libinput_timer_init(tp-tap.timer, + tp-device-base.seat-libinput, + tp_tap_handle_timeout, tp); tp-tap.enabled = 1; /* FIXME */ @@ -611,13 +561,5 @@ tp_init_tap(struct tp_dispatch *tp) void tp_destroy_tap(struct tp_dispatch *tp) { - if (tp-tap.source) { - libinput_remove_source(tp-device-base.seat-libinput, - tp-tap.source); - tp-tap.source = NULL; - } - if (tp-tap.timer_fd -1) { - close(tp-tap.timer_fd); - tp-tap.timer_fd = -1; - } + libinput_timer_cancel(tp-tap.timer); } diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 44f2139..e42af0c 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -750,7 +750,6 @@ tp_init(struct tp_dispatch *tp, tp-base.interface = tp_interface; tp-device = device; - tp-tap.timer_fd = -1; if (tp_init_slots(tp, device) != 0) return -1; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 1749a55..0b1457d 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -200,9 +200,7 @@ struct tp_dispatch { struct { bool enabled; - int timer_fd; - struct libinput_source *source; - unsigned int timeout; + struct libinput_timer timer; enum tp_tap_state state; } tap; }; @@ -219,9 +217,6 @@ tp_set_pointer(struct tp_dispatch *tp, struct tp_touch *t); int tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time); -unsigned int -tp_tap_handle_timeout(struct
Re: [PATCH libinput 1/8] Drop empty FFI_CFLAGS
Hi, All patches in this series look good and are: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans On 06/06/2014 08:18 AM, Peter Hutterer wrote: Leftover from weston Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/Makefile.am | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 3337a83..ff93911 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -38,7 +38,6 @@ libinput_la_LDFLAGS = -version-info $(LIBINPUT_LT_VERSION) pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = libinput.pc -AM_CPPFLAGS = $(FFI_CFLAGS) AM_CFLAGS = $(GCC_CFLAGS) DISTCLEANFILES = libinput-version.h ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput] Add a log_msg_va function
This is useful for when we use libraries which want us to provide them with a logging callback. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/libinput-private.h | 3 +++ src/libinput.c | 16 +++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index ed8190d..a5f1405 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -97,6 +97,9 @@ typedef void (*libinput_source_dispatch_t)(void *data); void log_msg(enum libinput_log_priority priority, const char *format, ...); +void +log_msg_va(enum libinput_log_priority priority, const char *format, + va_list args); int libinput_init(struct libinput *libinput, diff --git a/src/libinput.c b/src/libinput.c index 5c66159..a6d7af8 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -111,15 +111,21 @@ static struct log_data log_data = { }; void +log_msg_va(enum libinput_log_priority priority, const char *format, + va_list args) +{ + if (log_data.handler log_data.priority = priority) + log_data.handler(priority, log_data.user_data, format, args); +} + +void log_msg(enum libinput_log_priority priority, const char *format, ...) { va_list args; - if (log_data.handler log_data.priority = priority) { - va_start(args, format); - log_data.handler(priority, log_data.user_data, format, args); - va_end(args); - } + va_start(args, format); + log_msg_va(priority, format, args); + va_end(args); } LIBINPUT_EXPORT void -- 2.0.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 1/2] Add a log_msg_va function
This is useful for when we use libraries which want us to provide them with a logging callback. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/libinput-private.h | 3 +++ src/libinput.c | 16 +++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/libinput-private.h b/src/libinput-private.h index c18447a..fd4fc29 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -97,6 +97,9 @@ typedef void (*libinput_source_dispatch_t)(void *data); void log_msg(enum libinput_log_priority priority, const char *format, ...); +void +log_msg_va(enum libinput_log_priority priority, const char *format, + va_list args); int libinput_init(struct libinput *libinput, diff --git a/src/libinput.c b/src/libinput.c index aee373e..fda6c56 100644 --- a/src/libinput.c +++ b/src/libinput.c @@ -111,15 +111,21 @@ static struct log_data log_data = { }; void +log_msg_va(enum libinput_log_priority priority, const char *format, + va_list args) +{ + if (log_data.handler log_data.priority = priority) + log_data.handler(priority, log_data.user_data, format, args); +} + +void log_msg(enum libinput_log_priority priority, const char *format, ...) { va_list args; - if (log_data.handler log_data.priority = priority) { - va_start(args, format); - log_data.handler(priority, log_data.user_data, format, args); - va_end(args); - } + va_start(args, format); + log_msg_va(priority, format, args); + va_end(args); } LIBINPUT_EXPORT void -- 2.0.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 2/2] timer.h: Add #include libinput-util.h
libinput-util.h is needed for the linked list definitions. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/timer.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/timer.h b/src/timer.h index df12d0e..5e3b89b 100644 --- a/src/timer.h +++ b/src/timer.h @@ -25,6 +25,8 @@ #include stdint.h +#include libinput-util.h + struct libinput; struct libinput_timer { -- 2.0.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 2/6] test: add litest_assert_empty_queue helper function
Hi, On 06/11/2014 02:11 AM, Peter Hutterer wrote: Checks if the queue is empty and prints informatino about any events before failing. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans --- test/litest.c | 61 + test/litest.h | 1 + test/touchpad.c | 9 ++--- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/test/litest.c b/test/litest.c index 1f1bf7b..571cf76 100644 --- a/test/litest.c +++ b/test/litest.c @@ -737,6 +737,67 @@ litest_drain_events(struct libinput *li) } } +static void +litest_print_event(struct libinput_event *event) +{ + struct libinput_event_pointer *p; + struct libinput_device *dev; + enum libinput_event_type type; + double x, y; + + dev = libinput_event_get_device(event); + type = libinput_event_get_type(event); + + fprintf(stderr, + device %s type %d , + libinput_device_get_sysname(dev), + type); + switch (type) { + case LIBINPUT_EVENT_POINTER_MOTION: + p = libinput_event_get_pointer_event(event); + x = libinput_event_pointer_get_dx(p); + y = libinput_event_pointer_get_dy(p); + fprintf(stderr, motion: %.2f/%.2f, x, y); + break; + case LIBINPUT_EVENT_POINTER_MOTION_ABSOLUTE: + p = libinput_event_get_pointer_event(event); + x = libinput_event_pointer_get_absolute_x(p); + y = libinput_event_pointer_get_absolute_y(p); + fprintf(stderr, motion: %.2f/%.2f, x, y); + break; + case LIBINPUT_EVENT_POINTER_BUTTON: + p = libinput_event_get_pointer_event(event); + fprintf(stderr, + button: %d state %d, + libinput_event_pointer_get_button(p), + libinput_event_pointer_get_button_state(p)); + break; + default: + break; + } + + fprintf(stderr, \n); +} + +void +litest_assert_empty_queue(struct libinput *li) +{ + bool empty_queue = true; + struct libinput_event *event; + + libinput_dispatch(li); + while ((event = libinput_get_event(li))) { + empty_queue = false; + fprintf(stderr, + Unexpected event: ); + litest_print_event(event); + libinput_event_destroy(event); + libinput_dispatch(li); + } + + ck_assert(empty_queue); +} + struct libevdev_uinput * litest_create_uinput_device_from_description(const char *name, const struct input_id *id, diff --git a/test/litest.h b/test/litest.h index 32e1cb0..170c87c 100644 --- a/test/litest.h +++ b/test/litest.h @@ -123,6 +123,7 @@ void litest_keyboard_key(struct litest_device *d, unsigned int key, bool is_press); void litest_drain_events(struct libinput *li); +void litest_assert_empty_queue(struct libinput *li); struct libevdev_uinput * litest_create_uinput_device(const char *name, struct input_id *id, diff --git a/test/touchpad.c b/test/touchpad.c index 9c95309..ec412d3 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -182,9 +182,7 @@ START_TEST(touchpad_1fg_tap_n_drag) assert_button_event(li, BTN_LEFT, LIBINPUT_BUTTON_STATE_RELEASED); - libinput_dispatch(li); - event = libinput_get_event(li); - ck_assert(event == NULL); + litest_assert_empty_queue(li); } END_TEST @@ -192,7 +190,6 @@ START_TEST(touchpad_2fg_tap) { struct litest_device *dev = litest_current_device(); struct libinput *li = dev-libinput; - struct libinput_event *event; litest_drain_events(dev-libinput); @@ -209,9 +206,7 @@ START_TEST(touchpad_2fg_tap) assert_button_event(li, BTN_RIGHT, LIBINPUT_BUTTON_STATE_RELEASED); - libinput_dispatch(li); - event = libinput_get_event(li); - ck_assert(event == NULL); + litest_assert_empty_queue(li); } END_TEST ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/6] touchpad: always call into the the tap state machine
Hi, On 06/11/2014 02:11 AM, Peter Hutterer wrote: A button event consumed by the softbutton or clickpad code does not feed into the tap state machine, leaving it in its current state. The touch generating that event however may have triggered state changes. For some tap/click combinations this gives us either double press/release events or an inconsistent order of events. Those issues include: * a really short physical click causes a click + tap-click * a really short physical click on the right software button causes a right click + left tap-click * tap + click causes double button left press events To avoid these, notify the tap code that a button event has occured and process that accordingly. Depending on the state this may either continue to the DEAD state or release the current tap button and then go to the DEAD state. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans --- src/evdev-mt-touchpad.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 8b502b7..0294eb2 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -545,13 +545,12 @@ tp_post_events(struct tp_dispatch *tp, uint64_t time) { struct tp_touch *t = tp_current_touch(tp); double dx, dy; + int consumed = 0; - if (tp_post_button_events(tp, time) != 0) { - tp_stop_scroll_events(tp, time); - return; - } + consumed |= tp_tap_handle_state(tp, time); + consumed |= tp_post_button_events(tp, time); - if (tp_tap_handle_state(tp, time) != 0) { + if (consumed) { tp_stop_scroll_events(tp, time); return; } ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 3/6] test: add a bunch of test for click behavior on touchpads
Hi, On 06/11/2014 02:11 AM, Peter Hutterer wrote: Mainly testing the behaviour when clicking during a tap or tap-n-drag. Adds a new feature to the litest system, Apple clickpads don't have software buttons by default. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net I've some remarks on the last test, with those fixed: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans --- test/litest-bcm5974.c | 3 +- test/litest.h | 1 + test/touchpad.c | 180 ++ 3 files changed, 183 insertions(+), 1 deletion(-) diff --git a/test/litest-bcm5974.c b/test/litest-bcm5974.c index 33127d9..10a9eb4 100644 --- a/test/litest-bcm5974.c +++ b/test/litest-bcm5974.c @@ -97,7 +97,8 @@ static int events[] = { struct litest_test_device litest_bcm5974_device = { .type = LITEST_BCM5974, - .features = LITEST_TOUCHPAD | LITEST_CLICKPAD | LITEST_BUTTON, + .features = LITEST_TOUCHPAD | LITEST_CLICKPAD | + LITEST_BUTTON | LITEST_APPLE_CLICKPAD, .shortname = bcm5974, .setup = litest_bcm5974_setup, .interface = interface, diff --git a/test/litest.h b/test/litest.h index 170c87c..ea7d299 100644 --- a/test/litest.h +++ b/test/litest.h @@ -55,6 +55,7 @@ enum litest_device_feature { LITEST_WHEEL = 1 5, LITEST_TOUCH = 1 6, LITEST_SINGLE_TOUCH = 1 7, + LITEST_APPLE_CLICKPAD = 1 8, }; struct litest_device { diff --git a/test/touchpad.c b/test/touchpad.c index ec412d3..f9e2820 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -210,6 +210,179 @@ START_TEST(touchpad_2fg_tap) } END_TEST +START_TEST(touchpad_1fg_tap_click) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(dev-libinput); + + /* finger down, button click, finger up +- only one button left event pair */ + litest_touch_down(dev, 0, 50, 50); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_touch_up(dev, 0); + + libinput_dispatch(li); + + assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(touchpad_2fg_tap_click) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(dev-libinput); + + /* two fingers down, button click, fingers up +- only one button left event pair */ + litest_touch_down(dev, 0, 50, 50); + litest_touch_down(dev, 1, 70, 50); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_touch_up(dev, 1); + litest_touch_up(dev, 0); + + libinput_dispatch(li); + + assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(touchpad_2fg_tap_click_apple) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(dev-libinput); + + /* two fingers down, button click, fingers up +- only one button right event pair +(apple have clickfinger enabled by default) */ + litest_touch_down(dev, 0, 50, 50); + litest_touch_down(dev, 1, 70, 50); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_touch_up(dev, 1); + litest_touch_up(dev, 0); + + libinput_dispatch(li); + + assert_button_event(li, BTN_RIGHT, + LIBINPUT_BUTTON_STATE_PRESSED); + assert_button_event(li, BTN_RIGHT, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(touchpad_1fg_double_tap_click) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(dev-libinput); + + /* one finger down, up, down, button click, finger up +- two button left event pairs */ + litest_touch_down(dev, 0, 50, 50); + litest_touch_up(dev, 0); + litest_touch_down(dev, 0, 50, 50); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_event(dev
Re: [PATCH libinput 5/6] test: Add description for the T440 synaptics touchpad
Hi, On 06/11/2014 02:11 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/Makefile.am | 1 + test/litest-synaptics-t440.c | 112 +++ test/litest.c| 2 + test/litest.h| 2 + 4 files changed, 117 insertions(+) create mode 100644 test/litest-synaptics-t440.c Looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans diff --git a/test/Makefile.am b/test/Makefile.am index 70e83d4..b5dc33c 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -19,6 +19,7 @@ liblitest_la_SOURCES = \ litest-mouse.c \ litest-synaptics.c \ litest-synaptics-st.c \ + litest-synaptics-t440.c \ litest-trackpoint.c \ litest-wacom-touch.c \ litest.c diff --git a/test/litest-synaptics-t440.c b/test/litest-synaptics-t440.c new file mode 100644 index 000..1b66494 --- /dev/null +++ b/test/litest-synaptics-t440.c @@ -0,0 +1,112 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * + * Permission to use, copy, modify, distribute, and sell this software and its + * documentation for any purpose is hereby granted without fee, provided that + * the above copyright notice appear in all copies and that both that copyright + * notice and this permission notice appear in supporting documentation, and + * that the name of the copyright holders not be used in advertising or + * publicity pertaining to distribution of the software without specific, + * written prior permission. The copyright holders make no representations + * about the suitability of this software for any purpose. It is provided as + * is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE + * OF THIS SOFTWARE. + */ + +#if HAVE_CONFIG_H +#include config.h +#endif + +#include litest.h +#include litest-int.h + +static void +litest_synaptics_t440_setup(void) +{ + struct litest_device *d = litest_create_device(LITEST_SYNAPTICS_TOPBUTTONPAD); + litest_set_current_device(d); +} + +static struct input_event down[] = { + { .type = EV_KEY, .code = BTN_TOOL_FINGER, .value = 1 }, + { .type = EV_KEY, .code = BTN_TOUCH, .value = 1 }, + { .type = EV_ABS, .code = ABS_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_PRESSURE, .value = 30 }, + { .type = EV_ABS, .code = ABS_MT_SLOT, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_TRACKING_ID, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_SYN, .code = SYN_REPORT, .value = 0 }, + { .type = -1, .code = -1 }, +}; + +static struct input_event move[] = { + { .type = EV_ABS, .code = ABS_MT_SLOT, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_X, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_ABS, .code = ABS_MT_POSITION_Y, .value = LITEST_AUTO_ASSIGN }, + { .type = EV_KEY, .code = BTN_TOOL_FINGER, .value = 1 }, + { .type = EV_KEY, .code = BTN_TOUCH, .value = 1 }, + { .type = EV_SYN, .code = SYN_REPORT, .value = 0 }, + { .type = -1, .code = -1 }, +}; + +static struct litest_device_interface interface = { + .touch_down_events = down, + .touch_move_events = move, +}; + +static struct input_id input_id = { + .bustype = 0x11, + .vendor = 0x2, + .product = 0x7, +}; + +static int events[] = { + EV_KEY, BTN_LEFT, + EV_KEY, BTN_TOOL_FINGER, + EV_KEY, BTN_TOOL_QUINTTAP, + EV_KEY, BTN_TOUCH, + EV_KEY, BTN_TOOL_DOUBLETAP, + EV_KEY, BTN_TOOL_TRIPLETAP, + EV_KEY, BTN_TOOL_QUADTAP, + INPUT_PROP_MAX, INPUT_PROP_POINTER, + INPUT_PROP_MAX, INPUT_PROP_BUTTONPAD, + INPUT_PROP_MAX, INPUT_PROP_TOPBUTTONPAD, + -1, -1, +}; + +static struct input_absinfo absinfo[] = { + { ABS_X, 1024, 5112, 0, 0, 42 }, + { ABS_Y, 2024, 4832, 0, 0, 42 }, + { ABS_PRESSURE, 0, 255, 0, 0, 0 }, + { ABS_TOOL_WIDTH, 0, 15, 0, 0, 0 }, + { ABS_MT_SLOT, 0, 1, 0, 0, 0 }, + { ABS_MT_POSITION_X, 1024, 5112, 0, 0, 75 }, + { ABS_MT_POSITION_Y
Re: [PATCH libinput 6/6] test: add a couple of top software button test
Hi, On 06/11/2014 02:11 AM, Peter Hutterer wrote: Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/touchpad.c | 124 1 file changed, 124 insertions(+) Looks good: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans diff --git a/test/touchpad.c b/test/touchpad.c index 7b7cb7d..147d0e2 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -888,6 +888,125 @@ START_TEST(clickpad_softbutton_right_to_left) } END_TEST +START_TEST(clickpad_topsoftbuttons_left) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + + litest_touch_down(dev, 0, 10, 5); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + + assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_empty_queue(li); + + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_touch_up(dev, 0); + + assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(clickpad_topsoftbuttons_right) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + + litest_touch_down(dev, 0, 90, 5); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + + assert_button_event(li, + BTN_RIGHT, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_empty_queue(li); + + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_touch_up(dev, 0); + + assert_button_event(li, + BTN_RIGHT, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(clickpad_topsoftbuttons_middle) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + + litest_touch_down(dev, 0, 50, 5); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + + assert_button_event(li, + BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_empty_queue(li); + + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_touch_up(dev, 0); + + assert_button_event(li, + BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(clickpad_topsoftbuttons_move_out_ignore) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + /* Finger down in top button area, wait past enter timeout +Move into main area, wait past leave timeout +Click + - expect no events + */ + + litest_drain_events(li); + + litest_touch_down(dev, 0, 50, 5); + libinput_dispatch(li); + usleep(20); + libinput_dispatch(li); + litest_assert_empty_queue(li); + + litest_touch_move_to(dev, 0, 50, 5, 80, 90, 20); + libinput_dispatch(li); + usleep(40); + libinput_dispatch(li); + + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + + litest_touch_up(dev, 0); + + litest_assert_empty_queue(li); +} +END_TEST + int main(int argc, char **argv) { litest_add(touchpad:motion, touchpad_1fg_motion, LITEST_TOUCHPAD, LITEST_ANY); @@ -920,5 +1039,10 @@ int main(int argc, char **argv) { litest_add(touchpad:softbutton, clickpad_softbutton_left_to_right, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD); litest_add(touchpad:softbutton, clickpad_softbutton_right_to_left, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD); + litest_add(touchpad:topsoftbuttons, clickpad_topsoftbuttons_left, LITEST_TOPBUTTONPAD, LITEST_ANY); + litest_add(touchpad:topsoftbuttons, clickpad_topsoftbuttons_right, LITEST_TOPBUTTONPAD, LITEST_ANY); + litest_add(touchpad:topsoftbuttons, clickpad_topsoftbuttons_middle, LITEST_TOPBUTTONPAD, LITEST_ANY); + litest_add(touchpad:topsoftbuttons, clickpad_topsoftbuttons_move_out_ignore, LITEST_TOPBUTTONPAD, LITEST_ANY); + return litest_run(argc, argv); } ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http
Re: [PATCH libinput 1/3] test: move the interface declaration down
Hi, On 06/13/2014 04:48 AM, Peter Hutterer wrote: No functional changes, just some prep work. The commit message seems wrong, as the interface declaration is actually being moved up, not down (*). With a fixed commit message this is: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans *) Must not make a joke about everything being upside down in Australia :) Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/litest.c | 35 +-- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/test/litest.c b/test/litest.c index 0a9cc72..d3f8f0d 100644 --- a/test/litest.c +++ b/test/litest.c @@ -267,6 +267,23 @@ litest_log_handler(enum libinput_log_priority pri, vfprintf(stderr, format, args); } +static int +open_restricted(const char *path, int flags, void *userdata) +{ + return open(path, flags); +} + +static void +close_restricted(int fd, void *userdata) +{ + close(fd); +} + +struct libinput_interface interface = { + .open_restricted = open_restricted, + .close_restricted = close_restricted, +}; + static const struct option opts[] = { { list, 0, 0, 'l' }, { verbose, 0, 0, 'v' }, @@ -335,24 +352,6 @@ litest_run(int argc, char **argv) { return failed; } -static int -open_restricted(const char *path, int flags, void *userdata) -{ - return open(path, flags); -} - -static void -close_restricted(int fd, void *userdata) -{ - close(fd); -} - -const struct libinput_interface interface = { - .open_restricted = open_restricted, - .close_restricted = close_restricted, -}; - - static struct input_absinfo * merge_absinfo(const struct input_absinfo *orig, const struct input_absinfo *override) ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 2/3] udev: split libinput_udev context init into two functions
(udev_double_suspend) udev = udev_new(); ck_assert(udev != NULL); - li = libinput_udev_create_for_seat(simple_interface, NULL, udev, seat0); + li = libinput_udev_create_context(simple_interface, NULL, udev); ck_assert(li != NULL); + ck_assert_int_eq(libinput_udev_set_seat(li, seat0), 0); fd = libinput_get_fd(li); ck_assert_int_ge(fd, 0); @@ -215,8 +217,9 @@ START_TEST(udev_double_resume) udev = udev_new(); ck_assert(udev != NULL); - li = libinput_udev_create_for_seat(simple_interface, NULL, udev, seat0); + li = libinput_udev_create_context(simple_interface, NULL, udev); ck_assert(li != NULL); + ck_assert_int_eq(libinput_udev_set_seat(li, seat0), 0); fd = libinput_get_fd(li); ck_assert_int_ge(fd, 0); @@ -266,8 +269,9 @@ START_TEST(udev_suspend_resume) udev = udev_new(); ck_assert(udev != NULL); - li = libinput_udev_create_for_seat(simple_interface, NULL, udev, seat0); + li = libinput_udev_create_context(simple_interface, NULL, udev); ck_assert(li != NULL); + ck_assert_int_eq(libinput_udev_set_seat(li, seat0), 0); fd = libinput_get_fd(li); ck_assert_int_ge(fd, 0); @@ -305,8 +309,9 @@ START_TEST(udev_device_sysname) udev = udev_new(); ck_assert(udev != NULL); - li = libinput_udev_create_for_seat(simple_interface, NULL, udev, seat0); + li = libinput_udev_create_context(simple_interface, NULL, udev); ck_assert(li != NULL); + ck_assert_int_eq(libinput_udev_set_seat(li, seat0), 0); libinput_dispatch(li); @@ -342,8 +347,9 @@ START_TEST(udev_seat_recycle) udev = udev_new(); ck_assert(udev != NULL); - li = libinput_udev_create_for_seat(simple_interface, NULL, udev, seat0); + li = libinput_udev_create_context(simple_interface, NULL, udev); ck_assert(li != NULL); + ck_assert_int_eq(libinput_udev_set_seat(li, seat0), 0); libinput_dispatch(li); while ((ev = libinput_get_event(li))) { diff --git a/tools/event-debug.c b/tools/event-debug.c index 864f77e..af3f648 100644 --- a/tools/event-debug.c +++ b/tools/event-debug.c @@ -140,12 +140,18 @@ open_udev(struct libinput **li) return 1; } - *li = libinput_udev_create_for_seat(interface, NULL, udev, seat); + *li = libinput_udev_create_context(interface, NULL, udev); if (!*li) { fprintf(stderr, Failed to initialize context from udev\n); return 1; } + if (libinput_udev_set_seat(*li, seat)) { + fprintf(stderr, Failed to set seat\n); + libinput_destroy(*li); + return 1; + } + return 0; } With the free() removed this patch is: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 3/3] Change the logging system to be per-context
(libinput_log_handler log_handler, - void *user_data); - -/** * @defgroup seat Initialization and manipulation of seats * * A seat has two identifiers, the physical name and the logical name. The Your removing a bunch of exported symbols here, so you should also bump the soname to indicate ABI breakage. Other then that this looks good to me and is: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans diff --git a/src/path.c b/src/path.c index 27e5ad6..e9c0ee8 100644 --- a/src/path.c +++ b/src/path.c @@ -160,7 +160,9 @@ path_device_enable(struct path_input *input, const char *devnode) if (path_get_udev_properties(devnode, sysname, seat_name, seat_logical_name) == -1) { - log_info(failed to obtain sysname for device '%s'.\n, devnode); + log_info(input-base, + failed to obtain sysname for device '%s'.\n, + devnode); return NULL; } @@ -171,7 +173,9 @@ path_device_enable(struct path_input *input, const char *devnode) } else { seat = path_seat_create(input, seat_name, seat_logical_name); if (!seat) { - log_info(failed to create seat for device '%s'.\n, devnode); + log_info(input-base, + failed to create seat for device '%s'.\n, + devnode); goto out; } } @@ -181,10 +185,14 @@ path_device_enable(struct path_input *input, const char *devnode) if (device == EVDEV_UNHANDLED_DEVICE) { device = NULL; - log_info(not using input device '%s'.\n, devnode); + log_info(input-base, + not using input device '%s'.\n, + devnode); goto out; } else if (device == NULL) { - log_info(failed to create input device '%s'.\n, devnode); + log_info(input-base, + failed to create input device '%s'.\n, + devnode); goto out; } @@ -264,7 +272,7 @@ libinput_path_add_device(struct libinput *libinput, struct libinput_device *device; if (libinput-interface_backend != interface_backend) { - log_bug_client(Mismatching backends.\n); + log_bug_client(libinput, Mismatching backends.\n); return NULL; } @@ -301,7 +309,7 @@ libinput_path_remove_device(struct libinput_device *device) struct path_device *dev; if (libinput-interface_backend != interface_backend) { - log_bug_client(Mismatching backends.\n); + log_bug_client(libinput, Mismatching backends.\n); return; } diff --git a/src/timer.c b/src/timer.c index 65fdd17..f546185 100644 --- a/src/timer.c +++ b/src/timer.c @@ -59,7 +59,7 @@ libinput_timer_arm_timer_fd(struct libinput *libinput) r = timerfd_settime(libinput-timer.fd, TFD_TIMER_ABSTIME, its, NULL); if (r) - log_error(timerfd_settime error: %s\n, strerror(errno)); + log_error(libinput, timerfd_settime error: %s\n, strerror(errno)); } void @@ -96,7 +96,7 @@ libinput_timer_handler(void *data) r = clock_gettime(CLOCK_MONOTONIC, ts); if (r) { - log_error(clock_gettime error: %s\n, strerror(errno)); + log_error(libinput, clock_gettime error: %s\n, strerror(errno)); return; } diff --git a/src/udev-seat.c b/src/udev-seat.c index 1e1307b..1758b4e 100644 --- a/src/udev-seat.c +++ b/src/udev-seat.c @@ -80,10 +80,10 @@ device_added(struct udev_device *udev_device, struct udev_input *input) libinput_seat_unref(seat-base); if (device == EVDEV_UNHANDLED_DEVICE) { - log_info(not using input device '%s'.\n, devnode); + log_info(input-base, not using input device '%s'.\n, devnode); return 0; } else if (device == NULL) { - log_info(failed to create input device '%s'.\n, devnode); + log_info(input-base, failed to create input device '%s'.\n, devnode); return 0; } @@ -100,7 +100,8 @@ device_added(struct udev_device *udev_device, struct udev_input *input) device-abs.calibration[4], device-abs.calibration[5]) == 6) { device-abs.apply_calibration = 1; - log_info(Applying calibration: %f %f %f %f %f %f\n, + log_info(input-base, + Applying calibration: %f %f %f %f %f %f\n, device-abs.calibration[0], device-abs.calibration[1], device-abs.calibration[2], @@ -128,7 +129,8 @@ device_removed(struct
Re: [PATCH libinput 02/23] evdev: Add basic support for tablet devices
(Invalid axis update: %d\n, a); + break; + } + + axis_update_needed = true; + } + + if (axis_update_needed) { + tablet_notify_axis(base, time, tablet-changed_axes, tablet-axes); This looks like this function is recursing into itself, it took me 3 times reading the code to see this is tablet_notify_axes vs tablet_notify_axis 1 letter difference in the name really is not enough, please rename one of the functions to make the name more unique. With that fixed this patch is: Reviewed-by: Hans de Goede hdego...@redhat.com Regards, Hans + memset(tablet-changed_axes, 0, sizeof(tablet-changed_axes)); + } +} + +static void +tablet_flush(struct tablet_dispatch *tablet, + struct evdev_device *device, + uint32_t time) +{ + if (tablet_has_status(tablet, TABLET_AXES_UPDATED)) { + tablet_notify_axes(tablet, device, time); + tablet_unset_status(tablet, TABLET_AXES_UPDATED); + } +} + +static void +tablet_process(struct evdev_dispatch *dispatch, +struct evdev_device *device, +struct input_event *e, +uint64_t time) +{ + struct tablet_dispatch *tablet = + (struct tablet_dispatch *)dispatch; + + switch (e-type) { + case EV_ABS: + tablet_process_absolute(tablet, device, e, time); + break; + case EV_SYN: + tablet_flush(tablet, device, time); + break; + default: + log_error(Unexpected event type %#x\n, e-type); + break; + } +} + +static void +tablet_destroy(struct evdev_dispatch *dispatch) +{ + struct tablet_dispatch *tablet = + (struct tablet_dispatch*)dispatch; + + free(tablet); +} + +static struct evdev_dispatch_interface tablet_interface = { + tablet_process, + tablet_destroy +}; + +static int +tablet_init(struct tablet_dispatch *tablet, + struct evdev_device *device) +{ + tablet-base.interface = tablet_interface; + tablet-device = device; + tablet-status = TABLET_NONE; + + return 0; +} + +struct evdev_dispatch * +evdev_tablet_create(struct evdev_device *device) +{ + struct tablet_dispatch *tablet; + + tablet = zalloc(sizeof *tablet); + if (!tablet) + return NULL; + + if (tablet_init(tablet, device) != 0) { + tablet_destroy(tablet-base); + return NULL; + } + + return tablet-base; +} diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h new file mode 100644 index 000..d832c17 --- /dev/null +++ b/src/evdev-tablet.h @@ -0,0 +1,64 @@ +/* + * Copyright © 2014 Red Hat, Inc. + * Copyright © 2014 Stephen Chandler Lyude Paul + * + * Permission to use, copy, modify, distribute, and sell this software and + * its documentation for any purpose is hereby granted without fee, provided + * that the above copyright notice appear in all copies and that both that + * copyright notice and this permission notice appear in supporting + * documentation, and that the name of the copyright holders not be used in + * advertising or publicity pertaining to distribution of the software + * without specific, written prior permission. The copyright holders make + * no representations about the suitability of this software for any + * purpose. It is provided as is without express or implied warranty. + * + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + + +#ifndef EVDEV_TABLET_H +#define EVDEV_TABLET_H + +#include evdev.h + +enum tablet_status { + TABLET_NONE = 0, + TABLET_AXES_UPDATED = 1 0 +}; + +struct tablet_dispatch { + struct evdev_dispatch base; + struct evdev_device *device; + unsigned char status; + unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)]; + const struct input_absinfo *absinfo[LIBINPUT_TABLET_AXIS_CNT]; + double axes[LIBINPUT_TABLET_AXIS_CNT]; +}; + +static inline enum libinput_tablet_axis +evcode_to_axis(const uint32_t evcode) +{ + enum libinput_tablet_axis axis; + + switch (evcode) { + case ABS_X: + axis = LIBINPUT_TABLET_AXIS_X; + break; + case ABS_Y: + axis = LIBINPUT_TABLET_AXIS_Y; + break; + default: + axis = LIBINPUT_TABLET_AXIS_NONE; + break; + } + + return axis; +} + +#endif diff --git a/src/evdev.c b/src