Re: [PATCH 1/8] protocol: add presentation extension v4
On Tue, 16 Sep 2014 14:16:37 -0700 Bryce Harrington br...@osg.samsung.com wrote: On Tue, Sep 16, 2014 at 10:34:29AM +0300, Pekka Paalanen wrote: On Mon, 15 Sep 2014 23:12:24 -0700 Bryce Harrington br...@osg.samsung.com wrote: Mostly just some minor grammar improvement suggestions... Appreciated. :-) On Mon, Sep 15, 2014 at 04:16:40PM -0400, Louis-Francis Ratté-Boulianne wrote: snip diff --git a/Makefile.am b/Makefile.am diff --git a/protocol/presentation_timing.xml b/protocol/presentation_timing.xml new file mode 100644 index 000..8aadff0 --- /dev/null +++ b/protocol/presentation_timing.xml @@ -0,0 +1,247 @@ +?xml version=1.0 encoding=UTF-8? +protocol name=presentation_timing +!-- wrap:70 -- + + +request name=feedback + description summary=request presentation feedback information +With this request, presentation feedback will be provided for +the current content submission of the given surface. A new +presentation_feedback object is created, and that object will +deliver the information once. The object is tied to this +content submission only. Multiple presentation_feedback +objects may be created for the same submission, and they will +all return the same information. The language here is a bit clunky. Here's my try: In response to this request, a new presentation_feedback object is created for the given surface's current content submission. The object contains a static snapshot of the information. Each request against the same submission will create a new presentation_feedback object, but they will all have the same information. Not sure that's any better! But maybe gives some wording ideas. Yeah, not sure... How about: Request presentation feedback for the current content submission on the given surface. This creates a new presentation_feedback object, which will deliver the feedback information once. If multiple presentation_feedback objects are created for the same submission, they will all deliver the same information. There we go, that sounds better. Also, it sounds like the client will need to free this object itself? Either way, it might be worth stating explicitly here. Clients will always need to free all objects themselves, that's part of the Wayland C bindings. No need to add it here, as it applies to everything. Alright, then in that case I noticed at least one other spot where the management responsibility was documented in this patch, and so by this design convention could be dropped. Did you intend to point it out? We have two different cases here though. One is when the protocol object gets destroyed; after that it is invalid to try to send events or requests on/with it. The other is when the C object is freed. These are distinct cases, and one does not exactly imply the other. If there is a destroy request marked as destructor in the protocol, then freeing the C object automatically destroys the protocol object so that both sides will know about it. This is the normal case and does not need explicit documentation, IMO. If there is no destructor protocol for destroy, freeing the C object does not exactly destroy the protocol object; it will continue to exist in the server, and the client will just ignore events coming through it. In this case, the protocol designer must have guaranteed a way for the protocol object to get destroyed in the server (e.g. the done event for wl_callback). If the protocol specifies, that an event destroys the protocol object, the protocol must also guarantee that the client cannot race and send requests on the protocol object after the server destroyed it. An obvious way to do that is to not have any requests in the interface. Also the destructor protocol existing is a clue, that getting an event won't destroy the protocol object. That's actually something we could change. Remove the destructor protocol, and specify that 'presented' and 'discarded' events destroy the protocol object. Presentation_feedback being a pure feedback interface, I cannot imagine ever adding any requests to it. The only use case for destructor protocol would be to cancel the feedback, which probably is not common, and would still be possible anyway. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Stabilizing wl_scaler protocol extension
On Tue, 16 Sep 2014 15:33:19 +0100 Steven Newbury st...@snewbury.org.uk wrote: On Tue, 2014-09-16 at 14:51 +0300, Pekka Paalanen wrote: On Tue, 16 Sep 2014 13:26:12 +0200 Alexander Preisinger preisin...@gmail.com wrote: Hi pq, I use it in my wayland-next branch (for unstable wayland stuff) of the mpv player: http://mpv.io/ In this commit: https://github.com/mpv-player/mpv/commit/77cc885b44a9e95e5c3c9ae4961b9958ff5cf643 Good to know, thanks. I only just now realized that I should just use set_destination for my use case. So setting the destination separately is definitely a use case and I think the set request is redundant. True, but I'm worried how many upset people there will be if I break the protocol during the migration by removing or renaming something. :-) There should be no-one as it's all experimental still, but... So far I really like the scaler extensions. But the scaling quality has lots of room for improvement. I thought about improving the scaling quality, but didn't had the opportunity to look into it. You mean in the Weston implementation? Yeah, that could very well be. I think fixing that would come after https://bugs.freedesktop.org/show_bug.cgi?id=83895 as it should make detecting the overall scaling factor a lot easier. However, I'm more interested in the protocol aspect right now, and there the scaling quality cannot be specified IMHO. We have to leave room for hardware overlays doing the scaling in unknown ways. Wouldn't it be useful if the protocol could have a method of presenting available scaling methods to the application so that the user could configure the preferred trade-off of performance vs quality? We cannot enumerate the available scaling methods in the protocol, not statically in the spec, and not even dynamically (e.g. as arbitrary strings) at runtime. The compositor might not have any clue what scaling method a hardware scaler uses. If we did know, and we exposed that to clients, and clients wanted to use that, the compositor could not present the client's window at all, because it is not guaranteed that the hardware scaler is always available and usable (e.g. a partially occluded window may be impossible to put on a hardware overlay). Vice versa, if the client chooses some common scaling method, the compositor just cannot use any hardware scaler, because the scaling method might not be the one. We could do it as a hint instead of a requirement, but I'm not sure there is much benefit in that, considering we do have options for the user already: The application has two choices for the user to choose between: use compositor scaling, or don't use and do scaling itself. The compositor can also have preferences, like a tick box for use dedicated hardware video scaling or alike, vs. always use OpenGL for scaling. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 2/3] litest: Add litest_assert_scroll() helper function
Make check_2fg_scroll functionality available outside of touchpad.c , no functional changes. Signed-off-by: Hans de Goede hdego...@redhat.com --- test/litest.c | 39 +++ test/litest.h | 1 + test/touchpad.c | 49 - 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/test/litest.c b/test/litest.c index cf7e692..e5092b8 100644 --- a/test/litest.c +++ b/test/litest.c @@ -1084,3 +1084,42 @@ litest_assert_button_event(struct libinput *li, unsigned int button, state); libinput_event_destroy(event); } + +void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir) +{ + struct libinput_event *event, *next_event; + struct libinput_event_pointer *ptrev; + + event = libinput_get_event(li); + next_event = libinput_get_event(li); + ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */ + + while (event) { + ck_assert_int_eq(libinput_event_get_type(event), +LIBINPUT_EVENT_POINTER_AXIS); + ptrev = libinput_event_get_pointer_event(event); + ck_assert(ptrev != NULL); + ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis); + + if (next_event) { + /* Normal scroll event, check dir */ + if (dir 0) { + ck_assert_int_ge( + libinput_event_pointer_get_axis_value(ptrev), + dir); + } else { + ck_assert_int_le( + libinput_event_pointer_get_axis_value(ptrev), + dir); + } + } else { + /* Last scroll event, must be 0 */ + ck_assert_int_eq( + libinput_event_pointer_get_axis_value(ptrev), + 0); + } + libinput_event_destroy(event); + event = next_event; + next_event = libinput_get_event(li); + } +} diff --git a/test/litest.h b/test/litest.h index dd1386c..fdf815f 100644 --- a/test/litest.h +++ b/test/litest.h @@ -147,6 +147,7 @@ void litest_assert_empty_queue(struct libinput *li); void litest_assert_button_event(struct libinput *li, unsigned int button, enum libinput_button_state state); +void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir); 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 7ff3d14..522cee6 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -1346,47 +1346,6 @@ test_2fg_scroll(struct litest_device *dev, double dx, double dy, int sleep) libinput_dispatch(li); } -static void -check_2fg_scroll(struct litest_device *dev, unsigned int axis, int dir) -{ - struct libinput *li = dev-libinput; - struct libinput_event *event, *next_event; - struct libinput_event_pointer *ptrev; - - event = libinput_get_event(li); - next_event = libinput_get_event(li); - ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */ - - while (event) { - ck_assert_int_eq(libinput_event_get_type(event), -LIBINPUT_EVENT_POINTER_AXIS); - ptrev = libinput_event_get_pointer_event(event); - ck_assert(ptrev != NULL); - ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis); - - if (next_event) { - /* Normal scroll event, check dir */ - if (dir 0) { - ck_assert_int_ge( - libinput_event_pointer_get_axis_value(ptrev), - dir); - } else { - ck_assert_int_le( - libinput_event_pointer_get_axis_value(ptrev), - dir); - } - } else { - /* Last scroll event, must be 0 */ - ck_assert_int_eq( - libinput_event_pointer_get_axis_value(ptrev), - 0); - } - libinput_event_destroy(event); - event = next_event; - next_event = libinput_get_event(li); - } -} - START_TEST(touchpad_2fg_scroll) { struct litest_device *dev = litest_current_device(); @@ -1395,13 +1354,13 @@ START_TEST(touchpad_2fg_scroll)
[PATCH libinput 1/3] touchpad: Enlarge topbutton area a bit while the touchpad is disabled
Make it easier to hit the topbutton area when the touchpad is disabled, normally we don't want to make the topbutton area too big, so as to not interfere with normal touchpad operation, but when disabled we have no such worries. Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-buttons.c | 19 --- src/evdev-mt-touchpad.c | 4 src/evdev-mt-touchpad.h | 4 +++- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 865346b..f0800d7 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -496,7 +496,8 @@ tp_release_all_buttons(struct tp_dispatch *tp, void tp_init_softbuttons(struct tp_dispatch *tp, - struct evdev_device *device) + struct evdev_device *device, + double topbutton_size_mult) { int width, height; const struct input_absinfo *absinfo_x, *absinfo_y; @@ -524,14 +525,18 @@ tp_init_softbuttons(struct tp_dispatch *tp, tp-buttons.bottom_area.rightbutton_left_edge = width/2 + xoffset; if (tp-buttons.has_topbuttons) { - /* T440s has the top button line 5mm from the top, - event analysis has shown events to start down to ~10mm - from the top - which maps to 15% */ + /* T440s has the top button line 5mm from the top, event + analysis has shown events to start down to ~10mm from the + top - which maps to 15%. We allow the caller to enlarge the + area using a multiplier for the touchpad disabled case. */ + double topsize_mm = 10 * topbutton_size_mult; + double topsize_pct = .15 * topbutton_size_mult; + if (yres 1) { tp-buttons.top_area.bottom_edge = - yoffset + 10 * yres; + yoffset + topsize_mm * yres; } else { - tp-buttons.top_area.bottom_edge = height * .15 + yoffset; + tp-buttons.top_area.bottom_edge = height * topsize_pct + yoffset; } tp-buttons.top_area.rightbutton_left_edge = width * .58 + xoffset; tp-buttons.top_area.leftbutton_right_edge = width * .42 + xoffset; @@ -581,7 +586,7 @@ tp_init_buttons(struct tp_dispatch *tp, tp-buttons.use_clickfinger = true; if (tp-buttons.is_clickpad !tp-buttons.use_clickfinger) { - tp_init_softbuttons(tp, device); + tp_init_softbuttons(tp, device, 1.0); } else { tp-buttons.bottom_area.top_edge = INT_MAX; tp-buttons.top_area.bottom_edge = INT_MIN; diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 9e568ad..1c32cc6 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -645,6 +645,8 @@ tp_suspend(struct tp_dispatch *tp, struct evdev_device *device) */ if (tp-buttons.has_topbuttons) { evdev_notify_suspended_device(device); + /* Enlarge topbutton area while suspended */ + tp_init_softbuttons(tp, device, 1.5); } else { evdev_device_suspend(device); } @@ -656,6 +658,8 @@ tp_resume(struct tp_dispatch *tp, struct evdev_device *device) if (tp-buttons.has_topbuttons) { /* tap state-machine is offline while suspended, reset state */ tp_clear_state(tp, device); + /* restore original topbutton area size */ + tp_init_softbuttons(tp, device, 1.0); evdev_notify_resumed_device(device); } else { evdev_device_resume(device); diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 15cca76..e0c8c47 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -245,7 +245,9 @@ int tp_init_buttons(struct tp_dispatch *tp, struct evdev_device *device); void -tp_init_softbuttons(struct tp_dispatch *tp, struct evdev_device *device); +tp_init_softbuttons(struct tp_dispatch *tp, + struct evdev_device *device, + double topbutton_size_mult); void tp_destroy_buttons(struct tp_dispatch *tp); -- 2.1.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 0/3] Pointer locking revisited
Hi, This short series introduces a pointer lock interface based on the previous proposal[0] by Kristian Høgsberg from last year. The first patch adds a protocol more or less equal to Kristians proposal, but moved out of wl_seat into its own global object (currently _wl_pointer_lock), with some concerns raised in the original thread addressed. The implementation in weston is new. There was no point trying to rebase the previous implementation due to the architectural changes done since that patch was written. The second patch introduces a client side API in toytoolkit, and the forth makes use of that API in resizor, in the same way as in the previous proposal, to resize its window. Note that the reason for having the underscore prefix is to have the possibility to push a stable variant to the wayland repo without breaking any clients implementing the experimental version. The intention is that _wl_pointer_lock ever would be considered an official interface, it would be renamed to wl_pointer_lock. In short, the interface works by having the client create a separate wl_pointer object, using wl_pointer_lock.lock_pointer, that gets focus until wl_pointer.release is requested and the surface gets deactivated. If the locked pointer object is still not released when a surface is activated again, the pointer will be implicitly locked to the special wl_pointer object again. Limitations with this approach include that it would, as far as I know, not be possible to fully implement pointer warping support in Xwayland only using this protocol. However, not sure if support for legacy concepts is within the scope of an interface like this. Maybe Xwayland needs some private way to communicate (privileged wl_xwayland interface?) for these type of things? Maybe it could be emulated by locking if warping is done with some heuristic for releasing the lock, but have not tried to implement that. We wouldn't be able to implement API of certain toolkits (for example SDL, GLFW). This I suppose is not really a big deal as it wouldn't be the first (global positions etc) and the use case is most likely to lock the pointer and get relative motion events, and there is API available in both those toolkits for that. Pointer confinement (absolute motion events, but not letting the pointer leave the surface) is another functionality that wouldn't be possible. It could be emulated by having the client lock the pointer and draw the pointer itself given the relative motion events, or simply added to this protocol in some way (wl_pointer_lock.confine_pointer?). In the previous thread, surface transformations came up as something that needed to be thought through. In the updated protocol of this series, I clarified that relative motion events are as if the surface has not been, transformed. The reasoning behind this is more or less how typical use case looks like: 1:st person 3D games. I would expect that the viewport moves the same even if the window happen to have been transformed for example zoomed or rotated. Following the trend of keeping track of these kind of stuff, I filed[1] a bug on BZ as well. Thoughts? Is this the right way to go? Jonas [0] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007635.html [1] https://bugs.freedesktop.org/show_bug.cgi?id=84014 Jonas Ådahl (3): Introduce pointer lock interface clients: Introduce pointer lock API to toytoolkit resizor: Make resizor also use pointer locks for resizing Makefile.am | 11 +- clients/resizor.c | 62 +++ clients/window.c | 177 + clients/window.h | 42 +++ desktop-shell/exposay.c | 2 +- desktop-shell/shell.c | 10 +- protocol/pointer-lock.xml | 85 ++ src/compositor.c | 4 + src/compositor.h | 21 +++- src/input.c | 277 +- 10 files changed, 678 insertions(+), 13 deletions(-) create mode 100644 protocol/pointer-lock.xml -- 1.8.5.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 1/3] Introduce pointer lock interface
Introduce a pointer lock interface and implementation. The interface consists of a global currently called _wl_pointer_lock. It is prefixed with an underscore (_) in order to not conflict with a potential official protocol in Wayland, and if moving it there, the prefixed should be removed. The protocol works by exposing the global _wl_pointer_lock to the client, and when the client wants to lock the pointer, it creates a new wl_pointer object using the 'lock_pointer' request. A more detailed description of the protocol seen in pointer-lock.xml file. The interface is based on the W3C pointer lock interface [0]. [0] http://www.w3.org/TR/pointerlock/ Signed-off-by: Jonas Ådahl jad...@gmail.com --- The part of the implementation I'm not very happy with is the internal API change: the extra argument in the pointer lock motion callback used to determine if the motion event was a result of a relative motion or an absolute motion. In this implementation absolute events are simply ignored as they don't tend to behave in any sensible way. One could generate relative motion events from absolute ones as done from a touchpad, but one would need some kind of threshold as there are no touch down or touch up events. With backends where relative and absolute events are indistinguishable (X11, Wayland) is tricker, as there is no way to know whether the event is suitable or not. One could also just let the client deal with it making detached absolute motions just result in very long vectors. Jonas Makefile.am | 7 +- desktop-shell/exposay.c | 2 +- desktop-shell/shell.c | 10 +- protocol/pointer-lock.xml | 85 ++ src/compositor.c | 4 + src/compositor.h | 21 +++- src/input.c | 277 +- 7 files changed, 394 insertions(+), 12 deletions(-) create mode 100644 protocol/pointer-lock.xml diff --git a/Makefile.am b/Makefile.am index b2d6893..ed74983 100644 --- a/Makefile.am +++ b/Makefile.am @@ -79,7 +79,9 @@ nodist_weston_SOURCES = \ protocol/workspaces-protocol.c \ protocol/workspaces-server-protocol.h \ protocol/scaler-protocol.c \ - protocol/scaler-server-protocol.h + protocol/scaler-server-protocol.h \ + protocol/pointer-lock-protocol.c\ + protocol/pointer-lock-server-protocol.h BUILT_SOURCES += $(nodist_weston_SOURCES) @@ -987,7 +989,8 @@ EXTRA_DIST += \ protocol/wayland-test.xml \ protocol/xdg-shell.xml \ protocol/fullscreen-shell.xml \ - protocol/scaler.xml + protocol/scaler.xml \ + protocol/pointer-lock.xml man_MANS = weston.1 weston.ini.5 diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c index 4b65cbd..d6ec6a1 100644 --- a/desktop-shell/exposay.c +++ b/desktop-shell/exposay.c @@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab) static void exposay_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { struct desktop_shell *shell = container_of(grab, struct desktop_shell, exposay.grab_ptr); diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 3a5a702..35b05d0 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -1651,7 +1651,7 @@ constrain_position(struct weston_move_grab *move, int *cx, int *cy) static void move_grab_motion(struct weston_pointer_grab *grab, uint32_t time, -wl_fixed_t x, wl_fixed_t y) +wl_fixed_t x, wl_fixed_t y, int rel) { struct weston_move_grab *move = (struct weston_move_grab *) grab; struct weston_pointer *pointer = grab-pointer; @@ -1772,7 +1772,7 @@ struct weston_resize_grab { static void resize_grab_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { struct weston_resize_grab *resize = (struct weston_resize_grab *) grab; struct weston_pointer *pointer = grab-pointer; @@ -1981,7 +1981,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab *base) static void busy_cursor_grab_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { weston_pointer_move(grab-pointer, x, y); } @@ -3063,7 +3063,7 @@ popup_grab_focus(struct weston_pointer_grab *grab) static void popup_grab_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { struct weston_pointer *pointer = grab-pointer;
[PATCH weston 2/3] clients: Introduce pointer lock API to toytoolkit
Clients can lock the pointer surface wide, and receive relative pointer events while the pointer is locked. Signed-off-by: Jonas Ådahl jad...@gmail.com --- Makefile.am | 4 +- clients/window.c | 177 +++ clients/window.h | 42 + 3 files changed, 222 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index ed74983..cf4bf60 100644 --- a/Makefile.am +++ b/Makefile.am @@ -448,7 +448,9 @@ nodist_libtoytoolkit_la_SOURCES = \ protocol/workspaces-protocol.c \ protocol/workspaces-client-protocol.h \ protocol/xdg-shell-protocol.c \ - protocol/xdg-shell-client-protocol.h + protocol/xdg-shell-client-protocol.h\ + protocol/pointer-lock-protocol.c\ + protocol/pointer-lock-client-protocol.h BUILT_SOURCES += $(nodist_libtoytoolkit_la_SOURCES) diff --git a/clients/window.c b/clients/window.c index e44d65c..d67039b 100644 --- a/clients/window.c +++ b/clients/window.c @@ -68,6 +68,7 @@ typedef void *EGLContext; #include xdg-shell-client-protocol.h #include text-cursor-position-client-protocol.h #include workspaces-client-protocol.h +#include pointer-lock-client-protocol.h #include ../shared/os-compatibility.h #include window.h @@ -91,6 +92,7 @@ struct display { struct text_cursor_position *text_cursor_position; struct workspace_manager *workspace_manager; struct xdg_shell *xdg_shell; + struct _wl_pointer_lock *pointer_lock; EGLDisplay dpy; EGLConfig argb_config; EGLContext argb_ctx; @@ -242,6 +244,12 @@ struct window { window_output_handler_t output_handler; window_state_changed_handler_t state_changed_handler; + pointer_lock_enter_handler_t pointer_lock_enter_handler; + pointer_lock_leave_handler_t pointer_lock_leave_handler; + pointer_lock_motion_handler_t pointer_lock_motion_handler; + pointer_lock_button_handler_t pointer_lock_button_handler; + pointer_lock_axis_handler_t pointer_lock_axis_handler; + struct surface *main_surface; struct xdg_surface *xdg_surface; struct xdg_popup *xdg_popup; @@ -254,6 +262,8 @@ struct window { /* struct surface::link, contains also main_surface */ struct wl_list subsurface_list; + struct wl_pointer *locked_pointer; + void *user_data; struct wl_list link; }; @@ -4384,6 +4394,169 @@ window_damage(struct window *window, int32_t x, int32_t y, wl_surface_damage(window-main_surface-surface, x, y, width, height); } +void +window_set_pointer_lock_enter_handler(struct window *window, + pointer_lock_enter_handler_t handler) +{ + window-pointer_lock_enter_handler = handler; +} + +void +window_set_pointer_lock_leave_handler(struct window *window, + pointer_lock_leave_handler_t handler) +{ + window-pointer_lock_leave_handler = handler; +} + +void +window_set_pointer_lock_motion_handler(struct window *window, + pointer_lock_motion_handler_t handler) +{ + window-pointer_lock_motion_handler = handler; +} + +void +window_set_pointer_lock_button_handler(struct window *window, + pointer_lock_button_handler_t handler) +{ + window-pointer_lock_button_handler = handler; +} + +void +window_set_pointer_lock_axis_handler(struct window *window, +pointer_lock_axis_handler_t handler) +{ + window-pointer_lock_axis_handler = handler; +} + +static void +locked_pointer_handle_enter(void *data, struct wl_pointer *pointer, + uint32_t serial, struct wl_surface *surface, + wl_fixed_t dx, wl_fixed_t dy) +{ + struct input *input = data; + struct window *window; + + window = wl_surface_get_user_data(surface); + if (window-pointer_lock_enter_handler) { + window-pointer_lock_enter_handler(window, input, + wl_fixed_to_double(dx), + wl_fixed_to_double(dy), + window-user_data); + } + + input-display-serial = serial; + input-pointer_enter_serial = serial; + input-pointer_focus = window; +} + +static void +locked_pointer_handle_leave(void *data, struct wl_pointer *pointer, + uint32_t serial, struct wl_surface *surface) +{ + struct input *input = data; + struct window *window; + + window = wl_surface_get_user_data(surface); + if (window-pointer_lock_leave_handler) { + window-pointer_lock_leave_handler(window, input, + window-user_data); +
[PATCH weston 3/3] resizor: Make resizor also use pointer locks for resizing
Signed-off-by: Jonas Ådahl jad...@gmail.com --- clients/resizor.c | 62 +++ 1 file changed, 62 insertions(+) diff --git a/clients/resizor.c b/clients/resizor.c index 19c6eeb..66e1c2f 100644 --- a/clients/resizor.c +++ b/clients/resizor.c @@ -219,13 +219,70 @@ show_menu(struct resizor *resizor, struct input *input, uint32_t time) } static void +pointer_lock_handle_motion(struct window *window, + struct input *input, + uint32_t time, + float dx, + float dy, + void *data) +{ + struct resizor *resizor = data; + + resizor-width.current += dx; + resizor-width.previous = resizor-width.current; + resizor-width.target = resizor-width.current; + + resizor-height.current += dy; + resizor-height.previous = resizor-height.current; + resizor-height.target = resizor-height.current; + + widget_schedule_resize(resizor-widget, + resizor-width.current, + resizor-height.current); +} + +static void +pointer_lock_handle_button(struct window *window, + struct input *input, + uint32_t time, + uint32_t button, + uint32_t state, + void *data) +{ + struct resizor *resizor = data; + + if (state != WL_POINTER_BUTTON_STATE_PRESSED) + return; + + window_unlock_pointer(resizor-window); +} + +static void button_handler(struct widget *widget, struct input *input, uint32_t time, uint32_t button, enum wl_pointer_button_state state, void *data) { struct resizor *resizor = data; + struct rectangle allocation; switch (button) { + case BTN_LEFT: + if (state != WL_POINTER_BUTTON_STATE_PRESSED) + break; + + window_get_allocation(resizor-window, allocation); + + resizor-width.current = allocation.width; + resizor-width.previous = allocation.width; + resizor-width.target = allocation.width; + + resizor-height.current = allocation.height; + resizor-height.previous = allocation.height; + resizor-height.target = allocation.height; + + window_lock_pointer(resizor-window, input); + break; + case BTN_RIGHT: if (state == WL_POINTER_BUTTON_STATE_PRESSED) show_menu(resizor, input, time); @@ -250,6 +307,11 @@ resizor_create(struct display *display) window_set_keyboard_focus_handler(resizor-window, keyboard_focus_handler); + window_set_pointer_lock_motion_handler(resizor-window, + pointer_lock_handle_motion); + window_set_pointer_lock_button_handler(resizor-window, + pointer_lock_handle_button); + widget_set_button_handler(resizor-widget, button_handler); resizor-height.previous = 400; -- 1.8.5.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 0/3] Pointer locking revisited
On 17/09/2014 21:39, Jonas Ådahl wrote : Hi, This short series introduces a pointer lock interface based on the previous proposal[0] by Kristian Høgsberg from last year. The first patch adds a protocol more or less equal to Kristians proposal, but moved out of wl_seat into its own global object (currently _wl_pointer_lock), with some concerns raised in the original thread addressed. The implementation in weston is new. There was no point trying to rebase the previous implementation due to the architectural changes done since that patch was written. The second patch introduces a client side API in toytoolkit, and the forth makes use of that API in resizor, in the same way as in the previous proposal, to resize its window. Note that the reason for having the underscore prefix is to have the possibility to push a stable variant to the wayland repo without breaking any clients implementing the experimental version. The intention is that _wl_pointer_lock ever would be considered an official interface, it would be renamed to wl_pointer_lock. In short, the interface works by having the client create a separate wl_pointer object, using wl_pointer_lock.lock_pointer, that gets focus until wl_pointer.release is requested and the surface gets deactivated. If the locked pointer object is still not released when a surface is activated again, the pointer will be implicitly locked to the special wl_pointer object again. Limitations with this approach include that it would, as far as I know, not be possible to fully implement pointer warping support in Xwayland only using this protocol. However, not sure if support for legacy concepts is within the scope of an interface like this. Maybe Xwayland needs some private way to communicate (privileged wl_xwayland interface?) for these type of things? Maybe it could be emulated by locking if warping is done with some heuristic for releasing the lock, but have not tried to implement that. We wouldn't be able to implement API of certain toolkits (for example SDL, GLFW). This I suppose is not really a big deal as it wouldn't be the first (global positions etc) and the use case is most likely to lock the pointer and get relative motion events, and there is API available in both those toolkits for that. Pointer confinement (absolute motion events, but not letting the pointer leave the surface) is another functionality that wouldn't be possible. It could be emulated by having the client lock the pointer and draw the pointer itself given the relative motion events, or simply added to this protocol in some way (wl_pointer_lock.confine_pointer?). In the previous thread, surface transformations came up as something that needed to be thought through. In the updated protocol of this series, I clarified that relative motion events are as if the surface has not been, transformed. The reasoning behind this is more or less how typical use case looks like: 1:st person 3D games. I would expect that the viewport moves the same even if the window happen to have been transformed for example zoomed or rotated. Following the trend of keeping track of these kind of stuff, I filed[1] a bug on BZ as well. Thoughts? Is this the right way to go? Jonas [0] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007635.html [1] https://bugs.freedesktop.org/show_bug.cgi?id=84014 Jonas Ådahl (3): Introduce pointer lock interface clients: Introduce pointer lock API to toytoolkit resizor: Make resizor also use pointer locks for resizing Makefile.am | 11 +- clients/resizor.c | 62 +++ clients/window.c | 177 + clients/window.h | 42 +++ desktop-shell/exposay.c | 2 +- desktop-shell/shell.c | 10 +- protocol/pointer-lock.xml | 85 ++ src/compositor.c | 4 + src/compositor.h | 21 +++- src/input.c | 277 +- 10 files changed, 678 insertions(+), 13 deletions(-) create mode 100644 protocol/pointer-lock.xml Thanks for getting into this. Could you clarify where the cursor should be during the lock, and what should be the cursor position when a lock is ended ? Axel Davy ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] Introduce pointer lock interface
I haven't looked at the implementation yet, just at the protocol, but isn't _wl_pointer_lock.lock_pointer() returning a new wl_pointer a problem? Objects should have a unique factory interface, or else the version of the interface can't be determined. -- Giulio 2014-09-17 22:39 GMT+03:00 Jonas Ådahl jad...@gmail.com: Introduce a pointer lock interface and implementation. The interface consists of a global currently called _wl_pointer_lock. It is prefixed with an underscore (_) in order to not conflict with a potential official protocol in Wayland, and if moving it there, the prefixed should be removed. The protocol works by exposing the global _wl_pointer_lock to the client, and when the client wants to lock the pointer, it creates a new wl_pointer object using the 'lock_pointer' request. A more detailed description of the protocol seen in pointer-lock.xml file. The interface is based on the W3C pointer lock interface [0]. [0] http://www.w3.org/TR/pointerlock/ Signed-off-by: Jonas Ådahl jad...@gmail.com --- The part of the implementation I'm not very happy with is the internal API change: the extra argument in the pointer lock motion callback used to determine if the motion event was a result of a relative motion or an absolute motion. In this implementation absolute events are simply ignored as they don't tend to behave in any sensible way. One could generate relative motion events from absolute ones as done from a touchpad, but one would need some kind of threshold as there are no touch down or touch up events. With backends where relative and absolute events are indistinguishable (X11, Wayland) is tricker, as there is no way to know whether the event is suitable or not. One could also just let the client deal with it making detached absolute motions just result in very long vectors. Jonas Makefile.am | 7 +- desktop-shell/exposay.c | 2 +- desktop-shell/shell.c | 10 +- protocol/pointer-lock.xml | 85 ++ src/compositor.c | 4 + src/compositor.h | 21 +++- src/input.c | 277 +- 7 files changed, 394 insertions(+), 12 deletions(-) create mode 100644 protocol/pointer-lock.xml diff --git a/Makefile.am b/Makefile.am index b2d6893..ed74983 100644 --- a/Makefile.am +++ b/Makefile.am @@ -79,7 +79,9 @@ nodist_weston_SOURCES = \ protocol/workspaces-protocol.c \ protocol/workspaces-server-protocol.h \ protocol/scaler-protocol.c \ - protocol/scaler-server-protocol.h + protocol/scaler-server-protocol.h \ + protocol/pointer-lock-protocol.c\ + protocol/pointer-lock-server-protocol.h BUILT_SOURCES += $(nodist_weston_SOURCES) @@ -987,7 +989,8 @@ EXTRA_DIST += \ protocol/wayland-test.xml \ protocol/xdg-shell.xml \ protocol/fullscreen-shell.xml \ - protocol/scaler.xml + protocol/scaler.xml \ + protocol/pointer-lock.xml man_MANS = weston.1 weston.ini.5 diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c index 4b65cbd..d6ec6a1 100644 --- a/desktop-shell/exposay.c +++ b/desktop-shell/exposay.c @@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab) static void exposay_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { struct desktop_shell *shell = container_of(grab, struct desktop_shell, exposay.grab_ptr); diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 3a5a702..35b05d0 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -1651,7 +1651,7 @@ constrain_position(struct weston_move_grab *move, int *cx, int *cy) static void move_grab_motion(struct weston_pointer_grab *grab, uint32_t time, -wl_fixed_t x, wl_fixed_t y) +wl_fixed_t x, wl_fixed_t y, int rel) { struct weston_move_grab *move = (struct weston_move_grab *) grab; struct weston_pointer *pointer = grab-pointer; @@ -1772,7 +1772,7 @@ struct weston_resize_grab { static void resize_grab_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { struct weston_resize_grab *resize = (struct weston_resize_grab *) grab; struct weston_pointer *pointer = grab-pointer; @@ -1981,7 +1981,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab *base) static void busy_cursor_grab_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) +
Re: [PATCH weston 1/3] Introduce pointer lock interface
On Wed, Sep 17, 2014 at 10:56:13PM +0300, Giulio Camuffo wrote: I haven't looked at the implementation yet, just at the protocol, but isn't _wl_pointer_lock.lock_pointer() returning a new wl_pointer a problem? Objects should have a unique factory interface, or else the version of the interface can't be determined. Is it really? In the implementation below, the wl_pointer object gets the same version as the wl_pointer object that is locked. It is also specified in the last paragraph of _wl_pointer_lock.lock_pointer. Also, this would not be the first interface that creates objects that other interface do as well. For example wl_shm_pool.create_buffer and wl_drm.create_buffer both create wl_buffer's. Jonas -- Giulio 2014-09-17 22:39 GMT+03:00 Jonas Ådahl jad...@gmail.com: Introduce a pointer lock interface and implementation. The interface consists of a global currently called _wl_pointer_lock. It is prefixed with an underscore (_) in order to not conflict with a potential official protocol in Wayland, and if moving it there, the prefixed should be removed. The protocol works by exposing the global _wl_pointer_lock to the client, and when the client wants to lock the pointer, it creates a new wl_pointer object using the 'lock_pointer' request. A more detailed description of the protocol seen in pointer-lock.xml file. The interface is based on the W3C pointer lock interface [0]. [0] http://www.w3.org/TR/pointerlock/ Signed-off-by: Jonas Ådahl jad...@gmail.com --- The part of the implementation I'm not very happy with is the internal API change: the extra argument in the pointer lock motion callback used to determine if the motion event was a result of a relative motion or an absolute motion. In this implementation absolute events are simply ignored as they don't tend to behave in any sensible way. One could generate relative motion events from absolute ones as done from a touchpad, but one would need some kind of threshold as there are no touch down or touch up events. With backends where relative and absolute events are indistinguishable (X11, Wayland) is tricker, as there is no way to know whether the event is suitable or not. One could also just let the client deal with it making detached absolute motions just result in very long vectors. Jonas Makefile.am | 7 +- desktop-shell/exposay.c | 2 +- desktop-shell/shell.c | 10 +- protocol/pointer-lock.xml | 85 ++ src/compositor.c | 4 + src/compositor.h | 21 +++- src/input.c | 277 +- 7 files changed, 394 insertions(+), 12 deletions(-) create mode 100644 protocol/pointer-lock.xml diff --git a/Makefile.am b/Makefile.am index b2d6893..ed74983 100644 --- a/Makefile.am +++ b/Makefile.am @@ -79,7 +79,9 @@ nodist_weston_SOURCES = \ protocol/workspaces-protocol.c \ protocol/workspaces-server-protocol.h \ protocol/scaler-protocol.c \ - protocol/scaler-server-protocol.h + protocol/scaler-server-protocol.h \ + protocol/pointer-lock-protocol.c\ + protocol/pointer-lock-server-protocol.h BUILT_SOURCES += $(nodist_weston_SOURCES) @@ -987,7 +989,8 @@ EXTRA_DIST += \ protocol/wayland-test.xml \ protocol/xdg-shell.xml \ protocol/fullscreen-shell.xml \ - protocol/scaler.xml + protocol/scaler.xml \ + protocol/pointer-lock.xml man_MANS = weston.1 weston.ini.5 diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c index 4b65cbd..d6ec6a1 100644 --- a/desktop-shell/exposay.c +++ b/desktop-shell/exposay.c @@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab) static void exposay_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { struct desktop_shell *shell = container_of(grab, struct desktop_shell, exposay.grab_ptr); diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index 3a5a702..35b05d0 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -1651,7 +1651,7 @@ constrain_position(struct weston_move_grab *move, int *cx, int *cy) static void move_grab_motion(struct weston_pointer_grab *grab, uint32_t time, -wl_fixed_t x, wl_fixed_t y) +wl_fixed_t x, wl_fixed_t y, int rel) { struct weston_move_grab *move = (struct weston_move_grab *) grab; struct weston_pointer *pointer = grab-pointer; @@ -1772,7 +1772,7 @@ struct
Re: [PATCH weston 0/3] Pointer locking revisited
On Wed, Sep 17, 2014 at 09:55:36PM +0200, Axel Davy wrote: On 17/09/2014 21:39, Jonas Ådahl wrote : Hi, This short series introduces a pointer lock interface based on the previous proposal[0] by Kristian Høgsberg from last year. The first patch adds a protocol more or less equal to Kristians proposal, but moved out of wl_seat into its own global object (currently _wl_pointer_lock), with some concerns raised in the original thread addressed. The implementation in weston is new. There was no point trying to rebase the previous implementation due to the architectural changes done since that patch was written. The second patch introduces a client side API in toytoolkit, and the forth makes use of that API in resizor, in the same way as in the previous proposal, to resize its window. Note that the reason for having the underscore prefix is to have the possibility to push a stable variant to the wayland repo without breaking any clients implementing the experimental version. The intention is that _wl_pointer_lock ever would be considered an official interface, it would be renamed to wl_pointer_lock. In short, the interface works by having the client create a separate wl_pointer object, using wl_pointer_lock.lock_pointer, that gets focus until wl_pointer.release is requested and the surface gets deactivated. If the locked pointer object is still not released when a surface is activated again, the pointer will be implicitly locked to the special wl_pointer object again. Limitations with this approach include that it would, as far as I know, not be possible to fully implement pointer warping support in Xwayland only using this protocol. However, not sure if support for legacy concepts is within the scope of an interface like this. Maybe Xwayland needs some private way to communicate (privileged wl_xwayland interface?) for these type of things? Maybe it could be emulated by locking if warping is done with some heuristic for releasing the lock, but have not tried to implement that. We wouldn't be able to implement API of certain toolkits (for example SDL, GLFW). This I suppose is not really a big deal as it wouldn't be the first (global positions etc) and the use case is most likely to lock the pointer and get relative motion events, and there is API available in both those toolkits for that. Pointer confinement (absolute motion events, but not letting the pointer leave the surface) is another functionality that wouldn't be possible. It could be emulated by having the client lock the pointer and draw the pointer itself given the relative motion events, or simply added to this protocol in some way (wl_pointer_lock.confine_pointer?). In the previous thread, surface transformations came up as something that needed to be thought through. In the updated protocol of this series, I clarified that relative motion events are as if the surface has not been, transformed. The reasoning behind this is more or less how typical use case looks like: 1:st person 3D games. I would expect that the viewport moves the same even if the window happen to have been transformed for example zoomed or rotated. Following the trend of keeping track of these kind of stuff, I filed[1] a bug on BZ as well. Thoughts? Is this the right way to go? Jonas [0] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007635.html [1] https://bugs.freedesktop.org/show_bug.cgi?id=84014 Jonas Ådahl (3): Introduce pointer lock interface clients: Introduce pointer lock API to toytoolkit resizor: Make resizor also use pointer locks for resizing Makefile.am | 11 +- clients/resizor.c | 62 +++ clients/window.c | 177 + clients/window.h | 42 +++ desktop-shell/exposay.c | 2 +- desktop-shell/shell.c | 10 +- protocol/pointer-lock.xml | 85 ++ src/compositor.c | 4 + src/compositor.h | 21 +++- src/input.c | 277 +- 10 files changed, 678 insertions(+), 13 deletions(-) create mode 100644 protocol/pointer-lock.xml Thanks for getting into this. Could you clarify where the cursor should be during the lock, and what should be the cursor position when a lock is ended ? Sure. Can add a clause saying that the the position of the actual pointer will stay where it was before the lock was initiated (be it by lock_pointer or surface activation). Regarding the cursor, hiding it during it being locked probably makes most sense. Jonas Axel Davy ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] Introduce pointer lock interface
On Wed, Sep 17, 2014 at 11:16:06PM +0300, Giulio Camuffo wrote: 2014-09-17 23:11 GMT+03:00 Jonas Ådahl jad...@gmail.com: On Wed, Sep 17, 2014 at 10:56:13PM +0300, Giulio Camuffo wrote: I haven't looked at the implementation yet, just at the protocol, but isn't _wl_pointer_lock.lock_pointer() returning a new wl_pointer a problem? Objects should have a unique factory interface, or else the version of the interface can't be determined. Is it really? In the implementation below, the wl_pointer object gets the same version as the wl_pointer object that is locked. It is also specified in the last paragraph of _wl_pointer_lock.lock_pointer. Mmh, then maybe it is fine. I'm not convinced actually, but I'm too tired now. :) Also, this would not be the first interface that creates objects that other interface do as well. For example wl_shm_pool.create_buffer and wl_drm.create_buffer both create wl_buffer's. Yes, indeed wl_buffer and wl_callback version can't be increased, afaik. Then, another example, (of a non-final protoc though): wl_input_method_context.grab_keyboard vs wl_seat.get_keyboard, both creating wl_keyboard objects. Jonas -- Giulio Jonas -- Giulio 2014-09-17 22:39 GMT+03:00 Jonas Ådahl jad...@gmail.com: Introduce a pointer lock interface and implementation. The interface consists of a global currently called _wl_pointer_lock. It is prefixed with an underscore (_) in order to not conflict with a potential official protocol in Wayland, and if moving it there, the prefixed should be removed. The protocol works by exposing the global _wl_pointer_lock to the client, and when the client wants to lock the pointer, it creates a new wl_pointer object using the 'lock_pointer' request. A more detailed description of the protocol seen in pointer-lock.xml file. The interface is based on the W3C pointer lock interface [0]. [0] http://www.w3.org/TR/pointerlock/ Signed-off-by: Jonas Ådahl jad...@gmail.com --- The part of the implementation I'm not very happy with is the internal API change: the extra argument in the pointer lock motion callback used to determine if the motion event was a result of a relative motion or an absolute motion. In this implementation absolute events are simply ignored as they don't tend to behave in any sensible way. One could generate relative motion events from absolute ones as done from a touchpad, but one would need some kind of threshold as there are no touch down or touch up events. With backends where relative and absolute events are indistinguishable (X11, Wayland) is tricker, as there is no way to know whether the event is suitable or not. One could also just let the client deal with it making detached absolute motions just result in very long vectors. Jonas Makefile.am | 7 +- desktop-shell/exposay.c | 2 +- desktop-shell/shell.c | 10 +- protocol/pointer-lock.xml | 85 ++ src/compositor.c | 4 + src/compositor.h | 21 +++- src/input.c | 277 +- 7 files changed, 394 insertions(+), 12 deletions(-) create mode 100644 protocol/pointer-lock.xml diff --git a/Makefile.am b/Makefile.am index b2d6893..ed74983 100644 --- a/Makefile.am +++ b/Makefile.am @@ -79,7 +79,9 @@ nodist_weston_SOURCES = \ protocol/workspaces-protocol.c \ protocol/workspaces-server-protocol.h \ protocol/scaler-protocol.c \ - protocol/scaler-server-protocol.h + protocol/scaler-server-protocol.h \ + protocol/pointer-lock-protocol.c\ + protocol/pointer-lock-server-protocol.h BUILT_SOURCES += $(nodist_weston_SOURCES) @@ -987,7 +989,8 @@ EXTRA_DIST += \ protocol/wayland-test.xml \ protocol/xdg-shell.xml \ protocol/fullscreen-shell.xml \ - protocol/scaler.xml + protocol/scaler.xml \ + protocol/pointer-lock.xml man_MANS = weston.1 weston.ini.5 diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c index 4b65cbd..d6ec6a1 100644 --- a/desktop-shell/exposay.c +++ b/desktop-shell/exposay.c @@ -340,7 +340,7 @@ exposay_focus(struct weston_pointer_grab *grab) static void exposay_motion(struct weston_pointer_grab *grab, uint32_t time, - wl_fixed_t x, wl_fixed_t y) + wl_fixed_t x, wl_fixed_t y, int rel) { struct desktop_shell *shell = container_of(grab, struct desktop_shell, exposay.grab_ptr); diff --git
Re: [PATCH libinput 1/3] touchpad: Enlarge topbutton area a bit while the touchpad is disabled
On Wed, Sep 17, 2014 at 03:35:30PM +0200, Hans de Goede wrote: Make it easier to hit the topbutton area when the touchpad is disabled, normally we don't want to make the topbutton area too big, so as to not interfere with normal touchpad operation, but when disabled we have no such worries. even though it may not seem like the scalable solution, I'd really prefer this to be two functions, expand and normal or so. We don't need a generic multiplier here (we don't plan to add more than 2 sizes) and something like tp_expand_softbuttons(tp, device) or tp_set_softbutton_size_large(tp, device) is easier to comprehend IMO than adding the raw numbers as multipliers of some magic size. Plus, we may end up having some magic number being the best size that's not a straightforward multiplier in which case the multiplier just adds to the confusion. So let's just use two sizes, normal and large and go with those. Cheers, Peter Signed-off-by: Hans de Goede hdego...@redhat.com --- src/evdev-mt-touchpad-buttons.c | 19 --- src/evdev-mt-touchpad.c | 4 src/evdev-mt-touchpad.h | 4 +++- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 865346b..f0800d7 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -496,7 +496,8 @@ tp_release_all_buttons(struct tp_dispatch *tp, void tp_init_softbuttons(struct tp_dispatch *tp, - struct evdev_device *device) + struct evdev_device *device, + double topbutton_size_mult) { int width, height; const struct input_absinfo *absinfo_x, *absinfo_y; @@ -524,14 +525,18 @@ tp_init_softbuttons(struct tp_dispatch *tp, tp-buttons.bottom_area.rightbutton_left_edge = width/2 + xoffset; if (tp-buttons.has_topbuttons) { - /* T440s has the top button line 5mm from the top, -event analysis has shown events to start down to ~10mm -from the top - which maps to 15% */ + /* T440s has the top button line 5mm from the top, event +analysis has shown events to start down to ~10mm from the +top - which maps to 15%. We allow the caller to enlarge the +area using a multiplier for the touchpad disabled case. */ + double topsize_mm = 10 * topbutton_size_mult; + double topsize_pct = .15 * topbutton_size_mult; + if (yres 1) { tp-buttons.top_area.bottom_edge = - yoffset + 10 * yres; + yoffset + topsize_mm * yres; } else { - tp-buttons.top_area.bottom_edge = height * .15 + yoffset; + tp-buttons.top_area.bottom_edge = height * topsize_pct + yoffset; } tp-buttons.top_area.rightbutton_left_edge = width * .58 + xoffset; tp-buttons.top_area.leftbutton_right_edge = width * .42 + xoffset; @@ -581,7 +586,7 @@ tp_init_buttons(struct tp_dispatch *tp, tp-buttons.use_clickfinger = true; if (tp-buttons.is_clickpad !tp-buttons.use_clickfinger) { - tp_init_softbuttons(tp, device); + tp_init_softbuttons(tp, device, 1.0); } else { tp-buttons.bottom_area.top_edge = INT_MAX; tp-buttons.top_area.bottom_edge = INT_MIN; diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 9e568ad..1c32cc6 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -645,6 +645,8 @@ tp_suspend(struct tp_dispatch *tp, struct evdev_device *device) */ if (tp-buttons.has_topbuttons) { evdev_notify_suspended_device(device); + /* Enlarge topbutton area while suspended */ + tp_init_softbuttons(tp, device, 1.5); } else { evdev_device_suspend(device); } @@ -656,6 +658,8 @@ tp_resume(struct tp_dispatch *tp, struct evdev_device *device) if (tp-buttons.has_topbuttons) { /* tap state-machine is offline while suspended, reset state */ tp_clear_state(tp, device); + /* restore original topbutton area size */ + tp_init_softbuttons(tp, device, 1.0); evdev_notify_resumed_device(device); } else { evdev_device_resume(device); diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 15cca76..e0c8c47 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -245,7 +245,9 @@ int tp_init_buttons(struct tp_dispatch *tp, struct evdev_device *device); void -tp_init_softbuttons(struct tp_dispatch *tp, struct evdev_device *device); +tp_init_softbuttons(struct tp_dispatch *tp, + struct evdev_device *device, + double
Re: [PATCH libinput 3/3] test: Add trackpoint middlebutton scrolling tests
On Wed, Sep 17, 2014 at 03:35:32PM +0200, Hans de Goede wrote: Signed-off-by: Hans de Goede hdego...@redhat.com --- test/Makefile.am | 5 +++ test/litest-trackpoint.c | 2 +- test/litest.h| 1 + test/trackpoint.c| 100 +++ 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 test/trackpoint.c diff --git a/test/Makefile.am b/test/Makefile.am index 86859d8..6a68982 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -34,6 +34,7 @@ run_tests = \ test-touch \ test-log \ test-touchpad \ + test-trackpoint \ test-misc \ test-keyboard \ test-device @@ -72,6 +73,10 @@ test_touchpad_SOURCES = touchpad.c test_touchpad_LDADD = $(TEST_LIBS) test_touchpad_LDFLAGS = -no-install +test_trackpoint_SOURCES = trackpoint.c +test_trackpoint_LDADD = $(TEST_LIBS) +test_trackpoint_LDFLAGS = -no-install + test_misc_SOURCES = misc.c test_misc_LDADD = $(TEST_LIBS) test_misc_LDFLAGS = -no-install diff --git a/test/litest-trackpoint.c b/test/litest-trackpoint.c index 40b9ed0..01ad34e 100644 --- a/test/litest-trackpoint.c +++ b/test/litest-trackpoint.c @@ -56,7 +56,7 @@ static int events[] = { struct litest_test_device litest_trackpoint_device = { .type = LITEST_TRACKPOINT, - .features = LITEST_POINTER | LITEST_BUTTON, + .features = LITEST_POINTER | LITEST_BUTTON | LITEST_POINTINGSTICK, .shortname = trackpoint, .setup = litest_trackpoint_setup, .interface = interface, diff --git a/test/litest.h b/test/litest.h index fdf815f..fca6acb 100644 --- a/test/litest.h +++ b/test/litest.h @@ -61,6 +61,7 @@ enum litest_device_feature { LITEST_APPLE_CLICKPAD = 1 8, LITEST_TOPBUTTONPAD = 1 9, LITEST_SEMI_MT = 1 10, + LITEST_POINTINGSTICK = 1 11, }; struct litest_device { diff --git a/test/trackpoint.c b/test/trackpoint.c new file mode 100644 index 000..038185b --- /dev/null +++ b/test/trackpoint.c @@ -0,0 +1,100 @@ +/* + * 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 config.h + +#include check.h +#include errno.h +#include fcntl.h +#include libinput.h +#include unistd.h + +#include libinput-util.h +#include litest.h + +START_TEST(trackpoint_middlebutton) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + + /* A quick middle button click should get reported normally */ + litest_button_click(dev, BTN_MIDDLE, 1); + litest_button_click(dev, BTN_MIDDLE, 0); + + litest_assert_button_event(li, BTN_MIDDLE, 1); + litest_assert_button_event(li, BTN_MIDDLE, 0); + + litest_assert_empty_queue(li); +} +END_TEST add a test for a middlebutton timeout without trackstick motion please. + +static void +test_2fg_scroll(struct litest_device *dev, double dx, double dy) +{ + struct libinput *li = dev-libinput; + + litest_button_click(dev, BTN_MIDDLE, 1); + + libinput_dispatch(li); + msleep(300); + libinput_dispatch(li); + + litest_event(dev, EV_REL, REL_X, dx); + litest_event(dev, EV_REL, REL_Y, dy); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + + litest_button_click(dev, BTN_MIDDLE, 0); + + libinput_dispatch(li); +} + +START_TEST(trackpoint_scroll) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + + test_2fg_scroll(dev, 1, 6); + litest_assert_scroll(li, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL, 6); + test_2fg_scroll(dev, 1, -7); + litest_assert_scroll(li, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL, -7); +
Re: [PATCH libinput 2/3] litest: Add litest_assert_scroll() helper function
On Wed, Sep 17, 2014 at 03:35:31PM +0200, Hans de Goede wrote: Make check_2fg_scroll functionality available outside of touchpad.c , no functional changes. Signed-off-by: Hans de Goede hdego...@redhat.com --- test/litest.c | 39 +++ test/litest.h | 1 + test/touchpad.c | 49 - 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/test/litest.c b/test/litest.c index cf7e692..e5092b8 100644 --- a/test/litest.c +++ b/test/litest.c @@ -1084,3 +1084,42 @@ litest_assert_button_event(struct libinput *li, unsigned int button, state); libinput_event_destroy(event); } + +void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir) +{ + struct libinput_event *event, *next_event; + struct libinput_event_pointer *ptrev; + + event = libinput_get_event(li); + next_event = libinput_get_event(li); + ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */ ck_assert_notnull(), not that it makes much difference. + + while (event) { + ck_assert_int_eq(libinput_event_get_type(event), + LIBINPUT_EVENT_POINTER_AXIS); + ptrev = libinput_event_get_pointer_event(event); + ck_assert(ptrev != NULL); + ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis); + + if (next_event) { + /* Normal scroll event, check dir */ + if (dir 0) { + ck_assert_int_ge( + libinput_event_pointer_get_axis_value(ptrev), + dir); if you use a temporary var for the axis values we can skip the indentation/linewidth dance. Ok, and now that I wrote this I see that's just moving the code around. so Reviewed-by: Peter Hutterer peter.hutte...@who-t.net for this one and I'll push the updates on top of it. Cheers, Peter + } else { + ck_assert_int_le( + libinput_event_pointer_get_axis_value(ptrev), + dir); + } + } else { + /* Last scroll event, must be 0 */ + ck_assert_int_eq( + libinput_event_pointer_get_axis_value(ptrev), + 0); + } + libinput_event_destroy(event); + event = next_event; + next_event = libinput_get_event(li); + } +} diff --git a/test/litest.h b/test/litest.h index dd1386c..fdf815f 100644 --- a/test/litest.h +++ b/test/litest.h @@ -147,6 +147,7 @@ void litest_assert_empty_queue(struct libinput *li); void litest_assert_button_event(struct libinput *li, unsigned int button, enum libinput_button_state state); +void litest_assert_scroll(struct libinput *li, unsigned int axis, int dir); 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 7ff3d14..522cee6 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -1346,47 +1346,6 @@ test_2fg_scroll(struct litest_device *dev, double dx, double dy, int sleep) libinput_dispatch(li); } -static void -check_2fg_scroll(struct litest_device *dev, unsigned int axis, int dir) -{ - struct libinput *li = dev-libinput; - struct libinput_event *event, *next_event; - struct libinput_event_pointer *ptrev; - - event = libinput_get_event(li); - next_event = libinput_get_event(li); - ck_assert(next_event != NULL); /* At least 1 scroll + stop scroll */ - - while (event) { - ck_assert_int_eq(libinput_event_get_type(event), - LIBINPUT_EVENT_POINTER_AXIS); - ptrev = libinput_event_get_pointer_event(event); - ck_assert(ptrev != NULL); - ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis); - - if (next_event) { - /* Normal scroll event, check dir */ - if (dir 0) { - ck_assert_int_ge( - libinput_event_pointer_get_axis_value(ptrev), - dir); - } else { - ck_assert_int_le( - libinput_event_pointer_get_axis_value(ptrev), - dir); - } - } else { - /* Last scroll event, must be 0 */ - ck_assert_int_eq( -
Re: [PATCH libinput 2/8] evdev: Add middle button scrolling for trackpoints
On Tue, Sep 16, 2014 at 04:22:36PM +0200, Hans de Goede wrote: Most trackpoint users want to be able to scroll using the trackpoint with the middle button pressed, add support for this. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- include/linux/input.h| 1 + src/evdev.c | 56 src/evdev.h | 5 + test/litest-trackpoint.c | 2 ++ test/pointer.c | 4 +++- 5 files changed, 67 insertions(+), 1 deletion(-) diff --git a/include/linux/input.h b/include/linux/input.h index aa98ce7..39b550b 100644 --- a/include/linux/input.h +++ b/include/linux/input.h @@ -163,6 +163,7 @@ struct input_keymap_entry { #define INPUT_PROP_BUTTONPAD 0x02/* has button(s) under pad */ #define INPUT_PROP_SEMI_MT 0x03/* touch rectangle only */ #define INPUT_PROP_TOPBUTTONPAD 0x04/* softbuttons at top of pad */ +#define INPUT_PROP_POINTING_STICK0x05/* is a pointing stick */ #define INPUT_PROP_MAX 0x1f #define INPUT_PROP_CNT (INPUT_PROP_MAX + 1) diff --git a/src/evdev.c b/src/evdev.c index 45020ba..85e4d71 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -41,6 +41,7 @@ #include libinput-private.h #define DEFAULT_AXIS_STEP_DISTANCE 10 +#define DEFAULT_MIDDLE_BUTTON_SCROLL_TIMEOUT 200 enum evdev_key_type { EVDEV_KEY_TYPE_NONE, @@ -203,6 +204,15 @@ evdev_flush_pending_event(struct evdev_device *device, uint64_t time) device-rel.dx = 0; device-rel.dy = 0; + /* Use unaccelerated deltas for pointing stick scroll */ + if (device-scroll.has_middle_button_scroll + hw_is_key_down(device, BTN_MIDDLE)) { + if (device-scroll.middle_button_scroll_active) + evdev_post_scroll(device, time, + motion.dx, motion.dy); + break; + } + Just to verify: the behaviour here is that the scrolling does not activate until after the timeout, regardless of the number of motion events during the timeout period. Is this intentional, or do we need a threshold here? Cheers, Peter /* Apply pointer acceleration. */ filter_dispatch(device-pointer.filter, motion, device, time); @@ -346,6 +356,37 @@ get_key_type(uint16_t code) } static void +evdev_middle_button_scroll_timeout(uint64_t time, void *data) +{ + struct evdev_device *device = data; + + device-scroll.middle_button_scroll_active = true; +} + +static void +evdev_middle_button_scroll_button(struct evdev_device *device, + uint64_t time, int is_press) +{ + if (is_press) { + libinput_timer_set(device-scroll.timer, + time + DEFAULT_MIDDLE_BUTTON_SCROLL_TIMEOUT); + } else { + libinput_timer_cancel(device-scroll.timer); + if (device-scroll.middle_button_scroll_active) { + evdev_stop_scroll(device, time); + device-scroll.middle_button_scroll_active = false; + } else { + /* If the button is released quickly enough emit the + * button press/release events. */ + evdev_pointer_notify_button(device, time, BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_PRESSED); + evdev_pointer_notify_button(device, time, BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_RELEASED); + } + } +} + +static void evdev_process_touch_button(struct evdev_device *device, uint64_t time, int value) { @@ -405,6 +446,12 @@ evdev_process_key(struct evdev_device *device, LIBINPUT_KEY_STATE_RELEASED); break; case EVDEV_KEY_TYPE_BUTTON: + if (device-scroll.has_middle_button_scroll + e-code == BTN_MIDDLE) { + evdev_middle_button_scroll_button(device, time, + e-value); + break; + } evdev_pointer_notify_button( device, time, @@ -946,6 +993,15 @@ evdev_configure_device(struct evdev_device *device) device-mt.slot = active_slot; } } + + if (libevdev_has_property(evdev, INPUT_PROP_POINTING_STICK)) { + libinput_timer_init(device-scroll.timer, + device-base.seat-libinput, + evdev_middle_button_scroll_timeout, + device); +
Re: [PATCH libinput 5/8] touchpad: Route top softbuttons through the trackstick if we've one
On Tue, Sep 16, 2014 at 04:22:39PM +0200, Hans de Goede wrote: The touchpad top softbuttons such as found on the Lenove T440 are intended for use with the trackstick. Route their events through the trackstick, so that they can be used for e.g. middle button scrolling with the trackstick. Note that sending top button events to a disabled trackpoint makes no sense (and will mess up internal state). Likely a user with a disabled trackpoint will still expect the top buttons to work, so rather than not sending events in that case, simply treat a suspendeded trackpoint as not being there, and send the events directly from the touchpad device. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net but see the notes below --- doc/touchpad-softbutton-state-machine.svg | 200 +- src/evdev-mt-touchpad-buttons.c | 45 +-- src/evdev-mt-touchpad.c | 17 ++- src/evdev-mt-touchpad.h | 1 + 4 files changed, 163 insertions(+), 100 deletions(-) diff --git a/doc/touchpad-softbutton-state-machine.svg b/doc/touchpad-softbutton-state-machine.svg index 1d569bf..ffa17a2 100644 --- a/doc/touchpad-softbutton-state-machine.svg +++ b/doc/touchpad-softbutton-state-machine.svg [...] diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 02d3205..0fdabde 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -675,16 +675,40 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) return 0; } +static void +tp_notify_softbutton(struct tp_dispatch *tp, + uint64_t time, + uint32_t button, + uint32_t is_top, just because I prefer it: I replaced this with is_topbutton, and active_is_top with active_is_topbutton (simple sed, both times) + enum libinput_button_state state) +{ + /* If we've a trackpoint, send top buttons through the trackpoint */ + if (is_top tp-buttons.trackpoint) { + struct evdev_dispatch *dispatch = tp-buttons.trackpoint-dispatch; + struct input_event event; + + event.type = EV_KEY, + event.code = button, I'm going to assume you meant ; here instead of ',' so I've replaced those after applying. + event.value = (state == LIBINPUT_BUTTON_STATE_PRESSED) ? 1 : 0; for correctness, we should set the time here too, I've added + event.time.tv_sec = time/1000; + event.time.tv_usec = (time % 1000) * 1000; patch coming up, just double-check it please. Cheers, Peter + dispatch-interface-process(dispatch, tp-buttons.trackpoint, + event, time); + return; + } + + evdev_pointer_notify_button(tp-device, time, button, state); +} + static int tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) { - uint32_t current, old, button; + uint32_t current, old, button, is_top; enum libinput_button_state state; enum { AREA = 0x01, LEFT = 0x02, MIDDLE = 0x04, RIGHT = 0x08 }; current = tp-buttons.state; old = tp-buttons.old_state; button = 0; + is_top = 0; if (!tp-buttons.click_pending current == old) return 0; @@ -697,15 +721,18 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) case BUTTON_EVENT_IN_AREA: button |= AREA; break; - case BUTTON_EVENT_IN_BOTTOM_L: case BUTTON_EVENT_IN_TOP_L: + is_top = 1; + case BUTTON_EVENT_IN_BOTTOM_L: button |= LEFT; break; case BUTTON_EVENT_IN_TOP_M: + is_top = 1; button |= MIDDLE; break; - case BUTTON_EVENT_IN_BOTTOM_R: case BUTTON_EVENT_IN_TOP_R: + is_top = 1; + case BUTTON_EVENT_IN_BOTTOM_R: button |= RIGHT; break; default: @@ -727,21 +754,21 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) button = BTN_LEFT; tp-buttons.active = button; + tp-buttons.active_is_top = is_top; state = LIBINPUT_BUTTON_STATE_PRESSED; } else { button = tp-buttons.active; + is_top = tp-buttons.active_is_top; tp-buttons.active = 0; + tp-buttons.active_is_top = 0; state = LIBINPUT_BUTTON_STATE_RELEASED; }
[PATCH v2 libinput] touchpad: Route top softbuttons through the trackstick if we've one
From: Hans de Goede hdego...@redhat.com The touchpad top softbuttons such as found on the Lenove T440 are intended for use with the trackstick. Route their events through the trackstick, so that they can be used for e.g. middle button scrolling with the trackstick. Note that sending top button events to a disabled trackpoint makes no sense (and will mess up internal state). Likely a user with a disabled trackpoint will still expect the top buttons to work, so rather than not sending events in that case, simply treat a suspendeded trackpoint as not being there, and send the events directly from the touchpad device. Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Changes to v1: - rename is_top and active_is_top to *is_topbutton for better clarity - add timestamp to struct input_event generation - replace two commas with semicolons (typos) - change active_is_topbutton to a bool (from unsigned int) doc/touchpad-softbutton-state-machine.svg | 198 +- src/evdev-mt-touchpad-buttons.c | 47 +-- src/evdev-mt-touchpad.c | 17 ++- src/evdev-mt-touchpad.h | 1 + 4 files changed, 164 insertions(+), 99 deletions(-) diff --git a/doc/touchpad-softbutton-state-machine.svg b/doc/touchpad-softbutton-state-machine.svg index 1d569bf..ffa17a2 100644 --- a/doc/touchpad-softbutton-state-machine.svg +++ b/doc/touchpad-softbutton-state-machine.svg @@ -1,5 +1,5 @@ ?xml version=1.0? -svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=2190px height=1624px version=1.1 +svg xmlns=http://www.w3.org/2000/svg; xmlns:xlink=http://www.w3.org/1999/xlink; width=2180px height=1624px version=1.1 defs/ g transform=translate(0.5,0.5) path d=M 862 441 L 894 441 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ @@ -68,13 +68,13 @@ path d=M 712 441 L 668 441 Q 658 441 658 451 L 658 762 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ path d=M 786 62 L 786 81 Q 786 91 786 101 L 786 114 fill=none stroke=#00 stroke-miterlimit=10 pointer-events=none/ path d=M 786 120 L 783 113 L 786 114 L 790 113 Z fill=#00 stroke=#00 stroke-miterlimit=10 pointer-events=none/ -rect x=1753 y=472 width=120 height=40 rx=16 ry=16 fill=#c0 stroke=#ff pointer-events=none/ +rect x=1753 y=412 width=120 height=40 rx=16 ry=16 fill=#c0 stroke=#ff pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px - text x=1813 y=489Check state of/text - text x=1813 y=503all touches/text + text x=1813 y=429Check state of/text + text x=1813 y=443all touches/text /g -path d=M 1813 512 L 1813 518 Q 1813 524 1813 529 L 1813 534 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ -path d=M 1809 526 L 1813 535 L 1818 526 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ +path d=M 1813 452 L 1813 458 Q 1813 465 1813 470 L 1813 475 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ +path d=M 1809 467 L 1813 476 L 1818 467 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ ellipse cx=1813 cy=72 rx=11 ry=11 fill=#00 stroke=#ff pointer-events=none/ path d=M 1813 87 L 1813 115 Q 1813 125 1813 135 L 1813 160 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ path d=M 1809 152 L 1813 161 L 1818 152 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ @@ -82,88 +82,89 @@ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px text x=1813 y=41tp_post_softbutton_buttons()/text /g -path d=M 1813 212 L 1908 267 L 1813 322 L 1718 267 Z fill=#c0 stroke=#ff stroke-miterlimit=10 pointer-events=none/ +path d=M 1813 192 L 1908 247 L 1813 302 L 1718 247 Z fill=#c0 stroke=#ff stroke-miterlimit=10 pointer-events=none/ g fill=#00 font-family=Helvetica text-anchor=middle font-size=12px - text x=1813 y=264!buttons.click_pend/text - text x=1813 y=278amp;amp; current == old/text + text x=1813 y=244!buttons.click_pend/text + text x=1813 y=258amp;amp; current == old/text /g -path d=M 1908 267 L 1968 267 Q 1978 267 1988 267 L 2056 267 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ -path d=M 2048 272 L 2057 267 L 2048 263 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ +path d=M 1908 247 L 1968 247 Q 1978 247 1988 247 L 2056 247 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ +path d=M 2048 252 L 2057 247 L 2048 243 fill=none stroke=#ff stroke-miterlimit=10 pointer-events=none/ g fill=#00 font-family=Helvetica font-size=11px - rect fill=#ff stroke=none x=1998 y=252 width=18 height=14
[PATCH libinput 4/7] touchpad: move updating of buttons.old_state to the process function
Update the old_state once we've posted the respective buttons rather than waiting for post_process to kick in. This allows calling our function multiple times without double-posting the events. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 7 +++ src/evdev-mt-touchpad.c | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index fae6a13..2d4b2b9 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -640,6 +640,9 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) button, state); } + + tp-buttons.old_state = tp-buttons.state; + return 1; } @@ -672,6 +675,8 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) old = 1; } + tp-buttons.old_state = tp-buttons.state; + return 0; } @@ -775,6 +780,8 @@ tp_post_softbutton_buttons(struct tp_dispatch *tp, uint64_t time) if (button) tp_notify_softbutton(tp, time, button, is_top, state); + tp-buttons.old_state = tp-buttons.state; + return 1; } diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 26517b9..f9efc5f 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -460,7 +460,6 @@ tp_post_process_state(struct tp_dispatch *tp, uint64_t time) } tp-old_nfingers_down = tp-nfingers_down; - tp-buttons.old_state = tp-buttons.state; tp-queued = TOUCHPAD_EVENT_NONE; } -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 0/7] Handle jumping cursors
When a finger leaves the touchpad at the same time as another finger sets down, a touchpad may not notice the change in fingers but rather think the touchpoint moved to the new position. Bug 76722 has an event sequence but it really boils down to just a move of a touch, identifiable only by the distance moved. https://bugs.freedesktop.org/show_bug.cgi?id=76722 I've done some measurements on a T440 and a x220 and the most one can reasonably move on those two is under 20mm. Use the script at the url below if you want to verify yourself. https://github.com/whot/input-data-analysis/tree/master/touchpad-max-delta The idea of the fix was to consider a jumping touch a changing touch, i.e. end it and begin it at the new position. This also requires the new touch to be tap-capable, recognised by the softbutton code, etc. Handling this is a bit tricky because we handle the state of the touchpad at EV_SYN time, so we can't easily split event handling. Since we don't know which touchpoint moves (could be the second one, in theory) we'd have to do double-processing of the state otherwise, i.e. pull all events up to the EV_SYN, check each touchpoint for jumps, release those, process the state, then re-process the events again. The approach I've picked here is to handle the state twice (rather than the events), ignoring certain things the first time and other things the second time. Benjamin is currently looking into a (hopefully simpler) kernel fix so maybe we won't need this patchset anyway, but I prefer it to be archived on the list either way. All that aside, patches 1-3 are useful anyway. All available on https://github.com/whot/libinput/tree/wip/jumping-cursor Cheers, Peter ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 5/7] touchpad: only send button events if we have button events queued
Even if the button state differs, only send events if we claim we have a button press or release queued up. Otherwise, skip over the event. This cannot happen in the current code, it's prep-work for an upcoming patch. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 36 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 2d4b2b9..b0a89ae 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -618,7 +618,7 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) if (current == old) return 0; - if (current) { + if (current (tp-queued TOUCHPAD_EVENT_BUTTON_PRESS)) { switch (tp-nfingers_down) { case 1: button = BTN_LEFT; break; case 2: button = BTN_RIGHT; break; @@ -628,11 +628,12 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) } tp-buttons.active = button; state = LIBINPUT_BUTTON_STATE_PRESSED; - } else { + } else if (!current (tp-queued TOUCHPAD_EVENT_BUTTON_RELEASE)) { button = tp-buttons.active; tp-buttons.active = 0; state = LIBINPUT_BUTTON_STATE_RELEASED; - } + } else + return 0; if (button) { evdev_pointer_notify_button(tp-device, @@ -649,34 +650,45 @@ tp_post_clickfinger_buttons(struct tp_dispatch *tp, uint64_t time) static int tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) { - uint32_t current, old, button; + uint32_t current, old, button, mask; current = tp-buttons.state; old = tp-buttons.old_state; button = BTN_LEFT; + mask = 0x1; while (current || old) { enum libinput_button_state state; if ((current 0x1) ^ (old 0x1)) { - if (!!(current 0x1)) + bool skip = false; + if (!!(current 0x1) + tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) state = LIBINPUT_BUTTON_STATE_PRESSED; - else + else if (!(current 0x1) +tp-queued TOUCHPAD_EVENT_BUTTON_RELEASE) state = LIBINPUT_BUTTON_STATE_RELEASED; + else + skip = true; - evdev_pointer_notify_button(tp-device, - time, - button, - state); + if (!skip) { + evdev_pointer_notify_button(tp-device, + time, + button, + state); + if (state == LIBINPUT_BUTTON_STATE_PRESSED) + tp-buttons.old_state |= mask; + else + tp-buttons.old_state = ~mask; + } } button++; current = 1; old = 1; + mask = 1; } - tp-buttons.old_state = tp-buttons.state; - return 0; } -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 2/7] test: add helper functions for the two timers we care about
Rather than a random msleep() with a comment, use a helper function that describes what we're waiting for. Also makes changing the timeouts easier in the future. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/litest.c | 12 test/litest.h | 3 +++ test/touchpad.c | 42 +- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/test/litest.c b/test/litest.c index cf7e692..2b356be 100644 --- a/test/litest.c +++ b/test/litest.c @@ -1084,3 +1084,15 @@ litest_assert_button_event(struct libinput *li, unsigned int button, state); libinput_event_destroy(event); } + +void +litest_timeout_tap(void) +{ + msleep(200); +} + +void +litest_timeout_softbuttons(void) +{ + msleep(300); +} diff --git a/test/litest.h b/test/litest.h index dd1386c..40d4df9 100644 --- a/test/litest.h +++ b/test/litest.h @@ -156,6 +156,9 @@ struct libevdev_uinput * litest_create_uinput_abs_device(const char *name, const struct input_absinfo *abs, ...); +void litest_timeout_tap(void); +void litest_timeout_softbuttons(void); + #ifndef ck_assert_notnull #define ck_assert_notnull(ptr) ck_assert_ptr_ne(ptr, NULL) #endif diff --git a/test/touchpad.c b/test/touchpad.c index a6e2dd7..ea02f47 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -107,7 +107,7 @@ START_TEST(touchpad_1fg_tap) litest_assert_button_event(li, BTN_LEFT, LIBINPUT_BUTTON_STATE_PRESSED); - msleep(300); /* tap-n-drag timeout */ + litest_timeout_tap(); litest_assert_button_event(li, BTN_LEFT, LIBINPUT_BUTTON_STATE_RELEASED); @@ -162,7 +162,7 @@ START_TEST(touchpad_1fg_tap_n_drag) ck_assert_int_eq(libinput_next_event_type(li), LIBINPUT_EVENT_NONE); - msleep(300); /* tap-n-drag timeout */ + litest_timeout_tap(); litest_assert_button_event(li, BTN_LEFT, LIBINPUT_BUTTON_STATE_RELEASED); @@ -219,7 +219,7 @@ START_TEST(touchpad_2fg_tap) litest_assert_button_event(li, BTN_RIGHT, LIBINPUT_BUTTON_STATE_PRESSED); - msleep(300); /* tap-n-drag timeout */ + litest_timeout_tap(); litest_assert_button_event(li, BTN_RIGHT, LIBINPUT_BUTTON_STATE_RELEASED); @@ -246,7 +246,7 @@ START_TEST(touchpad_2fg_tap_inverted) litest_assert_button_event(li, BTN_RIGHT, LIBINPUT_BUTTON_STATE_PRESSED); - msleep(300); /* tap-n-drag timeout */ + litest_timeout_tap(); litest_assert_button_event(li, BTN_RIGHT, LIBINPUT_BUTTON_STATE_RELEASED); @@ -274,7 +274,7 @@ START_TEST(touchpad_1fg_tap_click) litest_event(dev, EV_KEY, BTN_LEFT, 0); litest_event(dev, EV_SYN, SYN_REPORT, 0); libinput_dispatch(li); - msleep(200); + litest_timeout_tap(); libinput_dispatch(li); @@ -423,7 +423,7 @@ START_TEST(touchpad_no_2fg_tap_after_timeout) */ litest_touch_down(dev, 0, 50, 50); libinput_dispatch(dev-libinput); - msleep(300); + litest_timeout_tap(); libinput_dispatch(dev-libinput); litest_drain_events(dev-libinput); @@ -577,7 +577,7 @@ START_TEST(touchpad_3fg_tap) litest_assert_button_event(li, BTN_MIDDLE, LIBINPUT_BUTTON_STATE_PRESSED); - msleep(300); /* tap-n-drag timeout */ + litest_timeout_tap(); litest_assert_button_event(li, BTN_MIDDLE, LIBINPUT_BUTTON_STATE_RELEASED); @@ -613,7 +613,7 @@ START_TEST(touchpad_3fg_tap_btntool) litest_assert_button_event(li, BTN_MIDDLE, LIBINPUT_BUTTON_STATE_PRESSED); - msleep(300); /* tap-n-drag timeout */ + litest_timeout_tap(); litest_assert_button_event(li, BTN_MIDDLE, LIBINPUT_BUTTON_STATE_RELEASED); @@ -648,7 +648,7 @@ START_TEST(touchpad_3fg_tap_btntool_inverted) litest_assert_button_event(li, BTN_MIDDLE, LIBINPUT_BUTTON_STATE_PRESSED); - msleep(300); /* tap-n-drag timeout */ + litest_timeout_tap(); litest_assert_button_event(li, BTN_MIDDLE, LIBINPUT_BUTTON_STATE_RELEASED); @@ -750,7 +750,7 @@ START_TEST(clickpad_1fg_tap_click) litest_event(dev, EV_SYN, SYN_REPORT, 0); litest_touch_up(dev, 0); libinput_dispatch(li); - msleep(200); + litest_timeout_tap(); libinput_dispatch(li); @@ -999,7 +999,7 @@ START_TEST(clickpad_softbutton_left_1st_fg_move) /* move out of the area,
[PATCH libinput 3/7] test: add litest_push/pop_event_frame() helpers
For some tests we need to string multiple event sequences together into one event frame. Use a push/pop frame approach that stops litest from sending any EV_SYN/SYN_REPORT events, so we can merge two touches together by e.g. litest_push_event_frame(d); litest_touch_down(d, 0, 10, 10); litest_touch_down(d, 1, 20, 50); litest_pop_event_frame(d); Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/litest.c | 22 +- test/litest.h | 5 + 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/test/litest.c b/test/litest.c index 2b356be..eed41ba 100644 --- a/test/litest.c +++ b/test/litest.c @@ -634,7 +634,12 @@ void litest_event(struct litest_device *d, unsigned int type, unsigned int code, int value) { - int ret = libevdev_uinput_write_event(d-uinput, type, code, value); + int ret; + + if (d-skip_ev_syn type == EV_SYN code == SYN_REPORT) + return; + + ret = libevdev_uinput_write_event(d-uinput, type, code, value); ck_assert_int_eq(ret, 0); } @@ -1096,3 +1101,18 @@ litest_timeout_softbuttons(void) { msleep(300); } + +void +litest_push_event_frame(struct litest_device *dev) +{ + assert(!dev-skip_ev_syn); + dev-skip_ev_syn = true; +} + +void +litest_pop_event_frame(struct litest_device *dev) +{ + assert(dev-skip_ev_syn); + dev-skip_ev_syn = false; + litest_event(dev, EV_SYN, SYN_REPORT, 0); +} diff --git a/test/litest.h b/test/litest.h index 40d4df9..540e07b 100644 --- a/test/litest.h +++ b/test/litest.h @@ -72,6 +72,8 @@ struct litest_device { struct litest_device_interface *interface; int ntouches_down; + bool skip_ev_syn; + void *private; /* device-specific data */ }; @@ -159,6 +161,9 @@ struct libevdev_uinput * litest_create_uinput_abs_device(const char *name, void litest_timeout_tap(void); void litest_timeout_softbuttons(void); +void litest_push_event_frame(struct litest_device *dev); +void litest_pop_event_frame(struct litest_device *dev); + #ifndef ck_assert_notnull #define ck_assert_notnull(ptr) ck_assert_ptr_ne(ptr, NULL) #endif -- 1.9.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput 7/7] test: add some tests for jumping fingers on touchpads
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- test/touchpad.c | 259 1 file changed, 259 insertions(+) diff --git a/test/touchpad.c b/test/touchpad.c index ea02f47..37df992 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -1623,6 +1623,255 @@ START_TEST(touchpad_palm_detect_no_palm_moving_into_edges) } END_TEST +START_TEST(touchpad_jump_finger_up) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + litest_touch_down(dev, 0, 20, 30); + litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10); + litest_drain_events(li); + + /* no motion event caused by jump*/ + litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1); + litest_touch_up(dev, 0); + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(touchpad_jump_finger_up_2fg_scroll) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + litest_touch_down(dev, 0, 20, 30); + litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10); + litest_drain_events(li); + + litest_push_event_frame(dev); + litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1); + litest_touch_down(dev, 1, 50, 50); + litest_pop_event_frame(dev); + + litest_assert_empty_queue(li); + + /* Now move the first finger, expect scroll */ + litest_touch_move_to(dev, 0, 90, 30, 20, 40, 10); + litest_wait_for_event_of_type(li, LIBINPUT_EVENT_POINTER_AXIS, -1); +} +END_TEST + +START_TEST(touchpad_jump_finger_up_2fg_tap) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + libinput_device_config_tap_set_enabled(dev-libinput_device, + LIBINPUT_CONFIG_TAP_ENABLED); + + litest_drain_events(li); + litest_touch_down(dev, 0, 20, 30); + litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10); + litest_drain_events(li); + + litest_push_event_frame(dev); + litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1); + litest_touch_down(dev, 1, 50, 50); + litest_pop_event_frame(dev); + + litest_assert_empty_queue(li); + + /* expect two-finger tap */ + + litest_touch_up(dev, 0); + litest_touch_up(dev, 1); + + litest_assert_button_event(li, + BTN_RIGHT, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_button_event(li, + BTN_RIGHT, + LIBINPUT_BUTTON_STATE_RELEASED); + +} +END_TEST + +START_TEST(touchpad_jump_finger_up_release) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + litest_touch_down(dev, 0, 20, 30); + litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10); + litest_button_click(dev, BTN_LEFT, 1); + litest_drain_events(li); + + /* send BTN_LEFT release on touch jump */ + litest_push_event_frame(dev); + litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1); + litest_event(dev, EV_KEY, BTN_LEFT, 0); + litest_pop_event_frame(dev); + litest_touch_up(dev, 0); + + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(touchpad_jump_finger_up_press) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + + litest_drain_events(li); + litest_touch_down(dev, 0, 20, 30); + litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10); + litest_drain_events(li); + + /* send BTN_LEFT press on touch jump */ + litest_push_event_frame(dev); + litest_touch_move_to(dev, 0, 90, 30, 20, 90, 1); + litest_event(dev, EV_KEY, BTN_LEFT, 1); + litest_pop_event_frame(dev); + litest_touch_up(dev, 0); + + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(touchpad_jump_finger_up_tap) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev-libinput; + struct libinput_event *event; + struct libinput_event_pointer *ptrev; + + libinput_device_config_tap_set_enabled(dev-libinput_device, + LIBINPUT_CONFIG_TAP_ENABLED); + litest_drain_events(li); + litest_touch_down(dev, 0, 20, 30); + litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10); + litest_drain_events(li); + + /* jumping finger counts as new finger, so
[PATCH libinput 6/7] touchpad: detect coordinate jumps on touchpads
Such jumps are usually consequences of the touchpad firmware failing to notice that a different finger is in fact touching the touchpad. This happens frequently when a finger is lifted at the same time as a second finger is set down. Instead of a ABS_MT_TRACKING_ID of -1 followed by a new touchpoing we get a large movement from the previous position to the next position. This leads to a cursor jump on the screen. This patch adds detection for these jumps. Since the new finger is a new touchpoint, all the usual rules for a new touch point should apply (softbutton detection, tapping, etc.). Thus, if we find one or more touchpoints to have jumped, we mark the touchpoint as jumped and end it. Then we process the current state, releasing all buttons where appropriate. Then we re-process the state, marking all jumped and ended touchpoints as TOUCH_BEGIN (and pressing buttons where appropriate). The jump threshold is 20mm on devices with resolution, or a quarter of the diagonal for devices without a resolution. https://bugs.freedesktop.org/show_bug.cgi?id=76722 Based on a patch by Alexander E. Patrakov patra...@gmail.com Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad-buttons.c | 17 +- src/evdev-mt-touchpad.c | 123 ++-- src/evdev-mt-touchpad.h | 7 ++- 3 files changed, 138 insertions(+), 9 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index b0a89ae..11b309b 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -416,7 +416,9 @@ tp_button_handle_event(struct tp_dispatch *tp, } int -tp_button_handle_state(struct tp_dispatch *tp, uint64_t time) +tp_button_handle_state(struct tp_dispatch *tp, + uint64_t time, + bool ignore_button_press) { struct tp_touch *t; @@ -440,9 +442,11 @@ tp_button_handle_state(struct tp_dispatch *tp, uint64_t time) else tp_button_handle_event(tp, t, BUTTON_EVENT_IN_AREA, time); } + if (tp-queued TOUCHPAD_EVENT_BUTTON_RELEASE) tp_button_handle_event(tp, t, BUTTON_EVENT_RELEASE, time); - if (tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) + if (!ignore_button_press + tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) tp_button_handle_event(tp, t, BUTTON_EVENT_PRESS, time); } @@ -662,6 +666,14 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) if ((current 0x1) ^ (old 0x1)) { bool skip = false; + + /* we may have button state changes but only process + them if the matching tp-queued mask is set. + + If a touch jump happens, the various post functions + are called separately with the queued mask manually + adjusted and we post release events before + press events */ if (!!(current 0x1) tp-queued TOUCHPAD_EVENT_BUTTON_PRESS) state = LIBINPUT_BUTTON_STATE_PRESSED; @@ -682,7 +694,6 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) tp-buttons.old_state = ~mask; } } - button++; current = 1; old = 1; diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index f9efc5f..426c22b 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -160,6 +160,67 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) tp-queued |= TOUCHPAD_EVENT_MOTION; } +static inline void +tp_disrupt_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) +{ + if (t-state == TOUCH_NONE) + return; + + tp_end_touch(tp, t, time); + t-has_jumped = true; +} + +static inline void +tp_undisrupt_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) +{ + if (!t-has_jumped) + return; + + t-has_jumped = false; + tp_begin_touch(tp, t, time); +} + +static inline int +tp_detect_jumps(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) +{ + struct tp_motion *oldpos; + double dx, dy; + + /* Detect cursor jumps. The sampling rate of touchpads limits what +* we can detect, if one finger leaves touch and another sets down +* quickly enough, the firmware may not notice the touch up/down +* event but assume the finger moved. +* See https://bugs.freedesktop.org/show_bug.cgi?id=76722 +* +* We detect the jump here, mark the touch as jumped so we can end +* it and then continue with the current touch as