On Thu, Dec 9, 2010 at 7:29 PM, Ping Cheng <pinglinux@xxxxxxxxx> wrote:
> On Thu, Dec 9, 2010 at 1:21 PM, Chris Bagwell <chris@xxxxxxxxxxxxxx> wrote:
>> On Thu, Dec 9, 2010 at 1:50 PM, Dmitry Torokhov
>> <dmitry.torokhov@xxxxxxxxx> wrote:
>>> On Thu, Dec 09, 2010 at 11:36:19AM -0800, Ping Cheng wrote:
>>>> On Thu, Dec 9, 2010 at 7:06 AM, Chris Bagwell <chris@xxxxxxxxxxxxxx> wrote:
>>>> > On Wed, Dec 8, 2010 at 7:23 PM, Ping Cheng <pinglinux@xxxxxxxxx> wrote:
>>>> >> @@ -228,9 +277,17 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>>> >> if (tmp == W8001_TOUCH_BYTE)
>>>> >> break;
>>>> >>
>>>> >> + if (w8001->has_touch) {
>>>> >> + /* send touch data out */
>>>> >> + w8001->has_touch = 0;
>>>> >> + input_report_key(dev, BTN_TOUCH, 0);
>>>> >> + input_report_key(dev, BTN_TOOL_FINGER, 0);
>>>> >
>>>> > Probably its better to set ABS_X/ABS_Y to zero and do a sync here? So
>>>> > duplicate x/y values don't get dropped and aligns with wacom_wac.c.
>>>> > This is related to comment about forcing ABS_X/Y to zero above. Its
>>>> > so pen has known starting point when coming in proximity. I wouldn't
>>>> > do one without the other.
>>>>
>>>> I'll do both to make you happy (just kidding, to make it safe ;).
>>>>
>>>
>>> Actually do not see how (0,0) is any safer than let's say (123,78)
>>> sincve i believe (0,0) is a valid coordinate. Userspace should still
>>> hang on to the last reported coordinate and use it if it did not get
>>> a new one.
>>>
>>> Also, if you add input_sync() here won't it cause (in certain
>>> situtations) false click or tap events - BTN_TOUCH goes from 1 to 0 and
>>> then again to 1 if pen is already in proximity...
>>
>> The patches behavior happens to align good with existing wacom
>> behavior and user land apps (xf86-input-wacom mostly) but its good to
>> get this input from those not concentrating on wacom as much. It
>> helps remind me the old way is not the only way.
>>
>> Let me answer the second point first.
>>
>> The above code is following a kinda rule that other wacom tablet
>> drivers obey. That rule is that if 2 tools share ABS_* events then
>> only 1 of those tools can be in proximity at one time. If a driver
>> follows that rule then you will not get the unwanted clicks you
>> mention.
>>
>> The 1 tool in proximity probably originates from physical attribute of
>> having to flip pen to get eraser or switching to another physical
>> stylus. I think it wasn't until touch+stylus that HW supported 2
>> tools in proximity with same ABS_*. The above logic comes in to
>> simulate older HW enforced behavior. There is some user land code I
>> know of that depends on that rule right now but easy enough to fix (by
>> buffering last (x,y,touch) values).
>>
>> For first point, your right that (123,78) is just a good "known
>> starting value" if a driver is going to use that concept because it
>> could be filtered just as easy as (0,0). Its the "known" part thats
>> more useful to userland. They can do a memset() when entering
>> proximity instead of buffering previous values.
>>
>> I think this part of past driver behavior originated because, before
>> touch tools, wacom hardware was good about sending (x,y,touch) as
>> (0,0,0) when tool was out of proximity and this behavior came out to
>> userland without driver doing anything. For touch case, software
>> driver must simulate older hardware behavior. Again, I happen to know
>> some userland code that came to depend on that (0,0,0) but its easy
>> enough to fix.
>>
>> So the overall patch is in line with existing multi-tool wacom tablets
>> and their code flow.
>>
>> My original comment can be restated as if you going to send
>> BTN_TOUCH=0 then probably you should send ABS_X/Y=0 for consistency
>> with other wacom behavior.
>
> So, do you mean if we do not send BTN_TOUCH=0, we do not need to send
> ABS_X/Y=0? That is we only need to send BTN_TOOL_FINGER=0?
>
Correct. Especially if there is no sync there because BTN_TOUCH gets
updated with real value shortly after and it implies userland needs to
know previous tools BTN_TOUCH in case a new BTN_TOUCH value is not
sent.
Chris
--
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
