Re: [PATCH 7/7] elantech: average the two coordinates when 2 fingers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/01/2010 01:28 PM, Éric Piel wrote:
[...]

>> How about this as a stab at the MT point problem. The idea here is that
>> get_corner_id() returns the rectangle corner which minimizes the change in
>> motion direction, according to the principle of inertia. For all cases,
>> position[0] is reported as the ABS_X/Y point.
>>
>> state = (num_finger, corner_id, motion_id, position[n])
>>
>> At each touch frame update, do
>>
>> 1. num_finger == 1
>>
>> Set
>>   position[0] = finger-position
>>
>> 2. num_finger == 2
>>
>> If last frame was one finger, do
>>   Find corner id such that position change is minimized
>>     id = get_initial_corner_id(position[0], corner-position[id])
>>
>> Else
>>   Find temporary motion id
>>     mid = get_motion_id(position[0], corner-position[corner_id])
>>   Find corner id such that motion change is minimized
>>     id = get_corner_id(corner_id, motion_id, mid)
>>
>> Set
>>   corner_id = id
>>   motion_id = get_motion_id(position[0], corner-position[id])
>>   position[0] = corner-position[id],
>>   position[1] = corner-position[opposite[id]]
>>
>> Report position[0] and position[1] as MT points
>>
>> Since this is all integer logic, it should be possible to detect motion changes
>> as sign changes, such that get_motion_id() and get_corner_id() can be
>> implemented as constant arrays.
> Hi, I've tried, with a simplified version, and it works!


:-)

> Basically, I track both coordinates independently, as the rectangle
> coordinates are independent anyway. Then I associate to position[0]
> always the closest coordinates compared to the previous frame.


Yep, looks equivalent to the first part of the pseudo code.

> It works pretty fine. At least it avoid the jump between one and two
> fingers. Of course, when the two fingers become horizontally or
> vertically aligned (eg: in a rotation gesture), it's not possible to
> differentiate anymore when they dis-align. And I don't think using motion
> will help because the hardware tends to report _exactly_ the same value
> as soon the two fingers become close (= a segment instead of a
> rectangle).  Or motion tracking would require a long window of past
> frames.


The case when the fingers are first resting in horizontal/vertical position,
then moving, is, as you say, not possible to resolve. I agree, the version you
implemented is probably as far as one gets without more information from the
device. Too bad. :-)

> I still think that for the very specific use case of scrolling when
> pressing one finger and moving up and dow the other one, reporting the
> average works better than the first finger. However, I guess this can be
> considered just as a drawback of the ST protocol, and fixed in userspace
> by using the MT protocol.
> 
> What do you think? Does it look fine to you? Below is the code.


I might have lost track of what problem needs to be solved. The current patch
seems to implement tracking, but still does not solve the individual MT finger
problem. And, it uses the same definition of ABS_X/Y as before. I was also under
the impression that synaptics needs fixing, anyways. All of this taken together
sadly suggests that this patch could just as well be reverted to the original
one. Or? Alternatively, one could switch to the type B protocol, since no
further tracking improvement is possible in userspace. The implementation is
tidy and simple enough, I think.

> Eric
> 
> 
> 8<--------------------------------------------------------------------
> The elantech hardware is limited when reporting two fingers. It just
> reports the minimum and maximum coordinates of the rectangle containing
> the fingers.
> 
> As multitouch protocol requires at least to report the correct position
> of each finger, we track the finger position when there is only one
> finger to distinguish which corner of the rectangle corresponds to the
> actual position of the fingers. When there are three fingers, only the
> minimum position is reported, so we just don't track it and hope the
> last tracked position has the most likelihood to be correct.
> 
> It works fine, at least as long as the two fingers don't get
> horizontally or vertically aligned, after what it's back to random.
> This also allows to avoid so "jumps" in the single-touch protocol.
> 
> Signed-off-by: Éric Piel <eric.piel@xxxxxxxxxxxxxxxx>
> ---
>  drivers/input/mouse/elantech.c |   51 ++++++++++++++++++++++++++++++++--------
>  drivers/input/mouse/elantech.h |    2 +
>  2 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index bb66ae8..a3a3d8d 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -242,6 +242,33 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
>  	input_sync(dev);
>  }
>  
> +
> +static void track_one_finger(struct elantech_data *etd, int x, int y)
> +{
> +	etd->prev_x = x;
> +	etd->prev_y = y;
> +}
> +
> +static void find_closest_and_swap(int *ref, int *a, int *b)
> +{
> +	int diff_a = abs(*ref - *a);
> +	int diff_b = abs(*ref - *b);
> +	int tmp;
> +	if (diff_b < diff_a) {
> +		tmp = *a;
> +		*a = *b;
> +		*b = tmp;
> +	}
> +	*ref = *a;
> +}


