I'm glad someone decided to add this! I wanted to but been to busy.
Similar to Peter's past comments on the file wcmTouchFilter.c, this
code is almost obsolete before being published and so I'm not
reviewing very strict.
Comments below.
On Sat, Dec 24, 2011 at 12:18 AM, Alexey Osipov <si...@lerlan.ru> wrote:
> First, we define two new GESTURE_ modes:
> - GESTURE_PREDRAG_MODE - when first tap happen and we wait for second touch.
> - GESTURE_DRAG_MODE - when actual drag happening (left button pressed).
>
> Second, we define tap timeout function wcmSingleFingerTapTimer(), which
> simulate single click if no drag operation started within timeout.
>
> Third, we make an exception for GESTURE_DRAG_MODE in wcmCommon.c, because
> we actually want cursor movements while dragging.
>
> Now, to do a single tap you just tap (touch and untouch). Actual click
> happens after TapTime period.
> To drag something you make a tap (touch and untouch) and then quickly
> (in TapTime period) touch device again. Then drag.
>
> Signed-off-by: Alexey Osipov <si...@lerlan.ru>
> ---
> src/common.mk | 3 +-
> src/wcmCommon.c | 8 +++---
> src/wcmTouchFilter.c | 54 ++++++++++++++++++++++++++++++++++++++++---------
> src/wcmTouchFilter.h | 38 +++++++++++++++++++++++++++++++++++
> src/xf86Wacom.h | 1 -
> 5 files changed, 88 insertions(+), 16 deletions(-)
> create mode 100644 src/wcmTouchFilter.h
>
> diff --git a/src/common.mk b/src/common.mk
> index fae8d9f..7f1eadd 100644
> --- a/src/common.mk
> +++ b/src/common.mk
> @@ -12,4 +12,5 @@ DRIVER_SOURCES= \
> $(top_srcdir)/src/wcmUSB.c \
> $(top_srcdir)/src/wcmXCommand.c \
> $(top_srcdir)/src/wcmValidateDevice.c \
> - $(top_srcdir)/src/wcmTouchFilter.c
> + $(top_srcdir)/src/wcmTouchFilter.c \
> + $(top_srcdir)/src/wcmTouchFilter.h
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index d0bff63..0cab803 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -24,6 +24,7 @@
> #include "xf86Wacom.h"
> #include "Xwacom.h"
> #include "wcmFilter.h"
> +#include "wcmTouchFilter.h"
> #include <xkbsrv.h>
> #include <xf86_OSproc.h>
>
> @@ -743,8 +744,8 @@ void wcmSendEvents(InputInfoPtr pInfo, const
> WacomDeviceState* ds)
> valuators[4] = v4;
> valuators[5] = v5;
>
> - /* don't move the cursor if in gesture mode */
> - if (!(!is_absolute(pInfo) && priv->common->wcmGestureMode)) {
> + /* don't move the cursor if in gesture mode (except drag mode) */
> + if (!(!is_absolute(pInfo) && priv->common->wcmGestureMode &
> ~GESTURE_DRAG_MODE)) {
> if (type == PAD_ID)
> wcmSendPadEvents(pInfo, ds, 3, 3, &valuators[3]); /*
> pad doesn't post x/y/z */
> else
> @@ -945,8 +946,7 @@ 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 ((priv->flags & ABSOLUTE_FLAG) && (common->wcmGestureMode))
> + if ((priv->flags & ABSOLUTE_FLAG) && (common->wcmGestureMode &
> ~GESTURE_DRAG_MODE))
> return;
These would need to align with comment on previous patch.
And more a personal opinion, I think the wcmGestureMode values are
meant to be more internal to the touch filter engine and shouldn't be
exposed to wcmCommon.c. I'd prefer if you exported a new function
with name like wcmTouchNeedsFilter() that returns yes/no answer and
then you add exact values to check inside wcmTouchFilter.c.
>
> /* For touch, only first finger moves the cursor */
> diff --git a/src/wcmTouchFilter.c b/src/wcmTouchFilter.c
> index 047490b..c1a6071 100644
> --- a/src/wcmTouchFilter.c
> +++ b/src/wcmTouchFilter.c
> @@ -1,5 +1,6 @@
> /*
> * Copyright 2009 - 2010 by Ping Cheng, Wacom. <pi...@wacom.com>
> + * Copyright 2011 by Alexey Osipov. <si...@lerlan.ru>
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> @@ -21,6 +22,7 @@
> #endif
>
> #include "xf86Wacom.h"
> +#include "wcmTouchFilter.h"
> #include <math.h>
>
> /* Defines for 2FC Gesture */
> @@ -28,12 +30,6 @@
> #define WACOM_VERT_ALLOWED 2
> #define WACOM_GESTURE_LAG_TIME 10
>
> -#define GESTURE_NONE_MODE 0
> -#define GESTURE_TAP_MODE 1
> -#define GESTURE_SCROLL_MODE 2
> -#define GESTURE_ZOOM_MODE 4
> -#define GESTURE_LAG_MODE 8
> -
> #define WCM_SCROLL_UP 5 /* vertical up */
> #define WCM_SCROLL_DOWN 4 /* vertical down */
> #define WCM_SCROLL_LEFT 6 /* horizontal left */
> @@ -147,6 +143,23 @@ static void wcmFingerTapToClick(WacomDevicePtr priv)
> }
> }
>
> +static CARD32 wcmSingleFingerTapTimer(OsTimerPtr timer, CARD32 time, pointer
> arg)
> +{
> + WacomDevicePtr priv = (WacomDevicePtr)arg;
> + WacomCommonPtr common = priv->common;
> +
> + if (common->wcmGestureMode == GESTURE_PREDRAG_MODE)
> + {
> + /* left button down */
> + wcmSendButtonClick(priv, 1, 1);
> +
> + /* left button up */
> + wcmSendButtonClick(priv, 1, 0);
> + common->wcmGestureMode = GESTURE_NONE_MODE;
> + }
> +
> + return 0;
> +}
>
> /* A single finger tap is defined as 1 finger tap that lasts less than
> * wcmTapTime. It results in a left button press.
> @@ -185,11 +198,10 @@ static void wcmSingleFingerTap(WacomDevicePtr priv)
> common->wcmGestureParameters.wcmTapTime &&
> ds[1].sample < dsLast[0].sample)
> {
> - /* left button down */
> - wcmSendButtonClick(priv, 1, 1);
> + common->wcmGestureMode = GESTURE_PREDRAG_MODE;
>
> - /* left button up */
> - wcmSendButtonClick(priv, 1, 0);
> + /* Delay to detect possible drag operation */
> + TimerSet(NULL, 0,
> common->wcmGestureParameters.wcmTapTime, wcmSingleFingerTapTimer, priv);
> }
> }
> }
> @@ -244,6 +256,17 @@ void wcmGestureFilter(WacomDevicePtr priv, int channel)
> if (!common->wcmGesture)
> goto ret;
>
> + if (common->wcmGestureMode == GESTURE_DRAG_MODE)
> + {
> + if (!ds[0].proximity)
> + {
> + /* left button up */
> + wcmSendButtonClick(priv, 1, 0);
> + common->wcmGestureMode = GESTURE_NONE_MODE;
> + }
> + goto ret;
> + }
> +
This feels out of place at the top of function. I'm pretty sure it
would live better inside the "if (!ds[0].proximity &&
!ds[1].proximity)" were we are already doing cleanup gesture logic
related to two fingers not touching.
> /* When 2 fingers are in proximity, it must always be in one of
> * the valid 2 fingers modes: LAG, SCROLL, or ZOOM.
> * LAG mode is used while deciding between SCROLL and ZOOM and
> @@ -306,6 +329,17 @@ void wcmGestureFilter(WacomDevicePtr priv, int channel)
> goto ret;
> }
>
> + if (common->wcmGestureMode == GESTURE_PREDRAG_MODE)
> + {
> + if (ds[0].proximity && !ds[1].proximity)
> + {
> + /* left button down */
> + wcmSendButtonClick(priv, 1, 1);
> + common->wcmGestureMode = GESTURE_DRAG_MODE;
> + goto ret;
> + }
> + }
> +
There is already a "if (ds[0].proximity && !ds[1].proximity)" right
above this. This logic should be moved inside that if().
> if (!ds[0].proximity && !ds[1].proximity)
> {
> /* first finger was out-prox when GestureMode was still on */
> diff --git a/src/wcmTouchFilter.h b/src/wcmTouchFilter.h
> new file mode 100644
> index 0000000..453ef41
> --- /dev/null
> +++ b/src/wcmTouchFilter.h
> @@ -0,0 +1,38 @@
> +/*
> + * Copyright 2009 - 2010 by Ping Cheng, Wacom. <pi...@wacom.com>
> + * Copyright 2011 by Alexey Osipov. <si...@lerlan.ru>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + */
> +
> +#ifndef __XF86_WCMTOUCHFILTER_H
> +#define __XF86_WCMTOUCHFILTER_H
> +
> +#include "xf86Wacom.h"
> +
> +/****************************************************************************/
> +
> +#define GESTURE_NONE_MODE 0
> +#define GESTURE_TAP_MODE 1
> +#define GESTURE_SCROLL_MODE 2
> +#define GESTURE_ZOOM_MODE 4
> +#define GESTURE_LAG_MODE 8
> +#define GESTURE_PREDRAG_MODE 16
> +#define GESTURE_DRAG_MODE 32
> +
> +void wcmGestureFilter(WacomDevicePtr priv, int channel);
Generally, this type stuff should be broken into a cleanup patch
(moving existing external defines to a better header file) and a new
feature patch. That way in case gesture patch needs to reverted for
some reason, we do not lose the header cleanup with it.
Overall, pretty trivial comments. Thanks for patch!
Chris
> +
> +/****************************************************************************/
> +#endif /* __XF86_WCMTOUCHFILTER_H */
> diff --git a/src/xf86Wacom.h b/src/xf86Wacom.h
> index e58e0b0..fc1b4f4 100644
> --- a/src/xf86Wacom.h
> +++ b/src/xf86Wacom.h
> @@ -152,7 +152,6 @@ extern int wcmDevSwitchMode(ClientPtr client,
> DeviceIntPtr dev, int mode);
> /* run-time modifications */
> extern void wcmChangeScreen(InputInfoPtr pInfo, int value);
> extern int wcmTilt2R(int x, int y, double offset);
> -extern void wcmGestureFilter(WacomDevicePtr priv, int channel);
> extern void wcmEmitKeycode(DeviceIntPtr keydev, int keycode, int state);
> extern void wcmSoftOutEvent(InputInfoPtr pInfo);
>
> --
> 1.7.0.4
>
>
>
>
> ------------------------------------------------------------------------------
> Write once. Port to many.
> Get the SDK and tools to simplify cross-platform app development. Create
> new or port existing apps to sell to consumers worldwide. Explore the
> Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
> http://p.sf.net/sfu/intel-appdev
> _______________________________________________
> Linuxwacom-devel mailing list
> Linuxwacom-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
------------------------------------------------------------------------------
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
