From 27f7a66de4e705b2cd00d2d8ba499c9a5829fee0 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 9 Jul 2018 16:04:50 +1000 Subject: [PATCH] filter: add a trackpoint multiplier factor Measuring the trackpoint range has not shown to be sufficient or precise enough to be used as an ingredient for trackpoint acceleration. So let's just switch back to a generic multiplier that we can apply to the input deltas do undo any device-specific lack of scaling. Signed-off-by: Peter Hutterer --- doc/pointer-acceleration.dox | 3 + doc/svg/trackpoint-delta-illustration.svg | 126 +++++++++++ doc/trackpoints.dox | 248 ++++++++++++---------- meson.build | 1 + src/evdev.c | 61 ++---- src/evdev.h | 2 +- src/filter-trackpoint.c | 33 +-- src/filter.h | 2 +- src/libinput-util.h | 1 - src/quirks.c | 9 + src/quirks.h | 1 + tools/shared.c | 6 + 12 files changed, 320 insertions(+), 173 deletions(-) create mode 100644 doc/svg/trackpoint-delta-illustration.svg diff --git a/doc/pointer-acceleration.dox b/doc/pointer-acceleration.dox index 63b742ce..1d93dbdc 100644 --- a/doc/pointer-acceleration.dox +++ b/doc/pointer-acceleration.dox @@ -124,6 +124,9 @@ sensitivity in hardware by modifying a sysfs file on the serio node. A higher sensitivity results in higher deltas, thus changing the definition of what is a unit again. +libinput attempts to normalize unit data to the best of its abilities, see +@ref trackpoint_multiplier. Beyond this, it is not possible to have +consistent behavior across different touchpad devices. @image html ptraccel-trackpoint.svg "Pointer acceleration curves for trackpoints" diff --git a/doc/svg/trackpoint-delta-illustration.svg b/doc/svg/trackpoint-delta-illustration.svg new file mode 100644 index 00000000..8423dfa2 --- /dev/null +++ b/doc/svg/trackpoint-delta-illustration.svg @@ -0,0 +1,126 @@ + + + +Gnuplot +Produced by GNUPLOT 5.0 patchlevel 6 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + minimum possible interval + + + delta is 1 + + + + + + + + + + + physical pressure + + + + + time between events + + + + + time between events + + + + + + (x, y) delta values + + + (x, y) delta values + + + + + + + + + + + + + + + + + diff --git a/doc/trackpoints.dox b/doc/trackpoints.dox index 5cdc64d2..8bd6c680 100644 --- a/doc/trackpoints.dox +++ b/doc/trackpoints.dox @@ -26,128 +26,144 @@ multiple laptops. The values provided by a trackpoint are motion deltas, usually corresponding to the pressure applied to the trackstick. For example, pressure towards the -screen on a laptop provides negative y deltas at a fixed rate (e.g. every -10ms). As the pressure increases, the delta increases too. As the pressure -decreases, the delta decreases until it hits the neutral state. +screen on a laptop provides negative y deltas. The reporting rate increases +as the pressure increases and once events are reported at the maximum rate, +the delta values increase. The figure below shows a rough illustration of +this concept. As the pressure +decreases, the delta decrease first, then the reporting rate until the +trackpoint is in a neutral state and no events are reported. Trackpoint data +is hart to generalize, see + +Observations on trackpoint input data for more details. + +@image html trackpoint-delta-illustration.svg Illustration of the relationship between reporting rate and delta values on a trackpoint The delta range itself can vary greatly between laptops, some devices send a -maximum delta value of 30, others can go beyond 100. To normalize the motion -trackpoint, libinput uses the available delta range and fits its -acceleration curve into this range. This requires calibration by the user, -see @ref trackpoint_range_measure. +maximum delta value of 30, others can go beyond 100. However, the useful +delta range is a fraction of the maximum range. It is uncomfortable to exert +sufficient pressure to even get close to the maximum ranges. + +@section trackpoint_multiplier The magic trackpoint multiplier + +To accomodate for the wildly different input data on trackpoint, libinput +uses a multiplier that is applied to input deltas. Trackpoints that send +comparatively high deltas can be "slowed down", trackpoints that send low +deltas can be "sped up" to match the expected range. The actual acceleration +profile is applied to these pre-multiplied deltas. + +Given a trackpoint delta (dx, dy), a multiplier M and a pointer acceleration +function f(dx, dy) → (dx', dy'), the algorithm is effectively: +@verbatim +f(M * dx, M * dy) → (dx', dy') +@endverbatim + +The magic trackpoint multiplier **is not user visible configuration**. It is +part of the @ref device-quirks system and provided once per device. +User-specific preferences can be adjusted with the pointer acceleration speed +setting libinput_device_config_accel_set_speed(). + +@subsection trackpoint_multiplier_adjustment Adjusting the magic trackpoint multiplier + +This section only applies if: +- the trackpoint default speed (speed setting 0) is unusably slow or + unusably fast, **and** +- the lowest speed setting (-1) is still too fast **or** the highest speed + setting is still too slow, **and** +- the @ref device-quirks for this device do not list a trackpoint multiplier + (see @ref device-quirks-debugging) + +If the only satisfactory speed settings are less than -0.75 or greater than +0.75, a multiplier *may* be required. + +A specific multiplier will apply to **all users with the same laptop +model**, so proceed with caution. You must be capable/willing to adjust +device quirks, build libinput from source and restart the session frequently +to adjust the multiplier. If this does not apply, wait for someone else with +the same hardware to do this. + +Finding the correct multiplier is difficult and requires some trial and +error. The default multiplier is always 1.0. A value between 0.0 and 1.0 +slows the trackpoint down, a value above 1.0 speeds the trackpoint up. +Values below zero are invalid. + +@note The multiplier is not a configuration to adjust to personal +preferences. The multiplier normalizes the input data into a range that can +then be configured with the speed setting. + +To adjust the local multiplier, first @ref building_libinput +"build libinput from git master". It is not required to install libinput +from git. The below assumes that all @ref building_dependencies are already +installed. + +@verbatim +$ cd path/to/libinput.git + +# Use an approximate multiplier in the quirks file +$ cat > data/99-trackpont-override.quirks <estimate the appropriate trackpoint range. For example, let's look at -the histogram below: - -@verbatim -Histogram for x axis deltas, in counts of 5 - -30: - -29: - -28: + - -27: + - -26: ++ - -25: ++++ - -24: +++++ - -23: ++ - -22: ++++++ - -21: +++ - -20: ++++ - -19: +++++++ - -18: ++++++++++++ - -17: ++++++++++++ - -16: ++++++++++++ - -15: ++++ - -14: +++++ - -13: +++++ - -12: ++++++ - -11: +++++ - -10: +++ - -9: ++++ - -8: +++++++ - -7: +++++++ - -6: ++++++++++++ - -5: ++++++++++++ - -4: ++++++++++++ - -3: +++++++++ - -2: +++++++++ - -1: ++++++++ - 0: +++++++ - 1: +++++ - 2: +++++ - 3: ++++++ - 4: ++++++ - 5: +++++++ - 6: ++++ - 7: ++ - 8: +++ - 9: +++ - 10: +++ - 11: +++ - 12: +++ - 13: ++++ - 14: ++++++ - 15: ++++ - 16: ++++ - 17: ++++ - 18: ++++++ - 19: +++++++ - 20: ++++ - 21: ++++++ - 22: ++++++ - 23: ++++++ - 24: ++++++ - 25: +++++++++ - 26: +++++++ - 27: ++++++++ - 28: +++++ - 29: ++ - 30: ++ - 31: + - 32: - 33: - 34: -@endverbatim - -The 0 delta is the neutral state, each + represents 5 events with that -delta value. Note how the curve is distributed, it's not a classic bell -curve. That can be a factor of the input provided or the firmware-based -pointer acceleration. - -Overall, the majority of events appear to be in the 0-25 range with a few -outliers. So the trackpoint range libinput should use for this particular -device would be 25. Note how there is a fair bit of guesswork involved, a -trackpoint's data is never clean enough to get a definitive value. It is -generally better to take a (slightly) smaller range than one too large. - -The device quirk set is `AttrTrackpointRange=25`. See @ref -device-quirks for details on how to apply device quirks. +If using libinput version 1.11.x or earlier, please see + +the 1.11.0 documentation */ diff --git a/meson.build b/meson.build index 7d21f364..5d146a62 100644 --- a/meson.build +++ b/meson.build @@ -421,6 +421,7 @@ if get_option('documentation') 'doc/svg/thumb-detection.svg', 'doc/svg/top-software-buttons.svg', 'doc/svg/touchscreen-gestures.svg', + 'doc/svg/trackpoint-delta-illustration.svg', 'doc/svg/twofinger-scrolling.svg', # style files 'doc/style/header.html', diff --git a/src/evdev.c b/src/evdev.c index 9fb11d83..2b729342 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -956,7 +956,7 @@ evdev_init_accel(struct evdev_device *device, if (which == LIBINPUT_CONFIG_ACCEL_PROFILE_FLAT) filter = create_pointer_accelerator_filter_flat(device->dpi); else if (device->tags & EVDEV_TAG_TRACKPOINT) - filter = create_pointer_accelerator_filter_trackpoint(device->trackpoint_range); + filter = create_pointer_accelerator_filter_trackpoint(device->trackpoint_multiplier); else if (device->dpi < DEFAULT_MOUSE_DPI) filter = create_pointer_accelerator_filter_linear_low_dpi(device->dpi); else @@ -1176,59 +1176,36 @@ evdev_read_wheel_tilt_props(struct evdev_device *device) return flags; } -static inline int -evdev_get_trackpoint_range(struct evdev_device *device) +static inline double +evdev_get_trackpoint_multiplier(struct evdev_device *device) { struct quirks_context *quirks; struct quirks *q; - const char *prop; - uint32_t range = DEFAULT_TRACKPOINT_RANGE; + double multiplier = 1.0; if (!(device->tags & EVDEV_TAG_TRACKPOINT)) - return DEFAULT_TRACKPOINT_RANGE; + return 1.0; quirks = evdev_libinput_context(device)->quirks; q = quirks_fetch_for_device(quirks, device->udev_device); - if (q && quirks_get_uint32(q, QUIRK_ATTR_TRACKPOINT_RANGE, &range)) { - goto out; + if (q) { + quirks_get_double(q, QUIRK_ATTR_TRACKPOINT_MULTIPLIER, &multiplier); + quirks_unref(q); } - evdev_log_info(device, - "trackpoint does not have a specified range, " - "guessing... see %strackpoints.html\n", - HTTP_DOC_LINK); - - prop = udev_device_get_property_value(device->udev_device, - "POINTINGSTICK_SENSITIVITY"); - if (prop) { - int sensitivity; - - if (!safe_atoi(prop, &sensitivity) || - (sensitivity < 0.0 || sensitivity > 255)) { - evdev_log_error(device, - "trackpoint sensitivity property is present but invalid, " - "using %d instead\n", - DEFAULT_TRACKPOINT_SENSITIVITY); - sensitivity = DEFAULT_TRACKPOINT_SENSITIVITY; - } - range = 1.0 * DEFAULT_TRACKPOINT_RANGE * - sensitivity/DEFAULT_TRACKPOINT_SENSITIVITY; - - evdev_log_debug(device, - "trackpoint udev sensitivity is %d\n", - sensitivity); + if (multiplier <= 0.0) { + evdev_log_bug_libinput(device, + "trackpoint multiplier %.2f is invalid\n", + multiplier); + multiplier = 1.0; } -out: - quirks_unref(q); + if (multiplier != 1.0) + evdev_log_info(device, + "trackpoint device set to range %.2f\n", + multiplier); - if (range == 0) { - evdev_log_bug_libinput(device, "trackpoint range is zero\n"); - range = DEFAULT_TRACKPOINT_RANGE; - } - - evdev_log_info(device, "trackpoint device set to range %d\n", range); - return range; + return multiplier; } static inline int @@ -1749,7 +1726,7 @@ evdev_configure_device(struct evdev_device *device) evdev_tag_external_mouse(device, device->udev_device); evdev_tag_trackpoint(device, device->udev_device); device->dpi = evdev_read_dpi_prop(device); - device->trackpoint_range = evdev_get_trackpoint_range(device); + device->trackpoint_multiplier = evdev_get_trackpoint_multiplier(device); device->seat_caps |= EVDEV_DEVICE_POINTER; diff --git a/src/evdev.h b/src/evdev.h index 82094824..3757dff3 100644 --- a/src/evdev.h +++ b/src/evdev.h @@ -189,7 +189,7 @@ struct evdev_device { bool is_mt; bool is_suspended; int dpi; /* HW resolution */ - int trackpoint_range; /* trackpoint max delta */ + double trackpoint_multiplier; /* trackpoint constant multiplier */ struct ratelimit syn_drop_limit; /* ratelimit for SYN_DROPPED logging */ struct ratelimit nonpointer_rel_limit; /* ratelimit for REL_* events from non-pointer devices */ uint32_t model_flags; diff --git a/src/filter-trackpoint.c b/src/filter-trackpoint.c index ebc7ee12..1b926268 100644 --- a/src/filter-trackpoint.c +++ b/src/filter-trackpoint.c @@ -41,6 +41,8 @@ struct trackpoint_accelerator { struct pointer_trackers trackers; double speed_factor; + + double multiplier; }; double @@ -76,16 +78,20 @@ trackpoint_accelerator_filter(struct motion_filter *filter, { struct trackpoint_accelerator *accel_filter = (struct trackpoint_accelerator *)filter; + struct device_float_coords multiplied; struct normalized_coords coords; double f; double velocity; - trackers_feed(&accel_filter->trackers, unaccelerated, time); + multiplied.x = unaccelerated->x * accel_filter->multiplier; + multiplied.y = unaccelerated->y * accel_filter->multiplier; + + trackers_feed(&accel_filter->trackers, &multiplied, time); velocity = trackers_velocity(&accel_filter->trackers, time); f = trackpoint_accel_profile(filter, data, velocity, time); - coords.x = unaccelerated->x * f; - coords.y = unaccelerated->y * f; + coords.x = multiplied.x * f; + coords.y = multiplied.y * f; return coords; } @@ -95,11 +101,12 @@ trackpoint_accelerator_filter_noop(struct motion_filter *filter, const struct device_float_coords *unaccelerated, void *data, uint64_t time) { - + struct trackpoint_accelerator *accel_filter = + (struct trackpoint_accelerator *)filter; struct normalized_coords coords; - coords.x = unaccelerated->x; - coords.y = unaccelerated->y; + coords.x = unaccelerated->x * accel_filter->multiplier; + coords.y = unaccelerated->y * accel_filter->multiplier; return coords; } @@ -175,27 +182,29 @@ struct motion_filter_interface accelerator_interface_trackpoint = { }; struct motion_filter * -create_pointer_accelerator_filter_trackpoint(int max_hw_delta) +create_pointer_accelerator_filter_trackpoint(double multiplier) { struct trackpoint_accelerator *filter; + assert(multiplier > 0.0); + /* Trackpoints are special. They don't have a movement speed like a * mouse or a finger, instead they send a stream of events based on * the pressure applied. * * Physical ranges on a trackpoint are the max values for relative - * deltas, but these are highly device-specific. + * deltas, but these are highly device-specific and unreliable to + * measure. * + * Instead, we just have a constant multiplier we have in the quirks + * system. */ filter = zalloc(sizeof *filter); if (!filter) return NULL; - /* FIXME: should figure out something here to deal with the - * trackpoint range/max hw delta. Or we just make it a literal - * "magic" number and live with it. - */ + filter->multiplier = multiplier; trackers_init(&filter->trackers); diff --git a/src/filter.h b/src/filter.h index 3865b819..7e001d2f 100644 --- a/src/filter.h +++ b/src/filter.h @@ -122,7 +122,7 @@ struct motion_filter * create_pointer_accelerator_filter_lenovo_x230(int dpi); struct motion_filter * -create_pointer_accelerator_filter_trackpoint(int max_delta); +create_pointer_accelerator_filter_trackpoint(double multiplier); struct motion_filter * create_pointer_accelerator_filter_tablet(int xres, int yres); diff --git a/src/libinput-util.h b/src/libinput-util.h index fadf11c2..c68b888a 100644 --- a/src/libinput-util.h +++ b/src/libinput-util.h @@ -54,7 +54,6 @@ /* The HW DPI rate we normalize to before calculating pointer acceleration */ #define DEFAULT_MOUSE_DPI 1000 -#define DEFAULT_TRACKPOINT_RANGE 20 #define DEFAULT_TRACKPOINT_SENSITIVITY 128 #define ANSI_HIGHLIGHT "\x1B[0;1;39m" diff --git a/src/quirks.c b/src/quirks.c index 8d0d6847..76639f05 100644 --- a/src/quirks.c +++ b/src/quirks.c @@ -265,6 +265,7 @@ quirk_get_name(enum quirk q) case QUIRK_ATTR_PALM_PRESSURE_THRESHOLD: return "AttrPalmPressureThreshold"; case QUIRK_ATTR_RESOLUTION_HINT: return "AttrResolutionHint"; case QUIRK_ATTR_TRACKPOINT_RANGE: return "AttrTrackpointRange"; + case QUIRK_ATTR_TRACKPOINT_MULTIPLIER: return "AttrTrackpointMultiplier"; case QUIRK_ATTR_THUMB_PRESSURE_THRESHOLD: return "AttrThumbPressureThreshold"; default: abort(); @@ -642,6 +643,7 @@ parse_attr(struct quirks_context *ctx, struct quirk_dimensions dim; struct quirk_range range; unsigned int v; + double d; if (streq(key, quirk_get_name(QUIRK_ATTR_SIZE_HINT))) { p->id = QUIRK_ATTR_SIZE_HINT; @@ -714,6 +716,13 @@ parse_attr(struct quirks_context *ctx, p->type = PT_UINT; p->value.u = v; rc = true; + } else if (streq(key, quirk_get_name(QUIRK_ATTR_TRACKPOINT_MULTIPLIER))) { + p->id = QUIRK_ATTR_TRACKPOINT_MULTIPLIER; + if (!safe_atod(value, &d)) + goto out; + p->type = PT_DOUBLE; + p->value.d = d; + rc = true; } else if (streq(key, quirk_get_name(QUIRK_ATTR_THUMB_PRESSURE_THRESHOLD))) { p->id = QUIRK_ATTR_THUMB_PRESSURE_THRESHOLD; if (!safe_atou(value, &v)) diff --git a/src/quirks.h b/src/quirks.h index df8b7dd3..e10c3ed1 100644 --- a/src/quirks.h +++ b/src/quirks.h @@ -94,6 +94,7 @@ enum quirk { QUIRK_ATTR_PALM_PRESSURE_THRESHOLD, QUIRK_ATTR_RESOLUTION_HINT, QUIRK_ATTR_TRACKPOINT_RANGE, + QUIRK_ATTR_TRACKPOINT_MULTIPLIER, QUIRK_ATTR_THUMB_PRESSURE_THRESHOLD, }; diff --git a/tools/shared.c b/tools/shared.c index fa03cc75..92c362b8 100644 --- a/tools/shared.c +++ b/tools/shared.c @@ -616,6 +616,7 @@ tools_list_device_quirks(struct quirks_context *ctx, QUIRK_ATTR_PALM_PRESSURE_THRESHOLD, QUIRK_ATTR_RESOLUTION_HINT, QUIRK_ATTR_TRACKPOINT_RANGE, + QUIRK_ATTR_TRACKPOINT_MULTIPLIER, QUIRK_ATTR_THUMB_PRESSURE_THRESHOLD, }; enum quirk *q; @@ -696,6 +697,11 @@ tools_list_device_quirks(struct quirks_context *ctx, snprintf(buf, sizeof(buf), "%s=%s", name, s); callback(userdata, buf); break; + case QUIRK_ATTR_TRACKPOINT_MULTIPLIER: + quirks_get_double(quirks, *q, &d); + snprintf(buf, sizeof(buf), "%s=%0.2f\n", name, d); + callback(userdata, buf); + break; } }