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.
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
