On Wed, Sep 7, 2011 at 8:35 PM, Peter Hutterer <peter.hutte...@who-t.net> wrote:
> On Wed, Sep 07, 2011 at 04:36:51PM -0700, Jason Gerecke wrote:
>> The 'set_output' function is now responsible for parsing the
>> command-line arguments to both determine the proper function
>> to call, as well as converting them into a form that can be
>> more easily used by the helper functions. This patch also
>> breaks the RandR compatibility check out into its own function
>> to improve readability.
>>
>> Signed-off-by: Jason Gerecke <killert...@gmail.com>
>> ---
>> Changes from v2:
>> * Helper functions no longer "intelligent" like earlier
>> patches tried to make them
>>
>> * Helper functions now take explicit arguments instead
>> of the remaining commandline string
>>
>> * Break-out Xinerama compatibility check into its own
>> function for readability
>>
>> tools/xsetwacom.c | 84
>> ++++++++++++++++++++++++++++++++++------------------
>> 1 files changed, 55 insertions(+), 29 deletions(-)
>>
>> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
>> index 7bc5b54..2f2e922 100644
>> --- a/tools/xsetwacom.c
>> +++ b/tools/xsetwacom.c
>> @@ -1930,6 +1930,32 @@ static void get_map(Display *dpy, XDevice *dev,
>> param_t *param, int argc, char**
>> }
>> }
>>
>> +/**
>> + * Determine if we need to use fall back to Xinerama, or if the RandR
>> + * extension will work OK. We depend on RandR 1.3 or better in order
>> + * to work.
>> + *
>> + * A server bug causes the NVIDIA driver to report RandR 1.3 support
>> + * despite not exposing RandR CRTCs. We need to fall back to Xinerama
>> + * for this case as well.
>> + */
>> +static Bool need_xinerama(Display *dpy)
>> +{
>> + int opcode, event, error;
>> + int maj, min;
>> +
>> + if (!XQueryExtension(dpy, "RANDR", &opcode, &event, &error) ||
>> + !XRRQueryVersion(dpy, &maj, &min) || (maj * 1000 + min) < 1002 ||
>> + XQueryExtension(dpy, "NV-CONTROL", &opcode, &event, &error))
>> + {
>> + TRACE("RandR extension not found, too old, or NV-CONTROL "
>> + "extension is also present.\n");
>> + return True;
>> + }
>> +
>> + return False;
>> +}
>> +
>> static void _set_matrix_prop(Display *dpy, XDevice *dev, const float
>> fmatrix[9])
>> {
>> Atom matrix_prop = XInternAtom(dpy, "Coordinate Transformation
>> Matrix", True);
>> @@ -2003,17 +2029,19 @@ static void _set_matrix(Display *dpy, XDevice *dev,
>> _set_matrix_prop(dpy, dev, matrix);
>> }
>>
>> -static void set_output_xrandr(Display *dpy, XDevice *dev, param_t *param,
>> int argc, char **argv)
>> +/**
>> + * Adjust the transformation matrix based on RandR settings. This function
>> + * will attempt to map the given device to the output with the given RandR
>> + * output name.
>> + */
>> +static void set_output_xrandr(Display *dpy, XDevice *dev, char *output_name)
>> {
>> int i, found = 0;
>> int x, y, width, height;
>> - char *output_name;
>> XRRScreenResources *res;
>> XRROutputInfo *output_info;
>> XRRCrtcInfo *crtc_info;
>>
>> - output_name = argv[0];
>> -
>> res = XRRGetScreenResources(dpy, DefaultRootWindow(dpy));
>> for (i = 0; i < res->noutput && !found; i++)
>> {
>> @@ -2054,17 +2082,20 @@ static void set_output_xrandr(Display *dpy, XDevice
>> *dev, param_t *param, int ar
>> }
>>
>> /**
>> - * Adjust the transformation matrix based on the Xinerama settings. For
>> - * TwinView This would better be done with libXNVCtrl but until they learn
>> - * to package it properly, rely on Xinerama. Besides, libXNVCtrl isn't
>> - * available on RHEL, so we'd have to do it through Xinerama there anyway.
>> + * Adjust the transformation matrix based on the Xinerama settings. This
>> + * function will attempt to map the given device to the specified Xinerama
>> + * head number.
>> + *
>> + * For TwinView This would better be done with libXNVCtrl but until they
>> + * learn to package it properly, we need to rely on Xinerama. Besides,
>> + * libXNVCtrl isn't available on RHEL, so we'd have to do it through
>> + * Xinerama there anyway.
>> */
>> -static void set_output_xinerama(Display *dpy, XDevice *dev, param_t *param,
>> int argc, char **argv)
>> +static void set_output_xinerama(Display *dpy, XDevice *dev, int head)
>> {
>> int event, error;
>> XineramaScreenInfo *screens;
>> int nscreens;
>> - int head;
>>
>> if (!XineramaQueryExtension(dpy, &event, &error))
>> {
>> @@ -2072,13 +2103,6 @@ static void set_output_xinerama(Display *dpy, XDevice
>> *dev, param_t *param, int
>> return;
>> }
>>
>> - if (!convert_value_from_user(param, argv[0], &head))
>> - {
>> - fprintf(stderr, "Please specify the output name as HEAD-X,"
>> - "where X is the screen number\n");
>> - return;
>> - }
>> -
>> screens = XineramaQueryScreens(dpy, &nscreens);
>>
>> if (nscreens == 0)
>> @@ -2087,8 +2111,8 @@ static void set_output_xinerama(Display *dpy, XDevice
>> *dev, param_t *param, int
>> goto out;
>> } else if (nscreens <= head)
>> {
>> - fprintf(stderr, "Found %d screens, but you requested %s.\n",
>> - nscreens, argv[0]);
>> + fprintf(stderr, "Found %d screens, but you requested number
>> %d.\n",
>> + nscreens, head);
>> goto out;
>> }
>>
>> @@ -2104,8 +2128,7 @@ out:
>>
>> static void set_output(Display *dpy, XDevice *dev, param_t *param, int
>> argc, char **argv)
>> {
>> - int opcode, event, error;
>> - int maj, min;
>> + int tmp_int;
>
> tmp_int suggests that this value isn't used. It is, afaict. Mabye change it
> to head_no, that seems to be what the value represents.
>
The value of the variables in this and the next patch only have
meaning if the associated else-block is entered. While you could in
theory use the variables again later by verifying the necessary
preconditions again, getting it right would be error-prone. Ideally
you'd *only* use the variable within the else-block where it has
meaning. Unfortunately I can't just declare the variables local to
that scope since the functions doing the checks
(convert_value_from_user here, and XParseGeometry in the next patch)
rely on the variables being present. Since I can't force the compiler
to complain when you start doing dangerous things, I'd like to ensure
the next person to touch this bit of code thinks twice before using
the variables.
>>
>> if (argc == 0)
>> {
>> @@ -2122,15 +2145,18 @@ static void set_output(Display *dpy, XDevice *dev,
>> param_t *param, int argc, cha
>> return;
>> }
>>
>> - /* Check for RandR 1.2. Server bug causes the NVIDIA driver to
>> - * report with RandR 1.3 support but it doesn't expose RandR CRTCs.
>> - * Force Xinerama if NV-CONTROL is present */
>> - if (XQueryExtension(dpy, "NV-CONTROL", &opcode, &event, &error) ||
>> - !XQueryExtension(dpy, "RANDR", &opcode, &event, &error) ||
>> - !XRRQueryVersion(dpy, &maj, &min) || (maj * 1000 + min) < 1002)
>> - set_output_xinerama(dpy, dev, param, argc, argv);
>> + if (!need_xinerama(dpy))
>> + {
>> + set_output_xrandr(dpy, dev, argv[0]);
>> + }
>> + else if (convert_value_from_user(param, argv[0], &tmp_int))
>> + {
>> + set_output_xinerama(dpy, dev, tmp_int);
>> + }
>> else
>> - set_output_xrandr(dpy, dev, param, argc, argv);
>> + {
>> + fprintf(stderr, "Unable to find an output '%s'.\n", argv[0]);
>> + }
>
> we usually don't use {} for single-line blocks.
Things started to look cluttered to me without them, but I'll swap them in v4.
>
> anyway, with those two fixed: Reviewed-by: Peter Hutterer
> <peter.hutte...@who-t.net>
>
> Cheers,
> Peter
>
Jason
---
Day xee-nee-svsh duu-'ushtlh-ts'it;
nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it.
Huu-chan xuu naa~-gha.
------------------------------------------------------------------------------
Doing More with Less: The Next Generation Virtual Desktop
What are the key obstacles that have prevented many mid-market businesses
from deploying virtual desktops? How do next-generation virtual desktops
provide companies an easier-to-deploy, easier-to-manage and more affordable
virtual desktop model.http://www.accelacomm.com/jaw/sfnl/114/51426474/
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
