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