[PATCH 14/18] Add gesture-mode notification via input properties

Peter Hutterer peter.hutterer at who-t.net
Wed Oct 13 21:16:58 PDT 2010


On Wed, Oct 13, 2010 at 08:25:12AM +0200, Takashi Iwai wrote:
> At Wed, 13 Oct 2010 14:06:48 +1000,
> Peter Hutterer wrote:
> > 
> > explanation please. I prefer looking at a patch knowing what to expect, this
> > title explains nothing, so I don't know if any of the code does what you
> > intended.
> 
> It's for telling outside which gesture mode is activated currently.
> I made a small program showing an icon beside the cursor pointer when
> the multi-touch gesture is activated, so that user can "see" whether it's
> in zoom or scroll or other modes.  This is pretty useful especially
> when different gestures are available and switched with each other.
> 
> > one thing though: properties are prone to race conditions. the event only
> > contains the name of the property, not the values. so whatever this patch
> > does, if you're trying to tell the client what gesture is currently active
> > or so that won't work. by the time the client can query the property, it may 
> > have updated several times already.
> 
> As long as X is a single thread, it works, so far :)
> When the state is changed in a signal handler, the notification is
> delayed so that it won't crash with the current reading.

no, that's not actually a single-thread/multi-thread issue, it's the
communication between client and server (and that's usually multi-process
:). the problem is that given enough client lag, you may get an sequence
like this:

property notify
property notify
        client receives first property event
        client sends request for property values
property notify
property notify
request arrives, reply sent with current property data
property notify
property notify
        client receives reply with outdated data

so you've updated the property 6 times in the time it took the client to get
the value of the property _once_, and at that point the value of the
property is essentially undefined (it will contain the data that's valid at
the time the request is processed).

properties are not a good vehicle for frequently updating data, their
primary function is configuration management.
 
> Anyway, this patch is just an experimental fun stuff.  I don't expect
> it will be merged as is, too.  But I wanted to raise some need for
> such a thing.

yeah, I know something like this is needed, but since I'm pushing rather
hard to get gestures interpreted client-side, that's easy enough to do there
:)

Cheers,
  Peter

> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > Cheers,
> >   Peter
> > 
> > 
> > On Fri, Oct 08, 2010 at 07:22:38PM +0200, Takashi Iwai wrote:
> > > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > > ---
> > >  include/synaptics-properties.h |    6 +++
> > >  src/properties.c               |   33 +++++++++++++++++++
> > >  src/synaptics.c                |   68 +++++++++++++++++++++++++++++++++-------
> > >  src/synapticsstr.h             |   11 ++++++
> > >  tools/synclient.c              |    1 +
> > >  5 files changed, 107 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > > index 7112bc0..5d440be 100644
> > > --- a/include/synaptics-properties.h
> > > +++ b/include/synaptics-properties.h
> > > @@ -173,4 +173,10 @@
> > >  /* 32 bit, 2 values, start, delta (0 disable) */
> > >  #define SYNAPTICS_PROP_MULTI_TOUCH_PINCH "Synaptics Multi-Touch Pinch"
> > >  
> > > +/* 8 bit (BOOL) */
> > > +#define SYNAPTICS_PROP_GESTURE_MODE_NOTIFY "Gesture Mode Notify"
> > > +
> > > +/* 32bit, read-only */
> > > +#define SYNAPTICS_PROP_GESTURE_MODE "Current Gesture Mode"
> > > +
> > >  #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > > diff --git a/src/properties.c b/src/properties.c
> > > index 0f679dd..7788a13 100644
> > > --- a/src/properties.c
> > > +++ b/src/properties.c
> > > @@ -88,6 +88,8 @@ Atom prop_led                   = 0;
> > >  Atom prop_led_status            = 0;
> > >  Atom prop_led_double_tap        = 0;
> > >  Atom prop_multi_touch_pinch     = 0;
> > > +Atom prop_gesture_mode_notify   = 0;
> > > +Atom prop_gesture_mode          = 0;
> > >  
> > >  static Atom
> > >  InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
> > > @@ -296,6 +298,11 @@ InitDeviceProperties(InputInfoPtr pInfo)
> > >      values[0] =  para->multi_touch_pinch_start;
> > >      values[1] =  para->multi_touch_pinch_dist;
> > >      prop_multi_touch_pinch = InitAtom(local->dev, SYNAPTICS_PROP_MULTI_TOUCH_PINCH, 32, 2, values);
> > > +
> > > +    prop_gesture_mode_notify = InitAtom(local->dev, SYNAPTICS_PROP_GESTURE_MODE_NOTIFY, 8, 1, &para->gesture_mode_notify);
> > > +
> > > +    values[0] = 0;
> > > +    prop_gesture_mode = InitAtom(local->dev, SYNAPTICS_PROP_GESTURE_MODE, 32, 1, values);
> > >  }
> > >  
> > >  int
> > > @@ -542,7 +549,13 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
> > >          dist = (INT32*)prop->data;
> > >  	para->multi_touch_pinch_start = dist[0];
> > >  	para->multi_touch_pinch_dist = dist[1];
> > > +    } else if (property == prop_gesture_mode_notify)
> > > +    {
> > > +        INT32 *dist;
> > > +        if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
> > > +            return BadMatch;
> > >  
> > > +        para->gesture_mode_notify = *(CARD8*)prop->data;
> > >      } else if (property == prop_gestures)
> > >      {
> > >          BOOL *gestures;
> > > @@ -724,3 +737,23 @@ void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off)
> > >          XIChangeDeviceProperty(dev, prop_off, XA_INTEGER, 8,
> > >                                 PropModeReplace, 1, &val, FALSE);
> > >  }
> > > +
> > > +void SynapticsSetGestureModeProperty(DeviceIntPtr dev, int mode)
> > > +{
> > > +    LocalDevicePtr local = (LocalDevicePtr) dev->public.devicePrivate;
> > > +    SynapticsPrivate *priv = (SynapticsPrivate *) local->private;
> > > +    SynapticsParameters *para = &priv->synpara;
> > > +    static int gesture_mode_vals[] = {
> > > +	[GESTURE_NONE] = 0,
> > > +	[GESTURE_PINCH] = 10,
> > > +	[GESTURE_HSCROLL] = 11,
> > > +	[GESTURE_VSCROLL] = 12,
> > > +	[GESTURE_CIRCULAR] = 13,
> > > +    };
> > > +
> > > +    if (!prop_gesture_mode || !para->gesture_mode_notify)
> > > +	    return;
> > > +    mode = gesture_mode_vals[mode];
> > > +    XIChangeDeviceProperty(dev, prop_gesture_mode, XA_INTEGER, 32,
> > > +			   PropModeReplace, 1, &mode, TRUE);
> > > +}
> > > diff --git a/src/synaptics.c b/src/synaptics.c
> > > index 5ec853a..2f264f0 100644
> > > --- a/src/synaptics.c
> > > +++ b/src/synaptics.c
> > > @@ -143,6 +143,7 @@ void InitDeviceProperties(InputInfoPtr pInfo);
> > >  int SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
> > >                  BOOL checkonly);
> > >  void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off);
> > > +void SynapticsSetGestureModeProperty(DeviceIntPtr dev, int val);
> > >  
> > >  InputDriverRec SYNAPTICS = {
> > >      1,
> > > @@ -608,6 +609,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
> > >      pars->led_double_tap = xf86SetBoolOption(opts, "LEDDoubleTap", TRUE);
> > >      pars->multi_touch_pinch_start = xf86SetIntOption(opts, "MultiTouchPinchStart", pinchStart);
> > >      pars->multi_touch_pinch_dist = xf86SetIntOption(opts, "MultiTouchPinchDelta", pinchDelta);
> > > +    pars->gesture_mode_notify = xf86SetBoolOption(opts, "GestureModeNotify", TRUE);
> > >  
> > >      /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge parameters */
> > >      if (pars->top_edge > pars->bottom_edge) {
> > > @@ -1191,7 +1193,7 @@ handle_toggle_led(LocalDevicePtr local, struct SynapticsHwState *hw, int finger)
> > >              para->touchpad_off = !para->touchpad_off;
> > >              if (priv->proto_ops && priv->proto_ops->UpdateLED)
> > >                  priv->proto_ops->UpdateLED(local);
> > > -	    priv->prop_change_pending = 1;
> > > +	    priv->prop_change_pending |= 1;
> > >              priv->led_tapped = FALSE;
> > >          }
> > >      } else
> > > @@ -1322,6 +1324,7 @@ handle_clickpad(LocalDevicePtr local, struct SynapticsHwState *hw)
> > >  	    if (in_multi_button)
> > >  		get_clickpad_button(priv, hw, hw->multi_touch_x);
> > >  	    priv->multi_touch_mode = MULTI_TOUCH_MODE_DRAG;
> > > +	    priv->gesture_mode = GESTURE_NONE;
> > >  	}
> > >      } else {
> > >  	/* button being released, reset dragging if necessary */
> > > @@ -1489,10 +1492,11 @@ timerFunc(OsTimerPtr timer, CARD32 now, pointer arg)
> > >      delay = HandleState(pInfo, &hw);
> > >  
> > >  #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) >= 3
> > > -    if (priv->prop_change_pending & 1) {
> > > +    if (priv->prop_change_pending & 1)
> > >  	SynapticsToggleOffProperty(local->dev, para->touchpad_off);
> > > -	priv->prop_change_pending = 0;
> > > -    }
> > > +    if (priv->prop_change_pending & 2)
> > > +	SynapticsSetGestureModeProperty(local->dev, priv->gesture_mode);
> > > +    priv->prop_change_pending = 0;
> > >  #endif
> > >  
> > >      /*
> > > @@ -2173,6 +2177,44 @@ multi_touch_distance(struct SynapticsHwState *hw)
> > >      return sqrt(SQR(hw->x - hw->multi_touch_x) + SQR(hw->y - hw->multi_touch_y));
> > >  }
> > >  
> > > +static void
> > > +update_scroll_gesture_mode(SynapticsPrivate *priv, struct ScrollData *sd)
> > > +{
> > > +    switch (priv->gesture_mode) {
> > > +    case GESTURE_CIRCULAR:
> > > +	if (!priv->circ_scroll_on)
> > > +	    priv->gesture_mode = GESTURE_NONE;
> > > +	break;
> > > +    case GESTURE_HSCROLL:
> > > +	if (!(priv->horiz_scroll_edge_on || priv->horiz_scroll_twofinger_on))
> > > +	    priv->gesture_mode = GESTURE_NONE;
> > > +	break;
> > > +    case GESTURE_VSCROLL:
> > > +	if (!(priv->vert_scroll_edge_on || priv->vert_scroll_twofinger_on))
> > > +	    priv->gesture_mode = GESTURE_NONE;
> > > +	break;
> > > +    default:
> > > +	break;
> > > +    }
> > > +    if (priv->circ_scroll_on) {
> > > +	if (sd->left || sd->right || sd->up || sd->down)
> > > +	    priv->gesture_mode = GESTURE_CIRCULAR;
> > > +    } else {
> > > +	if ((priv->horiz_scroll_edge_on || priv->horiz_scroll_twofinger_on) &&
> > > +	    (sd->left || sd->right)) {
> > > +	    if (priv->multi_touch_mode == MULTI_TOUCH_MODE_START)
> > > +		priv->multi_touch_mode = MULTI_TOUCH_MODE_SCROLL;
> > > +	    priv->gesture_mode = GESTURE_HSCROLL;
> > > +	}
> > > +	if ((priv->vert_scroll_edge_on || priv->vert_scroll_twofinger_on) &&
> > > +	    (sd->down || sd->up)) {
> > > +	    if (priv->multi_touch_mode == MULTI_TOUCH_MODE_START)
> > > +		priv->multi_touch_mode = MULTI_TOUCH_MODE_SCROLL;
> > > +	    priv->gesture_mode = GESTURE_VSCROLL;
> > > +	}
> > > +    }
> > > +}
> > > +
> > >  static int
> > >  HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> > >  		edge_type edge, Bool finger, struct ScrollData *sd)
> > > @@ -2189,6 +2231,7 @@ HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> > >  	priv->horiz_scroll_edge_on = FALSE;
> > >  	priv->vert_scroll_twofinger_on = FALSE;
> > >  	priv->horiz_scroll_twofinger_on = FALSE;
> > > +	priv->gesture_mode = GESTURE_NONE;
> > >  	return delay;
> > >      }
> > >  
> > > @@ -2354,14 +2397,10 @@ HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> > >  	    while (hw->y - priv->scroll_y > delta) {
> > >  		sd->down++;
> > >  		priv->scroll_y += delta;
> > > -		if (priv->multi_touch_mode == MULTI_TOUCH_MODE_START)
> > > -		    priv->multi_touch_mode = MULTI_TOUCH_MODE_SCROLL;
> > >  	    }
> > >  	    while (hw->y - priv->scroll_y < -delta) {
> > >  		sd->up++;
> > >  		priv->scroll_y -= delta;
> > > -		if (priv->multi_touch_mode == MULTI_TOUCH_MODE_START)
> > > -		    priv->multi_touch_mode = MULTI_TOUCH_MODE_SCROLL;
> > >  	    }
> > >  	}
> > >      }
> > > @@ -2372,14 +2411,10 @@ HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> > >  	    while (hw->x - priv->scroll_x > delta) {
> > >  		sd->right++;
> > >  		priv->scroll_x += delta;
> > > -		if (priv->multi_touch_mode == MULTI_TOUCH_MODE_START)
> > > -		    priv->multi_touch_mode = MULTI_TOUCH_MODE_SCROLL;
> > >  	    }
> > >  	    while (hw->x - priv->scroll_x < -delta) {
> > >  		sd->left++;
> > >  		priv->scroll_x -= delta;
> > > -		if (priv->multi_touch_mode == MULTI_TOUCH_MODE_START)
> > > -		    priv->multi_touch_mode = MULTI_TOUCH_MODE_SCROLL;
> > >  	    }
> > >  	}
> > >      }
> > > @@ -2450,6 +2485,8 @@ HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> > >  	}
> > >      }
> > >  
> > > +    update_scroll_gesture_mode(priv, sd);
> > > +
> > >      return delay;
> > >  }
> > >  
> > > @@ -2481,6 +2518,7 @@ handle_multi_touch_pinch(SynapticsPrivate *priv, struct SynapticsHwState *hw,
> > >      if (abs_diff > para->multi_touch_pinch_start) {
> > >  	if (priv->multi_touch_mode != MULTI_TOUCH_MODE_PINCH) {
> > >  	    priv->multi_touch_mode = MULTI_TOUCH_MODE_PINCH;
> > > +	    priv->gesture_mode = GESTURE_PINCH;
> > >  	    priv->vert_scroll_twofinger_on = FALSE;
> > >  	    priv->horiz_scroll_twofinger_on = FALSE;
> > >  	}
> > > @@ -2801,6 +2839,9 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> > >      int timeleft;
> > >      Bool inside_active_area;
> > >      int zoom_in = 0, zoom_out = 0;
> > > +    int prev_gesture_mode;
> > > +
> > > +    prev_gesture_mode = priv->gesture_mode;
> > >  
> > >      update_multi_touch(priv, hw);
> > >  
> > > @@ -2957,6 +2998,9 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> > >      if (inside_active_area)
> > >  	store_history(priv, hw->x, hw->y, hw->millis);
> > >  
> > > +    if (prev_gesture_mode != priv->gesture_mode)
> > > +	priv->prop_change_pending |= 2;
> > > +
> > >      return delay;
> > >  }
> > >  
> > > diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> > > index 31756cb..285c8f3 100644
> > > --- a/src/synapticsstr.h
> > > +++ b/src/synapticsstr.h
> > > @@ -167,6 +167,7 @@ typedef struct _SynapticsParameters
> > >      Bool led_double_tap;		    /* double-tap period in ms for touchpad LED control */
> > >      int multi_touch_pinch_start;            /* multi-touch pinch sthard threshold distance */
> > >      int multi_touch_pinch_dist;             /* multi-touch pinch delta threshold distance */
> > > +    Bool gesture_mode_notify;               /* Notify gesture mode via property */
> > >  } SynapticsParameters;
> > >  
> > >  
> > > @@ -180,6 +181,14 @@ enum MultiTouchMode {
> > >      MULTI_TOUCH_MODE_SCROLL,
> > >  };
> > >  
> > > +enum GestureMode {
> > > +    GESTURE_NONE,
> > > +    GESTURE_PINCH,
> > > +    GESTURE_HSCROLL,
> > > +    GESTURE_VSCROLL,
> > > +    GESTURE_CIRCULAR
> > > +};
> > > +
> > >  #define MAX_ACTIONS	4
> > >  enum { ACTION_BUTTON = 1, ACTION_KEY, ACTION_KEYMOD };
> > >  
> > > @@ -272,6 +281,8 @@ typedef struct _SynapticsPrivateRec
> > >      int multi_touch_distance;
> > >      int prev_multi_touch_distance;
> > >  
> > > +    enum GestureMode gesture_mode;
> > > +
> > >      int zoom_in_num_actions;
> > >      int zoom_in_action[MAX_ACTIONS];
> > >      int zoom_out_num_actions;
> > > diff --git a/tools/synclient.c b/tools/synclient.c
> > > index ab737dc..68faa13 100644
> > > --- a/tools/synclient.c
> > > +++ b/tools/synclient.c
> > > @@ -149,6 +149,7 @@ static struct Parameter params[] = {
> > >      {"LEDDoubleTap",          PT_BOOL,   0, 1,     SYNAPTICS_PROP_LED_DOUBLE_TAP,	8,	0},
> > >      {"MultiTouchPinchStart",  PT_INT,    0, 1000,  SYNAPTICS_PROP_MULTI_TOUCH_PINCH,	32,	0},
> > >      {"MultiTouchPinchDelta",  PT_INT,    0, 4000,  SYNAPTICS_PROP_MULTI_TOUCH_PINCH,	32,	1},
> > > +    {"GestureModeNotify",     PT_BOOL,   0, 1,     SYNAPTICS_PROP_GESTURE_MODE_NOTIFY,	8,	0},
> > >      { NULL, 0, 0, 0, 0 }
> > >  };
> > >  
> > > -- 
> > > 1.7.3.1
> > > 
> > Cheers,
> >   Peter
> > 


More information about the xorg-devel mailing list