Joonyoung Shim wrote:
> On 6/25/2010 11:08 PM, Henrik Rydberg wrote:
>> Hi Joonyoung,
>>
>> some follow-up comments on the MT events.
>>
>>> +static void qt602240_input_report(struct qt602240_data *data, int single_id)
>>> +{
>>> + struct qt602240_finger *finger = data->finger;
>>> + struct input_dev *input_dev = data->input_dev;
>>> + int finger_num = 0;
>>> + int id;
>>> +
>>> + for (id = 0; id < QT602240_MAX_FINGER; id++) {
>>> + if (!finger[id].status)
>>> + continue;
>>> +
>>> + input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>>> + finger[id].area);
>>> +
>>> + if (finger[id].status == QT602240_RELEASE)
>>> + finger[id].status = 0;
>>> + else {
>>> + input_report_abs(input_dev, ABS_MT_POSITION_X,
>>> + finger[id].x);
>>> + input_report_abs(input_dev, ABS_MT_POSITION_Y,
>>> + finger[id].y);
>>> + finger_num++;
>>> + }
>>> +
>>> + input_mt_sync(input_dev);
>>> + }
>>> +
>>> + input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
>>> +
>>> + if (finger[single_id].status != QT602240_RELEASE) {
>>> + input_report_abs(input_dev, ABS_X, finger[single_id].x);
>>> + input_report_abs(input_dev, ABS_Y, finger[single_id].y);
>>> + }
>>> +
>>> + input_sync(input_dev);
>>> +}
>> The problem still persists here. I will try to explain in code form instead:
>>
>> for (id = 0; id < QT602240_MAX_FINGER; id++) {
>> if (!finger[id].status)
>> continue;
>>
>> input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
>> finger[id].status != QT602240_RELEASE ? finger[id].area : 0);
>> input_report_abs(input_dev, ABS_MT_POSITION_X,
>> finger[id].x);
>> input_report_abs(input_dev, ABS_MT_POSITION_Y,
>> finger[id].y);
>
> Hmm, is it OK to report ABS_MT_POSITION_X/Y when releases?
Yes. The position should be the position where the finger left the surface. It
is different from the ABS_X/Y case simply because the type A protocol exchanges
data statelessly.
>> input_mt_sync(input_dev);
>>
>> if (finger[id].status == QT602240_RELEASE)
>> finger[id].status = 0;
>> else
>> finger_num++;
>> }
>>
>> Regarding the single_id, I can understand the need for it, but the logic for a
>> single touch is slightly confusing. If single_id is to be interpreted as the
>> contact currently being tracked as the single pointer, and that single_id
>> changes as the number of fingers on the pad is reduced, until there is only one
>> left, then it would still be clearer logically to do something like this:
>>
>> if (finger_num > 0) {
>> input_report_key(input_dev, BTN_TOUCH, 1);
>> input_report_abs(input_dev, ABS_X, finger[single_id].x);
>> input_report_abs(input_dev, ABS_Y, finger[single_id].y);
>> } else {
>> input_report_key(input_dev, BTN_TOUCH, 0);
>> }
>> input_sync(input_dev);
>>
>> Would it not?
>>
>
> There is a case which fingers more than one are touched and current
> status of single_id finger is release, then this will report ABS_X/Y
> events even if it is the release event.
I see. And you want BTN_TOUCH to follow the logic for the single touch? I think
that is the main issue here. We can have _one_ of the following definitions, but
not both:
1. input_report_key(input_dev, BTN_TOUCH, finger_num > 0);
2. input_report_key(input_dev, BTN_TOUCH,
finger[single_id].status != QT602240_RELEASE);
If you use the latter, there should be another event to denote the finger_num ==
0 case. This line at the end should do it:
if (finger_num == 0)
input_mt_sync(input_dev);
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
