diff --git a/meson.build b/meson.build index 3efa9293..622203f9 100644 --- a/meson.build +++ b/meson.build @@ -375,6 +375,7 @@ install_headers('src/libinput.h') src_libinput = src_libfilter + [ 'src/libinput.c', 'src/libinput-plugin.c', + 'src/libinput-plugin-tablet-double-tool.c', 'src/libinput-private-config.c', 'src/evdev.c', 'src/evdev-debounce.c', diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c index 19d9af28..44057024 100644 --- a/src/evdev-tablet.c +++ b/src/evdev-tablet.c @@ -2021,7 +2021,7 @@ tablet_proximity_out_quirk_set_timer(struct tablet_dispatch *tablet, time + FORCED_PROXOUT_TIMEOUT); } -static bool +static void tablet_update_tool_state(struct tablet_dispatch *tablet, struct evdev_device *device, uint64_t time) @@ -2029,7 +2029,6 @@ tablet_update_tool_state(struct tablet_dispatch *tablet, enum libinput_tablet_tool_type type; uint32_t changed; int state; - uint32_t doubled_up_new_tool_bit = 0; /* we were already out of proximity but now got a tool update but * our tool state is zero - i.e. we got a valid prox out from the @@ -2067,39 +2066,7 @@ tablet_update_tool_state(struct tablet_dispatch *tablet, } if (tablet->tool_state == tablet->prev_tool_state) - return false; - - /* Kernel tools are supposed to be mutually exclusive, but we may have - * two bits set due to firmware/kernel bugs. - * Two cases that have been seen in the wild: - * - BTN_TOOL_PEN on proximity in, followed by - * BTN_TOOL_RUBBER later, see #259 - * -> We force a prox-out of the pen, trigger prox-in for eraser - * - BTN_TOOL_RUBBER on proximity in, but BTN_TOOL_PEN when - * the tip is down, see #702. - * -> We ignore BTN_TOOL_PEN - * In both cases the eraser is what we want, so we bias - * towards that. - */ - if (tablet->tool_state & (tablet->tool_state - 1)) { - doubled_up_new_tool_bit = tablet->tool_state ^ tablet->prev_tool_state; - - /* The new tool is the pen. Ignore it */ - if (doubled_up_new_tool_bit == bit(LIBINPUT_TABLET_TOOL_TYPE_PEN)) { - tablet->tool_state &= ~bit(LIBINPUT_TABLET_TOOL_TYPE_PEN); - return false; - } - - /* The new tool is some tool other than pen (usually eraser). - * We set the current tool state to zero, thus setting - * everything up for a prox out on the tool. Once that is set - * up, we change the tool state to be the new one we just got. - * When we re-process this function we now get the new tool - * as prox in. Importantly, we basically rely on nothing else - * happening in the meantime. - */ - tablet->tool_state = 0; - } + return; changed = tablet->tool_state ^ tablet->prev_tool_state; type = ffs(changed) - 1; @@ -2126,12 +2093,6 @@ tablet_update_tool_state(struct tablet_dispatch *tablet, } tablet->prev_tool_state = tablet->tool_state; - - if (doubled_up_new_tool_bit) { - tablet->tool_state = doubled_up_new_tool_bit; - return true; /* need to re-process */ - } - return false; } static struct libinput_tablet_tool * @@ -2194,10 +2155,8 @@ tablet_flush(struct tablet_dispatch *tablet, uint64_t time) { struct libinput_tablet_tool *tool; - bool process_tool_twice; -reprocess: - process_tool_twice = tablet_update_tool_state(tablet, device, time); + tablet_update_tool_state(tablet, device, time); tool = tablet_get_current_tool(tablet); if (!tool) @@ -2271,9 +2230,6 @@ reprocess: tablet_change_area(device); tablet_history_reset(tablet); } - - if (process_tool_twice) - goto reprocess; } static inline void diff --git a/src/libinput-plugin-tablet-double-tool.c b/src/libinput-plugin-tablet-double-tool.c new file mode 100644 index 00000000..398bcd20 --- /dev/null +++ b/src/libinput-plugin-tablet-double-tool.c @@ -0,0 +1,371 @@ +/* + * Copyright © 2025 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include "config.h" + +#include +#include + +#include "util-mem.h" +#include "util-strings.h" + +#include "evdev-frame.h" + +#include "libinput-log.h" +#include "libinput-util.h" +#include "libinput-plugin.h" +#include "libinput-plugin-tablet-double-tool.h" + +enum { + TOOL_PEN_DOWN, + TOOL_PEN_UP, + TOOL_ERASER_DOWN, + TOOL_ERASER_UP, + TOOL_DOUBLE_TOOL, +}; + +enum tool_filter { + SKIP_PEN = bit(1), + SKIP_ERASER = bit(2), + PEN_IN_PROX = bit(3), + PEN_OUT_OF_PROX = bit(4), + ERASER_IN_PROX = bit(5), + ERASER_OUT_OF_PROX = bit(6), +}; + +struct plugin_device { + struct list link; + struct libinput_device *device; + bool ignore_pen; + bitmask_t tools_seen; + + int pen_value; + int eraser_value; +}; + +struct plugin_data { + struct list devices; +}; + +static void +plugin_device_destroy(struct plugin_device *device) +{ + libinput_device_unref(device->device); + list_remove(&device->link); + free(device); +} + +static void +plugin_data_destroy(void *d) +{ + struct plugin_data *data = d; + + struct plugin_device *device; + list_for_each_safe(device, &data->devices, link) { + plugin_device_destroy(device); + } + + free(data); +} + +DEFINE_DESTROY_CLEANUP_FUNC(plugin_data); + +static void +plugin_destroy(struct libinput_plugin *libinput_plugin) +{ + struct plugin_data *plugin = libinput_plugin_get_user_data(libinput_plugin); + plugin_data_destroy(plugin); +} + +static struct evdev_frame * +double_tool_plugin_filter_frame(struct libinput_plugin *plugin, + struct evdev_frame *frame_in, + enum tool_filter filter) +{ + size_t nevents; + struct evdev_event *events = evdev_frame_get_events(frame_in, &nevents); + + /* +2 because we may add BTN_TOOL_PEN and BTN_TOOL_RUBBER */ + struct evdev_frame *frame_out = evdev_frame_new(nevents + 2); + evdev_frame_set_time(frame_out, evdev_frame_get_time(frame_in)); + + for (size_t i = 0; i < nevents; i++) { + struct evdev_event *event = &events[i]; + + switch (evdev_usage_enum(event->usage)) { + case EVDEV_BTN_TOOL_PEN: + case EVDEV_BTN_TOOL_RUBBER: + /* skip */ + break; + default: + evdev_frame_append(frame_out, event, 1); + } + } + + if (filter & (PEN_IN_PROX|PEN_OUT_OF_PROX)) { + struct evdev_event event = { + .usage = evdev_usage_from(EVDEV_BTN_TOOL_PEN), + .value = (filter & PEN_IN_PROX) ? 1 : 0, + }; + evdev_frame_append(frame_out, &event, 1); + } + if (filter & (ERASER_IN_PROX|ERASER_OUT_OF_PROX)) { + struct evdev_event event = { + .usage = evdev_usage_from(EVDEV_BTN_TOOL_RUBBER), + .value = (filter & ERASER_IN_PROX) ? 1 : 0, + }; + evdev_frame_append(frame_out, &event, 1); + } + + return frame_out; +} + +/* Kernel tools are supposed to be mutually exclusive, but we may have + * two bits set due to firmware/kernel bugs. + * Two cases that have been seen in the wild: + * - BTN_TOOL_PEN on proximity in, followed by + * BTN_TOOL_RUBBER later, see #259 + * -> We force a prox-out of the pen, trigger prox-in for eraser + * - BTN_TOOL_RUBBER on proximity in, but BTN_TOOL_PEN when + * the tip is down, see #702. + * -> We ignore BTN_TOOL_PEN + * In both cases the eraser is what we want, so we bias + * towards that. + */ +static void +double_tool_plugin_device_handle_frame(struct libinput_plugin *libinput_plugin, + struct plugin_device *device, + struct evdev_frame *frame) +{ + size_t nevents; + struct evdev_event *events = evdev_frame_get_events(frame, &nevents); + + const struct evdev_event *eraser_toggle = NULL; + const struct evdev_event *pen_toggle = NULL; + + for (size_t i = 0; i < nevents; i++) { + struct evdev_event *event = &events[i]; + + switch (evdev_usage_enum(event->usage)) { + case EVDEV_BTN_TOOL_RUBBER: + eraser_toggle = event; + device->eraser_value = event->value; + break; + case EVDEV_BTN_TOOL_PEN: + pen_toggle = event; + device->pen_value = event->value; + break; + default: + break; + } + } + + bool eraser_is_down = !!device->eraser_value; + bool pen_is_down = !!device->pen_value; + bool eraser_toggled = eraser_toggle != NULL; + bool pen_toggled = pen_toggle != NULL; + +#if EVENT_DEBUGGING + plugin_log_debug(libinput_plugin, + "device %s: tool state: pen:%s eraser:%s\n", + libinput_device_get_name(device->device), + pen_toggled ? (pen_is_down ? "↓" : "↑") : pen_is_down ? "|" : ".", + eraser_toggled ? (eraser_is_down ? "↓" : "↑") : eraser_is_down ? "|" : "."); +#endif + + if (!bitmask_bit_is_set(device->tools_seen, TOOL_DOUBLE_TOOL)) { + bitmask_t tool_mask = bitmask_from_bits(TOOL_PEN_DOWN, + TOOL_PEN_UP, + TOOL_ERASER_DOWN, + TOOL_ERASER_UP); + if (eraser_toggled) { + if (eraser_is_down) + bitmask_set_bit(&device->tools_seen, TOOL_ERASER_DOWN); + else + bitmask_set_bit(&device->tools_seen, TOOL_ERASER_UP); + } + if (pen_toggled) { + if (pen_is_down) + bitmask_set_bit(&device->tools_seen, TOOL_PEN_DOWN); + else + bitmask_set_bit(&device->tools_seen, TOOL_PEN_UP); + } + /* If we successfully get all four tool events without + * a doubled-up tool, assume the device is sane and + * unregister this device */ + if (bitmask_all(device->tools_seen, tool_mask)) { + plugin_log_debug(libinput_plugin, + "device %s: device is fine, unregistering device\n", + libinput_device_get_name(device->device)); + plugin_device_destroy(device); + return; + } + } + + /* rubber after pen */ + if (eraser_toggled) { + if (eraser_is_down && pen_is_down) { + if (!pen_toggled) { + _unref_(evdev_frame) *pen_out_of_prox = + double_tool_plugin_filter_frame(libinput_plugin, + frame, + SKIP_ERASER|PEN_OUT_OF_PROX); + libinput_plugin_prepend_evdev_frame(libinput_plugin, + device->device, + pen_out_of_prox); + } + + _unref_(evdev_frame) *eraser_in_prox = + double_tool_plugin_filter_frame(libinput_plugin, + frame, + SKIP_PEN|ERASER_IN_PROX); + + libinput_plugin_prepend_evdev_frame(libinput_plugin, + device->device, + eraser_in_prox); + device->ignore_pen = true; + + bitmask_set_bit(&device->tools_seen, TOOL_DOUBLE_TOOL); + + /* discard the original frame */ + evdev_frame_reset(frame); + + return; + } else if (!eraser_is_down) { + _unref_(evdev_frame) *eraser_out_of_prox = + double_tool_plugin_filter_frame(libinput_plugin, + frame, + SKIP_PEN|ERASER_OUT_OF_PROX); + + libinput_plugin_prepend_evdev_frame(libinput_plugin, + device->device, + eraser_out_of_prox); + + /* Only revert back to the pen if the pen was actually toggled in this frame, + * otherwise it's just still set from before */ + if (pen_toggled && pen_is_down) { + _unref_(evdev_frame) *pen_in_prox = + double_tool_plugin_filter_frame(libinput_plugin, + frame, + SKIP_ERASER|PEN_IN_PROX); + libinput_plugin_prepend_evdev_frame(libinput_plugin, + device->device, + pen_in_prox); + } + + device->ignore_pen = false; + + /* discard the original frame */ + evdev_frame_reset(frame); + + return; + } + } + + /* pen after rubber */ + if (pen_toggled && eraser_is_down) { + device->ignore_pen = true; + } + + if (device->ignore_pen) { + _unref_(evdev_frame) *frame_out = + double_tool_plugin_filter_frame(libinput_plugin, frame, SKIP_PEN); + size_t out_nevents; + evdev_frame_set(frame, + evdev_frame_get_events(frame_out, &out_nevents), + nevents); + bitmask_set_bit(&device->tools_seen, TOOL_DOUBLE_TOOL); + } else if (pen_is_down) { + _unref_(evdev_frame) *frame_out = + double_tool_plugin_filter_frame(libinput_plugin, frame, PEN_IN_PROX); + size_t out_nevents; + evdev_frame_set(frame, + evdev_frame_get_events(frame_out, &out_nevents), + nevents); + } +} + +static void +double_tool_plugin_evdev_frame(struct libinput_plugin *libinput_plugin, + struct libinput_device *device, + struct evdev_frame *frame) +{ + struct plugin_data *plugin = libinput_plugin_get_user_data(libinput_plugin); + struct plugin_device *pd; + + list_for_each(pd, &plugin->devices, link) { + if (pd->device == device) { + double_tool_plugin_device_handle_frame(libinput_plugin, pd, frame); + break; + } + } +} + +static void +double_tool_plugin_device_added(struct libinput_plugin *libinput_plugin, + struct libinput_device *device) +{ + if (!libinput_device_has_capability(device, LIBINPUT_DEVICE_CAP_TABLET_TOOL)) + return; + + struct plugin_data *plugin = libinput_plugin_get_user_data(libinput_plugin); + struct plugin_device *pd = zalloc(sizeof(*pd)); + pd->device = libinput_device_ref(device); + list_take_append(&plugin->devices, pd, link); +} + +static void +double_tool_plugin_device_removed(struct libinput_plugin *libinput_plugin, + struct libinput_device *device) +{ + struct plugin_data *plugin = libinput_plugin_get_user_data(libinput_plugin); + struct plugin_device *dev; + list_for_each_safe(dev, &plugin->devices, link) { + if (dev->device == device) { + plugin_device_destroy(dev); + return; + } + } +} + +static const struct libinput_plugin_interface interface = { + .run = NULL, + .destroy = plugin_destroy, + .device_new = NULL, + .device_ignored = NULL, + .device_added = double_tool_plugin_device_added, + .device_removed = double_tool_plugin_device_removed, + .evdev_frame = double_tool_plugin_evdev_frame, +}; + +void +libinput_tablet_plugin_double_tool(struct libinput *libinput) +{ + _destroy_(plugin_data) *plugin = zalloc(sizeof(*plugin)); + list_init(&plugin->devices); + + _unref_(libinput_plugin) *p = libinput_plugin_new(libinput, + "tablet-double-tool", + &interface, + steal(&plugin)); +} diff --git a/src/libinput-plugin-tablet-double-tool.h b/src/libinput-plugin-tablet-double-tool.h new file mode 100644 index 00000000..a17bc3ec --- /dev/null +++ b/src/libinput-plugin-tablet-double-tool.h @@ -0,0 +1,30 @@ +/* + * Copyright © 2025 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include "config.h" + +#include "libinput.h" +#include "libinput-plugin.h" + +void +libinput_tablet_plugin_double_tool(struct libinput *libinput); diff --git a/src/libinput-plugin.c b/src/libinput-plugin.c index 493e821a..0b5f39d0 100644 --- a/src/libinput-plugin.c +++ b/src/libinput-plugin.c @@ -36,6 +36,7 @@ #include "libinput-util.h" #include "libinput-private.h" +#include "libinput-plugin-tablet-double-tool.h" #include "evdev-plugin.h" @@ -359,6 +360,10 @@ void libinput_plugin_system_load_internal_plugins(struct libinput *libinput, struct libinput_plugin_system *system) { + /* FIXME: this should really be one of the first in the sequence + * so plugins don't have to take care of this? */ + libinput_tablet_plugin_double_tool(libinput); + /* Our own event dispatch is implemented as mini-plugin, * guarantee this one to always be last (and after any * other plugins have run so none of the devices are diff --git a/test/test-tablet.c b/test/test-tablet.c index 8a1acb8e..a912c2c5 100644 --- a/test/test-tablet.c +++ b/test/test-tablet.c @@ -2974,10 +2974,12 @@ START_TEST(tool_direct_switch_skip_tool_update) libinput_tablet_tool_ref(pen); libinput_event_destroy(event); + litest_checkpoint("Switching directly to eraser"); litest_event(dev, EV_KEY, BTN_TOOL_RUBBER, 1); litest_event(dev, EV_SYN, SYN_REPORT, 0); litest_dispatch(li); + litest_checkpoint("Expecting pen prox out followed by eraser prox in "); event = libinput_get_event(li); tev = litest_is_proximity_event(event, LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_OUT); @@ -3002,6 +3004,7 @@ START_TEST(tool_direct_switch_skip_tool_update) eraser); libinput_event_destroy(event); + litest_checkpoint("Switching directly to pen, expecting eraser prox out"); litest_event(dev, EV_KEY, BTN_TOOL_RUBBER, 0); litest_event(dev, EV_SYN, SYN_REPORT, 0); litest_dispatch(li); @@ -3014,6 +3017,7 @@ START_TEST(tool_direct_switch_skip_tool_update) libinput_event_destroy(event); litest_with_event_frame(dev) { + litest_checkpoint("Prox in for eraser (pen still in prox)"); litest_event(dev, EV_KEY, BTN_TOOL_RUBBER, 1); litest_tablet_motion(dev, 30, 40, axes); }