Re: [RFC 6/6] Input: mt: introduce dmax in in-kernel tracking

From: Benjamin Tissoires
Date: Mon Jan 19 2015 - 16:03:12 EST


On Mon, Jan 19, 2015 at 3:53 PM, Benjamin Tissoires
<benjamin.tissoires@xxxxxxxxxx> wrote:
> When tapping a clickpad with two fingers, there is a chance that the sensor
> sees first only one finger, and at the next scan only the second one.
> In this case, the sensors says that there has been only one finger
> on the clickpad, which moved really fast between two scans.
>
> We can try to counter this by adding a limit to what an actual finger
> can move between 2 scans.
> We can try to provide a reasonable distance of 1cm between two scans for
> one finger. This is what is used in the synaptcs.c portion of the patch.
>
> However, this is not really accurate because the resolution in X and in Y
> differs. But heh, that's how the in-kernel tracking works right now, and
> its job is quite good even with this approximation.
>
> Once a driver has set the maximum distance for a jump of a finger between
> two scans, we can tell the in-kernel tracking to dissociate the slots
> and the touch with input_mt_check_slots().
>
> In order not to penalize better sensors, this parameter is not
> automatically enabled, but each driver has to manually set it to a
> reasonable value.
>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=76722
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
> ---
> drivers/input/input-mt.c | 67 +++++++++++++++++++++++++++++++++++++++--
> drivers/input/mouse/synaptics.c | 3 +-
> include/linux/input/mt.h | 4 +++
> 3 files changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index c7b3752..d326262 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -403,9 +403,51 @@ static int input_mt_set_matrix(struct input_mt *mt,
> }
>
> /**
> + * input_mt_check_slots() - validate the slot assignments
> + * @mt: the input_mt handle of the current device
> + * @pos: the position array to match
> + * @slots: the slot assignment to be filled
> + * @num_pos: number of positions
> + *
> + * For each assigned touch in pos, check if the distance between this touch
> + * and the previously reported position is not too big.
> + * If it is too big, break the assignment by resetting the slot value to -1.
> + * While syncing the frame, the previously matched slot will be freed, and
> + * a new one will be assigned in input_mt_set_slots().
> + *
> + * The "too big" parameter is stored in mt->dmax2.
> + */
> +static void input_mt_check_slots(struct input_mt *mt,
> + const struct input_mt_pos *pos, int *slots,
> + int num_pos)
> +{
> + struct input_mt_slot *s;
> + int *p;
> + int i, x, y, d2, dx, dy;
> + struct input_mt_pos *mt_pos;

Grmbl... checkpatch.pl complained about a missing line between the
declarations and the actual code (I first declared mt_pos within the
for loop). So I re-scrambled the things, and of course I forget to
re-add the "const" qualifer. So this line shows a pretty warning which
will be fixed in v2 :(

Cheers,
Benjamin

> +
> + if (!mt->dmax2)
> + return;
> +
> + for (i = 0; i < num_pos; i++) {
> + mt_pos = pos + i;
> + p = slots + i;
> + s = mt->slots + *p;
> + x = input_mt_get_value(s, ABS_MT_POSITION_X);
> + y = input_mt_get_value(s, ABS_MT_POSITION_Y);
> + dx = x - mt_pos->x;
> + dy = y - mt_pos->y;
> + d2 = dx * dx + dy * dy;
> + if (d2 >= mt->dmax2)
> + *p = -1;
> + }
> +}
> +
> +/**
> * input_mt_set_slots() - assign slots to the list of pos thanks to the reduced
> * cost matrix
> * @mt: the input_mt handle of the current device
> + * @pos: the position array to match
> * @slots: the slot assignment to be filled
> * @num_pos: number of positions
> *
> @@ -414,7 +456,8 @@ static int input_mt_set_matrix(struct input_mt *mt,
> * sent slots.
> */
> static void input_mt_set_slots(struct input_mt *mt,
> - int *slots, int num_pos)
> + const struct input_mt_pos *pos, int *slots,
> + int num_pos)
> {
> struct input_mt_slot *s;
> int *red_cost_matrix = mt->red;
> @@ -436,6 +479,8 @@ static void input_mt_set_slots(struct input_mt *mt,
> *p = s - mt->slots;
> }
>
> + input_mt_check_slots(mt, pos, slots, num_pos);
> +
> /* for each unassigned touch in pos, assign a new slot for it */
> for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
> if (input_mt_is_active(s))
> @@ -478,13 +523,31 @@ int input_mt_assign_slots(struct input_dev *dev, int *slots,
> n_rows = n_elems / num_pos;
> n_columns = num_pos;
> find_reduced_matrix(mt->red, n_columns, n_rows, n_elems);
> - input_mt_set_slots(mt, slots, num_pos);
> + input_mt_set_slots(mt, pos, slots, num_pos);
>
> return 0;
> }
> EXPORT_SYMBOL(input_mt_assign_slots);
>
> /**
> + * input_mt_set_dmax_tracking() - enhance tracking
> + * @dev: input device with allocated MT slots
> + * @dmax: the max distance between two touches to be considered as the same
> + *
> + * Enhance the in-kernel tracking by providing a maximum distance between
> + * 2 touches to be considered as the same slot.
> + * The stored value is the square of the provided distance to ease the tests
> + * later on. The input distance has to be given in the device coordinate system.
> + */
> +void input_mt_set_dmax_tracking(struct input_dev *dev, int dmax)
> +{
> + struct input_mt *mt = dev->mt;
> +
> + mt->dmax2 = dmax * dmax;
> +}
> +EXPORT_SYMBOL(input_mt_set_dmax_tracking);
> +
> +/**
> * input_mt_get_slot_by_key() - return slot matching key
> * @dev: input device with allocated MT slots
> * @key: the key of the sought slot
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index f89de89..95c15451 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -1077,7 +1077,8 @@ static void set_input_params(struct psmouse *psmouse,
> ABS_MT_POSITION_Y);
> /* Image sensors can report per-contact pressure */
> input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> - input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
> + input_mt_init_slots(dev, 4, INPUT_MT_POINTER | INPUT_MT_TRACK);
> + input_mt_set_dmax_tracking(dev, 10 * priv->x_res);
>
> /* Image sensors can signal 4 and 5 finger clicks */
> __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
> index f583ff6..c4b35c4 100644
> --- a/include/linux/input/mt.h
> +++ b/include/linux/input/mt.h
> @@ -41,6 +41,7 @@ struct input_mt_slot {
> * @flags: input_mt operation flags
> * @frame: increases every time input_mt_sync_frame() is called
> * @red: reduced cost matrix for in-kernel tracking
> + * @dmax2: max (squared) distance between 2 points when using in-kernel tracking
> * @slots: array of slots holding current values of tracked contacts
> */
> struct input_mt {
> @@ -50,6 +51,7 @@ struct input_mt {
> unsigned int flags;
> unsigned int frame;
> int *red;
> + int dmax2;
> struct input_mt_slot slots[];
> };
>
> @@ -100,6 +102,8 @@ static inline bool input_is_mt_axis(int axis)
> return axis == ABS_MT_SLOT || input_is_mt_value(axis);
> }
>
> +void input_mt_set_dmax_tracking(struct input_dev *dev, int dmax);
> +
> void input_mt_report_slot_state(struct input_dev *dev,
> unsigned int tool_type, bool active);
>
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/