Re: [PATCH v2 2/3] input: appletouch: use better cursor movement smoothing algorithm
From: Henrik Rydberg <hidden>
Date: 2014-02-09 12:13:16
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 <redacted> --- 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.
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
+ } + 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.
quoted hunk ↗ jump to hunk
} 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