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 <div class="gmail_quote">On Mar 18, 2011 2:42 AM, "Peter Hutterer" <<a href="mailto:peter.hutterer@who-t.net">peter.hutterer@who-t.net</a>> 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 <<a href="mailto:egore911@egore911.de">egore911@egore911.de</a>> >> --- >> 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 > _______________________________________________ > <a href="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</a>: X.Org development > Archives: <a href="http://lists.x.org/archives/xorg-devel">http://lists.x.org/archives/xorg-devel</a> > Info: <a href="http://lists.x.org/mailman/listinfo/xorg-devel">http://lists.x.org/mailman/listinfo/xorg-devel</a> </div>