On Fri, Mar 18, 2011 at 5:04 PM, Ping Cheng <[email protected]> wrote:
> Serial ISDv4 kernel driver, wacom_w8001.ko, provides both pen and
> touch events on the same logical port. Filtering touch events when
> pen is in proximity while allowing pen events (ABS_X/Y, etc) pass.
>
> Making this action configurable would make sense when XInput 2.1 is
> ready. At that point, we can post MT valuators while pen events are
> posted. Some code refactoring is needed to add a new channel to
> store the pen data as well as the 2FGT data.
>
> Defering the code refactoring until we support XInput 2.1.
>
> Signed-off-by: Ping Cheng <[email protected]>
> Reviewed-by: Peter Hutterer <[email protected]>
> ---
> v2 changes: Merged patch 2 into 1. Incorporated Peter's comments.
> Added Reviewed-by.
>
>  src/wcmUSB.c |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 92 insertions(+), 6 deletions(-)
>
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 9e0f310..79d9ae7 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -33,6 +33,8 @@
>  typedef struct {
>        int wcmLastToolSerial;
>        int wcmBTNChannel;
> +       int wcmDeviceType;
> +       Bool wcmPenTouch;
>        Bool wcmUseMT;
>        int wcmMTChannel;
>        int wcmPrevChannel;
> @@ -571,8 +573,14 @@ int usbWcmGetRanges(InputInfoPtr pInfo)
>                common->wcmMaxDist = absinfo.maximum;
>
>        if (ISBITSET(abs, ABS_MT_SLOT))
> +       {
>                private->wcmUseMT = 1;
>
> +               /* pen and MT on the same logical port */
> +               if (ISBITSET(common->wcmKeys, BTN_TOOL_PEN))
> +                       private->wcmPenTouch = TRUE;
> +       }
> +
>        /* A generic protocol device does not report ABS_MISC event */
>        if (!ISBITSET(abs, ABS_MISC))
>                common->wcmProtocolLevel = WCM_PROTOCOL_GENERIC;
> @@ -865,12 +873,33 @@ static int usbFilterEvent(WacomCommonPtr common, struct 
> input_event *event)
>                }
>                else if (event->type == EV_ABS)
>                {
> -                       switch(event->code)
> +                       /* When pen and touch are on the same port, we need
> +                        * to leave pen data alone.
> +                        */
> +                       if ((private->wcmDeviceType != STYLUS_ID) &&
> +                                       (private->wcmDeviceType != ERASER_ID))

Can you make this an affirmative check?  The intent is filter
X/Y/Pressure if device support MT and current tool overlaps MT events
(pointer emulation).  So I think it can be simplified for now to 'if
(TOUCH_ID)".

>                        {
> -                               case ABS_X:
> -                               case ABS_Y:
> -                               case ABS_PRESSURE:
> -                                       return 1;
> +                               /* filter ST for MT */
> +                               switch(event->code)
> +                               {
> +                                       case ABS_X:
> +                                       case ABS_Y:
> +                                       case ABS_PRESSURE:
> +                                               return 1;
> +                               }
> +                       }
> +                       else
> +                       {
> +                               /* filter MT for pen */
> +                               switch(event->code)
> +                               {
> +                                       case ABS_MT_SLOT:
> +                                       case ABS_MT_TRACKING_ID:
> +                                       case ABS_MT_POSITION_X:
> +                                       case ABS_MT_POSITION_Y:
> +                                       case ABS_MT_PRESSURE:
> +                                               return 1;
> +                               }
>                        }
>                }
>        }
> @@ -1219,6 +1248,46 @@ static int usbParseBTNEvent(WacomCommonPtr common,
>        return change;
>  }
>
> +/***
> + * Retrieve the tool type from an USB data packet by looking at the event
> + * codes. Refer to linux/input.h for event codes that define tool types.
> + *
> + * @param data A pointer to struct wcmUSBData to access device specific data.
> + */
> +static void usbGetToolType(wcmUSBData *data)
> +{

This function is much more readable then v1.   Thanks!

I'll study it more and may have a follow up patch that moves the other
BTN_TOOL_* logic to this function so its always done up front.  I
believe this process-tool-event-up-front aligns with recent
xf86-input-evdev changes as well.


> +       int i;
> +       struct input_event* event;
> +
> +       /* default to 0 if no pen/touch/eraser event code is in the packet */
> +       data->wcmDeviceType = 0;
> +
> +       for (i = 0; (i < data->wcmEventCnt) && !data->wcmDeviceType; ++i)
> +       {
> +               event = data->wcmEvents + i;
> +
> +               switch (event->code)
> +               {
> +                       case BTN_TOOL_PEN:
> +                       case BTN_TOOL_PENCIL:
> +                       case BTN_TOOL_BRUSH:
> +                       case BTN_TOOL_AIRBRUSH:
> +                               data->wcmDeviceType = STYLUS_ID;
> +                               break;
> +
> +                       case BTN_TOOL_FINGER:
> +                       case ABS_MT_SLOT:
> +                       case ABS_MT_TRACKING_ID:
> +                               data->wcmDeviceType = TOUCH_ID;
> +                               break;
> +
> +                       case BTN_TOOL_RUBBER:
> +                               data->wcmDeviceType = ERASER_ID;
> +                               break;
> +               }
> +       }
> +}
> +
>  static void usbDispatchEvents(InputInfoPtr pInfo)
>  {
>        int i;
> @@ -1228,11 +1297,28 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
>        WacomCommonPtr common = priv->common;
>        int channel;
>        int channel_change = 0, btn_channel_change = 0, mt_channel_change = 0;
> -       WacomDeviceState dslast;
> +       WacomDeviceState dslast = common->wcmChannel[0].valid.state;
> +       Bool pen_in_prox = ((dslast.device_type == STYLUS_ID) ||
> +                       (dslast.device_type == ERASER_ID)) && 
> dslast.proximity;

I'm very confused by why dslast is needed.  I'm sure if I thought
about it harder, I'd understand.  Can you add comment to explain for
future reference?

Offhand, I'm thinking it should be a function called usbIsStylusToolInProx().

>        wcmUSBData* private = common->private;
>
>        DBG(6, common, "%d events received\n", private->wcmEventCnt);
>
> +       if (private->wcmPenTouch)
> +       {
> +               usbGetToolType(private);

This doesn't seem to be getting anything.  Maybe call Setup or Init or similar.

> +
> +               /* don't process touch event when pen is in-prox.
> +                * Touch events will be posted to the userland when XInput 2.1
> +                * is ready.
> +                */
> +               if ((private->wcmDeviceType == TOUCH_ID) && pen_in_prox)
> +               {
> +                       private->wcmEventCnt = 0;
> +                       return;

This is to aggressive.  There could technically be two sync windows in
1 buffer and this will discard both.  I think you can add the if()
part to lower down were it optionally calls wcmEvent().

Overall, I can tell these patches gets these ISDv4 touch+pen screens
working.  Wish I had one to play with. :-)

Chris

> +               }
> +       }
> +
>        channel = usbChooseChannel(common);
>
>        /* couldn't decide channel? invalid data */
> --
> 1.7.4
>
>
> ------------------------------------------------------------------------------
> Colocation vs. Managed Hosting
> A question and answer guide to determining the best fit
> for your organization - today and in the future.
> http://p.sf.net/sfu/internap-sfd2d
> _______________________________________________
> Linuxwacom-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to