On Wed, Aug 10, 2011 at 6:43 PM, Peter Hutterer
<peter.hutte...@who-t.net> wrote:
> 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.
The change of "preferred" backend was unintentional, so I have no
aversion to swapping the order. However, I'm not certain that would
address your concerns, because...
> 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.
That would also be the case if I swapped the checks. Unless you have a
really weird driver that exposes a head named "HEAD-0" through RandR,
checking it first would still fail and fallback to Xinerama.
> 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".
What is stopping users from making those kind of forum entries *right
now*? Users of nVidia's binary driver don't care why "HEAD-0" works --
they'll still suggest it to users of the binary driver, open-source
driver, heck even to users of ATI drivers! If something works for
them, they're going to suggest other users try it.
The difference is that that their advice would actually *work* with a
patch like this, which appears to be the crux of the isue.
> 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.
I don't see it that way myself. Maintaining compatibility with
Xinerama does diminish demand for proper RandR support, but that's
hardly nudging users away from the newer feature. The only thing that
denying the use of Xinerama to those with the extension does is reduce
the amount of out-of-date information on the internet when Xinerama
finally goes the way of the dodo. There's something to be said for
keeping old information relevant (especially if the confused mass of
sample Wacom xorg.conf files a few years back was any indication), but
I can't see the fallout from the eventual death of Xinerama having
that big of an impact on us.
Jason
---
Day xee-nee-svsh duu-'ushtlh-ts'it;
nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it.
Huu-chan xuu naa~-gha.
>> ---
>> 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
