On Wed, Feb 2, 2011 at 5:54 PM, Ping Cheng <pingli...@gmail.com> wrote:
> On Tue, Feb 1, 2011 at 6:44 PM, Chris Bagwell <ch...@cnpbagwell.com> wrote:
>> Been out of action with a cold.  Trying slowing to get back online.
>
> Sorry to hear that. Hope you are getting "warmed up" here ;).

ha. :-)

>
>> I'll just comment on this patch in series.
>>
>> On Tue, Feb 1, 2011 at 3:36 PM, Ping Cheng <pingli...@gmail.com> wrote:
>>> We borrowed _TAP_ for 2FGT Tablet PC support when MT protocol was
>>> not available (before kernel 2.6.30 was released). MT support in
>>> the kernel is getting stable now. To move Tablet PC to MT format,
>>> we face two issues:
>>>
>>> 1. USB Tablet PC 1FGT devices will not emit BTN_TOOL_FINGER event
>>> since it is a touchscreen device.
>>>
>>> 2. Serial and USB Tablet PC 2FGT devices will not emit
>>> BTN_TOOL_DOUBLETAP event since they are not touchpads.
>>>
>>> This patch add those keys back to the supported key array so we
>>> can retrieve the proper keys when we need to decide the features
>>> that are associated with the device.
>>>
>>> Signed-off-by: Ping Cheng <pingli...@gmail.com>
>>> ---
>>>  src/wcmUSB.c |   22 ++++++++++++++++++++++
>>>  1 files changed, 22 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
>>> index 8343032..cc9ff72 100644
>>> --- a/src/wcmUSB.c
>>> +++ b/src/wcmUSB.c
>>> @@ -1340,6 +1340,25 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>>>        }
>>>  }
>>>
>>> +static void usbGenericTPCQuirks(unsigned long *keys, unsigned long *abs)
>>> +{
>>> +       /* quirks to unify the tool types for GENERIC protocol tablet PCs */
>>
>> I'd prefer if this was called usbGenericTouchscreenQuirks() to align
>> with xorg.conf.d's "MatchIsTouchscreen" terminology which is related
>> to this same quirk logic.
>
> I'll change the name to usbGenericTouchscreenQuirks().
>

Thanks.

>> Its a minor nitpick since tablet pc can be stylus only and so not
>> touchscreens but we are detecting real touchscreens below.
>>
>>> +
>>> +       /* USB Tablet PC signle finger touch devices do not emit
>>> +        * BTN_TOOL_FINGER since it is a touchscreen device.
>>> +        */
>>> +       if (ISBITSET(keys, BTN_TOUCH) &&
>>> +                       !ISBITSET(keys, BTN_TOOL_FINGER) &&
>>> +                       !ISBITSET(keys, BTN_TOOL_PEN))
>>> +               SETBIT(keys, BTN_TOOL_FINGER); /* 1FGT */
>>> +
>>> +       /* Serial and USB Tablet PC two finger touch devices do not emit
>>> +        * BTN_TOOL_DOUBLETAP since they are not touchpads.
>>> +        */
>>> +       if (ISBITSET(abs, ABS_MT_SLOT) && !ISBITSET(keys, 
>>> BTN_TOOL_DOUBLETAP))
>>> +               SETBIT(keys, BTN_TOOL_DOUBLETAP); /* 2FGT */
>>
>> I still think that Tablet PC should send BTN_TOOL_DOUBLETAP even
>> though its not going to send BTN_TOOL_FINGER (same as ntrig). But this
>> can be considered a safety net of sorts on X side since it works even
>> if you send it.
>
> I know your point. But we do not post BTN_TOOL_DOUBLETAP in
> wacom_w8001.c. I'd like to make it consistent for Wacom devices. Does
> this make sense to you?

This next comment is for kernel behaviour; not this patch.  This patch
seems fine and lets us be flexibly during kernel discussions.

I still maintain my past comment that wacom_wac.c's TPC behaviour is
easy to decide.  Its 1 input device with stylus+eraser and 1 input
device with run-of-the-mill touchscreen behaviour.  It should go out
of its way to be run-of-the-mill (including sending DOUBLETAP) and not
go out of its way to be consistent with wacom_w8001.c.  That is
limiting its application for no benefit to end user.

I *think* I mentioned that I think wacom_w8001.c's current behaviour
is wrong and we should not follow it for TPC.  Its trying to separate
events into two classes; similar to Protocol 4/5's stylus vs. mouse vs
pad classes.  It uses ABS_X/ABS_Y for stylus and ABS_MT_ for touch.  I
think this approach is fundamentally wrong.  It should be reporting
both stylus and touch (cursor emulation for touch) over ABS_X/ABS_Y
and then multi-touch events duplicated into ABS_MT*.

Until its at that point, I agree it makes no sense for it to report
BTN_TOOL_FINGER or BTN_TOOL_DOUBLETAP for wacom_w8001.  I also realize
that the way I think wacom_w8001 kernel *should* behave would be
harder for xf86-input-wacom to work with; but only slightly.

Chris

>
>> So either way:
>>
>> Acked-by: Chris Bagwell <ch...@cnpbagwell.com>
>
> Thank you.
>
> Ping
>
>>
>>> +}
>>> +
>>>  /**
>>>  * Query the device's fd for the key bits and the tablet ID. Returns the ID
>>>  * on success or 0 on failure.
>>> @@ -1379,7 +1398,10 @@ static int usbProbeKeys(InputInfoPtr pInfo)
>>>         * generic.
>>>         */
>>>        if (!ISBITSET(abs, ABS_MISC))
>>> +       {
>>>                common->wcmProtocolLevel = WCM_PROTOCOL_GENERIC;
>>> +               usbGenericTPCQuirks(common->wcmKeys, abs);
>>> +       }
>>>
>>>        return wacom_id.product;
>>>  }
>>> --
>>> 1.7.3.5
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)!
>>> Finally, a world-class log management solution at an even better price-free!
>>> Download using promo code Free_Logger_4_Dev2Dev. Offer expires
>>> February 28th, so secure your free ArcSight Logger TODAY!
>>> http://p.sf.net/sfu/arcsight-sfd2d
>>> _______________________________________________
>>> Linuxwacom-devel mailing list
>>> Linuxwacom-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>>>
>>
>

------------------------------------------------------------------------------
Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)!
Finally, a world-class log management solution at an even better price-free!
Download using promo code Free_Logger_4_Dev2Dev. Offer expires 
February 28th, so secure your free ArcSight Logger TODAY! 
http://p.sf.net/sfu/arcsight-sfd2d
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to