[PATCH synaptics] Add synaptics orientation support

[zt.tmzt at gmail.com]

Why does this need to be a SYNAPTICS property, synclient does not respect
XInput properties? Are they changing the same thing on the backend and using
the same semantics? (On the last, no, I believe the min/max have to be
changed and axes swapped on evdev devices)

--
Timothy Meade
tmzt on freenode
On Mar 18, 2011 2:42 AM, "Peter Hutterer" <peter.hutterer at who-t.net> wrote:
> Hi Aapo,
>
> I noticed this patch in the synaptics repo today. Unfortunately, it needs
> a bit more work, so I've reverted it for now. Please find my comments
> inline.
>
> fwiw, patches to synatics should go to the xorg-devel list first for
public
> review.
>
> On Fri, Mar 18, 2011 at 04:26:46PM +1000, Peter Hutterer wrote:
>> This patch allows usage of "synclient Orientation=0" (values from 0 to
>> 3). It will rotate the touchpad similar to "xrandr -o". Original patch
>> was extended for alps and ps2.
>>
>> Signed-off-by: Christoph Brill <egore911 at egore911.de>
>> ---
>> include/synaptics-properties.h | 3 +++
>> man/synaptics.man | 6 ++++++
>> src/alpscomm.c | 29 +++++++++++++++++++++++++----
>> src/eventcomm.c | 22 ++++++++++++++++++++--
>> src/properties.c | 8 ++++++++
>> src/ps2comm.c | 36 +++++++++++++++++++++++++++++-------
>> src/synaptics.c | 2 ++
>> src/synapticsstr.h | 1 +
>> tools/synclient.c | 1 +
>> 9 files changed, 95 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/synaptics-properties.h
b/include/synaptics-properties.h
>> index bdb2112..0b4e570 100644
>> --- a/include/synaptics-properties.h
>> +++ b/include/synaptics-properties.h
>> @@ -158,4 +158,7 @@
>> /* 32 Bit Integer, 2 values, horizontal hysteresis, vertical hysteresis
*/
>> #define SYNAPTICS_PROP_NOISE_CANCELLATION "Synaptics Noise Cancellation"
>>
>> +/* 32 bit, 4 values, normal, inverted, left, right */
>> +#define SYNAPTICS_ORIENTATION "Synaptics Orientation"
>
> why not use degrees here?
> this opens the way for a unified rotation property for devices that need a
> rotation outside of 90 degree values.
>
> "left" doesn't have a clear meaning. "90 degrees clockwise" is less
> ambiguous
>
> "orientation" vs "rotation"?
> I'm more of a fan of the latter, but can be convinced otherwise.
>
>
>> +
>> #endif /* _SYNAPTICS_PROPERTIES_H_ */
>> diff --git a/man/synaptics.man b/man/synaptics.man
>> index 0a35883..44d1c27 100644
>> --- a/man/synaptics.man
>> +++ b/man/synaptics.man
>> @@ -510,6 +510,12 @@ AreaBottomEdge option to any integer value other
than zero. If supported by the
>> server (version 1.9 and later), the edge may be specified in percent of
>> the total height of the touchpad. Property: "Synaptics Area"
>> .
>> +.TP
>> +.BI "Option \*qOrientation\*q \*q" integer \*q
>> +Rotate the touchpad similar to xrandr -o.
>> +.
>> +The orientation can be 0 (normal), 1(left), 2 (inverted) or 3(right).
>> +.
>
> same here, just because xrandr uses 0-3 doesn't make it a good idea ;)
>
>>
>> .SH CONFIGURATION DETAILS
>> .SS Area handling
>> diff --git a/src/alpscomm.c b/src/alpscomm.c
>> index dc76655..7d5cda2 100644
>> --- a/src/alpscomm.c
>> +++ b/src/alpscomm.c
>> @@ -149,11 +149,12 @@ ALPS_get_packet(struct CommData *comm, InputInfoPtr
pInfo)
>> * reflects left,right,down,up in lef1,rig1,mid1,up1.
>> */
>> static void
>> -ALPS_process_packet(unsigned char *packet, struct SynapticsHwState *hw)
>> +ALPS_process_packet(SynapticsPrivate *priv, unsigned char *packet,
struct SynapticsHwState *hw)
>> {
>> int x = 0, y = 0, z = 0;
>> int left = 0, right = 0, middle = 0;
>> int i;
>> + SynapticsParameters *para = &priv->synpara;
>>
>> x = (packet[1] & 0x7f) | ((packet[2] & 0x78) << (7-3));
>> y = (packet[4] & 0x7f) | ((packet[3] & 0x70) << (7-4));
>> @@ -172,8 +173,27 @@ ALPS_process_packet(unsigned char *packet, struct
SynapticsHwState *hw)
>> hw->multi[i] = FALSE;
>>
>> if (z > 0) {
>> - hw->x = x;
>> - hw->y = y;
>> + if (para->orientation==0)
>> + hw->x = x;
>> + else if (para->orientation==2)
>
> self-explanatory enums please, not magic numbers.
> else if (para->orientation == ROTATION_90CW)
> is much easier to read.
>
> (also, spaces before/after ==)
>
>> + hw->x = priv->maxx + priv->minx - x;
>> + else if (para->orientation==3)
>> + hw->y = (priv->maxx - x) * (priv->maxy - priv->miny) / (priv->maxx -
priv->minx) + priv->miny;
>> + else if (para->orientation==1)
>> + hw->y = (x - priv->minx) * (priv->maxy - priv->miny) / (priv->maxx -
priv->minx) + priv->miny;
>> + else
>> + hw->x = x;
>> +
>> + if (para->orientation==0)
>> + hw->y = y;
>> + else if (para->orientation==2)
>> + hw->y = priv->maxy + priv->miny - y;
>> + else if (para->orientation==3)
>> + hw->x = (y - priv->miny) * (priv->maxx - priv->minx) / (priv->maxy -
priv->miny) + priv->minx;
>> + else if (para->orientation==1)
>> + hw->x = (priv->maxy - y) * (priv->maxx - priv->minx) / (priv->maxy -
priv->miny) + priv->minx;
>> + else
>> + hw->y = y;
>
> this needs to be done in a function to avoid duplicating the code.
>
>> }
>> hw->z = z;
>> hw->numFingers = (z > 0) ? 1 : 0;
>> @@ -210,11 +230,12 @@ ALPSReadHwState(InputInfoPtr pInfo,
>> {
>> unsigned char *buf = comm->protoBuf;
>> struct SynapticsHwState *hw = &(comm->hwState);
>> + SynapticsPrivate *priv = (SynapticsPrivate *)pInfo->private;
>>
>> if (!ALPS_get_packet(comm, pInfo))
>> return FALSE;
>>
>> - ALPS_process_packet(buf, hw);
>> + ALPS_process_packet(priv, buf, hw);
>>
>> *hwRet = *hw;
>> return TRUE;
>> diff --git a/src/eventcomm.c b/src/eventcomm.c
>> index d394d3f..3d550f1 100644
>> --- a/src/eventcomm.c
>> +++ b/src/eventcomm.c
>> @@ -400,10 +400,28 @@ EventReadHwState(InputInfoPtr pInfo,
>> case EV_ABS:
>> switch (ev.code) {
>> case ABS_X:
>> - hw->x = ev.value;
>> + if (para->orientation==0)
>> + hw->x = ev.value;
>> + else if (para->orientation==2)
>> + hw->x = priv->maxx + priv->minx - ev.value;
>> + else if (para->orientation==3)
>> + hw->y = (priv->maxx - ev.value) * (priv->maxy - priv->miny) /
(priv->maxx - priv->minx) + priv->miny;
>> + else if (para->orientation==1)
>> + hw->y = (ev.value - priv->minx) * (priv->maxy - priv->miny) /
(priv->maxx - priv->minx) + priv->miny;
>> + else
>> + hw->x = ev.value;
>> break;
>> case ABS_Y:
>> - hw->y = ev.value;
>> + if (para->orientation==0)
>> + hw->y = ev.value;
>> + else if (para->orientation==2)
>> + hw->y = priv->maxy + priv->miny - ev.value;
>> + else if (para->orientation==3)
>> + hw->x = (ev.value - priv->miny) * (priv->maxx - priv->minx) /
(priv->maxy - priv->miny) + priv->minx;
>> + else if (para->orientation==1)
>> + hw->x = (priv->maxy - ev.value) * (priv->maxx - priv->minx) /
(priv->maxy - priv->miny) + priv->minx;
>> + else
>> + hw->y = ev.value;
>> break;
>> case ABS_PRESSURE:
>> hw->z = ev.value;
>> diff --git a/src/properties.c b/src/properties.c
>> index 23b5a6a..06909ed 100644
>> --- a/src/properties.c
>> +++ b/src/properties.c
>> @@ -83,6 +83,7 @@ Atom prop_capabilities = 0;
>> Atom prop_resolution = 0;
>> Atom prop_area = 0;
>> Atom prop_noise_cancellation = 0;
>> +Atom prop_orientation = 0;
>>
>> static Atom
>> InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int
*values)
>> @@ -285,6 +286,8 @@ InitDeviceProperties(InputInfoPtr pInfo)
>> prop_noise_cancellation = InitAtom(pInfo->dev,
>> SYNAPTICS_PROP_NOISE_CANCELLATION, 32, 2, values);
>>
>> + prop_orientation = InitAtom(pInfo->dev, SYNAPTICS_ORIENTATION, 32, 1,
&para->orientation);
>> +
>> }
>>
>> int
>> @@ -666,6 +669,11 @@ SetProperty(DeviceIntPtr dev, Atom property,
XIPropertyValuePtr prop,
>> return BadValue;
>> para->hyst_x = hyst[0];
>> para->hyst_y = hyst[1];
>> + } else if (property == prop_orientation)
>> + {
>> + if (prop->size != 1 || prop->format != 32 || prop->type != XA_INTEGER)
>> + return BadMatch;
>> + para->orientation = *(INT32*)prop->data;
>> }
>>
>> return Success;
>> diff --git a/src/ps2comm.c b/src/ps2comm.c
>> index 0e9b861..1c2bbc3 100644
>> --- a/src/ps2comm.c
>> +++ b/src/ps2comm.c
>> @@ -524,7 +524,7 @@ PS2ReadHwStateProto(InputInfoPtr pInfo,
>> SynapticsParameters *para = &priv->synpara;
>> struct PS2SynapticsHwInfo *synhw;
>> int newabs;
>> - int w, i;
>> + int w, i, x, y;
>>
>> synhw = (struct PS2SynapticsHwInfo*)priv->proto_data;
>> if (!synhw)
>> @@ -541,17 +541,17 @@ PS2ReadHwStateProto(InputInfoPtr pInfo,
>> return FALSE;
>>
>> /* Handle normal packets */
>> - hw->x = hw->y = hw->z = hw->numFingers = hw->fingerWidth = 0;
>> + hw->x = hw->y = hw->z = hw->numFingers = hw->fingerWidth = x = y = 0;
>
> urgh, just add a new line.
>
>> hw->left = hw->right = hw->up = hw->down = hw->middle = FALSE;
>> for (i = 0; i < 8; i++)
>> hw->multi[i] = FALSE;
>>
>> if (newabs) { /* newer protos...*/
>> DBG(7, "using new protocols\n");
>> - hw->x = (((buf[3] & 0x10) << 8) |
>> + x = (((buf[3] & 0x10) << 8) |
>> ((buf[1] & 0x0f) << 8) |
>> buf[4]);
>> - hw->y = (((buf[3] & 0x20) << 7) |
>> + y = (((buf[3] & 0x20) << 7) |
>> ((buf[1] & 0xf0) << 4) |
>> buf[5]);
>>
>> @@ -598,9 +598,9 @@ PS2ReadHwStateProto(InputInfoPtr pInfo,
>> }
>> } else { /* old proto...*/
>> DBG(7, "using old protocol\n");
>> - hw->x = (((buf[1] & 0x1F) << 8) |
>> + x = (((buf[1] & 0x1F) << 8) |
>> buf[2]);
>> - hw->y = (((buf[4] & 0x1F) << 8) |
>> + y = (((buf[4] & 0x1F) << 8) |
>> buf[5]);
>>
>> hw->z = (((buf[0] & 0x30) << 2) |
>> @@ -612,7 +612,29 @@ PS2ReadHwStateProto(InputInfoPtr pInfo,
>> hw->right = (buf[0] & 0x02) ? 1 : 0;
>> }
>>
>> - hw->y = YMAX_NOMINAL + YMIN_NOMINAL - hw->y;
>> + y = YMAX_NOMINAL + YMIN_NOMINAL - y;
>> +
>> + if (para->orientation==0)
>> + hw->x = x;
>> + else if (para->orientation==2)
>> + hw->x = priv->maxx + priv->minx - x;
>> + else if (para->orientation==3)
>> + hw->y = (priv->maxx - x) * (priv->maxy - priv->miny) / (priv->maxx -
priv->minx) + priv->miny;
>> + else if (para->orientation==1)
>> + hw->y = (x - priv->minx) * (priv->maxy - priv->miny) / (priv->maxx -
priv->minx) + priv->miny;
>> + else
>> + hw->x = x;
>> +
>> + if (para->orientation==0)
>> + hw->y = y;
>> + else if (para->orientation==2)
>> + hw->y = priv->maxy + priv->miny - y;
>> + else if (para->orientation==3)
>> + hw->x = (y - priv->miny) * (priv->maxx - priv->minx) / (priv->maxy -
priv->miny) + priv->minx;
>> + else if (para->orientation==1)
>> + hw->x = (priv->maxy - y) * (priv->maxx - priv->minx) / (priv->maxy -
priv->miny) + priv->minx;
>> + else
>> + hw->y = y;
>
> duplication again, needs abstraction.
>
>>
>> if (hw->z >= para->finger_high) {
>> int w_ok = 0;
>> diff --git a/src/synaptics.c b/src/synaptics.c
>> index 1233917..03a9f60 100644
>> --- a/src/synaptics.c
>> +++ b/src/synaptics.c
>> @@ -574,6 +574,8 @@ static void set_default_parameters(InputInfoPtr
pInfo)
>> pars->resolution_horiz = xf86SetIntOption(opts, "HorizResolution",
horizResolution);
>> pars->resolution_vert = xf86SetIntOption(opts, "VertResolution",
vertResolution);
>>
>> + pars->orientation = xf86SetIntOption(opts, "Orientation", 0);
>> +
>> /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge
parameters */
>> if (pars->top_edge > pars->bottom_edge) {
>> int tmp = pars->top_edge;
>> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
>> index 8f6593e..90640f7 100644
>> --- a/src/synapticsstr.h
>> +++ b/src/synapticsstr.h
>> @@ -161,6 +161,7 @@ typedef struct _SynapticsParameters
>> unsigned int resolution_vert; /* vertical resolution of touchpad in
units/mm */
>> int area_left_edge, area_right_edge, area_top_edge, area_bottom_edge; /*
area coordinates absolute */
>> int hyst_x, hyst_y; /* x and y width of hysteresis box */
>> + int orientation; /* orientation of the touchpad */
>> } SynapticsParameters;
>>
>>
>> diff --git a/tools/synclient.c b/tools/synclient.c
>> index 9776d23..1ac5502 100644
>> --- a/tools/synclient.c
>> +++ b/tools/synclient.c
>> @@ -143,6 +143,7 @@ static struct Parameter params[] = {
>> {"AreaRightEdge", PT_INT, 0, 10000, SYNAPTICS_PROP_AREA, 32, 1},
>> {"AreaTopEdge", PT_INT, 0, 10000, SYNAPTICS_PROP_AREA, 32, 2},
>> {"AreaBottomEdge", PT_INT, 0, 10000, SYNAPTICS_PROP_AREA, 32, 3},
>> + {"Orientation", PT_INT, 0, 3, SYNAPTICS_ORIENTATION, 32, 0},
> ^ tab missing?
>
> Cheers,
> Peter
>
>> { NULL, 0, 0, 0, 0 }
>> };
>>
>> --
>> 1.7.4
> _______________________________________________
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110319/0227b77a/attachment-0001.html>