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?
> 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.
How about codes like this? :
+ int status = finger[single_id].status;
[snip]
+ if (finger_num > 0) {
+ if (status != QT602240_RELEASE) {
+ 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);
--
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
