* [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement
@ 2014-01-18 1:52 Clinton Sprain
2014-01-18 1:55 ` [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
` (5 more replies)
0 siblings, 6 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-01-18 1:52 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Resubmitting the appletouch patches from last week with a couple of
updates, as no one's taken a look yet.
The appletouch driver can make some trackpads feel insensitive, with movement
that tends to jerk in steps. This is particularly evident when moving
diagonally. This can greatly hamper the trackpad's usability. These patches
address this by dialing back the fuzz and threshold parameters, by
implementing a new cursor movement smoothing algorithm and by discarding
movements that directly coincide with a change in the number of fingers
detected on the trackpad.
Revisions since last week:
Patch 1/3: Fuzz/threshold fix is identical, except for a typo (2 -> 3)
Patch 2/3: Smoothing algorithm is no longer inertia-based and is more
predictable / less complicated. I also dropped a change in atp_calculate_abs
that I couldn't definitively prove was beneficial.
Patch 3/3: (New) Addresses issues related to when a second finger enters
or leaves the trackpad, causing the cursor to jump or the page to scroll
unexpectedly; now, we discard any movement that happens at the exact moment
we detect a change in the number of fingers touching the trackpad.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
drivers/input/mouse/appletouch.c | 206 ++++++++++++++++++++++++++++++--------
1 file changed, 162 insertions(+), 44 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold
2014-01-18 1:52 [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
@ 2014-01-18 1:55 ` Clinton Sprain
2014-01-18 7:07 ` Henrik Rydberg
2014-01-18 1:56 ` [PATCH 2/3] input: appletouch: use better cursor movement smoothing algorithm Clinton Sprain
` (4 subsequent siblings)
5 siblings, 1 reply; 30+ messages in thread
From: Clinton Sprain @ 2014-01-18 1:55 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Dials back the default fuzz and threshold settings for
most devices using this driver, based on values from
user feedback from forums and bug reports. This
increases smoothness and movement granularity. No
changes were made for the older devices that use the
driver, as I did not find any user feedback for them
regarding these settings; however, the two settings can
also now both be specified as module parameters in case
there is a desire to change the values for devices that
do not have new defaults.
There are also a couple of style cleanups per checkpatch.pl.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 66 ++++++++++++++++++++++++--------------
1 file changed, 42 insertions(+), 24 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e42f1fa..f5a16ee 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -43,12 +43,14 @@
*/
struct atp_info {
int xsensors; /* number of X sensors */
- int xsensors_17; /* 17" models have more sensors */
+ int xsensors_17; /* 17" model has more sensors */
int ysensors; /* number of Y sensors */
int xfact; /* X multiplication factor */
int yfact; /* Y multiplication factor */
int datalen; /* size of USB transfers */
void (*callback)(struct urb *); /* callback function */
+ int fuzz; /* fuzz touchpad generates */
+ int threshold; /* sensitivity threshold */
};
static void atp_complete_geyser_1_2(struct urb *urb);
@@ -62,6 +64,8 @@ static const struct atp_info fountain_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser1_info = {
@@ -72,6 +76,8 @@ static const struct atp_info geyser1_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser2_info = {
@@ -82,6 +88,8 @@ static const struct atp_info geyser2_info = {
.yfact = 43,
.datalen = 64,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser3_info = {
@@ -91,6 +99,8 @@ static const struct atp_info geyser3_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser4_info = {
@@ -100,6 +110,8 @@ static const struct atp_info geyser4_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 3,
};
#define ATP_DEVICE(prod, info) \
@@ -156,18 +168,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ 16
-
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
-/*
- * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
- * ignored.
- */
-#define ATP_THRESHOLD 5
-
/* Geyser initialization constants */
#define ATP_GEYSER_MODE_READ_REQUEST_ID 1
#define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
@@ -213,14 +216,16 @@ struct atp {
struct work_struct work;
};
-#define dbg_dump(msg, tab) \
+#define dbg_dump(msg, tab) \
+{ \
if (debug > 1) { \
int __i; \
printk(KERN_DEBUG "appletouch: %s", msg); \
for (__i = 0; __i < ATP_XSENSORS + ATP_YSENSORS; __i++) \
printk(" %02x", tab[__i]); \
printk("\n"); \
- }
+ } \
+}
#define dprintk(format, a...) \
do { \
@@ -236,14 +241,20 @@ MODULE_AUTHOR("Sven Anders");
MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
MODULE_LICENSE("GPL");
-/*
- * Make the threshold a module parameter
- */
-static int threshold = ATP_THRESHOLD;
+static int threshold = -1;
module_param(threshold, int, 0644);
MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
" (the trackpad has many of these sensors)"
- " less than this value.");
+ " less than this value. Devices have defaults;"
+ " this parameter overrides them.");
+static int fuzz = -1;
+
+module_param(fuzz, int, 0644);
+MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
+ " this is, the less sensitive the trackpad"
+ " will feel, but values too low may generate"
+ " random movement. Devices have defaults;"
+ " this parameter overrides them.");
static int debug;
module_param(debug, int, 0644);
@@ -363,7 +374,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
(!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
(*fingers)++;
is_increasing = 1;
- } else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
+ } else if (i > 0 && (xy_sensors[i - 1] -
+ xy_sensors[i] > threshold)) {
is_increasing = 0;
}
@@ -456,7 +468,7 @@ static void atp_detect_size(struct atp *dev)
input_set_abs_params(dev->input, ABS_X, 0,
(dev->info->xsensors_17 - 1) *
dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
break;
}
}
@@ -513,7 +525,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
/* Y values */
dev->xy_cur[ATP_XSENSORS + i] = dev->data[5 * i + 1];
- dev->xy_cur[ATP_XSENSORS + i + 8] = dev->data[5 * i + 3];
+ dev->xy_cur[ATP_XSENSORS + i + 8] =
+ dev->data[5 * i + 3];
}
}
@@ -809,12 +822,17 @@ static int atp_probe(struct usb_interface *iface,
dev->info = info;
dev->overflow_warned = false;
+ if (fuzz < 0)
+ fuzz = dev->info->fuzz;
+ if (threshold < 0)
+ threshold = dev->info->threshold;
+
dev->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb)
goto err_free_devs;
- dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen, GFP_KERNEL,
- &dev->urb->transfer_dma);
+ dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen,
+ GFP_KERNEL, &dev->urb->transfer_dma);
if (!dev->data)
goto err_free_urb;
@@ -844,10 +862,10 @@ static int atp_probe(struct usb_interface *iface,
input_set_abs_params(input_dev, ABS_X, 0,
(dev->info->xsensors - 1) * dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_Y, 0,
(dev->info->ysensors - 1) * dev->info->yfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 2/3] input: appletouch: use better cursor movement smoothing algorithm
2014-01-18 1:52 [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-01-18 1:55 ` [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
@ 2014-01-18 1:56 ` Clinton Sprain
2014-01-18 1:58 ` [PATCH 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
` (3 subsequent siblings)
5 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-01-18 1:56 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Implements a new algorithm used to smooth cursor
movement, using a sampling of the cursor's last three
positions to modulate the next cursor movement. This
further mitigates the 'stepping' users see when moving
the cursor diagonally. A 'legacy' module parameter has
been added in case the older devices do not like the
new algorithm.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 99 ++++++++++++++++++++++++++++++++------
1 file changed, 83 insertions(+), 16 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index f5a16ee..4a3bbcd 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -209,13 +209,16 @@ struct atp {
bool overflow_warned;
int x_old; /* last reported x/y, */
int y_old; /* used for smoothing */
+ int x_older; /* 2nd to last reported x/y,*/
+ int y_older; /* used for smoothing */
+ int x_oldest; /* 3rd to last reported x/y,*/
+ int y_oldest; /* used for smoothing */
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
signed char xy_old[ATP_XSENSORS + ATP_YSENSORS];
int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
int idlecount; /* number of empty packets */
struct work_struct work;
};
-
#define dbg_dump(msg, tab) \
{ \
if (debug > 1) { \
@@ -260,6 +263,13 @@ static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Activate debugging output");
+static int legacy;
+module_param(legacy, int, 0644);
+MODULE_PARM_DESC(legacy, "1 = Use old cursor movement smoothing."
+ " Older behavior averages current with"
+ " last cursor position. New behavior"
+ " uses cursor movement inertia.");
+
/*
* By default newer Geyser devices send standard USB HID mouse
* packets (Report ID 2). This code changes device mode, so it
@@ -338,6 +348,49 @@ static void atp_reinit(struct work_struct *work)
retval);
}
+static int atp_smooth_legacy(int curr, int old)
+{
+ return (old * 3 + curr) >> 2;
+}
+
+static int atp_smooth(int curr, int old, int older, int oldest)
+{
+ return (oldest + older * 2 + old * 4 + curr) >> 3;
+}
+
+static void atp_refresh_old_xy(int x, int y, struct atp *dev)
+{
+ if (legacy != true) {
+ dev->x_oldest = dev->x_older;
+ dev->y_oldest = dev->y_older;
+ dev->x_older = dev->x_old;
+ dev->y_older = dev->y_old;
+ }
+ dev->x_old = x;
+ dev->y_old = y;
+}
+
+static void atp_reset_old_xy(struct atp *dev)
+{
+ if (legacy != true) {
+ dev->x_oldest = dev->y_oldest = -1;
+ dev->x_older = dev->y_older = -1;
+ }
+ dev->x_old = dev->y_old = -1;
+}
+
+static void atp_default_old_xy(struct atp *dev)
+{
+ if (dev->x_older == -1)
+ dev->x_older = dev->x_old;
+ if (dev->y_older == -1)
+ dev->y_older = dev->y_old;
+ if (dev->x_oldest == -1)
+ dev->x_oldest = dev->x_older;
+ if (dev->y_oldest == -1)
+ dev->y_oldest = dev->y_older;
+}
+
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
@@ -570,10 +623,18 @@ static void atp_complete_geyser_1_2(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
- dev->x_old = x;
- dev->y_old = y;
+ if (legacy == true) {
+ x = atp_smooth_legacy(x, dev->x_old);
+ y = atp_smooth_legacy(y, dev->y_old);
+ } else {
+
+ atp_default_old_xy(dev);
+
+ x = atp_smooth(x, dev->x_old,
+ dev->x_older, dev->x_oldest);
+ y = atp_smooth(y, dev->y_old,
+ dev->y_older, dev->y_oldest);
+ }
if (debug > 1)
printk(KERN_DEBUG "appletouch: "
@@ -587,12 +648,11 @@ static void atp_complete_geyser_1_2(struct urb *urb)
min(ATP_PRESSURE, x_z + y_z));
atp_report_fingers(dev->input, max(x_f, y_f));
}
- dev->x_old = x;
- dev->y_old = y;
+ atp_refresh_old_xy(x, y, dev);
} else if (!x && !y) {
- dev->x_old = dev->y_old = -1;
+ atp_reset_old_xy(dev);
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -682,10 +742,18 @@ static void atp_complete_geyser_3_4(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
- dev->x_old = x;
- dev->y_old = y;
+ if (legacy == true) {
+ x = atp_smooth_legacy(x, dev->x_old);
+ y = atp_smooth_legacy(y, dev->y_old);
+ } else {
+
+ atp_default_old_xy(dev);
+
+ x = atp_smooth(x, dev->x_old,
+ dev->x_older, dev->x_oldest);
+ y = atp_smooth(y, dev->y_old,
+ dev->y_older, dev->y_oldest);
+ }
if (debug > 1)
printk(KERN_DEBUG "appletouch: X: %3d Y: %3d "
@@ -699,12 +767,11 @@ static void atp_complete_geyser_3_4(struct urb *urb)
min(ATP_PRESSURE, x_z + y_z));
atp_report_fingers(dev->input, max(x_f, y_f));
}
- dev->x_old = x;
- dev->y_old = y;
+ atp_refresh_old_xy(x, y, dev);
} else if (!x && !y) {
- dev->x_old = dev->y_old = -1;
+ atp_reset_old_xy(dev);
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -730,7 +797,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
if (!x && !y && !key) {
dev->idlecount++;
if (dev->idlecount == 10) {
- dev->x_old = dev->y_old = -1;
+ atp_reset_old_xy(dev);
dev->idlecount = 0;
schedule_work(&dev->work);
/* Don't resubmit urb here, wait for reinit */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 3/3] input: appletouch: fix jumps when additional fingers are detected
2014-01-18 1:52 [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-01-18 1:55 ` [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
2014-01-18 1:56 ` [PATCH 2/3] input: appletouch: use better cursor movement smoothing algorithm Clinton Sprain
@ 2014-01-18 1:58 ` Clinton Sprain
2014-02-07 0:38 ` [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
` (2 subsequent siblings)
5 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-01-18 1:58 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Discard cursor movements if they directly coincide with
a change in the number of fingers detected. This helps
mitigate two issues - a sudden jump of the cursor when
a second finger enters or leaves the trackpad, and an
unexpected jump in page scrolling under the same
scenario. This doesn't completely eliminate all jumps
but does greatly reduce their frequency and velocity.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 41 ++++++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 4a3bbcd..406054a 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -218,6 +218,7 @@ struct atp {
int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
int idlecount; /* number of empty packets */
struct work_struct work;
+ int fingers_old; /* last # of fingers detected*/
};
#define dbg_dump(msg, tab) \
{ \
@@ -538,6 +539,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
int key;
+ int fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -621,7 +623,16 @@ static void atp_complete_geyser_1_2(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ /*
+ * Only act if we have valid x/y and # of fingers did not change.
+ * If the # of fingers just changed, acting on the new x/y values will
+ * cause the cursor to jump across the screen or the page to scroll
+ * unexpectedly.
+ */
+
+ if (x && y && (dev->fingers_old == fingers)) {
if (dev->x_old != -1) {
if (legacy == true) {
x = atp_smooth_legacy(x, dev->x_old);
@@ -646,7 +657,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
atp_refresh_old_xy(x, y, dev);
@@ -661,6 +672,12 @@ static void atp_complete_geyser_1_2(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ /* if # of fingers changed, old x/y values are no longer useful */
+ if (dev->fingers_old != fingers)
+ atp_reset_old_xy(dev);
+
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
@@ -679,6 +696,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
int key;
+ int fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -740,7 +758,16 @@ static void atp_complete_geyser_3_4(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ /*
+ * Only act if we have valid x/y and # of fingers did not change.
+ * If the # of fingers just changed, acting on the new x/y values will
+ * cause the cursor to jump across the screen or the page to scroll
+ * unexpectedly.
+ */
+
+ if (x && y && (dev->fingers_old == fingers)) {
if (dev->x_old != -1) {
if (legacy == true) {
x = atp_smooth_legacy(x, dev->x_old);
@@ -765,7 +792,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
atp_refresh_old_xy(x, y, dev);
@@ -780,6 +807,12 @@ static void atp_complete_geyser_3_4(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ /* if # of fingers changed, old x/y values are no longer useful */
+ if (dev->fingers_old != fingers)
+ atp_reset_old_xy(dev);
+
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold
2014-01-18 1:55 ` [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
@ 2014-01-18 7:07 ` Henrik Rydberg
0 siblings, 0 replies; 30+ messages in thread
From: Henrik Rydberg @ 2014-01-18 7:07 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
Hi Clinton,
> Dials back the default fuzz and threshold settings for
> most devices using this driver, based on values from
> user feedback from forums and bug reports. This
> increases smoothness and movement granularity. No
> changes were made for the older devices that use the
> driver, as I did not find any user feedback for them
> regarding these settings; however, the two settings can
> also now both be specified as module parameters in case
> there is a desire to change the values for devices that
> do not have new defaults.
>
> There are also a couple of style cleanups per checkpatch.pl.
>
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
> drivers/input/mouse/appletouch.c | 66 ++++++++++++++++++++++++--------------
> 1 file changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index e42f1fa..f5a16ee 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -43,12 +43,14 @@
> */
> struct atp_info {
> int xsensors; /* number of X sensors */
> - int xsensors_17; /* 17" models have more sensors */
> + int xsensors_17; /* 17" model has more sensors */
> int ysensors; /* number of Y sensors */
> int xfact; /* X multiplication factor */
> int yfact; /* Y multiplication factor */
> int datalen; /* size of USB transfers */
> void (*callback)(struct urb *); /* callback function */
> + int fuzz; /* fuzz touchpad generates */
> + int threshold; /* sensitivity threshold */
> };
>
> static void atp_complete_geyser_1_2(struct urb *urb);
> @@ -62,6 +64,8 @@ static const struct atp_info fountain_info = {
> .yfact = 43,
> .datalen = 81,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 16,
> + .threshold = 5,
> };
>
> static const struct atp_info geyser1_info = {
> @@ -72,6 +76,8 @@ static const struct atp_info geyser1_info = {
> .yfact = 43,
> .datalen = 81,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 16,
> + .threshold = 5,
> };
>
> static const struct atp_info geyser2_info = {
> @@ -82,6 +88,8 @@ static const struct atp_info geyser2_info = {
> .yfact = 43,
> .datalen = 64,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> static const struct atp_info geyser3_info = {
> @@ -91,6 +99,8 @@ static const struct atp_info geyser3_info = {
> .yfact = 64,
> .datalen = 64,
> .callback = atp_complete_geyser_3_4,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> static const struct atp_info geyser4_info = {
> @@ -100,6 +110,8 @@ static const struct atp_info geyser4_info = {
> .yfact = 64,
> .datalen = 64,
> .callback = atp_complete_geyser_3_4,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> #define ATP_DEVICE(prod, info) \
> @@ -156,18 +168,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
> #define ATP_XSENSORS 26
> #define ATP_YSENSORS 16
>
> -/* amount of fuzz this touchpad generates */
> -#define ATP_FUZZ 16
> -
> /* maximum pressure this driver will report */
> #define ATP_PRESSURE 300
>
> -/*
> - * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
> - * ignored.
> - */
> -#define ATP_THRESHOLD 5
> -
> /* Geyser initialization constants */
> #define ATP_GEYSER_MODE_READ_REQUEST_ID 1
> #define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
> @@ -213,14 +216,16 @@ struct atp {
> struct work_struct work;
> };
>
> -#define dbg_dump(msg, tab) \
> +#define dbg_dump(msg, tab) \
> +{ \
> if (debug > 1) { \
> int __i; \
> printk(KERN_DEBUG "appletouch: %s", msg); \
> for (__i = 0; __i < ATP_XSENSORS + ATP_YSENSORS; __i++) \
> printk(" %02x", tab[__i]); \
> printk("\n"); \
> - }
> + } \
> +}
Looks like the patch needs cleaning.
>
> #define dprintk(format, a...) \
> do { \
> @@ -236,14 +241,20 @@ MODULE_AUTHOR("Sven Anders");
> MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
> MODULE_LICENSE("GPL");
>
> -/*
> - * Make the threshold a module parameter
> - */
> -static int threshold = ATP_THRESHOLD;
> +static int threshold = -1;
> module_param(threshold, int, 0644);
> MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
> " (the trackpad has many of these sensors)"
> - " less than this value.");
> + " less than this value. Devices have defaults;"
> + " this parameter overrides them.");
> +static int fuzz = -1;
> +
> +module_param(fuzz, int, 0644);
> +MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
> + " this is, the less sensitive the trackpad"
> + " will feel, but values too low may generate"
> + " random movement. Devices have defaults;"
> + " this parameter overrides them.");
The fuzz can be modified via the input subsystem ioctls (EVIOCSABS), so no need
to add a parameter here.
> static int debug;
> module_param(debug, int, 0644);
> @@ -363,7 +374,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
> (*fingers)++;
> is_increasing = 1;
> - } else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
> + } else if (i > 0 && (xy_sensors[i - 1] -
> + xy_sensors[i] > threshold)) {
> is_increasing = 0;
> }
More noise, please clean the patchset.
> @@ -456,7 +468,7 @@ static void atp_detect_size(struct atp *dev)
> input_set_abs_params(dev->input, ABS_X, 0,
> (dev->info->xsensors_17 - 1) *
> dev->info->xfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
where is this variable defined?
> break;
> }
> }
> @@ -513,7 +525,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>
> /* Y values */
> dev->xy_cur[ATP_XSENSORS + i] = dev->data[5 * i + 1];
> - dev->xy_cur[ATP_XSENSORS + i + 8] = dev->data[5 * i + 3];
> + dev->xy_cur[ATP_XSENSORS + i + 8] =
> + dev->data[5 * i + 3];
> }
> }
>
> @@ -809,12 +822,17 @@ static int atp_probe(struct usb_interface *iface,
> dev->info = info;
> dev->overflow_warned = false;
>
> + if (fuzz < 0)
> + fuzz = dev->info->fuzz;
> + if (threshold < 0)
> + threshold = dev->info->threshold;
> +
> dev->urb = usb_alloc_urb(0, GFP_KERNEL);
> if (!dev->urb)
> goto err_free_devs;
>
> - dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen, GFP_KERNEL,
> - &dev->urb->transfer_dma);
> + dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen,
> + GFP_KERNEL, &dev->urb->transfer_dma);
> if (!dev->data)
> goto err_free_urb;
>
> @@ -844,10 +862,10 @@ static int atp_probe(struct usb_interface *iface,
>
> input_set_abs_params(input_dev, ABS_X, 0,
> (dev->info->xsensors - 1) * dev->info->xfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> input_set_abs_params(input_dev, ABS_Y, 0,
> (dev->info->ysensors - 1) * dev->info->yfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>
> set_bit(EV_KEY, input_dev->evbit);
>
Thanks,
Henrik
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement
2014-01-18 1:52 [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
` (2 preceding siblings ...)
2014-01-18 1:58 ` [PATCH 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
@ 2014-02-07 0:38 ` Clinton Sprain
2014-02-07 0:40 ` [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
` (2 more replies)
2014-03-09 6:03 ` [PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
5 siblings, 3 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-02-07 0:38 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
These patches address some usability issues by dialing back the fuzz and threshold parameters, by implementing some smoothing for the sensor data and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
Changes since last time:
1/3: Remove extraneous cleanup - I'll submit a separate patch another day
2/3: Apply smoothing to the sensor data before position calculation, eliminating the need to toss values based on a threshold; simplify formula for averaging with prior values
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
drivers/input/mouse/appletouch.c | 131 ++++++++++++++++++++++++++------------
1 file changed, 92 insertions(+), 39 deletions(-)
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold
2014-02-07 0:38 ` [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
@ 2014-02-07 0:40 ` Clinton Sprain
2014-02-09 12:01 ` Henrik Rydberg
2014-02-07 0:43 ` [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm Clinton Sprain
2014-02-07 0:46 ` [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2 siblings, 1 reply; 30+ messages in thread
From: Clinton Sprain @ 2014-02-07 0:40 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 48 ++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 800ca7d..d48181b 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -48,6 +48,8 @@ struct atp_info {
int yfact; /* Y multiplication factor */
int datalen; /* size of USB transfers */
void (*callback)(struct urb *); /* callback function */
+ int fuzz; /* fuzz touchpad generates */
+ int threshold; /* sensitivity threshold */
};
static void atp_complete_geyser_1_2(struct urb *urb);
@@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser1_info = {
@@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
+ .threshold = 5,
};
static const struct atp_info geyser2_info = {
@@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
.yfact = 43,
.datalen = 64,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser3_info = {
@@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 3,
};
static const struct atp_info geyser4_info = {
@@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
+ .threshold = 3,
};
#define ATP_DEVICE(prod, info) \
@@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ 16
-
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
-/*
- * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
- * ignored.
- */
-#define ATP_THRESHOLD 5
-
/* Geyser initialization constants */
#define ATP_GEYSER_MODE_READ_REQUEST_ID 1
#define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
@@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
MODULE_LICENSE("GPL");
-/*
- * Make the threshold a module parameter
- */
-static int threshold = ATP_THRESHOLD;
+static int threshold = -1;
module_param(threshold, int, 0644);
MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
" (the trackpad has many of these sensors)"
- " less than this value.");
+ " less than this value. Devices have defaults;"
+ " this parameter overrides them.");
+static int fuzz = -1;
+
+module_param(fuzz, int, 0644);
+MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
+ " this is, the less sensitive the trackpad"
+ " will feel, but values too low may generate"
+ " random movement. Devices have defaults;"
+ " this parameter overrides them.");
static int debug;
module_param(debug, int, 0644);
@@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
input_set_abs_params(dev->input, ABS_X, 0,
(dev->info->xsensors_17 - 1) *
dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
break;
}
}
@@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
dev->info = info;
dev->overflow_warned = false;
+ if (fuzz < 0)
+ fuzz = dev->info->fuzz;
+ if (threshold < 0)
+ threshold = dev->info->threshold;
+
dev->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb)
goto err_free_devs;
@@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
input_set_abs_params(input_dev, ABS_X, 0,
(dev->info->xsensors - 1) * dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_Y, 0,
(dev->info->ysensors - 1) * dev->info->yfact - 1,
- ATP_FUZZ, 0);
+ fuzz, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm
2014-02-07 0:38 ` [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-02-07 0:40 ` [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
@ 2014-02-07 0:43 ` Clinton Sprain
2014-02-09 12:15 ` Henrik Rydberg
2014-02-07 0:46 ` [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2 siblings, 1 reply; 30+ messages in thread
From: Clinton Sprain @ 2014-02-07 0:43 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Use smoothed version of sensor array data to calculate movement and add weight to prior values when calculating average. This gives more granular and more predictable movement.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 60 ++++++++++++++++++++++++++------------
1 file changed, 42 insertions(+), 18 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index d48181b..edbdd95 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
- int i;
+ int i, k;
+ int smooth[nb_sensors + 2];
+ int smooth_tmp[nb_sensors + 2];
+
/* values to calculate mean */
int pcum = 0, psum = 0;
int is_increasing = 0;
*fingers = 0;
+ /*
+ * Use a smoothed version of sensor data for movement calculations, to
+ * combat noise without needing to toss out values based on a threshold.
+ * This improves tracking. Finger count is calculated separately based
+ * on raw data.
+ */
+
+ for (i = 0; i < nb_sensors; i++) { /* Scale up to minimize */
+ smooth[i] = xy_sensors[i] << 12; /* rounding/truncation. */
+ }
+
+ /* Mitigate some of the data loss from smoothing on the edge sensors. */
+ smooth[-1] = smooth[0] >> 2;
+ smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+
+ for (k = 0; k < 4; k++) {
+ for (i = 0; i < nb_sensors; i++) { /* Blend with neighbors. */
+ smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+ }
+
+ for (i = 0; i < nb_sensors; i++) {
+ smooth[i] = smooth_tmp[i];
+ if (k == 3) /* Last go-round, so scale back down. */
+ smooth[i] = smooth[i] >> 12;
+ }
+
+ smooth[-1] = smooth[0] >> 2;
+ smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
+ }
+
for (i = 0; i < nb_sensors; i++) {
if (xy_sensors[i] < threshold) {
if (is_increasing)
is_increasing = 0;
- continue;
- }
-
/*
* Makes the finger detection more versatile. For example,
* two fingers with no gap will be detected. Also, my
@@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
*
* - Jason Parekh <jasonparekh@gmail.com>
*/
- if (i < 1 ||
+
+ } else if (i < 1 ||
(!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
(*fingers)++;
is_increasing = 1;
@@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
is_increasing = 0;
}
- /*
- * Subtracts threshold so a high sensor that just passes the
- * threshold won't skew the calculated absolute coordinate.
- * Fixes an issue where slowly moving the mouse would
- * occasionally jump a number of pixels (slowly moving the
- * finger makes this issue most apparent.)
- */
- pcum += (xy_sensors[i] - threshold) * i;
- psum += (xy_sensors[i] - threshold);
+ pcum += (smooth[i]) * i;
+ psum += (smooth[i]);
}
if (psum > 0) {
@@ -565,8 +589,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
@@ -677,8 +701,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected
2014-02-07 0:38 ` [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-02-07 0:40 ` [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
2014-02-07 0:43 ` [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm Clinton Sprain
@ 2014-02-07 0:46 ` Clinton Sprain
2014-02-09 12:16 ` Henrik Rydberg
2 siblings, 1 reply; 30+ messages in thread
From: Clinton Sprain @ 2014-02-07 0:46 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Discard cursor movements if they directly coincide with a change in the number of fingers detected. This helps with two issues - a sudden jump of the cursor when a second finger enters or leaves the touchpad, and an unexpected jump in page scrolling under the same scenario. The fix doesn't completely eliminate the problem but does greatly reduce its frequency and severity.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index edbdd95..370d0e9 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -212,6 +212,7 @@ struct atp {
signed char xy_old[ATP_XSENSORS + ATP_YSENSORS];
int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
int idlecount; /* number of empty packets */
+ int fingers_old; /* last reported finger count */
struct work_struct work;
};
@@ -505,6 +506,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
int key;
+ int fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -587,7 +589,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -604,7 +608,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -620,6 +624,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
@@ -638,6 +646,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
int key;
+ int fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -699,7 +708,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -716,7 +727,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -732,6 +743,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold
2014-02-07 0:40 ` [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
@ 2014-02-09 12:01 ` Henrik Rydberg
2014-02-11 4:46 ` Clinton Sprain
0 siblings, 1 reply; 30+ messages in thread
From: Henrik Rydberg @ 2014-02-09 12:01 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
Hi Clinton,
On 02/07/2014 01:40 AM, Clinton Sprain wrote:
> Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
>
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
> drivers/input/mouse/appletouch.c | 48 ++++++++++++++++++++++++--------------
> 1 file changed, 31 insertions(+), 17 deletions(-)
Thanks for the patches, they seem like a good improvement. There are still some
comments, though, you will find them inline.
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 800ca7d..d48181b 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -48,6 +48,8 @@ struct atp_info {
> int yfact; /* Y multiplication factor */
> int datalen; /* size of USB transfers */
> void (*callback)(struct urb *); /* callback function */
> + int fuzz; /* fuzz touchpad generates */
> + int threshold; /* sensitivity threshold */
> };
>
> static void atp_complete_geyser_1_2(struct urb *urb);
> @@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
> .yfact = 43,
> .datalen = 81,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 16,
> + .threshold = 5,
> };
>
> static const struct atp_info geyser1_info = {
> @@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
> .yfact = 43,
> .datalen = 81,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 16,
> + .threshold = 5,
> };
>
> static const struct atp_info geyser2_info = {
> @@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
> .yfact = 43,
> .datalen = 64,
> .callback = atp_complete_geyser_1_2,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> static const struct atp_info geyser3_info = {
> @@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
> .yfact = 64,
> .datalen = 64,
> .callback = atp_complete_geyser_3_4,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> static const struct atp_info geyser4_info = {
> @@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
> .yfact = 64,
> .datalen = 64,
> .callback = atp_complete_geyser_3_4,
> + .fuzz = 0,
> + .threshold = 3,
> };
>
> #define ATP_DEVICE(prod, info) \
> @@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
> #define ATP_XSENSORS 26
> #define ATP_YSENSORS 16
>
> -/* amount of fuzz this touchpad generates */
> -#define ATP_FUZZ 16
> -
> /* maximum pressure this driver will report */
> #define ATP_PRESSURE 300
>
> -/*
> - * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
> - * ignored.
> - */
> -#define ATP_THRESHOLD 5
> -
> /* Geyser initialization constants */
> #define ATP_GEYSER_MODE_READ_REQUEST_ID 1
> #define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
> @@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
> MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
> MODULE_LICENSE("GPL");
>
> -/*
> - * Make the threshold a module parameter
> - */
> -static int threshold = ATP_THRESHOLD;
> +static int threshold = -1;
> module_param(threshold, int, 0644);
> MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
> " (the trackpad has many of these sensors)"
> - " less than this value.");
> + " less than this value. Devices have defaults;"
> + " this parameter overrides them.");
> +static int fuzz = -1;
> +
> +module_param(fuzz, int, 0644);
> +MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
> + " this is, the less sensitive the trackpad"
> + " will feel, but values too low may generate"
> + " random movement. Devices have defaults;"
> + " this parameter overrides them.");
Given that there already is a userland interface for the fuzz parameter - and it
is indeed in frequent use - this one does not seem warranted. If we are unsure
if the changes to the default are not all good, perhaps they should not be
changed at all?
>
> static int debug;
> module_param(debug, int, 0644);
> @@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
> input_set_abs_params(dev->input, ABS_X, 0,
> (dev->info->xsensors_17 - 1) *
> dev->info->xfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> break;
> }
> }
> @@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
> dev->info = info;
> dev->overflow_warned = false;
>
> + if (fuzz < 0)
> + fuzz = dev->info->fuzz;
> + if (threshold < 0)
> + threshold = dev->info->threshold;
> +
> dev->urb = usb_alloc_urb(0, GFP_KERNEL);
> if (!dev->urb)
> goto err_free_devs;
> @@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
>
> input_set_abs_params(input_dev, ABS_X, 0,
> (dev->info->xsensors - 1) * dev->info->xfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> input_set_abs_params(input_dev, ABS_Y, 0,
> (dev->info->ysensors - 1) * dev->info->yfact - 1,
> - ATP_FUZZ, 0);
> + fuzz, 0);
> input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>
> set_bit(EV_KEY, input_dev->evbit);
>
Thanks,
Henrik
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm
2014-02-07 0:43 ` [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm Clinton Sprain
@ 2014-02-09 12:15 ` Henrik Rydberg
2014-02-11 7:19 ` Clinton Sprain
0 siblings, 1 reply; 30+ messages in thread
From: Henrik Rydberg @ 2014-02-09 12:15 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
Hi Clinton,
> Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.
>
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
> drivers/input/mouse/appletouch.c | 60 ++++++++++++++++++++++++++------------
> 1 file changed, 42 insertions(+), 18 deletions(-)
I like this patch, but there are some technical glitches, comments below.
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index d48181b..edbdd95 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
> static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> int *z, int *fingers)
> {
> - int i;
> + int i, k;
> + int smooth[nb_sensors + 2];
> + int smooth_tmp[nb_sensors + 2];
> +
> /* values to calculate mean */
> int pcum = 0, psum = 0;
> int is_increasing = 0;
>
> *fingers = 0;
>
> + /*
> + * Use a smoothed version of sensor data for movement calculations, to
> + * combat noise without needing to toss out values based on a threshold.
> + * This improves tracking. Finger count is calculated separately based
> + * on raw data.
> + */
> +
> + for (i = 0; i < nb_sensors; i++) { /* Scale up to minimize */
> + smooth[i] = xy_sensors[i] << 12; /* rounding/truncation. */
> + }
> +
> + /* Mitigate some of the data loss from smoothing on the edge sensors. */
> + smooth[-1] = smooth[0] >> 2;
> + smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
Out of bounds array... also, the boundary condition seems odd. If you want to
extrapolate the edge velocity, and assuming smooth[0] is the first sensor, you
would get something like (N = nb_sensors)
smooth_tmp[0] = 2 * smooth[1] - smooth[2];
smooth_tmp[N-1] = 2 * smooth[N-2] - smooth[N-3];
> + for (k = 0; k < 4; k++) {
> + for (i = 0; i < nb_sensors; i++) { /* Blend with neighbors. */
And here would be for (i = 1; i < nb_sensors - 1; i++)
> + smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
> + }
> +
> + for (i = 0; i < nb_sensors; i++) {
> + smooth[i] = smooth_tmp[i];
> + if (k == 3) /* Last go-round, so scale back down. */
> + smooth[i] = smooth[i] >> 12;
The scale-up is done separately, so why not make the scale-down separately too?
> + }
> +
> + smooth[-1] = smooth[0] >> 2;
> + smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
And these would be dropped.
> + }
> +
> for (i = 0; i < nb_sensors; i++) {
> if (xy_sensors[i] < threshold) {
> if (is_increasing)
> is_increasing = 0;
>
> - continue;
> - }
> -
> /*
> * Makes the finger detection more versatile. For example,
> * two fingers with no gap will be detected. Also, my
> @@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> *
> * - Jason Parekh <jasonparekh@gmail.com>
> */
> - if (i < 1 ||
> +
> + } else if (i < 1 ||
> (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
> (*fingers)++;
> is_increasing = 1;
> @@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> is_increasing = 0;
> }
>
> - /*
> - * Subtracts threshold so a high sensor that just passes the
> - * threshold won't skew the calculated absolute coordinate.
> - * Fixes an issue where slowly moving the mouse would
> - * occasionally jump a number of pixels (slowly moving the
> - * finger makes this issue most apparent.)
> - */
> - pcum += (xy_sensors[i] - threshold) * i;
> - psum += (xy_sensors[i] - threshold);
> + pcum += (smooth[i]) * i;
> + psum += (smooth[i]);
Great, this makes so much more sense.
> }
>
> if (psum > 0) {
> @@ -565,8 +589,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>
> if (x && y) {
> if (dev->x_old != -1) {
> - x = (dev->x_old * 3 + x) >> 2;
> - y = (dev->y_old * 3 + y) >> 2;
> + x = (dev->x_old * 7 + x) >> 3;
> + y = (dev->y_old * 7 + y) >> 3;
> dev->x_old = x;
> dev->y_old = y;
>
> @@ -677,8 +701,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>
> if (x && y) {
> if (dev->x_old != -1) {
> - x = (dev->x_old * 3 + x) >> 2;
> - y = (dev->y_old * 3 + y) >> 2;
> + x = (dev->x_old * 7 + x) >> 3;
> + y = (dev->y_old * 7 + y) >> 3;
> dev->x_old = x;
> dev->y_old = y;
Would you say that the cursor slow-down here is noticeable, or are there enough
samples per second that it does not matter?
Thanks,
Henrik
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected
2014-02-07 0:46 ` [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
@ 2014-02-09 12:16 ` Henrik Rydberg
0 siblings, 0 replies; 30+ messages in thread
From: Henrik Rydberg @ 2014-02-09 12:16 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
On 02/07/2014 01:46 AM, Clinton Sprain wrote:
> Discard cursor movements if they directly coincide with a change in the number of fingers detected. This helps with two issues - a sudden jump of the cursor when a second finger enters or leaves the touchpad, and an unexpected jump in page scrolling under the same scenario. The fix doesn't completely eliminate the problem but does greatly reduce its frequency and severity.
>
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
> drivers/input/mouse/appletouch.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index edbdd95..370d0e9 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -212,6 +212,7 @@ struct atp {
> signed char xy_old[ATP_XSENSORS + ATP_YSENSORS];
> int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
> int idlecount; /* number of empty packets */
> + int fingers_old; /* last reported finger count */
> struct work_struct work;
> };
>
> @@ -505,6 +506,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
> int x, y, x_z, y_z, x_f, y_f;
> int retval, i, j;
> int key;
> + int fingers;
> struct atp *dev = urb->context;
> int status = atp_status_check(urb);
>
> @@ -587,7 +589,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
> dev->info->yfact, &y_z, &y_f);
> key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
>
> - if (x && y) {
> + fingers = max(x_f, y_f);
> +
> + if (x && y && (fingers == dev->fingers_old)) {
> if (dev->x_old != -1) {
> x = (dev->x_old * 7 + x) >> 3;
> y = (dev->y_old * 7 + y) >> 3;
> @@ -604,7 +608,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
> input_report_abs(dev->input, ABS_Y, y);
> input_report_abs(dev->input, ABS_PRESSURE,
> min(ATP_PRESSURE, x_z + y_z));
> - atp_report_fingers(dev->input, max(x_f, y_f));
> + atp_report_fingers(dev->input, fingers);
> }
> dev->x_old = x;
> dev->y_old = y;
> @@ -620,6 +624,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
> memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
> }
>
> + if (fingers != dev->fingers_old)
> + dev->x_old = dev->y_old = -1;
> + dev->fingers_old = fingers;
> +
> input_report_key(dev->input, BTN_LEFT, key);
> input_sync(dev->input);
>
> @@ -638,6 +646,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
> int x, y, x_z, y_z, x_f, y_f;
> int retval, i, j;
> int key;
> + int fingers;
> struct atp *dev = urb->context;
> int status = atp_status_check(urb);
>
> @@ -699,7 +708,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
> dev->info->yfact, &y_z, &y_f);
> key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
>
> - if (x && y) {
> + fingers = max(x_f, y_f);
> +
> + if (x && y && (fingers == dev->fingers_old)) {
> if (dev->x_old != -1) {
> x = (dev->x_old * 7 + x) >> 3;
> y = (dev->y_old * 7 + y) >> 3;
> @@ -716,7 +727,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
> input_report_abs(dev->input, ABS_Y, y);
> input_report_abs(dev->input, ABS_PRESSURE,
> min(ATP_PRESSURE, x_z + y_z));
> - atp_report_fingers(dev->input, max(x_f, y_f));
> + atp_report_fingers(dev->input, fingers);
> }
> dev->x_old = x;
> dev->y_old = y;
> @@ -732,6 +743,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
> memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
> }
>
> + if (fingers != dev->fingers_old)
> + dev->x_old = dev->y_old = -1;
> + dev->fingers_old = fingers;
> +
> input_report_key(dev->input, BTN_LEFT, key);
> input_sync(dev->input);
>
>
Reviewed-by: Henrik Rydberg <rydberg@euromail.se>
Thanks,
Henrik
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold
2014-02-09 12:01 ` Henrik Rydberg
@ 2014-02-11 4:46 ` Clinton Sprain
0 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-02-11 4:46 UTC (permalink / raw)
To: Henrik Rydberg, linux-input; +Cc: Dmitry Torokhov
Hi Herik,
On 02/09/2014 06:01 AM, Henrik Rydberg wrote:
> Hi Clinton,
>
> On 02/07/2014 01:40 AM, Clinton Sprain wrote:
>> Dials back the default fuzz and threshold settings for most devices using this driver, based on user input from forums and bug reports, increasing sensitivity. These two settings can also now both be overridden as module parameters in the event that users have hardware that does not respond well to the new defaults, or there is a desire to change the values for devices that do not have new defaults.
>>
>> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
>> ---
>> drivers/input/mouse/appletouch.c | 48 ++++++++++++++++++++++++--------------
>> 1 file changed, 31 insertions(+), 17 deletions(-)
>
> Thanks for the patches, they seem like a good improvement. There are still some
> comments, though, you will find them inline.
>
>> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
>> index 800ca7d..d48181b 100644
>> --- a/drivers/input/mouse/appletouch.c
>> +++ b/drivers/input/mouse/appletouch.c
>> @@ -48,6 +48,8 @@ struct atp_info {
>> int yfact; /* Y multiplication factor */
>> int datalen; /* size of USB transfers */
>> void (*callback)(struct urb *); /* callback function */
>> + int fuzz; /* fuzz touchpad generates */
>> + int threshold; /* sensitivity threshold */
>> };
>>
>> static void atp_complete_geyser_1_2(struct urb *urb);
>> @@ -61,6 +63,8 @@ static const struct atp_info fountain_info = {
>> .yfact = 43,
>> .datalen = 81,
>> .callback = atp_complete_geyser_1_2,
>> + .fuzz = 16,
>> + .threshold = 5,
>> };
>>
>> static const struct atp_info geyser1_info = {
>> @@ -71,6 +75,8 @@ static const struct atp_info geyser1_info = {
>> .yfact = 43,
>> .datalen = 81,
>> .callback = atp_complete_geyser_1_2,
>> + .fuzz = 16,
>> + .threshold = 5,
>> };
>>
>> static const struct atp_info geyser2_info = {
>> @@ -81,6 +87,8 @@ static const struct atp_info geyser2_info = {
>> .yfact = 43,
>> .datalen = 64,
>> .callback = atp_complete_geyser_1_2,
>> + .fuzz = 0,
>> + .threshold = 3,
>> };
>>
>> static const struct atp_info geyser3_info = {
>> @@ -90,6 +98,8 @@ static const struct atp_info geyser3_info = {
>> .yfact = 64,
>> .datalen = 64,
>> .callback = atp_complete_geyser_3_4,
>> + .fuzz = 0,
>> + .threshold = 3,
>> };
>>
>> static const struct atp_info geyser4_info = {
>> @@ -99,6 +109,8 @@ static const struct atp_info geyser4_info = {
>> .yfact = 64,
>> .datalen = 64,
>> .callback = atp_complete_geyser_3_4,
>> + .fuzz = 0,
>> + .threshold = 3,
>> };
>>
>> #define ATP_DEVICE(prod, info) \
>> @@ -155,18 +167,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
>> #define ATP_XSENSORS 26
>> #define ATP_YSENSORS 16
>>
>> -/* amount of fuzz this touchpad generates */
>> -#define ATP_FUZZ 16
>> -
>> /* maximum pressure this driver will report */
>> #define ATP_PRESSURE 300
>>
>> -/*
>> - * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
>> - * ignored.
>> - */
>> -#define ATP_THRESHOLD 5
>> -
>> /* Geyser initialization constants */
>> #define ATP_GEYSER_MODE_READ_REQUEST_ID 1
>> #define ATP_GEYSER_MODE_WRITE_REQUEST_ID 9
>> @@ -235,14 +238,20 @@ MODULE_AUTHOR("Sven Anders");
>> MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
>> MODULE_LICENSE("GPL");
>>
>> -/*
>> - * Make the threshold a module parameter
>> - */
>> -static int threshold = ATP_THRESHOLD;
>> +static int threshold = -1;
>> module_param(threshold, int, 0644);
>> MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
>> " (the trackpad has many of these sensors)"
>> - " less than this value.");
>> + " less than this value. Devices have defaults;"
>> + " this parameter overrides them.");
>> +static int fuzz = -1;
>> +
>> +module_param(fuzz, int, 0644);
>> +MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
>> + " this is, the less sensitive the trackpad"
>> + " will feel, but values too low may generate"
>> + " random movement. Devices have defaults;"
>> + " this parameter overrides them.");
>
> Given that there already is a userland interface for the fuzz parameter - and it
> is indeed in frequent use - this one does not seem warranted. If we are unsure
> if the changes to the default are not all good, perhaps they should not be
> changed at all?
Forgive my ignorance - how would I interact with this parameter in
userland? I searched a bit but didn't find anything. If it's trivial to
fiddle with then I'm OK with dropping it as a module parameter.
I'm reasonably confident the defaults should change. They are based on
positive comments from multiple people with different hardware who
recompiled the driver with these values.
The majority of the benefit they saw was likely from the smaller
threshold value, due to its influence on the return value of
atp_calculate_abs. The second patch of this set takes it completely out
of the equation; however, there are still some small benefits to be
gained from changing these defaults. The threshold is still used for
finger count calculation and benefits from the increased sensitivity,
and the fuzz setting seems to inhibit more granular (1-2 px) cursor
movements.
>
>>
>> static int debug;
>> module_param(debug, int, 0644);
>> @@ -455,7 +464,7 @@ static void atp_detect_size(struct atp *dev)
>> input_set_abs_params(dev->input, ABS_X, 0,
>> (dev->info->xsensors_17 - 1) *
>> dev->info->xfact - 1,
>> - ATP_FUZZ, 0);
>> + fuzz, 0);
>> break;
>> }
>> }
>> @@ -808,6 +817,11 @@ static int atp_probe(struct usb_interface *iface,
>> dev->info = info;
>> dev->overflow_warned = false;
>>
>> + if (fuzz < 0)
>> + fuzz = dev->info->fuzz;
>> + if (threshold < 0)
>> + threshold = dev->info->threshold;
>> +
>> dev->urb = usb_alloc_urb(0, GFP_KERNEL);
>> if (!dev->urb)
>> goto err_free_devs;
>> @@ -843,10 +857,10 @@ static int atp_probe(struct usb_interface *iface,
>>
>> input_set_abs_params(input_dev, ABS_X, 0,
>> (dev->info->xsensors - 1) * dev->info->xfact - 1,
>> - ATP_FUZZ, 0);
>> + fuzz, 0);
>> input_set_abs_params(input_dev, ABS_Y, 0,
>> (dev->info->ysensors - 1) * dev->info->yfact - 1,
>> - ATP_FUZZ, 0);
>> + fuzz, 0);
>> input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>>
>> set_bit(EV_KEY, input_dev->evbit);
>>
>
> Thanks,
> Henrik
>
Thanks,
Clinton
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm
2014-02-09 12:15 ` Henrik Rydberg
@ 2014-02-11 7:19 ` Clinton Sprain
0 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-02-11 7:19 UTC (permalink / raw)
To: Henrik Rydberg, linux-input; +Cc: Dmitry Torokhov
Hi Henrik,
On 02/09/2014 06:15 AM, Henrik Rydberg wrote:
> Hi Clinton,
>
>> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
>>
>> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
>> ---
>> drivers/input/mouse/appletouch.c | 60 ++++++++++++++++++++++++++------------
>> 1 file changed, 42 insertions(+), 18 deletions(-)
>
> I like this patch, but there are some technical glitches, comments below.
>
>> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
>> index d48181b..edbdd95 100644
>> --- a/drivers/input/mouse/appletouch.c
>> +++ b/drivers/input/mouse/appletouch.c
>> @@ -338,21 +338,51 @@ static void atp_reinit(struct work_struct *work)
>> static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>> int *z, int *fingers)
>> {
>> - int i;
>> + int i, k;
>> + int smooth[nb_sensors + 2];
>> + int smooth_tmp[nb_sensors + 2];
>> +
>> /* values to calculate mean */
>> int pcum = 0, psum = 0;
>> int is_increasing = 0;
>>
>> *fingers = 0;
>>
>> + /*
>> + * Use a smoothed version of sensor data for movement calculations, to
>> + * combat noise without needing to toss out values based on a threshold.
>> + * This improves tracking. Finger count is calculated separately based
>> + * on raw data.
>> + */
>> +
>> + for (i = 0; i < nb_sensors; i++) { /* Scale up to minimize */
>> + smooth[i] = xy_sensors[i] << 12; /* rounding/truncation. */
>> + }
>> +
>> + /* Mitigate some of the data loss from smoothing on the edge sensors. */
>> + smooth[-1] = smooth[0] >> 2;
>> + smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
>
> Out of bounds array... also, the boundary condition seems odd. If you want to
> extrapolate the edge velocity, and assuming smooth[0] is the first sensor, you
> would get something like (N = nb_sensors)
>
> smooth_tmp[0] = 2 * smooth[1] - smooth[2];
> smooth_tmp[N-1] = 2 * smooth[N-2] - smooth[N-3];
>
This produces a big velocity increase when the finger crosses over from sensor 0 to sensor 1 or vice versa.
We might get by with something relatively simple:
smooth_tmp[0] = (3 * smooth[0] + smooth[1]) >> 2;
...and the same thing on the other edge. This produces predictable behavior while keeping values from bleeding off the board so to speak. (Less important now than if someone down the line ever decides to increase the number of passes.)
>> + for (k = 0; k < 4; k++) {
>> + for (i = 0; i < nb_sensors; i++) { /* Blend with neighbors. */
>
> And here would be for (i = 1; i < nb_sensors - 1; i++)
>
>> + smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
>> + }
>> +
>> + for (i = 0; i < nb_sensors; i++) {
>> + smooth[i] = smooth_tmp[i];
>> + if (k == 3) /* Last go-round, so scale back down. */
>> + smooth[i] = smooth[i] >> 12;
>
> The scale-up is done separately, so why not make the scale-down separately too?
>
>> + }
>> +
>> + smooth[-1] = smooth[0] >> 2;
>> + smooth[nb_sensors] = smooth[nb_sensors - 1] >> 2;
>
> And these would be dropped.
>
>> + }
>> +
>> for (i = 0; i < nb_sensors; i++) {
>> if (xy_sensors[i] < threshold) {
>> if (is_increasing)
>> is_increasing = 0;
>>
>> - continue;
>> - }
>> -
>> /*
>> * Makes the finger detection more versatile. For example,
>> * two fingers with no gap will be detected. Also, my
>> @@ -367,7 +397,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>> *
>> * - Jason Parekh <jasonparekh@gmail.com>
>> */
>> - if (i < 1 ||
>> +
>> + } else if (i < 1 ||
>> (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
>> (*fingers)++;
>> is_increasing = 1;
>> @@ -375,15 +406,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
>> is_increasing = 0;
>> }
>>
>> - /*
>> - * Subtracts threshold so a high sensor that just passes the
>> - * threshold won't skew the calculated absolute coordinate.
>> - * Fixes an issue where slowly moving the mouse would
>> - * occasionally jump a number of pixels (slowly moving the
>> - * finger makes this issue most apparent.)
>> - */
>> - pcum += (xy_sensors[i] - threshold) * i;
>> - psum += (xy_sensors[i] - threshold);
>> + pcum += (smooth[i]) * i;
>> + psum += (smooth[i]);
>
> Great, this makes so much more sense.
>
>> }
>>
>> if (psum > 0) {
>> @@ -565,8 +589,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>>
>> if (x && y) {
>> if (dev->x_old != -1) {
>> - x = (dev->x_old * 3 + x) >> 2;
>> - y = (dev->y_old * 3 + y) >> 2;
>> + x = (dev->x_old * 7 + x) >> 3;
>> + y = (dev->y_old * 7 + y) >> 3;
>> dev->x_old = x;
>> dev->y_old = y;
>>
>> @@ -677,8 +701,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>>
>> if (x && y) {
>> if (dev->x_old != -1) {
>> - x = (dev->x_old * 3 + x) >> 2;
>> - y = (dev->y_old * 3 + y) >> 2;
>> + x = (dev->x_old * 7 + x) >> 3;
>> + y = (dev->y_old * 7 + y) >> 3;
>> dev->x_old = x;
>> dev->y_old = y;
>
> Would you say that the cursor slow-down here is noticeable, or are there enough
> samples per second that it does not matter?
Noticeable but not major. It's still possible to move the cursor across the screen in one movement with typical defaults, and the gain in smoothness is pretty big, so it seems like a reasonable trade-off. (The 'base' speed is also still quicker than the native OS driver, FWIW.)
>
> Thanks,
> Henrik
>
Thanks,
Clinton
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement
2014-01-18 1:52 [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
` (3 preceding siblings ...)
2014-02-07 0:38 ` [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
@ 2014-03-09 6:03 ` Clinton Sprain
2014-03-09 6:05 ` [PATCH v3 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
` (2 more replies)
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
5 siblings, 3 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-09 6:03 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Changes from last time:
1: Dropped fuzz as a module parameter; also changed threshold defaults back to 5, as I found an outside case where the lower threshold was problematic for finger counting
2: Nix negative subscripts, make smooth array large enough that boundary condition is basically irrelevant (though still handled just in case), scale up and down a little bit more effectively, style
3: Unchanged, except for setting dev->fingers_old to 0 when x and y don't come back with coordinates
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
Clinton Sprain (3):
input: appletouch: dial back fuzz setting
input: appletouch: implement sensor data smoothing
input: appletouch: fix jumps when additional fingers are detected
drivers/input/mouse/appletouch.c | 114 +++++++++++++++++++++++++++-----------
1 file changed, 83 insertions(+), 31 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 1/3] input: appletouch: dial back fuzz setting
2014-03-09 6:03 ` [PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
@ 2014-03-09 6:05 ` Clinton Sprain
2014-03-09 6:17 ` [PATCH v3 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
2014-03-09 6:19 ` [PATCH v3 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-09 6:05 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Dials back the default fuzz setting for most devices using this driver, based
on values from user feedback from forums and bug reports.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 800ca7d..2745832 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -48,6 +48,7 @@ struct atp_info {
int yfact; /* Y multiplication factor */
int datalen; /* size of USB transfers */
void (*callback)(struct urb *); /* callback function */
+ int fuzz; /* fuzz touchpad generates */
};
static void atp_complete_geyser_1_2(struct urb *urb);
@@ -61,6 +62,7 @@ static const struct atp_info fountain_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
};
static const struct atp_info geyser1_info = {
@@ -71,6 +73,7 @@ static const struct atp_info geyser1_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
};
static const struct atp_info geyser2_info = {
@@ -81,6 +84,7 @@ static const struct atp_info geyser2_info = {
.yfact = 43,
.datalen = 64,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 0,
};
static const struct atp_info geyser3_info = {
@@ -90,6 +94,7 @@ static const struct atp_info geyser3_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
};
static const struct atp_info geyser4_info = {
@@ -99,6 +104,7 @@ static const struct atp_info geyser4_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
};
#define ATP_DEVICE(prod, info) \
@@ -155,9 +161,6 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ 16
-
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
@@ -455,7 +458,7 @@ static void atp_detect_size(struct atp *dev)
input_set_abs_params(dev->input, ABS_X, 0,
(dev->info->xsensors_17 - 1) *
dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ dev->info->fuzz, 0);
break;
}
}
@@ -843,10 +846,10 @@ static int atp_probe(struct usb_interface *iface,
input_set_abs_params(input_dev, ABS_X, 0,
(dev->info->xsensors - 1) * dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ dev->info->fuzz, 0);
input_set_abs_params(input_dev, ABS_Y, 0,
(dev->info->ysensors - 1) * dev->info->yfact - 1,
- ATP_FUZZ, 0);
+ dev->info->fuzz, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 2/3] input: appletouch: implement sensor data smoothing
2014-03-09 6:03 ` [PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-09 6:05 ` [PATCH v3 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
@ 2014-03-09 6:17 ` Clinton Sprain
2014-03-09 13:54 ` Henrik Rydberg
2014-03-09 6:19 ` [PATCH v3 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2 siblings, 1 reply; 30+ messages in thread
From: Clinton Sprain @ 2014-03-09 6:17 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 72 ++++++++++++++++++++++++++++----------
1 file changed, 53 insertions(+), 19 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 2745832..0252bab 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
- int i;
+ int i, k;
+ int smooth[nb_sensors + 8];
+ int smooth_tmp[nb_sensors + 8];
+ int scale = 12;
+
/* values to calculate mean */
int pcum = 0, psum = 0;
int is_increasing = 0;
@@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
if (is_increasing)
is_increasing = 0;
- continue;
- }
-
/*
* Makes the finger detection more versatile. For example,
* two fingers with no gap will be detected. Also, my
@@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
*
* - Jason Parekh <jasonparekh@gmail.com>
*/
- if (i < 1 ||
+
+ } else if (i < 1 ||
(!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
(*fingers)++;
is_increasing = 1;
} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
is_increasing = 0;
}
+ }
- /*
- * Subtracts threshold so a high sensor that just passes the
- * threshold won't skew the calculated absolute coordinate.
- * Fixes an issue where slowly moving the mouse would
- * occasionally jump a number of pixels (slowly moving the
- * finger makes this issue most apparent.)
- */
- pcum += (xy_sensors[i] - threshold) * i;
- psum += (xy_sensors[i] - threshold);
+ /*
+ * Use a smoothed version of sensor data for movement calculations, to
+ * combat noise without needing to rely so heavily on a threshold.
+ * This improves tracking.
+ *
+ * The smoothed array is bigger than the original so that the smoothing
+ * doesn't result in edge values being truncated.
+ */
+
+ for (i = 0; i < 4; i++)
+ smooth[i] = 0;
+ for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
+ smooth[i] = 0;
+
+ /* Pull base values, scaled up to help avoid truncation errors. */
+
+ for (i = 0; i < nb_sensors; i++)
+ smooth[i + 4] = xy_sensors[i] << scale;
+
+ for (k = 0; k < 4; k++) {
+
+ /* Handle edge. */
+ smooth_tmp[0] = (smooth[0] * 1 + smooth[1]) >> 1;
+
+ /* Average values with neighbors. */
+ for (i = 1; i < nb_sensors + 7; i++)
+ smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+
+ /* Handle other edge. */
+ smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
+
+ for (i = 0; i < nb_sensors + 7; i++)
+ smooth[i] = smooth_tmp[i];
+ }
+
+ for (i = 0; i < nb_sensors + 8; i++) {
+ if ((smooth[i] >> scale) > 0) { /* Skip individual values if */
+ pcum += (smooth[i]) * i; /* they are small enough to */
+ psum += (smooth[i]); /* be truncated to 0 by our */
+ } /* scale; mostly just noise. */
}
if (psum > 0) {
- *z = psum;
+ *z = psum >> scale; /* Scale down pressure output. */
return pcum * fact / psum;
}
@@ -559,8 +593,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
@@ -671,8 +705,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 3/3] input: appletouch: fix jumps when additional fingers are detected
2014-03-09 6:03 ` [PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-09 6:05 ` [PATCH v3 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
2014-03-09 6:17 ` [PATCH v3 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
@ 2014-03-09 6:19 ` Clinton Sprain
2 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-09 6:19 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Addresses issues related to when a second finger enters or leaves the field,
causing the cursor to jump or the page to scroll unexpectedly; now, we
discard any movement change that happens at the exact moment we detect a
change in the number of fingers touching the trackpad. This doesn't
completely resolve the issue but does greatly mitigate it.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 0252bab..0c7bc6e 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -206,6 +206,7 @@ struct atp {
bool valid; /* are the samples valid? */
bool size_detect_done;
bool overflow_warned;
+ int fingers_old; /* last reported finger count */
int x_old; /* last reported x/y, */
int y_old; /* used for smoothing */
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
@@ -508,7 +509,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -591,7 +592,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -608,7 +611,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -616,6 +619,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -624,6 +628,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
@@ -641,7 +649,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -703,7 +711,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -720,7 +730,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -728,6 +738,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -736,6 +747,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3 2/3] input: appletouch: implement sensor data smoothing
2014-03-09 6:17 ` [PATCH v3 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
@ 2014-03-09 13:54 ` Henrik Rydberg
2014-03-09 15:03 ` Clinton Sprain
0 siblings, 1 reply; 30+ messages in thread
From: Henrik Rydberg @ 2014-03-09 13:54 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
Hi Clinton,
> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
Great patch, just one thing:
> + /* Handle other edge. */
> + smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
> +
> + for (i = 0; i < nb_sensors + 7; i++)
> + smooth[i] = smooth_tmp[i];
Should be "< nb_sensors + 8" here, no?
Thanks,
Henrik
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 2/3] input: appletouch: implement sensor data smoothing
2014-03-09 13:54 ` Henrik Rydberg
@ 2014-03-09 15:03 ` Clinton Sprain
0 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-09 15:03 UTC (permalink / raw)
To: Henrik Rydberg, linux-input; +Cc: Dmitry Torokhov
Ah, you are correct. Good catch. The last few far edge values are guaranteed to be 0 on all but the older 17" devices so I didn't see anything askew.
Thanks,
Clinton
On 03/09/2014 08:54 AM, Henrik Rydberg wrote:
> Hi Clinton,
>
>> Use smoothed version of sensor array data to calculate movement and add weight
>> to prior values when calculating average. This gives more granular and more
>> predictable movement.
>
> Great patch, just one thing:
>
>> + /* Handle other edge. */
>> + smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
>> +
>> + for (i = 0; i < nb_sensors + 7; i++)
>> + smooth[i] = smooth_tmp[i];
>
> Should be "< nb_sensors + 8" here, no?
>
> Thanks,
> Henrik
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement
2014-01-18 1:52 [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
` (4 preceding siblings ...)
2014-03-09 6:03 ` [PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
@ 2014-03-12 23:13 ` Clinton Sprain
2014-03-12 23:14 ` [PATCH v4 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
` (4 more replies)
5 siblings, 5 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-12 23:13 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
input: appletouch: fixes for jagged/uneven cursor movement
The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.
These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
Changes since v3:
1: No change
2: Fix loop counter in atp_calculate_abs
3: No change
Clinton Sprain (3):
input: appletouch: dial back fuzz setting
input: appletouch: implement sensor data smoothing
input: appletouch: fix jumps when additional fingers are detected
drivers/input/mouse/appletouch.c | 114 +++++++++++++++++++++++++++-----------
1 file changed, 83 insertions(+), 31 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/3] input: appletouch: dial back fuzz setting
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
@ 2014-03-12 23:14 ` Clinton Sprain
2014-03-12 23:16 ` [PATCH v4 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
` (3 subsequent siblings)
4 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-12 23:14 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
input: appletouch: dial back fuzz setting
Dials back the default fuzz setting for most devices using this driver, based on values from user feedback from forums and bug reports.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 800ca7d..2745832 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -48,6 +48,7 @@ struct atp_info {
int yfact; /* Y multiplication factor */
int datalen; /* size of USB transfers */
void (*callback)(struct urb *); /* callback function */
+ int fuzz; /* fuzz touchpad generates */
};
static void atp_complete_geyser_1_2(struct urb *urb);
@@ -61,6 +62,7 @@ static const struct atp_info fountain_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
};
static const struct atp_info geyser1_info = {
@@ -71,6 +73,7 @@ static const struct atp_info geyser1_info = {
.yfact = 43,
.datalen = 81,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 16,
};
static const struct atp_info geyser2_info = {
@@ -81,6 +84,7 @@ static const struct atp_info geyser2_info = {
.yfact = 43,
.datalen = 64,
.callback = atp_complete_geyser_1_2,
+ .fuzz = 0,
};
static const struct atp_info geyser3_info = {
@@ -90,6 +94,7 @@ static const struct atp_info geyser3_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
};
static const struct atp_info geyser4_info = {
@@ -99,6 +104,7 @@ static const struct atp_info geyser4_info = {
.yfact = 64,
.datalen = 64,
.callback = atp_complete_geyser_3_4,
+ .fuzz = 0,
};
#define ATP_DEVICE(prod, info) \
@@ -155,9 +161,6 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ 16
-
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
@@ -455,7 +458,7 @@ static void atp_detect_size(struct atp *dev)
input_set_abs_params(dev->input, ABS_X, 0,
(dev->info->xsensors_17 - 1) *
dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ dev->info->fuzz, 0);
break;
}
}
@@ -843,10 +846,10 @@ static int atp_probe(struct usb_interface *iface,
input_set_abs_params(input_dev, ABS_X, 0,
(dev->info->xsensors - 1) * dev->info->xfact - 1,
- ATP_FUZZ, 0);
+ dev->info->fuzz, 0);
input_set_abs_params(input_dev, ABS_Y, 0,
(dev->info->ysensors - 1) * dev->info->yfact - 1,
- ATP_FUZZ, 0);
+ dev->info->fuzz, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 2/3] input: appletouch: implement sensor data smoothing
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-12 23:14 ` [PATCH v4 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
@ 2014-03-12 23:16 ` Clinton Sprain
2014-03-23 13:59 ` Henrik Rydberg
2014-03-27 18:26 ` Dmitry Torokhov
2014-03-12 23:17 ` [PATCH v4 " Clinton Sprain
` (2 subsequent siblings)
4 siblings, 2 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-12 23:16 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
input: appletouch: implement sensor data smoothing
Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 72 ++++++++++++++++++++++++++++----------
1 file changed, 53 insertions(+), 19 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 2745832..e00f126 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
- int i;
+ int i, k;
+ int smooth[nb_sensors + 8];
+ int smooth_tmp[nb_sensors + 8];
+ int scale = 12;
+
/* values to calculate mean */
int pcum = 0, psum = 0;
int is_increasing = 0;
@@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
if (is_increasing)
is_increasing = 0;
- continue;
- }
-
/*
* Makes the finger detection more versatile. For example,
* two fingers with no gap will be detected. Also, my
@@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
*
* - Jason Parekh <jasonparekh@gmail.com>
*/
- if (i < 1 ||
+
+ } else if (i < 1 ||
(!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
(*fingers)++;
is_increasing = 1;
} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
is_increasing = 0;
}
+ }
- /*
- * Subtracts threshold so a high sensor that just passes the
- * threshold won't skew the calculated absolute coordinate.
- * Fixes an issue where slowly moving the mouse would
- * occasionally jump a number of pixels (slowly moving the
- * finger makes this issue most apparent.)
- */
- pcum += (xy_sensors[i] - threshold) * i;
- psum += (xy_sensors[i] - threshold);
+ /*
+ * Use a smoothed version of sensor data for movement calculations, to
+ * combat noise without needing to rely so heavily on a threshold.
+ * This improves tracking.
+ *
+ * The smoothed array is bigger than the original so that the smoothing
+ * doesn't result in edge values being truncated.
+ */
+
+ for (i = 0; i < 4; i++)
+ smooth[i] = 0;
+ for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
+ smooth[i] = 0;
+
+ /* Pull base values, scaled up to help avoid truncation errors. */
+
+ for (i = 0; i < nb_sensors; i++)
+ smooth[i + 4] = xy_sensors[i] << scale;
+
+ for (k = 0; k < 4; k++) {
+
+ /* Handle edge. */
+ smooth_tmp[0] = (smooth[0] + smooth[1]) >> 1;
+
+ /* Average values with neighbors. */
+ for (i = 1; i < nb_sensors + 7; i++)
+ smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
+
+ /* Handle other edge. */
+ smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
+
+ for (i = 0; i < nb_sensors + 8; i++)
+ smooth[i] = smooth_tmp[i];
+ }
+
+ for (i = 0; i < nb_sensors + 8; i++) {
+ if ((smooth[i] >> scale) > 0) { /* Skip individual values if */
+ pcum += (smooth[i]) * i; /* they are small enough to */
+ psum += (smooth[i]); /* be truncated to 0 by our */
+ } /* scale; mostly just noise. */
}
if (psum > 0) {
- *z = psum;
+ *z = psum >> scale; /* Scale down pressure output. */
return pcum * fact / psum;
}
@@ -559,8 +593,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
@@ -671,8 +705,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 3/3] input: appletouch: fix jumps when additional fingers are detected
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-12 23:14 ` [PATCH v4 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
2014-03-12 23:16 ` [PATCH v4 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
@ 2014-03-12 23:17 ` Clinton Sprain
2014-03-23 13:34 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-23 14:02 ` Henrik Rydberg
4 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-12 23:17 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
input: appletouch: fix jumps when additional fingers are detected
Addresses issues related to when a second finger enters or leaves the field,
causing the cursor to jump or the page to scroll unexpectedly; now, we
discard any movement change that happens at the exact moment we detect a
change in the number of fingers touching the trackpad. This doesn't
completely resolve the issue but does greatly mitigate it.
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e00f126..5140293 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -206,6 +206,7 @@ struct atp {
bool valid; /* are the samples valid? */
bool size_detect_done;
bool overflow_warned;
+ int fingers_old; /* last reported finger count */
int x_old; /* last reported x/y, */
int y_old; /* used for smoothing */
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
@@ -508,7 +509,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -591,7 +592,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -608,7 +611,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -616,6 +619,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -624,6 +628,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
@@ -641,7 +649,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -703,7 +711,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
dev->info->yfact, &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -720,7 +730,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -728,6 +738,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -736,6 +747,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
` (2 preceding siblings ...)
2014-03-12 23:17 ` [PATCH v4 " Clinton Sprain
@ 2014-03-23 13:34 ` Clinton Sprain
2014-03-23 14:02 ` Henrik Rydberg
4 siblings, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-23 13:34 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
Ping?
On 03/12/2014 06:13 PM, Clinton Sprain wrote:
> input: appletouch: fixes for jagged/uneven cursor movement
>
> The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.
>
> These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
>
> Changes since v3:
>
> 1: No change
> 2: Fix loop counter in atp_calculate_abs
> 3: No change
>
> Clinton Sprain (3):
> input: appletouch: dial back fuzz setting
> input: appletouch: implement sensor data smoothing
> input: appletouch: fix jumps when additional fingers are detected
>
> drivers/input/mouse/appletouch.c | 114 +++++++++++++++++++++++++++-----------
> 1 file changed, 83 insertions(+), 31 deletions(-)
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 2/3] input: appletouch: implement sensor data smoothing
2014-03-12 23:16 ` [PATCH v4 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
@ 2014-03-23 13:59 ` Henrik Rydberg
2014-03-27 18:26 ` Dmitry Torokhov
1 sibling, 0 replies; 30+ messages in thread
From: Henrik Rydberg @ 2014-03-23 13:59 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
Hi Clinton,
> input: appletouch: implement sensor data smoothing
>
> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
>
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
> drivers/input/mouse/appletouch.c | 72 ++++++++++++++++++++++++++++----------
> 1 file changed, 53 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 2745832..e00f126 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
> static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> int *z, int *fingers)
> {
> - int i;
> + int i, k;
> + int smooth[nb_sensors + 8];
> + int smooth_tmp[nb_sensors + 8];
> + int scale = 12;
> +
> /* values to calculate mean */
> int pcum = 0, psum = 0;
> int is_increasing = 0;
> @@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> if (is_increasing)
> is_increasing = 0;
>
> - continue;
> - }
> -
> /*
> * Makes the finger detection more versatile. For example,
> * two fingers with no gap will be detected. Also, my
> @@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> *
> * - Jason Parekh <jasonparekh@gmail.com>
> */
> - if (i < 1 ||
> +
> + } else if (i < 1 ||
> (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
> (*fingers)++;
> is_increasing = 1;
> } else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
> is_increasing = 0;
> }
> + }
>
> - /*
> - * Subtracts threshold so a high sensor that just passes the
> - * threshold won't skew the calculated absolute coordinate.
> - * Fixes an issue where slowly moving the mouse would
> - * occasionally jump a number of pixels (slowly moving the
> - * finger makes this issue most apparent.)
> - */
> - pcum += (xy_sensors[i] - threshold) * i;
> - psum += (xy_sensors[i] - threshold);
> + /*
> + * Use a smoothed version of sensor data for movement calculations, to
> + * combat noise without needing to rely so heavily on a threshold.
> + * This improves tracking.
> + *
> + * The smoothed array is bigger than the original so that the smoothing
> + * doesn't result in edge values being truncated.
> + */
> +
> + for (i = 0; i < 4; i++)
> + smooth[i] = 0;
> + for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
> + smooth[i] = 0;
> +
> + /* Pull base values, scaled up to help avoid truncation errors. */
> +
> + for (i = 0; i < nb_sensors; i++)
> + smooth[i + 4] = xy_sensors[i] << scale;
> +
> + for (k = 0; k < 4; k++) {
> +
> + /* Handle edge. */
> + smooth_tmp[0] = (smooth[0] + smooth[1]) >> 1;
> +
> + /* Average values with neighbors. */
> + for (i = 1; i < nb_sensors + 7; i++)
> + smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
> +
> + /* Handle other edge. */
> + smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
> +
> + for (i = 0; i < nb_sensors + 8; i++)
> + smooth[i] = smooth_tmp[i];
> + }
> +
> + for (i = 0; i < nb_sensors + 8; i++) {
> + if ((smooth[i] >> scale) > 0) { /* Skip individual values if */
> + pcum += (smooth[i]) * i; /* they are small enough to */
> + psum += (smooth[i]); /* be truncated to 0 by our */
> + } /* scale; mostly just noise. */
> }
>
> if (psum > 0) {
> - *z = psum;
> + *z = psum >> scale; /* Scale down pressure output. */
> return pcum * fact / psum;
> }
>
> @@ -559,8 +593,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>
> if (x && y) {
> if (dev->x_old != -1) {
> - x = (dev->x_old * 3 + x) >> 2;
> - y = (dev->y_old * 3 + y) >> 2;
> + x = (dev->x_old * 7 + x) >> 3;
> + y = (dev->y_old * 7 + y) >> 3;
> dev->x_old = x;
> dev->y_old = y;
>
> @@ -671,8 +705,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>
> if (x && y) {
> if (dev->x_old != -1) {
> - x = (dev->x_old * 3 + x) >> 2;
> - y = (dev->y_old * 3 + y) >> 2;
> + x = (dev->x_old * 7 + x) >> 3;
> + y = (dev->y_old * 7 + y) >> 3;
> dev->x_old = x;
> dev->y_old = y;
>
>
Reviewed-by: Henrik Rydberg <rydberg@euromail.se>
Thanks,
Henrik
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
` (3 preceding siblings ...)
2014-03-23 13:34 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
@ 2014-03-23 14:02 ` Henrik Rydberg
4 siblings, 0 replies; 30+ messages in thread
From: Henrik Rydberg @ 2014-03-23 14:02 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
> input: appletouch: fixes for jagged/uneven cursor movement
>
> The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.
>
> These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
Thanks Clinton, for resolving this long-standing nuisance. It all looks good to
me now.
Henrik
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 2/3] input: appletouch: implement sensor data smoothing
2014-03-12 23:16 ` [PATCH v4 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
2014-03-23 13:59 ` Henrik Rydberg
@ 2014-03-27 18:26 ` Dmitry Torokhov
2014-03-29 21:47 ` [PATCH v5 " Clinton Sprain
2014-03-29 21:48 ` [PATCH v5 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
1 sibling, 2 replies; 30+ messages in thread
From: Dmitry Torokhov @ 2014-03-27 18:26 UTC (permalink / raw)
To: Clinton Sprain; +Cc: linux-input, Henrik Rydberg
Hi Clinton,
On Wed, Mar 12, 2014 at 06:16:21PM -0500, Clinton Sprain wrote:
> input: appletouch: implement sensor data smoothing
>
> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
>
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
> drivers/input/mouse/appletouch.c | 72 ++++++++++++++++++++++++++++----------
> 1 file changed, 53 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 2745832..e00f126 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
> static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> int *z, int *fingers)
> {
> - int i;
> + int i, k;
> + int smooth[nb_sensors + 8];
> + int smooth_tmp[nb_sensors + 8];
This unfortunately introduces variable-length arraus on stack which
makes sparse unhappy. Can we add these scratch buffers to the device
structure instead?
> + int scale = 12;
Probably better use a define rather than a variable.
> +
> /* values to calculate mean */
> int pcum = 0, psum = 0;
> int is_increasing = 0;
> @@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> if (is_increasing)
> is_increasing = 0;
>
> - continue;
> - }
> -
> /*
> * Makes the finger detection more versatile. For example,
> * two fingers with no gap will be detected. Also, my
> @@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> *
> * - Jason Parekh <jasonparekh@gmail.com>
> */
> - if (i < 1 ||
> +
> + } else if (i < 1 ||
> (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
> (*fingers)++;
> is_increasing = 1;
> } else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
> is_increasing = 0;
> }
> + }
>
> - /*
> - * Subtracts threshold so a high sensor that just passes the
> - * threshold won't skew the calculated absolute coordinate.
> - * Fixes an issue where slowly moving the mouse would
> - * occasionally jump a number of pixels (slowly moving the
> - * finger makes this issue most apparent.)
> - */
> - pcum += (xy_sensors[i] - threshold) * i;
> - psum += (xy_sensors[i] - threshold);
> + /*
> + * Use a smoothed version of sensor data for movement calculations, to
> + * combat noise without needing to rely so heavily on a threshold.
> + * This improves tracking.
> + *
> + * The smoothed array is bigger than the original so that the smoothing
> + * doesn't result in edge values being truncated.
> + */
> +
> + for (i = 0; i < 4; i++)
> + smooth[i] = 0;
memset might be better?
> + for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
> + smooth[i] = 0;
And here as well.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 2/3] input: appletouch: implement sensor data smoothing
2014-03-27 18:26 ` Dmitry Torokhov
@ 2014-03-29 21:47 ` Clinton Sprain
2014-03-29 21:48 ` [PATCH v5 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
1 sibling, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-29 21:47 UTC (permalink / raw)
To: Dmitry Torokhov, Clinton Sprain; +Cc: linux-input, Henrik Rydberg
input: appletouch: implement sensor data smoothing
Use smoothed version of sensor array data to calculate movement and add weight
to prior values when calculating average. This gives more granular and more
predictable movement.
Changes since v4:
- Scale is now defined as ATP_SCALE
- Smoothing scratch buffers now a part of dev structure
- Now passing dev into atp_calculate_abs
- Now leaving atp_calculate_abs early if no fingers are detected
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 110 +++++++++++++++++++++++++++-----------
1 file changed, 80 insertions(+), 30 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 2745832..8e01363 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -161,6 +161,12 @@ MODULE_DEVICE_TABLE(usb, atp_table);
#define ATP_XSENSORS 26
#define ATP_YSENSORS 16
+/*
+ * The largest possible bank of sensors with additional buffer of 4 extra values
+ * on either side, for an array of smoothed sensor values.
+ */
+#define ATP_SMOOTHSIZE 34
+
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
@@ -168,7 +174,13 @@ MODULE_DEVICE_TABLE(usb, atp_table);
* Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
* ignored.
*/
-#define ATP_THRESHOLD 5
+#define ATP_THRESHOLD 5
+
+/*
+ * How far we'll bitshift our sensor values before averaging them. Mitigates
+ * rounding errors.
+ */
+#define ATP_SCALE 12
/* Geyser initialization constants */
#define ATP_GEYSER_MODE_READ_REQUEST_ID 1
@@ -211,6 +223,8 @@ struct atp {
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
signed char xy_old[ATP_XSENSORS + ATP_YSENSORS];
int xy_acc[ATP_XSENSORS + ATP_YSENSORS];
+ int smooth[ATP_SMOOTHSIZE];
+ int smooth_tmp[ATP_SMOOTHSIZE];
int idlecount; /* number of empty packets */
struct work_struct work;
};
@@ -329,10 +343,15 @@ static void atp_reinit(struct work_struct *work)
retval);
}
-static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
- int *z, int *fingers)
+static int atp_calculate_abs(struct atp *dev, int offset, int nb_sensors,
+ int fact, int *z, int *fingers)
{
- int i;
+ int i, k;
+
+ /* Use offset to point xy_sensors at the first value in dev->xy_acc */
+ /* for whichever dimension we're looking at this particular go-round. */
+ int *xy_sensors = dev->xy_acc + offset;
+
/* values to calculate mean */
int pcum = 0, psum = 0;
int is_increasing = 0;
@@ -344,9 +363,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
if (is_increasing)
is_increasing = 0;
- continue;
- }
-
/*
* Makes the finger detection more versatile. For example,
* two fingers with no gap will be detected. Also, my
@@ -361,27 +377,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
*
* - Jason Parekh <jasonparekh@gmail.com>
*/
- if (i < 1 ||
+
+ } else if (i < 1 ||
(!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
(*fingers)++;
is_increasing = 1;
} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
is_increasing = 0;
}
+ }
- /*
- * Subtracts threshold so a high sensor that just passes the
- * threshold won't skew the calculated absolute coordinate.
- * Fixes an issue where slowly moving the mouse would
- * occasionally jump a number of pixels (slowly moving the
- * finger makes this issue most apparent.)
- */
- pcum += (xy_sensors[i] - threshold) * i;
- psum += (xy_sensors[i] - threshold);
+ if (*fingers < 1) /* No need to continue if no fingers are found. */
+ return 0;
+
+ /*
+ * Use a smoothed version of sensor data for movement calculations, to
+ * combat noise without needing to rely so heavily on a threshold.
+ * This improves tracking.
+ *
+ * The smoothed array is bigger than the original so that the smoothing
+ * doesn't result in edge values being truncated.
+ */
+
+ memset(dev->smooth, 0, sizeof(dev->smooth));
+ memset(dev->smooth_tmp, 0, sizeof(dev->smooth_tmp));
+
+ /* Pull base values, scaled up to help avoid truncation errors. */
+
+ for (i = 0; i < nb_sensors; i++)
+ dev->smooth[i + 4] = xy_sensors[i] << ATP_SCALE;
+
+ for (k = 0; k < 4; k++) {
+ /* Handle edge. */
+ dev->smooth_tmp[0] = (dev->smooth[0] + dev->smooth[1]) >> 1;
+
+ /* Average values with neighbors. */
+ for (i = 1; i < nb_sensors + 7; i++)
+ dev->smooth_tmp[i] = (dev->smooth[i - 1] + dev->smooth[i] * 2 + dev->smooth[i + 1]) >> 2;
+
+ /* Handle other edge. */
+ dev->smooth_tmp[nb_sensors + 7] = (dev->smooth[nb_sensors + 7] + dev->smooth[nb_sensors + 6]) >> 1;
+
+ for (i = 0; i < nb_sensors + 8; i++)
+ dev->smooth[i] = dev->smooth_tmp[i];
+ }
+
+ for (i = 0; i < nb_sensors + 8; i++) {
+ if ((dev->smooth[i] >> ATP_SCALE) > 0) { /* Skip values if */
+ pcum += (dev->smooth[i]) * i; /* they're small enough */
+ psum += (dev->smooth[i]); /* to be truncated to 0 */
+ } /* by scale. Mostly noise. */
}
if (psum > 0) {
- *z = psum;
+ *z = psum >> ATP_SCALE; /* Scale down pressure output. */
return pcum * fact / psum;
}
@@ -551,16 +600,16 @@ static void atp_complete_geyser_1_2(struct urb *urb)
dbg_dump("accumulator", dev->xy_acc);
- x = atp_calculate_abs(dev->xy_acc, ATP_XSENSORS,
- dev->info->xfact, &x_z, &x_f);
- y = atp_calculate_abs(dev->xy_acc + ATP_XSENSORS, ATP_YSENSORS,
- dev->info->yfact, &y_z, &y_f);
+ x = atp_calculate_abs(dev, 0, ATP_XSENSORS, dev->info->xfact,
+ &x_z, &x_f);
+ y = atp_calculate_abs(dev, ATP_XSENSORS, ATP_YSENSORS, dev->info->yfact,
+ &y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
@@ -663,16 +712,17 @@ static void atp_complete_geyser_3_4(struct urb *urb)
dbg_dump("accumulator", dev->xy_acc);
- x = atp_calculate_abs(dev->xy_acc, ATP_XSENSORS,
- dev->info->xfact, &x_z, &x_f);
- y = atp_calculate_abs(dev->xy_acc + ATP_XSENSORS, ATP_YSENSORS,
- dev->info->yfact, &y_z, &y_f);
+ x = atp_calculate_abs(dev, 0, ATP_XSENSORS, dev->info->xfact,
+ &x_z, &x_f);
+ y = atp_calculate_abs(dev, ATP_XSENSORS, ATP_YSENSORS, dev->info->yfact,
+ &y_z, &y_f);
+
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
if (x && y) {
if (dev->x_old != -1) {
- x = (dev->x_old * 3 + x) >> 2;
- y = (dev->y_old * 3 + y) >> 2;
+ x = (dev->x_old * 7 + x) >> 3;
+ y = (dev->y_old * 7 + y) >> 3;
dev->x_old = x;
dev->y_old = y;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 3/3] input: appletouch: fix jumps when additional fingers are detected
2014-03-27 18:26 ` Dmitry Torokhov
2014-03-29 21:47 ` [PATCH v5 " Clinton Sprain
@ 2014-03-29 21:48 ` Clinton Sprain
1 sibling, 0 replies; 30+ messages in thread
From: Clinton Sprain @ 2014-03-29 21:48 UTC (permalink / raw)
To: Dmitry Torokhov, Clinton Sprain; +Cc: linux-input, Henrik Rydberg
input: appletouch: fix jumps when additional fingers are detected
Addresses issues related to when a second finger enters or leaves the field,
causing the cursor to jump or the page to scroll unexpectedly; now, we
discard any movement change that happens at the exact moment we detect a
change in the number of fingers touching the trackpad. This doesn't
completely resolve the issue but does greatly mitigate it.
(No changes since v4)
Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
drivers/input/mouse/appletouch.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 8e01363..39c7ba2 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -218,6 +218,7 @@ struct atp {
bool valid; /* are the samples valid? */
bool size_detect_done;
bool overflow_warned;
+ int fingers_old; /* last reported finger count */
int x_old; /* last reported x/y, */
int y_old; /* used for smoothing */
signed char xy_cur[ATP_XSENSORS + ATP_YSENSORS];
@@ -523,7 +524,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -606,7 +607,9 @@ static void atp_complete_geyser_1_2(struct urb *urb)
&y_z, &y_f);
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -623,7 +626,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -631,6 +634,7 @@ static void atp_complete_geyser_1_2(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -639,6 +643,10 @@ static void atp_complete_geyser_1_2(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
@@ -656,7 +664,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
{
int x, y, x_z, y_z, x_f, y_f;
int retval, i, j;
- int key;
+ int key, fingers;
struct atp *dev = urb->context;
int status = atp_status_check(urb);
@@ -719,7 +727,9 @@ static void atp_complete_geyser_3_4(struct urb *urb)
key = dev->data[dev->info->datalen - 1] & ATP_STATUS_BUTTON;
- if (x && y) {
+ fingers = max(x_f, y_f);
+
+ if (x && y && (fingers == dev->fingers_old)) {
if (dev->x_old != -1) {
x = (dev->x_old * 7 + x) >> 3;
y = (dev->y_old * 7 + y) >> 3;
@@ -736,7 +746,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
input_report_abs(dev->input, ABS_Y, y);
input_report_abs(dev->input, ABS_PRESSURE,
min(ATP_PRESSURE, x_z + y_z));
- atp_report_fingers(dev->input, max(x_f, y_f));
+ atp_report_fingers(dev->input, fingers);
}
dev->x_old = x;
dev->y_old = y;
@@ -744,6 +754,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
} else if (!x && !y) {
dev->x_old = dev->y_old = -1;
+ dev->fingers_old = 0;
input_report_key(dev->input, BTN_TOUCH, 0);
input_report_abs(dev->input, ABS_PRESSURE, 0);
atp_report_fingers(dev->input, 0);
@@ -752,6 +763,10 @@ static void atp_complete_geyser_3_4(struct urb *urb)
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
+ if (fingers != dev->fingers_old)
+ dev->x_old = dev->y_old = -1;
+ dev->fingers_old = fingers;
+
input_report_key(dev->input, BTN_LEFT, key);
input_sync(dev->input);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread
end of thread, other threads:[~2014-03-29 21:48 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-18 1:52 [PATCH 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-01-18 1:55 ` [PATCH 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
2014-01-18 7:07 ` Henrik Rydberg
2014-01-18 1:56 ` [PATCH 2/3] input: appletouch: use better cursor movement smoothing algorithm Clinton Sprain
2014-01-18 1:58 ` [PATCH 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2014-02-07 0:38 ` [PATCH v2 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-02-07 0:40 ` [PATCH v2 1/3] input: appletouch: parametrize and set saner defaults for fuzz and threshold Clinton Sprain
2014-02-09 12:01 ` Henrik Rydberg
2014-02-11 4:46 ` Clinton Sprain
2014-02-07 0:43 ` [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm Clinton Sprain
2014-02-09 12:15 ` Henrik Rydberg
2014-02-11 7:19 ` Clinton Sprain
2014-02-07 0:46 ` [PATCH v2 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2014-02-09 12:16 ` Henrik Rydberg
2014-03-09 6:03 ` [PATCH v3 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-09 6:05 ` [PATCH v3 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
2014-03-09 6:17 ` [PATCH v3 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
2014-03-09 13:54 ` Henrik Rydberg
2014-03-09 15:03 ` Clinton Sprain
2014-03-09 6:19 ` [PATCH v3 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2014-03-12 23:13 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-12 23:14 ` [PATCH v4 1/3] input: appletouch: dial back fuzz setting Clinton Sprain
2014-03-12 23:16 ` [PATCH v4 2/3] input: appletouch: implement sensor data smoothing Clinton Sprain
2014-03-23 13:59 ` Henrik Rydberg
2014-03-27 18:26 ` Dmitry Torokhov
2014-03-29 21:47 ` [PATCH v5 " Clinton Sprain
2014-03-29 21:48 ` [PATCH v5 3/3] input: appletouch: fix jumps when additional fingers are detected Clinton Sprain
2014-03-12 23:17 ` [PATCH v4 " Clinton Sprain
2014-03-23 13:34 ` [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement Clinton Sprain
2014-03-23 14:02 ` Henrik Rydberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.