Use swap(a, b);

> +
> +static void track_two_fingers(struct elantech_data *etd, int *x1, int *y1, int *x2, int *y2)
> +{
> +	find_closest_and_swap(&etd->prev_x, x1, x2);
> +	find_closest_and_swap(&etd->prev_y, y1, y2);
> +}
> +
> +
>  /*
>   * Interpret complete data packets and report absolute mode input events for
>   * hardware version 2. (6 byte packets)
> @@ -284,6 +311,8 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>  
>  		input_report_abs(dev, ABS_X, x1);
>  		input_report_abs(dev, ABS_Y, y1);
> +		if (fingers == 1)
> +			track_one_finger(etd, x1, y1);
>  
>  		pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
>  		width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
> @@ -296,29 +325,31 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>  		 * byte 0:  .   .  ay8 ax8  .   .   .   .
>  		 * byte 1: ax7 ax6 ax5 ax4 ax3 ax2 ax1 ax0
>  		 */
> -		x1 = ((packet[0] & 0x10) << 4) | packet[1];
> +		x1 = (((packet[0] & 0x10) << 4) | packet[1]) << 2;
>  		/* byte 2: ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0 */
> -		y1 = ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2]);
> +		y1 = (ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2])) << 2;
>  		/*
>  		 * byte 3:  .   .  by8 bx8  .   .   .   .
>  		 * byte 4: bx7 bx6 bx5 bx4 bx3 bx2 bx1 bx0
>  		 */
> -		x2 = ((packet[3] & 0x10) << 4) | packet[4];
> +		x2 = (((packet[3] & 0x10) << 4) | packet[4]) << 2;
>  		/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
> -		y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]);
> +		y2 = (ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5])) << 2;
> +
> +		track_two_fingers(etd, &x1, &y1, &x2, &y2);
>  		/* Multitouch */
> -		input_report_abs(dev, ABS_MT_POSITION_X, x1 << 2);
> -		input_report_abs(dev, ABS_MT_POSITION_Y, y1 << 2);
> +		input_report_abs(dev, ABS_MT_POSITION_X, x1);
> +		input_report_abs(dev, ABS_MT_POSITION_Y, y1);
>  		input_mt_sync(dev);
> -		input_report_abs(dev, ABS_MT_POSITION_X, x2 << 2);
> -		input_report_abs(dev, ABS_MT_POSITION_Y, y2 << 2);
> +		input_report_abs(dev, ABS_MT_POSITION_X, x2);
> +		input_report_abs(dev, ABS_MT_POSITION_Y, y2);
>  		input_mt_sync(dev);
>  		/*
>  		 * For compatibility with non-multitouch userspace apps
>  		 * report the average of both coordinates and scale up.
>  		 */
> -		input_report_abs(dev, ABS_X, (x1 + x2) << 1);
> -		input_report_abs(dev, ABS_Y, (y1 + y2) << 1);
> +		input_report_abs(dev, ABS_X, (x1 + x2) >> 1);
> +		input_report_abs(dev, ABS_Y, (y1 + y2) >> 1);
>  		/*
>  		 * For compatibility with the proprietary X Elantech driver
>  		 * report both coordinates as hat coordinates
> diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
> index f6e3be5..fa9a6b4 100644
> --- a/drivers/input/mouse/elantech.h
> +++ b/drivers/input/mouse/elantech.h
> @@ -111,6 +111,8 @@ struct elantech_data {
>  	unsigned char hw_version;
>  	unsigned int  fw_version;
>  	unsigned char parity[256];
> +	unsigned int prev_x;
> +	unsigned int prev_y;
>  };
>  
>  enum paritycheck_types {


Thanks,
Henrik


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


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux