[PATCH RFC libinput] filter: force touchpads to a fixed report rate
Peter Hutterer
peter.hutterer at who-t.net
Mon Jul 10 05:11:01 UTC 2017
From: Hans de Goede <hdegoede at redhat.com>
Some devices, specifically some bluetooth touchpads generate quite
unreliable timestamps for their events. The problem seems to be that
(some of) these touchpads sample at aprox 90 Hz, but the bluetooth stack
only communicates about every 30 ms (*) and then sends mutiple HID input
reports in one batch.
This results in 2-4 packets / SYNs every 30 ms. With timestamps really
close together. The finger coordinate deltas in these packets change by
aprox. the same amount between each packet when moving a finger at
constant speed. But the time deltas are e.g. 28 ms, 1 ms, 1 ms resulting
in calculate_tracker_velocity returning vastly different speeds for the
1st and 2nd packet, which in turn results in very "jerky" mouse pointer
movement.
*) Maybe it is waiting for a transmit time slot or some such.
This commit adds support for a fixed report rate on touchpads. We take the
assumption that a device with a fixed report rate will send events at that
rate but those events may be delayed by the kernel or accelerated following
some previous delay. Any timestamp that falls within the acceptable range (1.5
times the frequency) is set to the fixed report rate instead.
This does not affect the event timestamps, it only applies to pointer
acceleration.
Signed-off-by: Hans de Goede <hdegoede at redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
This replaces Hans' two patches in this thread.
Changes to Hans' patch:
- both patches squashed together
- instead of smoothing threshold/value assume a fixed report rate
- save the fixed timestamp in the trackers
Hans, please give this one a try. btw, what touchpad are we talking about
here?
It's the same principle, but instead of smoothing I figured we should just
go all the way and encode the report rate, assume that's what the device
will produce even if the kernel or some stack adds delays and adjust the
timestamps for that report rate. There's a fixme that needs to be addressed still
and obviously the 82Hz value measured on my device won't work for everyone
so it needs to move into a udev property.
But please give it try and let me know what you think
src/evdev-mt-touchpad.c | 2 +-
src/filter.c | 53 +++++++++++++++++++++++++++++++++++++++++--------
src/filter.h | 3 ++-
tools/ptraccel-debug.c | 2 +-
4 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 0a4f4d98..b2968444 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -2140,7 +2140,7 @@ tp_init_accel(struct tp_dispatch *tp)
tp->device->model_flags & EVDEV_MODEL_LENOVO_X220_TOUCHPAD_FW81)
filter = create_pointer_accelerator_filter_lenovo_x230(tp->device->dpi);
else
- filter = create_pointer_accelerator_filter_touchpad(tp->device->dpi);
+ filter = create_pointer_accelerator_filter_touchpad(device->dpi, 82);
if (!filter)
return false;
diff --git a/src/filter.c b/src/filter.c
index 7c500f87..d470d633 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -176,6 +176,10 @@ struct pointer_accelerator {
double accel; /* unitless factor */
double incline; /* incline of the function */
+ /* For smoothing timestamps from devices with unreliable timing */
+ unsigned int report_rate; /* in Hz */
+ uint64_t report_rate_tdelta; /* in us */
+
int dpi;
};
@@ -208,6 +212,30 @@ feed_trackers(struct pointer_accelerator *accel,
trackers[i].delta.y += delta->y;
}
+ /* If we have a fixed report rate on the device we don't take the
+ * kernel timestamps, they may be inaccurate. Instead, anything
+ * less than 1.5 times the scanout rate is forced to that scanout
+ * rate.
+ */
+ if (accel->report_rate != 0) {
+ uint64_t last_time;
+ int tdelta = 0;
+
+ last_time = trackers[accel->cur_tracker].time;
+ /* FIXME: if last_time >= time, the device's report_rate is
+ higher than the one we set. This can be ignored (e.g.
+ bluetooth touchpad that send 3 packets every 30ms,
+ but packets two and three have a delta of 1ms) or it can
+ become a problem (e.g. 80Hz set but 100Hz reported, our
+ timestamps will get more and more out of sync)
+ */
+ if (last_time < time)
+ tdelta = time - last_time;
+
+ if (tdelta < accel->report_rate_tdelta * 1.5)
+ time = last_time + tdelta;
+ }
+
current = (accel->cur_tracker + 1) % NUM_POINTER_TRACKERS;
accel->cur_tracker = current;
@@ -227,14 +255,18 @@ tracker_by_offset(struct pointer_accelerator *accel, unsigned int offset)
}
static double
-calculate_tracker_velocity(struct pointer_tracker *tracker, uint64_t time)
+calculate_tracker_velocity(struct pointer_accelerator *accel,
+ struct pointer_tracker *tracker, uint64_t time)
{
- double tdelta = time - tracker->time + 1;
- return hypot(tracker->delta.x, tracker->delta.y) / tdelta; /* units/us */
+ uint64_t tdelta = time - tracker->time + 1;
+
+ return hypot(tracker->delta.x, tracker->delta.y) /
+ (double)tdelta; /* units/us */
}
static inline double
-calculate_velocity_after_timeout(struct pointer_tracker *tracker)
+calculate_velocity_after_timeout(struct pointer_accelerator *accel,
+ struct pointer_tracker *tracker)
{
/* First movement after timeout needs special handling.
*
@@ -247,7 +279,7 @@ calculate_velocity_after_timeout(struct pointer_tracker *tracker)
* for really slow movements but provides much more useful initial
* movement in normal use-cases (pause, move, pause, move)
*/
- return calculate_tracker_velocity(tracker,
+ return calculate_tracker_velocity(accel, tracker,
tracker->time + MOTION_TIMEOUT);
}
@@ -282,11 +314,11 @@ calculate_velocity(struct pointer_accelerator *accel, uint64_t time)
/* Stop if too far away in time */
if (time - tracker->time > MOTION_TIMEOUT) {
if (offset == 1)
- result = calculate_velocity_after_timeout(tracker);
+ result = calculate_velocity_after_timeout(accel, tracker);
break;
}
- velocity = calculate_tracker_velocity(tracker, time);
+ velocity = calculate_tracker_velocity(accel, tracker, time);
/* Stop if direction changed */
dir &= tracker->dir;
@@ -1017,16 +1049,21 @@ struct motion_filter_interface accelerator_interface_touchpad = {
};
struct motion_filter *
-create_pointer_accelerator_filter_touchpad(int dpi)
+create_pointer_accelerator_filter_touchpad(int dpi,
+ unsigned int report_rate_hz)
{
struct pointer_accelerator *filter;
+ assert(report_rate_hz < 1000);
+
filter = create_default_filter(dpi);
if (!filter)
return NULL;
filter->base.interface = &accelerator_interface_touchpad;
filter->profile = touchpad_accel_profile_linear;
+ filter->report_rate_tdelta = ms2us(1000/report_rate_hz);
+ filter->report_rate = report_rate_hz;
return &filter->base;
}
diff --git a/src/filter.h b/src/filter.h
index e24c20d4..82758523 100644
--- a/src/filter.h
+++ b/src/filter.h
@@ -114,7 +114,8 @@ struct motion_filter *
create_pointer_accelerator_filter_linear_low_dpi(int dpi);
struct motion_filter *
-create_pointer_accelerator_filter_touchpad(int dpi);
+create_pointer_accelerator_filter_touchpad(int dpi,
+ unsigned int report_rate_hz);
struct motion_filter *
create_pointer_accelerator_filter_lenovo_x230(int dpi);
diff --git a/tools/ptraccel-debug.c b/tools/ptraccel-debug.c
index acb82c69..c026542b 100644
--- a/tools/ptraccel-debug.c
+++ b/tools/ptraccel-debug.c
@@ -314,7 +314,7 @@ main(int argc, char **argv)
filter = create_pointer_accelerator_filter_linear_low_dpi(dpi);
profile = pointer_accel_profile_linear_low_dpi;
} else if (streq(filter_type, "touchpad")) {
- filter = create_pointer_accelerator_filter_touchpad(dpi);
+ filter = create_pointer_accelerator_filter_touchpad(dpi, 80);
profile = touchpad_accel_profile_linear;
} else if (streq(filter_type, "x230")) {
filter = create_pointer_accelerator_filter_lenovo_x230(dpi);
--
2.13.0
More information about the wayland-devel
mailing list