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

Reply via email to