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 <[email protected]>
> ---
>  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 <[email protected]>
>                */
> -             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

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to