On Mon, Aug 22, 2011 at 11:36:43AM -0700, Jason Gerecke wrote:
> On Sun, Aug 21, 2011 at 11:39 PM, Peter Hutterer
> <peter.hutte...@who-t.net> wrote:
> >> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
> >> index dd0a3e5..ffb2875 100644
> >> --- a/tools/xsetwacom.c
> >> +++ b/tools/xsetwacom.c
> >> @@ -2003,8 +2003,15 @@ 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 named in the first
> >> + * command-line argument. If the function succeeds, it returns 'true'.
> >> + */
> >> +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;
> >> int x, y, width, height;
> >> char *output_name;
> >> @@ -2012,6 +2019,21 @@ static void set_output_xrandr(Display *dpy, XDevice
> >> *dev, param_t *param, int ar
> >> 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))
> >
> > this blurb is used twice, just inverted. would be easier to just define a
> > small function need_xrandr() and pick based on that.
> >
> That's probably a good idea. Its need may be negated if we can get a
> parser to work (since only the parser would need to know that info,
> given our restricted syntax of Xinerama heads), but I'll keep it in
> mind.
this part doesn't even have to do any parsing, you just wrap the above
condition into a function and then call that function from
set_output_xinerama and set_output_xrandr.
> >> @@ -2123,14 +2169,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);
> >
> > and now that I've read the other patches too, I think this approach may
> > simply not be ideally suited. Why don't you split argument parsing into a
> > separate function and then switch based on the return code. I think this
> > would fit well with the goal. I'm thinking of something similar to
> > convert_value_from_user, returning the type of output so that the set_output
> > logic can be:
> >
> > enum OutputType output_type;
> > char *output_name;
> >
> > output_type = convert_output_name(argc, argv, &output_name);
> > switch(output_type) {
> > case OUTPUT_TYPE_XINERAMA: set_output_xinerama(...) ...
> > case OUTPUT_TYPE_XRANDR: set_output_xrandr(...) ...
> > case OUTPUT_TYPE_INVALID: show error ...
> > }
> >
> > the output parsing itself doesn't need to check for validity either, that
> > can then be done in the respective handlers
> >
> I considered using a parser (it'd be cleaner, more logical, and fix
> the issue of whether its better to TRACE or printf multiple functions
> failing), but was having some difficulty coming up with something that
> would handle the empty-string used for desktop mapping. I'll do some
> more thinking... I'll probably just have to deny "true" and "false"
> from being valid XRandR output names.
I'm not sure how the randr output names are defined, but i very much doubt
that true/false will ever be an output name :)
either way, just return a special string "_desktop" (with matching define)
or so as output_name and then the logic above should just work, right?
Cheers,
Peter
------------------------------------------------------------------------------
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
