On Tue, Aug 09, 2011 at 05:50:11PM -0700, Jason Gerecke wrote: > Instead of having 'set_output' be responsible for choosing an > appropriate helper function, we let the helper functions be > autonomous. If they cannot find an appropriate output (or > encounter an error), they simply return False to let the > caller know that it failed.
NAK. this changes the logic from RandR preferred to Xinerama preferred. In the original code, we only use xinerama for old servers or the nvidia blob. ok, let's be honest. it's only because of the nvidia blob. in this code, you always call into the xinerama code first. I don't think there are any drivers that support randr but not xinerama (the server forces it) so any output specification in the form of "HEAD-X" will always work. Why is this a problem? if HEAD-0 always works but the correct randr notation VGA0 (etc.) only work on some machines, users will start using the one that always works. so you get forum entries sprinkled everywhere with "I don't know why but HEAD-0 works for me, try that". the _only_ reason for supporting xinerama is the nvidia blob. So because of one driver, we end up nudging user towards a feature that's 11 year old instead of the newer feature. so, no, I don't think that's a good idea. Cheers, Peter * where "new" is defined as 4 years old! > --- > tools/xsetwacom.c | 46 +++++++++++++++++++++++++++++++--------------- > 1 files changed, 31 insertions(+), 15 deletions(-) > > diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c > index 310fb30..8c5178f 100644 > --- a/tools/xsetwacom.c > +++ b/tools/xsetwacom.c > @@ -2003,14 +2003,31 @@ 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) > +static Bool set_output_xrandr(Display *dpy, XDevice *dev, param_t *param, > int argc, char **argv) > { > + int opcode, event, error; > + int maj, min; > int i, found = 0; > char *output_name; > XRRScreenResources *res; > XRROutputInfo *output_info; > XRRCrtcInfo *crtc_info; > > + if (!XQueryExtension(dpy, "RANDR", &opcode, &event, &error) || > + !XRRQueryVersion(dpy, &maj, &min) || (maj * 1000 + min) < 1002 || > + XQueryExtension(dpy, "NV-CONTROL", &opcode, &event, &error)) > + { > + /* RandR not present or earlier than version 1.2 > + * > + * Server bug causes the NVIDIA driver to report RandR 1.3 > + * support but it doesn't expose RandR CRTCs; ignore if > + * this is the case. > + */ > + TRACE("RandR extension not found, too old, or NV-CONTROL " > + "extension is also present.\n"); > + return False; > + } > + > output_name = argv[0]; > > res = XRRGetScreenResources(dpy, DefaultRootWindow(dpy)); > @@ -2042,10 +2059,15 @@ static void set_output_xrandr(Display *dpy, XDevice > *dev, param_t *param, int ar > TRACE("Setting CRTC %s\n", output_name); > _set_matrix(dpy, dev, crtc_info->x, crtc_info->y, > crtc_info->width, crtc_info->height); > + > + return True; > } else > + { > printf("Unable to find output '%s'. " > "Output may not be connected.\n", output_name); > > + return False; > + } > } > > /** > @@ -2054,24 +2076,25 @@ static void set_output_xrandr(Display *dpy, XDevice > *dev, param_t *param, int ar > * 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. > */ > -static void set_output_xinerama(Display *dpy, XDevice *dev, param_t *param, > int argc, char **argv) > +static Bool set_output_xinerama(Display *dpy, XDevice *dev, param_t *param, > int argc, char **argv) > { > int event, error; > XineramaScreenInfo *screens; > int nscreens; > int head; > + Bool success = False; > > if (!XineramaQueryExtension(dpy, &event, &error)) > { > fprintf(stderr, "Unable to set screen mapping. Xinerama > extension not found\n"); > - return; > + return False; > } > > 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; > + return False; > } > > screens = XineramaQueryScreens(dpy, &nscreens); > @@ -2093,15 +2116,15 @@ static void set_output_xinerama(Display *dpy, XDevice > *dev, param_t *param, int > screens[head].x_org, screens[head].y_org, > screens[head].width, screens[head].height); > > + success = True; > + > out: > XFree(screens); > + return success; > } > > static void set_output(Display *dpy, XDevice *dev, param_t *param, int argc, > char **argv) > { > - int opcode, event, error; > - int maj, min; > - > if (argc == 0) > { > float matrix[9] = { 1, 0, 0, > @@ -2117,14 +2140,7 @@ 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); > - else > + if (!set_output_xinerama(dpy, dev, param, argc, argv)) > set_output_xrandr(dpy, dev, param, argc, argv); > } > > -- > 1.7.5.2 ------------------------------------------------------------------------------ Get a FREE DOWNLOAD! and learn more about uberSVN rich system, user administration capabilities and model configuration. Take the hassle out of deploying and managing Subversion and the tools developers use with it. http://p.sf.net/sfu/wandisco-dev2dev _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel