On Thu, Sep 08, 2011 at 03:42:55PM -0700, Jason Gerecke wrote:
> On Wed, Sep 7, 2011 at 8:42 PM, Peter Hutterer <[email protected]>
> wrote:
> > On Wed, Sep 07, 2011 at 04:36:52PM -0700, Jason Gerecke wrote:
> >> This patch allows the MapToOutput command to accept fully-
> >> specified X geometry strings as valid output names. The
> >> XParseGeometry function describes these strings as having
> >> the folowing format:
> >
> > hehe, funny. didn't know that function even existed :)
> >
> >>
> >> [=][<width>{xX}<height>][{+-}<xoffset>{+-}<yoffset>]
> >>
> >> Strings with width, height, xoffset, and yoffset defined
> >> will be accepted and the device mapped to the rectangle
> >> it describes.
> >>
> >> This patch also renames the function '_set_matrix' to bring
> >> its naming in line with the other set_output_XXXX functions.
> >>
> >> Signed-off-by: Jason Gerecke <[email protected]>
> >> ---
> >> Changes from v2:
> >> * Uses an X geometry string (and the server's own parser)
> >> instead of hacking together my own format
> >>
> >> * 'set_output_area' was such a thin wrapper around '_set_matrix'
> >> that the latter has been renamed instead of adding a new
> >> function.
> >>
> >> * No more inversion of the logic for when to print success/failure
> >>
> >> man/xsetwacom.man | 17 +++++++++--------
> >> tools/xsetwacom.c | 36 ++++++++++++++++++++++++------------
> >> 2 files changed, 33 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/man/xsetwacom.man b/man/xsetwacom.man
> >> index 539f405..84c91e9 100644
> >> --- a/man/xsetwacom.man
> >> +++ b/man/xsetwacom.man
> >> @@ -120,16 +120,17 @@ device is unbound and will react to any tool of the
> >> matching type.
> >> Default: 0
> >> .TP
> >> \fBMapToOutput\fR [output]
> >> -Map the tablet's input area to the given output (e.g. "VGA1"). The output
> >> -must specify one of those available through the XRandR extension. A list
> >> of
> >> -outputs may be obtained with the xrandr tool. If no output is provided,
> >> -the tablet is mapped to the entire desktop. The output mapping
> >> -configuration is a onetime setting and does not track output
> >> +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 X11 geometry string
> >> of
> >> +the from WIDTHxHEIGHT+X+Y. Users of the NVIDIA binary driver should use
> >> the
> >> +output names "HEAD-0" and "HEAD-1" until proper XRandR support is
> >> included.
> >
> > "until the driver supports XRandR 1.2 or later"
> >
> >
> > should the geometry be supported by MapToArea instead? seems like a more
> > self-explanatory UI. The code can still be (mostly) the same, but exposing a
> > separate option seems worthwhile here.
> > makes error-handling much simpler too.
> >
> Dunno -- I'm pretty happy with the code right now, but there's
> definitely some merit to the idea. I'd be a little more
> self-explanatory too, but probably not enough to make much of a
> difference. I think the main upside would be a little cleaner
> implementation. Extending the thought further though, would desktop
> and relative mapping also be exposed via their own commands (e.g.
> MapToDesktop and/or MapToNext)? The obvious downside I see is
> "pollution" of the parameter namespace.
MapToDesktop I could get behind, don't quite agree with MapToNext. the
former is quite obvious, the latter isn't. MapToNext what? "MapToOutput
next" is better here.
Cheers,
Peter
> >> +
> >> +The output mapping configuration is a onetime setting and does not track
> >> output
> >> reconfigurations; the command needs to be re-run whenever the output
> >> configuration changes. When used with tablet rotation, the tablet must be
> >> -rotated before it is mapped to the new screen. When running the NVIDIA
> >> -binary driver, the output names are "HEAD-0" and "HEAD-1".
> >> -This parameter is write-only and cannot be queried.
> >> +rotated before it is mapped to the new screen. This parameter is
> >> write-only
> >> +and cannot be queried.
> >> .TP
> >> \fBMode\fR Absolute|Relative
> >> Set the device mode as either Relative or Absolute. Relative means pointer
> >> diff --git a/tools/xsetwacom.c b/tools/xsetwacom.c
> >> index 2f2e922..f887064 100644
> >> --- a/tools/xsetwacom.c
> >> +++ b/tools/xsetwacom.c
> >> @@ -40,6 +40,8 @@
> >> #include <X11/extensions/Xinerama.h>
> >> #include <X11/XKBlib.h>
> >>
> >> +#define MaskIsSet(bitfield, mask) !!(((bitfield) & (mask)) == (mask))
> >
> > don't we have the same macro in the driver? Can we move that into a shared
> > header file?
> >
> Looking into this, it may not be necessary to move the macro. The
> 'wacom-tests.c' file in the 'tests' directory manages to get it with
> an "#include <xf86Wacom.h>".
>
> >> +
> >> #define TRACE(...) \
> >> if (verbose) fprintf(stderr, "... " __VA_ARGS__)
> >>
> >> @@ -1995,12 +1997,13 @@ static void _set_matrix_prop(Display *dpy, XDevice
> >> *dev, const float fmatrix[9])
> >> }
> >>
> >> /**
> >> - * Set the matrix for the given device for the screen defined by offset
> >> and
> >> - * dimensions.
> >> + * Adjust the transformation matrix based on a user-defined area.
> >> + * This function will attempt to map the given pointer to an arbitrary
> >> + * rectangular portion of the desktop.
> >> */
> >> -static void _set_matrix(Display *dpy, XDevice *dev,
> >> +static void set_output_area(Display *dpy, XDevice *dev,
> >> int offset_x, int offset_y,
> >> - int screen_width, int screen_height)
> >> + int output_width, int output_height)
> >> {
> >> int width = DisplayWidth(dpy, DefaultScreen(dpy));
> >> int height = DisplayHeight(dpy, DefaultScreen(dpy));
> >> @@ -2010,8 +2013,8 @@ static void _set_matrix(Display *dpy, XDevice *dev,
> >> float y = 1.0 * offset_y/height;
> >>
> >> /* mapping */
> >> - float w = 1.0 * screen_width/width;
> >> - float h = 1.0 * screen_height/height;
> >> + float w = 1.0 * output_width/width;
> >> + float h = 1.0 * output_height/height;
> >>
> >> float matrix[9] = { 1, 0, 0,
> >> 0, 1, 0,
> >> @@ -2021,6 +2024,9 @@ static void _set_matrix(Display *dpy, XDevice *dev,
> >> matrix[0] = w;
> >> matrix[4] = h;
> >>
> >> + TRACE("Remapping to output area %dx%d @ %d,%d.\n", output_width,
> >> + output_height, offset_x, offset_y);
> >> +
> >> TRACE("Transformation matrix:\n");
> >> TRACE(" [ %f %f %f ]\n", matrix[0], matrix[1], matrix[2]);
> >> TRACE(" [ %f %f %f ]\n", matrix[3], matrix[4], matrix[5]);
> >> @@ -2074,7 +2080,7 @@ static void set_output_xrandr(Display *dpy, XDevice
> >> *dev, char *output_name)
> >> if (found)
> >> {
> >> TRACE("Setting CRTC %s\n", output_name);
> >> - _set_matrix(dpy, dev, x, y, width, height);
> >> + set_output_area(dpy, dev, x, y, width, height);
> >> } else
> >> printf("Unable to find output '%s'. "
> >> "Output may not be connected.\n", output_name);
> >> @@ -2118,7 +2124,7 @@ static void set_output_xinerama(Display *dpy,
> >> XDevice *dev, int head)
> >>
> >> TRACE("Setting xinerama head %d\n", head);
> >>
> >> - _set_matrix(dpy, dev,
> >> + set_output_area(dpy, dev,
> >> screens[head].x_org, screens[head].y_org,
> >> screens[head].width, screens[head].height);
> >>
> >> @@ -2128,7 +2134,8 @@ out:
> >>
> >> static void set_output(Display *dpy, XDevice *dev, param_t *param, int
> >> argc, char **argv)
> >> {
> >> - int tmp_int;
> >> + int tmp_int[2];
> >> + unsigned int tmp_uint[2];
> >
> > these should be variables that specify the actual names. width, height,
> > xoffset, yoffset.
> >
> > and a separate one for head_no, the compiler will squish them together for
> > us anyway I guess. and this isn't a particularly memory-critical path :)
> >
> >>
> >> if (argc == 0)
> >> {
> >> @@ -2145,13 +2152,18 @@ static void set_output(Display *dpy, XDevice *dev,
> >> param_t *param, int argc, cha
> >> return;
> >> }
> >>
> >> - if (!need_xinerama(dpy))
> >> + if (MaskIsSet(XParseGeometry(argv[0], &tmp_int[0], &tmp_int[1],
> >> &tmp_uint[0], &tmp_uint[1]),
> >> + XValue|YValue|WidthValue|HeightValue))
> >
> > urgh. split this up please into
> >
> > flags = XParseGeometry(...);
> > if (MaskIsSet(flags, XValue|...)) { ... }
> >
> >> + {
> >> + set_output_area(dpy, dev, tmp_int[0], tmp_int[1],
> >> tmp_uint[0], tmp_uint[1]);
> >> + }
> >
> > no {} please
> >
> > minor issues, the approach itself looks good, thanks.
> >
> > Cheers,
> > Peter
> >
> >> + else if (!need_xinerama(dpy))
> >> {
> >> set_output_xrandr(dpy, dev, argv[0]);
> >> }
> >> - else if (convert_value_from_user(param, argv[0], &tmp_int))
> >> + else if (convert_value_from_user(param, argv[0], &tmp_int[0]))
> >> {
> >> - set_output_xinerama(dpy, dev, tmp_int);
> >> + set_output_xinerama(dpy, dev, tmp_int[0]);
> >> }
> >> else
> >> {
> >> --
> >> 1.7.6
> >>
> >>
> >> ------------------------------------------------------------------------------
> >> Using storage to extend the benefits of virtualization and iSCSI
> >> Virtualization increases hardware utilization and delivers a new level of
> >> agility. Learn what those decisions are and how to modernize your storage
> >> and backup environments for virtualization.
> >> http://www.accelacomm.com/jaw/sfnl/114/51434361/
> >> _______________________________________________
> >> Linuxwacom-devel mailing list
> >> [email protected]
> >> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
> >>
> >
>
>
>
> Jason
>
> ---
> Day xee-nee-svsh duu-'ushtlh-ts'it;
> nuu-wee-ya' duu-xan' 'vm-nvshtlh-ts'it.
> Huu-chan xuu naa~-gha.
>
------------------------------------------------------------------------------
Why Cloud-Based Security and Archiving Make Sense
Osterman Research conducted this study that outlines how and why cloud
computing security and archiving is rapidly being adopted across the IT
space for its ease of implementation, lower cost, and increased
reliability. Learn more. http://www.accelacomm.com/jaw/sfnl/114/51425301/
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
