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.
>> +
>> +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.
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
