[PATCH libinput] tablet: ignore tools already in proximity at startup
Peter Hutterer
peter.hutterer at who-t.net
Thu Feb 26 14:27:58 PST 2015
On Thu, Feb 26, 2015 at 01:54:31PM -0800, Ping Cheng wrote:
> On Thu, Feb 26, 2015 at 1:31 PM, Benjamin Tissoires
> <benjamin.tissoires at gmail.com> wrote:
> > On Thu, Feb 26, 2015 at 12:05 AM, Peter Hutterer
> > <peter.hutterer at who-t.net> wrote:
> >> Require a tool to be moved into proximity before we handle any events from it.
> >> This requires the user to lift the tool for first use but a tool that's
> >> already on the tablet on init is likely to send garbage anyway and interfere.
> >> This way users can leave the mouse/pen on the tablet overnight and their
> >> events won't interfere until they actually use it.
> >>
> >> Reported-by: Jason Gerecke <killertofu at gmail.com>
> >> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >
> > This fixes the crash, but it is a bad fix for the user.
>
> I agree with Benjamin.
>
> > Each time you restart libinput/Xorg and you have your mouse on the
> > Wacom, you need to remove it and re-add it.
>
> We should retrieve previous events through ioctl as we did in wcmUSB.c
> for xf86-input-wacom.
>
> > I'd really prefer that we send the in-prox event and directly send the
> > incoming events.
>
> Only sending incoming events is not enough if tool is on the tablet
> when libinput/Xorg is restarted. Repeated events would not be posted
> by input drivers in the kernel.
>
> > And honestly, given the precision of the wacoms, there won't be any
> > garbage events: most axes are updated at each frame so we should be
> > fine.
>
> The root cause of garbage events is we failed to retrieve the initial
> states of those events.
let me rephrase the term "garbage". except for the mouse device which is in
relative mode by default, the tools will still send small coordinate
updates whenever they move. so if you leave the tool on the tablet
inadvertently, it causes the pointer to be either stuck on the screen or
jump back irregularly. this has caught me quite a few times over the years
during testing. so it's not about the event data itself, it's about the
behaviour.
For the mouse I can understand that we should put it in-prox immediately
because it's likely left on the tablet. for the other tools, not so much.
Cheers,
Peter
>
> >> ---
> >> src/evdev-tablet.c | 10 +++++++++-
> >> test/tablet.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 52 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> >> index 4524202..1b87ec3 100644
> >> --- a/src/evdev-tablet.c
> >> +++ b/src/evdev-tablet.c
> >> @@ -190,7 +190,7 @@ tablet_update_tool(struct tablet_dispatch *tablet,
> >> tablet_set_status(tablet, TABLET_TOOL_ENTERING_PROXIMITY);
> >> tablet_unset_status(tablet, TABLET_TOOL_OUT_OF_PROXIMITY);
> >> }
> >> - else
> >> + else if (!tablet_has_status(tablet, TABLET_TOOL_OUT_OF_PROXIMITY))
> >> tablet_set_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY);
> >> }
> >>
> >> @@ -863,6 +863,9 @@ tablet_flush(struct tablet_dispatch *tablet,
> >> tablet->current_tool_id,
> >> tablet->current_tool_serial);
> >>
> >> + if (tablet_has_status(tablet, TABLET_TOOL_OUT_OF_PROXIMITY))
> >> + return;
> >> +
> >> if (tablet_has_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY)) {
> >> /* Release all stylus buttons */
> >> memset(tablet->button_state.stylus_buttons,
> >> @@ -910,7 +913,11 @@ tablet_flush(struct tablet_dispatch *tablet,
> >>
> >> tablet_change_to_left_handed(device);
> >> }
> >> +}
> >>
> >> +static inline void
> >> +tablet_reset_state(struct tablet_dispatch *tablet)
> >> +{
> >> /* Update state */
> >> memcpy(&tablet->prev_button_state,
> >> &tablet->button_state,
> >> @@ -941,6 +948,7 @@ tablet_process(struct evdev_dispatch *dispatch,
> >> break;
> >> case EV_SYN:
> >> tablet_flush(tablet, device, time);
> >> + tablet_reset_state(tablet);
> >> break;
> >> default:
> >> log_error(device->base.seat->libinput,
> >> diff --git a/test/tablet.c b/test/tablet.c
> >> index d7486cb..94aa93d 100644
> >> --- a/test/tablet.c
> >> +++ b/test/tablet.c
> >> @@ -1157,6 +1157,48 @@ START_TEST(tool_capabilities)
> >> }
> >> END_TEST
> >>
> >> +START_TEST(tool_in_prox_before_start)
> >> +{
> >> + struct libinput *li;
> >> + struct litest_device *dev = litest_current_device();
> >> + struct libinput_event *event;
> >> + struct axis_replacement axes[] = {
> >> + { ABS_DISTANCE, 10 },
> >> + { ABS_TILT_X, 0 },
> >> + { ABS_TILT_Y, 0 },
> >> + { -1, -1 }
> >> + };
> >> + const char *devnode;
> >> +
> >> + litest_tablet_proximity_in(dev, 10, 10, axes);
> >> +
> >> + /* for simplicity, we create a new litest context */
> >> + devnode = libevdev_uinput_get_devnode(dev->uinput);
> >> + li = litest_create_context();
> >> + libinput_path_add_device(li, devnode);
> >> +
> >> + litest_wait_for_event_of_type(li,
> >> + LIBINPUT_EVENT_DEVICE_ADDED,
> >> + -1);
> >> + event = libinput_get_event(li);
> >> + libinput_event_destroy(event);
> >> +
> >> + litest_assert_empty_queue(li);
> >> + litest_tablet_motion(dev, 10, 20, axes);
> >> + litest_tablet_motion(dev, 30, 40, axes);
> >> + litest_assert_empty_queue(li);
> >> + litest_event(dev, EV_KEY, BTN_STYLUS, 1);
> >> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> >> + litest_event(dev, EV_KEY, BTN_STYLUS, 1);
> >> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> >> + litest_assert_empty_queue(li);
> >> + litest_tablet_proximity_out(dev);
> >> + litest_assert_empty_queue(li);
> >> +
> >> + libinput_unref(li);
> >> +}
> >> +END_TEST
> >> +
> >> START_TEST(mouse_tool)
> >> {
> >> struct litest_device *dev = litest_current_device();
> >> @@ -1615,6 +1657,7 @@ main(int argc, char **argv)
> >> {
> >> litest_add("tablet:tool", tool_ref, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> >> litest_add_no_device("tablet:tool", tool_capabilities);
> >> + litest_add("tablet:tool", tool_in_prox_before_start, LITEST_TABLET, LITEST_ANY);
> >> litest_add("tablet:tool_serial", tool_serial, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> >> litest_add("tablet:tool_serial", serial_changes_tool, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> >> litest_add("tablet:tool_serial", invalid_serials, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> >> --
> >> 2.1.0
More information about the wayland-devel
mailing list