I've tested this patch with linuxwacom's protocol 4 Bamboo driver with
success.
To compile linuxwacom with my 2.6.35 headers around, I had to remove
the duplicate touch work around logic.
This points out a good thing. Because we are starting at previous
tools values, its OK that kernel is filtering events and we will not
lose them anymore. The 2 touch gesture logic in today's non-MT
drivers should work better with this patch.
I'd love to hear how protocol 5 devices work with patch.
Chris
On Mon, Dec 20, 2010 at 7:59 PM, <ch...@cnpbagwell.com> wrote:
> From: Chris Bagwell <ch...@cnpbagwell.com>
>
> Kernel side input event filtering forces user land to track previous
> tools values when switching to new tools. Sending new-but-duplicate
> values for new tool would cause confusion.
>
> At one time, all wacom's sent zero values when going out of proximity
> which allowed xf86-input-wacom to take a short cut and memset() values
> when entering proximity. This is not true of all drivers any more.
>
> Change to initialize current tools values based on previous tools value;
> which is only needed when tools use a new channel.
>
> In this change, moved a strange force to in proximity to special case
> of no tool found. See deleted comments in patch for background on that.
>
> Signed-off-by: Chris Bagwell <ch...@cnpbagwell.com>
> ---
>
> I've tested this lightly on Bamboo only. Pen/Eraser/Touch all seem
> to work fine in random tests I did. It does fix the cursor jumps
> that Favux reported with MT Bamboo driver.
>
> I think this is the right way to go instead of the unreliable memset().
> It may expose some bugs but we can always fix those.
>
> Could people with protocol 5 devices give this a spin? Those are the
> ones that change channels the most. I'll give it a spin with
> protocol 4 version (linuxwacom version) of Bamboo driver first chance.
>
> src/wcmUSB.c | 90 +++++++++++++++++++--------------------------------------
> 1 files changed, 30 insertions(+), 60 deletions(-)
>
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 79c7e31..d6ac6d5 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -35,6 +35,7 @@ typedef struct {
> int wcmBTNChannel;
> Bool wcmUseMT;
> int wcmMTChannel;
> + int wcmPrevChannel;
> int wcmEventCnt;
> struct input_event wcmEvents[MAX_USB_EVENTS];
> } wcmUSBData;
> @@ -917,6 +918,12 @@ static int usbParseAbsMTEvent(WacomCommonPtr common,
> struct input_event *event)
> dsnew = &common->
> wcmChannel[private->wcmMTChannel].work;
>
> + /* Set tool specific data here. Since
> + * MT slots have fixed mapping to channel,
> + * the channel will always have previous
> + * tools values. Since MT event filtering
> + * is also per slot, this works as expected.
> + */
> dsnew->device_type = TOUCH_ID;
> dsnew->device_id = TOUCH_DEVICE_ID;
> dsnew->serial_num = event->value+1;
> @@ -980,15 +987,9 @@ static int usbParseKeyEvent(WacomCommonPtr common,
> int change = 1;
>
> /* BTN_TOOL_* are sent to indicate when a specific tool is going
> - * in our out of proximity. When going out of proximity, ds
> - * is initialized to zeros elsewere. When going in proximity,
> - * here we initialize tool specific values.
> - *
> - * This requires tools that map to same channel of an input
> - * device and that share events (ABS_X of PEN and ERASER for
> - * example) not to be in proximity at the same time. Tools
> - * that map to different channels can be in proximity at same
> - * time with no confusion.
> + * in our out of proximity. When going in proximity, here we
> + * initialize tool specific values. Making sure shared values
> + * are correct values during tool change is done elsewhere.
> */
> switch (event->code)
> {
> @@ -1175,7 +1176,6 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> WacomCommonPtr common = priv->common;
> int channel;
> int channel_change = 0, btn_channel_change = 0, mt_channel_change = 0;
> - WacomChannelPtr pChannel;
> WacomDeviceState dslast;
> wcmUSBData* private = common->private;
>
> @@ -1189,65 +1189,34 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> return;
> }
>
> - pChannel = common->wcmChannel + channel;
> - dslast = pChannel->valid.state;
> -
> - /* Because of linux input filtering, the kernel driver can not
> - * always force total set of event data when new tool comes into
> - * proximity. This includes simple case of flipping stylus
> - * from pen to eraser tool. Therefore, when new tool is in-prox
> - * we must initialize all shared event values to same as previous
> - * tool to account for filtered events.
> - *
> - * For Generic and Protocol 4 devices that have fixed channel
> - * mappings, this is no problem. Protocol 5 devices are difficult
> - * because they dynamically assign channel #'s and even simple
> - * case above can switch from channel 1 to channel 0.
> + /* Because of linux input filtering, each switch to a new
> + * tool is required to have its initial values match values
> + * of previous tool.
> *
> - * To simplify things, we take advantage of fact wacom kernel
> - * drivers force all values to zero when going out of proximity so
> - * we take a short cut and memset() to align when going in-prox
> - * instead of a memcpy().
> + * When tools share a channel then this is no issue. When
> + * switching channels, it needs to be accounted for here.
> *
> - * TODO: Some non-wacom tablets send X/Y data right before coming
> - * in proximity. The following discards that data.
> - * Adding "&& dslast.proximimty" to check would probably help
> - * this case.
> - * Some non-wacom tablets may also never reset their values
> - * to zero when out-of-prox. The memset() can loss this data.
> - * Adding a !WCM_PROTOCOL_GENERIC check would probably help this case.
> + * Note: This logic would normally be done where we process
> + * BTN_TOOL_* events but protocol 4/5 devices have always
> + * sent BTN_TOOL_* as last event within a event sync window
> + * instead of as first event when changing tool. So we must
> + * handle up front to prevent discarding other values set in
> + * same sync window.
> */
> - if (!common->wcmChannel[channel].work.proximity)
> + if (private->wcmPrevChannel != channel)
> {
> - memset(&common->wcmChannel[channel],0,sizeof(WacomChannel));
> -
> - /* in case the in-prox event was missing */
> - /* TODO: There are not valid times when in-prox
> - * events are not sent by a driver except:
> - *
> - * 1) Starting X while tool is already in prox.
> - * 2) Non-wacom tablet sends only BTN_TOUCH without
> - * BTN_TOOL_PEN since it only support 1 tool.
> - *
> - * Case 1) should be handled in same location as
> - * below check of (ds->device_type == 0) since its
> - * same reason. It is better to query for real
> - * value instead of assuming in-prox.
> - * Case 2) should be handled in case statement that
> - * processes BTN_TOUCH for WCM_PROTOCOL_GENERIC devices.
> - *
> - * So we should not be forcing to in-prox here because
> - * it could cause cursor jump from (X,Y)=(0,0) if events
> - * are sent while out-of-prox; which can happen only
> - * with WCM_PROTOCOL_GENERIC devices. Hint: see TODO above.
> - */
> - common->wcmChannel[channel].work.proximity = 1;
> + common->wcmChannel[channel].work =
> + common->wcmChannel[private->wcmPrevChannel].work;
> + private->wcmPrevChannel = channel;
> }
>
> - /* all USB data operates from previous context except relative
> values*/
> ds = &common->wcmChannel[channel].work;
> + dslast = common->wcmChannel[channel].valid.state;
> +
> + /* all USB data operates from previous context except relative
> values*/
> ds->relwheel = 0;
> ds->serial_num = private->wcmLastToolSerial;
> +
> /* For protocol 4 and 5 devices, ds == btn_ds. */
> btn_ds = &common->wcmChannel[private->wcmBTNChannel].work;
>
> @@ -1311,6 +1280,7 @@ static void usbDispatchEvents(InputInfoPtr pInfo)
> if (ISBITSET(keys, wcmTypeToKey[i].tool_key))
> {
> ds->device_type = wcmTypeToKey[i].device_type;
> + ds->proximity = 1;
> break;
> }
> }
> --
> 1.7.3.3
>
>
------------------------------------------------------------------------------
Forrester recently released a report on the Return on Investment (ROI) of
Google Apps. They found a 300% ROI, 38%-56% cost savings, and break-even
within 7 months. Over 3 million businesses have gone Google with Google Apps:
an online email calendar, and document program that's accessible from your
browser. Read the Forrester report: http://p.sf.net/sfu/googleapps-sfnew
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
