Hi, For some reason some of the patches have not made it to my mailbox, so I'm reviewing the entire set here, partially from the web archive.
Patches 1 and 2 looks good and are: Reviewed-by: Hans de Goede <hdego...@redhat.com> The state machine code in patch 3 has some issues: In evdev_middlebutton_ldown_handle_event() you do: + case MIDDLEBUTTON_EVENT_R_UP: + break; But in evdev_middlebutton_rdown_handle_event() you do: + case MIDDLEBUTTON_EVENT_L_UP: + middlebutton_state_error(device, event); + break; This is not consistent. Also you forget to cancel the timer in a whole bunch of code paths, one example is: Press left button and release before timeout, now we're back in idle with the timer running and have: +evdev_middlebutton_idle_handle_event(struct evdev_device *device, + uint64_t time, + enum evdev_middlebutton_event event) + case MIDDLEBUTTON_EVENT_TIMEOUT: + middlebutton_state_error(device, event); + break; Triggering. I suggest doing what we've ended up doing in most other state machines and have a set_state helper which is the only one to ever touch device->middlebutton.state, that can then always cancel the timer when called, and (re)set the timer and device->middlebutton.first_event_time for the states where we want to start the timer on entering the state. Also you may want to make the sleep in litest_timeout_middlebutton slightly larger, at least in all other litest_timeout functions the sleep is somewhat longer then the timeout in libinput to avoid surprises / races. Patches 4 - 6 look good and are: Reviewed-by: Hans de Goede <hdego...@redhat.com> Regards, Hans On 15-04-15 05:51, Peter Hutterer wrote:
No functional changes at this point, this merely splits up any physical buttons (i.e. that represent buttons that exist on that device) vs. other buttons that are emulated in some way or another. This is in preparation for the addition of middle button emulation. Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net> --- src/evdev-mt-touchpad-buttons.c | 8 ++++---- src/evdev.c | 17 +++++++++++++---- src/evdev.h | 5 +++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c index 18c32fd..56a054c 100644 --- a/src/evdev-mt-touchpad-buttons.c +++ b/src/evdev-mt-touchpad-buttons.c @@ -734,10 +734,10 @@ tp_post_physical_buttons(struct tp_dispatch *tp, uint64_t time) state = LIBINPUT_BUTTON_STATE_RELEASED; b = evdev_to_left_handed(tp->device, button); - evdev_pointer_notify_button(tp->device, - time, - b, - state); + evdev_pointer_notify_physical_button(tp->device, + time, + b, + state); } button++; diff --git a/src/evdev.c b/src/evdev.c index 5b4b2b6..6f87484 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -138,6 +138,15 @@ evdev_keyboard_notify_key(struct evdev_device *device, } void +evdev_pointer_notify_physical_button(struct evdev_device *device, + uint32_t time, + int button, + enum libinput_button_state state) +{ + evdev_pointer_notify_button(device, time, button, state); +} + +void evdev_pointer_notify_button(struct evdev_device *device, uint32_t time, int button, @@ -430,10 +439,10 @@ evdev_button_scroll_button(struct evdev_device *device, } else { /* If the button is released quickly enough emit the * button press/release events. */ - evdev_pointer_notify_button(device, time, + evdev_pointer_notify_physical_button(device, time, device->scroll.button, LIBINPUT_BUTTON_STATE_PRESSED); - evdev_pointer_notify_button(device, time, + evdev_pointer_notify_physical_button(device, time, device->scroll.button, LIBINPUT_BUTTON_STATE_RELEASED); } @@ -505,7 +514,7 @@ evdev_process_key(struct evdev_device *device, evdev_button_scroll_button(device, time, e->value); break; } - evdev_pointer_notify_button( + evdev_pointer_notify_physical_button( device, time, evdev_to_left_handed(device, e->code), @@ -2208,7 +2217,7 @@ release_pressed_keys(struct evdev_device *device) LIBINPUT_KEY_STATE_RELEASED); break; case EVDEV_KEY_TYPE_BUTTON: - evdev_pointer_notify_button( + evdev_pointer_notify_physical_button( device, time, evdev_to_left_handed(device, code), diff --git a/src/evdev.h b/src/evdev.h index af09baa..f2ce9bc 100644 --- a/src/evdev.h +++ b/src/evdev.h @@ -306,6 +306,11 @@ evdev_pointer_notify_button(struct evdev_device *device, uint32_t time, int button, enum libinput_button_state state); +void +evdev_pointer_notify_physical_button(struct evdev_device *device, + uint32_t time, + int button, + enum libinput_button_state state); void evdev_init_natural_scroll(struct evdev_device *device);
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel