On Mon, Aug 22, 2011 at 12:04 AM, Peter Hutterer
<peter.hutte...@who-t.net> wrote:
> On Fri, Aug 19, 2011 at 05:23:57PM -0700, Jason Gerecke wrote:
>> At the moment, this helper is only capable of understanding the
>> keyword "next". This will cause it to move to the next available
>> output, with the order determined by the underlying list of heads.
>>
>> Its hoped this can be expanded to support other keywords, most
>> notably "left" and "right", which would choose the correct head
>> based on its relative position in space.
>
> whoah. tbh that's getting a bit past the point where I'd like xsetwacom to
> be. if we're intending to make xsetwacom to be a useful tool, then we should
> re-think a few things - especially about desktop integration.
> what is the concrete use-case you need this for?
>
>> ---
>> man/xsetwacom.man | 3 +-
>> tools/xsetwacom.c | 198
>> +++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 194 insertions(+), 7 deletions(-)
>>
>> diff --git a/man/xsetwacom.man b/man/xsetwacom.man
>> index 68599b8..fbdfc16 100644
>> --- a/man/xsetwacom.man
>> +++ b/man/xsetwacom.man
>> @@ -123,7 +123,8 @@ Default: 0
>> Map the tablet's input area to the given output (e.g. "VGA1"), or the entire
>> desktop if no output is provided. Output names may either be the name of
>> a head available through the XRandR extension, or an defined area of the
>> -desktop with the form WIDTHxHEIGHT@X,Y. Users of the NVIDIA binary driver
>> +desktop with the form WIDTHxHEIGHT@X,Y. To switch to the next available
>> output,
>> +the "next" keyword is also supported. Users of the NVIDIA binary driver
>> should use the output names "HEAD-0" and "HEAD-1" until proper XRandR
>> support
>> is included.
>>
>> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
>> index 32a1d83..66e2697 100644
>> --- a/tools/xsetwacom.c
>> +++ b/tools/xsetwacom.c
>> @@ -2150,12 +2150,168 @@ out:
>> }
>>
>> /**
>> - * Adjust the transformation matrix based on a user-defined area.
>> - * This function will attempt to map the given pointer to the area defined
>> - * in the first command-line argument (which should be formatted as
>> - * 'WIDTHxHEIGHT@X,Y'). If this function succeeds in modifying the
>> - * transformation matrix, it returns 'true'.
>> + * Given the dimensions and origin of one of the outputs, this
>> + * function finds the corresponding output in the list of Xinerama
>> + * heads and then returns the information for the next output.
>> + *
>> + * If the function fails (e.g. is unable to locate the output
>> + * matching the arguments), it returns False.
>> */
>> +Bool get_next_output_area(Display *dpy, int *width, int *height, int
>> *x_org, int *y_org)
>> +{
>> + XineramaScreenInfo *screens;
>> + int event, error, nscreens, head;
>> + int theOne = -1;
>
> under_scores please, not javaNotationOfVariableNames.
> theOne is a bit ambiguous (pun not intended), how about current_screen?
>
>> + int display_width = DisplayWidth(dpy, DefaultScreen(dpy));
>> + int display_height = DisplayHeight(dpy, DefaultScreen(dpy));
>> +
>> + if (!XineramaQueryExtension(dpy, &event, &error))
>> + {
>> + TRACE("Unable to get screen mapping. Xinerama extension not
>> found\n");
>> + return False;
>> + }
>> +
>> + screens = XineramaQueryScreens(dpy, &nscreens);
>> +
>> + if (nscreens == 0)
>> + {
>> + TRACE("Xinerama failed to query screens.\n");
>> + XFree(screens);
>> + return False;
>> + }
>> +
>> + /* Compare the given dimensions to all the heads */
>> + for (head=0; head<nscreens; head++)
>
> spaces before and after '=' and others please
>
>> + {
>> + if (screens[head].width == *width && screens[head].height ==
>> *height &&
>> + screens[head].x_org == *x_org && screens[head].y_org ==
>> *y_org)
>> + theOne = head;
>> +
>> + TRACE("%s%d: %dx%d @ %d,%d\n",
>> + theOne == head ? "*" : " ", head,
>> + screens[head].width, screens[head].height,
>> + screens[head].x_org, screens[head].y_org);
>> + }
>> +
>> + /* Compare the given dimensions to the entire desktop */
>> + if (display_width == *width && display_height == *height)
>> + theOne = nscreens;
>> + TRACE("%sS: %dx%d @ %d,%d\n",
>> + theOne == nscreens? "*" : " ", display_width, display_height,
>> 0, 0);
>> +
>> +
>> + /* Special case: check to see if we're mapped to the entire desktop */
>> + if (theOne == nscreens)
>> + {
>> + *width = screens[0].width;
>> + *height = screens[0].height;
>> + *x_org = screens[0].x_org;
>> + *y_org = screens[0].y_org;
>> + }
>> + else if (++theOne == nscreens)
>> + {
>> + *width = display_width;
>> + *height = display_height;
>> + *x_org = 0;
>> + *y_org = 0;
>> + }
>> + else if (theOne >= 0)
>
> this condition is always true since you use ++theOne in the previous
> condition. also, this is confusing me. First condition catches a mapping to
> the desktop (i.e. no mapping), in which case we assume the first screen.
> Second condition matches on the last screen, in which case we assume the
> desktop size? last condition (always true) then uses the matched screen.
>
> Is "map to desktop" one more in the cycling order? i.e. continuous use of
> "next" gives you screen 0, screen 1, desktop, screen 0, ...? If so, this
> needs to be documented.
>
>> + {
>> + *width = screens[theOne].width;
>> + *height = screens[theOne].height;
>> + *x_org = screens[theOne].x_org;
>> + *y_org = screens[theOne].y_org;
>> + }
>> +
>> + XFree(screens);
>> + return True;
>> +}
>> +
>> +
>> +/**
>> + * Uses the area of the desktop and the server's transformation
>> + * matrix to calculate the dimensions and location of the area
>> + * the given device is mapped to. If the matrix describes a
>> + * non-rectangular transform (e.g. rotation or shear), this
>> + * function returns False.
>> + */
>> +Bool get_mapped_area(Display *dpy, XDevice *dev, int *width, int *height,
>> int *x_org, int *y_org)
>> +{
>> + Atom matrix_prop = XInternAtom(dpy, "Coordinate Transformation
>> Matrix", True);
>> + Atom type;
>> + int format;
>> + unsigned long nitems, bytes_after;
>> + float *data;
>> + int isGoodMatrix = 1;
>
> matrix_is_valid
>
>> + int i;
>> +
>> + int display_width = DisplayWidth(dpy, DefaultScreen(dpy));
>> + int display_height = DisplayHeight(dpy, DefaultScreen(dpy));
>> + TRACE("Desktop width: %d, height: %d\n", display_width,
>> display_height);
>> +
>> + if (!matrix_prop)
>> + {
>> + TRACE("Server does not support transformation\n");
>> + return False;
>> + }
>> +
>> + XGetDeviceProperty(dpy, dev, matrix_prop, 0, 9, False,
>> + AnyPropertyType, &type, &format, &nitems,
>> + &bytes_after, (unsigned char**)&data);
>> +
>> + if (format != 32 || type != XInternAtom(dpy, "FLOAT", True))
>> + {
>> + TRACE("Property for '%s' has unexpected type - this is a
>> bug.\n",
>> + "Coordinate Transformation Matrix");
>
> should probably use a define for that property name. I've already spent some
> time trying to debug typos and I guess there will be more time spent on that
> in the future ;)
>
>> + XFree(data);
>> + return False;
>> + }
>> +
>> + TRACE("Current transformation matrix:\n");
>> + TRACE(" [ %f %f %f ]\n", data[0], data[1], data[2]);
>> + TRACE(" [ %f %f %f ]\n", data[3], data[4], data[5]);
>> + TRACE(" [ %f %f %f ]\n", data[6], data[7], data[8]);
>> +
>> + for (i = 0; i < nitems; i++)
>> + {
>> + if (i == 0)
>> + *width = rint(display_width * data[i]);
>> + else if (i == 2)
>> + *x_org = rint(display_width * data[i]);
>> + else if (i == 4)
>> + *height = rint(display_height * data[i]);
>> + else if (i == 5)
>> + *y_org = rint(display_height * data[i]);
>> + else if (i == 8)
>> + {
>> + if ((float)data[i] != 1)
>
> why do you need to cast to float here? isn't data already float?
>
>> + {
>> + isGoodMatrix = 0;
>> + TRACE("The matrix looks bad at position
>> %d.\n", i);
>
> something more descriptive than "looks bad" would be nice
>
>> + }
>> + }
>> + else
>> + {
>> + if ((float)data[i] != 0)
>> + {
>> + isGoodMatrix = 0;
>> + TRACE("The matrix looks bad at position
>> %d.\n", i);
>> + }
>> + }
>
> de-duplicate please, plus - if one is bad you could just exit the loop, so
> you could move the TRACE.
>
> else if (i == 8) {
> if (data[i] != 1.0f)
> matrix_is_valid = 0;
> } else if (data[i] != 0.0f) {
> matrix_is_valid = 0;
> }
>
> if (!matrix_is_valid) {
> TRACE("Non-rectangular matrix detected...
> break;
> }
>
>
>> + }
>> +
>> + XFree(data);
>> +
>> + return isGoodMatrix == 1;
>
> make it a Bool instead of an int and you won't need the == 1.
>
> btw: what about vertical or quad-mapped configurations? supported?
>
> Cheers,
> Peter
>
>> +}
>> +
>> +/**
>> +* Adjust the transformation matrix based on a user-defined area.
>> +* This function will attempt to map the given pointer to the area defined
>> +* in the first command-line argument (which should be formatted as
>> +* 'WIDTHxHEIGHT@X,Y'). If this function succeeds in modifying the
>> +* transformation matrix, it returns 'true'.
>> +*/
>> static Bool set_output_area(Display *dpy, XDevice *dev, param_t *param, int
>> argc, char **argv)
>> {
>> int width, height, xorig, yorig;
>> @@ -2174,10 +2330,40 @@ static Bool set_output_area(Display *dpy, XDevice
>> *dev, param_t *param, int argc
>> return False;
>> }
>>
>> +/**
>> + * Adjust the transformation matrix based on its current value and a
>> + * desired relative direction the mapping should move. At the moment,
>> + * this function only supports the command-line argument "next" (indicating
>> + * the user's desire to switch to the next output in an arbitrary list).
>> + * If this function succeeds in modifying the transformation matrix, it
>> + * returns 'true'.
>> + */
>> +static Bool set_output_relative(Display *dpy, XDevice *dev, param_t *param,
>> int argc, char **argv)
>> +{
>> + int w, h, x_org, y_org;
>> +
>> + if (strcasecmp(argv[0], "next") == 0)
>> + {
>> + if (get_mapped_area(dpy, dev, &w, &h, &x_org, &y_org))
>> + if (get_next_output_area(dpy, &w, &h, &x_org, &y_org))
>> + {
>> + _set_matrix(dpy, dev, x_org, y_org, w, h);
>> + fprintf(stderr, "Remapping to output area
>> obtained by relative position.\n");
>> + return True;
>> + }
>> + }
>> + else
>> + {
>> + TRACE("Unable to parse '%s' as a relative movement.\n",
>> argv[0]);
>> + }
>> +
>> + return False;
>> +}
>> +
>> static void set_output(Display *dpy, XDevice *dev, param_t *param, int
>> argc, char **argv)
>> {
>> Bool (*fn[]) (Display * dpy, XDevice *dev, param_t *param, int argc,
>> char **argv) =
>> - { set_output_area, set_output_xinerama, set_output_xrandr };
>> + { set_output_relative, set_output_area, set_output_xinerama,
>> set_output_xrandr };
>>
>> if (argc == 0)
>> {
>> --
>> 1.7.6
>
Almost all the inline replies would be some combination of "whoops",
"confound you, Java indoctrination", and/or "I figured you wouldn't
accept this patch (even I think it stinks), but mostly needed to know
if the concept was on track". :)
As for the directional keywords, I have no pressing need for keywords
specifying absolute direction; the comment merely hypothesizes what
could be done. To be honest, I figured you would rather have concrete
left/right/up/down keywords than nebulous next/prev keywords. Vertical
and quad-mapped outputs should work fine with the 'next' keyword, and
if absolute directions are ever included, I would certainly hope that
they take into account varying rows of monitors.
Jason
---
Day xee-nee-svsh duu-'ushtlh-ts'it;
nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it.
Huu-chan xuu naa~-gha.
------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model
configuration take the hassle out of deploying and managing Subversion and
the tools developers use with it. Learn more about uberSVN and get a free
download at: http://p.sf.net/sfu/wandisco-dev2dev
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
