Re: [Linuxwacom-devel] [PATCH 3/4] 'Left mouse button' dragging support.

2011-12-31 Thread Alexey Osipov 
В Птн, 30/12/2011 в 13:54 -0600, Chris Bagwell пишет:
 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.

Could you please point me to that Perer's comment? I'm not quite
understand what you are meaning.

 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.

Agreed.

 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.

Yes, this can be done that way. But isn't calling extra function will
slow down overall events processing? I don't think that extra CPU usage
in driver is a good idea. What you think?

 
 /* 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_ALLOWED2
   #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

Re: [Linuxwacom-devel] [PATCH 3/4] 'Left mouse button' dragging support.

2011-12-31 Thread Chris Bagwell 
On Sat, Dec 31, 2011 at 2:16 AM, Alexey Osipov si...@lerlan.ru wrote:
 В Птн, 30/12/2011 в 13:54 -0600, Chris Bagwell пишет:
 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.

 Could you please point me to that Perer's comment? I'm not quite
 understand what you are meaning.

I'm referring to comment Peter made when he first submitting the patch
for wcmTouchFilter.c to xf86-input-wacom.  Its not real important but
I just wanted to qualify changes to this file aren't as strict in my
opinion.


 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.

 Agreed.

 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.

 Yes, this can be done that way. But isn't calling extra function will
 slow down overall events processing? I don't think that extra CPU usage
 in driver is a good idea. What you think?

No, function calls are good in driver to improve readability. We do
this quite often. In this case, there is 99% chance that compiler will
inline the function anyways.


 
         /* 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
   

[Linuxwacom-devel] [PATCH 3/4] 'Left mouse button' dragging support.

2011-12-23 Thread Alexey Osipov 
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;
 
/* 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_ALLOWED2
 #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;
 
-