Synaptics patch review request - add option for invert circular scrolling direction

Peter Hutterer peter.hutterer at who-t.net
Mon Feb 6 08:48:00 PST 2012


On Sun, Feb 05, 2012 at 09:35:50PM +0100, Lukáš Karas wrote:
> Synaptics driver generates negative (up) scrolling event with clockwise 
> circular scrolling gesture. This make sense, when you start this gesture 
> on left corner of touchpad. 
> 
> But when you start on right corner and move your finger from top to bottom, 
> continue to left clockwise, then page scrolls top. This should be intuitive inverted 
> in this case.
> 
> My patch add simple option (bool) that invert circular scrolling direction. 
> User can setup his gesture preferences with that option.
> 
> Please, review this patch, I am not sure in start_coasting function.

thanks, I appreciate your effort here and I realise that there's some need
for a new function. There are two parts to this issue though:
- for servers pre 1.11 where we don't have smooth scrolling support, you can
  just swap the button mapping for buttons 4 and 5 to get inverted scrolling
  behaviour
- for servers 1.12, we should be changing the increment for the scroll axis,
  we don't have an interface for that yet though so this is where it gets
  trickier.

As for a driver option - no. I don't want this option at all. For two
reasons: the synaptics driver is already in a bad state and I'm not going to
add any more options that don't directly relate to hardware support. And
the second, more important, reason is that this isn't an isolated function
that only applies to the synaptics driver, it would have to be a server-wide
option to change the increment for smooth scrolling axes.
that will most likely require a protocol bump and some new requests, but
it's unclear at this point how they should look like.

Cheers,
  Peter

