On Thu, Feb 26, 2015 at 1:31 PM, Benjamin Tissoires
<benjamin.tissoi...@gmail.com> wrote:
> On Thu, Feb 26, 2015 at 12:05 AM, Peter Hutterer
> <peter.hutte...@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 <killert...@gmail.com>
>> Signed-off-by: Peter Hutterer <peter.hutte...@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.

Cheers,

Ping

>> ---
>>  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
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to