I've one comment below but other than that:
Reviewed-by: Chris Bagwell
If you make the modification suggested below, you can include my tag
in the next revision.
Chris
On Mon, Jan 2, 2012 at 11:58 AM, Alexey Osipov wrote:
> When touchpad is in Relative mode of operation, we need allow driver
> update internal old{x,y,z} variables to eliminate cursor jump after
> gesture ended. That update performed in wcmSendEvents() function in
> wcmCommon.c. So, when we in gesture mode, allow call wcmSendEvents()
> to update variables, but don't allow actual events sending to X server.
>
> Unnecessary axes modification removed from wcmSendButtonClick() in
> wcmTouchFilter.c, which was causing cursor movement in Absolute mode
> while gestures are active with respect to above changes.
>
> Signed-off-by: Alexey Osipov
> ---
>
> Changes to previous version (in reply to Chris Bagwell review):
> - check for wcmGestureMode moved from two places to one.
> - first change brakes absolute mode, which is fixed by removing
> unnecessary code from wcmSendButtonClick().
>
> src/wcmCommon.c | 11 +--
> src/wcmTouchFilter.c | 10 +-
> 2 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index 21afcc5..10a154c 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -810,8 +810,11 @@ void wcmSendEvents(InputInfoPtr pInfo, const
> WacomDeviceState* ds)
>
> if (type == PAD_ID)
> wcmSendPadEvents(pInfo, ds, 3, priv->naxes - 3,
> &valuators[3]); /* pad doesn't post x/y/z */
> - else
> - wcmSendNonPadEvents(pInfo, ds, 0, priv->naxes, valuators);
> + else {
> + /* don't move the cursor if in gesture mode */
> + if (!priv->common->wcmGestureMode)
> + wcmSendNonPadEvents(pInfo, ds, 0, priv->naxes,
> valuators);
> + }
>
> priv->oldProximity = ds->proximity;
> if (ds->proximity)
> @@ -1008,10 +1011,6 @@ void wcmEvent(WacomCommonPtr common, unsigned int
> channel,
> if ((ds.device_type == TOUCH_ID) && common->wcmTouch)
> wcmGestureFilter(priv, channel);
>
> - /* don't move the cursor if in gesture mode */
> - if (common->wcmGestureMode)
> - return;
> -
> /* For touch, only first finger moves the cursor */
> if ((ds.device_type == TOUCH_ID && common->wcmTouch && !channel) ||
> (ds.device_type != TOUCH_ID))
> diff --git a/src/wcmTouchFilter.c b/src/wcmTouchFilter.c
> index 047490b..7f9f4f2 100644
> --- a/src/wcmTouchFilter.c
> +++ b/src/wcmTouchFilter.c
> @@ -93,19 +93,11 @@ static Bool pointsInLine(WacomCommonPtr common,
> WacomDeviceState ds0,
> /* send a button event */
> static void wcmSendButtonClick(WacomDevicePtr priv, int button, int state)
> {
> - int x = 0;
> - int y = 0;
> int mode = is_absolute(priv->pInfo);
>
> - if (mode)
> - {
> - x = priv->oldX;
> - y = priv->oldY;
> - }
> -
> /* send button event in state */
> xf86PostButtonEvent(priv->pInfo->dev, mode,button,
> - state,0,priv->naxes,x,y,0,0,0,0);
> + state,0,0);
Great job tracking down exact source causing absolute mode the issue.
This is a good chance to have on its own.
I hope Ping or Peter will be able to comment on this though. I think
there is some historical bug in X server for xf86PostButtonEvent()
that doesn't support 0 axis and probably why its written as it was. I
believe the bug never existed in xf86PostButtonEventP() and seems to
be the preferred interface now anyways (wcmCommon.c was changed to use
only that over the last year or so).
To be safe, you can switch to it by adding a P to end of Event in
above line and adding an extra ",0" before ")".
>
> /* We have changed the button state (from down to up) for the device
> * so we need to update the record */
> --
> 1.7.0.4
>
>
>
--
Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual
desktops for less than the cost of PCs and save 60% on VDI infrastructure
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
