On Wed, Jun 01, 2016 at 08:41:17AM -0500, Yong Bakos wrote: > On May 31, 2016, at 7:42 PM, Peter Hutterer <[email protected]> wrote: > > > > Quite a few bugs are caused by touchpad ranges being out of whack. If we get > > input events significantly outside the expected range (5% width/height as > > error margin) print a warning to the log. > > > > And add a new doc page to explain what is happening and how to fix it. > > > > Signed-off-by: Peter Hutterer <[email protected]> > > Hi Peter, > FWIW, one minor nit (whildcard) I noticed, inline below.
fixed, thanks. Cheers, Peter > > --- > > doc/Makefile.am | 1 + > > doc/absolute-coordinate-ranges.dox | 120 > > +++++++++++++++++++++++++++++++++++++ > > doc/page-hierarchy.dox | 1 + > > src/evdev-mt-touchpad.c | 59 ++++++++++++++++++ > > src/evdev-mt-touchpad.h | 5 ++ > > 5 files changed, 186 insertions(+) > > create mode 100644 doc/absolute-coordinate-ranges.dox > > > > diff --git a/doc/Makefile.am b/doc/Makefile.am > > index f2a47cb..1e501a8 100644 > > --- a/doc/Makefile.am > > +++ b/doc/Makefile.am > > @@ -11,6 +11,7 @@ header_files = \ > > $(top_srcdir)/src/libinput.h \ > > $(top_srcdir)/README.txt \ > > $(srcdir)/absolute-axes.dox \ > > + $(srcdir)/absolute-coordinate-ranges.dox \ > > $(srcdir)/clickpad-softbuttons.dox \ > > $(srcdir)/device-configuration-via-udev.dox \ > > $(srcdir)/faqs.dox \ > > diff --git a/doc/absolute-coordinate-ranges.dox > > b/doc/absolute-coordinate-ranges.dox > > new file mode 100644 > > index 0000000..f9d9e98 > > --- /dev/null > > +++ b/doc/absolute-coordinate-ranges.dox > > @@ -0,0 +1,120 @@ > > +/** > > +@page absolute_coordinate_ranges Coordinate ranges for absolute axes > > + > > +libinput requires that all touchpads provide a correct axis range and > > +resolution. These are used to enable or disable certain features or adapt > > +the interaction with the touchpad. For example, the software button area is > > +narrower on small touchpads to avoid reducing the interactive surface too > > +much. Likewise, palm detection works differently on small touchpads as palm > > +interference is less likely to happen. > > + > > +Touchpads with incorrect axis ranges generate error messages > > +in the form: > > +<blockquote> > > +Axis 0x35 value 4000 is outside expected range [0, 3000] > > +</blockquote> > > + > > +This error message indicates that the ABS_MT_POSITION_X axis (i.e. the x > > +axis) generated an event outside the expected range of 0-3000. In this case > > +the value was 4000. > > +This discrepancy between the coordinate range the kernels advertises vs. > > +what the touchpad sends can be the source of a number of perceived > > +bugs in libinput. > > + > > +@section absolute_coordinate_ranges_fix Measuring and fixing touchpad > > ranges > > + > > +To fix the touchpad you need to: > > +-# measure the physical size of your touchpad in mm > > +-# run touchpad-edge-detector > > +-# trim the udev match rule to something sensible > > +-# replace the resolution with the calculated resolution based on physical > > + settings > > +-# test locally > > +-# send a patch to the systemd project > > + > > +Detailed explanations are below. > > + > > +[libevdev](http://freedesktop.org/wiki/Software/libevdev/) provides a tool > > +called **touchpad-edge-detector** that allows measuring the touchpad's > > input > > +ranges. Run the tool as root against the device node of your touchpad > > device > > +and repeatedly move a finger around the whole outside area of the > > +touchpad. Then control+c the process and note the output. > > +An example output is below: > > + > > +@code > > +$> sudo touchpad-edge-detector /dev/input/event4 > > +Touchpad SynPS/2 Synaptics TouchPad on /dev/input/event4 > > +Move one finger around the touchpad to detect the actual edges > > +Kernel says: x [1024..3112], y [2024..4832] > > +Touchpad sends: x [2445..4252], y [3464..4071] > > + > > +Touchpad size as listed by the kernel: 49x66mm > > +Calculate resolution as: > > + x axis: 2088/<width in mm> > > + y axis: 2808/<height in mm> > > + > > +Suggested udev rule: > > +# <Laptop model description goes here> > > +evdev:name:SynPS/2 Synaptics > > TouchPad:dmi:bvnLENOVO:bvrGJET72WW(2.22):bd02/21/2014:svnLENOVO:pn20ARS25701:pvrThinkPadT440s:rvnLENOVO:rn20ARS25701:rvrSDK0E50512STD:cvnLENOVO:ct10:cvrNotAvailable:* > > + EVDEV_ABS_00=2445:4252:<x resolution> > > + EVDEV_ABS_01=3464:4071:<y resolution> > > + EVDEV_ABS_35=2445:4252:<x resolution> > > + EVDEV_ABS_36=3464:4071:<y resolution> > > + > > +@endcode > > + > > +Note the discrepancy between the coordinate range the kernels advertises > > vs. > > +what the touchpad sends. > > +To fix the advertised ranges, the udev rule should be taken and trimmed > > +before being sent to the [systemd > > project](https://github.com/systemd/systemd). > > +An example commit can be found > > +[here](https://github.com/systemd/systemd/commit/26f667eac1c5e89b689aa0a1daef6a80f473e045). > > + > > +In most cases the match can and should be trimmed to the system vendor > > (svn) > > +and the product version (pvr), with everything else replaced by a wildcard > > +(*). In this case, a Lenovo T440s, a suitable match string would be: @code > > +evdev:name:SynPS/2 Synaptics TouchPad:dmi:*svnLENOVO:*pvrThinkPadT440s* > > +@endcode > > + > > +@note hwdb match strings only allow for alphanumeric ascii characters. Use > > a > > +whildcard (* or ?, whichever appropriate) for special characters. > > wildcard > > > > > + > > +The actual axis overrides are in the form: > > +@code > > +# axis number=min:max:resolution > > + EVDEV_ABS_00=2445:4252:42 > > +@endcode > > +or, if the range is correct but the resolution is wrong > > +@code > > +# axis number=::resolution > > + EVDEV_ABS_00=::42 > > +@endcode > > + > > +Note the leading single space. The axis numbers are in hex and can be found > > +in *linux/input-event-codes.h*. For touchpads ABS_X, ABS_Y, > > +ABS_MT_POSITION_X and ABS_MT_POSITION_Y are required. > > + > > +@note The touchpad's ranges and/or resolution should only be fixed when > > +there is a significant discrepancy. A few units do not make a difference > > and > > +a resolution that is off by 2 or less usually does not matter either. > > + > > +Once a match and override rule has been found, follow the instructions at > > +the top of the > > +[60-evdev.hwdb](https://github.com/systemd/systemd/blob/master/hwdb/60-evdev.hwdb) > > +file to save it locally and trigger the udev hwdb reload. Rebooting is > > +always a good idea. If the match string is correct, the new properties will > > +show up in the > > +output of > > +@code > > + udevadm info /sys/class/input/event4 > > +@endcode > > + > > +Adjust the command for the event node of your touchpad. > > +A udev builtin will apply the new axis ranges automatically. > > + > > +When the axis override is confirmed to work, please submit it as a patch to > > +the [systemd project](https://github.com/systemd/systemd) or if that is not > > +possible as a libinput bug here: > > +https://bugs.freedesktop.org/enter_bug.cgi?product=Wayland&component=libinput > > +*/ > > + > > diff --git a/doc/page-hierarchy.dox b/doc/page-hierarchy.dox > > index e47e98e..8455fca 100644 > > --- a/doc/page-hierarchy.dox > > +++ b/doc/page-hierarchy.dox > > @@ -8,6 +8,7 @@ > > - @subpage palm_detection > > - @subpage t440_support > > - @subpage touchpad_jumping_cursor > > +- @subpage absolute_coordinate_ranges > > > > @page touchscreens Touchscreens > > > > diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c > > index 61d955a..a7b7a68 100644 > > --- a/src/evdev-mt-touchpad.c > > +++ b/src/evdev-mt-touchpad.c > > @@ -289,6 +289,39 @@ tp_get_delta(struct tp_touch *t) > > return tp_normalize_delta(t->tp, delta); > > } > > > > +static inline void > > +tp_check_axis_range(struct tp_dispatch *tp, > > + unsigned int code, > > + int value) > > +{ > > + int min, max; > > + > > + switch(code) { > > + case ABS_X: > > + case ABS_MT_POSITION_X: > > + min = tp->warning_range.min.x; > > + max = tp->warning_range.max.x; > > + break; > > + case ABS_Y: > > + case ABS_MT_POSITION_Y: > > + min = tp->warning_range.min.y; > > + max = tp->warning_range.max.y; > > + break; > > + default: > > + return; > > + } > > + > > + if (value < min || value > max) { > > + log_info_ratelimit(tp_libinput_context(tp), > > + &tp->warning_range.range_warn_limit, > > + "Axis %#x value %d is outside expected range > > [%d, %d]\n" > > + "See %s/absolute_coordinate_ranges.html for > > details\n", > > + code, value, min, max, > > + HTTP_DOC_LINK); > > + > > + } > > +} > > + > > static void > > tp_process_absolute(struct tp_dispatch *tp, > > const struct input_event *e, > > @@ -298,12 +331,14 @@ tp_process_absolute(struct tp_dispatch *tp, > > > > switch(e->code) { > > case ABS_MT_POSITION_X: > > + tp_check_axis_range(tp, e->code, e->value); > > t->point.x = e->value; > > t->millis = time; > > t->dirty = true; > > tp->queued |= TOUCHPAD_EVENT_MOTION; > > break; > > case ABS_MT_POSITION_Y: > > + tp_check_axis_range(tp, e->code, e->value); > > t->point.y = e->value; > > t->millis = time; > > t->dirty = true; > > @@ -339,12 +374,14 @@ tp_process_absolute_st(struct tp_dispatch *tp, > > > > switch(e->code) { > > case ABS_X: > > + tp_check_axis_range(tp, e->code, e->value); > > t->point.x = e->value; > > t->millis = time; > > t->dirty = true; > > tp->queued |= TOUCHPAD_EVENT_MOTION; > > break; > > case ABS_Y: > > + tp_check_axis_range(tp, e->code, e->value); > > t->point.y = e->value; > > t->millis = time; > > t->dirty = true; > > @@ -2063,6 +2100,26 @@ want_hysteresis: > > return; > > } > > > > +static void > > +tp_init_range_warnings(struct tp_dispatch *tp, > > + struct evdev_device *device, > > + int width, > > + int height) > > +{ > > + const struct input_absinfo *x, *y; > > + > > + x = device->abs.absinfo_x; > > + y = device->abs.absinfo_y; > > + > > + tp->warning_range.min.x = x->minimum - 0.05 * width; > > + tp->warning_range.min.y = y->minimum - 0.05 * height; > > + tp->warning_range.max.x = x->maximum + 0.05 * width; > > + tp->warning_range.max.y = y->maximum + 0.05 * height; > > + > > + /* One warning every 5 min is enough */ > > + ratelimit_init(&tp->warning_range.range_warn_limit, s2us(3000), 1); > > +} > > + > > static int > > tp_init(struct tp_dispatch *tp, > > struct evdev_device *device) > > @@ -2086,6 +2143,8 @@ tp_init(struct tp_dispatch *tp, > > height = device->abs.dimensions.y; > > diagonal = sqrt(width*width + height*height); > > > > + tp_init_range_warnings(tp, device, width, height); > > + > > tp->reports_distance = libevdev_has_event_code(device->evdev, > > EV_ABS, > > ABS_MT_DISTANCE); > > diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h > > index cbc33aa..1efe25c 100644 > > --- a/src/evdev-mt-touchpad.h > > +++ b/src/evdev-mt-touchpad.h > > @@ -245,6 +245,11 @@ struct tp_dispatch { > > struct device_coords hysteresis_margin; > > > > struct { > > + struct device_coords min, max; > > + struct ratelimit range_warn_limit; > > + } warning_range; > > + > > + struct { > > double x_scale_coeff; > > double y_scale_coeff; > > } accel; > > -- > > 2.7.4 > > yong > > > _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