> ---
> 
> 
>  include/synaptics-properties.h |    3 +++
>  man/synaptics.man              |    7 +++++++
>  src/properties.c               |   10 ++++++++++
>  src/synaptics.c                |   19 +++++++++++--------
>  src/synapticsstr.h             |    1 +
>  tools/synclient.c              |    1 +
>  6 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> index c550cef..ec1c4f5 100644
> --- a/include/synaptics-properties.h
> +++ b/include/synaptics-properties.h
> @@ -113,6 +113,9 @@
>  /* 8 bit (BOOL) */
>  #define SYNAPTICS_PROP_CIRCULAR_SCROLLING "Synaptics Circular Scrolling"
>  
> +/* 8 bit (BOOL) */
> +#define SYNAPTICS_PROP_CIRCULAR_SCROLLING_INVERT "Synaptics Invert Circular Scrolling"
> +
>  /* FLOAT */
>  #define SYNAPTICS_PROP_CIRCULAR_SCROLLING_DIST "Synaptics Circular Scrolling Distance"
>  
> diff --git a/man/synaptics.man b/man/synaptics.man
> index b6b1dce..5600de5 100644
> --- a/man/synaptics.man
> +++ b/man/synaptics.man
> @@ -368,6 +368,9 @@ Set to 0 to disable. Property: "Synaptics Click Action"
>  .BI "Option \*qCircularScrolling\*q \*q" boolean \*q
>  If on, circular scrolling is used. Property: "Synaptics Circular Scrolling"
>  .TP
> +.BI "Option \*qCircScrollInvert\*q \*q" boolean \*q
> +If on, direction of circular scrolling is inverted. Property: "Synaptics Invert Circular Scrolling"
> +.TP
>  .BI "Option \*qCircScrollDelta\*q \*q" float \*q
>  Move angle (radians) of finger to generate a scroll event. Property: "Synaptics
>  Circular Scrolling Distance"
> @@ -867,6 +870,10 @@ order: Finger 1, 2, 3.
>  8 bit (BOOL).
>  
>  .TP 7
> +.BI "Synaptics Invert Circular Scrolling"
> +8 bit (BOOL).
> +
> +.TP 7
>  .BI "Synaptics Circular Scrolling Distance"
>  FLOAT.
>  
> diff --git a/src/properties.c b/src/properties.c
> index 0a52801..ebdfbf4 100644
> --- a/src/properties.c
> +++ b/src/properties.c
> @@ -78,6 +78,7 @@ Atom prop_lockdrags_time        = 0;
>  Atom prop_tapaction             = 0;
>  Atom prop_clickaction           = 0;
>  Atom prop_circscroll            = 0;
> +Atom prop_circscroll_invert     = 0;
>  Atom prop_circscroll_dist       = 0;
>  Atom prop_circscroll_trigger    = 0;
>  Atom prop_circpad               = 0;
> @@ -248,6 +249,8 @@ InitDeviceProperties(InputInfoPtr pInfo)
>  
>      prop_circscroll = InitAtom(pInfo->dev, SYNAPTICS_PROP_CIRCULAR_SCROLLING, 8, 1, &para->circular_scrolling);
>  
> +    prop_circscroll_invert = InitAtom(pInfo->dev, SYNAPTICS_PROP_CIRCULAR_SCROLLING_INVERT, 8, 1, &para->circular_scrolling_invert);
> +
>      fvalues[0] = para->scroll_dist_circ;
>      prop_circscroll_dist = InitFloatAtom(pInfo->dev, SYNAPTICS_PROP_CIRCULAR_SCROLLING_DIST, 1, fvalues);
>  
> @@ -610,6 +613,13 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
>  
>          para->circular_scrolling = *(BOOL*)prop->data;
>  
> +    } else if (property == prop_circscroll_invert)
> +    {
> +        if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
> +            return BadMatch;
> +
> +        para->circular_scrolling_invert = *(BOOL*)prop->data;
> +
>      } else if (property == prop_circscroll_dist)
>      {
>          float circdist;
> diff --git a/src/synaptics.c b/src/synaptics.c
> index 1d963cb..85e62f0 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -551,10 +551,10 @@ static void set_default_parameters(InputInfoPtr pInfo)
>      pars->edge_motion_max_speed = xf86SetIntOption(opts, "EdgeMotionMaxSpeed", edgeMotionMaxSpeed);
>      pars->edge_motion_use_always = xf86SetBoolOption(opts, "EdgeMotionUseAlways", FALSE);
>      if (priv->has_scrollbuttons) {
> -	pars->updown_button_scrolling = xf86SetBoolOption(opts, "UpDownScrolling", TRUE);
> -	pars->leftright_button_scrolling = xf86SetBoolOption(opts, "LeftRightScrolling", TRUE);
> -	pars->updown_button_repeat = xf86SetBoolOption(opts, "UpDownScrollRepeat", TRUE);
> -	pars->leftright_button_repeat = xf86SetBoolOption(opts, "LeftRightScrollRepeat", TRUE);
> +        pars->updown_button_scrolling = xf86SetBoolOption(opts, "UpDownScrolling", TRUE);
> +        pars->leftright_button_scrolling = xf86SetBoolOption(opts, "LeftRightScrolling", TRUE);
> +        pars->updown_button_repeat = xf86SetBoolOption(opts, "UpDownScrollRepeat", TRUE);
> +        pars->leftright_button_repeat = xf86SetBoolOption(opts, "LeftRightScrollRepeat", TRUE);
>      }
>      pars->scroll_button_repeat = xf86SetIntOption(opts,"ScrollButtonRepeat", 100);
>      pars->touchpad_off = xf86SetIntOption(opts, "TouchpadOff", 0);
> @@ -571,6 +571,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
>      pars->click_action[F2_CLICK1] = xf86SetIntOption(opts, "ClickFinger2", clickFinger2);
>      pars->click_action[F3_CLICK1] = xf86SetIntOption(opts, "ClickFinger3", clickFinger3);
>      pars->circular_scrolling = xf86SetBoolOption(opts, "CircularScrolling", FALSE);
> +    pars->circular_scrolling_invert = xf86SetBoolOption(opts, "CircScrollInvert", FALSE);
>      pars->circular_trigger   = xf86SetIntOption(opts, "CircScrollTrigger", 0);
>      pars->circular_pad       = xf86SetBoolOption(opts, "CircularPad", FALSE);
>      pars->palm_detect        = xf86SetBoolOption(opts, "PalmDetect", FALSE);
> @@ -2098,16 +2099,17 @@ start_coasting(SynapticsPrivate *priv, struct SynapticsHwState *hw,
>  	if (circ) {
>  	    double da = estimate_delta_circ(priv);
>  	    double sdelta = para->scroll_dist_circ;
> +		double invert = para->circular_scrolling_invert ? -1 : 1;
>  	    if (pkt_time > 0 && sdelta > 0) {
>  	        double scrolls_per_sec = da / pkt_time / sdelta;
>  	        if (fabs(scrolls_per_sec) >= para->coasting_speed) {
>  	            if (vert) {
>  	                priv->scroll.coast_speed_y = scrolls_per_sec;
> -	                priv->scroll.coast_delta_y = diffa(priv->scroll.last_a, angle(priv, hw->x, hw->y)) / sdelta;
> +	                priv->scroll.coast_delta_y = diffa(priv->scroll.last_a, angle(priv, hw->x, hw->y)) / sdelta * invert;
>  	            }
>  	            else if (horiz) {
>  	                priv->scroll.coast_speed_x = scrolls_per_sec;
> -	                priv->scroll.coast_delta_x = diffa(priv->scroll.last_a, angle(priv, hw->x, hw->y)) / sdelta;
> +	                priv->scroll.coast_delta_x = diffa(priv->scroll.last_a, angle(priv, hw->x, hw->y)) / sdelta * invert;
>  	            }
>  	        }
>  	    }
> @@ -2321,12 +2323,13 @@ HandleScrolling(SynapticsPrivate *priv, struct SynapticsHwState *hw,
>      if (priv->circ_scroll_on) {
>  	/* + = counter clockwise, - = clockwise */
>  	double delta = para->scroll_dist_circ;
> +	double invert = para->circular_scrolling_invert ? -1 : 1;
>  	double diff = diffa(priv->scroll.last_a, angle(priv, hw->x, hw->y));
>  	if (delta >= 0.005 && diff != 0.0) {
>  	    if (priv->circ_scroll_vert)
> -		priv->scroll.delta_y += diff / delta * para->scroll_dist_vert;
> +			priv->scroll.delta_y += diff / delta * para->scroll_dist_vert * invert;
>  	    else
> -		priv->scroll.delta_x += diff / delta * para->scroll_dist_horiz;;
> +			priv->scroll.delta_x += diff / delta * para->scroll_dist_horiz * invert;
>  	    priv->scroll.last_a = angle(priv, hw->x, hw->y);
>          }
>      }
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index d3b8607..47ebe7e 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -161,6 +161,7 @@ typedef struct _SynapticsParameters
>      int tap_action[MAX_TAP];		    /* Button to report on tap events */
>      int click_action[MAX_CLICK];	    /* Button to report on click with fingers */
>      Bool circular_scrolling;		    /* Enable circular scrolling */
> +    Bool circular_scrolling_invert;		/* Invert circular scrolling direction*/
>      double scroll_dist_circ;		    /* Scrolling angle radians */
>      int circular_trigger;		    /* Trigger area for circular scrolling */
>      Bool circular_pad;			    /* Edge has an oval or circular shape */
> diff --git a/tools/synclient.c b/tools/synclient.c
> index 08bd22b..e1ced79 100644
> --- a/tools/synclient.c
> +++ b/tools/synclient.c
> @@ -124,6 +124,7 @@ static struct Parameter params[] = {
>      {"ClickFinger2",          PT_INT,    0, SYN_MAX_BUTTONS, SYNAPTICS_PROP_CLICK_ACTION,	8,	1},
>      {"ClickFinger3",          PT_INT,    0, SYN_MAX_BUTTONS, SYNAPTICS_PROP_CLICK_ACTION,	8,	2},
>      {"CircularScrolling",     PT_BOOL,   0, 1,     SYNAPTICS_PROP_CIRCULAR_SCROLLING,	8,	0},
> +    {"CircScrollInvert",      PT_BOOL,   0, 1,     SYNAPTICS_PROP_CIRCULAR_SCROLLING_INVERT,	8,	0},
>      {"CircScrollDelta",       PT_DOUBLE, .01, 3,   SYNAPTICS_PROP_CIRCULAR_SCROLLING_DIST,	0 /* float */,	0},
>      {"CircScrollTrigger",     PT_INT,    0, 8,     SYNAPTICS_PROP_CIRCULAR_SCROLLING_TRIGGER,	8,	0},
>      {"CircularPad",           PT_BOOL,   0, 1,     SYNAPTICS_PROP_CIRCULAR_PAD,	8,	0},
> -- 
> 1.7.8.3
> 
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 


More information about the xorg-devel mailing list